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

Commit Message

John Brandt May 6, 2024, 12:30 a.m. UTC

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(-)

Comments

Denis Kenzior May 7, 2024, 3:11 p.m. UTC | #1

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

Patch

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;
+
+	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) ||
+	    (!sm->handshake->authenticator && l_get_le16(frame) != sm->group))
 		return sae_reject(sm, SAE_STATE_COMMITTED,
 				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP);