Re: [PATCH] add support for parsing WPA and RSN/WPA2 information elements

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 2580 bytes --]

Hi,

> > > +static unsigned char vendor_oui[3] = { 0x00, 0x50, 0xf2 };
> > > +static unsigned char cipher_oui[3] = { 0x00, 0x0f, 0xac };
> > 
> > ??
> > 00-50-f2 is "WiFi OUI" (registered to Microsoft), 00-0f-ac is "802.11
> > OUI", registered to 802.11.
> 
> I can rename them if that helps.

Yes, please do, but also synchronise the things you print out. _All_
cipher specs are effectively vendor-specified, but some use vendor
"WiFi" and some use vendor "802.11" so are standardised in some form. I
don't think printing "Vendor specified: ..." for the WiFi OUI or "Other"
for "real" vendor-specified ones helps.

> > Why are you passing in the OUI?
> 
> The WPA1 and WPA2 IE are uses a different OUI for basically exactly the
> same thing.

Yeah, I noticed later.

> > > +	if (len < 4) {
> > > +		tab_on_first(&first);
> > > +		printf("\t * Group cipher: TKIP\n");
> > > +		printf("\t * Pairwise ciphers: TKIP\n");
> > > +		return;
> > > +	}
> > 
> > Huh? I don't quite understand this? Is that some backward compat code?
> > Or is this some WPA1 thing I don't know about?
> 
> The specification says that the only mandatory field is the version and
> after that everything else is optional and falls back to default
> TKIP/TKIP. At least that is what I read of it.

Ok, makes sense I guess.

> > > +static void print_rsn(unsigned char type, unsigned char len, unsigned char *data)
> > > +{
> > > +	print_wpa("WPA2", cipher_oui, len, data);
> > > +}
> > 
> > That's "oui_80211" I guess, not "cipher_oui". Ok I see now why you want
> > to pass in the OUI... However, it would be better to just duplicate the
> > code, I think for example 11w won't be announced in WPA1 IEs so we
> > shouldn't parse it there when we add support for parsing it to RSN IEs.
> 
> Since iw is just printing the actual IE, I don't think we should be
> bothering here with code duplication. We can just print what the element
> actually contains. If for some weird fucked up AP, has 11w inside WPA1,
> then I actually wanna have iw print that :)

No, this is the wrong approach. See, there are two defined cipher suites
for, say, CCMP:
00:50:f2-4 and 00:0f:ac-4

However, it is not necessarily true that 00:50:f2-N is _always_ the same
as 00:0f:ac-N. 11w adds 00:0f:ac-6 (AES-128-CMAC, you could add that to
your patch), but 00:50:f2-6 stays undefined since the WiFi spec defines
that one, not the 802.11 spec. The WiFi spec could very well define
00:50:f2-6 as "quantum cryptography mode reserved for future" if it
wishes.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]