Controlling wpa_supplicant/hostapd from a text based interface is problematic in that there is no way of knowing if an event corresponds to a request. In certain cases if wpa_s/hostapd is sending out multiple events and we make a request, a random event may come back after the request, but before the actual result. To fix this, at least for this specific case, we can continue to read from the socket until the result is numeric. --- autotests/util/wpas.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/autotests/util/wpas.py b/autotests/util/wpas.py index 0caa947f..2e50eb58 100644 --- a/autotests/util/wpas.py +++ b/autotests/util/wpas.py @@ -242,19 +242,29 @@ class Wpas: self._ctrl_request('SET ' + key + ' ' + value, **kwargs) def dpp_enrollee_start(self, uri=None): + self._rx_data = [] self._ctrl_request('DPP_BOOTSTRAP_GEN type=qrcode') self.wait_for_result() if uri: + self._rx_data = [] self._ctrl_request('DPP_QR_CODE ' + uri) self._dpp_qr_id = self.wait_for_result() + self._rx_data = [] self._ctrl_request('DPP_AUTH_INIT peer=%s role=enrollee' % self._dpp_qr_id) def dpp_configurator_create(self, uri): + self._rx_data = [] self._ctrl_request('DPP_CONFIGURATOR_ADD') self._dpp_conf_id = self.wait_for_result() + while not self._dpp_conf_id.isnumeric(): + self._dpp_conf_id = self.wait_for_result() + + self._rx_data = [] self._ctrl_request('DPP_QR_CODE ' + uri) self._dpp_qr_id = self.wait_for_result() + while not self._dpp_conf_id.isnumeric(): + self._dpp_qr_id = self.wait_for_result() print("DPP Configurator ID: %s. DPP QR ID: %s" % (self._dpp_conf_id, self._dpp_qr_id)) -- 2.31.1

Hi James, On 1/11/22 18:55, James Prestwood wrote: We customarily call this member 'pending', so I changed this naming and applied. Regards, -Denis

