[PATCH v3 1/7] Fixes: 6e8b7652788ac (add check for CMD_AUTH/CMD_ASSOC support)

[PATCH v3 1/7] Fixes: 6e8b7652788ac (add check for CMD_AUTH/CMD_ASSOC support)

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

Commit 6e8b76527 added a switch statement for AKM suites which
was not correct as this is a bitmask and may contain multiple
values. Intead we can rely on wiphy_select_akm which is a more
robust check anyways.
---
 src/wiphy.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

v3:
 * Return the result of wiphy_select_akm
 * Pass false for fils_hint parameter

diff --git a/src/wiphy.c b/src/wiphy.c
index 676f236c..ab57a2d2 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -407,24 +407,7 @@ bool wiphy_can_connect(struct wiphy *wiphy, struct scan_bss *bss)
 					rsn_info.group_management_cipher))
 			return false;
 
-
-		switch (rsn_info.akm_suites) {
-		case IE_RSN_AKM_SUITE_SAE_SHA256:
-		case IE_RSN_AKM_SUITE_FT_OVER_SAE_SHA256:
-			if (!wiphy_can_connect_sae(wiphy))
-				return false;
-
-			break;
-		case IE_RSN_AKM_SUITE_OWE:
-		case IE_RSN_AKM_SUITE_FILS_SHA256:
-		case IE_RSN_AKM_SUITE_FILS_SHA384:
-		case IE_RSN_AKM_SUITE_FT_OVER_FILS_SHA256:
-		case IE_RSN_AKM_SUITE_FT_OVER_FILS_SHA384:
-			if (!wiphy->support_cmds_auth_assoc)
-				return false;
-
-			break;
-		}
+		return wiphy_select_akm(wiphy, bss, false);
 	} else if (r != -ENOENT)
 		return false;
 
-- 
2.26.2

[PATCH v3 2/7] wiphy: add offload out parameter to wiphy_can_connect_sae

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]

This will be set if the SAE connection requires offloading to
work.
---
 src/wiphy.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/wiphy.c b/src/wiphy.c
index ab57a2d2..cf8c7c56 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -126,7 +126,7 @@ enum ie_rsn_cipher_suite wiphy_select_cipher(struct wiphy *wiphy, uint16_t mask)
 	return 0;
 }
 
-static bool wiphy_can_connect_sae(struct wiphy *wiphy)
+static bool wiphy_can_connect_sae(struct wiphy *wiphy, bool *offload)
 {
 	/*
 	 * SAE support in the kernel is a complete mess in that there are 3
@@ -153,8 +153,11 @@ static bool wiphy_can_connect_sae(struct wiphy *wiphy)
 
 	if (wiphy_has_feature(wiphy, NL80211_FEATURE_SAE)) {
 		/* Case (1) */
-		if (wiphy->support_cmds_auth_assoc)
+		if (wiphy->support_cmds_auth_assoc) {
+			if (offload)
+				*offload = false;
 			return true;
+		}
 
 		/*
 		 * Case (3)
@@ -165,8 +168,12 @@ static bool wiphy_can_connect_sae(struct wiphy *wiphy)
 	} else {
 		/* Case (2) */
 		if (wiphy_has_ext_feature(wiphy,
-					NL80211_EXT_FEATURE_SAE_OFFLOAD))
+					NL80211_EXT_FEATURE_SAE_OFFLOAD)) {
+			if (offload)
+				*offload = true;
+
 			return true;
+		}
 
 		return false;
 	}
@@ -234,7 +241,7 @@ enum ie_rsn_akm_suite wiphy_select_akm(struct wiphy *wiphy,
 				goto wpa2_personal;
 			}
 
-			if (!wiphy_can_connect_sae(wiphy))
+			if (!wiphy_can_connect_sae(wiphy, NULL))
 				goto wpa2_personal;
 
 			if (info.akm_suites &
-- 
2.26.2

[PATCH v3 3/7] wiphy: allow FT AKM to be used if Auth/Assoc is not supported

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 5426 bytes --]

