[PATCH v3 1/5] ie: add is_ie_default_sae_group_oui

[PATCH v3 1/5] ie: add is_ie_default_sae_group_oui

[James Prestwood]

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

Start an OUI list of vendors who have buggy SAE group negotiation
---
 src/ie.c | 26 ++++++++++++++++++++++++++
 src/ie.h |  2 ++
 2 files changed, 28 insertions(+)

v3
 * Changed to a for loop
 * Added length prefix to OUI match table

diff --git a/src/ie.c b/src/ie.c
index c03b16b4..6f487912 100644
--- a/src/ie.c
+++ b/src/ie.c
@@ -1364,6 +1364,32 @@ bool is_ie_wpa_ie(const uint8_t *data, uint8_t len)
 	return false;
 }
 
+/*
+ * List of vendor OUIs (prefixed with a length byte) which require forcing
+ * the default SAE group.
+ */
+static const uint8_t use_default_sae_group_ouis[][5] = {
+	{ 0x04, 0xf4, 0xf5, 0xe8, 0x05 },
+};
+
+bool is_ie_default_sae_group_oui(const uint8_t *data, uint16_t len)
+{
+	unsigned int i;
+	const uint8_t *oui;
+
+	for (i = 0; i < L_ARRAY_SIZE(use_default_sae_group_ouis); i++) {
+		oui = use_default_sae_group_ouis[i];
+
+		if (len < oui[0])
+			continue;
+
+		if (!memcmp(oui + 1, data, oui[0]))
+			return true;
+	}
+
+	return false;
+}
+
 int ie_parse_wpa(struct ie_tlv_iter *iter, struct ie_rsn_info *out_info)
 {
 	const uint8_t *data = iter->data;
diff --git a/src/ie.h b/src/ie.h
index 05afce39..8d4ab3c1 100644
--- a/src/ie.h
+++ b/src/ie.h
@@ -551,6 +551,8 @@ int ie_parse_wpa_from_data(const uint8_t *data, size_t len,
 						struct ie_rsn_info *info);
 bool is_ie_wfa_ie(const uint8_t *data, uint8_t len, uint8_t oi_type);
 bool is_ie_wpa_ie(const uint8_t *data, uint8_t len);
+bool is_ie_default_sae_group_oui(const uint8_t *data, uint16_t len);
+
 bool ie_build_wpa(const struct ie_rsn_info *info, uint8_t *to);
 
 int ie_parse_bss_load(struct ie_tlv_iter *iter, uint16_t *out_sta_count,
-- 
2.31.1

[PATCH v3 2/5] scan: set force_default_sae_group if OUI matches

[James Prestwood]

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

---
 src/scan.c | 4 +++-
 src/scan.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/scan.c b/src/scan.c
index 3faa6eb4..16bd2a87 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -924,7 +924,9 @@ static bool scan_parse_vendor_specific(struct scan_bss *bss, const void *data,
 			return false;
 
 		bss->hs20_capable = true;
-	} else
+	} else if (is_ie_default_sae_group_oui(data, len))
+		bss->force_default_sae_group = true;
+	else
 		return false;
 
 	return true;
diff --git a/src/scan.h b/src/scan.h
index 81c84bae..8a57c2b3 100644
--- a/src/scan.h
+++ b/src/scan.h
@@ -83,6 +83,7 @@ struct scan_bss {
 	bool vht_capable : 1;
 	bool anqp_capable : 1;
 	bool hs20_capable : 1;
+	bool force_default_sae_group : 1;
 };
 
 struct scan_parameters {
-- 
2.31.1

[PATCH v3 3/5] sae: add sae_sm_set_force_group_19

[James Prestwood]

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

Setter which forces the use of group 19 rather than the group order
that ELL provides. Certain APs have been found to have buggy group
negotiation and only work if group 19 is tried first, and only. When
an AP like this this is found (based on vendor OUI match) SAE will
use group 19 unconditionally, and fail if group 19 does not work.
Other groups could be tried upon failure but per the spec group 19
must be supported so there isn't much use in trying other, optional
groups.
---
 src/sae.c | 29 +++++++++++++++++++++++++++++
 src/sae.h |  2 ++
 2 files changed, 31 insertions(+)

v3:
 * Use a setter rather than passing in scan_bss

diff --git a/src/sae.c b/src/sae.c
index 5099473c..6b282e8e 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -83,6 +83,8 @@ struct sae_sm {
 	sae_tx_associate_func_t tx_assoc;
 	void *user_data;
 	enum crypto_sae sae_type;
+
+	bool force_default_group : 1;
 };
 
 static enum mmpdu_status_code sae_status_code(struct sae_sm *sm)
@@ -139,6 +141,24 @@ static int sae_choose_next_group(struct sae_sm *sm)
 	const unsigned int *ecc_groups = l_ecc_supported_ike_groups();
 	bool reset = sm->group_retry >= 0;
 
+	/*
+	 * If this is a buggy AP in which group negotiation is broken use the
+	 * default group 19 and fail if this is a retry.
+	 */
+	if (sm->sae_type == CRYPTO_SAE_LOOPING && sm->force_default_group) {
+		if (sm->group_retry != -1) {
+			l_warn("Forced default group but was rejected!");
+			return -ENOENT;
+		}
+
+		l_debug("Forcing default SAE group 19");
+
+		sm->group_retry++;
+		sm->group = 19;
+
+		goto get_curve;
+	}
+
 	do {
 		sm->group_retry++;
 
@@ -151,6 +171,8 @@ static int sae_choose_next_group(struct sae_sm *sm)
 		sae_reset_state(sm);
 
 	sm->group = ecc_groups[sm->group_retry];
+
+get_curve:
 	sm->curve = l_ecc_curve_from_ike_group(sm->group);
 
 	return 0;
@@ -1298,6 +1320,13 @@ bool sae_sm_is_h2e(struct auth_proto *ap)
 	return sm->sae_type != CRYPTO_SAE_LOOPING;
 }
 
+void sae_sm_set_force_group_19(struct auth_proto *ap)
+{
+	struct sae_sm *sm = l_container_of(ap, struct sae_sm, ap);
+
+	sm->force_default_group = true;
+}
+
 static void sae_free(struct auth_proto *ap)
 {
 	struct sae_sm *sm = l_container_of(ap, struct sae_sm, ap);
diff --git a/src/sae.h b/src/sae.h
index 668d084f..ec29e624 100644
--- a/src/sae.h
+++ b/src/sae.h
@@ -30,6 +30,8 @@ typedef void (*sae_tx_associate_func_t)(void *user_data);
 
 bool sae_sm_is_h2e(struct auth_proto *ap);
 
+void sae_sm_set_force_group_19(struct auth_proto *ap);
+
 struct auth_proto *sae_sm_new(struct handshake_state *hs,
 				sae_tx_authenticate_func_t tx_auth,
 				sae_tx_associate_func_t tx_assoc,
-- 
2.31.1

[PATCH v3 4/5] netdev: force SAE group 19 if BSS requires

[James Prestwood]

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

---
 src/netdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/netdev.c b/src/netdev.c
index 513e22d9..54856f6a 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -3485,6 +3485,9 @@ static void netdev_connect_common(struct netdev *netdev,
 			}
 		}
 
+		if (bss->force_default_sae_group)
+			sae_sm_set_force_group_19(netdev->ap);
+
 		break;
 	case IE_RSN_AKM_SUITE_OWE:
 		netdev->ap = owe_sm_new(hs, netdev_owe_tx_authenticate,
-- 
2.31.1

[PATCH v3 5/5] auto-t: add force SAE group 19 autotest

[James Prestwood]

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

The vendor element was taken directly from observed logs of an AP
behaving this way. This causes IWD to force group 19.
---
 autotests/testSAE/connection_test.py | 34 ++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/autotests/testSAE/connection_test.py b/autotests/testSAE/connection_test.py
index f3e865e1..88aa077e 100644
--- a/autotests/testSAE/connection_test.py
+++ b/autotests/testSAE/connection_test.py
@@ -25,7 +25,7 @@ class Test(unittest.TestCase):
 
         wd.wait_for_object_condition(device, 'not obj.scanning')
 
-        device.scan()
+        device.debug_scan([2412])
 
         wd.wait_for_object_condition(device, 'obj.scanning')
         wd.wait_for_object_condition(device, 'not obj.scanning')
@@ -57,8 +57,21 @@ class Test(unittest.TestCase):
         wd.unregister_psk_agent(psk_agent)
 
     def test_SAE(self):
-        self.hostapd.set_value('sae_pwe', '0');
-        self.hostapd.set_value('sae_groups', '19');
+        self.hostapd.set_value('sae_pwe', '0')
+        self.hostapd.set_value('sae_groups', '19')
+        self.hostapd.set_value('vendor_elements', '')
+        self.hostapd.reload()
+        self.hostapd.wait_for_event("AP-ENABLED")
+        wd = IWD(True)
+        self.validate_connection(wd)
+        wd.clear_storage()
+
+    def test_SAE_force_group_19(self):
+        self.hostapd.set_value('sae_pwe', '0')
+        self.hostapd.set_value('sae_groups', '19')
+        # Vendor data from APs which require group 19 be used first
+        # TODO: (for all tests) verify the expected group was used
+        self.hostapd.set_value('vendor_elements', 'dd0cf4f5e8050500000000000000')
         self.hostapd.reload()
         self.hostapd.wait_for_event("AP-ENABLED")
         wd = IWD(True)
@@ -66,8 +79,9 @@ class Test(unittest.TestCase):
         wd.clear_storage()
 
     def test_SAE_Group20(self):
-        self.hostapd.set_value('sae_pwe', '0');
-        self.hostapd.set_value('sae_groups', '20');
+        self.hostapd.set_value('sae_pwe', '0')
+        self.hostapd.set_value('sae_groups', '20')
+        self.hostapd.set_value('vendor_elements', '')
         self.hostapd.reload()
         self.hostapd.wait_for_event("AP-ENABLED")
         wd = IWD(True)
@@ -75,8 +89,9 @@ class Test(unittest.TestCase):
         wd.clear_storage()
 
     def test_SAE_H2E(self):
-        self.hostapd.set_value('sae_pwe', '1');
-        self.hostapd.set_value('sae_groups', '19');
+        self.hostapd.set_value('sae_pwe', '1')
+        self.hostapd.set_value('sae_groups', '19')
+        self.hostapd.set_value('vendor_elements', '')
         self.hostapd.reload()
         self.hostapd.wait_for_event("AP-ENABLED")
         wd = IWD(True)
@@ -84,8 +99,9 @@ class Test(unittest.TestCase):
         wd.clear_storage()
 
     def test_SAE_H2E_Group20(self):
-        self.hostapd.set_value('sae_pwe', '1');
-        self.hostapd.set_value('sae_groups', '20');
+        self.hostapd.set_value('sae_pwe', '1')
+        self.hostapd.set_value('sae_groups', '20')
+        self.hostapd.set_value('vendor_elements', '')
         self.hostapd.reload()
         self.hostapd.wait_for_event("AP-ENABLED")
         wd = IWD(True)
-- 
2.31.1

Re: [PATCH v3 1/5] ie: add is_ie_default_sae_group_oui

[Denis Kenzior]

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

Hi James,

On 8/25/21 12:12 PM, James Prestwood wrote:
> Start an OUI list of vendors who have buggy SAE group negotiation
> ---
>   src/ie.c | 26 ++++++++++++++++++++++++++
>   src/ie.h |  2 ++
>   2 files changed, 28 insertions(+)
> 
> v3
>   * Changed to a for loop
>   * Added length prefix to OUI match table
> 

All applied, thanks.

Regards,
-Denis