| Message ID | 1539338353-62784-1-git-send-email-stanley.hsu@cypress.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | driver_nl80211: Fix 802.1X auth failure when offloading 4-way handshake | expand |
On Fri, Oct 12, 2018 at 09:59:30AM +0000, Stanley Hsu wrote: > If a driver advertises 4-way handshake offload support for 802.1X, it > may reject the NL80211_CMD_CONNECT when NL80211_ATTR_WANT_1X_4WAY_HS > attribute flag is not present. This patch includes the attribute flag > in NL80211_CMD_CONNECT for the offload. > diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c > @@ -5405,11 +5405,15 @@ static int nl80211_connect_common(struct wpa_driver_nl80211_data *drv, > - /* Add PSK in case of 4-way handshake offload */ > - if (params->psk && > - (drv->capa.flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE)) { > - wpa_hexdump_key(MSG_DEBUG, " * PSK", params->psk, 32); > - if (nla_put(msg, NL80211_ATTR_PMK, 32, params->psk)) > + if (drv->capa.flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE) { This WPA_DRIVER_FLAGS_4WAY_HANDSHAKE flag is currently set if either NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_PSK or NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X are advertised by the driver. > + /* Add PSK in case of 4-way handshake offload */ > + if (params->psk) { > + wpa_hexdump_key(MSG_DEBUG, " * PSK", params->psk, 32); > + if (nla_put(msg, NL80211_ATTR_PMK, 32, params->psk)) > + return -1; > + } > + if (wpa_key_mgmt_wpa_ieee8021x(params->key_mgmt_suite) && > + nla_put_flag(msg, NL80211_ATTR_WANT_1X_4WAY_HS)) > return -1; So this NL80211_ATTR_WANT_1X_4WAY_HS flag could be added even if the driver does not advertise NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X. Is that really the expected behavior here? Shouldn't NL80211_ATTR_WANT_1X_4WAY_HS be used only if the driver has advertised NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X? cfg80211 would seem to reject NL80211_CMD_CONNECT with NL80211_ATTR_WANT_1X_4WAY_HS if that feature flag is not advertised..
On Thu, 2018-11-22 at 15:49 +0200, Jouni Malinen wrote: > > So this NL80211_ATTR_WANT_1X_4WAY_HS flag could be added even if the > driver does not advertise NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X. Is > that really the expected behavior here? Shouldn't > NL80211_ATTR_WANT_1X_4WAY_HS be used only if the driver has advertised > NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_1X? cfg80211 would seem to reject > NL80211_CMD_CONNECT with NL80211_ATTR_WANT_1X_4WAY_HS if that feature > flag is not advertised.. IIRC this is a bit of a historic accident/problem - the first version(s) of the nl80211 extended feature bits didn't differentiate, but then eventually we changed that when it got merged. I think the code here - setting WPA_DRIVER_FLAGS_4WAY_HANDSHAKE based on both - predates those changes. I guess WPA_DRIVER_FLAGS_4WAY_HANDSHAKE should really be split similarly johannes
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c index 39a02d3ee51f..8699efd1ba51 100644 --- a/src/drivers/driver_nl80211.c +++ b/src/drivers/driver_nl80211.c @@ -5405,11 +5405,15 @@ static int nl80211_connect_common(struct wpa_driver_nl80211_data *drv, return -1; } - /* Add PSK in case of 4-way handshake offload */ - if (params->psk && - (drv->capa.flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE)) { - wpa_hexdump_key(MSG_DEBUG, " * PSK", params->psk, 32); - if (nla_put(msg, NL80211_ATTR_PMK, 32, params->psk)) + if (drv->capa.flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE) { + /* Add PSK in case of 4-way handshake offload */ + if (params->psk) { + wpa_hexdump_key(MSG_DEBUG, " * PSK", params->psk, 32); + if (nla_put(msg, NL80211_ATTR_PMK, 32, params->psk)) + return -1; + } + if (wpa_key_mgmt_wpa_ieee8021x(params->key_mgmt_suite) && + nla_put_flag(msg, NL80211_ATTR_WANT_1X_4WAY_HS)) return -1; }
If a driver advertises 4-way handshake offload support for 802.1X, it may reject the NL80211_CMD_CONNECT when NL80211_ATTR_WANT_1X_4WAY_HS attribute flag is not present. This patch includes the attribute flag in NL80211_CMD_CONNECT for the offload. Signed-off-by: Chung-Hsien Hsu <stanley.hsu@cypress.com> --- src/drivers/driver_nl80211.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)