Until now FT was only supported via Auth/Assoc commands which barred
any fullmac cards from using FT AKMs. With PSK offload support these
cards can do FT but only when offloading is used. This situation
is checked now by wiphy_select_akm and a boolean is returned as an
out parameter indicating if offloading is needed.

A new option [General].4WayOffload was added which is required to
enable this functionality. This was done mainly because it is
unknown how stable firmware offloading is from card to card.

Note: Enabling offload has one strange side effect which could
cause users problems when experimenting with this option. Once
offload is enabled the firmware no longer forwards eapol frames
for subsequent connections, even if offload is no longer being
used. Once enabled a user must restart the firmware (e.g. via
rebooting the machine) if they want to turn off the option.
---
 src/p2p.c     |  2 +-
 src/station.c |  2 +-
 src/wiphy.c   | 50 ++++++++++++++++++++++++++++++++++++++++++--------
 src/wiphy.h   |  3 ++-
 4 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/src/p2p.c b/src/p2p.c
index e3130e21..7ee753a2 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -1475,7 +1475,7 @@ static void p2p_try_connect_group(struct p2p_device *dev)
 
 	scan_bss_get_rsn_info(bss, &bss_info);
 
-	rsn_info.akm_suites = wiphy_select_akm(dev->wiphy, bss, false);
+	rsn_info.akm_suites = wiphy_select_akm(dev->wiphy, bss, false, NULL);
 	if (!rsn_info.akm_suites)
 		goto not_supported;
 
diff --git a/src/station.c b/src/station.c
index cf4dd2e7..0329d990 100644
--- a/src/station.c
+++ b/src/station.c
@@ -824,7 +824,7 @@ static int station_build_handshake_rsn(struct handshake_state *hs,
 	if (security == SECURITY_8021X && hs->support_fils)
 		fils_hint = station_has_erp_identity(network);
 
-	info.akm_suites = wiphy_select_akm(wiphy, bss, fils_hint);
+	info.akm_suites = wiphy_select_akm(wiphy, bss, fils_hint, NULL);
 
 	/*
 	 * Special case for OWE. With OWE we still need to build up the
diff --git a/src/wiphy.c b/src/wiphy.c
index cf8c7c56..b6b82933 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -181,16 +181,24 @@ static bool wiphy_can_connect_sae(struct wiphy *wiphy, bool *offload)
 
 enum ie_rsn_akm_suite wiphy_select_akm(struct wiphy *wiphy,
 					struct scan_bss *bss,
-					bool fils_capable_hint)
+					bool fils_capable_hint,
+					bool *offload)
 {
 	struct ie_rsn_info info;
 	enum security security;
+	bool offload_on;
+	bool offload_needed;
+	const struct l_settings *settings = iwd_get_config();
 
 	memset(&info, 0, sizeof(info));
 	scan_bss_get_rsn_info(bss, &info);
 
 	security = security_determine(bss->capability, &info);
 
+	if (!l_settings_get_bool(settings, "General", "4WayOffload",
+					&offload_on))
+		offload_on = false;
+
 	/*
 	 * If FT is available, use FT authentication to keep the door open
 	 * for fast transitions.  Otherwise use SHA256 version if present.
@@ -241,22 +249,48 @@ enum ie_rsn_akm_suite wiphy_select_akm(struct wiphy *wiphy,
 				goto wpa2_personal;
 			}
 
-			if (!wiphy_can_connect_sae(wiphy, NULL))
+			if (!wiphy_can_connect_sae(wiphy, &offload_needed))
+				goto wpa2_personal;
+
+			if (offload_needed && !offload_on)
 				goto wpa2_personal;
 
 			if (info.akm_suites &
-					IE_RSN_AKM_SUITE_FT_OVER_SAE_SHA256)
+					IE_RSN_AKM_SUITE_FT_OVER_SAE_SHA256) {
+				if (offload)
+					*offload = offload_needed;
+
 				return IE_RSN_AKM_SUITE_FT_OVER_SAE_SHA256;
+			}
+
+			if (info.akm_suites & IE_RSN_AKM_SUITE_SAE_SHA256) {
+				if (offload)
+					*offload = offload_needed;
 
-			if (info.akm_suites & IE_RSN_AKM_SUITE_SAE_SHA256)
 				return IE_RSN_AKM_SUITE_SAE_SHA256;
+			}
 		}
 
 wpa2_personal:
+		/*
+		 * Allow FT if either Auth/Assoc is supported OR if the card
+		 * supports PSK offload. Without Auth/Assoc, PSK offload is the
+		 * only mechanism to allow FT on these cards.
+		 */
 		if ((info.akm_suites & IE_RSN_AKM_SUITE_FT_USING_PSK) &&
-				bss->rsne && bss->mde_present &&
-				wiphy->support_cmds_auth_assoc)
-			return IE_RSN_AKM_SUITE_FT_USING_PSK;
+					bss->rsne && bss->mde_present) {
+			if (wiphy->support_cmds_auth_assoc)
+				return IE_RSN_AKM_SUITE_FT_USING_PSK;
+
+			if (wiphy_has_ext_feature(wiphy,
+				NL80211_EXT_FEATURE_4WAY_HANDSHAKE_STA_PSK) &&
+					offload_on) {
+				if (offload)
+					*offload = true;
+
+				return IE_RSN_AKM_SUITE_FT_USING_PSK;
+			}
+		}
 
 		if (info.akm_suites & IE_RSN_AKM_SUITE_PSK_SHA256)
 			return IE_RSN_AKM_SUITE_PSK_SHA256;
@@ -414,7 +448,7 @@ bool wiphy_can_connect(struct wiphy *wiphy, struct scan_bss *bss)
 					rsn_info.group_management_cipher))
 			return false;
 
-		return wiphy_select_akm(wiphy, bss, false);
+		return wiphy_select_akm(wiphy, bss, false, NULL);
 	} else if (r != -ENOENT)
 		return false;
 
diff --git a/src/wiphy.h b/src/wiphy.h
index 6c91220c..7ac1df95 100644
--- a/src/wiphy.h
+++ b/src/wiphy.h
@@ -57,7 +57,8 @@ enum ie_rsn_cipher_suite wiphy_select_cipher(struct wiphy *wiphy,
 							uint16_t mask);
 enum ie_rsn_akm_suite wiphy_select_akm(struct wiphy *wiphy,
 					struct scan_bss *bss,
-					bool fils_capable_hint);
+					bool fils_capable_hint,
+					bool *offload);
 
 struct wiphy *wiphy_find(int wiphy_id);
 #define wiphy_find_by_wdev(w) wiphy_find(w >> 32)
-- 
2.26.2

[PATCH v3 4/7] netdev: allow PSK offload for FT AKMs

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 4449 bytes --]

If the handshake has offloading set, use ATTR_PMK (for WPA2)
which enables PSK offloading.

The CMD_ROAM event path was also modified to take into account
handshake offloading. If the handshake is offloaded we still
must issue GET_SCAN, but not start eapol since the firmware
takes care of this.
---
 src/netdev.c | 56 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 22 deletions(-)

v3:
 * Changed around some logic deciding whether to start eapol, and
   how CMD_ROAM is handled.
 * Moved EVENT_SETTING_KEYS into a CMD_CONNECT path only. No need
   to do this for CMD_ROAM.

diff --git a/src/netdev.c b/src/netdev.c
index 914f6479..cb0f6829 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -1212,11 +1212,6 @@ static void netdev_connect_ok(struct netdev *netdev)
 		netdev->fw_roam_bss = NULL;
 	}
 
-	/* Allow station to sync the PSK to disk */
-	if (netdev->handshake && netdev->handshake->offload)
-		handshake_event(netdev->handshake,
-				HANDSHAKE_EVENT_SETTING_KEYS);
-
 	if (netdev->connect_cb) {
 		netdev->connect_cb(netdev, NETDEV_RESULT_OK, NULL,
 					netdev->user_data);
@@ -1985,26 +1980,25 @@ process_resp_ies:
 	}
 
 	/*
-	 * TODO: Only SAE/WPA3-personal offload is supported. In this case IWD
-	 * is 'done'. In the case of 8021x offload EAP still needs to take
-	 * place, so this must be updated accordingly when that is implemented.
+	 * Let station know about the roam so a state change can occur.
 	 */
-	if (netdev->handshake->offload)
-		goto done;
-
-	if (netdev->sm) {
-		/*
-		 * Let station know about the roam so a state change can occur.
-		 */
-		if (cmd == NL80211_CMD_ROAM) {
-			if (netdev->event_filter)
-				netdev->event_filter(netdev,
+	if (cmd == NL80211_CMD_ROAM) {
+		if (netdev->event_filter)
+			netdev->event_filter(netdev,
 						NETDEV_EVENT_ROAMING,
 						NULL, netdev->user_data);
-			/* EAPoL started after GET_SCAN */
+		/* EAPoL started after GET_SCAN */
+		if (!netdev->handshake->offload)
 			return;
-		}
 
+		/*
+		 * TODO: Only WPA2/WPA3-personal offload is supported. In this
+		 * case IWD is 'done'. In the case of 8021x offload EAP still
+		 * needs to take place, so this must be updated accordingly when
+		 * that is implemented.
+		 */
+		goto done;
+	} else if (netdev->sm) {
 		/*
 		 * Start processing EAPoL frames now that the state machine
 		 * has all the input data even in FT mode.
@@ -2015,6 +2009,11 @@ process_resp_ies:
 		return;
 	}
 
+	/* Allow station to sync the PSK to disk */
+	if (netdev->handshake && netdev->handshake->offload)
+		handshake_event(netdev->handshake,
+				HANDSHAKE_EVENT_SETTING_KEYS);
+
 done:
 	netdev_connect_ok(netdev);
 
@@ -2641,6 +2640,8 @@ static struct l_genl_msg *netdev_build_cmd_connect(struct netdev *netdev,
 		if (IE_AKM_IS_SAE(hs->akm_suite))
 			l_genl_msg_append_attr(msg, NL80211_ATTR_SAE_PASSWORD,
 					strlen(hs->passphrase), hs->passphrase);
+		else
+			l_genl_msg_append_attr(msg, NL80211_ATTR_PMK, 32, hs->pmk);
 	}
 
 	if (prev_bssid)
@@ -4000,7 +4001,7 @@ static bool netdev_get_fw_scan_cb(int err, struct l_queue *bss_list,
 	 * In this case we should just ignore this and allow the disconnect
 	 * logic to continue.
 	 */
-	if (!netdev->sm)
+	if (!netdev->handshake->offload && !netdev->sm)
 		return false;
 
 	if (err < 0) {
@@ -4028,6 +4029,11 @@ static bool netdev_get_fw_scan_cb(int err, struct l_queue *bss_list,
 
 	handshake_state_set_authenticator_ie(netdev->handshake, bss->rsne);
 
+	if (netdev->handshake->offload) {
+		netdev_connect_ok(netdev);
+		return false;
+	}
+
 	eapol_start(netdev->sm);
 
 	return false;
@@ -4063,14 +4069,20 @@ static bool netdev_roam_event(struct l_genl_msg *msg, struct netdev *netdev)
 		goto failed;
 	}
 
+	/* Handshake completed in firmware, just get the roamed BSS */
+	if (netdev->handshake->offload)
+		goto get_fw_scan;
+
 	/* Reset handshake state */
 	nhs->complete = false;
 	nhs->ptk_installed = false;
 	nhs->gtk_installed = true;
 	nhs->igtk_installed = true;
-	handshake_state_set_authenticator_address(netdev->handshake, mac);
 	netdev->handshake->ptk_complete = false;
 
+get_fw_scan:
+	handshake_state_set_authenticator_address(netdev->handshake, mac);
+
 	if (!scan_get_firmware_scan(netdev->wdev_id, netdev_get_fw_scan_cb,
 					netdev, NULL))
 		goto failed;
-- 
2.26.2

[PATCH v3 5/7] station: set handshake->offload if required

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1841 bytes --]

If the selected AKM requires offloading set the flag in the
handshake as such. This also removes the need for
wiphy_support_cmds_auth_assoc.
---
 src/station.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

v3:
 * Moved this logic into station_build_handshake_rsn

diff --git a/src/station.c b/src/station.c
index 0329d990..e161421b 100644
--- a/src/station.c
+++ b/src/station.c
@@ -802,6 +802,7 @@ static int station_build_handshake_rsn(struct handshake_state *hs,
 	enum security security = network_get_security(network);
 	bool add_mde = false;
 	bool fils_hint = false;
+	bool offload = false;
 
 	struct ie_rsn_info bss_info;
 	uint8_t rsne_buf[256];
@@ -824,7 +825,12 @@ static int station_build_handshake_rsn(struct handshake_state *hs,
 	if (security == SECURITY_8021X && hs->support_fils)
 		fils_hint = station_has_erp_identity(network);
 
-	info.akm_suites = wiphy_select_akm(wiphy, bss, fils_hint, NULL);
+	info.akm_suites = wiphy_select_akm(wiphy, bss, fils_hint, &offload);
+
+	hs->offload = offload;
+
+	if (hs->offload)
+		l_debug("Connection will offload 4-way handshake to firmware");
 
 	/*
 	 * Special case for OWE. With OWE we still need to build up the
@@ -981,16 +987,6 @@ static struct handshake_state *station_handshake_setup(struct station *station,
 				goto no_psk;
 
 			handshake_state_set_passphrase(hs, passphrase);
-
-			/*
-			 * TODO: This check isn't strictly correct since
-			 * some drivers may support EXTERNAL_AUTH but since
-			 * wiphy_can_connect takes this into account IWD should
-			 * have already rejected the connection if this was the
-			 * case.
-			 */
-			if (!wiphy_supports_cmds_auth_assoc(wiphy))
-				hs->offload = true;
 		} else {
 			const uint8_t *psk = network_get_psk(network);
 
-- 
2.26.2

[PATCH v3 6/7] doc: document new [General].4WayOffload

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1429 bytes --]

This option will allow the 4-way handshake to be offloaded to
firmware if it supports it.
---
 src/iwd.config.rst | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

v3:
 * re-worded the description to make clear that 4-way offloading does
   not always indicate a lack of Auth/Assoc support.

diff --git a/src/iwd.config.rst b/src/iwd.config.rst
index 478bd616..c9e67a2d 100644
--- a/src/iwd.config.rst
+++ b/src/iwd.config.rst
@@ -174,6 +174,21 @@ The group ``[General]`` contains general settings.
        off by default.  If you want to easily utilize Hotspot 2.0 networks,
        then setting ``DisableANQP`` to ``false`` is recommended.
 
+   * - 4WayOffload
+     - Values: **false**, true
+
+       Enables the use of 4-way handshake offloading. Some drivers can offload
+       the 4-way handshake to firmware which may be required to support certain
+       protocols that act on Authenticate/Associate frames (OWE, SAE, FT etc)
+       for drivers which do not support Authenticate/Associate commands. This
+       option is turned off by default as it is not widely tested.
+
+       Note: There seems to be a bug in the brcmfmac driver/firmware where once
+       offloading is enabled (by turning on this option) it cannot be turned
+       off in the firmware without a hard reboot (or some means of restarting
+       the firmware).
+
+
 Network
 ---------
 
-- 
2.26.2

[PATCH v3 7/7] wiphy: remove wiphy_supports_cmds_auth_assoc

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]

This is no longer needed.
---
 src/wiphy.c | 5 -----
 src/wiphy.h | 1 -
 2 files changed, 6 deletions(-)

diff --git a/src/wiphy.c b/src/wiphy.c
index b6b82933..a72773f7 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -455,11 +455,6 @@ bool wiphy_can_connect(struct wiphy *wiphy, struct scan_bss *bss)
 	return true;
 }
 
-bool wiphy_supports_cmds_auth_assoc(struct wiphy *wiphy)
-{
-	return wiphy->support_cmds_auth_assoc;
-}
-
 bool wiphy_has_feature(struct wiphy *wiphy, uint32_t feature)
 {
 	return wiphy->feature_flags & feature;
diff --git a/src/wiphy.h b/src/wiphy.h
index 7ac1df95..c83cd15a 100644
--- a/src/wiphy.h
+++ b/src/wiphy.h
@@ -80,7 +80,6 @@ uint32_t wiphy_get_supported_bands(struct wiphy *wiphy);
 const struct scan_freq_set *wiphy_get_supported_freqs(
 						const struct wiphy *wiphy);
 bool wiphy_can_connect(struct wiphy *wiphy, struct scan_bss *bss);
-bool wiphy_supports_cmds_auth_assoc(struct wiphy *wiphy);
 bool wiphy_can_randomize_mac_addr(struct wiphy *wiphy);
 bool wiphy_rrm_capable(struct wiphy *wiphy);
 bool wiphy_has_feature(struct wiphy *wiphy, uint32_t feature);
-- 
2.26.2

Re: [PATCH v3 1/7] Fixes: 6e8b7652788ac (add check for CMD_AUTH/CMD_ASSOC support)

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

Hi James,

On 3/30/21 4:09 PM, James Prestwood wrote:
> Commit 6e8b76527 added a switch statement for AKM suites which
> was not correct as this is a bitmask and may contain multiple
> values. Intead we can rely on wiphy_select_akm which is a more
> robust check anyways.

Fixes: is used as a tag, not a commit header.  Linux Kernel uses a special 
format for that, which we should do as well.

Just add this to your .git/config

[pretty]
         fixes = Fixes: %h (\"%s\")


And you can use git show --pretty=fixes 6e8b76527 to have it print in the right 
format.

> ---
>   src/wiphy.c | 19 +------------------
>   1 file changed, 1 insertion(+), 18 deletions(-)
> 
> v3:
>   * Return the result of wiphy_select_akm
>   * Pass false for fils_hint parameter
> 

I fixed up the commit description and applied, thanks.

Regards,
-Denis

Re: [PATCH v3 1/7] Fixes: 6e8b7652788ac (add check for CMD_AUTH/CMD_ASSOC support)

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]

On Tue, 2021-03-30 at 17:33 -0500, Denis Kenzior wrote:
> Hi James,
> 
> On 3/30/21 4:09 PM, James Prestwood wrote:
> > Commit 6e8b76527 added a switch statement for AKM suites which
> > was not correct as this is a bitmask and may contain multiple
> > values. Intead we can rely on wiphy_select_akm which is a more
> > robust check anyways.
> 
> Fixes: is used as a tag, not a commit header.  Linux Kernel uses a
> special 
> format for that, which we should do as well.
> 
> Just add this to your .git/config
> 
> [pretty]
>          fixes = Fixes: %h (\"%s\")
> 
> 
> And you can use git show --pretty=fixes 6e8b76527 to have it print in
> the right 
> format.

Ah ok thanks. I figured there was some git command to do this
automatically but wasn't sure.

> 
> > ---
> >   src/wiphy.c | 19 +------------------
> >   1 file changed, 1 insertion(+), 18 deletions(-)
> > 
> > v3:
> >   * Return the result of wiphy_select_akm
> >   * Pass false for fils_hint parameter
> > 
> 
> I fixed up the commit description and applied, thanks.
> 
> Regards,
> -Denis