[PATCH v3 01/13] auto-t: add explicit stop() to IWD class

[PATCH v3 01/13] auto-t: add explicit stop() to IWD class

[James Prestwood]

If tests end in an unknown state it is sometimes required that IWD
be stopped manually in order for future tests to run. Add a stop()
method so test tearDown() methods can explicitly stop IWD.
---
 autotests/util/iwd.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/autotests/util/iwd.py b/autotests/util/iwd.py
index 3f200beb..b1a57cab 100755
--- a/autotests/util/iwd.py
+++ b/autotests/util/iwd.py
@@ -1363,6 +1363,10 @@ class IWD(AsyncOpAbstract):
 
         self.psk_agents = []
 
+    def stop(self):
+        if self.namespace.is_process_running('iwd'):
+            self._iwd_proc.kill()
+
     def __del__(self):
         for agent in self.psk_agents:
             self.unregister_psk_agent(agent)
-- 
2.34.1

[PATCH v3 02/13] auto-t: add association timeout test

[James Prestwood]

This tests ensures IWD disconnects after receiving an association
timeout event. This exposes a current bug where IWD does not
transition to disconnected after an association timeout when
FT-roaming.
---
 autotests/testPSK-roam/failed_roam_test.py | 56 +++++++++++++++-------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/autotests/testPSK-roam/failed_roam_test.py b/autotests/testPSK-roam/failed_roam_test.py
index 90f07f6b..d42002d4 100644
--- a/autotests/testPSK-roam/failed_roam_test.py
+++ b/autotests/testPSK-roam/failed_roam_test.py
@@ -46,11 +46,9 @@ class Test(unittest.TestCase):
         self.rule2.enabled = True
         self.rule3.enabled = True
 
-        wd = IWD(True)
+        device = self.wd.list_devices(1)[0]
 
-        device = wd.list_devices(1)[0]
-
-        self.connect(wd, device, self.bss_hostapd[0])
+        self.connect(self.wd, device, self.bss_hostapd[0])
 
         self.rule0.enabled = True
 
@@ -63,10 +61,25 @@ class Test(unittest.TestCase):
 
         # IWD should then try BSS 2, and succeed
         device.wait_for_event('ft-roam', timeout=60)
-        self.verify_roam(wd, device, self.bss_hostapd[0], self.bss_hostapd[2])
+        self.verify_roam(self.wd, device, self.bss_hostapd[0], self.bss_hostapd[2])
 
         self.bss_hostapd[2].deauthenticate(device.address)
 
+    # Tests that an associate even should cause a disconnect
+    def test_ft_over_air_assoc_timeout(self):
+        self.rule2.enabled = True
+        self.rule3.enabled = True
+        self.assoc_rule.enabled = True
+
+        device = self.wd.list_devices(1)[0]
+
+        self.connect(self.wd, device, self.bss_hostapd[0])
+
+        device.wait_for_event('ft-roam', timeout=60)
+
+        condition = 'obj.state == DeviceState.disconnected'
+        self.wd.wait_for_object_condition(device, condition)
+
     # FT-over-Air failure with Invalid PMKID, should reassociate
     def test_ft_over_air_fallback(self):
         self.rule_bss0.signal = -8000
@@ -81,18 +94,16 @@ class Test(unittest.TestCase):
         self.bss_hostapd[2].set_value('ft_psk_generate_local', '0')
         self.bss_hostapd[2].reload()
 
-        wd = IWD(True)
+        device = self.wd.list_devices(1)[0]
 
-        device = wd.list_devices(1)[0]
-
-        self.connect(wd, device, self.bss_hostapd[0])
+        self.connect(self.wd, device, self.bss_hostapd[0])
 
         # IWD should connect, then attempt a roam to BSS 1, which should
         # fail and cause a fallback to reassociation
         device.wait_for_event('ft-fallback-to-reassoc', timeout=60)
         device.wait_for_event('reassoc-roam', timeout=60)
 
-        self.verify_roam(wd, device, self.bss_hostapd[0], self.bss_hostapd[2])
+        self.verify_roam(self.wd, device, self.bss_hostapd[0], self.bss_hostapd[2])
 
         # Trigger another roam
         self.rule_bss2.signal = -8000
@@ -100,11 +111,11 @@ class Test(unittest.TestCase):
         device.wait_for_event('ft-roam', timeout=60)
 
         # Ensure an FT roam back to a properly configured AP works.
-        self.verify_roam(wd, device, self.bss_hostapd[2], self.bss_hostapd[1])
+        self.verify_roam(self.wd, device, self.bss_hostapd[2], self.bss_hostapd[1])
 
         self.bss_hostapd[1].deauthenticate(device.address)
         condition = 'obj.state == DeviceState.disconnected'
-        wd.wait_for_object_condition(device, condition)
+        self.wd.wait_for_object_condition(device, condition)
 
     # FT-over-Air failure with Invalid PMKID. The ranking is such that other
     # FT candidates are available so it should FT elsewhere rather than
@@ -122,11 +133,9 @@ class Test(unittest.TestCase):
         self.bss_hostapd[2].set_value('ft_psk_generate_local', '0')
         self.bss_hostapd[2].reload()
 
-        wd = IWD(True)
+        device = self.wd.list_devices(1)[0]
 
-        device = wd.list_devices(1)[0]
-
-        self.connect(wd, device, self.bss_hostapd[0])
+        self.connect(self.wd, device, self.bss_hostapd[0])
 
         # IWD should connect, then attempt a roam to BSS 1, which should
         # fail and cause the rank to be re-computed. This should then put
@@ -134,12 +143,14 @@ class Test(unittest.TestCase):
         device.wait_for_event('ft-fallback-to-reassoc', timeout=60)
         device.wait_for_event('ft-roam', timeout=60)
 
