The hwsim rules did not treat frames and ACKs any differently which can mislead the developer especially when setting a rule prefix. If a prefix was used the frame ACK was actually being matched against the original frame payload which seems wrong because the ACK is not the original frame. Though strange, matching the frame prefix on an ACK has its place if the developer wants to block just the ACK rather than the frame so to make this case more clear 'DropAck' was added as a rule property. And only if this is true will an ACK be checked and potentially dropped. To maintain the current hwsim behavior DropAck will default to true. --- tools/hwsim.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/tools/hwsim.c b/tools/hwsim.c index 18ba111a..8fb9b0a4 100644 --- a/tools/hwsim.c +++ b/tools/hwsim.c @@ -124,6 +124,7 @@ struct hwsim_rule { bool destination_any : 1; bool bidirectional : 1; bool drop : 1; + bool drop_ack : 1; bool enabled : 1; uint32_t frequency; int priority; @@ -1170,7 +1171,7 @@ static bool radio_match_addr(const struct radio_info_rec *radio, static void process_rules(const struct radio_info_rec *src_radio, const struct radio_info_rec *dst_radio, - struct hwsim_frame *frame, bool *drop, + struct hwsim_frame *frame, bool ack, bool *drop, uint32_t *delay) { const struct l_queue_entry *rule_entry; @@ -1224,7 +1225,9 @@ static void process_rules(const struct radio_info_rec *src_radio, if (rule->signal) frame->signal = rule->signal / 100; - *drop = rule->drop; + /* Don't drop if this is an ACK, unless drop_ack is set */ + if (!ack || (ack && rule->drop_ack)) + *drop = rule->drop; if (delay) *delay = rule->delay; @@ -1314,7 +1317,7 @@ static void hwsim_frame_unref(struct hwsim_frame *frame) bool drop = false; process_rules(frame->ack_radio, frame->src_radio, - frame, &drop, NULL); + frame, true, &drop, NULL); if (!drop) frame->flags |= HWSIM_TX_STAT_ACK; @@ -1458,7 +1461,8 @@ static void process_frame(struct hwsim_frame *frame) bool drop_mcast = false; if (util_is_broadcast_address(frame->dst_ether_addr)) - process_rules(frame->src_radio, NULL, frame, &drop_mcast, NULL); + process_rules(frame->src_radio, NULL, frame, false, + &drop_mcast, NULL); for (entry = l_queue_get_entries(radio_info); entry; entry = entry->next) { @@ -1498,7 +1502,8 @@ static void process_frame(struct hwsim_frame *frame) continue; } - process_rules(frame->src_radio, radio, frame, &drop, &delay); + process_rules(frame->src_radio, radio, frame, false, + &drop, &delay); if (drop) continue; @@ -2015,6 +2020,7 @@ static struct l_dbus_message *rule_add(struct l_dbus *dbus, rule->delay = 0; rule->enabled = false; rule->match_times = -1; + rule->drop_ack = true; if (!rules) rules = l_queue_new(); @@ -2450,6 +2456,37 @@ static struct l_dbus_message *rule_property_set_match_times( return l_dbus_message_new_method_return(message); } +static bool rule_property_get_drop_ack(struct l_dbus *dbus, + struct l_dbus_message *message, + struct l_dbus_message_builder *builder, + void *user_data) +{ + struct hwsim_rule *rule = user_data; + bool bval = rule->drop_ack; + + l_dbus_message_builder_append_basic(builder, 'b', &bval); + + return true; +} + +static struct l_dbus_message *rule_property_set_drop_ack( + struct l_dbus *dbus, + struct l_dbus_message *message, + struct l_dbus_message_iter *new_value, + l_dbus_property_complete_cb_t complete, + void *user_data) +{ + struct hwsim_rule *rule = user_data; + bool bval; + + if (!l_dbus_message_iter_get_variant(new_value, "b", &bval)) + return dbus_error_invalid_args(message); + + rule->drop_ack = bval; + + return l_dbus_message_new_method_return(message); +} + static void setup_rule_interface(struct l_dbus_interface *interface) { l_dbus_interface_method(interface, "Remove", 0, rule_remove, "", ""); @@ -2498,6 +2535,10 @@ static void setup_rule_interface(struct l_dbus_interface *interface) L_DBUS_PROPERTY_FLAG_AUTO_EMIT, "q", rule_property_get_match_times, rule_property_set_match_times); + l_dbus_interface_property(interface, "DropAck", + L_DBUS_PROPERTY_FLAG_AUTO_EMIT, "b", + rule_property_get_drop_ack, + rule_property_set_drop_ack); } static void request_name_callback(struct l_dbus *dbus, bool success, -- 2.31.1

--- src/netdev.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/netdev.c b/src/netdev.c index a5d1b8ed..b356cb01 100644 --- a/src/netdev.c +++ b/src/netdev.c @@ -2712,9 +2712,10 @@ static void netdev_new_scan_results_event(struct l_genl_msg *msg, static void netdev_assoc_cb(struct l_genl_msg *msg, void *user_data) { struct netdev *netdev = user_data; + int err = l_genl_msg_get_error(msg); - if (l_genl_msg_get_error(msg) < 0) { - l_error("Error sending CMD_ASSOCIATE"); + if (err < 0) { + l_error("Error sending CMD_ASSOCIATE (%d)", err); netdev_connect_failed(netdev, NETDEV_RESULT_ASSOCIATION_FAILED, MMPDU_STATUS_CODE_UNSPECIFIED); -- 2.31.1

There were two places where SAE returned a fatal error code rather than silently drop the frame as the spec requires. These two cases return -EAGAIN which ultimately drops the frame in netdev. --- src/sae.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sae.c b/src/sae.c index 62fd6c88..fbd0298d 100644 --- a/src/sae.c +++ b/src/sae.c @@ -678,7 +678,7 @@ static int sae_process_commit(struct sae_sm *sm, const uint8_t *from, if (l_ecc_scalars_are_equal(sm->p_scalar, sm->scalar) || l_ecc_points_are_equal(sm->p_element, sm->element)) { l_warn("peer scalar or element matched own, discarding frame"); - return -ENOMSG; + return -EAGAIN; } sm->sc++; @@ -1185,7 +1185,7 @@ static int sae_verify_accepted(struct sae_sm *sm, uint16_t trans, /* spec does not specify what to do here, so print and discard */ if (trans != SAE_STATE_CONFIRMED) { l_error("received transaction %u in accepted state", trans); - return -EBADMSG; + return -EAGAIN; } if (sm->sync > SAE_SYNC_MAX) -- 2.31.1

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. --- src/sae.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/sae.c b/src/sae.c index fbd0298d..4eda9225 100644 --- a/src/sae.c +++ b/src/sae.c @@ -1165,7 +1165,18 @@ static int sae_verify_confirmed(struct sae_sm *sm, uint16_t trans, sm->sync++; sm->sc++; - sae_send_commit(sm, true); + /* + * 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 + * only send a confirm here. + * + * sae_send_commit(sm, true); + */ if (!sae_send_confirm(sm)) return -EPROTO; -- 2.31.1