[PATCH] add basic support for parsing country information element

[PATCH] add basic support for parsing country information element

[Marcel Holtmann]

---
 scan.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/scan.c b/scan.c
index 2976115..c503487 100644
--- a/scan.c
+++ b/scan.c
@@ -91,6 +91,16 @@ static void print_ign(unsigned char type, unsigned char len, unsigned char *data
 	/* ignore for now, not too useful */
 }
 
+static void print_country(unsigned char type, unsigned char len, unsigned char *data)
+{
+	int i;
+
+	printf("\tCountry: %.*s data:", 3, data);
+	for(i=0; i<len-3; i++)
+		printf(" %.02x", data[i + 3]);
+	printf("\n");
+}
+
 static void print_erp(unsigned char type, unsigned char len, unsigned char *data)
 {
 	if (data[0] == 0x00)
@@ -245,6 +255,7 @@ static const printfn ieprinters[] = {
 	[1] = print_supprates,
 	[3] = print_ds,
 	[5] = print_ign,
+	[7] = print_country,
 	[42] = print_erp,
 	[48] = print_rsn,
 	[50] = print_supprates,
-- 
1.6.0.6

Re: [PATCH] add basic support for parsing country information element

[Johannes Berg]

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

On Sun, 2009-05-03 at 22:09 -0700, Marcel Holtmann wrote:
> ---
>  scan.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/scan.c b/scan.c
> index 2976115..c503487 100644
> --- a/scan.c
> +++ b/scan.c
> @@ -91,6 +91,16 @@ static void print_ign(unsigned char type, unsigned char len, unsigned char *data
>  	/* ignore for now, not too useful */
>  }
>  
> +static void print_country(unsigned char type, unsigned char len, unsigned char *data)
> +{
> +	int i;
> +
> +	printf("\tCountry: %.*s data:", 3, data);

IIRC the third characters there is a space if it's in- and outdoor, it
might make sense to parse it instead of just printing it?

johannes

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

Re: [PATCH] add basic support for parsing country information element

[Johannes Berg]

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

Everything else looks good.

johannes

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

Re: [PATCH] add basic support for parsing country information element

[Marcel Holtmann]

Hi Johannes,

> >  
> > +static void print_country(unsigned char type, unsigned char len, unsigned char *data)
> > +{
> > +	int i;
> > +
> > +	printf("\tCountry: %.*s data:", 3, data);
> 
> IIRC the third characters there is a space if it's in- and outdoor, it
> might make sense to parse it instead of just printing it?

that explains why I have seen USI here and couldn't make sense of it and
just declared that AP broken :)

Regards

Marcel

Re: [PATCH] add basic support for parsing country information element

[Johannes Berg]

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


> > IIRC the third characters there is a space if it's in- and outdoor, it
> > might make sense to parse it instead of just printing it?
> 
> that explains why I have seen USI here and couldn't make sense of it and
> just declared that AP broken :)

Heh, no, I think only "I", "O" and " " are valid there as the third
character :)

johannes

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

Re: [PATCH] add basic support for parsing country information element

[Marcel Holtmann]

Hi Johannes,

> Everything else looks good.

I re-sent the patches now in a different order so that the WPA stuff is
the latest in case you want extra fixes. I didn't include all defined
ciphers since I would have to dig though specs again. However it should
now be pretty easy to extend it since the basic parsing is available.

This bring iw scan at the same stage what we had with iwlist and so
another piece to deprecate WEXT.

Regards

Marcel

Re: [PATCH] add basic support for parsing country information element

[Marcel Holtmann]

Hi Johannes,

> > > IIRC the third characters there is a space if it's in- and outdoor, it
> > > might make sense to parse it instead of just printing it?
> > 
> > that explains why I have seen USI here and couldn't make sense of it and
> > just declared that AP broken :)
> 
> Heh, no, I think only "I", "O" and " " are valid there as the third
> character :)

added decoding for I and O to it :)

Regards

Marcel

Re: [PATCH] add basic support for parsing country information element

[Johannes Berg]

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

Hi Marcel,

> I re-sent the patches now in a different order so that the WPA stuff is
> the latest in case you want extra fixes. I didn't include all defined
> ciphers since I would have to dig though specs again. However it should
> now be pretty easy to extend it since the basic parsing is available.

I've committed everything, and am doing some minor cleanups. I also
noticed we don't validate everything as good as we should so I'm adding
some support for that.

johannes

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

Re: [PATCH] add basic support for parsing country information element

[Johannes Berg]

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

Marcel,

> I've committed everything, and am doing some minor cleanups. I also
> noticed we don't validate everything as good as we should so I'm adding
> some support for that.

I've done that now, and pushed it out, but I think the RSN IE printing
you added (and I renamed to print_rsn_ie) is missing some input length
validation, could you take a look at that please? In particular it takes
the count from the IE, but doesn't verify it fits, I think?

johannes

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

Re: [PATCH] add basic support for parsing country information element

[Marcel Holtmann]

Hi Johannes,

