2020-06-17 08:30:36

by Markus Theil

[permalink] [raw]
Subject: [PATCH v3 0/2] Fixes for 802.1X in mesh mode

In order to support 802.1X in mesh mode, userspace should be able
to rx and tx eapol frames, after an initial MPM, before a second AMPE.

Some checks regarding mesh forwarding were conflicting with this.
Therefore this little series adds some checks for control port frames
and disables mesh path lookups on the tx side or drop of unencrypted
frames on the rx side.

v3: continue with previous drop check, if eapol check fails

v2: fixes for rx path with packet sockets

Markus Theil (2):
mac80211: skip mpath lookup also for control port tx
mac80211: allow rx of mesh eapol frames with default rx key

net/mac80211/rx.c | 26 ++++++++++++++++++++++++++
net/mac80211/tx.c | 13 ++++++++++---
2 files changed, 36 insertions(+), 3 deletions(-)

--
2.27.0


2020-06-17 08:30:36

by Markus Theil

[permalink] [raw]
Subject: [PATCH v3 2/2] mac80211: allow rx of mesh eapol frames with default rx key

Without this patch, eapol frames cannot be received in mesh
mode, when 802.1X should be used. Initially only a MGTK is
defined, which is found and set as rx->key, when there are
no other keys set. ieee80211_drop_unencrypted would then
drop these eapol frames, as they are data frames without
encryption and there exists some rx->key.

Fix this by differentiating between mesh eapol frames and
other data frames with existing rx->key. Allow mesh mesh
eapol frames only if they are for our vif address. If a
check for eapol frames fails, the previous drop check is
executed (thanks to a kernel test bot, which found an issue
in a previous patch version here).

With this patch in-place, ieee80211_rx_h_mesh_fwding continues
after the ieee80211_drop_unencrypted check and notices, that
these eapol frames have to be delivered locally, as they should.

Reported-by: kernel test robot <[email protected]>
Signed-off-by: Markus Theil <[email protected]>
---
net/mac80211/rx.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 21854a61a2b7..60293a9fa3dc 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2396,6 +2396,7 @@ static int ieee80211_802_1x_port_control(struct ieee80211_rx_data *rx)

static int ieee80211_drop_unencrypted(struct ieee80211_rx_data *rx, __le16 fc)
{
+ struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)rx->skb->data;
struct sk_buff *skb = rx->skb;
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);

@@ -2406,12 +2407,37 @@ static int ieee80211_drop_unencrypted(struct ieee80211_rx_data *rx, __le16 fc)
if (status->flag & RX_FLAG_DECRYPTED)
return 0;

+ /* check mesh EAPOL frames first */
+ if (unlikely(rx->sta && ieee80211_vif_is_mesh(&rx->sdata->vif) && ieee80211_is_data(fc))) {
+ struct ieee80211s_hdr *mesh_hdr;
+ u16 hdr_len = ieee80211_hdrlen(fc);
+ u16 ethertype_offset;
+ __be16 ethertype;
+
+ /* make sure fixed part of mesh header is there, also checks skb len */
+ if (!pskb_may_pull(rx->skb, hdr_len + 6))
+ goto drop_check;
+
+ mesh_hdr = (struct ieee80211s_hdr *)(skb->data + hdr_len);
+ ethertype_offset = hdr_len + ieee80211_get_mesh_hdrlen(mesh_hdr)
+ + sizeof(rfc1042_header);
+ if (!pskb_may_pull(rx->skb, ethertype_offset + sizeof(ethertype)) ||
+ !ether_addr_equal(hdr->addr1, rx->sdata->vif.addr))
+ goto drop_check;
+
+ skb_copy_bits(rx->skb, ethertype_offset, &ethertype, 2);
+ if (ethertype == rx->sdata->control_port_protocol)
+ goto pass_frame;
+ }
+
+drop_check:
/* Drop unencrypted frames if key is set. */
if (unlikely(!ieee80211_has_protected(fc) &&
!ieee80211_is_any_nullfunc(fc) &&
ieee80211_is_data(fc) && rx->key))
return -EACCES;

+pass_frame:
return 0;
}

--
2.27.0

2020-06-25 09:53:45

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] mac80211: allow rx of mesh eapol frames with default rx key


> + /* check mesh EAPOL frames first */
> + if (unlikely(rx->sta && ieee80211_vif_is_mesh(&rx->sdata->vif) && ieee80211_is_data(fc))) {
> + struct ieee80211s_hdr *mesh_hdr;
> + u16 hdr_len = ieee80211_hdrlen(fc);
> + u16 ethertype_offset;
> + __be16 ethertype;
> +
> + /* make sure fixed part of mesh header is there, also checks skb len */
> + if (!pskb_may_pull(rx->skb, hdr_len + 6))
> + goto drop_check;
> +
> + mesh_hdr = (struct ieee80211s_hdr *)(skb->data + hdr_len);
> + ethertype_offset = hdr_len + ieee80211_get_mesh_hdrlen(mesh_hdr)
> + + sizeof(rfc1042_header);
> + if (!pskb_may_pull(rx->skb, ethertype_offset + sizeof(ethertype)) ||
> + !ether_addr_equal(hdr->addr1, rx->sdata->vif.addr))

might be nicer to check the address first, pskb_may_pull() is
potentially more expensive, and you should be able to check the header
address already before even the first pskb_may_pull() here since it's in
the normal header.

But I don't understand the second pskb_may_pull() anyway, because you
use skb_copy_bits() below?


> + skb_copy_bits(rx->skb, ethertype_offset, &ethertype, 2);
> + if (ethertype == rx->sdata->control_port_protocol)
> + goto pass_frame;

can return 0 here immediately and save the label.

johannes