diff mbox series

[2/2] drivers: nl80211: indicate 802.1X 4-way handshake offload in connect

Message ID 1546859681-4597-2-git-send-email-arend.vanspriel@broadcom.com
State Accepted
Headers show
Series [1/2] drivers: add separate driver flags for 802.1X and WPA/WPA2-Personal | expand

Commit Message

Arend van Spriel Jan. 7, 2019, 11:14 a.m. UTC
Upon issuing a connect request we need to indicate that we want the
driver to offload the 802.1X 4-way handshake for us. Indicate it if
the driver capability supports the offload.

Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
Hi Jouni,

Here the patch to indicate to the driver that it should offload the 802.1X
handshake. I am not entirely sure about all the key management suites that
are to be considered as 802.1X offload. I reused the req_key_mgmt_offload flag
as it sounded like a nice fit, but not sure if that could cause issues. At
least with the brcmfmac it seems to work as intended.

Regards,
Arend
---
 src/drivers/driver_nl80211.c    | 5 +++++
 wpa_supplicant/wpa_supplicant.c | 7 +++++++
 2 files changed, 12 insertions(+)

--
1.9.1

Comments

Jouni Malinen Jan. 7, 2019, 11:54 p.m. UTC | #1
On Mon, Jan 07, 2019 at 12:14:41PM +0100, Arend van Spriel wrote:
> Upon issuing a connect request we need to indicate that we want the
> driver to offload the 802.1X 4-way handshake for us. Indicate it if
> the driver capability supports the offload.

Thanks, applied.

> Here the patch to indicate to the driver that it should offload the 802.1X
> handshake. I am not entirely sure about all the key management suites that
> are to be considered as 802.1X offload. I reused the req_key_mgmt_offload flag
> as it sounded like a nice fit, but not sure if that could cause issues. At
> least with the brcmfmac it seems to work as intended.

req_key_mgmt_offload sounds fine for this. As far as the AKM suites are
concerned:

> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
> @@ -3113,6 +3113,13 @@ static void wpas_start_assoc_cb(struct wpa_radio_work *work, int deinit)
> +	if ((wpa_s->drv_flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE_8021X) &&
> +	    (params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X ||
> +	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SHA256 ||
> +	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B ||
> +	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B_192 ||
> +	     params.key_mgmt_suite == WPA_KEY_MGMT_FT_IEEE8021X))
> +		params.req_key_mgmt_offload = 1;

I dropped WPA_KEY_MGMT_FT_IEEE8021X for now since the initial mobility
domain association in FT has different rules for 4-way handshake and key
derivation. I would not be surprised if there are drivers that don't
support offload for it even if they can handle other AKMs. At minimum,
I'd prefer to add this only once someone has actually confirmed it
working (and also allowing FT protocol to be used after the initial
mobility domain association). FILS AKMs might also be here for the
initial connection, but same comment about testing preference applies
for it as well. Other AKMs might also work (e.g., OWE, OSEN), but need
testing and potentially new driver capability indication flags/lists..
Arend van Spriel Jan. 8, 2019, 9:51 a.m. UTC | #2
On 1/8/2019 12:54 AM, Jouni Malinen wrote:
> On Mon, Jan 07, 2019 at 12:14:41PM +0100, Arend van Spriel wrote:
>> Upon issuing a connect request we need to indicate that we want the
>> driver to offload the 802.1X 4-way handshake for us. Indicate it if
>> the driver capability supports the offload.
> 
> Thanks, applied.
> 
>> Here the patch to indicate to the driver that it should offload the 802.1X
>> handshake. I am not entirely sure about all the key management suites that
>> are to be considered as 802.1X offload. I reused the req_key_mgmt_offload flag
>> as it sounded like a nice fit, but not sure if that could cause issues. At
>> least with the brcmfmac it seems to work as intended.
> 
> req_key_mgmt_offload sounds fine for this. As far as the AKM suites are
> concerned:
> 
>> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
>> @@ -3113,6 +3113,13 @@ static void wpas_start_assoc_cb(struct wpa_radio_work *work, int deinit)
>> +	if ((wpa_s->drv_flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE_8021X) &&
>> +	    (params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X ||
>> +	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SHA256 ||
>> +	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B ||
>> +	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B_192 ||
>> +	     params.key_mgmt_suite == WPA_KEY_MGMT_FT_IEEE8021X))
>> +		params.req_key_mgmt_offload = 1;
> 
> I dropped WPA_KEY_MGMT_FT_IEEE8021X for now since the initial mobility
> domain association in FT has different rules for 4-way handshake and key
> derivation. I would not be surprised if there are drivers that don't
> support offload for it even if they can handle other AKMs. At minimum,
> I'd prefer to add this only once someone has actually confirmed it
> working (and also allowing FT protocol to be used after the initial
> mobility domain association). FILS AKMs might also be here for the
> initial connection, but same comment about testing preference applies
> for it as well. Other AKMs might also work (e.g., OWE, OSEN), but need
> testing and potentially new driver capability indication flags/lists..