> > I've committed everything, and am doing some minor cleanups. I also
> > noticed we don't validate everything as good as we should so I'm adding
> > some support for that.
> 
> I've done that now, and pushed it out, but I think the RSN IE printing
> you added (and I renamed to print_rsn_ie) is missing some input length
> validation, could you take a look at that please? In particular it takes
> the count from the IE, but doesn't verify it fits, I think?

I can have a look at it since it is by no means perfect, but at least we
know have something that allows us to not bother with iwlist.

Btw. I named it WPA2 on purpose since not everybody using iw will
understand that it is actually named RSN. In the end it is your call,
just to let you know why I did it. One of my first patches actually did
use RSN in it :)

Regards

Marcel

Re: [PATCH] add basic support for parsing country information element

[Johannes Berg]

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

Hi,

> > I've done that now, and pushed it out, but I think the RSN IE printing
> > you added (and I renamed to print_rsn_ie) is missing some input length
> > validation, could you take a look at that please? In particular it takes
> > the count from the IE, but doesn't verify it fits, I think?
> 
> I can have a look at it since it is by no means perfect, but at least we
> know have something that allows us to not bother with iwlist.

Certainly. I just don't want to have it crash on an AP that advertises
completely bogus information :)

> Btw. I named it WPA2 on purpose since not everybody using iw will
> understand that it is actually named RSN. In the end it is your call,
> just to let you know why I did it. One of my first patches actually did
> use RSN in it :)

:)
I was going to call it RSN/WPA2 but that was too long for nice
displaying. Now that I think about it though, I think RSN is more
correct because WPA2 certification doesn't include any 11w things which
this will include. I suspect something like WPA3 will be done for that.
Who knows :)

johannes

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

Re: [PATCH] add basic support for parsing country information element

[Marcel Holtmann]

Hi Johannes,

> > > I've done that now, and pushed it out, but I think the RSN IE printing
> > > you added (and I renamed to print_rsn_ie) is missing some input length
> > > validation, could you take a look at that please? In particular it takes
> > > the count from the IE, but doesn't verify it fits, I think?
> > 
> > I can have a look at it since it is by no means perfect, but at least we
> > know have something that allows us to not bother with iwlist.
> 
> Certainly. I just don't want to have it crash on an AP that advertises
> completely bogus information :)

if nobody beats me to it, then I will fix this. Just have to deal with
some other stuff first.

> > Btw. I named it WPA2 on purpose since not everybody using iw will
> > understand that it is actually named RSN. In the end it is your call,
> > just to let you know why I did it. One of my first patches actually did
> > use RSN in it :)
> 
> :)
> I was going to call it RSN/WPA2 but that was too long for nice
> displaying. Now that I think about it though, I think RSN is more
> correct because WPA2 certification doesn't include any 11w things which
> this will include. I suspect something like WPA3 will be done for that.
> Who knows :)

tried the RSN/WPA2 too, but it is too long :)

On a different note, what is your preferred coding style for loops?

	for (i = 0; i < len; i++) {

or

	for(i=0; i<len-3; i++)

I was using the latter one since I assumed that is what you want (it is
not my preferred one). Then I realized that you are using both or that
something slipped in. Do you want me to send a patch to fix this?

Regards

Marcel

Re: [PATCH] add basic support for parsing country information element

[Johannes Berg]

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

On Mon, 2009-05-04 at 09:09 -0700, Marcel Holtmann wrote:

> > Certainly. I just don't want to have it crash on an AP that advertises
> > completely bogus information :)
> 
> if nobody beats me to it, then I will fix this. Just have to deal with
> some other stuff first.

Alright, thanks. Not too pressing I guess. Just should be done before I
cut the next release.

> tried the RSN/WPA2 too, but it is too long :)
> 
> On a different note, what is your preferred coding style for loops?
> 
> 	for (i = 0; i < len; i++) {
> 
> or
> 
> 	for(i=0; i<len-3; i++)
> 
> I was using the latter one since I assumed that is what you want (it is
> not my preferred one). Then I realized that you are using both or that
> something slipped in. Do you want me to send a patch to fix this?

Oh, well, I prefer the former, but some other people put in the latter,
and I didn't really care too much. No need to send a patch though.

johannes

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

Re: [PATCH] add basic support for parsing country information element

[Marcel Holtmann]

Hi Johannes,

> > On a different note, what is your preferred coding style for loops?
> > 
> > 	for (i = 0; i < len; i++) {
> > 
> > or
> > 
> > 	for(i=0; i<len-3; i++)
> > 
> > I was using the latter one since I assumed that is what you want (it is
> > not my preferred one). Then I realized that you are using both or that
> > something slipped in. Do you want me to send a patch to fix this?
> 
> Oh, well, I prefer the former, but some other people put in the latter,
> and I didn't really care too much. No need to send a patch though.

I will sent a patch since I prefer the former too, but assumed you liked
that latter one and that was my mistake in cluttering the code with more
mixed coding style than needed.

Regards

Marcel