Re: [PATCHv3] wireless: check A-MSDU inner frame source address on AP interfaces

[Johannes Berg <johannes@sipsolutions.net>]

> > First of all - there's nothing specific to "AP interfaces", which
> > you say in the subject, as far as I can tell? That should be
> > removed?
> 
> > 
> > > 
> > > if (unlikely(ta &&
> > > +			     (iftype == NL80211_IFTYPE_AP ||
> > > +			      iftype == NL80211_IFTYPE_AP_VLAN)
> > > &&
> > > +			     !ether_addr_equal(ta, eth.h_source)
> > > +		   ))
> > > +			goto purge;
> 
> So the A-MSDU packets are only dropped if received by an interface in
> AP or AP_VLAN mode, not on client side, as my original issue was
> about arp/ip filters being circumvented on AP side.

D'oh. Not paying attention, I guess.

Nevertheless, why should this be conditional on the interface type? It
seems to me that we should apply this to any type? What if, for
example, another BSS client sends an A-MSDU encrypted with the GTK and
then fakes something inside that way? We verify today that only
multicast frames can be encrypted with the GTK, but that applies to the
outer header, so we're susceptible to a variant of the hole-196 attack,
afaict?

> > > IEEE 802.11-2012 mandates that the outer source mac address
> > > should match the inner source address (section 8.3.2.2). For the
> > > destination mac address, matching is not required (section
> > > 10.23.15).
> > 
> > I think this is wrong. As we do not support DMS (yet), we should
> > adhere to 8.3.2.2 and only accept matching TA/SA and DA/RA.
> 
> IEEE 802.11-2012 8.3.2.2 contains the note "NOTE—It is possible to
> have different DA and SA parameter values in A-MSDU subframe headers
> of the same A-MSDU as long as they all map to the same Address 1 and
> Address 2 parameter values."
> 
> I conclude that embedding multicast in unicast A-MSDU frames is
> generally allowed, because "mapping" does not mean "be identical".

Yeah, I saw this. It's not clear to me that they intended this wording
to be about multicast though. I'm not really sure what they had in mind
here, but there's an exception for multicast for DMS, which would seem
pointless if they had intended this "mapping" to be about multicast.

Then again, I don't know of any "address mapping" service or the like
in the spec either.

johannes