mac80211: discard multicast and 4-addr A-MSDUs

Commit Message

Johannes Berg Oct. 5, 2016, 8:19 a.m. UTC

From: Johannes Berg <[e-mail]>

In mac80211, multicast A-MSDUs are accepted in many cases that
they shouldn't be accepted in:
 * drop A-MSDUs with a multicast A1 (RA), as required by the
   spec in 9.11 (802.11-2012 version)
 * drop A-MSDUs with a 4-addr header, since the fourth address
   can't actually be useful for them; this was already done in
   the case where 4-addr behaviour was requested, but bizarrely
   not in the common cases

Accepting the first case, in particular, is very problematic
since it allows anyone else with possession of a GTK to send
unicast frames encapsulated in a multicast A-MSDU, even when
the AP has client isolation enabled.

Cc: [e-mail]
Signed-off-by: Johannes Berg <[e-mail]>
---
 net/mac80211/rx.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Comments

Felix Fietkau Oct. 5, 2016, 9:31 a.m. UTC | #1

On 2016-10-05 10:19, Johannes Berg wrote:
> From: Johannes Berg <[e-mail]>
> 
> In mac80211, multicast A-MSDUs are accepted in many cases that
> they shouldn't be accepted in:
>  * drop A-MSDUs with a multicast A1 (RA), as required by the
>    spec in 9.11 (802.11-2012 version)
>  * drop A-MSDUs with a 4-addr header, since the fourth address
>    can't actually be useful for them; this was already done in
>    the case where 4-addr behaviour was requested, but bizarrely
>    not in the common cases
Won't this break the use of A-MSDU in existing 4-addr AP/STA setups?

- Felix

Johannes Berg Oct. 5, 2016, 9:38 a.m. UTC | #2

> Won't this break the use of A-MSDU in existing 4-addr AP/STA setups?

I didn't think it did, but looking closer, that does seem indeed to be
the case.

Do you remember why you explicitly added code to *not* accept 4-addr
frames in non-4addr AP_VLAN, but no other cases? This seems oddly
specific.

I can change it to accept 4-addr frames in 4-addr cases, but I'll note
that it's completely pointless to carry A4 since it will not be used
for decapsulation.

johannes

Johannes Berg Oct. 5, 2016, 9:39 a.m. UTC | #3

> Do you remember why you explicitly added code to *not* accept 4-addr
> frames in non-4addr AP_VLAN, but no other cases? This seems oddly
> specific.
> 

Oh, maybe that's because more checks are done
in __ieee80211_data_to_8023().

johannes

Patch

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 6175db385ba7..31aadc769021 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2308,16 +2308,10 @@  ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
 	if (!(status->rx_flags & IEEE80211_RX_AMSDU))
 		return RX_CONTINUE;
 
-	if (ieee80211_has_a4(hdr->frame_control) &&
-	    rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-	    !rx->sdata->u.vlan.sta)
+	if (ieee80211_has_a4(hdr->frame_control))
 		return RX_DROP_UNUSABLE;
 
-	if (is_multicast_ether_addr(hdr->addr1) &&
-	    ((rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
-	      rx->sdata->u.vlan.sta) ||
-	     (rx->sdata->vif.type == NL80211_IFTYPE_STATION &&
-	      rx->sdata->u.mgd.use_4addr)))
+	if (is_multicast_ether_addr(hdr->addr1))
 		return RX_DROP_UNUSABLE;
 
 	skb->dev = dev;