This works around a hostapd bug (described more in the TODO comment) that deaths IWD if the initial commit is not acked. This behavior has been identified in consumer access points and likely won't ever be patched for older devices. Because of this IWD must work around the problem which can be eliminated by not sending out this commit message. This bug was reported to the hostapd ML: https://lists.infradead.org/pipermail/hostap/2021-September/039842.html This change should not cause any compatibility problems to non-hostapd access points and is identical to how wpa_supplicant treats this scenario. --- src/sae.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) v2: * Changed to simply drop the message. Sending only a confirm is pointless since it would not verify without a commit. Also remove sc++ since this should only be incremented when a confirm is sent. diff --git a/src/sae.c b/src/sae.c index c14b646f..de2e0e0a 100644 --- a/src/sae.c +++ b/src/sae.c @@ -1163,14 +1163,26 @@ static int sae_verify_confirmed(struct sae_sm *sm, uint16_t trans, * transmit its Commit and Confirm (with the new Sc value) messages. */ sm->sync++; - sm->sc++; - - sae_send_commit(sm, true); - if (!sae_send_confirm(sm)) - return -EPROTO; + /* + * TODO: There is a bug in hostapd which deaths stations if a commit + * is received in an Accepted SAE state. This can be triggered if the + * STA's commit is not acked, which triggers a re-transmission, and + * ultimated causes the AP to death IWD. + * + * Since this bug is present in production APs out in the wild we must + * work around it by going against the spec (802.11-2020 12.4.8.6.5) and + * simply drop the message. + * + * sm->sc++; + * + * sae_send_commit(sm, true); + * + * if (!sae_send_confirm(sm)) + * return -EPROTO; + */ - return -EAGAIN; + return -EBADMSG; } /* -- 2.31.1

Since all auth-protos are hidden behind an abstraction they need to be consisten with the return values as some should be handled specially. --- src/auth-proto.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/auth-proto.h b/src/auth-proto.h index d3c4686f..f39c361c 100644 --- a/src/auth-proto.h +++ b/src/auth-proto.h @@ -25,6 +25,18 @@ struct auth_proto { bool (*start)(struct auth_proto *ap); void (*free)(struct auth_proto *ap); + /* + * Callback to receive an Authenticate frame. auth-protos should + * return error codes consistent with one another as some are treated + * specially: + * + * 0 indicates success + * -ENOMSG or -EBADMSG indicates the message should be ignored silently + * -EAGAIN indicates to try again (handled by auth-proto internally) + * -EPROTO indicates a fatal error + * Any other < 0 return will be treated as a fatal error + * > 0 indicates a fatal error with status code. + */ int (*rx_authenticate)(struct auth_proto *driver, const uint8_t *frame, size_t len); int (*rx_associate)(struct auth_proto *driver, -- 2.31.1

This keeps FILS consistent with what netdev expects for a fatal auth-proto return. --- src/fils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fils.c b/src/fils.c index 7fffefb3..f661f1bf 100644 --- a/src/fils.c +++ b/src/fils.c @@ -289,7 +289,7 @@ static int fils_derive_key_data(struct fils_sm *fils) if (ie_parse_rsne_from_data(fils->hs->supplicant_ie, fils->hs->supplicant_ie[1] + 2, &rsn_info) < 0) - return -EBADMSG; + return -EPROTO; rsn_info.num_pmkids = 1; rsn_info.pmkids = fils->hs->pmk_r1_name; -- 2.31.1

This keeps FT consistent with what netdev expects for a fatal auth-proto return. --- src/ft.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ft.c b/src/ft.c index 13733019..c167f4bc 100644 --- a/src/ft.c +++ b/src/ft.c @@ -520,7 +520,7 @@ static int ft_process_ies(struct handshake_state *hs, const uint8_t *ies, return 0; ft_error: - return -EBADMSG; + return -EPROTO; } int ft_over_ds_parse_action_response(const uint8_t *frame, size_t frame_len, -- 2.31.1

This adds -EBADMSG/-ENOMSG to returns which won't result in a deauth/failed connection. --- src/netdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/netdev.c b/src/netdev.c index 7909c37e..8bbd5abb 100644 --- a/src/netdev.c +++ b/src/netdev.c @@ -2715,7 +2715,8 @@ static void netdev_authenticate_event(struct l_genl_msg *msg, status_code = L_CPU_TO_LE16(auth->status); ret = auth_proto_rx_authenticate(netdev->ap, frame, frame_len); - if (ret == 0 || ret == -EAGAIN) + if (ret == 0 || ret == -EAGAIN || ret == -EBADMSG || + ret == -ENOMSG) return; else if (ret > 0) status_code = (uint16_t)ret; -- 2.31.1

This test simulates the scenario where IWDs commit is not acked which exposes a hostapd bug that ultimately fails the connection. This behavior can be seen by reverting the commit which works around this issue: "sae: don't send commit in confirmed state" With the above patch applied this test should pass. Note: The existing timeout test was reused as it was not of much use anyways. All it did was block auth/assoc frames and expect a failure which didn't exercise any SAE logic anyways. --- autotests/testSAE/timeout_test.py | 41 ++++++++++++++++--------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/autotests/testSAE/timeout_test.py b/autotests/testSAE/timeout_test.py index abef109b..1792f1c0 100644 --- a/autotests/testSAE/timeout_test.py +++ b/autotests/testSAE/timeout_test.py @@ -9,13 +9,12 @@ from iwd import IWD from iwd import PSKAgent from iwd import NetworkType from hwsim import Hwsim +from hostapd import HostapdCLI +from config import ctx class Test(unittest.TestCase): def validate_connection(self, wd): - hwsim = Hwsim() - bss_radio = hwsim.get_radio('rad0') - psk_agent = PSKAgent(["secret123", "secret123"]) wd.register_psk_agent(psk_agent) @@ -30,29 +29,31 @@ class Test(unittest.TestCase): condition = 'not obj.connected' wd.wait_for_object_condition(network.network_object, condition) + network.network_object.connect() + + condition = 'obj.state == DeviceState.connected' + wd.wait_for_object_condition(device, condition) + + def test_not_acked_commit(self): + # + # TODO: This merely forces group 19 by acting as a 'buggy' AP. This is + # needed because the hwsim rule only matches once and must be matched + # on the first commit, not during group negotiation. + # + hostapd = HostapdCLI(config='ssidSAE.conf') + hostapd.set_value('vendor_elements', 'dd0cf4f5e8050500000000000000') + + hwsim = Hwsim() + bss_radio = hwsim.get_radio('rad0') + rule0 = hwsim.rules.create() rule0.source = bss_radio.addresses[0] - rule0.bidirectional = True rule0.drop = True rule0.prefix = 'b0' + rule0.match_times = 1 + rule0.drop_ack = True rule0.enabled = True - wd.wait(1) - print(rule0) - - with self.assertRaises(iwd.FailedEx): - network.network_object.connect() - - condition = 'not obj.connected' - wd.wait_for_object_condition(network.network_object, condition) - - rule0.prefix = '00' - with self.assertRaises(iwd.FailedEx): - network.network_object.connect() - - wd.unregister_psk_agent(psk_agent) - - def test_connection_success(self): wd = IWD(True) self.validate_connection(wd) -- 2.31.1