* [PATCH v2 1/9] auto-t: add explicit stop() to IWD class
2023-12-06 15:06 [PATCH v2 0/9] Reassoc/FT roaming unification James Prestwood
@ 2023-12-06 15:07 ` James Prestwood
2023-12-06 15:07 ` [PATCH v2 2/9] auto-t: add association timeout test James Prestwood
` (7 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: James Prestwood @ 2023-12-06 15:07 UTC (permalink / raw)
To: iwd; +Cc: 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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 2/9] auto-t: add association timeout test
2023-12-06 15:06 [PATCH v2 0/9] Reassoc/FT roaming unification James Prestwood
2023-12-06 15:07 ` [PATCH v2 1/9] auto-t: add explicit stop() to IWD class James Prestwood
@ 2023-12-06 15:07 ` James Prestwood
2023-12-06 15:07 ` [PATCH v2 3/9] auto-t: only call set_value for changed values in default() James Prestwood
` (6 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: James Prestwood @ 2023-12-06 15:07 UTC (permalink / raw)
To: iwd; +Cc: 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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 3/9] auto-t: only call set_value for changed values in default()
2023-12-06 15:06 [PATCH v2 0/9] Reassoc/FT roaming unification James Prestwood
2023-12-06 15:07 ` [PATCH v2 1/9] auto-t: add explicit stop() to IWD class James Prestwood
2023-12-06 15:07 ` [PATCH v2 2/9] auto-t: add association timeout test James Prestwood
@ 2023-12-06 15:07 ` James Prestwood
2023-12-06 15:07 ` [PATCH v2 4/9] ft: add FTE/RSNE building to ft_prepare_handshake James Prestwood
` (5 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: James Prestwood @ 2023-12-06 15:07 UTC (permalink / raw)
To: iwd; +Cc: 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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 4/9] ft: add FTE/RSNE building to ft_prepare_handshake
2023-12-06 15:06 [PATCH v2 0/9] Reassoc/FT roaming unification James Prestwood
` (2 preceding siblings ...)
2023-12-06 15:07 ` [PATCH v2 3/9] auto-t: only call set_value for changed values in default() James Prestwood
@ 2023-12-06 15:07 ` James Prestwood
2023-12-06 16:36 ` Denis Kenzior
2023-12-06 15:07 ` [PATCH v2 5/9] ft: add ft_handshake_setup James Prestwood
` (4 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: James Prestwood @ 2023-12-06 15:07 UTC (permalink / raw)
To: iwd; +Cc: 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 2cc611b8..358a4594 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_fte(hs, fte);
+
+ return true;
}
static bool ft_send_action(struct wiphy_radio_work_item *work)
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 4/9] ft: add FTE/RSNE building to ft_prepare_handshake
2023-12-06 15:07 ` [PATCH v2 4/9] ft: add FTE/RSNE building to ft_prepare_handshake James Prestwood
@ 2023-12-06 16:36 ` Denis Kenzior
2023-12-06 17:08 ` James Prestwood
0 siblings, 1 reply; 18+ messages in thread
From: Denis Kenzior @ 2023-12-06 16:36 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
On 12/6/23 09:07, James Prestwood wrote:
> 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(-)
>
<snip>
> @@ -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);
This is probably safe since we over-write the supplicant ie in
netdev_connect_event() -> parse_request_ies()
> +
> + /*
> + * 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_fte(hs, fte);
However, this is less clear to me. Looking at how FILS and FT uses this API, it
seems that set_fte is meant for the authenticator FTE element? So I think
rekeying after FT would be broken by this change.
> +
> + return true;
> }
>
> static bool ft_send_action(struct wiphy_radio_work_item *work)
Regards,
-Denis
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 4/9] ft: add FTE/RSNE building to ft_prepare_handshake
2023-12-06 16:36 ` Denis Kenzior
@ 2023-12-06 17:08 ` James Prestwood
2023-12-06 17:14 ` Denis Kenzior
0 siblings, 1 reply; 18+ messages in thread
From: James Prestwood @ 2023-12-06 17:08 UTC (permalink / raw)
To: Denis Kenzior, iwd
Hi Denis,
On 12/6/23 08:36, Denis Kenzior wrote:
> Hi James,
>
> On 12/6/23 09:07, James Prestwood wrote:
>> 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(-)
>>
>
> <snip>
>> + 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_fte(hs, fte);
>
> However, this is less clear to me. Looking at how FILS and FT uses
> this API, it seems that set_fte is meant for the authenticator FTE
> element? So I think rekeying after FT would be broken by this change.
Good question. Rekeys do appear to work as-is but you are right, FILS/FT
uses set_fte() for the authenticators element, but eapol seems to use
hs->fte for building message 2/4, as well as checks that the handshakes
FTE matches what the authenticator sends in 3/4. maybe this is actually
a bug in eapol? I think the reason everything "works" is because the FTE
should be the same between both peers.
We may want to refactor and do:
handshake_state_set_authenticator_fte()
handshake_state_set_supplicant_fte()
Thanks,
James
>
>> +
>> + return true;
>> }
>> static bool ft_send_action(struct wiphy_radio_work_item *work)
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/9] ft: add FTE/RSNE building to ft_prepare_handshake
2023-12-06 17:08 ` James Prestwood
@ 2023-12-06 17:14 ` Denis Kenzior
2023-12-06 17:59 ` James Prestwood
0 siblings, 1 reply; 18+ messages in thread
From: Denis Kenzior @ 2023-12-06 17:14 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
>>> + handshake_state_set_fte(hs, fte);
>>
>> However, this is less clear to me. Looking at how FILS and FT uses this API,
>> it seems that set_fte is meant for the authenticator FTE element? So I think
>> rekeying after FT would be broken by this change.
>
> Good question. Rekeys do appear to work as-is but you are right, FILS/FT uses
> set_fte() for the authenticators element, but eapol seems to use hs->fte for
I'm pretty sure the intent was for the FTE element to be from the authenticator.
> building message 2/4, as well as checks that the handshakes FTE matches what the
I'll have to look at how ptk_2_of_4 uses it. Memory is fuzzy now. Need to open
the spec.
> authenticator sends in 3/4. maybe this is actually a bug in eapol? I think the
> reason everything "works" is because the FTE should be the same between both peers.
Yes, but also we have logic in: netdev_connect_event() that sets the FTE from
the response IEs. So that's probably how things end up working in the end.
>
> We may want to refactor and do:
>
> handshake_state_set_authenticator_fte()
>
> handshake_state_set_supplicant_fte()
Yeah, that seems reasonable.
Regards,
-Denis
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/9] ft: add FTE/RSNE building to ft_prepare_handshake
2023-12-06 17:14 ` Denis Kenzior
@ 2023-12-06 17:59 ` James Prestwood
0 siblings, 0 replies; 18+ messages in thread
From: James Prestwood @ 2023-12-06 17:59 UTC (permalink / raw)
To: Denis Kenzior, iwd
On 12/6/23 09:14, Denis Kenzior wrote:
> Hi James,
>
>>>> + handshake_state_set_fte(hs, fte);
>>>
>>> However, this is less clear to me. Looking at how FILS and FT uses
>>> this API, it seems that set_fte is meant for the authenticator FTE
>>> element? So I think rekeying after FT would be broken by this change.
>>
>> Good question. Rekeys do appear to work as-is but you are right,
>> FILS/FT uses set_fte() for the authenticators element, but eapol
>> seems to use hs->fte for
>
> I'm pretty sure the intent was for the FTE element to be from the
> authenticator.
>
>> building message 2/4, as well as checks that the handshakes FTE
>> matches what the
>
> I'll have to look at how ptk_2_of_4 uses it. Memory is fuzzy now.
> Need to open the spec.
All I could find quickly was:
12.7.6.3 4-way handshake message 2
"and that the FTE and MDE are the same as those provided in the AP’s
(Re)Association Response frame"
So it does appear for EAPoL its the authenticators FTE.
>
>> authenticator sends in 3/4. maybe this is actually a bug in eapol? I
>> think the reason everything "works" is because the FTE should be the
>> same between both peers.
>
> Yes, but also we have logic in: netdev_connect_event() that sets the
> FTE from the response IEs. So that's probably how things end up
> working in the end.
>
>>
>> We may want to refactor and do:
>>
>> handshake_state_set_authenticator_fte()
>>
>> handshake_state_set_supplicant_fte()
>
> Yeah, that seems reasonable.
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 5/9] ft: add ft_handshake_setup
2023-12-06 15:06 [PATCH v2 0/9] Reassoc/FT roaming unification James Prestwood
` (3 preceding siblings ...)
2023-12-06 15:07 ` [PATCH v2 4/9] ft: add FTE/RSNE building to ft_prepare_handshake James Prestwood
@ 2023-12-06 15:07 ` James Prestwood
2023-12-06 16:38 ` Denis Kenzior
2023-12-06 15:07 ` [PATCH v2 6/9] netdev: add netdev_ft_reassociate James Prestwood
` (3 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: James Prestwood @ 2023-12-06 15:07 UTC (permalink / raw)
To: iwd; +Cc: 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 | 32 ++++++++++++++++++++++++++++++++
src/ft.h | 2 ++
2 files changed, 34 insertions(+)
diff --git a/src/ft.c b/src/ft.c
index 358a4594..738e08c3 100644
--- a/src/ft.c
+++ b/src/ft.c
@@ -1276,6 +1276,38 @@ 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;
+
+ 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;
+ }
+
+ if (L_WARN_ON(!ft_prepare_handshake(info, hs)))
+ return -EINVAL;
+
+ /* After this no previous auths will be valid */
+ ft_clear_authentications(ifindex);
+
+ return 0;
+}
+
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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 5/9] ft: add ft_handshake_setup
2023-12-06 15:07 ` [PATCH v2 5/9] ft: add ft_handshake_setup James Prestwood
@ 2023-12-06 16:38 ` Denis Kenzior
2023-12-06 16:46 ` James Prestwood
0 siblings, 1 reply; 18+ messages in thread
From: Denis Kenzior @ 2023-12-06 16:38 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
On 12/6/23 09:07, James Prestwood wrote:
> 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 | 32 ++++++++++++++++++++++++++++++++
> src/ft.h | 2 ++
> 2 files changed, 34 insertions(+)
>
> diff --git a/src/ft.c b/src/ft.c
> index 358a4594..738e08c3 100644
> --- a/src/ft.c
> +++ b/src/ft.c
> @@ -1276,6 +1276,38 @@ 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;
> +
> + 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;
> + }
> +
> + if (L_WARN_ON(!ft_prepare_handshake(info, hs)))
> + return -EINVAL;
It isn't quite clear how this case should be handled? Would you still remove it
from the queue and destroy this info object? Or destroy all authentications for
the ifindex?
> +
> + /* After this no previous auths will be valid */
> + ft_clear_authentications(ifindex);
> +
> + return 0;
> +}
> +
> static bool remove_ifindex(void *data, void *user_data)
> {
> struct ft_info *info = data;
Regards,
-Denis
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 5/9] ft: add ft_handshake_setup
2023-12-06 16:38 ` Denis Kenzior
@ 2023-12-06 16:46 ` James Prestwood
0 siblings, 0 replies; 18+ messages in thread
From: James Prestwood @ 2023-12-06 16:46 UTC (permalink / raw)
To: Denis Kenzior, iwd
Hi Denis,
On 12/6/23 08:38, Denis Kenzior wrote:
> Hi James,
>
> On 12/6/23 09:07, James Prestwood wrote:
>> 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 | 32 ++++++++++++++++++++++++++++++++
>> src/ft.h | 2 ++
>> 2 files changed, 34 insertions(+)
>>
>> diff --git a/src/ft.c b/src/ft.c
>> index 358a4594..738e08c3 100644
>> --- a/src/ft.c
>> +++ b/src/ft.c
>> @@ -1276,6 +1276,38 @@ 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;
>> +
>> + 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;
>> + }
>> +
>> + if (L_WARN_ON(!ft_prepare_handshake(info, hs)))
>> + return -EINVAL;
>
> It isn't quite clear how this case should be handled? Would you still
> remove it from the queue and destroy this info object? Or destroy all
> authentications for the ifindex?
Hmm good point. If that fails we've already wiped the old keys from the
handshake so I don't think we could even try and roam elsewhere. This
case wasn't handled prior, we would just send the associate without
deriving the proper IEs. So I think we either need to handle this in
station and fatally fail the roam, and disconnect
I don't feel like doing it at the moment but we could create a new
handshake object too and set only after everything succeeds.
>> + /* After this no previous auths will be valid */
>> + ft_clear_authentications(ifindex);
>> +
>> + return 0;
>> +}
>> +
>> static bool remove_ifindex(void *data, void *user_data)
>> {
>> struct ft_info *info = data;
>
> Regards,
> -Denis
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 6/9] netdev: add netdev_ft_reassociate
2023-12-06 15:06 [PATCH v2 0/9] Reassoc/FT roaming unification James Prestwood
` (4 preceding siblings ...)
2023-12-06 15:07 ` [PATCH v2 5/9] ft: add ft_handshake_setup James Prestwood
@ 2023-12-06 15:07 ` James Prestwood
2023-12-06 16:40 ` Denis Kenzior
2023-12-06 15:07 ` [PATCH v2 7/9] station: use netdev_ft_reassociate James Prestwood
` (2 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: James Prestwood @ 2023-12-06 15:07 UTC (permalink / raw)
To: iwd; +Cc: 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 | 44 ++++++++++++++++++++++++++------------------
src/netdev.h | 5 +++++
2 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/src/netdev.c b/src/netdev.c
index f2e887b4..7d52ffea 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -1409,16 +1409,14 @@ 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;
+ netdev->in_reassoc = false;
+ } else
+ l_warn("Connection event without a connect callback!");
netdev_rssi_polling_update(netdev);
@@ -4205,13 +4203,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 +4222,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 +4273,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->fte)
+ c_iov = iov_ie_append(iov, n_iov, c_iov, hs->fte,
+ IE_LEN(hs->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 +6265,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
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 6/9] netdev: add netdev_ft_reassociate
2023-12-06 15:07 ` [PATCH v2 6/9] netdev: add netdev_ft_reassociate James Prestwood
@ 2023-12-06 16:40 ` Denis Kenzior
2023-12-06 16:49 ` James Prestwood
0 siblings, 1 reply; 18+ messages in thread
From: Denis Kenzior @ 2023-12-06 16:40 UTC (permalink / raw)
To: James Prestwood, iwd
Hi James,
On 12/6/23 09:07, James Prestwood wrote:
> 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.
>
I like it!
Nitpick below...
> 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 | 44 ++++++++++++++++++++++++++------------------
> src/netdev.h | 5 +++++
> 2 files changed, 31 insertions(+), 18 deletions(-)
>
<snip>
> diff --git a/src/netdev.c b/src/netdev.c
> index f2e887b4..7d52ffea 100644
> --- a/src/netdev.c
> +++ b/src/netdev.c
> @@ -1409,16 +1409,14 @@ 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;
> + netdev->in_reassoc = false;
Why do we set in_reassoc to false here? Shouldn't it be taken care of already
by the associate_event?
> + } else
> + l_warn("Connection event without a connect callback!");
>
> netdev_rssi_polling_update(netdev);
>
<snip>
> @@ -6256,7 +6265,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);
This part probably belongs in the next patch or even patch 8.
>
> unicast_watch = l_genl_add_unicast_watch(genl, NL80211_GENL_NAME,
> netdev_unicast_notify,
Regards,
-Denis
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 6/9] netdev: add netdev_ft_reassociate
2023-12-06 16:40 ` Denis Kenzior
@ 2023-12-06 16:49 ` James Prestwood
0 siblings, 0 replies; 18+ messages in thread
From: James Prestwood @ 2023-12-06 16:49 UTC (permalink / raw)
To: Denis Kenzior, iwd
On 12/6/23 08:40, Denis Kenzior wrote:
> Hi James,
>
> On 12/6/23 09:07, James Prestwood wrote:
>> 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.
>>
>
> I like it!
>
> Nitpick below...
>
>> 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 | 44 ++++++++++++++++++++++++++------------------
>> src/netdev.h | 5 +++++
>> 2 files changed, 31 insertions(+), 18 deletions(-)
>>
>
> <snip>
>
>> diff --git a/src/netdev.c b/src/netdev.c
>> index f2e887b4..7d52ffea 100644
>> --- a/src/netdev.c
>> +++ b/src/netdev.c
>> @@ -1409,16 +1409,14 @@ 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;
>> + netdev->in_reassoc = false;
>
> Why do we set in_reassoc to false here? Shouldn't it be taken care of
> already by the associate_event?
I did this out of an abundance of caution. But yes I see we set it to
false if !netdev->ap && !netdev->in_ft, so I can remove this.
>
>> + } else
>> + l_warn("Connection event without a connect callback!");
>> netdev_rssi_polling_update(netdev);
>
> <snip>
>
>> @@ -6256,7 +6265,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);
>
> This part probably belongs in the next patch or even patch 8.
The problem is I removed the function so gcc will warn/break the build.
>
>> unicast_watch = l_genl_add_unicast_watch(genl,
>> NL80211_GENL_NAME,
>> netdev_unicast_notify,
>
> Regards,
> -Denis
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 7/9] station: use netdev_ft_reassociate
2023-12-06 15:06 [PATCH v2 0/9] Reassoc/FT roaming unification James Prestwood
` (5 preceding siblings ...)
2023-12-06 15:07 ` [PATCH v2 6/9] netdev: add netdev_ft_reassociate James Prestwood
@ 2023-12-06 15:07 ` James Prestwood
2023-12-06 15:07 ` [PATCH v2 8/9] ft: remove ft_associate and helpers James Prestwood
2023-12-06 15:07 ` [PATCH v2 9/9] netdev: station: remove NETDEV_EVENT_FT_ROAMED James Prestwood
8 siblings, 0 replies; 18+ messages in thread
From: James Prestwood @ 2023-12-06 15:07 UTC (permalink / raw)
To: iwd; +Cc: 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.
Fixes: 30c6a10f28 ("netdev: Separate connect_failed and disconnected paths")
---
src/station.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/station.c b/src/station.c
index 48a595dc..b5e78a37 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,6 +2345,13 @@ 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 roam_failed;
+
station->connected_bss = bss;
station->preparing_roam = false;
station_enter_state(station, STATION_STATE_FT_ROAMING);
@@ -2354,6 +2363,7 @@ try_next:
if (ret > 0)
goto try_next;
+roam_failed:
station_roam_failed(station);
break;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 8/9] ft: remove ft_associate and helpers
2023-12-06 15:06 [PATCH v2 0/9] Reassoc/FT roaming unification James Prestwood
` (6 preceding siblings ...)
2023-12-06 15:07 ` [PATCH v2 7/9] station: use netdev_ft_reassociate James Prestwood
@ 2023-12-06 15:07 ` James Prestwood
2023-12-06 15:07 ` [PATCH v2 9/9] netdev: station: remove NETDEV_EVENT_FT_ROAMED James Prestwood
8 siblings, 0 replies; 18+ messages in thread
From: James Prestwood @ 2023-12-06 15:07 UTC (permalink / raw)
To: iwd; +Cc: 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 738e08c3..add6a42b 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
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 9/9] netdev: station: remove NETDEV_EVENT_FT_ROAMED
2023-12-06 15:06 [PATCH v2 0/9] Reassoc/FT roaming unification James Prestwood
` (7 preceding siblings ...)
2023-12-06 15:07 ` [PATCH v2 8/9] ft: remove ft_associate and helpers James Prestwood
@ 2023-12-06 15:07 ` James Prestwood
8 siblings, 0 replies; 18+ messages in thread
From: James Prestwood @ 2023-12-06 15:07 UTC (permalink / raw)
To: iwd; +Cc: 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 b5e78a37..f5d6914f 100644
--- a/src/station.c
+++ b/src/station.c
@@ -3436,12 +3436,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
^ permalink raw reply related [flat|nested] 18+ messages in thread