[PATCH 1/2] dpp: check that DPP is running in station watch

[PATCH 1/2] dpp: check that DPP is running in station watch

[James Prestwood]

This was causing unneeded WARNING prints because the DPP state
was never checked. Fix this and bail out if DPP isn't running.
---
 src/dpp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/dpp.c b/src/dpp.c
index 5cf9ca23..18b2a7c6 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -3686,6 +3686,9 @@ static void dpp_station_state_watch(enum station_state state, void *user_data)
 {
 	struct dpp_sm *dpp = user_data;
 
+	if (dpp->state == DPP_STATE_NOTHING)
+		return;
+
 	switch (state) {
 	case STATION_STATE_DISCONNECTED:
 	case STATION_STATE_DISCONNECTING:
-- 
2.34.1

[PATCH 2/2] dpp: check for non-utf8 SSID's in scan results

[James Prestwood]

In theory this shouldn't be possible because the configuration object
validates that the SSID is utf-8. But it doesn't hurt to check
especially since we can't control what the kernel sends us.
---
 src/dpp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/dpp.c b/src/dpp.c
index 18b2a7c6..8da79603 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -884,6 +884,9 @@ static bool dpp_scan_results(int err, struct l_queue *bss_list,
 	/* Purely for grabbing the SSID */
 	bss = l_queue_peek_head(bss_list);
 
+	if (L_WARN_ON(!util_ssid_is_utf8(bss->ssid_len, bss->ssid)))
+		goto reset;
+
 	memcpy(ssid, bss->ssid, bss->ssid_len);
 	ssid[bss->ssid_len] = '\0';
 
-- 
2.34.1

Re: [PATCH 1/2] dpp: check that DPP is running in station watch

[Denis Kenzior]

Hi James,

On 11/16/23 09:44, James Prestwood wrote:
> This was causing unneeded WARNING prints because the DPP state
> was never checked. Fix this and bail out if DPP isn't running.
> ---
>   src/dpp.c | 3 +++
>   1 file changed, 3 insertions(+)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH 2/2] dpp: check for non-utf8 SSID's in scan results

[Denis Kenzior]

Hi James,

On 11/16/23 09:44, James Prestwood wrote:
> In theory this shouldn't be possible because the configuration object
> validates that the SSID is utf-8. But it doesn't hurt to check
> especially since we can't control what the kernel sends us.
> ---
>   src/dpp.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/src/dpp.c b/src/dpp.c
> index 18b2a7c6..8da79603 100644
> --- a/src/dpp.c
> +++ b/src/dpp.c
> @@ -884,6 +884,9 @@ static bool dpp_scan_results(int err, struct l_queue *bss_list,
>   	/* Purely for grabbing the SSID */
>   	bss = l_queue_peek_head(bss_list);
>   
> +	if (L_WARN_ON(!util_ssid_is_utf8(bss->ssid_len, bss->ssid)))
> +		goto reset;
> +

This still seems brittle.  You have the validated SSID from the DPP session, 
shouldn't you be storing that and using it to filter the scan results?  There's 
no guarantee that a filtered active scan is going to return only the SSID you 
asked for (lets say a misbehaving or malicious AP), so assuming that the first 
BSS in the scan results list is the SSID you want isn't really guaranteed.

>   	memcpy(ssid, bss->ssid, bss->ssid_len);
>   	ssid[bss->ssid_len] = '\0';
>   

Regards,
-Denis

Re: [PATCH 2/2] dpp: check for non-utf8 SSID's in scan results

[James Prestwood]

Hi Denis,

On 11/16/23 07:51, Denis Kenzior wrote:
> Hi James,
>
> On 11/16/23 09:44, James Prestwood wrote:
>> In theory this shouldn't be possible because the configuration object
>> validates that the SSID is utf-8. But it doesn't hurt to check
>> especially since we can't control what the kernel sends us.
>> ---
>>   src/dpp.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/src/dpp.c b/src/dpp.c
>> index 18b2a7c6..8da79603 100644
>> --- a/src/dpp.c
>> +++ b/src/dpp.c
>> @@ -884,6 +884,9 @@ static bool dpp_scan_results(int err, struct 
>> l_queue *bss_list,
>>       /* Purely for grabbing the SSID */
>>       bss = l_queue_peek_head(bss_list);
>>   +    if (L_WARN_ON(!util_ssid_is_utf8(bss->ssid_len, bss->ssid)))
>> +        goto reset;
>> +
>
> This still seems brittle.  You have the validated SSID from the DPP 
> session, shouldn't you be storing that and using it to filter the scan 
> results?  There's no guarantee that a filtered active scan is going to 
> return only the SSID you asked for (lets say a misbehaving or 
> malicious AP), so assuming that the first BSS in the scan results list 
> is the SSID you want isn't really guaranteed.

Sure I can do that. I was banking on the kernel filtering, but we can be 
100% sure and just store the SSID in the dpp_sm.

>
>>       memcpy(ssid, bss->ssid, bss->ssid_len);
>>       ssid[bss->ssid_len] = '\0';
>
> Regards,
> -Denis