[PATCH 3/5] mac80211: fix key restricted/open display

[PATCH 3/5] mac80211: fix key restricted/open display

[Hong Liu]

Signed-off-by: Hong Liu <hong.liu@intel.com>

---

 net/mac80211/ieee80211_ioctl.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

26742fdf9c5835a0abcb75a364840beee08953f8
diff --git a/net/mac80211/ieee80211_ioctl.c b/net/mac80211/ieee80211_ioctl.c
index f57e48f..46fd125 100644
--- a/net/mac80211/ieee80211_ioctl.c
+++ b/net/mac80211/ieee80211_ioctl.c
@@ -2934,6 +2934,14 @@ static int ieee80211_ioctl_siwencode(str
 	else
 		idx--;
 
+	if (erq->flags & (IW_ENCODE_OPEN | IW_ENCODE_RESTRICTED))
+		if (sdata->type == IEEE80211_IF_TYPE_STA ||
+		    sdata->type == IEEE80211_IF_TYPE_IBSS)
+			sdata->u.sta.auth_algs =
+				(erq->flags & IW_ENCODE_RESTRICTED) ?
+				IEEE80211_AUTH_ALG_SHARED_KEY :
+				IEEE80211_AUTH_ALG_OPEN;
+
 	if (erq->flags & IW_ENCODE_DISABLED)
 		alg = ALG_NONE;
 	else if (erq->length == 0) {
@@ -2993,6 +3001,14 @@ static int ieee80211_ioctl_giwencode(str
 	erq->length = sdata->keys[idx]->keylen;
 	erq->flags |= IW_ENCODE_ENABLED;
 
+	if (sdata->type == IEEE80211_IF_TYPE_STA ||
+	    sdata->type == IEEE80211_IF_TYPE_IBSS) {
+		if (sdata->u.sta.auth_algs & IEEE80211_AUTH_ALG_OPEN)
+			erq->flags |= IW_ENCODE_OPEN;
+		else if (sdata->u.sta.auth_algs & IEEE80211_AUTH_ALG_SHARED_KEY)
+			erq->flags |= IW_ENCODE_RESTRICTED;
+	}
+
 	return 0;
 }
 
-- 
1.3.3

Re: [PATCH 3/5] mac80211: fix key restricted/open display

[Michael Wu]

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

On Thursday 15 March 2007 23:28, Hong Liu wrote:
> +	if (erq->flags & (IW_ENCODE_OPEN | IW_ENCODE_RESTRICTED))
> +		if (sdata->type == IEEE80211_IF_TYPE_STA ||
> +		    sdata->type == IEEE80211_IF_TYPE_IBSS)
> +			sdata->u.sta.auth_algs =
> +				(erq->flags & IW_ENCODE_RESTRICTED) ?
> +				IEEE80211_AUTH_ALG_SHARED_KEY :
> +				IEEE80211_AUTH_ALG_OPEN;
> +
This is not right because encrypted access points often do not require shared 
key authentication to associate. In fact, some cannot or refuse to use shared 
key authentication and your patch prevents retrying with a different 
authentication algorithm.

IW_ENCODE_RESTRICTED simply means that the interface should not make/accept 
unencrypted connections. In client mode without wpa_supplicant, the AP 
selection code already refuses to select any APs without encryption enabled 
if the default key is set. In adhoc mode, there's no such check, but I'm not 
sure how much it matters.

-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 3/5] mac80211: fix key restricted/open display

[Dan Williams]

On Fri, 2007-03-16 at 13:28 -0400, Michael Wu wrote:
> On Thursday 15 March 2007 23:28, Hong Liu wrote:
> > +	if (erq->flags & (IW_ENCODE_OPEN | IW_ENCODE_RESTRICTED))
> > +		if (sdata->type == IEEE80211_IF_TYPE_STA ||
> > +		    sdata->type == IEEE80211_IF_TYPE_IBSS)
> > +			sdata->u.sta.auth_algs =
> > +				(erq->flags & IW_ENCODE_RESTRICTED) ?
> > +				IEEE80211_AUTH_ALG_SHARED_KEY :
> > +				IEEE80211_AUTH_ALG_OPEN;
> > +
> This is not right because encrypted access points often do not require shared 
> key authentication to associate. In fact, some cannot or refuse to use shared 
> key authentication and your patch prevents retrying with a different 
> authentication algorithm.

I think you're misreading the patch?  It looks correct to me.  The
second check for (erq->flags & IW_ENCODE_RESTRICTED) should ensure that
Shared Key is only selected when the userspace program requested it.

> IW_ENCODE_RESTRICTED simply means that the interface should not make/accept 
> unencrypted connections. In client mode without wpa_supplicant, the AP 
> selection code already refuses to select any APs without encryption enabled 
> if the default key is set. In adhoc mode, there's no such check, but I'm not 
> sure how much it matters.

Not quite.  Somewhere along the line WEXT turned ENCODE_RESTRICTED into
the selector for Shared Key, while ENCODE_OPEN is Open System.  Arguably
there's a larger need to specifying auth mode than rejecting unencrypted
associations.  Most drivers do it this way, with the exception of
madwifi because they like to be irritatingly different.  Nobody ever
really used the 'don't accept unencrypted' thing anyway in the old days,
plus ENCODEEXT has a separate flag for this.

So I think the patch is correct.  Ideally all this gets fixed and all
the overloaded meanings go away with cfg80211 :)

