[PATCH 1/9] hwsim: add MatchTimes property

[PATCH 1/9] hwsim: add MatchTimes property

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 3045 bytes --]

This integer property can be set to only match a rule a number of
times rather than all packets. This is useful for testing behavior
of a single dropped frame or ack. Once the rule has been matched
'MatchTimes' the rules will no longer be applied (unless set again
to some integer greater than zero).
---
 tools/hwsim.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tools/hwsim.c b/tools/hwsim.c
index cd4a99b7..18ba111a 100644
--- a/tools/hwsim.c
+++ b/tools/hwsim.c
@@ -131,6 +131,7 @@ struct hwsim_rule {
 	int delay;
 	uint8_t *prefix;
 	size_t prefix_len;
+	int match_times; /* negative value indicates unused */
 };
 
 struct hwsim_support {
@@ -1217,6 +1218,8 @@ static void process_rules(const struct radio_info_rec *src_radio,
 		}
 
 		/* Rule deemed to match frame, apply any changes */
+		if (rule->match_times == 0)
+			continue;
 
 		if (rule->signal)
 			frame->signal = rule->signal / 100;
@@ -1225,6 +1228,9 @@ static void process_rules(const struct radio_info_rec *src_radio,
 
 		if (delay)
 			*delay = rule->delay;
+
+		if (rule->match_times > 0)
+			rule->match_times--;
 	}
 }
 
@@ -2008,6 +2014,7 @@ static struct l_dbus_message *rule_add(struct l_dbus *dbus,
 	rule->destination_any = true;
 	rule->delay = 0;
 	rule->enabled = false;
+	rule->match_times = -1;
 
 	if (!rules)
 		rules = l_queue_new();
@@ -2412,6 +2419,37 @@ static struct l_dbus_message *rule_property_set_enabled(
 	return l_dbus_message_new_method_return(message);
 }
 
+static bool rule_property_get_match_times(struct l_dbus *dbus,
+					struct l_dbus_message *message,
+					struct l_dbus_message_builder *builder,
+					void *user_data)
+{
+	struct hwsim_rule *rule = user_data;
+	uint16_t val = rule->match_times;
+
+	l_dbus_message_builder_append_basic(builder, 'q', &val);
+
+	return true;
+}
+
+static struct l_dbus_message *rule_property_set_match_times(
+					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;
+	uint16_t val;
+
+	if (!l_dbus_message_iter_get_variant(new_value, "q", &val))
+		return dbus_error_invalid_args(message);
+
+	rule->match_times = val;
+
+	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, "", "");
@@ -2456,6 +2494,10 @@ static void setup_rule_interface(struct l_dbus_interface *interface)
 					L_DBUS_PROPERTY_FLAG_AUTO_EMIT, "b",
 					rule_property_get_enabled,
 					rule_property_set_enabled);
+	l_dbus_interface_property(interface, "MatchTimes",
+					L_DBUS_PROPERTY_FLAG_AUTO_EMIT, "q",
+					rule_property_get_match_times,
+					rule_property_set_match_times);
 }
 
 static void request_name_callback(struct l_dbus *dbus, bool success,
-- 
2.31.1

[PATCH 2/9] auto-t: hwsim.py: add match_times property

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 805 bytes --]

---
 autotests/util/hwsim.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/autotests/util/hwsim.py b/autotests/util/hwsim.py
index cc62495c..496c06a0 100755
--- a/autotests/util/hwsim.py
+++ b/autotests/util/hwsim.py
@@ -153,6 +153,14 @@ class Rule(HwsimDBusAbstract):
                             reply_handler=self._success, error_handler=self._failure)
         self._wait_for_async_op()
 
+    @property
+    def match_times(self):
+        return self._properties['MatchTimes']
+
+    @match_times.setter
+    def match_times(self, value):
+        self._prop_proxy.Set(self._iface_name, 'MatchTimes', dbus.UInt16(value))
+
     def remove(self):
         self._iface.Remove(reply_handler=self._success,
                 error_handler=self._failure)
