Interop problem with Quantum Fiber WiFi7 pods (duplicate RSNXE IE)

Interop problem with Quantum Fiber WiFi7 pods (duplicate RSNXE IE)

[Vivek Das Mohapatra]

Hi - we've had reports of an incompatibility between network manager and
the wifi7 hardware mentioned when using iwd as the backend.

 From what we've been able to tell:

validate_mgmt_ies() fails
cause: disallowed duplicate IEs in the AssocResponse frame

We appear to have two dupes:

221 - IE_TYPE_VENDOR_SPECIFIC - allowed
244 - IE_TYPE_RSNX - not allowed as a dup.

We checked IEEE 80211-2020.pdf couldn't find specific details about IEs
that can be duplicated vs the ones that can’t.
[ Sections 9.4.2.1 and 9.4.2.241 ]

We've tried various devices and software combinations (linux and non)
and it seems to be an interop problem with iwd specifically - we think 
adding IE_TYPE_RSNX to the allowed dups list in validate_mgmt_ies should
do the trick.

To summarise:

  - nm + iwd (v2.14) cannot auth against a Quantum Fiber WiFi7 pod
  - there's a duplicate IE in the assocresponse which iwd is rejecting
  - we think that IE should be allowed as a dup
  - nm + wpa_supplicant seems to work
  - we tried some non-linux device and they were also able to auth

Does that sound like a reasonable fix? Do you want a patch?

Re: Interop problem with Quantum Fiber WiFi7 pods (duplicate RSNXE IE)

[James Prestwood]

Hi,

On 10/2/24 10:08 AM, Vivek Das Mohapatra wrote:
> Hi - we've had reports of an incompatibility between network manager and
> the wifi7 hardware mentioned when using iwd as the backend.
>
> From what we've been able to tell:
>
> validate_mgmt_ies() fails
> cause: disallowed duplicate IEs in the AssocResponse frame
>
> We appear to have two dupes:
>
> 221 - IE_TYPE_VENDOR_SPECIFIC - allowed

This makes sense.

> 244 - IE_TYPE_RSNX - not allowed as a dup.

This does not. The issue being which one do we use? The first, second, 
Nth element? This is definitely an AP problem.

>
> We checked IEEE 80211-2020.pdf couldn't find specific details about IEs
> that can be duplicated vs the ones that can’t.
> [ Sections 9.4.2.1 and 9.4.2.241 ]
I don't remember there being anything in the spec that dictates which 
IEs can have duplicantes but IWD has this restriction for all elements 
except those that make sense to have duplicates, like vendor specific, 
neighbor reports, etc.
>
> We've tried various devices and software combinations (linux and non)
> and it seems to be an interop problem with iwd specifically - we think 
> adding IE_TYPE_RSNX to the allowed dups list in validate_mgmt_ies should
> do the trick.
Denis will have the final say, but I would say if we are to allow the 
RSNX element to have a duplicate it should include a warning indicating 
the AP is buggy, and a comment in the code with the specific AP/vendor 
information so we know why we added this case.
>
> To summarise:
>
>  - nm + iwd (v2.14) cannot auth against a Quantum Fiber WiFi7 pod
>  - there's a duplicate IE in the assocresponse which iwd is rejecting
>  - we think that IE should be allowed as a dup
>  - nm + wpa_supplicant seems to work
>  - we tried some non-linux device and they were also able to auth
>
> Does that sound like a reasonable fix? Do you want a patch?

Thanks,

James

Re: Interop problem with Quantum Fiber WiFi7 pods (duplicate RSNXE IE)

[Denis Kenzior]

Hi Vivek,

On 10/2/24 12:08 PM, Vivek Das Mohapatra wrote:
> Hi - we've had reports of an incompatibility between network manager and
> the wifi7 hardware mentioned when using iwd as the backend.
> 
>  From what we've been able to tell:
> 
> validate_mgmt_ies() fails
> cause: disallowed duplicate IEs in the AssocResponse frame
> 
> We appear to have two dupes:
> 
> 221 - IE_TYPE_VENDOR_SPECIFIC - allowed
> 244 - IE_TYPE_RSNX - not allowed as a dup.
> 
> We checked IEEE 80211-2020.pdf couldn't find specific details about IEs
> that can be duplicated vs the ones that can’t.
> [ Sections 9.4.2.1 and 9.4.2.241 ]

