Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On 6/9/2017 12:59 PM, Johannes Berg wrote: On Fri, 2017-06-09 at 12:34 +0200, Arend van Spriel wrote: Not trying to change your opinion ;-). Just getting it all clear. As a matter of fact I already have my wpa_s use the offload without any debug/config option. As brcmfmac can use override to disable firmware features I also do not need such in wpa_s. Sounds good to me :) So I guess we're aligned on this - do you need to respin as a result of this discussion, or should I review the patches again? I will respin. Actually only change will be the extra documentation section in which I need to incorporate your feedback. Regards, Arend
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On Fri, 2017-06-09 at 12:34 +0200, Arend van Spriel wrote: > Not trying to change your opinion ;-). Just getting it all clear. As > a matter of fact I already have my wpa_s use the offload without any > debug/config option. As brcmfmac can use override to disable > firmware features I also do not need such in wpa_s. Sounds good to me :) So I guess we're aligned on this - do you need to respin as a result of this discussion, or should I review the patches again? johannes
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On 6/9/2017 11:08 AM, Johannes Berg wrote: Hi Arend, Sorry about the delay. No problem. Then you have to support NEW_KEY (AP case) and then using NEW_KEY to detect whether or not a wpa_s configuration option to not use offloaded 4-way-HS can be used will not work correctly. I don't really see that this is a sensible configuration, but I could imagine it existing if somebody "bolted on" AP functionality for a client-side chipset or something like that. So I want to get back to this as to assure we have the same understanding. First off, the proposed offloads are STA-only. Right. But we at least also want to have them for AP mode, which answers your below question. So if a driver supports STA and AP mode and the 4-way-HS offload, we could already have the case you describe here. So not really a "bolted on" scenario I would say. But if you say you don't have it in AP mode, or you didn't even think about it in AP mode yet, then I think you're right and the "bolted on" part doesn't really apply (*). yup. However, this does mean that checking for NEW_KEY support _isn't_ sufficient to understand whether or not the device also supports doing the 4-way-HS in the host, because the device might _not_ support that in client mode, yet implement NEW_KEY for AP mode, right? Indeed. I did not state it explicitly, but that is also my understanding so not going to do that. In any case - I don't think this changes much my opinion. I think that newer wpa_s should always use the offload where available, and if there's a debug option to turn that off, and that debug option causes failures because the driver rejects the NEW_KEY command, rather than causing an up-front configuration failure because wpa_s knows that the NEW_KEY wouldn't be supported, then I don't think for a debug option that makes a big difference. For something that higher layers might need to set, it would make a difference - but that's not at stake here. Not trying to change your opinion ;-). Just getting it all clear. As a matter of fact I already have my wpa_s use the offload without any debug/config option. As brcmfmac can use override to disable firmware features I also do not need such in wpa_s. (*) my line of thinking was that if you have the necessary state machine for client in the firmware, then it's not a huge step to also have it for AP, since you have the crypto primitives for it Agree. However, in our firmware they are two modules that can be compiled in, ie. idsup and idauth. Regards, Arend
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
Hi Arend, Sorry about the delay. > > Then you have to support NEW_KEY (AP case) and then using NEW_KEY > > to > > detect whether or not a wpa_s configuration option to not use > > offloaded > > 4-way-HS can be used will not work correctly. > > > > I don't really see that this is a sensible configuration, but I > > could > > imagine it existing if somebody "bolted on" AP functionality for a > > client-side chipset or something like that. > So I want to get back to this as to assure we have the same > understanding. First off, the proposed offloads are STA-only. Right. But we at least also want to have them for AP mode, which answers your below question. > So if a > driver supports STA and AP mode and the 4-way-HS offload, we could > already have the case you describe here. So not really a "bolted on" > scenario I would say. But if you say you don't have it in AP mode, or you didn't even think about it in AP mode yet, then I think you're right and the "bolted on" part doesn't really apply (*). However, this does mean that checking for NEW_KEY support _isn't_ sufficient to understand whether or not the device also supports doing the 4-way-HS in the host, because the device might _not_ support that in client mode, yet implement NEW_KEY for AP mode, right? In any case - I don't think this changes much my opinion. I think that newer wpa_s should always use the offload where available, and if there's a debug option to turn that off, and that debug option causes failures because the driver rejects the NEW_KEY command, rather than causing an up-front configuration failure because wpa_s knows that the NEW_KEY wouldn't be supported, then I don't think for a debug option that makes a big difference. For something that higher layers might need to set, it would make a difference - but that's not at stake here. johannes (*) my line of thinking was that if you have the necessary state machine for client in the firmware, then it's not a huge step to also have it for AP, since you have the crypto primitives for it
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On 29-05-17 11:31, Johannes Berg wrote: > So if something supports the following: > > * client: offloaded 4-way-HS only > * AP: not offloaded 4-way-HS only > > Then you have to support NEW_KEY (AP case) and then using NEW_KEY to > detect whether or not a wpa_s configuration option to not use offloaded > 4-way-HS can be used will not work correctly. > > I don't really see that this is a sensible configuration, but I could > imagine it existing if somebody "bolted on" AP functionality for a > client-side chipset or something like that. Hi Johannes, So I want to get back to this as to assure we have the same understanding. First off, the proposed offloads are STA-only. So if a driver supports STA and AP mode and the 4-way-HS offload, we could already have the case you describe here. So not really a "bolted on" scenario I would say. It does raise the question whether or not we want similar offloads for AP. Regards, Arend
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On Fri, 2017-06-02 at 13:19 +0200, Arend van Spriel wrote: > I was thinking about adding a DOC section in nl80211.h: Sure! > /** > * DOC: WPA/WPA2 temporal key exchange offload > * > * By setting @NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_PSK flag > drivers > * can indicate offload support of EAPOL handshakes for WPA/WPA2 can indicate they support offloading EAPOL ...? > * preshared key authentication. In %NL80211_CMD_CONNECT the > preshared > * key should be specified using %NL80211_ATTR_PMK. Drivers > supporting > * this offload may reject the %NL80211_CMD_CONNECT when no > preshared > * key material is provided. For example when that driver does not > * support setting the temporal keys through %NL80211_CMD_NEW_KEY. [...] is provided, for example when [...] I think? Starting with "For example" seems odd to me. > * > * Similarly @NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X flag can be > * set by drivers indicating offload support of the PTK/GTK EAPOL > * handshakes during 802.1X authentication. In order to use the > offload > * the %NL80211_CMD_CONNECT should have > %NL80211_ATTR_WANT_1X_4WAY_HS > * attribute flag. Drivers supporting this offload may reject the > * %NL80211_CMD_CONNECT when the attribute flag is not present. > */ > > Could add description for FT, ie. PMK-R0 handling as well. Do you > think this change warrants a separate section or not. Any comments on > the text itself are welcome. I think that's good to have. johannes
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On 5/29/2017 11:31 AM, Johannes Berg wrote: Hi Arend, Note that this (checking NEW_KEY) only works when you don't have any split between AP/client cases. Not sure what's the case for you. Late response so hopefully you recall, but what do you mean by "any split between AP/client cases"? I meant the capability split - let's say you support 4-way-HS only for client, but not for AP. Then you have to support the NEW_KEY command for the AP case, even if you might not support non-offloaded 4-way-HS for the client case. So if something supports the following: * client: offloaded 4-way-HS only * AP: not offloaded 4-way-HS only Then you have to support NEW_KEY (AP case) and then using NEW_KEY to detect whether or not a wpa_s configuration option to not use offloaded 4-way-HS can be used will not work correctly. I don't really see that this is a sensible configuration, but I could imagine it existing if somebody "bolted on" AP functionality for a client-side chipset or something like that. Again, I think I'm happy to leave this up to you - this kind of configuration option should really only be used for debugging anyway, so just getting errors later is probably fine. Hi Johannes, I was thinking about adding a DOC section in nl80211.h: /** * DOC: WPA/WPA2 temporal key exchange offload * * By setting @NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_PSK flag drivers * can indicate offload support of EAPOL handshakes for WPA/WPA2 * preshared key authentication. In %NL80211_CMD_CONNECT the preshared * key should be specified using %NL80211_ATTR_PMK. Drivers supporting * this offload may reject the %NL80211_CMD_CONNECT when no preshared * key material is provided. For example when that driver does not * support setting the temporal keys through %NL80211_CMD_NEW_KEY. * * Similarly @NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X flag can be * set by drivers indicating offload support of the PTK/GTK EAPOL * handshakes during 802.1X authentication. In order to use the offload * the %NL80211_CMD_CONNECT should have %NL80211_ATTR_WANT_1X_4WAY_HS * attribute flag. Drivers supporting this offload may reject the * %NL80211_CMD_CONNECT when the attribute flag is not present. */ Could add description for FT, ie. PMK-R0 handling as well. Do you think this change warrants a separate section or not. Any comments on the text itself are welcome. Regards, Arend
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
Hi Arend, > > Note that this (checking NEW_KEY) only works when you don't have > > any split between AP/client cases. Not sure what's the case for > > you. > > Late response so hopefully you recall, but what do you mean by "any > split between AP/client cases"? I meant the capability split - let's say you support 4-way-HS only for client, but not for AP. Then you have to support the NEW_KEY command for the AP case, even if you might not support non-offloaded 4-way-HS for the client case. So if something supports the following: * client: offloaded 4-way-HS only * AP: not offloaded 4-way-HS only Then you have to support NEW_KEY (AP case) and then using NEW_KEY to detect whether or not a wpa_s configuration option to not use offloaded 4-way-HS can be used will not work correctly. I don't really see that this is a sensible configuration, but I could imagine it existing if somebody "bolted on" AP functionality for a client-side chipset or something like that. Again, I think I'm happy to leave this up to you - this kind of configuration option should really only be used for debugging anyway, so just getting errors later is probably fine. johannes
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On Mon, 2017-05-22 at 12:14 +0200, Arend van Spriel wrote: > > There is a (small) chance of regression with older devices as stated > above and I would like to keep the fallback option for that. Also > people may like to have a choice. I am not so sure about whether > NEW_KEY support is enough or a new ext_feature flag is needed. I am > inclined to say the NEW_KEY support is sufficient. Ok, your call. Note that this (checking NEW_KEY) only works when you don't have any split between AP/client cases. Not sure what's the case for you. johannes
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On 5/19/2017 12:21 PM, Johannes Berg wrote: On Thu, 2017-05-18 at 14:48 +0200, Arend Van Spriel wrote: True. However, we touched this topic a while ago in generic context, ie. preference for ext_features over supported_commands. Right now wpa_supplicant does not check NEW_KEY support so we can go either way. Right. That's a separate discussion though - whether we add two flags here or check NEW_KEY support doesn't really matter, except that we need to decide * if we want to support such an override at all, and * if it should be tested at all (perhaps we can just live with that failing entirely?) I have cleaned up the wpa_supplicant patches for the offloads, but waited with submitting them until the kernel side got applied. So depending on what is decided here I can rework it. I don't really know. I personally don't think that we need to allow both ways, but then I'm coming with a driver that doesn't even support the old way. Ok. I know for sure that I have firmware in linux-firmware that supports the offload. So for people using that out there things will change from one way to the other when they change to a newer wpa_supplicant. So you should probably decide yourself if you want to keep this fallback option, and how well it should work (be rejected if used on a driver that doesn't support it, or just fail later?) There is a (small) chance of regression with older devices as stated above and I would like to keep the fallback option for that. Also people may like to have a choice. I am not so sure about whether NEW_KEY support is enough or a new ext_feature flag is needed. I am inclined to say the NEW_KEY support is sufficient. Regards, Arend
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On 18-5-2017 12:40, Johannes Berg wrote: > On Thu, 2017-05-18 at 12:29 +0200, Arend Van Spriel wrote: >> On 18-5-2017 11:22, Johannes Berg wrote: >>> On Thu, 2017-05-18 at 10:18 +0200, Arend Van Spriel wrote: > We should therefore probably set the expectation that wpa_s - > if > it's new enough - always uses the offloaded functionality and > always sets the WANT_1X. Then this is even better with such > drivers, since they can immediately reject the connect() > command if > want_1x isn't set. >> >> Getting back at this. With "always" you mean for every connect() >> regardless whether it is using 1X or PSK? > > No, I just meant it would never use the non-offloaded functionality for > 1X, as long as wpa_s was new enough to support it. > > The same consideration kinda applies to (non-)offloaded 4-way-HS for > PSK though I guess, with some drivers (devices) not able to not offload > it. Thanks for clarifying that. And indeed it applies to both cases. >> You mean adding a nl80211 command in which user-space can indicate >> what features it supports? Do you want to use the same feature bits >> on both sides to easily determine the combined feature set? >> ext_feature does not really have much overlapping so not sure if it >> adds value. > > No, I meant that we have NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X > today, but then we might need also > NL80211_EXT_FEATURE_HOST_4WAY_HANDSHAKE_STA_1X. > > Come to think of it though, I guess the fact that the NEW_KEY command > isn't support would already indicate that. True. However, we touched this topic a while ago in generic context, ie. preference for ext_features over supported_commands. Right now wpa_supplicant does not check NEW_KEY support so we can go either way. I have cleaned up the wpa_supplicant patches for the offloads, but waited with submitting them until the kernel side got applied. So depending on what is decided here I can rework it. Regards, Arend
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On Thu, 2017-05-18 at 12:29 +0200, Arend Van Spriel wrote: > On 18-5-2017 11:22, Johannes Berg wrote: > > On Thu, 2017-05-18 at 10:18 +0200, Arend Van Spriel wrote: > > > > > > > We should therefore probably set the expectation that wpa_s - > > > > if > > > > it's new enough - always uses the offloaded functionality and > > > > always sets the WANT_1X. Then this is even better with such > > > > drivers, since they can immediately reject the connect() > > > > command if > > > > want_1x isn't set. > > Getting back at this. With "always" you mean for every connect() > regardless whether it is using 1X or PSK? No, I just meant it would never use the non-offloaded functionality for 1X, as long as wpa_s was new enough to support it. The same consideration kinda applies to (non-)offloaded 4-way-HS for PSK though I guess, with some drivers (devices) not able to not offload it. > You mean adding a nl80211 command in which user-space can indicate > what features it supports? Do you want to use the same feature bits > on both sides to easily determine the combined feature set? > ext_feature does not really have much overlapping so not sure if it > adds value. No, I meant that we have NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X today, but then we might need also NL80211_EXT_FEATURE_HOST_4WAY_HANDSHAKE_STA_1X. Come to think of it though, I guess the fact that the NEW_KEY command isn't support would already indicate that. johannes
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On 18-5-2017 11:22, Johannes Berg wrote: > On Thu, 2017-05-18 at 10:18 +0200, Arend Van Spriel wrote: >> >>> We should therefore probably set the expectation that wpa_s - if >>> it's new enough - always uses the offloaded functionality and >>> always sets the WANT_1X. Then this is even better with such >>> drivers, since they can immediately reject the connect() command if >>> want_1x isn't set. Getting back at this. With "always" you mean for every connect() regardless whether it is using 1X or PSK? >> Personally I am fine with this and it is how I tested it. So no >> network configuration or driver parameter in wpa_s.conf (mainly >> because I am lazy ;-) ). However, if the driver supports both offload >> and non-offload why not leave it up to user-space. Might be useful if >> people can try either way for example when debugging connection >> issues. > > If that should be possible though, then we'd probably want to have > feature bit both ways, otherwise we'll probably get hard-to-debug > failures at some point with this? You mean adding a nl80211 command in which user-space can indicate what features it supports? Do you want to use the same feature bits on both sides to easily determine the combined feature set? ext_feature does not really have much overlapping so not sure if it adds value. Regards, Arend
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
On Thu, 2017-05-18 at 10:18 +0200, Arend Van Spriel wrote: > > > We should therefore probably set the expectation that wpa_s - if > > it's new enough - always uses the offloaded functionality and > > always sets the WANT_1X. Then this is even better with such > > drivers, since they can immediately reject the connect() command if > > want_1x isn't set. > Personally I am fine with this and it is how I tested it. So no > network configuration or driver parameter in wpa_s.conf (mainly > because I am lazy ;-) ). However, if the driver supports both offload > and non-offload why not leave it up to user-space. Might be useful if > people can try either way for example when debugging connection > issues. If that should be possible though, then we'd probably want to have feature bit both ways, otherwise we'll probably get hard-to-debug failures at some point with this? johannes
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
+ hostap list On 17-5-2017 16:19, Johannes Berg wrote: > Hi, > > I think this looks really good. One thing though: > >> Another change is the >> addition of the flag ATTR_WANT_1X_4WAY_HS that user-space has to pass >> in CONNECT request. Some drivers may need to be aware before the PMK >> is programmed through SET_PMK request. > > I wonder how we really should do this, and if this is good enough. > > There might be drivers that simply don't support the non-offloaded > case, so they assume you always have the newer wpa_s. That would seem > to be a legitimate decision, since the compatibility with that might > not make much sense for a completely new driver, and it might be a lot > of work to support TK operations. > > We should therefore probably set the expectation that wpa_s - if it's > new enough - always uses the offloaded functionality and always sets > the WANT_1X. Then this is even better with such drivers, since they can > immediately reject the connect() command if want_1x isn't set. > > Thoughts? Personally I am fine with this and it is how I tested it. So no network configuration or driver parameter in wpa_s.conf (mainly because I am lazy ;-) ). However, if the driver supports both offload and non-offload why not leave it up to user-space. Might be useful if people can try either way for example when debugging connection issues. Regards, Arend
Re: [PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
Hi, I think this looks really good. One thing though: > Another change is the > addition of the flag ATTR_WANT_1X_4WAY_HS that user-space has to pass > in CONNECT request. Some drivers may need to be aware before the PMK > is programmed through SET_PMK request. I wonder how we really should do this, and if this is good enough. There might be drivers that simply don't support the non-offloaded case, so they assume you always have the newer wpa_s. That would seem to be a legitimate decision, since the compatibility with that might not make much sense for a completely new driver, and it might be a lot of work to support TK operations. We should therefore probably set the expectation that wpa_s - if it's new enough - always uses the offloaded functionality and always sets the WANT_1X. Then this is even better with such drivers, since they can immediately reject the connect() command if want_1x isn't set. Thoughts? johannes
[PATCH V2 0/9] nl80211: add support for PTK/GTK handshake offload
This patch series add support for offloading the PTK/GTK handshakes for WPA/WPA2-Personal and 802.1X in nl80211. The nl80211 patches have been posted as RFC. Compared to that RFC this series also adds a new flag ATTR_PORT_AUTHORIZED used in both CONNECT and ROAM notifications to indicate the driver has completed the offloads successfully and the encryption keys for the connection are applied. Another change is the addition of the flag ATTR_WANT_1X_4WAY_HS that user-space has to pass in CONNECT request. Some drivers may need to be aware before the PMK is programmed through SET_PMK request. This series also comes with driver implementation in brcmfmac although it does not use the authorized flag in the ROAM event (yet). The series applies to the master branch of the mac80211-next repository. V2: - changed patch 2/9 addressing comments from Johannes. Arend van Spriel (6): nl80211: add authorized flag to CONNECT event nl80211: remove desciption about request from NL80211_CMD_ROAM brcmfmac: support 4-way handshake offloading for WPA/WPA2-PSK brcmfmac: support 4-way handshake offloading for 802.1X brcmfmac: switch to using cfg80211_connect_done() brcmfmac: provide port authorized state in CONNECT event Avraham Stern (2): cfg80211: support 4-way handshake offloading for 802.1X nl80211: add authorized flag to ROAM event Eliad Peller (1): cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK .../broadcom/brcm80211/brcmfmac/cfg80211.c | 159 +++-- .../broadcom/brcm80211/brcmfmac/cfg80211.h | 15 +- .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 1 + .../wireless/broadcom/brcm80211/brcmfmac/feature.h | 4 +- .../wireless/broadcom/brcm80211/brcmfmac/fweh.h| 30 .../wireless/broadcom/brcm80211/brcmfmac/fwil.h| 1 + .../broadcom/brcm80211/brcmfmac/fwil_types.h | 16 +++ include/linux/ieee80211.h | 4 + include/net/cfg80211.h | 41 ++ include/uapi/linux/nl80211.h | 50 ++- net/wireless/core.c| 5 + net/wireless/nl80211.c | 120 +++- net/wireless/rdev-ops.h| 25 net/wireless/sme.c | 2 + net/wireless/trace.h | 60 15 files changed, 510 insertions(+), 23 deletions(-) -- 1.9.1
