[v2,05/15] dpp: fix config request header check

Commit Message

James Prestwood Oct. 26, 2023, 8:26 p.m. UTC

The check for the header was incorrect according to the spec.
Table 58 indicates that the "Query Response Info" should be set
to 0x00 for the configuration request. The frame handler was
expecting 0x7f which is the value for the config response frame.

Unfortunately wpa_supplicant also gets this wrong and uses 0x7f
in all cases which is likely why this value was set incorrectly
in IWD. The issue is that IWD's config request is correct which
means IWD<->IWD configuration is broken. (and wpa_supplicant as
a configurator likely doesn't validate the config request).

Fix this by checking both 0x7f and 0x00 to handle both
supplicants.
---
 src/dpp.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

Comments

James Prestwood Oct. 26, 2023, 9:53 p.m. UTC | #1

On 10/26/23 1:26 PM, James Prestwood wrote:
> The check for the header was incorrect according to the spec.
> Table 58 indicates that the "Query Response Info" should be set
> to 0x00 for the configuration request. The frame handler was
> expecting 0x7f which is the value for the config response frame.
> 
> Unfortunately wpa_supplicant also gets this wrong and uses 0x7f
> in all cases which is likely why this value was set incorrectly
> in IWD. The issue is that IWD's config request is correct which
> means IWD<->IWD configuration is broken. (and wpa_supplicant as
> a configurator likely doesn't validate the config request).
> 
> Fix this by checking both 0x7f and 0x00 to handle both
> supplicants.
> ---
>   src/dpp.c | 21 +++++++++++++++++----
>   1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/src/dpp.c b/src/dpp.c
> index dff0ecaf..6fd37272 100644
> --- a/src/dpp.c
> +++ b/src/dpp.c
> @@ -887,6 +887,21 @@ static void dpp_send_config_response(struct dpp_sm *dpp, uint8_t status)
>   	dpp_send_frame(dpp, iov, 2, dpp->current_freq);
>   }
>   
> +static bool dpp_check_config_header(const uint8_t *ptr)
> +{
> +	/*
> +	 * Table 58. General Format of DPP Configuration Request frame
> +	 *
> +	 * Unfortunately wpa_supplicant hard codes 0x7f as the Query Response
> +	 * Info so we need to handle both cases.
> +	 */
> +	return ptr[0] != IE_TYPE_ADVERTISEMENT_PROTOCOL ||
> +			ptr[1] != 0x08 ||
> +			(ptr[2] != 0x7f || ptr[2] != 0x00) ||
> +			ptr[3] != IE_TYPE_VENDOR_SPECIFIC ||
> +			ptr[4] != 5;
> +}

I somehow got this logic completely backwards. This will always be true, 
will send v2 after any review comments.

> +
>   static void dpp_handle_config_request_frame(const struct mmpdu_header *frame,
>   				const void *body, size_t body_len,
>   				int rssi, void *user_data)
> @@ -904,8 +919,6 @@ static void dpp_handle_config_request_frame(const struct mmpdu_header *frame,
>   	const uint8_t *e_nonce = NULL;
>   	size_t wrapped_len = 0;
>   	_auto_(l_free) uint8_t *unwrapped = NULL;
> -	uint8_t hdr_check[] = { IE_TYPE_ADVERTISEMENT_PROTOCOL, 0x08, 0x7f,
> -				IE_TYPE_VENDOR_SPECIFIC, 5 };
>   	struct json_iter jsiter;
>   	_auto_(l_free) char *tech = NULL;
>   	_auto_(l_free) char *role = NULL;
> @@ -932,10 +945,10 @@ static void dpp_handle_config_request_frame(const struct mmpdu_header *frame,
>   
>   	dpp->diag_token = *ptr++;
>   
> -	if (memcmp(ptr, hdr_check, sizeof(hdr_check)))
> +	if (!dpp_check_config_header(ptr))
>   		return;
>   
> -	ptr += sizeof(hdr_check);
> +	ptr += 5;
>   
>   	if (memcmp(ptr, wifi_alliance_oui, sizeof(wifi_alliance_oui)))
>   		return;

Patch

diff --git a/src/dpp.c b/src/dpp.c
index dff0ecaf..6fd37272 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -887,6 +887,21 @@  static void dpp_send_config_response(struct dpp_sm *dpp, uint8_t status)
 	dpp_send_frame(dpp, iov, 2, dpp->current_freq);
 }
 
+static bool dpp_check_config_header(const uint8_t *ptr)
+{
+	/*
+	 * Table 58. General Format of DPP Configuration Request frame
+	 *
+	 * Unfortunately wpa_supplicant hard codes 0x7f as the Query Response
+	 * Info so we need to handle both cases.
+	 */
+	return ptr[0] != IE_TYPE_ADVERTISEMENT_PROTOCOL ||
+			ptr[1] != 0x08 ||
+			(ptr[2] != 0x7f || ptr[2] != 0x00) ||
+			ptr[3] != IE_TYPE_VENDOR_SPECIFIC ||
+			ptr[4] != 5;
+}
+
 static void dpp_handle_config_request_frame(const struct mmpdu_header *frame,
 				const void *body, size_t body_len,
 				int rssi, void *user_data)
@@ -904,8 +919,6 @@  static void dpp_handle_config_request_frame(const struct mmpdu_header *frame,
 	const uint8_t *e_nonce = NULL;
 	size_t wrapped_len = 0;
 	_auto_(l_free) uint8_t *unwrapped = NULL;
-	uint8_t hdr_check[] = { IE_TYPE_ADVERTISEMENT_PROTOCOL, 0x08, 0x7f,
-				IE_TYPE_VENDOR_SPECIFIC, 5 };
 	struct json_iter jsiter;
 	_auto_(l_free) char *tech = NULL;
 	_auto_(l_free) char *role = NULL;
@@ -932,10 +945,10 @@  static void dpp_handle_config_request_frame(const struct mmpdu_header *frame,
 
 	dpp->diag_token = *ptr++;
 
-	if (memcmp(ptr, hdr_check, sizeof(hdr_check)))
+	if (!dpp_check_config_header(ptr))
 		return;
 
-	ptr += sizeof(hdr_check);
+	ptr += 5;
 
 	if (memcmp(ptr, wifi_alliance_oui, sizeof(wifi_alliance_oui)))
 		return;