Acked-by: Dan Williams <dcbw@redhat.com>

Dan

Re: [PATCH 3/5] mac80211: fix key restricted/open display

[Michael Wu]

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

On Friday 16 March 2007 23:46, Dan Williams wrote:
> I think you're misreading the patch?  It looks correct to me.  The
> second check for (erq->flags & IW_ENCODE_RESTRICTED) should ensure that
> Shared Key is only selected when the userspace program requested it.
>
This breaks authentication algorithm fallback for sure.

> Not quite.  Somewhere along the line WEXT turned ENCODE_RESTRICTED into
> the selector for Shared Key, while ENCODE_OPEN is Open System.  Arguably
> there's a larger need to specifying auth mode than rejecting unencrypted
> associations.  Most drivers do it this way, with the exception of
> madwifi because they like to be irritatingly different.  Nobody ever
> really used the 'don't accept unencrypted' thing anyway in the old days,
> plus ENCODEEXT has a separate flag for this.
>
Even if it got redefined along the way, mac80211 has no need for that 
particular definition since it can automatically cycle between authentication 
algorithms. Besides, "its meaning depends on the card used" according to the 
iwconfig man page.

> So I think the patch is correct.  Ideally all this gets fixed and all
> the overloaded meanings go away with cfg80211 :)
>
> Acked-by: Dan Williams <dcbw@redhat.com>
>
NACK. It is not useful and if implemented, gives the user an unnecessary 
choice that can only cause more problems.

-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 3/5] mac80211: fix key restricted/open display

[Dan Williams]

On Fri, 2007-03-16 at 23:57 -0400, Michael Wu wrote:
> On Friday 16 March 2007 23:46, Dan Williams wrote:
> > I think you're misreading the patch?  It looks correct to me.  The
> > second check for (erq->flags & IW_ENCODE_RESTRICTED) should ensure that
> > Shared Key is only selected when the userspace program requested it.
> >
> This breaks authentication algorithm fallback for sure.

Well, then it's broken in most of the non mac80211 drivers then.

> > Not quite.  Somewhere along the line WEXT turned ENCODE_RESTRICTED into
> > the selector for Shared Key, while ENCODE_OPEN is Open System.  Arguably
> > there's a larger need to specifying auth mode than rejecting unencrypted
> > associations.  Most drivers do it this way, with the exception of
> > madwifi because they like to be irritatingly different.  Nobody ever
> > really used the 'don't accept unencrypted' thing anyway in the old days,
> > plus ENCODEEXT has a separate flag for this.
> >
> Even if it got redefined along the way, mac80211 has no need for that 
> particular definition since it can automatically cycle between authentication 
> algorithms. Besides, "its meaning depends on the card used" according to the 
> iwconfig man page.