-- 
2.31.1

[PATCH 3/9] hwsim: add DropAck rule property

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 4627 bytes --]

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

[PATCH 4/9] auto-t: hwsim.py: add drop_ack property

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

---
 autotests/util/hwsim.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/autotests/util/hwsim.py b/autotests/util/hwsim.py
index 496c06a0..8ba9550a 100755
--- a/autotests/util/hwsim.py
+++ b/autotests/util/hwsim.py
@@ -161,6 +161,14 @@ class Rule(HwsimDBusAbstract):
     def match_times(self, value):
         self._prop_proxy.Set(self._iface_name, 'MatchTimes', dbus.UInt16(value))
 
+    @property
+    def drop_ack(self):
+        return self._properties(['DropAck'])
+
+    @drop_ack.setter
+    def drop_ack(self, value):
+        self._prop_proxy.Set(self._iface_name, 'DropAck', value)
+
     def remove(self):
         self._iface.Remove(reply_handler=self._success,
                 error_handler=self._failure)
-- 
2.31.1

[PATCH 5/9] netdev: print error if CMD_ASSOCIATE fails

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

---
 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

[PATCH 6/9] sae: print state and transaction on received packets

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]

This will make SAE a bit easier to debug in the future.
---
 src/sae.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/sae.c b/src/sae.c
index 6b282e8e..62fd6c88 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -1226,10 +1226,28 @@ static int sae_verify_accepted(struct sae_sm *sm, uint16_t trans,
 	return -EAGAIN;
 }
 
+static const char *sae_state_to_str(enum sae_state state)
+{
+	switch (state) {
+	case SAE_STATE_NOTHING:
+		return "nothing";
+	case SAE_STATE_COMMITTED:
+		return "committed";
+	case SAE_STATE_CONFIRMED:
+		return "confirmed";
+	case SAE_STATE_ACCEPTED:
+		return "accepted";
+	}
+
+	return "unknown";
+}
+
 static int sae_verify_packet(struct sae_sm *sm, uint16_t trans,
 				uint16_t status, const uint8_t *frame,
 				size_t len)
 {
+	l_debug("rx trans=%u, state=%s", trans, sae_state_to_str(sm->state));
+
 	if (trans != SAE_STATE_COMMITTED && trans != SAE_STATE_CONFIRMED)
 		return -EBADMSG;
 
-- 
2.31.1

[PATCH 7/9] sae: fix a few spec violations regarding dropped frames

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

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

Re: [PATCH 7/9] sae: fix a few spec violations regarding dropped frames

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

Hi James,

On 9/7/21 4:14 PM, James Prestwood wrote:
> 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;

Hmm, this one seems like it should remain unchanged?

>   	}
>   
>   	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;

I guess you probably just want 'return 0' here and a quote from 12.4.8.6.1?

>   	}
>   
>   	if (sm->sync > SAE_SYNC_MAX)
> 

Regards,
-Denis

[PATCH 8/9] sae: don't send commit in confirmed state

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

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

Re: [PATCH 8/9] sae: don't send commit in confirmed state

[Paul Menzel]

[-- Attachment #1: Type: text/plain, Size: 1652 bytes --]

Dear James,


Am 07.09.21 um 23:14 schrieb James Prestwood:
> This works around a hostapd bug (described more in the TODO comment)

It’d be great, if you listed the hostapd version, you reproduced this 
with, and, if you did, add a reference to the hostapd bug report.

> 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.

Excuse my ignorance, but how likely is it, that non-hostapd setups keep 
working?


Kind regards,

Paul


> ---
>   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;
> 

Re: [PATCH 8/9] sae: don't send commit in confirmed state

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1105 bytes --]

Hi Paul,

