[PATCH 01/10] scan: parse password identifier/exclusive bits

[PATCH 01/10] scan: parse password identifier/exclusive bits

[James Prestwood]

These bits are used to communicate to the station that SAE password
identifiers are used or required.
---
 src/scan.c | 20 +++++++++++++++++---
 src/scan.h |  2 ++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/src/scan.c b/src/scan.c
index 5aa92a90..f48ffdef 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -1404,10 +1404,24 @@ static bool scan_parse_bss_information_elements(struct scan_bss *bss,
 			 * Currently only Proxy ARP bit (12) is checked, and if
 			 * not found, this is not a fatal error.
 			 */
-			if (iter.len < 2)
-				break;
+			if (iter.len >= 2)
+				bss->proxy_arp = test_bit(iter.data, 12);
+
+			/*
+			 * 802.11-2020 Table 9-153
+			 *
+			 * The spec merely mentions the "exclusive" bit and
+			 * doesn't enforce a requirement to check it anywhere.
+			 * But if set it would indicate the AP will only accept
+			 * auths when a password ID is used so store this in
+			 * order to fail early if no ID is set.
+			 */
+			if (iter.len >= 11) {
+				bss->sae_pw_id_used = test_bit(iter.data, 81);
+				bss->sae_pw_id_exclusive =
+							test_bit(iter.data, 82);
+			}
 
-			bss->proxy_arp = test_bit(iter.data, 12);
 		}
 	}
 
diff --git a/src/scan.h b/src/scan.h
index 0db7752d..65caf41c 100644
--- a/src/scan.h
+++ b/src/scan.h
@@ -88,6 +88,8 @@ struct scan_bss {
 	uint8_t cost_level : 3;
 	uint8_t cost_flags : 4;
 	bool dpp_configurator : 1;
+	bool sae_pw_id_used : 1;
+	bool sae_pw_id_exclusive : 1;
 };
 
 struct scan_parameters {
-- 
2.34.1

[PATCH 02/10] network: pass scan_bss into network_load_psk

[James Prestwood]

For adding SAE password identifiers the capability bits need to be
verified when loading the identifier from the profile. Pass the
BSS object in to network_load_psk rather than the 'need_passphrase'
boolean.
---
 src/network.c | 60 +++++++++++++++++++++++++--------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/src/network.c b/src/network.c
index f203834c..79f964b2 100644
--- a/src/network.c
+++ b/src/network.c
@@ -594,8 +594,34 @@ generate:
 	return -EIO;
 }
 
-static int network_load_psk(struct network *network, bool need_passphrase)
+static inline bool __bss_is_sae(const struct scan_bss *bss,
+						const struct ie_rsn_info *rsn)
 {
+	if (rsn->akm_suites & IE_RSN_AKM_SUITE_SAE_SHA256)
+		return true;
+
+	return false;
+}
+
+static bool bss_is_sae(const struct scan_bss *bss)
+{
+	struct ie_rsn_info rsn;
+
+	memset(&rsn, 0, sizeof(rsn));
+	scan_bss_get_rsn_info(bss, &rsn);
+
+	return __bss_is_sae(bss, &rsn);
+}
+
+static int network_load_psk(struct network *network, struct scan_bss *bss)
+{
+	/*
+	 * A legacy psk file may only contain the PreSharedKey entry. For SAE
+	 * networks the raw Passphrase is required. So in this case where
+	 * the psk is found but no Passphrase, we ask the agent.  The psk file
+	 * will then be re-written to contain the raw passphrase.
+	 */
+	bool is_sae = bss_is_sae(bss);
 	const char *ssid = network_get_ssid(network);
 	enum security security = network_get_security(network);
 	size_t psk_len;
@@ -616,7 +642,7 @@ static int network_load_psk(struct network *network, bool need_passphrase)
 	}
 
 	/* PSK can be generated from the passphrase but not the other way */