The SM has the wdev_id already, so rather than using that or looking up just pass the dpp_sm directly. --- src/dpp.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/dpp.c b/src/dpp.c index e0ae8aa6..c22788e7 100644 --- a/src/dpp.c +++ b/src/dpp.c @@ -213,20 +213,20 @@ static void dpp_send_frame_cb(struct l_genl_msg *msg, void *user_data) l_error("Error sending frame"); } -static void dpp_send_frame(uint64_t wdev_id, struct iovec *iov, size_t iov_len, +static void dpp_send_frame(struct dpp_sm *dpp, struct iovec *iov, size_t iov_len, uint32_t freq) { struct l_genl_msg *msg; msg = l_genl_msg_new_sized(NL80211_CMD_FRAME, 512); - l_genl_msg_append_attr(msg, NL80211_ATTR_WDEV, 8, &wdev_id); + l_genl_msg_append_attr(msg, NL80211_ATTR_WDEV, 8, &dpp->wdev_id); l_genl_msg_append_attr(msg, NL80211_ATTR_WIPHY_FREQ, 4, &freq); l_genl_msg_append_attr(msg, NL80211_ATTR_OFFCHANNEL_TX_OK, 0, NULL); l_genl_msg_append_attrv(msg, NL80211_ATTR_FRAME, iov, iov_len); l_debug("Sending frame on frequency %u", freq); - if (!l_genl_family_send(nl80211, msg, dpp_send_frame_cb, NULL, NULL)) { + if (!l_genl_family_send(nl80211, msg, dpp_send_frame_cb, dpp, NULL)) { l_error("Could not send CMD_FRAME"); l_genl_msg_unref(msg); } @@ -378,7 +378,7 @@ static void dpp_configuration_start(struct dpp_sm *dpp, const uint8_t *addr) dpp->state = DPP_STATE_CONFIGURING; - dpp_send_frame(dpp->wdev_id, iov, 2, dpp->current_freq); + dpp_send_frame(dpp, iov, 2, dpp->current_freq); } static void send_config_result(struct dpp_sm *dpp, const uint8_t *to) @@ -401,7 +401,7 @@ static void send_config_result(struct dpp_sm *dpp, const uint8_t *to) iov[1].iov_base = attrs; iov[1].iov_len = ptr - attrs; - dpp_send_frame(dpp->wdev_id, iov, 2, dpp->current_freq); + dpp_send_frame(dpp, iov, 2, dpp->current_freq); } static void dpp_write_config(struct dpp_configuration *config, @@ -730,7 +730,7 @@ static void dpp_send_config_response(struct dpp_sm *dpp, uint8_t status) iov[1].iov_base = attrs; iov[1].iov_len = ptr - attrs; - dpp_send_frame(dpp->wdev_id, iov, 2, dpp->current_freq); + dpp_send_frame(dpp, iov, 2, dpp->current_freq); } static void dpp_handle_config_request_frame(const struct mmpdu_header *frame, @@ -1033,8 +1033,7 @@ static void send_authenticate_response(struct dpp_sm *dpp) iov[1].iov_base = attrs; iov[1].iov_len = ptr - attrs; - dpp_send_frame(netdev_get_wdev_id(dpp->netdev), iov, 2, - dpp->current_freq); + dpp_send_frame(dpp, iov, 2, dpp->current_freq); } static void authenticate_confirm(struct dpp_sm *dpp, const uint8_t *from, @@ -1200,8 +1199,7 @@ static void dpp_auth_request_failed(struct dpp_sm *dpp, iov[1].iov_base = attrs; iov[1].iov_len = ptr - attrs; - dpp_send_frame(netdev_get_wdev_id(dpp->netdev), iov, 2, - dpp->current_freq); + dpp_send_frame(dpp, iov, 2, dpp->current_freq); } static bool dpp_check_roles(struct dpp_sm *dpp, uint8_t peer_capa) @@ -1241,7 +1239,7 @@ static void dpp_presence_announce(struct dpp_sm *dpp) l_debug("Sending presense annoucement on frequency %u and waiting %u", dpp->current_freq, dpp->dwell); - dpp_send_frame(netdev_get_wdev_id(netdev), iov, 2, dpp->current_freq); + dpp_send_frame(dpp, iov, 2, dpp->current_freq); } static void dpp_roc_started(void *user_data) -- 2.31.1

The DPP spec says nothing about how to handle re-transmits but it was found in testing this can happen relatively easily for a few reasons. If the configurator requests a channel switch but does not get onto the new channel quick enough the enrollee may have already sent the authenticate response and it was missed. Also by nature of how the kernel goes offchannel there are moments in time between ROC when the card is idle and not receiving any frames. Only frames where there was no ACK will be retransmitted. If the peer received the frame and dropped it resending the same frame wont do any good. --- src/dpp.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-) diff --git a/src/dpp.c b/src/dpp.c index c22788e7..a4cc315b 100644 --- a/src/dpp.c +++ b/src/dpp.c @@ -52,6 +52,9 @@ #include "src/scan.h" #include "src/network.h" #include "src/handshake.h" +#include "src/nl80211util.h" + +#define DPP_FRAME_MAX_RETRIES 5 static uint32_t netdev_watch; static struct l_genl_family *nl80211; @@ -118,6 +121,9 @@ struct dpp_sm { struct dpp_configuration *config; uint32_t connect_scan_id; + uint64_t frame_cookie; + uint32_t mlme_watch; + uint8_t frame_retry; struct l_dbus_message *message; }; @@ -174,6 +180,7 @@ static void dpp_reset(struct dpp_sm *dpp) dpp->state = DPP_STATE_NOTHING; dpp->new_freq = 0; + dpp->frame_retry = 0; explicit_bzero(dpp->r_nonce, dpp->nonce_len); explicit_bzero(dpp->i_nonce, dpp->nonce_len); @@ -204,13 +211,26 @@ static void dpp_free(struct dpp_sm *dpp) dpp->boot_private = NULL; } + if (dpp->mlme_watch) { + l_genl_family_unregister(nl80211, dpp->mlme_watch); + dpp->mlme_watch = 0; + } + l_free(dpp); } static void dpp_send_frame_cb(struct l_genl_msg *msg, void *user_data) { - if (l_genl_msg_get_error(msg) < 0) + struct dpp_sm *dpp = user_data; + + if (l_genl_msg_get_error(msg) < 0) { l_error("Error sending frame"); + return; + } + + if (nl80211_parse_attrs(msg, NL80211_ATTR_COOKIE, &dpp->frame_cookie, + NL80211_ATTR_UNSPEC) < 0) + l_error("Error parsing frame cookie"); } static void dpp_send_frame(struct dpp_sm *dpp, struct iovec *iov, size_t iov_len, @@ -1575,6 +1595,76 @@ static void dpp_handle_frame(const struct mmpdu_header *frame, } } +static void dpp_mlme_notify(struct l_genl_msg *msg, void *user_data) +{ + struct dpp_sm *dpp = user_data; + uint64_t wdev_id = 0; + uint64_t cookie = 0; + bool ack = false; + struct l_genl_attr attr; + const void *data; + uint16_t len; + uint16_t type; + const void *frame = NULL; + uint16_t frame_len = 0; + struct iovec iov; + uint8_t cmd = l_genl_msg_get_command(msg); + + if (cmd != NL80211_CMD_FRAME_TX_STATUS) + return; + + if (dpp->state <= DPP_STATE_PRESENCE) + return; + + l_genl_attr_init(&attr, msg); + + while (l_genl_attr_next(&attr, &type, &len, &data)) { + switch (type) { + case NL80211_ATTR_WDEV: + if (len != 8) + return; + + wdev_id = l_get_u64(data); + break; + case NL80211_ATTR_COOKIE: + if (len != 8) + return; + + cookie = l_get_u64(data); + break; + case NL80211_ATTR_ACK: + ack = true; + break; + case NL80211_ATTR_FRAME: + frame = data; + frame_len = len; + break; + } + } + + /* + * Only want to handle the no-ACK case. Re-transmitting an ACKed + * frame likely wont do any good, at least in the case of DPP. + */ + if (dpp->wdev_id != wdev_id || dpp->frame_cookie != cookie || + ack || !frame) + return; + + if (dpp->frame_retry > DPP_FRAME_MAX_RETRIES) { + dpp_reset(dpp); + return; + } + + iov.iov_base = (void *)frame; + iov.iov_len = frame_len; + + l_debug("No ACK from peer, re-transmitting"); + + dpp->frame_retry++; + + dpp_send_frame(dpp, &iov, 1, dpp->current_freq); +} + static void dpp_create(struct netdev *netdev) { struct l_dbus *dbus = dbus_get_bus(); @@ -1612,6 +1702,9 @@ static void dpp_create(struct netdev *netdev) dpp_conf_request_prefix, sizeof(dpp_conf_request_prefix), dpp_handle_config_request_frame, dpp, NULL); + + dpp->mlme_watch = l_genl_family_register(nl80211, "mlme", + dpp_mlme_notify, dpp, NULL); } static void dpp_netdev_watch(struct netdev *netdev, -- 2.31.1