I added FT here because in brcmfmac two related commits were added by 
Cypress folks:

commit a858376cdbb3 ("brcmfmac: add 4-way handshake offload detection 
for FT-802.1X")
commit 4ad298da9392 ("brcmfmac: add FT-based AKMs in 
brcmf_set_key_mgmt() for FT support")

And there is also a patch pending in linux-wireless patchwork 
("brcmfmac: send port authorized event for FT-802.1X [1]") regarding 
roaming behavior for FT protocol. However, I can imagine not all drivers 
could do this and we may need another feature flag for this or a list of 
supported AKM suites for offload.

Regards,
Arend

[1] https://patchwork.kernel.org/patch/10748067/
Jouni Malinen Jan. 8, 2019, 10:46 a.m. UTC | #3
On Tue, Jan 08, 2019 at 10:51:37AM +0100, Arend Van Spriel wrote:
> I added FT here because in brcmfmac two related commits were added by
> Cypress folks:
> 
> commit a858376cdbb3 ("brcmfmac: add 4-way handshake offload detection for
> FT-802.1X")
> commit 4ad298da9392 ("brcmfmac: add FT-based AKMs in brcmf_set_key_mgmt()
> for FT support")
> 
> And there is also a patch pending in linux-wireless patchwork ("brcmfmac:
> send port authorized event for FT-802.1X [1]") regarding roaming behavior
> for FT protocol. However, I can imagine not all drivers could do this and we
> may need another feature flag for this or a list of supported AKM suites for
> offload.

> [1] https://patchwork.kernel.org/patch/10748067/

Interesting. I'd be fine adding back the FT AKM, but I'd like to see a
wpa_supplicant debug log showing a sequence of initial mobility domain
association followed by FT protocol roaming. The key derivation for this
case is quite a bit different, but then again, I'd also assume that in
this case both the 4-way handshake and FT protocol exchange are actually
offloaded to the driver/firmware, so that might work fine. Well,
assuming it also takes care of PTK/GTK rekeying exchanges (i.e.,
offloads all EAPOL-Key frame handling).

Even EAPOL-Key error reporting (e.g., Michael MIC failures for TKIP, but
that's not the only use case for these) should be made sure to work when
it is initiated by wpa_supplicant.. That's one part where the derived
PTK (KEK and KCK) need to be synchronized and I had not realized there
would be sufficient functionality for this in upstream cfg80211/nl80211
design yet.
diff mbox series

Patch

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 5081b5b..e260d56 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -5568,6 +5568,11 @@  static int nl80211_connect_common(struct wpa_driver_nl80211_data *drv,
 			return -1;
 	}

+	if (params->req_key_mgmt_offload &&
+	    (drv->capa.flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE_8021X) &&
+	    nla_put_flag(msg, NL80211_ATTR_WANT_1X_4WAY_HS))
+		return -1;
+
 	/* Add PSK in case of 4-way handshake offload */
 	if (params->psk &&
 	    (drv->capa.flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE_PSK)) {
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 7d80946..68f2b1f 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -3113,6 +3113,13 @@  static void wpas_start_assoc_cb(struct wpa_radio_work *work, int deinit)
 		if (ssid->psk_set)
 			params.psk = ssid->psk;
 	}
+	if ((wpa_s->drv_flags & WPA_DRIVER_FLAGS_4WAY_HANDSHAKE_8021X) &&
+	    (params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X ||
+	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SHA256 ||
+	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B ||
+	     params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X_SUITE_B_192 ||
+	     params.key_mgmt_suite == WPA_KEY_MGMT_FT_IEEE8021X))
+		params.req_key_mgmt_offload = 1;

 	if (wpa_s->conf->key_mgmt_offload) {
 		if (params.key_mgmt_suite == WPA_KEY_MGMT_IEEE8021X ||