On Wed, 2021-09-08 at 01:02 +0200, Paul Menzel wrote:
> Dear James,
> 
> 
> Am 07.09.21 um 23:14 schrieb James Prestwood:
> > This works around a hostapd bug (described more in the TODO
> > comment)
> 
> It’d be great, if you listed the hostapd version, you reproduced this
> with, and, if you did, add a reference to the hostapd bug report.

Sure. I did send an email to the hostapd ML if that is considered a bug
report.

> 
> > 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.
> 
> Excuse my ignorance, but how likely is it, that non-hostapd setups
> keep 
> working?

Do non-hostapd setups even exist? :) But in reality no it shouldn't
effect anything. I do need to amend this commit slightly, but v2 will
have identical behavior as wpa_supplicant, i.e. ignore the commit
message completely.

Thanks,
James

Re: [PATCH 8/9] sae: don't send commit in confirmed state

[Paul Menzel]

[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]

Dear James,


Am 08.09.21 um 01:34 schrieb James Prestwood:

> On Wed, 2021-09-08 at 01:02 +0200, Paul Menzel wrote:

>> Am 07.09.21 um 23:14 schrieb James Prestwood:
>>> This works around a hostapd bug (described more in the TODO
>>> comment)
>>
>> It’d be great, if you listed the hostapd version, you reproduced this
>> with, and, if you did, add a reference to the hostapd bug report.
> 
> Sure. I did send an email to the hostapd ML if that is considered a bug
> report.

Thank you for doing that. In my book it is. Please add the URL to the 
commit message.

https://lists.infradead.org/pipermail/hostap/2021-September/039842.html

>>> 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.
>>
>> Excuse my ignorance, but how likely is it, that non-hostapd setups
>> keep working?
> 
> Do non-hostapd setups even exist? :) But in reality no it shouldn't
> effect anything. I do need to amend this commit slightly, but v2 will
> have identical behavior as wpa_supplicant, i.e. ignore the commit
> message completely.

Great, it’d be great if you mentioned that.

Also, from the other thread *Re: Cannot connect to SAE protected AP with 
iwd 1.16 and beyond*, if the ell commit really introduced the 
regression, it’d be great if you mentioned that too.


Kind regards,

Paul

Re: [PATCH 8/9] sae: don't send commit in confirmed state

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1298 bytes --]

Hi Paul,

> > 
> > Do non-hostapd setups even exist? :) But in reality no it shouldn't
> > effect anything. I do need to amend this commit slightly, but v2
> > will
> > have identical behavior as wpa_supplicant, i.e. ignore the commit
> > message completely.
> 
> Great, it’d be great if you mentioned that.
> 
> Also, from the other thread *Re: Cannot connect to SAE protected AP
> with 
> iwd 1.16 and beyond*, if the ell commit really introduced the 
> regression, it’d be great if you mentioned that too.

So we actually had two separate problems here:

First was some APs don't properly do SAE group negotiation. The ELL
change, while not a bug, changed the default group to 20 which 802.11
does not mandate support for (but it is more secure). In this case the
SAE protocol negotiates the group but as mentioned these APs end up
failing the connection. This was worked around in an earlier commit by
detecting these problem APs and forcing group 19 from the beginning.

The second problem was one addressed in this patche where the AP
receives an unexpected commit message and fails the connection rather
than dropping the message.

In any case, I will add a bit more context to the commit message.

Thanks,
James

> 
> 
> Kind regards,
> 
> Paul

[PATCH 9/9] auto-t: add sae test for non-acked commit

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 2951 bytes --]

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

Re: [PATCH 1/9] hwsim: add MatchTimes property

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 540 bytes --]

Hi James,

On 9/7/21 4:14 PM, James Prestwood wrote:
> This integer property can be set to only match a rule a number of
> times rather than all packets. This is useful for testing behavior
> of a single dropped frame or ack. Once the rule has been matched
> 'MatchTimes' the rules will no longer be applied (unless set again
> to some integer greater than zero).
> ---
>   tools/hwsim.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 42 insertions(+)
> 

Patches 1-6 applied, thanks.

Regards,
-Denis