-        self.verify_roam(wd, device, self.bss_hostapd[0], self.bss_hostapd[1])
+        self.verify_roam(self.wd, device, self.bss_hostapd[0], self.bss_hostapd[1])
 
         self.bss_hostapd[1].deauthenticate(device.address)
         condition = 'obj.state == DeviceState.disconnected'
-        wd.wait_for_object_condition(device, condition)
+        self.wd.wait_for_object_condition(device, condition)
 
+    def setUp(self):
+        self.wd = IWD(True)
 
     def tearDown(self):
         os.system('ip link set "' + self.bss_hostapd[0].ifname + '" down')
@@ -154,10 +165,14 @@ class Test(unittest.TestCase):
         self.rule_bss0.enabled = False
         self.rule_bss1.enabled = False
         self.rule_bss2.enabled = False
+        self.assoc_rule.enabled = False
 
         for hapd in self.bss_hostapd:
             hapd.default()
 
+        self.wd.stop()
+        self.wd = None
+
     @classmethod
     def setUpClass(cls):
         hwsim = Hwsim()
@@ -178,6 +193,11 @@ class Test(unittest.TestCase):
         cls.rule0.prefix = 'b0'
         cls.rule0.drop = True
 
+        # Drop Associate frames
+        cls.assoc_rule = hwsim.rules.create()
+        cls.assoc_rule.prefix = '20'
+        cls.assoc_rule.drop = True
+
         # Drop Action frames
         cls.rule1 = hwsim.rules.create()
         cls.rule1.bidirectional = True
-- 
2.34.1

[PATCH v3 03/13] auto-t: only call set_value for changed values in default()

[James Prestwood]

The default() method was added for convenience but was extending the
test times significantly when the hostapd config was lengthy. This
was because it called set_value for every value regardless if it
had changed. Instead store the current configuration and in default()
only reset values that differ.
---
 autotests/util/hostapd.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/autotests/util/hostapd.py b/autotests/util/hostapd.py
index 1b1bc399..cee08092 100644
--- a/autotests/util/hostapd.py
+++ b/autotests/util/hostapd.py
@@ -75,6 +75,8 @@ class HostapdCLI(object):
         if self._default_config.get('vendor_elements', None) == None:
             self._default_config['vendor_elements'] = ''
 
+        self._current_config = self._default_config.copy()
+
         if not self.interface:
             raise Exception('config %s not found' % config)
 
@@ -170,6 +172,8 @@ class HostapdCLI(object):
         if self._default_config.get(key, None) == None:
             raise Exception("Untracked setting '%s'! Please set default in hostapd config" % key)
 
+        self._current_config[key] = value
+
         cmd = self.cmdline + ['set', key, value]
         ctx.start_process(cmd).wait()
 
@@ -200,7 +204,10 @@ class HostapdCLI(object):
 
     def default(self):
         for k, v in self._default_config.items():
-            self.set_value(k, v)
+            # Only bother setting the value if it differs
+            if self._current_config[k] != v:
+                self.set_value(k, v)
+                self._current_config[k] = v
 
         self.reload()
 
-- 
2.34.1

[PATCH v3 04/13] handshake: add setters for authenticator/supplicant_fte

[James Prestwood]

In general only the authenticator FTE is used/validated but with
some FT refactoring coming there needs to be a way to build the
supplicants FTE into the handshake object. Because of this there
needs to be separate FTE buffers for both the authenticator and
supplicant.
---
 src/handshake.c | 14 ++++++++++++++
 src/handshake.h |  7 +++++++
 2 files changed, 21 insertions(+)

diff --git a/src/handshake.c b/src/handshake.c
index 1c5ed2c9..f35da9b1 100644
--- a/src/handshake.c
+++ b/src/handshake.c
@@ -123,6 +123,8 @@ void handshake_state_free(struct handshake_state *s)
 	l_free(s->supplicant_rsnxe);
 	l_free(s->mde);
 	l_free(s->fte);
+	l_free(s->authenticator_fte);
+	l_free(s->supplicant_fte);
 	l_free(s->fils_ip_req_ie);
 	l_free(s->fils_ip_resp_ie);
 	l_free(s->vendor_ies);
@@ -313,6 +315,18 @@ void handshake_state_set_fte(struct handshake_state *s, const uint8_t *fte)
 	replace_ie(&s->fte, fte);
 }
 
+void handshake_state_set_authenticator_fte(struct handshake_state *s,
+						const uint8_t *fte)
+{
+	replace_ie(&s->authenticator_fte, fte);
+}
+
+void handshake_state_set_supplicant_fte(struct handshake_state *s,
+						const uint8_t *fte)
+{
+	replace_ie(&s->supplicant_fte, fte);
+}
+
 void handshake_state_set_vendor_ies(struct handshake_state *s,
 					const struct iovec *iov,
 					size_t n_iovs)
