[PATCH v3 0/2] Fixes for 802.1X in mesh mode

[PATCH v3 0/2] Fixes for 802.1X in mesh mode

[Markus Theil]

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

[PATCH v3 1/2] mac80211: skip mpath lookup also for control port tx

[Markus Theil]

When using 802.1X over mesh networks, at first an ordinary
mesh peering is established, then the 802.1X EAPOL dialog
happens, afterwards an authenticated mesh peering exchange
(AMPE) happens, finally the peering is complete and we can
set the STA authorized flag.

As 802.1X is an intermediate step here and key material is
not yet exchanged for stations we have to skip mesh path lookup
for these EAPOL frames. Otherwise the already configure mesh
group encryption key would be used to send a mesh path request
which no one can decipher, because we didn't already establish
key material on both peers, like with SAE and directly using AMPE.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 net/mac80211/tx.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index e9ce658141f5..bd86f85a5b73 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3933,6 +3933,7 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
 	struct ieee80211_local *local = sdata->local;
 	struct sta_info *sta;
 	struct sk_buff *next;
+	u32 ctrl_flags_adapted;
 
 	if (unlikely(skb->len < ETH_HLEN)) {
 		kfree_skb(skb);
@@ -3996,8 +3997,12 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
 	skb_list_walk_safe(skb, skb, next) {
 		skb_mark_not_on_list(skb);
 
+		ctrl_flags_adapted = ctrl_flags;
+		if (sdata->control_port_protocol == skb->protocol)
+			ctrl_flags_adapted |= IEEE80211_TX_CTRL_SKIP_MPATH_LOOKUP;
+
 		skb = ieee80211_build_hdr(sdata, skb, info_flags,
-					  sta, ctrl_flags, cookie);
+					  sta, ctrl_flags_adapted, cookie);
 		if (IS_ERR(skb)) {
 			kfree_skb_list(next);
 			goto out;
@@ -5370,8 +5375,10 @@ int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev,
 	    proto != cpu_to_be16(ETH_P_PREAUTH))
 		return -EINVAL;
 
-	if (proto == sdata->control_port_protocol)
-		ctrl_flags |= IEEE80211_TX_CTRL_PORT_CTRL_PROTO;
+	if (proto == sdata->control_port_protocol) {
+		ctrl_flags |= IEEE80211_TX_CTRL_PORT_CTRL_PROTO |
+			      IEEE80211_TX_CTRL_SKIP_MPATH_LOOKUP;
+	}
 
 	if (unencrypted)
 		flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT;
-- 
2.27.0

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

[Markus Theil]

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 <rong.a.chen@intel.com>
Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 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

Re: [PATCH v3 1/2] mac80211: skip mpath lookup also for control port tx

[Johannes Berg]

Applied (at least tentatively, haven't run all the tests yet), but

> @@ -3933,6 +3933,7 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
>  	struct ieee80211_local *local = sdata->local;
>  	struct sta_info *sta;
>  	struct sk_buff *next;
> +	u32 ctrl_flags_adapted;

I removed this - the loop can only process fragments of the same frame
that should all be handled the same.
 
> -	if (proto == sdata->control_port_protocol)
> -		ctrl_flags |= IEEE80211_TX_CTRL_PORT_CTRL_PROTO;
> +	if (proto == sdata->control_port_protocol) {
> +		ctrl_flags |= IEEE80211_TX_CTRL_PORT_CTRL_PROTO |
> +			      IEEE80211_TX_CTRL_SKIP_MPATH_LOOKUP;
> +	}

And this doesn't need braces.

johannes

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

[Johannes Berg]

> +	/* 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