Re: [PATCH v3 3/4] dpp: fix fragile scan/connecting logic

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

On 11/13/23 11:54, James Prestwood wrote:
> The post-DPP connection was never done quite right due to station's
> state being unknown. The state is now tracked in DPP by a previous
> patch but the scan path in DPP is still wrong.
> 
> It relies on station autoconnect logic which has the potential to
> connect to a different network than what was configured with DPP.
> Its unlikely but still could happen in theory. In addition the scan
> was not selectively filtering results by the SSID that DPP
> configured.
> 
> This fixes the above problems by first filtering the scan by the
> SSID. Then setting the scan results into station without triggering
> autoconnect. And finally using network_autoconnect() directly
> instead of relying on station to choose the SSID.
> ---
>   src/dpp.c | 44 +++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 41 insertions(+), 3 deletions(-)
> 

All 4 applied, but one comment:

> diff --git a/src/dpp.c b/src/dpp.c
> index 06ae2929..a95f93e2 100644
> --- a/src/dpp.c
> +++ b/src/dpp.c
> @@ -853,13 +853,42 @@ static bool dpp_scan_results(int err, struct l_queue *bss_list,
>   {
>   	struct dpp_sm *dpp = userdata;
>   	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
> +	struct scan_bss *bss;
> +	char ssid[33];
> +	struct network *network;
>   
>   	if (err < 0)
> -		return false;
> +		goto reset;
> +
> +	if (!bss_list || l_queue_length(bss_list) == 0)
> +		goto reset;
> +
> +	/*
> +	 * The station watch _should_ detect this and reset, which cancels the
> +	 * scan. But just in case...
> +	 */
> +	if (L_WARN_ON(station_get_connected_network(station)))
> +		goto reset;
> +
> +	/* Purely for grabbing the SSID */
> +	bss = l_queue_peek_head(bss_list);
>   
> -	station_set_scan_results(station, bss_list, freqs, true);
> +	memcpy(ssid, bss->ssid, bss->ssid_len);
> +	ssid[bss->ssid_len] = '\0';

Are you sure this SSID is UTF8? station_set_scan_results will filter any SSIDs 
that are not.

> +
> +	station_set_scan_results(station, bss_list, freqs, false);
> +
> +	network = station_network_find(station, ssid, SECURITY_PSK);

And so this can fail.

> +
> +	dpp_reset(dpp);
> +
> +	bss = network_bss_select(network, true);
> +	network_autoconnect(network, bss);
>   
>   	return true;
> +
> +reset:
> +	return false;
>   }
>   
>   static void dpp_scan_destroy(void *userdata)

Regards,
-Denis