diff --git a/src/handshake.h b/src/handshake.h
index 815eb44f..e6344048 100644
--- a/src/handshake.h
+++ b/src/handshake.h
@@ -104,6 +104,8 @@ struct handshake_state {
 	uint8_t *supplicant_rsnxe;
 	uint8_t *mde;
 	uint8_t *fte;
+	uint8_t *authenticator_fte;
+	uint8_t *supplicant_fte;
 	uint8_t *vendor_ies;
 	size_t vendor_ies_len;
 	enum ie_rsn_cipher_suite pairwise_cipher;
@@ -215,6 +217,11 @@ void handshake_state_set_ssid(struct handshake_state *s,
 void handshake_state_set_mde(struct handshake_state *s,
 					const uint8_t *mde);
 void handshake_state_set_fte(struct handshake_state *s, const uint8_t *fte);
+void handshake_state_set_authenticator_fte(struct handshake_state *s,
+						const uint8_t *fte);
+void handshake_state_set_supplicant_fte(struct handshake_state *s,
+						const uint8_t *fte);
+
 void handshake_state_set_vendor_ies(struct handshake_state *s,
 					const struct iovec *iov,
 					size_t n_iovs);
-- 
2.34.1

[PATCH v3 05/13] handshake: use authenticator_fte instead of 'fte'

[James Prestwood]

---
 src/eapol.c     | 5 +++--
 src/fils.c      | 2 +-
 src/ft.c        | 2 +-
 src/handshake.c | 2 +-
 src/netdev.c    | 2 +-
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/eapol.c b/src/eapol.c
index 3d7b3d38..bad4bbb7 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -1299,7 +1299,8 @@ static void eapol_handle_ptk_1_of_4(struct eapol_sm *sm,
 		ies_len = ies[1] + 2;
 
 		ies_len += append_ie(ies + ies_len, sm->handshake->mde);
-		ies_len += append_ie(ies + ies_len, sm->handshake->fte);
+		ies_len += append_ie(ies + ies_len,
+					sm->handshake->authenticator_fte);
 	} else {
 		ies_len = append_ie(ies, own_ie);
 	}
@@ -1795,7 +1796,7 @@ static void eapol_handle_ptk_3_of_4(struct eapol_sm *sm,
 		if (eapol_ie_matches(decrypted_key_data,
 					decrypted_key_data_size,
 					IE_TYPE_FAST_BSS_TRANSITION,
-					hs->fte) < 0)
+					hs->authenticator_fte) < 0)
 			goto error_ie_different;
 	}
 
diff --git a/src/fils.c b/src/fils.c
index c3337352..f876e7e5 100644
--- a/src/fils.c
+++ b/src/fils.c
@@ -445,7 +445,7 @@ static int fils_rx_authenticate(struct auth_proto *driver, const uint8_t *frame,
 					kck_len, &ft_info) < 0)
 			goto invalid_ies;
 
-		handshake_state_set_fte(fils->hs, fte);
+		handshake_state_set_authenticator_fte(fils->hs, fte);
 		handshake_state_set_kh_ids(fils->hs, ft_info.r0khid,
 							ft_info.r0khid_len,
 							ft_info.r1khid);
diff --git a/src/ft.c b/src/ft.c
index 2cc611b8..43399828 100644
--- a/src/ft.c
+++ b/src/ft.c
@@ -922,7 +922,7 @@ static void ft_prepare_handshake(struct ft_info *info,
 
 	memcpy(hs->snonce, info->snonce, sizeof(hs->snonce));
 
-	handshake_state_set_fte(hs, info->fte);
+	handshake_state_set_authenticator_fte(hs, info->fte);
 
 	handshake_state_set_anonce(hs, info->ft_info.anonce);
 
diff --git a/src/handshake.c b/src/handshake.c
index f35da9b1..ad8533f4 100644
--- a/src/handshake.c
+++ b/src/handshake.c
@@ -528,7 +528,7 @@ bool handshake_state_derive_ptk(struct handshake_state *s)
 				IE_RSN_AKM_SUITE_FT_OVER_SAE_SHA256 |
 				IE_RSN_AKM_SUITE_FT_OVER_FILS_SHA256 |
 				IE_RSN_AKM_SUITE_FT_OVER_FILS_SHA384)) &&
-			(!s->mde || !s->fte))
+			(!s->mde || !s->authenticator_fte))
 		return false;
 
 	s->ptk_complete = false;
