Re: [PATCH v2 07/18] sae: verify offered group in AP mode

[Denis Kenzior <denkenz@gmail.com>]

Hi John,

On 5/5/24 7:30 PM, John Brandt wrote:
> When receiving a Commit frame in AP mode, first verify that we support
> the offered group before further processing the frame.
> ---
>   src/sae.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/src/sae.c b/src/sae.c
> index 7ba9b0eb..7405a561 100644
> --- a/src/sae.c
> +++ b/src/sae.c
> @@ -216,6 +216,18 @@ static int sae_valid_group(struct sae_sm *sm, unsigned int group)
>   	return -ENOENT;
>   }
>   
> +static int sae_supported_group(struct sae_sm *sm, unsigned int group)
> +{
> +	const unsigned int *ecc_groups = l_ecc_supported_ike_groups();
> +	unsigned int i;
> +
> +	for (i = 0; ecc_groups[i]; i++)
> +		if (ecc_groups[i] == group)
> +			return true;

Function declared as returning int, but you're returning true/false here.

> +
> +	return false;
> +}
> +
>   static bool sae_pwd_seed(const uint8_t *addr1, const uint8_t *addr2,
>   				uint8_t *base, size_t base_len,
>   				uint8_t counter, uint8_t *out)
> @@ -1053,7 +1065,8 @@ static int sae_verify_nothing(struct sae_sm *sm, uint16_t transaction,
>   		return -EBADMSG;
>   
>   	/* reject with unsupported group */
> -	if (l_get_le16(frame) != sm->group)
> +	if ((sm->handshake->authenticator && sae_supported_group(sm, l_get_le16(frame)) < 0) ||

nit: We still use 80 column lines.  This line is way too long.

Also, this if condition will never be true due to sae_supported_group returning 
true/false.

> +	    (!sm->handshake->authenticator && l_get_le16(frame) != sm->group))
>   		return sae_reject(sm, SAE_STATE_COMMITTED,
>   				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP);
>   

Regards,
-Denis