SAE: Fix KCK, PMK, and PMKID derivation for groups 22, 23, 24

IEEE Std 802.11-2016 is not exactly clear on the encoding of the bit
string that is needed for KCK, PMK, and PMKID derivation, but it seems
to make most sense to encode the (commit-scalar + peer-commit-scalar)
mod r part as a bit string by zero padding it from left to the length of
the order (in full octets).

The previous implementation used the length of the prime (in full
octets). This would work for KCK/PMK, but this results in deriving all
zero PMKIDs for the groups where the size of the order is smaller than
the size of the prime. This is the case for groups 22, 23, and 24.
However, those groups have been marked as being unsuitable for use with
SAE, so this fix should not really have a practical impact anymore.
Anyway, better fix it and document this clearly in the implementation
taken into account the unclarity of the standard in this area.

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

diff --git a/src/common/sae.c b/src/common/sae.c
index 2d520939a8741eb2d54e56994722209b1b7ab3b2..5614fe960cd4a171a24e7d98895a67268bfc2e2d 100644 (file)
--- a/src/common/sae.c
+++ b/src/common/sae.c
@@ -45,6 +45,7 @@ int sae_set_group(struct sae_data *sae, int group)
                sae->group = group;
                tmp->prime_len = crypto_ec_prime_len(tmp->ec);
                tmp->prime = crypto_ec_get_prime(tmp->ec);
+               tmp->order_len = crypto_ec_order_len(tmp->ec);
                tmp->order = crypto_ec_get_order(tmp->ec);
                return 0;
        }
@@ -709,10 +710,16 @@ static int sae_derive_keys(struct sae_data *sae, const u8 *k)
        crypto_bignum_add(sae->tmp->own_commit_scalar, sae->peer_commit_scalar,
                          tmp);
        crypto_bignum_mod(tmp, sae->tmp->order, tmp);
-       crypto_bignum_to_bin(tmp, val, sizeof(val), sae->tmp->prime_len);
+       /* IEEE Std 802.11-2016 is not exactly clear on the encoding of the bit
+        * string that is needed for KCK, PMK, and PMKID derivation, but it
+        * seems to make most sense to encode the
+        * (commit-scalar + peer-commit-scalar) mod r part as a bit string by
+        * zero padding it from left to the length of the order (in full
+        * octets). */
+       crypto_bignum_to_bin(tmp, val, sizeof(val), sae->tmp->order_len);
        wpa_hexdump(MSG_DEBUG, "SAE: PMKID", val, SAE_PMKID_LEN);
        if (sha256_prf(keyseed, sizeof(keyseed), "SAE KCK and PMK",
-                      val, sae->tmp->prime_len, keys, sizeof(keys)) < 0)
+                      val, sae->tmp->order_len, keys, sizeof(keys)) < 0)
                goto fail;
        os_memset(keyseed, 0, sizeof(keyseed));
        os_memcpy(sae->tmp->kck, keys, SAE_KCK_LEN);

diff --git a/src/common/sae.h b/src/common/sae.h
index 3eb6e323a68ffb2e019d07e6cfff4574cf71f2d1..10f9302e3d63e68c98a41ba2358b0e8f03fbfa65 100644 (file)
--- a/src/common/sae.h
+++ b/src/common/sae.h
@@ -33,6 +33,7 @@ struct sae_temporary_data {
        struct crypto_bignum *sae_rand;
        struct crypto_ec *ec;
        int prime_len;
+       int order_len;
        const struct dh_group *dh;
        const struct crypto_bignum *prime;
        const struct crypto_bignum *order;