diff --git a/src/netdev.c b/src/netdev.c
index f2e887b4..b9dc930e 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -2777,7 +2777,7 @@ process_resp_ies:
 			 */
 			if (ie_parse_fast_bss_transition_from_data(fte,
 					fte[1] + 2, kck_len, &ft_info) >= 0) {
-				handshake_state_set_fte(hs, fte);
+				handshake_state_set_authenticator_fte(hs, fte);
 				handshake_state_set_kh_ids(hs,
 							ft_info.r0khid,
 							ft_info.r0khid_len,
-- 
2.34.1

[PATCH v3 06/13] unit: use authenticator_fte

[James Prestwood]

---
 unit/test-eapol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/unit/test-eapol.c b/unit/test-eapol.c
index 563541f5..5317f9ad 100644
--- a/unit/test-eapol.c
+++ b/unit/test-eapol.c
@@ -3493,7 +3493,7 @@ static void eapol_ft_handshake_test(const void *data)
 	handshake_state_set_supplicant_ie(hs, own_rsne);
 	handshake_state_set_authenticator_ie(hs, ap_rsne);
 	handshake_state_set_mde(hs, mde);
-	handshake_state_set_fte(hs, fte);
+	handshake_state_set_authenticator_fte(hs, fte);
 	handshake_state_set_kh_ids(hs, r0khid, strlen((void *) r0khid), r1khid);
 	eapol_start(sm);
 
-- 
2.34.1

[PATCH v3 07/13] handshake: remove handshake_state_set_fte

[James Prestwood]

Replaced by set_authenticator_fte
---
 src/handshake.c | 6 ------
 src/handshake.h | 2 --
 2 files changed, 8 deletions(-)

diff --git a/src/handshake.c b/src/handshake.c
index ad8533f4..8fab4498 100644
--- a/src/handshake.c
+++ b/src/handshake.c
@@ -122,7 +122,6 @@ void handshake_state_free(struct handshake_state *s)
 	l_free(s->authenticator_rsnxe);
 	l_free(s->supplicant_rsnxe);
 	l_free(s->mde);
-	l_free(s->fte);
 	l_free(s->authenticator_fte);
 	l_free(s->supplicant_fte);
 	l_free(s->fils_ip_req_ie);
@@ -310,11 +309,6 @@ void handshake_state_set_mde(struct handshake_state *s, const uint8_t *mde)
 	replace_ie(&s->mde, mde);
 }
 
-void handshake_state_set_fte(struct handshake_state *s, const uint8_t *fte)
-{
-	replace_ie(&s->fte, fte);
-}
-
 void handshake_state_set_authenticator_fte(struct handshake_state *s,
 						const uint8_t *fte)
 {
diff --git a/src/handshake.h b/src/handshake.h
index e6344048..562e44ce 100644
--- a/src/handshake.h
+++ b/src/handshake.h
@@ -103,7 +103,6 @@ struct handshake_state {
 	uint8_t *authenticator_rsnxe;
 	uint8_t *supplicant_rsnxe;
 	uint8_t *mde;
-	uint8_t *fte;
 	uint8_t *authenticator_fte;
 	uint8_t *supplicant_fte;
 	uint8_t *vendor_ies;
@@ -216,7 +215,6 @@ void handshake_state_set_ssid(struct handshake_state *s,
 					const uint8_t *ssid, size_t ssid_len);
 void handshake_state_set_mde(struct handshake_state *s,
 					const uint8_t *mde);
-void handshake_state_set_fte(struct handshake_state *s, const uint8_t *fte);
 void handshake_state_set_authenticator_fte(struct handshake_state *s,
 						const uint8_t *fte);
 void handshake_state_set_supplicant_fte(struct handshake_state *s,
-- 
2.34.1

[PATCH v3 08/13] ft: add FTE/RSNE building to ft_prepare_handshake

[James Prestwood]

In preparation to remove ft_associate build the FTE/RSNE in
ft_prepare_handshake and set into the handshake object directly.
---
 src/ft.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 82 insertions(+), 2 deletions(-)

diff --git a/src/ft.c b/src/ft.c
index 43399828..d3b9234a 100644
--- a/src/ft.c
+++ b/src/ft.c
@@ -904,9 +904,15 @@ static void ft_info_destroy(void *data)
 	l_free(info);
 }
 
-static void ft_prepare_handshake(struct ft_info *info,
+static bool ft_prepare_handshake(struct ft_info *info,
 					struct handshake_state *hs)
 {
+	uint32_t kck_len = handshake_state_get_kck_len(hs);
+	struct ie_rsn_info rsn_info;
+	struct ie_ft_info ft_info;
+	uint8_t *fte;
+	uint8_t *rsne;
+
 	handshake_state_set_authenticator_address(hs, info->aa);
 
 	memcpy(hs->mde + 2, info->mde, 3);
@@ -914,7 +920,7 @@ static void ft_prepare_handshake(struct ft_info *info,
 	handshake_state_set_chandef(hs, NULL);
 
 	if (!hs->supplicant_ie)
-		return;
+		return true;
 
 	if (info->authenticator_ie)
 		handshake_state_set_authenticator_ie(hs,
@@ -931,6 +937,80 @@ static void ft_prepare_handshake(struct ft_info *info,
 						info->ft_info.r1khid);
 
 	handshake_state_derive_ptk(hs);
+
+	/*
+	 * Rebuild the RSNE to include the PMKR1Name and append
+	 * MDE + FTE.
+	 *
+	 * 12.8.4: "If present, the RSNE shall be set as follows:
+	 * - Version field shall be set to 1.
+	 * - PMKID Count field shall be set to 1.
+	 * - PMKID field shall contain the PMKR1Name.
+	 * - All other fields shall be as specified in 8.4.2.27
+	 *   and 11.5.3."
+	 */
+	if (ie_parse_rsne_from_data(hs->supplicant_ie,
+					hs->supplicant_ie[1] + 2,
+					&rsn_info) < 0)
+		return false;
+
+	rsn_info.num_pmkids = 1;
+	rsn_info.pmkids = hs->pmk_r1_name;
+	/* Always set OCVC false for FT for now */
+	rsn_info.ocvc = false;
+	rsne = alloca(256);
+
+	ie_build_rsne(&rsn_info, rsne);
+	handshake_state_set_supplicant_ie(hs, rsne);
+
+	/*
+	 * 12.8.4: "If present, the FTE shall be set as follows:
+	 * - ANonce, SNonce, R0KH-ID, and R1KH-ID shall be set to
+	 *   the values contained in the second message of this
+	 *   sequence.
+	 * - The Element Count field of the MIC Control field shall
+	 *   be set to the number of elements protected in this
+	 *   frame (variable).
+	 * [...]
+	 * - All other fields shall be set to 0."
+	 */
+	memset(&ft_info, 0, sizeof(ft_info));
+	ft_info.mic_element_count = 3;
+	memcpy(ft_info.r0khid, hs->r0khid, hs->r0khid_len);
+	ft_info.r0khid_len = hs->r0khid_len;
+	memcpy(ft_info.r1khid, hs->r1khid, 6);
+	ft_info.r1khid_present = true;
+	memcpy(ft_info.anonce, hs->anonce, 32);
+	memcpy(ft_info.snonce, hs->snonce, 32);
+
+	/*
+	 * IEEE 802.11-2020 Section 13.7.1 FT reassociation in an RSN
+	 *
+	 * "If dot11RSNAOperatingChannelValidationActivated is true and
+	 *  the FTO indicates OCVC capability, the target AP shall
+	 *  ensure that OCI subelement of the FTE matches by ensuring
+	 *  that all of the following are true:
+	 *      - OCI subelement is present
+	 *      - Channel information in the OCI matches current
+	 *        operating channel parameters (see 12.2.9)"
+	 */
+	if (hs->supplicant_ocvc && hs->chandef) {
+		oci_from_chandef(hs->chandef, ft_info.oci);
+		ft_info.oci_present = true;
+	}
+
+	fte = alloca(256);
+	ie_build_fast_bss_transition(&ft_info, kck_len, fte);
+
+	if (!ft_calculate_fte_mic(hs, 5, rsne, fte, NULL, ft_info.mic))
+		return false;
+
+	/* Rebuild the FT IE now with the MIC included */
+	ie_build_fast_bss_transition(&ft_info, kck_len, fte);
+
+	handshake_state_set_supplicant_fte(hs, fte);
+
+	return true;
 }
 
 static bool ft_send_action(struct wiphy_radio_work_item *work)
-- 
2.34.1

[PATCH v3 09/13] ft: add ft_handshake_setup

[James Prestwood]

This will be called from station after FT-authentication has
finished. It sets up the handshake object to perform reassociation.

This is essentially a copy-paste of ft_associate without sending
the actual frame.
---
 src/ft.c | 40 ++++++++++++++++++++++++++++++++++++++++
 src/ft.h |  2 ++
 2 files changed, 42 insertions(+)

diff --git a/src/ft.c b/src/ft.c
index d3b9234a..c612a433 100644
--- a/src/ft.c
+++ b/src/ft.c
@@ -1276,6 +1276,46 @@ int ft_associate(uint32_t ifindex, const uint8_t *addr)
 	return ret;
 }
 
+int ft_handshake_setup(uint32_t ifindex, const uint8_t *target)
+{
+	struct netdev *netdev = netdev_find(ifindex);
+	struct handshake_state *hs = netdev_get_handshake(netdev);
+	struct ft_info *info;
+	int ret = 0;
+
+	info = ft_info_find(ifindex, target);
+	if (!info)
+		return -ENOENT;
+
+	/*
+	 * Either failed or no response. This may have been an FT-over-DS
+	 * attempt so clear out the entry so FT-over-Air can try again.
+	 */
+	if (info->status != 0) {
+		int status = info->status;
+
+		l_queue_remove(info_list, info);
+		ft_info_destroy(info);
+
+		return status;
+	}
+
+	/*
+	 * This shouldn't ever fail:
+	 *  - supplicant_ie has already been validated long ago
+	 *  - l_checksum_* shouldn't fail since we presumable have kernel
+	 *    support, how else could we have made it this far.
+	 * But just in case...
+	 */
+	if (L_WARN_ON(!ft_prepare_handshake(info, hs)))
+		ret = -EINVAL;
+
+	/* After this no previous auths will be valid */
+	ft_clear_authentications(ifindex);
+
+	return ret;
+}
+
 static bool remove_ifindex(void *data, void *user_data)
 {
 	struct ft_info *info = data;
diff --git a/src/ft.h b/src/ft.h
index 51bbe3bc..23d0136e 100644
--- a/src/ft.h
+++ b/src/ft.h
@@ -39,6 +39,8 @@ void __ft_rx_action(uint32_t ifindex, const uint8_t *frame, size_t frame_len);
 void __ft_rx_authenticate(uint32_t ifindex, const uint8_t *frame,
 				size_t frame_len);
 
+int ft_handshake_setup(uint32_t ifindex, const uint8_t *target);
+
 void ft_clear_authentications(uint32_t ifindex);
 int ft_action(uint32_t ifindex, uint32_t freq, const struct scan_bss *target);
 int ft_associate(uint32_t ifindex, const uint8_t *addr);
-- 
2.34.1

[PATCH v3 10/13] netdev: add netdev_ft_reassociate

[James Prestwood]

Essentially exposes (and renames) netdev_ft_tx_associate in order to
be called similarly to netdev_reassociate/netdev_connect where a
connect callback can be provided. This will fix the current bug where
if association times out during FT IWD will hang and never transition
to disconnected.

This also removes the calling of the FT_ROAMED event and instead just
calls the connect callback (since its now set). This unifies the
callback path for reassociation and FT roaming.
---
 src/netdev.c | 43 +++++++++++++++++++++++++------------------
 src/netdev.h |  5 +++++
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index b9dc930e..522baf7a 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -1409,16 +1409,13 @@ static void netdev_connect_ok(struct netdev *netdev)
 			scan_bss_free(netdev->fw_roam_bss);
 
 		netdev->fw_roam_bss = NULL;
-	} else if (netdev->in_ft) {
-		if (netdev->event_filter)
-			netdev->event_filter(netdev, NETDEV_EVENT_FT_ROAMED,
-						NULL, netdev->user_data);
-		netdev->in_ft = false;
 	} else if (netdev->connect_cb) {
 		netdev->connect_cb(netdev, NETDEV_RESULT_OK, NULL,
 					netdev->user_data);
 		netdev->connect_cb = NULL;
-	}
+		netdev->in_ft = false;
+	} else
+		l_warn("Connection event without a connect callback!");
 
 	netdev_rssi_polling_update(netdev);
 
@@ -4205,13 +4202,14 @@ static int netdev_tx_ft_frame(uint32_t ifindex, uint16_t frame_type,
 	return 0;
 }
 
-static int netdev_ft_tx_associate(uint32_t ifindex, uint32_t freq,
-					const uint8_t *prev_bssid,
-					struct iovec *ft_iov, size_t n_ft_iov)
+int netdev_ft_reassociate(struct netdev *netdev,
+				const struct scan_bss *target_bss,
+				const struct scan_bss *orig_bss,
+				netdev_event_func_t event_filter,
+				netdev_connect_cb_t cb, void *user_data)
 {
-	struct netdev *netdev = netdev_find(ifindex);
-	struct netdev_handshake_state *nhs;
 	struct handshake_state *hs = netdev->handshake;
+	struct netdev_handshake_state *nhs;
 	struct l_genl_msg *msg;
 	struct iovec iov[64];
 	unsigned int n_iov = L_ARRAY_SIZE(iov);
@@ -4223,11 +4221,14 @@ static int netdev_ft_tx_associate(uint32_t ifindex, uint32_t freq,
 	 * At this point there is no going back with FT so reset all the flags
 	 * needed to associate with a new BSS.
 	 */
-	netdev->frequency = freq;
+	netdev->frequency = target_bss->frequency;
 	netdev->handshake->active_tk_index = 0;
 	netdev->associated = false;
 	netdev->operational = false;
 	netdev->in_ft = true;
+	netdev->event_filter = event_filter;
+	netdev->connect_cb = cb;
+	netdev->user_data = user_data;
 
 	/*
 	 * Cancel commands that could be running because of EAPoL activity
@@ -4271,15 +4272,22 @@ static int netdev_ft_tx_associate(uint32_t ifindex, uint32_t freq,
 
 	c_iov = netdev_populate_common_ies(netdev, hs, msg, iov, n_iov, c_iov);
 
-	if (!L_WARN_ON(n_iov - c_iov < n_ft_iov)) {
-		memcpy(iov + c_iov, ft_iov, sizeof(*ft_iov) * n_ft_iov);
-		c_iov += n_ft_iov;
-	}
+	if (hs->supplicant_ie)
+		c_iov = iov_ie_append(iov, n_iov, c_iov, hs->supplicant_ie,
+					IE_LEN(hs->supplicant_ie));
+
+	if (hs->supplicant_fte)
+		c_iov = iov_ie_append(iov, n_iov, c_iov, hs->supplicant_fte,
+					IE_LEN(hs->supplicant_fte));
+
+	if (hs->mde)
+		c_iov = iov_ie_append(iov, n_iov, c_iov, hs->mde,
+					IE_LEN(hs->mde));
 
 	mpdu_sort_ies(subtype, iov, c_iov);
 
 	l_genl_msg_append_attr(msg, NL80211_ATTR_PREV_BSSID, ETH_ALEN,
-				prev_bssid);
+				orig_bss->addr);
 	l_genl_msg_append_attrv(msg, NL80211_ATTR_IE, iov, c_iov);
 
 	netdev->connect_cmd_id = l_genl_family_send(nl80211, msg,
@@ -6256,7 +6264,6 @@ static int netdev_init(void)
 	__eapol_set_install_pmk_func(netdev_set_pmk);
 
 	__ft_set_tx_frame_func(netdev_tx_ft_frame);
-	__ft_set_tx_associate_func(netdev_ft_tx_associate);
 
 	unicast_watch = l_genl_add_unicast_watch(genl, NL80211_GENL_NAME,
 						netdev_unicast_notify,
diff --git a/src/netdev.h b/src/netdev.h
index 03d1b6e9..fb31b571 100644
--- a/src/netdev.h
+++ b/src/netdev.h
@@ -165,6 +165,11 @@ int netdev_reassociate(struct netdev *netdev,
 			struct handshake_state *hs,
 			netdev_event_func_t event_filter,
 			netdev_connect_cb_t cb, void *user_data);
+int netdev_ft_reassociate(struct netdev *netdev,
+				const struct scan_bss *target_bss,
+				const struct scan_bss *orig_bss,
+				netdev_event_func_t event_filter,
+				netdev_connect_cb_t cb, void *user_data);
 
 int netdev_preauthenticate(struct netdev *netdev,
 				const struct scan_bss *target_bss,
-- 
2.34.1

[PATCH v3 11/13] station: use netdev_ft_reassociate

[James Prestwood]

Using this will provide netdev with a connect callback and unify the
roaming result notification between FT and reassociation. Both paths
will now end up in station_reassociate_cb.

This also adds another return case for ft_handshake_setup which was
previously ignored by ft_associate. Its likely impossible to actually
happen but should be handled nevertheless.

Fixes: 30c6a10f28 ("netdev: Separate connect_failed and disconnected paths")
---
 src/station.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/station.c b/src/station.c
index 48a595dc..52d87c84 100644
--- a/src/station.c
+++ b/src/station.c
@@ -2181,7 +2181,8 @@ static void station_reassociate_cb(struct netdev *netdev,
 
 	l_debug("%u, result: %d", netdev_get_ifindex(station->netdev), result);
 
-	if (station->state != STATION_STATE_ROAMING)
+	if (station->state != STATION_STATE_ROAMING &&
+			station->state != STATION_STATE_FT_ROAMING)
 		return;
 
 	if (result == NETDEV_RESULT_OK)
@@ -2314,7 +2315,8 @@ static bool station_ft_work_ready(struct wiphy_radio_work_item *item)
 	if (!bss)
 		goto try_next;
 
-	ret = ft_associate(netdev_get_ifindex(station->netdev), bss->addr);
+	ret = ft_handshake_setup(netdev_get_ifindex(station->netdev),
+					bss->addr);
 	switch (ret) {
 	case MMPDU_STATUS_CODE_INVALID_PMKID:
 		/*
@@ -2343,12 +2345,28 @@ try_next:
 		station_transition_start(station);
 		break;
 	case 0:
+		ret = netdev_ft_reassociate(station->netdev, bss,
+					station->connected_bss,
+					station_netdev_event,
+					station_reassociate_cb, station);
+		if (ret < 0)
+			goto disassociate;
+
 		station->connected_bss = bss;
 		station->preparing_roam = false;
 		station_enter_state(station, STATION_STATE_FT_ROAMING);
 
 		station_debug_event(station, "ft-roam");
 
+		break;
+	case -EINVAL:
+		/*
+		 * Likely an impossible situation, but since ft_handshake_setup
+		 * rederived the handshake keys we can't do anything but
+		 * disconnect.
+		 */
+disassociate:
+		station_disassociated(station);
 		break;
 	default:
 		if (ret > 0)
-- 
2.34.1

[PATCH v3 12/13] ft: remove ft_associate and helpers

[James Prestwood]

The reassociation is done through netdev directly, these are no
longer needed.
---
 src/ft.c | 160 +------------------------------------------------------
 src/ft.h |   2 -
 2 files changed, 1 insertion(+), 161 deletions(-)

diff --git a/src/ft.c b/src/ft.c
index c612a433..d8bee74c 100644
--- a/src/ft.c
+++ b/src/ft.c
@@ -43,7 +43,6 @@
 static const unsigned int FT_ONCHANNEL_TIME = 300u; /* ms */
 
 static ft_tx_frame_func_t tx_frame = NULL;
-static ft_tx_associate_func_t tx_assoc = NULL;
 static struct l_queue *info_list = NULL;
 
 struct ft_info {
@@ -224,117 +223,6 @@ static bool ft_parse_associate_resp_frame(const uint8_t *frame, size_t frame_len
 	return true;
 }
 
-static int ft_tx_reassociate(uint32_t ifindex, uint32_t freq,
-				const uint8_t *prev_bssid)
-{
-	struct netdev *netdev = netdev_find(ifindex);
-	struct handshake_state *hs = netdev_get_handshake(netdev);
-	struct iovec iov[3];
-	int iov_elems = 0;
-	uint32_t kck_len = handshake_state_get_kck_len(hs);
-	bool is_rsn = hs->supplicant_ie != NULL;
-	uint8_t *rsne = NULL;
-
-	if (is_rsn) {
-		struct ie_rsn_info rsn_info;
-
-		/*
-		 * Rebuild the RSNE to include the PMKR1Name and append
-		 * MDE + FTE.
-		 *
-		 * 12.8.4: "If present, the RSNE shall be set as follows:
-		 * - Version field shall be set to 1.
-		 * - PMKID Count field shall be set to 1.
-		 * - PMKID field shall contain the PMKR1Name.
-		 * - All other fields shall be as specified in 8.4.2.27
-		 *   and 11.5.3."
-		 */
-		if (ie_parse_rsne_from_data(hs->supplicant_ie,
-						hs->supplicant_ie[1] + 2,
-						&rsn_info) < 0)
-			goto error;
-
-		rsn_info.num_pmkids = 1;
-		rsn_info.pmkids = hs->pmk_r1_name;
-
-		/* Always set OCVC false for FT for now */
-		rsn_info.ocvc = false;
-
-		rsne = alloca(256);
-		ie_build_rsne(&rsn_info, rsne);
-
-		iov[iov_elems].iov_base = rsne;
-		iov[iov_elems].iov_len = rsne[1] + 2;
-		iov_elems += 1;
-	}
-
-	/* The MDE advertised by the BSS must be passed verbatim */
-	iov[iov_elems].iov_base = (void *) hs->mde;
-	iov[iov_elems].iov_len = hs->mde[1] + 2;
-	iov_elems += 1;
-
-	if (is_rsn) {
-		struct ie_ft_info ft_info;
-		uint8_t *fte;
-
-		/*
-		 * 12.8.4: "If present, the FTE shall be set as follows:
-		 * - ANonce, SNonce, R0KH-ID, and R1KH-ID shall be set to
-		 *   the values contained in the second message of this
-		 *   sequence.
-		 * - The Element Count field of the MIC Control field shall
-		 *   be set to the number of elements protected in this
-		 *   frame (variable).
-		 * [...]
-		 * - All other fields shall be set to 0."
-		 */
-
-		memset(&ft_info, 0, sizeof(ft_info));
-
-		ft_info.mic_element_count = 3;
-		memcpy(ft_info.r0khid, hs->r0khid, hs->r0khid_len);
-		ft_info.r0khid_len = hs->r0khid_len;
-		memcpy(ft_info.r1khid, hs->r1khid, 6);
-		ft_info.r1khid_present = true;
-		memcpy(ft_info.anonce, hs->anonce, 32);
-		memcpy(ft_info.snonce, hs->snonce, 32);
-
-		/*
-		 * IEEE 802.11-2020 Section 13.7.1 FT reassociation in an RSN
-		 *
-		 * "If dot11RSNAOperatingChannelValidationActivated is true and
-		 *  the FTO indicates OCVC capability, the target AP shall
-		 *  ensure that OCI subelement of the FTE matches by ensuring
-		 *  that all of the following are true:
-		 *      - OCI subelement is present
-		 *      - Channel information in the OCI matches current
-		 *        operating channel parameters (see 12.2.9)"
-		 */
-		if (hs->supplicant_ocvc && hs->chandef) {
-			oci_from_chandef(hs->chandef, ft_info.oci);
-			ft_info.oci_present = true;
-		}
-
-		fte = alloca(256);
-		ie_build_fast_bss_transition(&ft_info, kck_len, fte);
-
-		if (!ft_calculate_fte_mic(hs, 5, rsne, fte, NULL, ft_info.mic))
-			goto error;
-
-		/* Rebuild the FT IE now with the MIC included */
-		ie_build_fast_bss_transition(&ft_info, kck_len, fte);
-
-		iov[iov_elems].iov_base = fte;
-		iov[iov_elems].iov_len = fte[1] + 2;
-		iov_elems += 1;
-	}
-
-	return tx_assoc(ifindex, freq, prev_bssid, iov, iov_elems);
-
-error:
-	return -EINVAL;
-}
-
 static bool ft_verify_rsne(const uint8_t *rsne, const uint8_t *pmk_r0_name,
 				const uint8_t *authenticator_ie)
 {
@@ -762,11 +650,6 @@ void __ft_set_tx_frame_func(ft_tx_frame_func_t func)
 	tx_frame = func;
 }
 
-void __ft_set_tx_associate_func(ft_tx_associate_func_t func)
-{
-	tx_assoc = func;
-}
-
 static bool ft_parse_ies(struct ft_info *info, struct handshake_state *hs,
 			const uint8_t *ies, size_t ies_len)
 {
@@ -1173,7 +1056,7 @@ static void ft_authenticate_destroy(int error, void *user_data)
 /*
  * There is no callback here because its assumed that another work item will
  * be inserted following this call which will check if authentication succeeded
- * via ft_associate.
+ * via ft_handshake_setup.
  *
  * If the netdev goes away while authentication is in-flight station will clear
  * the authentications during cleanup, and in turn cancel the offchannel
@@ -1235,47 +1118,6 @@ int ft_authenticate_onchannel(uint32_t ifindex, const struct scan_bss *target)
 	return 0;
 }
 
-int ft_associate(uint32_t ifindex, const uint8_t *addr)
-{
-	struct netdev *netdev = netdev_find(ifindex);
-	struct handshake_state *hs = netdev_get_handshake(netdev);
-	struct ft_info *info;
-	int ret;
-
-	/*
-	 * TODO: Since FT-over-DS is done early, before the time of roaming, it
-	 *       may end up that a completely new BSS is the best candidate and
-	 *       we haven't yet authenticated. We could actually authenticate
-	 *       at this point, but for now just assume the caller will choose
-	 *       a different BSS.
-	 */
-	info = ft_info_find(ifindex, addr);
-	if (!info)
-		return -ENOENT;
-
-	/*
-	 * Either failed or no response. This may have been an FT-over-DS
-	 * attempt so clear out the entry so FT-over-Air can try again.
-	 */
-	if (info->status != 0) {
-		int status = info->status;
-
-		l_queue_remove(info_list, info);
-		ft_info_destroy(info);
-
-		return status;
-	}
-
-	ft_prepare_handshake(info, hs);
-
-	ret = ft_tx_reassociate(ifindex, info->frequency, info->prev_bssid);
-
-	/* After this no previous auths will be valid */
-	ft_clear_authentications(ifindex);
-
-	return ret;
-}
-
 int ft_handshake_setup(uint32_t ifindex, const uint8_t *target)
 {
 	struct netdev *netdev = netdev_find(ifindex);
diff --git a/src/ft.h b/src/ft.h
index 23d0136e..92c207fb 100644
--- a/src/ft.h
+++ b/src/ft.h
@@ -32,7 +32,6 @@ typedef int (*ft_tx_associate_func_t)(uint32_t ifindex, uint32_t freq,
 					struct iovec *ie_iov, size_t iov_len);
 
 void __ft_set_tx_frame_func(ft_tx_frame_func_t func);
-void __ft_set_tx_associate_func(ft_tx_associate_func_t func);
 int __ft_rx_associate(uint32_t ifindex, const uint8_t *frame,
 			size_t frame_len);
 void __ft_rx_action(uint32_t ifindex, const uint8_t *frame, size_t frame_len);
@@ -43,6 +42,5 @@ int ft_handshake_setup(uint32_t ifindex, const uint8_t *target);
 
 void ft_clear_authentications(uint32_t ifindex);
 int ft_action(uint32_t ifindex, uint32_t freq, const struct scan_bss *target);
-int ft_associate(uint32_t ifindex, const uint8_t *addr);
 int ft_authenticate(uint32_t ifindex, const struct scan_bss *target);
 int ft_authenticate_onchannel(uint32_t ifindex, const struct scan_bss *target);
-- 
2.34.1

[PATCH v3 13/13] netdev: station: remove NETDEV_EVENT_FT_ROAMED

[James Prestwood]

The notification for roaming success/failure is now handled with
the connect callback.
---
 src/netdev.h  | 1 -
 src/station.c | 6 ------
 2 files changed, 7 deletions(-)

diff --git a/src/netdev.h b/src/netdev.h
index fb31b571..d87f09f4 100644
--- a/src/netdev.h
+++ b/src/netdev.h
@@ -50,7 +50,6 @@ enum netdev_event {
 	NETDEV_EVENT_RSSI_THRESHOLD_HIGH,
 	NETDEV_EVENT_RSSI_LEVEL_NOTIFY,
 	NETDEV_EVENT_PACKET_LOSS_NOTIFY,
-	NETDEV_EVENT_FT_ROAMED,
 	NETDEV_EVENT_BEACON_LOSS_NOTIFY,
 };
 
diff --git a/src/station.c b/src/station.c
index 52d87c84..73de26bb 100644
--- a/src/station.c
+++ b/src/station.c
@@ -3444,12 +3444,6 @@ static void station_netdev_event(struct netdev *netdev, enum netdev_event event,
 	case NETDEV_EVENT_PACKET_LOSS_NOTIFY:
 		station_packets_lost(station, l_get_u32(event_data));
 		break;
-	case NETDEV_EVENT_FT_ROAMED:
-		if (L_WARN_ON(station->state != STATION_STATE_FT_ROAMING))
-			return;
-
-		station_roamed(station);
-		break;
 	case NETDEV_EVENT_BEACON_LOSS_NOTIFY:
 		station_beacon_lost(station);
 		break;
-- 
2.34.1

Re: [PATCH v3 01/13] auto-t: add explicit stop() to IWD class

[Denis Kenzior]

Hi James,

On 12/6/23 14:17, James Prestwood wrote:
> If tests end in an unknown state it is sometimes required that IWD
> be stopped manually in order for future tests to run. Add a stop()
> method so test tearDown() methods can explicitly stop IWD.
> ---
>   autotests/util/iwd.py | 4 ++++
>   1 file changed, 4 insertions(+)
> 

All applied, thanks.

Regards,
-Denis