| Message ID | 20200213085112.27376-2-sergey.matyukevich.os@quantenna.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | OWE: cleanup and changes for SME drivers | expand |
On Thu, Feb 13, 2020 at 08:51:21AM +0000, Sergey Matyukevich wrote: > Specify WPA3 AKM suites in set_ap command. Drivers may need to pass > this information to firmware to setup SAE/OWE offload properly. > Besides,drivers may use this information to perform early check > SAE/OWE offload in hostapd. > diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c > @@ -4202,6 +4202,10 @@ static int wpa_driver_nl80211_set_ap(void *priv, > suites[num_suites++] = RSN_AUTH_KEY_MGMT_UNSPEC_802_1X; > if (params->key_mgmt_suites & WPA_KEY_MGMT_PSK) > suites[num_suites++] = RSN_AUTH_KEY_MGMT_PSK_OVER_802_1X; > + if (params->key_mgmt_suites & WPA_KEY_MGMT_SAE) > + suites[num_suites++] = RSN_AUTH_KEY_MGMT_SAE; > + if (params->key_mgmt_suites & WPA_KEY_MGMT_OWE) > + suites[num_suites++] = RSN_AUTH_KEY_MGMT_OWE; > if (num_suites && > nla_put(msg, NL80211_ATTR_AKM_SUITES, num_suites * sizeof(u32), > suites)) Unfortunately, this cannot be done with the current cfg80211/nl80211 constraints. NL80211_MAX_NR_AKM_SUITES was defined to be 2 and if more than two values are included in NL80211_ATTR_AKM_SUITES, cfg80211 will reject he command. cfg80211 would first need to be extended to allow more AKM suites to be configured and this will unfortunately require number of changes to do properly without breaking backwards compatibility. There would likely need to be new mechanism for advertising maximum number of AKM suites in this attribute and hostapd/wpa_supplicant could then use that information from the running kernel to determine what to do based on how many AKM suites would need to be configured.
> On Thu, Feb 13, 2020 at 08:51:21AM +0000, Sergey Matyukevich wrote: > > Specify WPA3 AKM suites in set_ap command. Drivers may need to pass > > this information to firmware to setup SAE/OWE offload properly. > > Besides,drivers may use this information to perform early check > > SAE/OWE offload in hostapd. > > > diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c > > @@ -4202,6 +4202,10 @@ static int wpa_driver_nl80211_set_ap(void *priv, > > suites[num_suites++] = RSN_AUTH_KEY_MGMT_UNSPEC_802_1X; > > if (params->key_mgmt_suites & WPA_KEY_MGMT_PSK) > > suites[num_suites++] = RSN_AUTH_KEY_MGMT_PSK_OVER_802_1X; > > + if (params->key_mgmt_suites & WPA_KEY_MGMT_SAE) > > + suites[num_suites++] = RSN_AUTH_KEY_MGMT_SAE; > > + if (params->key_mgmt_suites & WPA_KEY_MGMT_OWE) > > + suites[num_suites++] = RSN_AUTH_KEY_MGMT_OWE; > > if (num_suites && > > nla_put(msg, NL80211_ATTR_AKM_SUITES, num_suites * sizeof(u32), > > suites)) > > Unfortunately, this cannot be done with the current cfg80211/nl80211 > constraints. NL80211_MAX_NR_AKM_SUITES was defined to be 2 and if more > than two values are included in NL80211_ATTR_AKM_SUITES, cfg80211 will > reject he command. cfg80211 would first need to be extended to allow > more AKM suites to be configured and this will unfortunately require > number of changes to do properly without breaking backwards > compatibility. There would likely need to be new mechanism for > advertising maximum number of AKM suites in this attribute and > hostapd/wpa_supplicant could then use that information from the running > kernel to determine what to do based on how many AKM suites would need > to be configured. Thanks for clarification. Lets assume that we would like to support at least pure OWE or SAE configuration for the time being. Then what do you think about the change along the following lines: diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c index 7305ed60b..b5b24a921 100644 --- a/src/drivers/driver_nl80211.c +++ b/src/drivers/driver_nl80211.c @@ -4202,6 +4202,12 @@ static int wpa_driver_nl80211_set_ap(void *priv, suites[num_suites++] = RSN_AUTH_KEY_MGMT_UNSPEC_802_1X; if (params->key_mgmt_suites & WPA_KEY_MGMT_PSK) suites[num_suites++] = RSN_AUTH_KEY_MGMT_PSK_OVER_802_1X; + if (num_suites < NL80211_MAX_NR_AKM_SUITES && + params->key_mgmt_suites & WPA_KEY_MGMT_SAE) + suites[num_suites++] = RSN_AUTH_KEY_MGMT_SAE; + if (num_suites < NL80211_MAX_NR_AKM_SUITES && + params->key_mgmt_suites & WPA_KEY_MGMT_OWE) + suites[num_suites++] = RSN_AUTH_KEY_MGMT_OWE; if (num_suites && nla_put(msg, NL80211_ATTR_AKM_SUITES, num_suites * sizeof(u32), suites)) Regards, Sergey
On Thu, Feb 13, 2020 at 04:32:44PM +0000, Sergey Matyukevich wrote: > Thanks for clarification. Lets assume that we would like to support > at least pure OWE or SAE configuration for the time being. Then > what do you think about the change along the following lines: > > diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c > @@ -4202,6 +4202,12 @@ static int wpa_driver_nl80211_set_ap(void *priv, > suites[num_suites++] = RSN_AUTH_KEY_MGMT_UNSPEC_802_1X; > if (params->key_mgmt_suites & WPA_KEY_MGMT_PSK) > suites[num_suites++] = RSN_AUTH_KEY_MGMT_PSK_OVER_802_1X; > + if (num_suites < NL80211_MAX_NR_AKM_SUITES && > + params->key_mgmt_suites & WPA_KEY_MGMT_SAE) > + suites[num_suites++] = RSN_AUTH_KEY_MGMT_SAE; > + if (num_suites < NL80211_MAX_NR_AKM_SUITES && > + params->key_mgmt_suites & WPA_KEY_MGMT_OWE) > + suites[num_suites++] = RSN_AUTH_KEY_MGMT_OWE; > if (num_suites && > nla_put(msg, NL80211_ATTR_AKM_SUITES, num_suites * sizeof(u32), > suites)) This can result in conflicting configuration since anything beyond NL80211_MAX_NR_AKM_SUITES would be ignored from kernel side configuration while hostapd internally would have additional AKMs enabled. I don't think this would be a good thing to do. Really, this needs cfg80211 to be extended to allow more AKM suites to be configured. If any workaround is needed before that happens, I think the only acceptable approach would be to allow cases where only one or two AKMs are enabled in the configuration. In other words, wpa_driver_nl80211_set_ap() could be extended with SAE and OWE (and other AKM suites for that matter) as long as it does not pass NL80211_ATTR_AKM_SUITES, to the kernel if more than NL80211_MAX_NR_AKM_SUITES suites are enabled.
> On Thu, Feb 13, 2020 at 04:32:44PM +0000, Sergey Matyukevich wrote: > > Thanks for clarification. Lets assume that we would like to support > > at least pure OWE or SAE configuration for the time being. Then > > what do you think about the change along the following lines: > > > > diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c > > @@ -4202,6 +4202,12 @@ static int wpa_driver_nl80211_set_ap(void *priv, > > suites[num_suites++] = RSN_AUTH_KEY_MGMT_UNSPEC_802_1X; > > if (params->key_mgmt_suites & WPA_KEY_MGMT_PSK) > > suites[num_suites++] = RSN_AUTH_KEY_MGMT_PSK_OVER_802_1X; > > + if (num_suites < NL80211_MAX_NR_AKM_SUITES && > > + params->key_mgmt_suites & WPA_KEY_MGMT_SAE) > > + suites[num_suites++] = RSN_AUTH_KEY_MGMT_SAE; > > + if (num_suites < NL80211_MAX_NR_AKM_SUITES && > > + params->key_mgmt_suites & WPA_KEY_MGMT_OWE) > > + suites[num_suites++] = RSN_AUTH_KEY_MGMT_OWE; > > if (num_suites && > > nla_put(msg, NL80211_ATTR_AKM_SUITES, num_suites * sizeof(u32), > > suites)) > > This can result in conflicting configuration since anything beyond > NL80211_MAX_NR_AKM_SUITES would be ignored from kernel side > configuration while hostapd internally would have additional AKMs > enabled. I don't think this would be a good thing to do. > > Really, this needs cfg80211 to be extended to allow more AKM suites to > be configured. If any workaround is needed before that happens, I think > the only acceptable approach would be to allow cases where only one or > two AKMs are enabled in the configuration. In other words, > wpa_driver_nl80211_set_ap() could be extended with SAE and OWE (and > other AKM suites for that matter) as long as it does not pass > NL80211_ATTR_AKM_SUITES, to the kernel if more than > NL80211_MAX_NR_AKM_SUITES suites are enabled. Ok. That make total sense. I will take a look at the required cfg80211 changes. Meanwhile, following your logic, existing hostapd code has the same issue with possible conflict between kernel and hostapd configuration. Hostapd may have SAE/OWE bit, but now it does not inform kernel about it. So, unless I am missing something, it looks like checking the total amount of suites and appropriate error is needed anyway: diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c index f1c98b90b..f2c43e80a 100644 --- a/src/drivers/driver_nl80211.c +++ b/src/drivers/driver_nl80211.c @@ -4202,9 +4202,15 @@ static int wpa_driver_nl80211_set_ap(void *priv, suites[num_suites++] = RSN_AUTH_KEY_MGMT_UNSPEC_802_1X; if (params->key_mgmt_suites & WPA_KEY_MGMT_PSK) suites[num_suites++] = RSN_AUTH_KEY_MGMT_PSK_OVER_802_1X; - if (num_suites && + if (params->key_mgmt_suites & WPA_KEY_MGMT_SAE) + suites[num_suites++] = RSN_AUTH_KEY_MGMT_SAE; + if (params->key_mgmt_suites & WPA_KEY_MGMT_OWE) + suites[num_suites++] = RSN_AUTH_KEY_MGMT_OWE; + + /* any other suites here ? */ + + if (num_suites && (num_suites > NL80211_MAX_NR_AKM_SUITES || nla_put(msg, NL80211_ATTR_AKM_SUITES, num_suites * sizeof(u32), - suites)) + suites))) goto fail; Later on, fixed NL80211_MAX_NR_AKM_SUITES can be replaced by the wiphy specific value configured by a driver and passed by cfg80211 to hostapd. Thoughts ? Comments ? Regards, Sergey
On Sun, Feb 16, 2020 at 06:02:06PM +0300, Sergey Matyukevich wrote: > Meanwhile, following your logic, existing hostapd code has the same issue > with possible conflict between kernel and hostapd configuration. Hostapd > may have SAE/OWE bit, but now it does not inform kernel about it. Yes, that's a known issue that was waiting for someone to get motivated enough to address the cfg80211/nl80211 side of this.. The main difference with the previous state was, though, in not being able to hit the limit that would make the full command itself fail, i.e., not breaking this for any driver that does not use information from NL80211_MAX_NR_AKM_SUITES. > So, unless I am missing something, it looks like checking the total amount > of suites and appropriate error is needed anyway: > diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c > @@ -4202,9 +4202,15 @@ static int wpa_driver_nl80211_set_ap(void *priv, > + if (params->key_mgmt_suites & WPA_KEY_MGMT_SAE) > + suites[num_suites++] = RSN_AUTH_KEY_MGMT_SAE; > + if (params->key_mgmt_suites & WPA_KEY_MGMT_OWE) > + suites[num_suites++] = RSN_AUTH_KEY_MGMT_OWE; > + > + /* any other suites here ? */ Yes, lots of them.. > + if (num_suites && (num_suites > NL80211_MAX_NR_AKM_SUITES || > nla_put(msg, NL80211_ATTR_AKM_SUITES, num_suites * sizeof(u32), > - suites)) > + suites))) > goto fail; This is not acceptable. This would break all cases where more than two AKMs are used. That must not happen for drivers that do not use NL80211_ATTR_AKM_SUITES. > Later on, fixed NL80211_MAX_NR_AKM_SUITES can be replaced by the wiphy > specific value configured by a driver and passed by cfg80211 to hostapd. Yes, this part can be done separately and should indeed be done. As far as the temporary workaround is concerned, I applied this change to handle all cases where at most two AKM suites are configured: https://w1.fi/cgit/hostap/commit/?id=dd74ddd0dff67c59e416bee9f764b27044a2ade5 This does not work with more than two AKM suites if the driver needs NL80211_ATTR_AKM_SUITES, but continues to work fine if the driver does not need that. It would be nicer to be able to reject the cases where this attribute is known to be needed, but cannot be added, but that does not seem to be something that could be easily determined with the current cfg80211 design, so this may be the best that can be done for now. Once the kernel extension becomes available, this can be addressed by working fine with new kernel versions but falling back to this removal of attribute for cases where things may or may not work based on the driver needs.
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c index 7305ed60b..5d54614fc 100644 --- a/src/drivers/driver_nl80211.c +++ b/src/drivers/driver_nl80211.c @@ -4202,6 +4202,10 @@ static int wpa_driver_nl80211_set_ap(void *priv, suites[num_suites++] = RSN_AUTH_KEY_MGMT_UNSPEC_802_1X; if (params->key_mgmt_suites & WPA_KEY_MGMT_PSK) suites[num_suites++] = RSN_AUTH_KEY_MGMT_PSK_OVER_802_1X; + if (params->key_mgmt_suites & WPA_KEY_MGMT_SAE) + suites[num_suites++] = RSN_AUTH_KEY_MGMT_SAE; + if (params->key_mgmt_suites & WPA_KEY_MGMT_OWE) + suites[num_suites++] = RSN_AUTH_KEY_MGMT_OWE; if (num_suites && nla_put(msg, NL80211_ATTR_AKM_SUITES, num_suites * sizeof(u32), suites))
Specify WPA3 AKM suites in set_ap command. Drivers may need to pass this information to firmware to setup SAE/OWE offload properly. Besides,drivers may use this information to perform early check SAE/OWE offload in hostapd. Signed-off-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com> --- src/drivers/driver_nl80211.c | 4 ++++ 1 file changed, 4 insertions(+)