Refer to section 9.3.3.1:
"The frame body consists of fields and elements as defined for each management 
frame subtype. All fields and elements are mandatory unless stated otherwise. 
Fields and elements appear in the specified, relative order, skipping fields or 
elements that are not present. STAs that encounter an element ID they do not 
recognize in the frame body of a received Management frame ignore that element 
and continue to parse the remainder of the management frame body (if any) for 
additional elements with recognizable element IDs."

IEs that allow duplication are explicitly called out:
"Vendor Specific
One or more Vendor Specific elements are optionally present. These elements 
follow all other elements."

Why are the elements duplicated?  Are they the same or different?  If different, 
which one would iwd use?

> 
> We've tried various devices and software combinations (linux and non)
> and it seems to be an interop problem with iwd specifically - we think adding 
> IE_TYPE_RSNX to the allowed dups list in validate_mgmt_ies should
> do the trick.

How about fixing the firmware?

Regards,
-Denis

Re: Interop problem with Quantum Fiber WiFi7 pods (duplicate RSNXE IE)

[Vivek Das Mohapatra]

On 02/10/2024 19:14, Denis Kenzior wrote:

> IEs that allow duplication are explicitly called out:
> "Vendor Specific
> One or more Vendor Specific elements are optionally present. These 
> elements follow all other elements."
> 
> Why are the elements duplicated?  Are they the same or different?  If 
> different, which one would iwd use?

Looking at the OTA capture it looks like it's a genuine repeat, the 
contents are the same.

> How about fixing the firmware?

To be clear: We're not the people preparing the firmware. We support
a distro that uses iwd. So fixing the firmware would be great, but
we don't have any way to make that happen.

Re: Interop problem with Quantum Fiber WiFi7 pods (duplicate RSNXE IE)

[Vivek Das Mohapatra]

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

On 02/10/2024 22:41, Vivek Das Mohapatra wrote:
> On 02/10/2024 19:14, Denis Kenzior wrote:
>>
>> Why are the elements duplicated?  Are they the same or different?  If 
>> different, which one would iwd use?

OTA capture:

I've linked 1 file to this email:

   * 0926_qfiber.tar.xz
     Size: 5.2 MB
     Link:
 
https://www.dropbox.com/scl/fi/i573yehaenpvkgy29smwb/0926_qfiber.tar.xz?rlkey=sxhhiwhmqppyic5wsc446h88r&dl=0