Well, if mac80211 can cycle (is this like airo's auto_wep?) then I guess
we don't care about the auth mode.  As long as we don't break userspace
programs that try to set the auth mode, I'm fine with that.

> > So I think the patch is correct.  Ideally all this gets fixed and all
> > the overloaded meanings go away with cfg80211 :)
> >
> > Acked-by: Dan Williams <dcbw@redhat.com>
> >
> NACK. It is not useful and if implemented, gives the user an unnecessary 
> choice that can only cause more problems.

As long as existing stuff isn't broken, and as long as mac80211 silently
ignores it, and as long as the auth cycle stuff works, then fine.

Dan

Re: [PATCH 3/5] mac80211: fix key restricted/open display

[Michael Wu]

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

On Saturday 17 March 2007 00:38, Dan Williams wrote:
> On Fri, 2007-03-16 at 23:57 -0400, Michael Wu wrote:
> > On Friday 16 March 2007 23:46, Dan Williams wrote:
> > > I think you're misreading the patch?  It looks correct to me.  The
> > > second check for (erq->flags & IW_ENCODE_RESTRICTED) should ensure that
> > > Shared Key is only selected when the userspace program requested it.
> >
> > This breaks authentication algorithm fallback for sure.
>
> Well, then it's broken in most of the non mac80211 drivers then.
>
Why is that? The reason this breaks mac80211 is because auth_algs is a 
bitfield which indicates what authentication algorithms can be used. This 
patch makes it so that if a user chooses an authentication algorithm, that is 
the only one that will ever be used.

> Well, if mac80211 can cycle (is this like airo's auto_wep?) then I guess
> we don't care about the auth mode.  As long as we don't break userspace
> programs that try to set the auth mode, I'm fine with that.
>
mac80211 should just ignore the bits. Airo's auto_wep appears to cycle through 
different encryption keys while mac80211 does not.

-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 3/5] mac80211: fix key restricted/open display

[Dan Williams]

On Sat, 2007-03-17 at 00:57 -0400, Michael Wu wrote:
> On Saturday 17 March 2007 00:38, Dan Williams wrote:
> > On Fri, 2007-03-16 at 23:57 -0400, Michael Wu wrote:
> > > On Friday 16 March 2007 23:46, Dan Williams wrote:
> > > > I think you're misreading the patch?  It looks correct to me.  The
> > > > second check for (erq->flags & IW_ENCODE_RESTRICTED) should ensure that
> > > > Shared Key is only selected when the userspace program requested it.
> > >
> > > This breaks authentication algorithm fallback for sure.
> >
> > Well, then it's broken in most of the non mac80211 drivers then.
> >
> Why is that? The reason this breaks mac80211 is because auth_algs is a 
> bitfield which indicates what authentication algorithms can be used. This 
> patch makes it so that if a user chooses an authentication algorithm, that is 
> the only one that will ever be used.

Well, what I meant here was that older fullmac drivers don't do fallback
and users must explicitly choose the auth method they required (which
sucks).  Therefore, we've got to make sure that everything works as
expected.  And I think you're right, mac80211 can just ignore requests
to explicitly set the WEP auth alg.

