SAE: Reject commit-scalar value 1

IEEE Std 802.11-2012 description of SAE does not require this, i.e., it
describes the requirement as 0 < scalar < r for processing the Commit
message. However, this is not correct and will be changes to 1 < scalar
< r to match the Dragonfly description so that a trivial secret case
will be avoided explicitly.

This is not much of an issue for the locally generated commit-scalar
since it would be very unlikely to get the value of 1. For Commit
message processing, a peer with knowledge of the password could
potentially force the exchange to expose key material without this
check.

Signed-off-by: Jouni Malinen <j@w1.fi>

diff --git a/src/common/sae.c b/src/common/sae.c
index eb5157bc19339517ddf9c7af85ada66911f2a564..1d9997599aeaa154fcca6f30460b870f59e4d2dc 100644 (file)
--- a/src/common/sae.c
+++ b/src/common/sae.c
@@ -650,23 +650,38 @@ static int sae_derive_commit(struct sae_data *sae)
 {
        struct crypto_bignum *mask;
        int ret = -1;
+       unsigned int counter = 0;
 
-       mask = sae_get_rand_and_mask(sae);
-       if (mask == NULL) {
-               wpa_printf(MSG_DEBUG, "SAE: Could not get rand/mask");
-               return -1;
-       }
+       do {
+               counter++;
+               if (counter > 100) {
+                       /*
+                        * This cannot really happen in practice if the random
+                        * number generator is working. Anyway, to avoid even a
+                        * theoretical infinite loop, break out after 100
+                        * attemps.
+                        */
+                       return -1;
+               }
 
-       /* commit-scalar = (rand + mask) modulo r */
-       if (!sae->tmp->own_commit_scalar) {
-               sae->tmp->own_commit_scalar = crypto_bignum_init();
-               if (!sae->tmp->own_commit_scalar)
-                       goto fail;
-       }
-       crypto_bignum_add(sae->tmp->sae_rand, mask,
-                         sae->tmp->own_commit_scalar);
-       crypto_bignum_mod(sae->tmp->own_commit_scalar, sae->tmp->order,
-                         sae->tmp->own_commit_scalar);
+               mask = sae_get_rand_and_mask(sae);
+               if (mask == NULL) {
+                       wpa_printf(MSG_DEBUG, "SAE: Could not get rand/mask");
+                       return -1;
+               }
+
+               /* commit-scalar = (rand + mask) modulo r */
+               if (!sae->tmp->own_commit_scalar) {
+                       sae->tmp->own_commit_scalar = crypto_bignum_init();
+                       if (!sae->tmp->own_commit_scalar)
+                               goto fail;
+               }
+               crypto_bignum_add(sae->tmp->sae_rand, mask,
+                                 sae->tmp->own_commit_scalar);
+               crypto_bignum_mod(sae->tmp->own_commit_scalar, sae->tmp->order,
+                                 sae->tmp->own_commit_scalar);
+       } while (crypto_bignum_is_zero(sae->tmp->own_commit_scalar) ||
+                crypto_bignum_is_one(sae->tmp->own_commit_scalar));
 
        if ((sae->tmp->ec && sae_derive_commit_element_ecc(sae, mask) < 0) ||
            (sae->tmp->dh && sae_derive_commit_element_ffc(sae, mask) < 0))
@@ -954,8 +969,9 @@ static u16 sae_parse_commit_scalar(struct sae_data *sae, const u8 **pos,
                return WLAN_STATUS_UNSPECIFIED_FAILURE;
        }
 
-       /* 0 < scalar < r */
+       /* 1 < scalar < r */
        if (crypto_bignum_is_zero(peer_scalar) ||
+           crypto_bignum_is_one(peer_scalar) ||
            crypto_bignum_cmp(peer_scalar, sae->tmp->order) >= 0) {
                wpa_printf(MSG_DEBUG, "SAE: Invalid peer scalar");
                crypto_bignum_deinit(peer_scalar, 0);