Re: [PATCH 04/10] network: add support for SAE password identifiers

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

On 12/5/23 09:46, James Prestwood wrote:
> Adds a new network profile setting [Security].PasswordIdentifier.
> When set (and the BSS enables SAE password identifiers) the network
> and handshake object will read this and use it for the SAE
> exchange.
> 
> Loading the PSK will fail if there is no password identifier set
> and the BSS sets the "exclusive" bit. If a password identifier is

I'm not so sure about this.  The trouble is that this logic is sufficient for 
the initial connection, but isn't sufficient when you consider re-association.

> set and the BSS doesn't indicate support the setting will be ignored
> (with a debug print).
> ---
>   src/network.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 49 insertions(+), 1 deletion(-)
> 

<snip>

> @@ -641,6 +657,32 @@ static int network_load_psk(struct network *network, struct scan_bss *bss)
>   		psk_len = 0;
>   	}
>   
> +	/*
> +	 * Sort out if the password identifier is required, should be used, "
> +	 * or should be ignored.
> +	 */
> +	if (is_sae) {
> +		if (bss->sae_pw_id_exclusive && !password_id) {

This likely needs to be taken into consideration much later, when building the 
actual handshake state.

> +			l_error("BSS requires SAE password identifiers, check "
> +				"[Security].PasswordIdentifier");
> +			return -ENOKEY;
> +		}
> +
> +		/*
> +		 * If the profile contains a password identifier but the network
> +		 * does not support it IWD will still attempt to connect. The

Password identifier is only used by SAE H2E.  One can easily imagine a weird 
network of mixed APs with some being SAE H2E, some not.  This is why I didn't 
bother implementing this, it is a half-baked feature.

> +		 * caveat here is if the connection is successful the sync will
> +		 * remove the password identifier entry. Though this might be
> +		 * unexpected to the user, retaining this (invalid) setting
> +		 * isn't worth special casing.

And this doesn't sound nice at all...  I think the setting should be preserved.

> +		 */
> +		if (!bss->sae_pw_id_used && password_id) {
> +			l_debug("[Security].PasswordIdentifier set but BSS "
> +				"does not not use password identifiers");
> +			l_free(l_steal_ptr(password_id));
> +		}
> +	}
> +
>   	/* PSK can be generated from the passphrase but not the other way */
>   	if (!psk || is_sae) {
>   		if (!passphrase)

Regards,
-Denis