> > Well, if mac80211 can cycle (is this like airo's auto_wep?) then I guess
> > we don't care about the auth mode.  As long as we don't break userspace
> > programs that try to set the auth mode, I'm fine with that.
> >
> mac80211 should just ignore the bits. Airo's auto_wep appears to cycle through 
> different encryption keys while mac80211 does not.

Half-right.  auto_wep can be 0 -> 4.  0 is off, and 1 - 4 is the max
index of WEP keys to try.  It cycles through _both_ the WEP keys and the
auth modes trying each one out of the card isn't currently connected.
Sort of crack to include the WEP keys too, but it appears to be the same
thing.

Dan

Re: [PATCH 3/5] mac80211: fix key restricted/open display

[Jouni Malinen]

On Fri, Mar 16, 2007 at 11:46:17PM -0400, Dan Williams wrote:
> On Fri, 2007-03-16 at 13:28 -0400, Michael Wu wrote:
> > On Thursday 15 March 2007 23:28, Hong Liu wrote:
> > > +	if (erq->flags & (IW_ENCODE_OPEN | IW_ENCODE_RESTRICTED))
> > > +		if (sdata->type == IEEE80211_IF_TYPE_STA ||
> > > +		    sdata->type == IEEE80211_IF_TYPE_IBSS)
> > > +			sdata->u.sta.auth_algs =
> > > +				(erq->flags & IW_ENCODE_RESTRICTED) ?
> > > +				IEEE80211_AUTH_ALG_SHARED_KEY :
> > > +				IEEE80211_AUTH_ALG_OPEN;

NAK.

> I think you're misreading the patch?  It looks correct to me.  The
> second check for (erq->flags & IW_ENCODE_RESTRICTED) should ensure that
> Shared Key is only selected when the userspace program requested it.

IW_ENCODE_RESTRICTED and Shared Key are different things (IMHO).

> > IW_ENCODE_RESTRICTED simply means that the interface should not make/accept 
> > unencrypted connections.

Agreed.

> Not quite.  Somewhere along the line WEXT turned ENCODE_RESTRICTED into
> the selector for Shared Key, while ENCODE_OPEN is Open System.  Arguably
> there's a larger need to specifying auth mode than rejecting unencrypted
> associations.  Most drivers do it this way, with the exception of
> madwifi because they like to be irritatingly different.  Nobody ever
> really used the 'don't accept unencrypted' thing anyway in the old days,
> plus ENCODEEXT has a separate flag for this.

Take a look at Host AP driver.. It has always mapped HFA384x WEPFLAGS
excludeunencrypted to IW_ENCODE_RESTRICTED..

The IW_ENCODE_OPEN/RESTRICTED is quite unfortunate part of WEXT and
since it has been used for two completely different things, it should
not really be used. I would hope that this change is not introduced into
mac80211.

WE-18 introduced IW_AUTH_80211_AUTH_ALG and that should be used if user
space wants to explicitly select which 802.11 authentication algorithm
is to be used. Please let the IW_ENCODE_OPEN/RESTRICTED misuse die.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH 3/5] mac80211: fix key restricted/open display

[Dan Williams]

On Sun, 2007-03-18 at 09:45 -0700, Jouni Malinen wrote:
> On Fri, Mar 16, 2007 at 11:46:17PM -0400, Dan Williams wrote:
> > On Fri, 2007-03-16 at 13:28 -0400, Michael Wu wrote:
> > > On Thursday 15 March 2007 23:28, Hong Liu wrote:
> > > > +	if (erq->flags & (IW_ENCODE_OPEN | IW_ENCODE_RESTRICTED))
> > > > +		if (sdata->type == IEEE80211_IF_TYPE_STA ||
> > > > +		    sdata->type == IEEE80211_IF_TYPE_IBSS)
> > > > +			sdata->u.sta.auth_algs =
> > > > +				(erq->flags & IW_ENCODE_RESTRICTED) ?
> > > > +				IEEE80211_AUTH_ALG_SHARED_KEY :
> > > > +				IEEE80211_AUTH_ALG_OPEN;
> 
> NAK.
> 
> > I think you're misreading the patch?  It looks correct to me.  The
> > second check for (erq->flags & IW_ENCODE_RESTRICTED) should ensure that
> > Shared Key is only selected when the userspace program requested it.
> 
> IW_ENCODE_RESTRICTED and Shared Key are different things (IMHO).