-	if (!psk || need_passphrase) {
+	if (!psk || is_sae) {
 		if (!passphrase)
 			return -ENOKEY;
 
@@ -778,25 +804,6 @@ bool network_get_force_default_owe_group(struct network *network)
 	return network->force_default_owe_group;
 }
 
-static inline bool __bss_is_sae(const struct scan_bss *bss,
-						const struct ie_rsn_info *rsn)
-{
-	if (rsn->akm_suites & IE_RSN_AKM_SUITE_SAE_SHA256)
-		return true;
-
-	return false;
-}
-
-static bool bss_is_sae(const struct scan_bss *bss)
-{
-	struct ie_rsn_info rsn;
-
-	memset(&rsn, 0, sizeof(rsn));
-	scan_bss_get_rsn_info(bss, &rsn);
-
-	return __bss_is_sae(bss, &rsn);
-}
-
 int network_can_connect_bss(struct network *network, const struct scan_bss *bss)
 {
 	struct station *station = network->station;
@@ -959,7 +966,7 @@ int network_autoconnect(struct network *network, struct scan_bss *bss)
 
 	switch (security) {
 	case SECURITY_PSK:
-		ret = network_load_psk(network, bss_is_sae(bss));
+		ret = network_load_psk(network, bss);
 		if (ret < 0)
 			goto close_settings;
 
@@ -1285,20 +1292,13 @@ static struct l_dbus_message *network_connect_psk(struct network *network,
 					struct l_dbus_message *message)
 {
 	struct station *station = network->station;
-	/*
-	 * A legacy psk file may only contain the PreSharedKey entry. For SAE
-	 * networks the raw Passphrase is required. So in this case where
-	 * the psk is found but no Passphrase, we ask the agent.  The psk file
-	 * will then be re-written to contain the raw passphrase.
-	 */
-	bool need_passphrase = bss_is_sae(bss);
 
 	if (!network_settings_load(network)) {
 		network->settings = l_settings_new();
 		network->ask_passphrase = true;
 	} else if (!network->ask_passphrase)
 		network->ask_passphrase =
-			network_load_psk(network, need_passphrase) < 0;
+			network_load_psk(network, bss) < 0;
 
 	l_debug("ask_passphrase: %s",
 		network->ask_passphrase ? "true" : "false");
-- 
2.34.1

[PATCH 03/10] handshake: add password identifier/setter

[James Prestwood]

---
 src/handshake.c | 12 ++++++++++++
 src/handshake.h |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/src/handshake.c b/src/handshake.c
index 1c5ed2c9..cf9c18d5 100644
--- a/src/handshake.c
+++ b/src/handshake.c
@@ -137,6 +137,12 @@ void handshake_state_free(struct handshake_state *s)
 		l_free(s->passphrase);
 	}
 
+	if (s->password_identifier) {
+		explicit_bzero(s->password_identifier,
+				strlen(s->password_identifier));
+		l_free(s->password_identifier);
+	}
+
 	if (s->ecc_sae_pts) {
 		unsigned int i;
 
@@ -364,6 +370,12 @@ void handshake_state_set_passphrase(struct handshake_state *s,
 	s->passphrase = l_strdup(passphrase);
 }
 
+void handshake_state_set_password_identifier(struct handshake_state *s,
+						const char *id)
+{
+	s->password_identifier = l_strdup(id);
+}
+
 void handshake_state_set_no_rekey(struct handshake_state *s, bool no_rekey)
 {
 	s->no_rekey = no_rekey;
diff --git a/src/handshake.h b/src/handshake.h
index 815eb44f..3b51cb34 100644
--- a/src/handshake.h
+++ b/src/handshake.h
@@ -143,6 +143,7 @@ struct handshake_state {
 	uint8_t ssid[32];
 	size_t ssid_len;
 	char *passphrase;
+	char *password_identifier;
 	uint8_t r0khid[48];
 	size_t r0khid_len;
 	uint8_t r1khid[6];
@@ -228,6 +229,8 @@ void handshake_state_set_event_func(struct handshake_state *s,
 					void *user_data);
 void handshake_state_set_passphrase(struct handshake_state *s,
 					const char *passphrase);
+void handshake_state_set_password_identifier(struct handshake_state *s,
+						const char *id);
 bool handshake_state_add_ecc_sae_pt(struct handshake_state *s,
 					const struct l_ecc_point *pt);
 void handshake_state_set_no_rekey(struct handshake_state *s, bool no_rekey);
-- 
2.34.1

[PATCH 04/10] network: add support for SAE password identifiers

[James Prestwood]

Adds a new network profile setting [Security].PasswordIdentifier.
When set (and the BSS enables SAE password identifiers) the network
and handshake object will read this and use it for the SAE
exchange.

Loading the PSK will fail if there is no password identifier set
and the BSS sets the "exclusive" bit. If a password identifier is
set and the BSS doesn't indicate support the setting will be ignored
(with a debug print).
---
 src/network.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/src/network.c b/src/network.c
index 79f964b2..d422b282 100644
--- a/src/network.c
+++ b/src/network.c
@@ -70,6 +70,7 @@ struct network {
 	struct network_info *info;
 	unsigned char *psk;
 	char *passphrase;
+	char *password_identifier;
 	struct l_ecc_point *sae_pt_19; /* SAE PT for Group 19 */
 	struct l_ecc_point *sae_pt_20; /* SAE PT for Group 20 */
 	unsigned int agent_request;
@@ -124,6 +125,13 @@ static void network_reset_passphrase(struct network *network)
 		network->passphrase = NULL;
 	}
 
+	if (network->password_identifier) {
+		explicit_bzero(network->password_identifier,
+				strlen(network->password_identifier));
+		l_free(network->password_identifier);
+		network->password_identifier = NULL;
+	}
+
 	if (network->sae_pt_19) {
 		l_ecc_point_free(network->sae_pt_19);
 		network->sae_pt_19 = NULL;
@@ -317,7 +325,8 @@ static struct l_ecc_point *network_generate_sae_pt(struct network *network,
 	l_debug("Generating PT for Group %u", group);
 
 	pt = crypto_derive_sae_pt_ecc(group, network->ssid,
-						network->passphrase, NULL);
+						network->passphrase,
+						network->password_identifier);
 	if (!pt)
 		l_warn("SAE PT generation for Group %u failed", group);
 
@@ -462,6 +471,10 @@ static int network_set_handshake_secrets_psk(struct network *network,
 
 		handshake_state_set_passphrase(hs, network->passphrase);
 
+		if (network->password_identifier)
+			handshake_state_set_password_identifier(hs,
+						network->password_identifier);
+
 		if (ie_rsnxe_capable(hs->authenticator_rsnxe,
 							IE_RSNX_SAE_H2E)) {
 			l_debug("Authenticator is SAE H2E capable");
@@ -631,6 +644,9 @@ static int network_load_psk(struct network *network, struct scan_bss *bss)
 	_auto_(l_free) char *passphrase =
 			l_settings_get_string(network->settings,
 						"Security", "Passphrase");
+	_auto_(l_free) char *password_id =
+			l_settings_get_string(network->settings, "Security",
+						"PasswordIdentifier");
 	_auto_(l_free) char *path =
 		storage_get_network_file_path(security, ssid);
 
@@ -641,6 +657,32 @@ static int network_load_psk(struct network *network, struct scan_bss *bss)
 		psk_len = 0;
 	}
 
+	/*
+	 * Sort out if the password identifier is required, should be used, "
+	 * or should be ignored.
+	 */
+	if (is_sae) {
+		if (bss->sae_pw_id_exclusive && !password_id) {
+			l_error("BSS requires SAE password identifiers, check "
+				"[Security].PasswordIdentifier");
+			return -ENOKEY;
+		}
+
+		/*
+		 * If the profile contains a password identifier but the network
+		 * does not support it IWD will still attempt to connect. The
+		 * caveat here is if the connection is successful the sync will
+		 * remove the password identifier entry. Though this might be
+		 * unexpected to the user, retaining this (invalid) setting
+		 * isn't worth special casing.
+		 */
+		if (!bss->sae_pw_id_used && password_id) {
+			l_debug("[Security].PasswordIdentifier set but BSS "
+				"does not not use password identifiers");
+			l_free(l_steal_ptr(password_id));
+		}
+	}
+
 	/* PSK can be generated from the passphrase but not the other way */
 	if (!psk || is_sae) {
 		if (!passphrase)
@@ -655,6 +697,7 @@ static int network_load_psk(struct network *network, struct scan_bss *bss)
 	network_reset_passphrase(network);
 	network_reset_psk(network);
 	network->passphrase = l_steal_ptr(passphrase);
+	network->password_identifier = l_steal_ptr(password_id);
 
 	if (network_settings_load_pt_ecc(network, path,
 						19, &network->sae_pt_19) > 0)
@@ -726,6 +769,11 @@ static void network_settings_save(struct network *network,
 		l_settings_set_string(settings, "Security", "Passphrase",
 					network->passphrase);
 
+	if (network->password_identifier)
+		l_settings_set_string(settings, "Security",
+					"PasswordIdentifier",
+					network->password_identifier);
+
 	if (network->sae_pt_19)
 		network_settings_save_sae_pt_ecc(settings, network->sae_pt_19);
 
-- 
2.34.1

[PATCH 05/10] sae: include password identifier IE in commit

[James Prestwood]

Include the IE if a password identifier is being used. This is only
supported by H2E as required by 802.11.
---
 src/sae.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/sae.c b/src/sae.c
index 336954b4..da00e4da 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -637,6 +637,14 @@ old_commit:
 		ie_tlv_builder_set_data(&builder, sm->token, sm->token_len);
 	}
 
+	if (sm->sae_type == CRYPTO_SAE_HASH_TO_ELEMENT &&
+					sm->handshake->password_identifier) {
+		ie_tlv_builder_next(&builder, IE_TYPE_PASSWORD_IDENTIFIER);
+		ie_tlv_builder_set_data(&builder,
+				sm->handshake->password_identifier,
+				strlen(sm->handshake->password_identifier));
+	}
+
 	ie_tlv_builder_finalize(&builder, &len);
 
 	return ptr - commit + len;
-- 
2.34.1

[PATCH 06/10] doc: document [Security].PasswordIdentifier

[James Prestwood]

---
 src/iwd.network.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/iwd.network.rst b/src/iwd.network.rst
index 719853fa..d467b7b1 100644
--- a/src/iwd.network.rst
+++ b/src/iwd.network.rst
@@ -199,6 +199,16 @@ connect to that network.
        required if the *PreSharedKey* is not provided.  If not provided in
        settings, the agent will be asked for the passphrase at connection
        time.
+   * - PasswordIdentifier
+     - string
+
+       An identifer string to be used with the passphrase. This is used for
+       WPA3-Personal (SAE) networks if the security has enabled password
+       identifiers for clients.
+
+       Note: if the network does not support password identifiers but one is
+       set in the profile it will be ignored and removed upon a successful
+       connection to the network.
    * - PreSharedKey
      - 64 character hex string
 
-- 
2.34.1

[PATCH 07/10] auto-t: add H2E password identifier test

[James Prestwood]

In order to support identifiers the test profiles needed to be
reworked due to hostapd allowing multiple password entires. You
cannot just call set_value() with a new entry as the old ones
still exist. Instead use a unique password for the identifier and
non-identifier use cases.

After adding this test the failure_test started failing due to
hostapd not starting up. This was due to the group being unsupported
but oddly only when hostapd was reloaded (running the test
individually worked). To fix this the group number was changed to 21
which hostapd does support but IWD does not.
---
 autotests/testSAE/autoconnect_test.py         | 20 +++++++++++++------
 autotests/testSAE/failure_test.py             |  2 +-
 autotests/testSAE/ssidSAE.conf                |  3 ++-
 .../{ssidSAE.psk => ssidSAE.psk.default}      |  0
 autotests/testSAE/ssidSAE.psk.identifier      |  3 +++
 5 files changed, 20 insertions(+), 8 deletions(-)
 rename autotests/testSAE/{ssidSAE.psk => ssidSAE.psk.default} (100%)
 create mode 100644 autotests/testSAE/ssidSAE.psk.identifier

diff --git a/autotests/testSAE/autoconnect_test.py b/autotests/testSAE/autoconnect_test.py
index cba59274..4ce3b845 100644
--- a/autotests/testSAE/autoconnect_test.py
+++ b/autotests/testSAE/autoconnect_test.py
@@ -35,12 +35,23 @@ class Test(unittest.TestCase):
         wd.wait_for_object_condition(ordered_network.network_object, condition)
 
     def test_SAE(self):
+        IWD.copy_to_storage("ssidSAE.psk.default", name="ssidSAE.psk")
         self.hostapd.wait_for_event("AP-ENABLED")
 
         wd = IWD(True)
         self.validate_connection(wd)
 
     def test_SAE_H2E(self):
+        IWD.copy_to_storage("ssidSAE.psk.default", name="ssidSAE.psk")
+        self.hostapd.set_value('sae_pwe', '1')
+        self.hostapd.set_value('sae_groups', '20')
+        self.hostapd.reload()
+        self.hostapd.wait_for_event("AP-ENABLED")
+        wd = IWD(True)
+        self.validate_connection(wd)
+
+    def test_SAE_H2E_password_identifier(self):
+        IWD.copy_to_storage("ssidSAE.psk.identifier", name="ssidSAE.psk")
         self.hostapd.set_value('sae_pwe', '1')
         self.hostapd.set_value('sae_groups', '20')
         self.hostapd.reload()
@@ -51,15 +62,12 @@ class Test(unittest.TestCase):
     def setUp(self):
         self.hostapd.default()
 
+    def tearDown(self):
+        IWD.clear_storage()
+
     @classmethod
     def setUpClass(cls):
         cls.hostapd = HostapdCLI(config='ssidSAE.conf')
-        IWD.copy_to_storage('ssidSAE.psk')
-        pass
-
-    @classmethod
-    def tearDownClass(cls):
-        IWD.clear_storage()
 
 if __name__ == '__main__':
     unittest.main(exit=True)
diff --git a/autotests/testSAE/failure_test.py b/autotests/testSAE/failure_test.py
index 2aac3a07..aa4d14b9 100644
--- a/autotests/testSAE/failure_test.py
+++ b/autotests/testSAE/failure_test.py
@@ -37,7 +37,7 @@ class Test(unittest.TestCase):
         self.validate_connection(wd, 'InvalidSecret')
 
     def test_no_supported_groups(self):
-        self.hostapd.set_value('sae_groups', '1')
+        self.hostapd.set_value('sae_groups', '21')
         self.hostapd.reload()
 
         wd = IWD(True)
diff --git a/autotests/testSAE/ssidSAE.conf b/autotests/testSAE/ssidSAE.conf
index 41f46cad..f5ce537d 100644
--- a/autotests/testSAE/ssidSAE.conf
+++ b/autotests/testSAE/ssidSAE.conf
@@ -5,7 +5,8 @@ ssid=ssidSAE
 wpa=2
 wpa_key_mgmt=SAE
 wpa_pairwise=CCMP
-sae_password=secret123|mac=ff:ff:ff:ff:ff:ff
+sae_password=secret123
+sae_password=withidentifier|id=myidentifier
 sae_groups=19
 ieee80211w=2
 sae_pwe=0
diff --git a/autotests/testSAE/ssidSAE.psk b/autotests/testSAE/ssidSAE.psk.default
similarity index 100%
rename from autotests/testSAE/ssidSAE.psk
rename to autotests/testSAE/ssidSAE.psk.default
diff --git a/autotests/testSAE/ssidSAE.psk.identifier b/autotests/testSAE/ssidSAE.psk.identifier
new file mode 100644
index 00000000..3664063a
--- /dev/null
+++ b/autotests/testSAE/ssidSAE.psk.identifier
@@ -0,0 +1,3 @@
+[Security]
+Passphrase=withidentifier
+PasswordIdentifier=myidentifier
-- 
2.34.1

[PATCH 08/10] mpdu: add unknown password identifier status

[James Prestwood]

---
 src/mpdu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mpdu.h b/src/mpdu.h
index f6f19744..cf3114db 100644
--- a/src/mpdu.h
+++ b/src/mpdu.h
@@ -231,6 +231,7 @@ enum mmpdu_status_code {
 	MMPDU_STATUS_CODE_ENABLEMENT_DENIED = 105,
 	MMPDU_STATUS_CODE_RESTRICT_AUTH_GDB = 106,
 	MMPDU_STATUS_CODE_AUTHORIZATION_DEENABLED = 107,
+	MMPDU_STATUS_CODE_UNKNOWN_PASSWORD_IDENTIFIER = 123,
 	MMPDU_STATUS_CODE_SAE_HASH_TO_ELEMENT = 126,
 	MMPDU_STATUS_CODE_SAE_PK = 127,
 };
-- 
2.34.1

[PATCH 09/10] sae: add debugging for incorrect password identifier

[James Prestwood]

If the AP rejects the auth because of an unknown identifier catch
this and log the error.
---
 src/sae.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/sae.c b/src/sae.c
index da00e4da..9a10292e 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -1104,11 +1104,19 @@ static int sae_verify_committed(struct sae_sm *sm, uint16_t transaction,
 	 * If the Status is some other nonzero value, the frame shall be
 	 * silently discarded and the t0 (retransmission) timer shall be set.
 	 */
-	if (status != 0 && status != MMPDU_STATUS_CODE_SAE_HASH_TO_ELEMENT)
+	switch (status) {
+	case 0:
+	case MMPDU_STATUS_CODE_SAE_HASH_TO_ELEMENT:
+		if (status != sae_status_code(sm))
+			return -EBADMSG;
+		break;
+	case MMPDU_STATUS_CODE_UNKNOWN_PASSWORD_IDENTIFIER:
+		sae_debug("Incorrect password identifier, check "
+				"[Security].PasswordIdentifier");
+		/* fall through */
+	default:
 		return -ENOMSG;
-
-	if (status != sae_status_code(sm))
-		return -EBADMSG;
+	}
 
 	if (len < 2)
 		return -EBADMSG;
-- 
2.34.1

[PATCH 10/10] auto-t: throw exception if executable is missing

[James Prestwood]

Certain tests may require external processes to work
(e.g. testNetconfig) and if missing the test will just hang until
the maximum test timeout. Check in start_process if the exe
actually exists and if not throw an exception.
---
 tools/utils.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/utils.py b/tools/utils.py
index a07c3183..8219542e 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -34,6 +34,9 @@ class Process(subprocess.Popen):
 
 		logfile = args[0]
 
+		if not shutil.which(args[0]):
+			raise Exception("%s is not found on system" % args[0])
+
 		if Process.is_verbose(args[0], log=False):
 			self.verbose = True
 
-- 
2.34.1

Re: [PATCH 01/10] scan: parse password identifier/exclusive bits

[Denis Kenzior]

Hi James,

On 12/5/23 09:46, James Prestwood wrote:
> These bits are used to communicate to the station that SAE password
> identifiers are used or required.
> ---
>   src/scan.c | 20 +++++++++++++++++---
>   src/scan.h |  2 ++
>   2 files changed, 19 insertions(+), 3 deletions(-)
> 

I applied patches 1, 2, 3, 5, 8 and 9.

Regards,
-Denis

Re: [PATCH 04/10] network: add support for SAE password identifiers

[Denis Kenzior]

Hi James,

On 12/5/23 09:46, James Prestwood wrote:
> Adds a new network profile setting [Security].PasswordIdentifier.
> When set (and the BSS enables SAE password identifiers) the network
> and handshake object will read this and use it for the SAE
> exchange.
> 
> Loading the PSK will fail if there is no password identifier set
> and the BSS sets the "exclusive" bit. If a password identifier is

I'm not so sure about this.  The trouble is that this logic is sufficient for 
the initial connection, but isn't sufficient when you consider re-association.

> set and the BSS doesn't indicate support the setting will be ignored
> (with a debug print).
> ---
>   src/network.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 49 insertions(+), 1 deletion(-)
> 

<snip>

> @@ -641,6 +657,32 @@ static int network_load_psk(struct network *network, struct scan_bss *bss)
>   		psk_len = 0;
>   	}
>   
> +	/*
> +	 * Sort out if the password identifier is required, should be used, "
> +	 * or should be ignored.
> +	 */
> +	if (is_sae) {
> +		if (bss->sae_pw_id_exclusive && !password_id) {

This likely needs to be taken into consideration much later, when building the 
actual handshake state.

> +			l_error("BSS requires SAE password identifiers, check "
> +				"[Security].PasswordIdentifier");
> +			return -ENOKEY;
> +		}
> +
> +		/*
> +		 * If the profile contains a password identifier but the network
> +		 * does not support it IWD will still attempt to connect. The

Password identifier is only used by SAE H2E.  One can easily imagine a weird 
network of mixed APs with some being SAE H2E, some not.  This is why I didn't 
bother implementing this, it is a half-baked feature.

> +		 * caveat here is if the connection is successful the sync will
> +		 * remove the password identifier entry. Though this might be
> +		 * unexpected to the user, retaining this (invalid) setting
> +		 * isn't worth special casing.

And this doesn't sound nice at all...  I think the setting should be preserved.

> +		 */
> +		if (!bss->sae_pw_id_used && password_id) {
> +			l_debug("[Security].PasswordIdentifier set but BSS "
> +				"does not not use password identifiers");
> +			l_free(l_steal_ptr(password_id));
> +		}
> +	}
> +
>   	/* PSK can be generated from the passphrase but not the other way */
>   	if (!psk || is_sae) {
>   		if (!passphrase)

Regards,
-Denis

Re: [PATCH 04/10] network: add support for SAE password identifiers

[James Prestwood]

Hi Denis,

On 12/6/23 09:08, Denis Kenzior wrote:
> Hi James,
>
> On 12/5/23 09:46, James Prestwood wrote:
>> Adds a new network profile setting [Security].PasswordIdentifier.
>> When set (and the BSS enables SAE password identifiers) the network
>> and handshake object will read this and use it for the SAE
>> exchange.
>>
>> Loading the PSK will fail if there is no password identifier set
>> and the BSS sets the "exclusive" bit. If a password identifier is
>
> I'm not so sure about this.  The trouble is that this logic is 
> sufficient for the initial connection, but isn't sufficient when you 
> consider re-association.
Your right, roaming would be entirely broken between BSS's that mismatch 
using password identifiers. Maybe even hunt-and-peck and H2E? not 
entirely sure. We would need to re-derive the point for each roam, like 
in network_set_handshake_secrets_psk().
>
>> set and the BSS doesn't indicate support the setting will be ignored
>> (with a debug print).
>> ---
>>   src/network.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 49 insertions(+), 1 deletion(-)
>>
>
> <snip>
>
>> @@ -641,6 +657,32 @@ static int network_load_psk(struct network 
>> *network, struct scan_bss *bss)
>>           psk_len = 0;
>>       }
>>   +    /*
>> +     * Sort out if the password identifier is required, should be 
>> used, "
>> +     * or should be ignored.
>> +     */
>> +    if (is_sae) {
>> +        if (bss->sae_pw_id_exclusive && !password_id) {
>
> This likely needs to be taken into consideration much later, when 
> building the actual handshake state.

Yeah, we'd need to move this into network_set_handshake_secrets_psk and 
rederive the points. And actually if we do this storing the points in 
the network profile doesn't make a whole lot of sense anymore since its 
being rederived every time.

Alternatively we just keep it how I have it and tell they user they're 
network isn't configured properly :)

>
>> +            l_error("BSS requires SAE password identifiers, check "
>> +                "[Security].PasswordIdentifier");
>> +            return -ENOKEY;
>> +        }
>> +
>> +        /*
>> +         * If the profile contains a password identifier but the 
>> network
>> +         * does not support it IWD will still attempt to connect. The
>
> Password identifier is only used by SAE H2E.  One can easily imagine a 
> weird network of mixed APs with some being SAE H2E, some not.  This is 
> why I didn't bother implementing this, it is a half-baked feature.

Ugh, ok. I guess we have to keep the identifier around then.

>
>> +         * caveat here is if the connection is successful the sync will
>> +         * remove the password identifier entry. Though this might be
>> +         * unexpected to the user, retaining this (invalid) setting
>> +         * isn't worth special casing.
>
> And this doesn't sound nice at all...  I think the setting should be 
> preserved.
>
>> +         */
>> +        if (!bss->sae_pw_id_used && password_id) {
>> +            l_debug("[Security].PasswordIdentifier set but BSS "
>> +                "does not not use password identifiers");
>> +            l_free(l_steal_ptr(password_id));
>> +        }
>> +    }
>> +
>>       /* PSK can be generated from the passphrase but not the other 
>> way */
>>       if (!psk || is_sae) {
>>           if (!passphrase)
>
> Regards,
> -Denis
>

Re: [PATCH 04/10] network: add support for SAE password identifiers

[Denis Kenzior]

Hi James,

>>> Loading the PSK will fail if there is no password identifier set
>>> and the BSS sets the "exclusive" bit. If a password identifier is
>>
>> I'm not so sure about this.  The trouble is that this logic is sufficient for 
>> the initial connection, but isn't sufficient when you consider re-association.
> Your right, roaming would be entirely broken between BSS's that mismatch using 
> password identifiers. Maybe even hunt-and-peck and H2E? not entirely sure. We 

Well, ReAssociate would just use SAE passphrase directly, so it would work in 
theory...  But it is a bit of a strange case.

> would need to re-derive the point for each roam, like in 
> network_set_handshake_secrets_psk().

??  You mean SAE-H2E with password identifier for BSSes that report 
exclusive/in-use bit and SAE-H2E for BSSes without?  Or something else?

>>
>> This likely needs to be taken into consideration much later, when building the 
>> actual handshake state.
> 
> Yeah, we'd need to move this into network_set_handshake_secrets_psk and rederive 
> the points. And actually if we do this storing the points in the network profile 
> doesn't make a whole lot of sense anymore since its being rederived every time.

I would hate for this to be the outcome.  Re-deriving the PT is pretty expensive.

> 
> Alternatively we just keep it how I have it and tell they user they're network 
> isn't configured properly :)

I think it could be argued that if PasswordIdentifier is set, then any BSSes 
that are not H2E/do not set the in-use bit are not connectable.

Regards,
-Denis

Re: [PATCH 04/10] network: add support for SAE password identifiers

[James Prestwood]

On 12/6/23 11:44, Denis Kenzior wrote:
> Hi James,
>
>>>> Loading the PSK will fail if there is no password identifier set
>>>> and the BSS sets the "exclusive" bit. If a password identifier is
>>>
>>> I'm not so sure about this.  The trouble is that this logic is 
>>> sufficient for the initial connection, but isn't sufficient when you 
>>> consider re-association.
>> Your right, roaming would be entirely broken between BSS's that 
>> mismatch using password identifiers. Maybe even hunt-and-peck and 
>> H2E? not entirely sure. We 
>
> Well, ReAssociate would just use SAE passphrase directly, so it would 
> work in theory...  But it is a bit of a strange case.
>
>> would need to re-derive the point for each roam, like in 
>> network_set_handshake_secrets_psk().
>
> ??  You mean SAE-H2E with password identifier for BSSes that report 
> exclusive/in-use bit and SAE-H2E for BSSes without?  Or something else?

Yeah, I'm talking about multiple H2E BSS's that set or don't set the 
exclusive/in-use bits. Maybe it would be ok actually. I got concerned 
when I saw the points being set into the handshake in 
network_set_handshake_secrets_psk() (which happens on roaming) but it 
looks like this is only used for initial SAE association. Maybe 
roaming/FT would be ok? But this is all moot if we just bail early if 
the password identifier setting does not match the BSS's capabilities.

>>>
>>> This likely needs to be taken into consideration much later, when 
>>> building the actual handshake state.
>>
>> Yeah, we'd need to move this into network_set_handshake_secrets_psk 
>> and rederive the points. And actually if we do this storing the 
>> points in the network profile doesn't make a whole lot of sense 
>> anymore since its being rederived every time.
>
> I would hate for this to be the outcome.  Re-deriving the PT is pretty 
> expensive.
>
>>
>> Alternatively we just keep it how I have it and tell they user 
>> they're network isn't configured properly :)
>
> I think it could be argued that if PasswordIdentifier is set, then any 
> BSSes that are not H2E/do not set the in-use bit are not connectable.
This works for me. Much easier and will enforce a properly configured 
network.
>
> Regards,
> -Denis