[-- Attachment #2: 0926_qfiber.tar.xz.html --]
[-- Type: text/html, Size: 3413 bytes --]

Re: Interop problem with Quantum Fiber WiFi7 pods (duplicate RSNXE IE)

[James Prestwood]

Hi Vivek,

On 10/2/24 2:55 PM, Vivek Das Mohapatra wrote:
> On 02/10/2024 22:41, Vivek Das Mohapatra wrote:
>> On 02/10/2024 19:14, Denis Kenzior wrote:
>>>
>>> Why are the elements duplicated?  Are they the same or different?  
>>> If different, which one would iwd use?
>
> OTA capture:
>
> I've linked 1 file to this email:
>
>   * 0926_qfiber.tar.xz
>     Size: 5.2 MB
>     Link:
>
> https://www.dropbox.com/scl/fi/i573yehaenpvkgy29smwb/0926_qfiber.tar.xz?rlkey=sxhhiwhmqppyic5wsc446h88r&dl=0 
>
>
>
So not only is it duplicating the IE, but its also violating the IE 
ordering which is actually a spec violation (Table 9-35—Association 
Response frame body). So I suspect even if you allow RSNXE duplicates 
you may still run into a failure with the ordering. Is there some route 
to reporting this to the vendor?

Thanks,

James

Interop problem with Quantum Fiber WiFi7 pods (duplicate RSNXE IE)

[Vivek Das Mohapatra]

On 03/10/2024 13:17, James Prestwood wrote:
> Hi Vivek,
> 
> So not only is it duplicating the IE, but its also violating the IE 
> ordering which is actually a spec violation (Table 9-35—Association 
> Response frame body). So I suspect even if you allow RSNXE duplicates 
> you may still run into a failure with the ordering. Is there some route 
> to reporting this to the vendor?

We're looking into it but we're not even a retail customer of theirs
so no idea if they'll respond (I mean they might, I don't know,
I've just never dealt with them before).

Re: Interop problem with Quantum Fiber WiFi7 pods (duplicate RSNXE IE)

[Denis Kenzior]

Hi James,

> So not only is it duplicating the IE, but its also violating the IE ordering 
> which is actually a spec violation (Table 9-35—Association Response frame body). 
> So I suspect even if you allow RSNXE duplicates you may still run into a failure 
> with the ordering. Is there some route to reporting this to the vendor?

https://git.kernel.org/pub/scm/network/wireless/iwd.git/commit/?id=8f9ed66bdd4c26e619f0878cc8ef987ce27e4305

We gave up on enforcing the order a long time ago.

Vivek, if you want to submit a patch allowing duplicate IEs as long as they are 
bit-identical (length, and contents), I think that would be okay.  From what I 
can see our logic should be only picking up the first occurrence.

Regards,
-Denis

Re: Interop problem with Quantum Fiber WiFi7 pods (duplicate RSNXE IE)

[Vivek Das Mohapatra]

On 04/10/2024 03:58, Denis Kenzior wrote:
> Hi James,
[cut]
> https://git.kernel.org/pub/scm/network/wireless/iwd.git/commit/?id=8f9ed66bdd4c26e619f0878cc8ef987ce27e4305
> 
> We gave up on enforcing the order a long time ago.
> 
> Vivek, if you want to submit a patch allowing duplicate IEs as long as 
> they are bit-identical (length, and contents), I think that would be 
> okay.  From what I can see our logic should be only picking up the first 
> occurrence.

I'll get started on it, thanks.

RFC: Be more lenient about duplicate IEs with identical payloads

[Vivek Das Mohapatra]

I think these patches do the right thing:
 - duplicate IEs with identical payloads don't count as validation failures
 - the duplicate-IE test data has been altered to make the duplicate IE
   have differeing payloads so that test still passes

[PATCH 1/2] unit: add a test for harmless IE clones

[Vivek Das Mohapatra]

Test handling of technically illegal but harmless cloned IEs.
Based on real traffic captured from retail APs.

As cloned IEs are now allowed the

  "/IE order/Bad (Duplicate + Out of Order IE) 1"

test payload has been altered to be more-wrong so it still fails
verification as expected.
---
 unit/test-mpdu.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/unit/test-mpdu.c b/unit/test-mpdu.c
index 89b5bb0a..416756e9 100644
--- a/unit/test-mpdu.c
+++ b/unit/test-mpdu.c
@@ -145,7 +145,7 @@ static const uint8_t probe_req_ie_duplicate1[] = {
 	0x00, 0x00, 0x03, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x50, 0x64,
 	/* SSID, SSID, Supported Rates, Extended Supported Rates */
 	0x00, 0x05, 0x74, 0x65, 0x73, 0x74, 0x31, 0x00, 0x05, 0x74, 0x65, 0x73,
-	0x74, 0x31, 0x01, 0x08, 0x02, 0x04, 0x0b, 0x16, 0x0c, 0x12, 0x18, 0x24,
+	0x74, 0x32, 0x01, 0x08, 0x02, 0x04, 0x0b, 0x16, 0x0c, 0x12, 0x18, 0x24,
 	0x32, 0x04, 0x30, 0x48, 0x60, 0x6c,
 };
 
@@ -168,6 +168,85 @@ static const struct test_frame_data probe_req_ie_out_of_order2_data = {
 	probe_req_ie_out_of_order2, sizeof(probe_req_ie_out_of_order2), true
 };
 
+static const uint8_t assoc_rsp_ie_ooo_tolerated_dup[] = {
+	/* Association Response Flags (24 bytes)*/
+	0x10, 0x00, 0x24, 0x00, 0xe8, 0x8d, 0xa6, 0x77, 0xa7, 0x37, 0x0a, 0x58,
+	0x28, 0x16, 0x65, 0x42, 0x0a, 0x58, 0x28, 0x16, 0x65, 0x42, 0x40, 0x14,
+	/* Fixed Parameters (6 bytes) */
+	0x31, 0x19, 0x00, 0x00, 0x08, 0xc0,
+	/* Tagged parameters: 364 bytes */
+	/* Support Rates [1] (10 bytes) */
+	0x01, 0x08, 0x8c, 0x12, 0x98, 0x24, 0xb0, 0x48, 0x60, 0x6c,
+	/* Vendor Specific [221] (12 bytes) */
+	0xdd, 0x0a, 0x50, 0x6f, 0x9a, 0x1b, 0x06, 0x01, 0x20, 0x07, 0x01, 0x03,
+	/* RM Enabled Capabilities [70] (7 bytes) */
+	0x46, 0x05, 0x02, 0x00, 0x00, 0x00, 0x00,
+	/* Vendor Specific [221] (26 bytes) */
+	0xdd, 0x18, 0x00, 0x50, 0xf2, 0x02, 0x01, 0x01, 0x86, 0x00, 0x03, 0xa4,
+	0x00, 0x00, 0x27, 0xa4, 0x00, 0x00, 0x42, 0x43, 0x5e, 0x00, 0x62, 0x32,
+	0x2f, 0x00,
+	/* HT Capabilities [45] (28 bytes) */
+	0x2d, 0x1a, 0xef, 0x09, 0x03, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18,
+	0x04, 0x87, 0x19, 0x00,
+	/* HT Information [61] (24 bytes)*/
+	0x3d, 0x16, 0x24, 0x05, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	/* Vendor Specific [221] (32 bytes) */
+	0xdd, 0x1e, 0x00, 0x90, 0x4c, 0x33, 0xef, 0x09, 0x03, 0xff, 0xff, 0xff,
+	0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x18, 0x04, 0x87, 0x19, 0x00,
+	/* Vendor Specific [221] (28 bytes) */
+	0xdd, 0x1a, 0x00, 0x90, 0x4c, 0x34, 0x24, 0x05, 0x06, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00,
+	/* BSS Max Idle Period [90] (5 bytes)*/
+	0x5a, 0x03, 0xe0, 0x01, 0x00,
+	/* VHT Capabilities [191] (14 bytes)*/
+	0xbf, 0x0c, 0xf5, 0x39, 0xc3, 0x33, 0x9a, 0xff, 0x30, 0x0c, 0x9a, 0xff,
+	0x30, 0x2c,
+	/* VHT Operation [192] (7 bytes) */
+	0xc0, 0x05, 0x01, 0x2a, 0x00, 0xfc, 0xff,
+	/* Extended: HE Capabilities 80211ax/D3.0 [256 + 35] (38 bytes) */
+	0xff, 0x24, 0x23, 0x05, 0x00, 0x08, 0x1a, 0x44, 0x10, 0x0c, 0x20, 0xce,
+	0x92, 0x6f, 0x1b, 0xaf, 0xd4, 0x11, 0x0c, 0x00, 0xaa, 0xff, 0xaa, 0xff,
+	0xaa, 0xff, 0xaa, 0xff, 0x3b, 0x1c, 0xc7, 0x71, 0x1c, 0xc7, 0x71, 0x1c,
+	0xc7, 0x71,
+	/* Extended: HE Operation 80211ax/D3.0 [256 + 36] (9 bytes) */
+	0xff, 0x07, 0x24, 0xf4, 0x3f, 0x00, 0x17, 0xfc, 0xff,
+	/* Extended: Spatial Reuse Parameter Set [256 + 39] (4 bytes) */
+	0xff, 0x02, 0x27, 0x03,
+	/* Extended: MU EDCA Parameter Set [256 + 38] (16 bytes) */
+	0xff, 0x0e, 0x26, 0x06, 0x00, 0xff, 0x00, 0x20, 0xff, 0x00, 0x40, 0xff,
+	0x00, 0x60, 0xff, 0x00,
+	/* Extended Capabilities [127] (13 bytes) */
+	0x7f, 0x0b, 0x00, 0x00, 0x08, 0x80, 0x00, 0x40, 0x00, 0x40, 0x00, 0x40,
+	0x00,
+	/* Vendor Specific [221] (9 bytes) */
+	0xdd, 0x07, 0x00, 0x0c, 0x43, 0x00, 0x00, 0x00, 0x00,
+	/* Vendor Specific [221] (35 bytes) */
+	0xdd, 0x21, 0x00, 0x0c, 0xe7, 0x00, 0x00, 0x00, 0x00, 0xbf, 0x0c, 0xb1,
+	0x01, 0xc0, 0x33, 0x2a, 0xff, 0x92, 0x04, 0x2a, 0xff, 0x92, 0x04, 0xc0,
+	0x05, 0x00, 0x00, 0x00, 0x2a, 0xff, 0xc3, 0x03, 0x01, 0x02, 0x02,
+	/* Vendor Specific [221] (9 bytes) */
+	0xdd, 0x07, 0x50, 0x6f, 0x9a, 0x16, 0x01, 0x01, 0x00,
+	/* RSN Extension [244] (3 bytes) */
+	0xf4, 0x01, 0x20,
+	/* Vendor Specific [221] (9 bytes) */ /* Legal duplicate IE */
+	0xdd, 0x07, 0x50, 0x6f, 0x9a, 0x16, 0x01, 0x01, 0x00,
+	/* FILS Indication [240] (4 bytes) */
+	0xf0, 0x02, 0x00, 0x00,
+	/* RSN Extension [244] (3 bytes) */ /* TECHNICALLY ILLEGAL cloned ie */
+	0xf4, 0x01, 0x20,
+	/* Vendor Specific [221] (19 bytes) */
+	0xdd, 0x11, 0x00, 0x0c, 0xe7, 0x01, 0x00, 0x00, 0x00, 0x04, 0x08, 0x02,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+
+static const struct test_frame_data assoc_rsp_ie_ooo_tolerated_dup_data = {
+	assoc_rsp_ie_ooo_tolerated_dup, sizeof(assoc_rsp_ie_ooo_tolerated_dup), true
+};
+
 static void ie_order_test(const void *data)
 {
 	const struct test_frame_data *frame = data;
@@ -227,6 +306,9 @@ int main(int argc, char *argv[])
 	l_test_add("/IE order/Bad (Duplicate + Out of Order IE) 1",
 				ie_order_test,
 				&probe_req_ie_duplicate1_data);
+	l_test_add("/IE order/Ugly (Duplicate + Out of Order IE) 2",
+				ie_order_test,
+				&assoc_rsp_ie_ooo_tolerated_dup_data);
 	l_test_add("/IE order/Good (Out of Order IE) 2", ie_order_test,
 				&probe_req_ie_out_of_order2_data);
 
-- 
2.30.2

[PATCH 2/2] mpdu: tolerate technically illegal but harmless cloned IEs

[Vivek Das Mohapatra]

---
 src/ie.h   | 17 +++++++++++++++++
 src/mpdu.c | 10 +++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/ie.h b/src/ie.h
index 4498785a..bc750696 100644
--- a/src/ie.h
+++ b/src/ie.h
@@ -598,6 +598,23 @@ static inline const unsigned char *ie_tlv_iter_get_data(
 	return iter->data;
 }
 
+static inline bool ie_tlv_iter_data_eq(struct ie_tlv_iter *a,
+					struct ie_tlv_iter *b)
+{
+	if (a == b)
+		return true;
+
+	if (a == NULL || b == NULL)
+		return false;
+
+	if (ie_tlv_iter_get_length(a) != ie_tlv_iter_get_length(b))
+		return false;
+
+	return memcmp(ie_tlv_iter_get_data(a),
+			ie_tlv_iter_get_data(b),
+			ie_tlv_iter_get_length(a)) == 0;
+}
+
 void *ie_tlv_extract_wsc_payload(const uint8_t *ies, size_t len,
 							ssize_t *out_len);
 void *ie_tlv_encapsulate_wsc_payload(const uint8_t *data, size_t len,
diff --git a/src/mpdu.c b/src/mpdu.c
index 9d0409d2..09df696e 100644
--- a/src/mpdu.c
+++ b/src/mpdu.c
@@ -398,9 +398,17 @@ static bool validate_mgmt_ies(const uint8_t *ies, size_t ies_len,
 
 			memcpy(&clone, &iter, sizeof(clone));
 
+			/*
+			 * Some APs send completely identical duplicate IEs:
+			 * Since these are harmless (and ignored by us) we're
+			 * going to allow them here for interoperability.
+			 */
 			while (ie_tlv_iter_next(&clone)) {
-				if (ie_tlv_iter_get_tag(&clone) != tag)
+				if (ie_tlv_iter_get_tag(&clone) != tag) {
 					continue;
+				} else if (ie_tlv_iter_data_eq(&iter, &clone)) {
+					continue;
+				}
 
 				return false;
 			}
-- 
2.30.2

Re: [PATCH 1/2] unit: add a test for harmless IE clones

[Denis Kenzior]

Hi Vivek,

On 10/4/24 9:19 PM, Vivek Das Mohapatra wrote:
> Test handling of technically illegal but harmless cloned IEs.
> Based on real traffic captured from retail APs.
> 
> As cloned IEs are now allowed the
> 
>    "/IE order/Bad (Duplicate + Out of Order IE) 1"
> 
> test payload has been altered to be more-wrong so it still fails
> verification as expected.
> ---
>   unit/test-mpdu.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 83 insertions(+), 1 deletion(-)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH 2/2] mpdu: tolerate technically illegal but harmless cloned IEs

[Denis Kenzior]

Hi Vivek,

On 10/4/24 9:19 PM, Vivek Das Mohapatra wrote:
> ---
>   src/ie.h   | 17 +++++++++++++++++
>   src/mpdu.c | 10 +++++++++-
>   2 files changed, 26 insertions(+), 1 deletion(-)
> 

<snip>

> @@ -398,9 +398,17 @@ static bool validate_mgmt_ies(const uint8_t *ies, size_t ies_len,
>   
>   			memcpy(&clone, &iter, sizeof(clone));
>   
> +			/*
> +			 * Some APs send completely identical duplicate IEs:
> +			 * Since these are harmless (and ignored by us) we're
> +			 * going to allow them here for interoperability.
> +			 */
>   			while (ie_tlv_iter_next(&clone)) {
> -				if (ie_tlv_iter_get_tag(&clone) != tag)
> +				if (ie_tlv_iter_get_tag(&clone) != tag) {
>   					continue;
> +				} else if (ie_tlv_iter_data_eq(&iter, &clone)) {
> +					continue;
> +				}

No need for {}.  I took these out and applied.  Thanks.

Regards,
-Denis

Re: [PATCH 2/2] mpdu: tolerate technically illegal but harmless cloned IEs

[Vivek Das Mohapatra]

On 07/10/2024 19:27, Denis Kenzior wrote:
> Hi Vivek,
> 
> On 10/4/24 9:19 PM, Vivek Das Mohapatra wrote:
>> ---
>>   src/ie.h   | 17 +++++++++++++++++
>>   src/mpdu.c | 10 +++++++++-
>>   2 files changed, 26 insertions(+), 1 deletion(-)

[cut]

> 
> No need for {}.  I took these out and applied.  Thanks.

Thanks! I wasn't sure what the house style would be there.