Yes, I _know_ that.  I thought that was apparent in my message.  But
WEXT has commingled them to some degree.  Otherwise, there's no way at
all to select between Open System or Shared Key when associating.

I would also argue that there is more of a use for selecting between SK
and OS auth than using RESTRICTED for it's real meaning anyway.

> > > IW_ENCODE_RESTRICTED simply means that the interface should not make/accept 
> > > unencrypted connections.
> 
> Agreed.
> 
> > Not quite.  Somewhere along the line WEXT turned ENCODE_RESTRICTED into
> > the selector for Shared Key, while ENCODE_OPEN is Open System.  Arguably
> > there's a larger need to specifying auth mode than rejecting unencrypted
> > associations.  Most drivers do it this way, with the exception of
> > madwifi because they like to be irritatingly different.  Nobody ever
> > really used the 'don't accept unencrypted' thing anyway in the old days,
> > plus ENCODEEXT has a separate flag for this.
> 
> Take a look at Host AP driver.. It has always mapped HFA384x WEPFLAGS
> excludeunencrypted to IW_ENCODE_RESTRICTED..

Which is somewhat unfortunate.  How does the HostAP driver select for OS
or SK when userspace programs don't know about WE-18 and therefore don't
use AUTH_ALG?

This is the same problem I submitted that "auth alg fallback" patch to
wpa_supplicant for last year.  Older drivers like orinoco or atmel just
don't have ENCODEEXT (I added it to prism54 and airo), and instead use
RESTRICTED.

There is no good way for a userspace program to set SK/OS except try
ENCODEEXT and fall back to ENCODE.  Which is, um, suboptimal.

> The IW_ENCODE_OPEN/RESTRICTED is quite unfortunate part of WEXT and
> since it has been used for two completely different things, it should
> not really be used. I would hope that this change is not introduced into
> mac80211.

Definitely.

> WE-18 introduced IW_AUTH_80211_AUTH_ALG and that should be used if user
> space wants to explicitly select which 802.11 authentication algorithm
> is to be used. Please let the IW_ENCODE_OPEN/RESTRICTED misuse die.

Obviously we want it to die.  But the better solution is to make apps
that care about "exclude unencrypted", which there are few, use
ENCODEEXT and keep OPEN/RESTRICTED be what most of the old drivers
expect it to be; the selector for OS/SK.  Then clean up all the double
meanings with cfg80211 and call it day.

Dan

Re: [PATCH 3/5] mac80211: fix key restricted/open display

[Hong Liu]

On Mon, 2007-03-19 at 07:35, Dan Williams wrote:
> On Sun, 2007-03-18 at 09:45 -0700, Jouni Malinen wrote:
> > WE-18 introduced IW_AUTH_80211_AUTH_ALG and that should be used if user
> > space wants to explicitly select which 802.11 authentication algorithm
> > is to be used. Please let the IW_ENCODE_OPEN/RESTRICTED misuse die.
> 
> Obviously we want it to die.  But the better solution is to make apps
> that care about "exclude unencrypted", which there are few, use
> ENCODEEXT and keep OPEN/RESTRICTED be what most of the old drivers
> expect it to be; the selector for OS/SK.  Then clean up all the double
> meanings with cfg80211 and call it day.
> 
> Dan

As far as mac80211 is concerned, I think the patch is not needed.
mac80211 has done what it needed to do through the SIOC{S/G}IWAUTH
interface.
It seems the fix should be in userspace app, iwconfig should use the
SIOC{S/G}IWAUTH interface and resort to IW_ENCODE_RESTRICTED if it wants
to maintain the compatibility with old drivers.

Thanks,
Hong

Re: [PATCH 3/5] mac80211: fix key restricted/open display

[Jiri Benc]

On Thu, 22 Mar 2007 11:43:45 +0800, Hong Liu wrote:
> As far as mac80211 is concerned, I think the patch is not needed.
> mac80211 has done what it needed to do through the SIOC{S/G}IWAUTH
> interface.

Ok. Let's drop this patch.

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs