All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/7] wiphy: add offload out parameter to wiphy_can_connect_sae
@ 2021-03-30 18:48 James Prestwood
  2021-03-30 18:48 ` [PATCH v2 2/7] wiphy: fix wiphy_can_connect AKM checks James Prestwood
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: James Prestwood @ 2021-03-30 18:48 UTC (permalink / raw)
  To: iwd

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

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

diff --git a/src/wiphy.c b/src/wiphy.c
index 676f236c..35872577 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 &
@@ -411,7 +418,7 @@ bool wiphy_can_connect(struct wiphy *wiphy, struct scan_bss *bss)
 		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))
+			if (!wiphy_can_connect_sae(wiphy, NULL))
 				return false;
 
 			break;
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/7] wiphy: fix wiphy_can_connect AKM checks
  2021-03-30 18:48 [PATCH v2 1/7] wiphy: add offload out parameter to wiphy_can_connect_sae James Prestwood
@ 2021-03-30 18:48 ` James Prestwood
  2021-03-30 19:58   ` Denis Kenzior
  2021-03-30 18:48 ` [PATCH v2 3/7] wiphy: allow FT AKM to be used if Auth/Assoc is not supported James Prestwood
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: James Prestwood @ 2021-03-30 18:48 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 1391 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 | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/src/wiphy.c b/src/wiphy.c
index 35872577..f570495e 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -414,24 +414,14 @@ bool wiphy_can_connect(struct wiphy *wiphy, struct scan_bss *bss)
 					rsn_info.group_management_cipher))
 			return false;
 
+		/*
+		 * Just assume FILS capable at this point. Station will verify
+		 * this later, but at this point we cannot know if the network
+		 * settings have the needed FILS identity.
+		 */
+		if (!wiphy_select_akm(wiphy, bss, true))
+			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, NULL))
-				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;
-		}
 	} else if (r != -ENOENT)
 		return false;
 
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/7] wiphy: allow FT AKM to be used if Auth/Assoc is not supported
  2021-03-30 18:48 [PATCH v2 1/7] wiphy: add offload out parameter to wiphy_can_connect_sae James Prestwood
  2021-03-30 18:48 ` [PATCH v2 2/7] wiphy: fix wiphy_can_connect AKM checks James Prestwood
@ 2021-03-30 18:48 ` James Prestwood
  2021-03-30 18:48 ` [PATCH v2 4/7] netdev: allow PSK offload for FT AKMs James Prestwood
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: James Prestwood @ 2021-03-30 18:48 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 5479 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 f570495e..b95f3d67 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;
@@ -419,7 +453,7 @@ bool wiphy_can_connect(struct wiphy *wiphy, struct scan_bss *bss)
 		 * this later, but at this point we cannot know if the network
 		 * settings have the needed FILS identity.
 		 */
-		if (!wiphy_select_akm(wiphy, bss, true))
+		if (!wiphy_select_akm(wiphy, bss, true, NULL))
 			return false;
 
 	} else if (r != -ENOENT)
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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/7] netdev: allow PSK offload for FT AKMs
  2021-03-30 18:48 [PATCH v2 1/7] wiphy: add offload out parameter to wiphy_can_connect_sae James Prestwood
  2021-03-30 18:48 ` [PATCH v2 2/7] wiphy: fix wiphy_can_connect AKM checks James Prestwood
  2021-03-30 18:48 ` [PATCH v2 3/7] wiphy: allow FT AKM to be used if Auth/Assoc is not supported James Prestwood
@ 2021-03-30 18:48 ` James Prestwood
  2021-03-30 20:25   ` Denis Kenzior
  2021-03-30 18:48 ` [PATCH v2 5/7] station: set handshake->offload if required James Prestwood
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: James Prestwood @ 2021-03-30 18:48 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 3266 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 | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index 914f6479..5c5fcd86 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -1992,19 +1992,7 @@ process_resp_ies:
 	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,
-						NETDEV_EVENT_ROAMING,
-						NULL, netdev->user_data);
-			/* EAPoL started after GET_SCAN */
-			return;
-		}
-
+	if (netdev->sm && cmd != NL80211_CMD_ROAM) {
 		/*
 		 * Start processing EAPoL frames now that the state machine
 		 * has all the input data even in FT mode.
@@ -2016,6 +2004,19 @@ process_resp_ies:
 	}
 
 done:
+	/*
+	 * 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,
+						NETDEV_EVENT_ROAMING,
+						NULL, netdev->user_data);
+		/* EAPoL started after GET_SCAN */
+		if (!netdev->handshake->offload)
+			return;
+	}
+
 	netdev_connect_ok(netdev);
 
 	return;
@@ -2641,6 +2642,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 +4003,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 +4031,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 +4071,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 5/7] station: set handshake->offload if required
  2021-03-30 18:48 [PATCH v2 1/7] wiphy: add offload out parameter to wiphy_can_connect_sae James Prestwood
                   ` (2 preceding siblings ...)
  2021-03-30 18:48 ` [PATCH v2 4/7] netdev: allow PSK offload for FT AKMs James Prestwood
@ 2021-03-30 18:48 ` James Prestwood
  2021-03-30 18:48 ` [PATCH v2 6/7] doc: document new [General].4WayOffload James Prestwood
  2021-03-30 18:48 ` [PATCH v2 7/7] wiphy: remove wiphy_supports_cmds_auth_assoc James Prestwood
  5 siblings, 0 replies; 11+ messages in thread
From: James Prestwood @ 2021-03-30 18:48 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 1780 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(-)

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 6/7] doc: document new [General].4WayOffload
  2021-03-30 18:48 [PATCH v2 1/7] wiphy: add offload out parameter to wiphy_can_connect_sae James Prestwood
                   ` (3 preceding siblings ...)
  2021-03-30 18:48 ` [PATCH v2 5/7] station: set handshake->offload if required James Prestwood
@ 2021-03-30 18:48 ` James Prestwood
  2021-03-30 20:49   ` Denis Kenzior
  2021-03-30 18:48 ` [PATCH v2 7/7] wiphy: remove wiphy_supports_cmds_auth_assoc James Prestwood
  5 siblings, 1 reply; 11+ messages in thread
From: James Prestwood @ 2021-03-30 18:48 UTC (permalink / raw)
  To: iwd

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

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

diff --git a/src/iwd.config.rst b/src/iwd.config.rst
index 478bd616..e2060312 100644
--- a/src/iwd.config.rst
+++ b/src/iwd.config.rst
@@ -174,6 +174,20 @@ 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 is needed for protocols which
+       act on Authenticate/Associate frames such as Fast Transition. 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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 7/7] wiphy: remove wiphy_supports_cmds_auth_assoc
  2021-03-30 18:48 [PATCH v2 1/7] wiphy: add offload out parameter to wiphy_can_connect_sae James Prestwood
                   ` (4 preceding siblings ...)
  2021-03-30 18:48 ` [PATCH v2 6/7] doc: document new [General].4WayOffload James Prestwood
@ 2021-03-30 18:48 ` James Prestwood
  5 siblings, 0 replies; 11+ messages in thread
From: James Prestwood @ 2021-03-30 18:48 UTC (permalink / raw)
  To: iwd

[-- 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 b95f3d67..10b9c1f9 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -462,11 +462,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/7] wiphy: fix wiphy_can_connect AKM checks
  2021-03-30 18:48 ` [PATCH v2 2/7] wiphy: fix wiphy_can_connect AKM checks James Prestwood
@ 2021-03-30 19:58   ` Denis Kenzior
  0 siblings, 0 replies; 11+ messages in thread
From: Denis Kenzior @ 2021-03-30 19:58 UTC (permalink / raw)
  To: iwd

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

Hi James,

On 3/30/21 1:48 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.

Might as well use a Fixes: tag.

Also, please re-order this to be the first in the series.  No sense in touching 
wiphy_can_connect in patch 1 just to remove all traces in patch 2.

> ---
>   src/wiphy.c | 24 +++++++-----------------
>   1 file changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/src/wiphy.c b/src/wiphy.c
> index 35872577..f570495e 100644
> --- a/src/wiphy.c
> +++ b/src/wiphy.c
> @@ -414,24 +414,14 @@ bool wiphy_can_connect(struct wiphy *wiphy, struct scan_bss *bss)
>   					rsn_info.group_management_cipher))
>   			return false;
>   
> +		/*
> +		 * Just assume FILS capable at this point. Station will verify
> +		 * this later, but at this point we cannot know if the network
> +		 * settings have the needed FILS identity.
> +		 */
> +		if (!wiphy_select_akm(wiphy, bss, true))
> +			return false;

Why not return wiphy_select_akm() here?  Also, since we only support FILS on 
SoftMAC, the safer option might be false actually.

>   
> -		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, NULL))
> -				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;
> -		}
>   	} else if (r != -ENOENT)
>   		return false;
>   
> 

Regards,
-Denis

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/7] netdev: allow PSK offload for FT AKMs
  2021-03-30 18:48 ` [PATCH v2 4/7] netdev: allow PSK offload for FT AKMs James Prestwood
@ 2021-03-30 20:25   ` Denis Kenzior
  2021-03-30 20:40     ` James Prestwood
  0 siblings, 1 reply; 11+ messages in thread
From: Denis Kenzior @ 2021-03-30 20:25 UTC (permalink / raw)
  To: iwd

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

Hi James,

On 3/30/21 1:48 PM, James Prestwood wrote:
> 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 | 44 +++++++++++++++++++++++++++++---------------
>   1 file changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/src/netdev.c b/src/netdev.c
> index 914f6479..5c5fcd86 100644
> --- a/src/netdev.c
> +++ b/src/netdev.c
> @@ -1992,19 +1992,7 @@ process_resp_ies:
>   	if (netdev->handshake->offload)
>   		goto done;
>   

Hmm, would it not be simpler to move the above check into the below if statement 
instead?

> -	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,
> -						NETDEV_EVENT_ROAMING,
> -						NULL, netdev->user_data);
> -			/* EAPoL started after GET_SCAN */
> -			return;
> -		}
> -
> +	if (netdev->sm && cmd != NL80211_CMD_ROAM) {
>   		/*
>   		 * Start processing EAPoL frames now that the state machine
>   		 * has all the input data even in FT mode.
> @@ -2016,6 +2004,19 @@ process_resp_ies:
>   	}
>   
>   done:
> +	/*
> +	 * 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,
> +						NETDEV_EVENT_ROAMING,
> +						NULL, netdev->user_data);
> +		/* EAPoL started after GET_SCAN */
> +		if (!netdev->handshake->offload)
> +			return;
> +	}
> +
>   	netdev_connect_ok(netdev);
>   
>   	return;
> @@ -2641,6 +2642,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 +4003,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 +4031,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);

Hmm... netdev_connect_ok seems to invoke HANDSHAKE_EVENT_SETTING_KEYS event 
which is probably wrong on an offloaded roam event.

> +		return false;
> +	}
> +
>   	eapol_start(netdev->sm);
>   
>   	return false;
> @@ -4063,14 +4071,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;
> 

Regards,
-Denis

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/7] netdev: allow PSK offload for FT AKMs
  2021-03-30 20:25   ` Denis Kenzior
@ 2021-03-30 20:40     ` James Prestwood
  0 siblings, 0 replies; 11+ messages in thread
From: James Prestwood @ 2021-03-30 20:40 UTC (permalink / raw)
  To: iwd

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

On Tue, 2021-03-30 at 15:25 -0500, Denis Kenzior wrote:
> Hi James,
> 
> On 3/30/21 1:48 PM, James Prestwood wrote:
> > 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 | 44 +++++++++++++++++++++++++++++---------------
> >   1 file changed, 29 insertions(+), 15 deletions(-)
> > 
> > diff --git a/src/netdev.c b/src/netdev.c
> > index 914f6479..5c5fcd86 100644
> > --- a/src/netdev.c
> > +++ b/src/netdev.c
> > @@ -1992,19 +1992,7 @@ process_resp_ies:
> >   	if (netdev->handshake->offload)
> >   		goto done;
> >   
> 
> Hmm, would it not be simpler to move the above check into the below
> if statement 
> instead?

So I did this in order to get station to transition to roaming when
using offload, so we couldn't move the check into if (netdev->sm) since
sm is NULL for offload.

Basically we need to trigger the ROAMING event in both offload and non
offload cases if cmd == CMD_ROAM, meaning netdev->sm shouldn't have an
effect.

I do agree these checks got somewhat nasty. Let me see if I can re-
order something to make it look better.
> 
> > -	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,
> > -						NETDEV_EVENT_ROAMING,
> > -						NULL, netdev-
> > >user_data);
> > -			/* EAPoL started after GET_SCAN */
> > -			return;
> > -		}
> > -
> > +	if (netdev->sm && cmd != NL80211_CMD_ROAM) {
> >   		/*
> >   		 * Start processing EAPoL frames now that the state
> > machine
> >   		 * has all the input data even in FT mode.
> > @@ -2016,6 +2004,19 @@ process_resp_ies:
> >   	}
> >   
> >   done:
> > +	/*
> > +	 * 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,
> > +						NETDEV_EVENT_ROAMING,
> > +						NULL, netdev-
> > >user_data);
> > +		/* EAPoL started after GET_SCAN */
> > +		if (!netdev->handshake->offload)
> > +			return;
> > +	}
> > +
> >   	netdev_connect_ok(netdev);
> >   
> >   	return;
> > @@ -2641,6 +2642,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 +4003,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 +4031,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);
> 
> Hmm... netdev_connect_ok seems to invoke HANDSHAKE_EVENT_SETTING_KEYS
> event 
> which is probably wrong on an offloaded roam event.

Yeah, I can move this out into a non roaming path. Its wrong in both
offload and non offload roaming cases.
> 
> > +		return false;
> > +	}
> > +
> >   	eapol_start(netdev->sm);
> >   
> >   	return false;
> > @@ -4063,14 +4071,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;
> > 
> 
> Regards,
> -Denis

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 6/7] doc: document new [General].4WayOffload
  2021-03-30 18:48 ` [PATCH v2 6/7] doc: document new [General].4WayOffload James Prestwood
@ 2021-03-30 20:49   ` Denis Kenzior
  0 siblings, 0 replies; 11+ messages in thread
From: Denis Kenzior @ 2021-03-30 20:49 UTC (permalink / raw)
  To: iwd

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

Hi James,

On 3/30/21 1:48 PM, James Prestwood wrote:
> This option will allow the 4-way handshake to be offloaded to
> firmware if it supports it.
> ---
>   src/iwd.config.rst | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/src/iwd.config.rst b/src/iwd.config.rst
> index 478bd616..e2060312 100644
> --- a/src/iwd.config.rst
> +++ b/src/iwd.config.rst
> @@ -174,6 +174,20 @@ 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 is needed for protocols which
> +       act on Authenticate/Associate frames such as Fast Transition. This
> +       option is turned off by default as it is not widely tested.

Well, this isn't really true.  Handshake offload is just that, not needing to 
perform it in userspace.  You can in theory have a SoftMac driver with handshake 
offloading capabilities for example.

FT is a bit orthogonal and really depends on whether the firmware can roam by 
itself...

> +
> +       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
>   ---------
>   
> 

Regards,
-Denis

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-03-30 20:49 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-30 18:48 [PATCH v2 1/7] wiphy: add offload out parameter to wiphy_can_connect_sae James Prestwood
2021-03-30 18:48 ` [PATCH v2 2/7] wiphy: fix wiphy_can_connect AKM checks James Prestwood
2021-03-30 19:58   ` Denis Kenzior
2021-03-30 18:48 ` [PATCH v2 3/7] wiphy: allow FT AKM to be used if Auth/Assoc is not supported James Prestwood
2021-03-30 18:48 ` [PATCH v2 4/7] netdev: allow PSK offload for FT AKMs James Prestwood
2021-03-30 20:25   ` Denis Kenzior
2021-03-30 20:40     ` James Prestwood
2021-03-30 18:48 ` [PATCH v2 5/7] station: set handshake->offload if required James Prestwood
2021-03-30 18:48 ` [PATCH v2 6/7] doc: document new [General].4WayOffload James Prestwood
2021-03-30 20:49   ` Denis Kenzior
2021-03-30 18:48 ` [PATCH v2 7/7] wiphy: remove wiphy_supports_cmds_auth_assoc James Prestwood

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.