mac80211: fix key restricted/open display
PATCH 3/5 mac80211: fix key restricted/open display
---
net/mac80211/ieee80211_ioctl.c | 16 ++++++++++++++++ 1 files changed, 16 insertions(+), 0 deletions(-)
26742fdf9c5835a0abcb75a364840beee08953f8
net/mac80211/ieee80211_ioctl.c +16 -0
@@ -2934,6 +2934,14 @@ static int ieee80211_ioctl_siwencode(strelse 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 ; }
Signature · 1 line
1.3.3
net/mac80211/ieee80211_ioctl.c static int ieee80211_ioctl_siwencode(str · line 2944
Quoted 8 lines · Hong Liu
net/mac80211/ieee80211_ioctl.c +8
-0+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
Dan Williams
Acked-byQuoted 13 lines · Michael Wu
net/mac80211/ieee80211_ioctl.c static int ieee80211_ioctl_siwencode(str · line 2944
Quoted 8 lines · Hong Liu
net/mac80211/ieee80211_ioctl.c +8
-0+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.
Quoted 5 lines
IW_ENCODE_RESTRICTEDsimply means that the interface should not make/accept unencrypted connections. In client mode withoutwpa_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 :)
Dan
Quoted 4 lines · Dan Williams
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.
Quoted 8 lines
Not quite. Somewhere along the line WEXT turned
ENCODE_RESTRICTEDinto the selector for Shared Key, whileENCODE_OPENis 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.
Quoted 5 lines
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 <[email protected]>
NACK. It is not useful and if implemented, gives the user an unnecessary choice that can only cause more problems.
-Michael Wu
Quoted 6 lines · Michael Wu
Quoted 4 lines · Dan Williams
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.
Quoted 8 lines
Quoted 8 lines
Not quite. Somewhere along the line WEXT turned
ENCODE_RESTRICTEDinto the selector for Shared Key, whileENCODE_OPENis 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.
Quoted 4 lines
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.
Quoted 7 lines
Quoted 5 lines
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 <[email protected]>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
Quoted 10 lines · Dan Williams
Quoted 6 lines · Michael Wu
Quoted 3 lines · Dan Williams
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.
Quoted 4 lines
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
Quoted 7 lines · Michael Wu
Quoted 6 lines · Dan Williams
Quoted 5 lines · Michael Wu
Quoted 3 lines · Dan Williams
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.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
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_algsis 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.
Quoted 6 lines
Quoted 4 lines
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_wepappears 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
Jouni Malinen
NAKQuoted 9 lines · Dan Williams
Quoted 8 lines · Michael Wu
net/mac80211/ieee80211_ioctl.c static int ieee80211_ioctl_siwencode(str · line 2943
Quoted 7 lines · Hong Liu
net/mac80211/ieee80211_ioctl.c +7
-0+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.
Quoted 3 lines
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).
Quoted 2 lines
Quoted 2 lines
IW_ENCODE_RESTRICTEDsimply means that the interface should not make/accept unencrypted connections.
Agreed.
Quoted 7 lines
Not quite. Somewhere along the line WEXT turned
ENCODE_RESTRICTEDinto the selector for Shared Key, whileENCODE_OPENis 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.
Signature · 1 line
Jouni Malinen PGP id EFC895FA
Quoted 13 lines · Jouni Malinen
Quoted 9 lines · Dan Williams
Quoted 8 lines · Michael Wu
net/mac80211/ieee80211_ioctl.c static int ieee80211_ioctl_siwencode(str · line 2943
Quoted 7 lines · Hong Liu
net/mac80211/ieee80211_ioctl.c +7
-0+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.
Quoted 5 lines
Quoted 3 lines
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_RESTRICTEDand 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.
Quoted 13 lines
Quoted 2 lines
Quoted 2 lines
IW_ENCODE_RESTRICTEDsimply means that the interface should not make/accept unencrypted connections.Agreed.
Quoted 7 lines
Not quite. Somewhere along the line WEXT turned
ENCODE_RESTRICTEDinto the selector for Shared Key, whileENCODE_OPENis 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.
Quoted 2 lines
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.
Quoted 4 lines
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.
Quoted 3 lines
WE-18 introduced
IW_AUTH_80211_AUTH_ALGand 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
Quoted 5 lines · Dan Williams
Quoted 3 lines · Jouni Malinen
WE-18 introduced
IW_AUTH_80211_AUTH_ALGand 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.
Quoted 7 lines
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
Quoted 3 lines · Hong Liu
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
Signature · 2 lines
Jiri Benc SUSE Labs