[PATCH 1/5] sae: remove authenticate timeout handler

[PATCH 1/5] sae: remove authenticate timeout handler

[James Prestwood]

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

This fixes an infinite loop issue when authenticate frames time
out. If the AP is not responding IWD ends up retrying indefinitely
due to how SAE was handling this timeout. Inside sae_auth_timeout
it was actually sending another authenticate frame to reject
the SAE handshake. This, again, resulted in a timeout which called
the SAE timeout handler and repeated indefinitely.

The kernel resend behavior was not taken into account when writing
the SAE timeout behavior and in practice there is actually no need
for SAE to do much of anything in response to a timeout. The
kernel automatically resends Authenticate frames 3 times which mirrors
IWDs SAE behavior anyways. Because of this the authenticate timeout
handler can be completely removed, which will cause the connection
to fail in the case of an autentication timeout.
---
 src/sae.c | 30 ------------------------------
 1 file changed, 30 deletions(-)

diff --git a/src/sae.c b/src/sae.c
index b6cc0b15..98050483 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -672,35 +672,6 @@ static bool sae_send_commit(struct sae_sm *sm, bool retry)
 	return true;
 }
 
-static bool sae_auth_timeout(struct auth_proto *ap)
-{
-	struct sae_sm *sm = l_container_of(ap, struct sae_sm, ap);
-
-	/* regardless of state, reject if sync exceeds max */
-	if (sm->sync > SAE_SYNC_MAX) {
-		sae_reject_authentication(sm, MMPDU_REASON_CODE_UNSPECIFIED);
-		return false;
-	}
-
-	sm->sync++;
-
-	switch (sm->state) {
-	case SAE_STATE_COMMITTED:
-		sae_send_commit(sm, true);
-		break;
-	case SAE_STATE_CONFIRMED:
-		sm->sc++;
-		sae_send_confirm(sm);
-		break;
-	default:
-		/* should never happen */
-		l_error("SAE timeout in bad state %u", sm->state);
-		return false;
-	}
-
-	return true;
-}
-
 static bool sae_assoc_timeout(struct auth_proto *ap)
 {
 	struct sae_sm *sm = l_container_of(ap, struct sae_sm, ap);
@@ -1194,7 +1165,6 @@ struct auth_proto *sae_sm_new(struct handshake_state *hs,
 	sm->ap.free = sae_free;
 	sm->ap.rx_authenticate = sae_rx_authenticate;
 	sm->ap.rx_associate = sae_rx_associate;
-	sm->ap.auth_timeout = sae_auth_timeout;
 	sm->ap.assoc_timeout = sae_assoc_timeout;
 
 	return &sm->ap;
-- 
2.26.2

[PATCH 2/5] hwsim: add Prefix match rule support

[James Prestwood]

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

Hwsim rules now have a 'Prefix' property which will allow
matching frames based on their payload data.
---
 tools/hwsim.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/tools/hwsim.c b/tools/hwsim.c
index 08198b56..fcb83248 100644
--- a/tools/hwsim.c
+++ b/tools/hwsim.c
@@ -114,6 +114,7 @@ enum hwsim_tx_control_flags {
 
 #define IEEE80211_TX_RATE_TABLE_SIZE	4
 #define HWSIM_DELAY_MIN_MS		1
+#define HWSIM_MAX_PREFIX_LEN		128
 
 struct hwsim_rule {
 	unsigned int id;
@@ -127,6 +128,8 @@ struct hwsim_rule {
 	int priority;
 	int signal;
 	int delay;
+	uint8_t *prefix;
+	size_t prefix_len;
 };
 
 struct hwsim_support {
@@ -1203,6 +1206,12 @@ static void process_rules(const struct radio_info_rec *src_radio,
 		if (rule->frequency && rule->frequency != frame->frequency)
 			continue;
 
+		if (rule->prefix && frame->payload_len >= rule->prefix_len) {
+			if (memcmp(rule->prefix, frame->payload,
+					rule->prefix_len) != 0)
+				continue;
+		}
+
 		/* Rule deemed to match frame, apply any changes */
 
 		if (rule->signal)
@@ -2025,6 +2034,10 @@ static struct l_dbus_message *rule_remove(struct l_dbus *dbus,
 
 	path = rule_get_path(rule);
 	l_queue_remove(rules, rule);
+
+	if (rule->prefix)
+		l_free(rule->prefix);
+
 	l_free(rule);
 	l_dbus_unregister_object(dbus, path);
 
@@ -2303,6 +2316,59 @@ static struct l_dbus_message *rule_property_set_delay(
 	return l_dbus_message_new_method_return(message);
 }
 
+static bool rule_property_get_prefix(struct l_dbus *dbus,
+					struct l_dbus_message *message,
+					struct l_dbus_message_builder *builder,
+					void *user_data)
+{
+	struct hwsim_rule *rule = user_data;
+	size_t i;
+
+	l_dbus_message_builder_enter_array(builder, "y");
+
+	for (i = 0; i < rule->prefix_len; i++)
+		l_dbus_message_builder_append_basic(builder, 'y',
+							rule->prefix + i);
+
+	l_dbus_message_builder_leave_array(builder);
+
+	return true;
+}
+
+static struct l_dbus_message *rule_property_set_prefix(
+					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;
+	struct l_dbus_message_iter iter;
+	const uint8_t *prefix;
+	uint32_t len;
+
+	if (!l_dbus_message_iter_get_variant(new_value, "ay", &iter))
+		goto invalid_args;
+
+	if (!l_dbus_message_iter_get_fixed_array(&iter,
+						(const void **)&prefix, &len))
+		goto invalid_args;
+
+	if (len > HWSIM_MAX_PREFIX_LEN)
+		goto invalid_args;
+
+	if (rule->prefix)
+		l_free(rule->prefix);
+
+	rule->prefix = l_memdup(prefix, len);
+	rule->prefix_len = len;
+
+	return l_dbus_message_new_method_return(message);
+
+invalid_args:
+	return dbus_error_invalid_args(message);
+}
+
 static void setup_rule_interface(struct l_dbus_interface *interface)
 {
 	l_dbus_interface_method(interface, "Remove", 0, rule_remove, "", "");
@@ -2339,6 +2405,10 @@ static void setup_rule_interface(struct l_dbus_interface *interface)
 					L_DBUS_PROPERTY_FLAG_AUTO_EMIT, "u",
 					rule_property_get_delay,
 					rule_property_set_delay);
+	l_dbus_interface_property(interface, "Prefix",
+					L_DBUS_PROPERTY_FLAG_AUTO_EMIT, "ay",
+					rule_property_get_prefix,
+					rule_property_set_prefix);
 }
 
 static void request_name_callback(struct l_dbus *dbus, bool success,
-- 
2.26.2

[PATCH 3/5] auto-t: add python wrapper for hwsim rule prefix

[James Prestwood]

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

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

diff --git a/autotests/util/hwsim.py b/autotests/util/hwsim.py
index ad4bec1f..320e9dfa 100755
--- a/autotests/util/hwsim.py
+++ b/autotests/util/hwsim.py
@@ -132,6 +132,16 @@ class Rule(HwsimDBusAbstract):
                             reply_handler=self._success, error_handler=self._failure)
         self._wait_for_async_op()
 
+    @property
+    def prefix(self):
+        return self._properties['Prefix']
+
+    @prefix.setter
+    def prefix(self, value):
+        self._prop_proxy.Set(self._iface_name, 'Prefix', dbus.ByteArray.fromhex(value),
+                            reply_handler=self._success, error_handler=self._failure)
+        self._wait_for_async_op()
+
     def remove(self):
         self._iface.Remove(reply_handler=self._success,
                 error_handler=self._failure)
@@ -150,7 +160,8 @@ class Rule(HwsimDBusAbstract):
                prefix + '\tPriority:\t' + str(self.priority) + '\n' +\
                prefix + '\tFrequency:\t' + str(self.frequency) + '\n' + \
                prefix + '\tApply rssi:\t' + str(self.signal) + '\n' + \
-               prefix + '\tApply drop:\t' + str(self.drop) + '\n'
+               prefix + '\tApply drop:\t' + str(self.drop) + '\n' + \
+               prefix + '\tPrefix:\t' + str([hex(b) for b in self.prefix]) + '\n'
 
 class RuleSet(collections.Mapping):
     def __init__(self, hwsim, objects):
-- 
2.26.2

[PATCH 4/5] auto-t: add timeout test to SAE

[James Prestwood]

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

This tests the timeout handling in SAE for both authenticate
and associate frames.
---
 autotests/testSAE/hw.conf         |  1 -
 autotests/testSAE/timeout_test.py | 83 +++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 autotests/testSAE/timeout_test.py

diff --git a/autotests/testSAE/hw.conf b/autotests/testSAE/hw.conf
index 86f42630..15bc3a55 100644
--- a/autotests/testSAE/hw.conf
+++ b/autotests/testSAE/hw.conf
@@ -1,7 +1,6 @@
 [SETUP]
 num_radios=7
 start_iwd=0
-hwsim_medium=no
 
 [HOSTAPD]
 rad0=ssidSAE.conf
diff --git a/autotests/testSAE/timeout_test.py b/autotests/testSAE/timeout_test.py
new file mode 100644
index 00000000..70462cec
--- /dev/null
+++ b/autotests/testSAE/timeout_test.py
@@ -0,0 +1,83 @@
+#!/usr/bin/python3
+
+import unittest
+import sys
+
+sys.path.append('../util')
+import iwd
+from iwd import IWD
+from iwd import PSKAgent
+from iwd import NetworkType
+from hwsim import Hwsim
+
+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)
+
+        devices = wd.list_devices(4)
+
+        # These devices aren't used in this test, this makes logs a bit nicer
+        # since these devices would presumably start autoconnecting.
+        devices[1].disconnect()
+        devices[2].disconnect()
+        devices[3].disconnect()
+
+        self.assertIsNotNone(devices)
+        device = devices[0]
+
+        condition = 'not obj.scanning'
+        wd.wait_for_object_condition(device, condition)
+
+        device.scan()
+
+        condition = 'not obj.scanning'
+        wd.wait_for_object_condition(device, condition)
+
+        network = device.get_ordered_network('ssidSAE')
+
+        self.assertEqual(network.type, NetworkType.psk)
+
+        condition = 'not obj.connected'
+        wd.wait_for_object_condition(network.network_object, condition)
+
+        rule0 = hwsim.rules.create()
+        rule0.source = bss_radio.addresses[0]
+        rule0.bidirectional = True
+        rule0.drop = True
+        rule0.prefix = 'b0'
+
+        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)
+
+    @classmethod
+    def setUpClass(cls):
+        pass
+
+    @classmethod
+    def tearDownClass(cls):
+        IWD.clear_storage()
+
+if __name__ == '__main__':
+    unittest.main(exit=True)
-- 
2.26.2

[PATCH 5/5] auto-t: add auth/assoc timeouts to testOWE

[James Prestwood]

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

---
 autotests/testOWE/timeout_test.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/autotests/testOWE/timeout_test.py b/autotests/testOWE/timeout_test.py
index d02432e9..39c2d429 100644
--- a/autotests/testOWE/timeout_test.py
+++ b/autotests/testOWE/timeout_test.py
@@ -45,6 +45,14 @@ class Test(unittest.TestCase):
         rule0.source = bss_radio.addresses[0]
         rule0.bidirectional = True
         rule0.drop = True
+        rule0.prefix = 'b0'
+
+        # Test Authenticate (b0) and Association (00) timeouts
+
+        with self.assertRaises(iwd.FailedEx):
+            ordered_network.network_object.connect()
+
+        rule0.prefix = '00'
 
         with self.assertRaises(iwd.FailedEx):
             ordered_network.network_object.connect()
-- 
2.26.2

Re: [PATCH 1/5] sae: remove authenticate timeout handler

[Denis Kenzior]

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

Hi James,

On 4/5/21 5:40 PM, James Prestwood wrote:
> This fixes an infinite loop issue when authenticate frames time
> out. If the AP is not responding IWD ends up retrying indefinitely
> due to how SAE was handling this timeout. Inside sae_auth_timeout
> it was actually sending another authenticate frame to reject
> the SAE handshake. This, again, resulted in a timeout which called
> the SAE timeout handler and repeated indefinitely.
> 
> The kernel resend behavior was not taken into account when writing
> the SAE timeout behavior and in practice there is actually no need
> for SAE to do much of anything in response to a timeout. The
> kernel automatically resends Authenticate frames 3 times which mirrors
> IWDs SAE behavior anyways. Because of this the authenticate timeout
> handler can be completely removed, which will cause the connection
> to fail in the case of an autentication timeout.
> ---
>   src/sae.c | 30 ------------------------------
>   1 file changed, 30 deletions(-)
> 

All applied, thanks.

Regards,
-Denis