On Wed, 2022-01-12 at 10:04 -0600, Denis Kenzior wrote: Yep, that was why. But yes, I hadn't thought about each instance having to parse out the wdev, I can just add dpp_find instead. Only would matter if cookies are reused or counted per-wdev but it doesn't hurt to zero it out. Yeah I'll do this instead. I played around with passing data/length but I always forget about iovec's.

This adds 3 more tests: - Channel switch during authentication - Scan after configuring - Retransmit auth frames --- autotests/testDPP/connection_test.py | 103 +++++++++++++++++++-------- autotests/testDPP/hw.conf | 1 + 2 files changed, 76 insertions(+), 28 deletions(-) diff --git a/autotests/testDPP/connection_test.py b/autotests/testDPP/connection_test.py index 43822c55..3c3705e4 100644 --- a/autotests/testDPP/connection_test.py +++ b/autotests/testDPP/connection_test.py @@ -7,58 +7,105 @@ sys.path.append('../util') from iwd import IWD from wpas import Wpas from hostapd import HostapdCLI +from hwsim import Hwsim class Test(unittest.TestCase): - def test_iwd_as_enrollee(self): - wpas = Wpas('wpas.conf') + self.device.autoconnect = True + self.hapd.reload() + + uri = self.device.dpp_start_enrollee() - wd = IWD(True) + self.wpas.dpp_configurator_create(uri) + self.wpas.dpp_configurator_start('ssidCCMP', 'secret123') - devices = wd.list_devices(1) - device = devices[0] - device.autoconnect = True + condition = 'obj.state == DeviceState.connected' + self.wd.wait_for_object_condition(self.device, condition) - uri = device.dpp_start_enrollee() + def test_iwd_as_enrollee_channel_switch(self): + self.device.autoconnect = True + self.hapd.reload() - wpas.dpp_configurator_create(uri) - wpas.dpp_configurator_start('ssidCCMP', 'secret123') + uri = self.device.dpp_start_enrollee() + + self.wpas.dpp_configurator_create(uri) + self.wpas.dpp_configurator_start('ssidCCMP', 'secret123', freq=2462) condition = 'obj.state == DeviceState.connected' - wd.wait_for_object_condition(device, condition) + self.wd.wait_for_object_condition(self.device, condition) - device.disconnect() - device.dpp_stop() + def test_iwd_as_enrollee_scan_after(self): + uri = self.device.dpp_start_enrollee() - del wpas + self.wpas.dpp_configurator_create(uri) + self.wpas.dpp_configurator_start('ssidCCMP', 'secret123') - def test_iwd_as_configurator(self): - IWD.copy_to_storage('ssidCCMP.psk') + self.hapd.reload() + + with self.assertRaises(Exception): + self.device.get_ordered_network('ssidCCMP', scan_if_needed=False) - wpas = Wpas('wpas.conf') - hapd = HostapdCLI('hostapd.conf') + self.hapd.wait_for_event('AP-ENABLED') - wd = IWD(True) + self.device.autoconnect = True + + condition = 'obj.state == DeviceState.connected' + self.wd.wait_for_object_condition(self.device, condition) + + def test_iwd_as_enrollee_no_ack(self): + self.rule0.enabled = True + self.device.autoconnect = True + self.hapd.reload() + + uri = self.device.dpp_start_enrollee() + + self.wpas.dpp_configurator_create(uri) + self.wpas.dpp_configurator_start('ssidCCMP', 'secret123') + + condition = 'obj.state == DeviceState.connected' + self.wd.wait_for_object_condition(self.device, condition) + + def test_iwd_as_configurator(self): + self.device.autoconnect = True + IWD.copy_to_storage('ssidCCMP.psk') - devices = wd.list_devices(1) - device = devices[0] - device.autoconnect = True + self.hapd.reload() condition = 'obj.state == DeviceState.connected' - wd.wait_for_object_condition(device, condition) + self.wd.wait_for_object_condition(self.device, condition) - uri = device.dpp_start_configurator() + uri = self.device.dpp_start_configurator() - wpas.dpp_enrollee_start(uri) + self.wpas.dpp_enrollee_start(uri) - hapd.wait_for_event('AP-STA-CONNECTED 42:00:00:00:00:00') + self.hapd.wait_for_event('AP-STA-CONNECTED 42:00:00:00:00:00') - device.disconnect() - device.dpp_stop() + def setUp(self): + self.wd = IWD(True) + self.device = self.wd.list_devices(1)[0] + self.wpas = Wpas('wpas.conf') + self.hapd = HostapdCLI('hostapd.conf') + self.hapd.disable() + self.hwsim = Hwsim() - del wpas + self.rule0 = self.hwsim.rules.create() + self.rule0.prefix = 'd0' + self.rule0.match_offset = 24 + self.rule0.match = '04 09 50 6f 9a 1a 01 01' + self.rule0.match_times = 1 + self.rule0.drop = True def tearDown(self): + print("calling Disconnect()") + self.device.disconnect() + self.device.dpp_stop() + self.wpas.dpp_configurator_remove() + + self.wd = None + self.device = None + self.wpas = None + self.hapd = None + self.rule0 = None IWD.clear_storage() @classmethod diff --git a/autotests/testDPP/hw.conf b/autotests/testDPP/hw.conf index 478f0633..923595ea 100644 --- a/autotests/testDPP/hw.conf +++ b/autotests/testDPP/hw.conf @@ -1,6 +1,7 @@ [SETUP] num_radios=3 start_iwd=0 +hwsim_medium=yes [WPA_SUPPLICANT] rad0=wpas.conf -- 2.31.1