Add explicit checks for peer's DH public key

Pass the group order (if known/specified) to crypto_dh_derive_secret()
(and also to OpenSSL DH_generate_key() in case of Group 5) and verify
that the public key received from the peer meets 1 < pubkey < p and
pubkey^q == 1 mod p conditions.

While all these use cases were using only ephemeral DH keys, it is
better to use more explicit checks while deriving the shared secret to
avoid unexpected behavior.

Signed-off-by: Jouni Malinen <jouni@codeaurora.org>

diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h
index 507b7cab86fc7f03c8e2979035ae864b95c50791..a28ddbd719d66cb440553f3232331e1c53485e64 100644 (file)
--- a/src/crypto/crypto.h
+++ b/src/crypto/crypto.h
@@ -420,6 +420,7 @@ int __must_check crypto_public_key_decrypt_pkcs1(
 int crypto_dh_init(u8 generator, const u8 *prime, size_t prime_len, u8 *privkey,
                   u8 *pubkey);
 int crypto_dh_derive_secret(u8 generator, const u8 *prime, size_t prime_len,
+                           const u8 *order, size_t order_len,
                            const u8 *privkey, size_t privkey_len,
                            const u8 *pubkey, size_t pubkey_len,
                            u8 *secret, size_t *len);

diff --git a/src/crypto/crypto_gnutls.c b/src/crypto/crypto_gnutls.c
index 7a797b5c359d10e8f120d5509fcdc96672efd6b8..4ef11462b36e82d7dd5e5cfabb4496c681e4d179 100644 (file)
--- a/src/crypto/crypto_gnutls.c
+++ b/src/crypto/crypto_gnutls.c
@@ -310,12 +310,51 @@ int crypto_dh_init(u8 generator, const u8 *prime, size_t prime_len, u8 *privkey,
 
 
 int crypto_dh_derive_secret(u8 generator, const u8 *prime, size_t prime_len,
+                           const u8 *order, size_t order_len,
                            const u8 *privkey, size_t privkey_len,
                            const u8 *pubkey, size_t pubkey_len,
                            u8 *secret, size_t *len)
 {
-       return crypto_mod_exp(pubkey, pubkey_len, privkey, privkey_len,
-                             prime, prime_len, secret, len);
+       gcry_mpi_t pub = NULL;
+       int res = -1;
+
+       if (pubkey_len > prime_len ||
+           (pubkey_len == prime_len &&
+            os_memcmp(pubkey, prime, prime_len) >= 0))
+               return -1;
+
+       if (gcry_mpi_scan(&pub, GCRYMPI_FMT_USG, pubkey, pubkey_len, NULL) !=
+           GPG_ERR_NO_ERROR ||
+           gcry_mpi_cmp_ui(pub, 1) <= 0)
+               goto fail;
+
+       if (order) {
+               gcry_mpi_t p = NULL, q = NULL, tmp;
+               int failed;
+
+               /* verify: pubkey^q == 1 mod p */
+               tmp = gcry_mpi_new(prime_len * 8);
+               failed = !tmp ||
+                       gcry_mpi_scan(&p, GCRYMPI_FMT_USG, prime, prime_len,
+                                     NULL) != GPG_ERR_NO_ERROR ||
+                       gcry_mpi_scan(&q, GCRYMPI_FMT_USG, order, order_len,
+                                     NULL) != GPG_ERR_NO_ERROR;
+               if (!failed) {
+                       gcry_mpi_powm(tmp, pub, q, p);
+                       failed = gcry_mpi_cmp_ui(tmp, 1) != 0;
+               }
+               gcry_mpi_release(p);
+               gcry_mpi_release(q);
+               gcry_mpi_release(tmp);
+               if (failed)
+                       goto fail;
+       }
+
+       res = crypto_mod_exp(pubkey, pubkey_len, privkey, privkey_len,
+                            prime, prime_len, secret, len);
+fail:
+       gcry_mpi_release(pub);
+       return res;
 }
 
 

diff --git a/src/crypto/crypto_internal-modexp.c b/src/crypto/crypto_internal-modexp.c
index 92581ac676d31883a58951d5681940ce7823c1a0..6819f1a6ab6a7149f223503eb3e882b1dce02e58 100644 (file)
--- a/src/crypto/crypto_internal-modexp.c
+++ b/src/crypto/crypto_internal-modexp.c
@@ -40,12 +40,49 @@ int crypto_dh_init(u8 generator, const u8 *prime, size_t prime_len, u8 *privkey,
 
 
 int crypto_dh_derive_secret(u8 generator, const u8 *prime, size_t prime_len,
+                           const u8 *order, size_t order_len,
                            const u8 *privkey, size_t privkey_len,
                            const u8 *pubkey, size_t pubkey_len,
                            u8 *secret, size_t *len)
 {
-       return crypto_mod_exp(pubkey, pubkey_len, privkey, privkey_len,
-                             prime, prime_len, secret, len);
+       struct bignum *pub;
+       int res = -1;
+
+       if (pubkey_len > prime_len ||
+           (pubkey_len == prime_len &&
+            os_memcmp(pubkey, prime, prime_len) >= 0))
+               return -1;
+
+       pub = bignum_init();
+       if (!pub || bignum_set_unsigned_bin(pub, pubkey, pubkey_len) < 0 ||
+           bignum_cmp_d(pub, 1) <= 0)
+               goto fail;
+
+       if (order) {
+               struct bignum *p, *q, *tmp;
+               int failed;
+
+               /* verify: pubkey^q == 1 mod p */
+               p = bignum_init();
+               q = bignum_init();
+               tmp = bignum_init();
+               failed = !p || !q || !tmp ||
+                       bignum_set_unsigned_bin(p, prime, prime_len) < 0 ||
+                       bignum_set_unsigned_bin(q, order, order_len) < 0 ||
+                       bignum_exptmod(pub, q, p, tmp) < 0 ||
+                       bignum_cmp_d(tmp, 1) != 0;
+               bignum_deinit(p);
+               bignum_deinit(q);
+               bignum_deinit(tmp);
+               if (failed)
+                       goto fail;
+       }
+
+       res = crypto_mod_exp(pubkey, pubkey_len, privkey, privkey_len,
+                            prime, prime_len, secret, len);
+fail:
+       bignum_deinit(pub);
+       return res;
 }
 
 

diff --git a/src/crypto/crypto_libtomcrypt.c b/src/crypto/crypto_libtomcrypt.c
index 259f99500bcd3b69823efd0b5d12b7e97a6a08c2..980fa42274f6cb9e541a7540c68676c2d5d1aade 100644 (file)
--- a/src/crypto/crypto_libtomcrypt.c
+++ b/src/crypto/crypto_libtomcrypt.c
@@ -721,10 +721,12 @@ int crypto_dh_init(u8 generator, const u8 *prime, size_t prime_len, u8 *privkey,
 
 
 int crypto_dh_derive_secret(u8 generator, const u8 *prime, size_t prime_len,
+                           const u8 *order, size_t order_len,
                            const u8 *privkey, size_t privkey_len,
                            const u8 *pubkey, size_t pubkey_len,
                            u8 *secret, size_t *len)
 {
+       /* TODO: check pubkey */
        return crypto_mod_exp(pubkey, pubkey_len, privkey, privkey_len,
                              prime, prime_len, secret, len);
 }

diff --git a/src/crypto/crypto_nettle.c b/src/crypto/crypto_nettle.c
index 4e31bc8011325af6e60877a4a7ed5d06313dff01..f85d36532ea1921b0373f177765e8ad070a584f0 100644 (file)
--- a/src/crypto/crypto_nettle.c
+++ b/src/crypto/crypto_nettle.c
@@ -331,12 +331,44 @@ int crypto_dh_init(u8 generator, const u8 *prime, size_t prime_len, u8 *privkey,
 
 
 int crypto_dh_derive_secret(u8 generator, const u8 *prime, size_t prime_len,
+                           const u8 *order, size_t order_len,
                            const u8 *privkey, size_t privkey_len,
                            const u8 *pubkey, size_t pubkey_len,
                            u8 *secret, size_t *len)
 {
-       return crypto_mod_exp(pubkey, pubkey_len, privkey, privkey_len,
-                             prime, prime_len, secret, len);
+       mpz_t pub;
+       int res = -1;
+
+       if (pubkey_len > prime_len ||
+           (pubkey_len == prime_len &&
+            os_memcmp(pubkey, prime, prime_len) >= 0))
+               return -1;
+
+       mpz_init(pub);
+       mpz_import(pub, pubkey_len, 1, 1, 1, 0, pubkey);
+       if (mpz_cmp_d(pub, 1) <= 0)
+               goto fail;
+
+       if (order) {
+               mpz_t p, q, tmp;
+               int failed;
+
+               /* verify: pubkey^q == 1 mod p */
+               mpz_inits(p, q, tmp, NULL);
+               mpz_import(p, prime_len, 1, 1, 1, 0, prime);
+               mpz_import(q, order_len, 1, 1, 1, 0, order);
+               mpz_powm(tmp, pub, q, p);
+               failed = mpz_cmp_d(tmp, 1) != 0;
+               mpz_clears(p, q, tmp, NULL);
+               if (failed)
+                       goto fail;
+       }
+
+       res = crypto_mod_exp(pubkey, pubkey_len, privkey, privkey_len,
+                            prime, prime_len, secret, len);
+fail:
+       mpz_clear(pub);
+       return res;
 }
 
 

diff --git a/src/crypto/crypto_openssl.c b/src/crypto/crypto_openssl.c
index f89053a89d67efbd6905402a1725d3639d94d0e9..9c2ba58d5b65c82dd8451ca6adabee8569690fdd 100644 (file)
--- a/src/crypto/crypto_openssl.c
+++ b/src/crypto/crypto_openssl.c
@@ -111,6 +111,31 @@ static BIGNUM * get_group5_prime(void)
 #endif
 }
 
+
+static BIGNUM * get_group5_order(void)
+{
+       static const unsigned char RFC3526_ORDER_1536[] = {
+               0x7F,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xE4,0x87,0xED,0x51,
+               0x10,0xB4,0x61,0x1A,0x62,0x63,0x31,0x45,0xC0,0x6E,0x0E,0x68,
+               0x94,0x81,0x27,0x04,0x45,0x33,0xE6,0x3A,0x01,0x05,0xDF,0x53,
+               0x1D,0x89,0xCD,0x91,0x28,0xA5,0x04,0x3C,0xC7,0x1A,0x02,0x6E,
+               0xF7,0xCA,0x8C,0xD9,0xE6,0x9D,0x21,0x8D,0x98,0x15,0x85,0x36,
+               0xF9,0x2F,0x8A,0x1B,0xA7,0xF0,0x9A,0xB6,0xB6,0xA8,0xE1,0x22,
+               0xF2,0x42,0xDA,0xBB,0x31,0x2F,0x3F,0x63,0x7A,0x26,0x21,0x74,
+               0xD3,0x1B,0xF6,0xB5,0x85,0xFF,0xAE,0x5B,0x7A,0x03,0x5B,0xF6,
+               0xF7,0x1C,0x35,0xFD,0xAD,0x44,0xCF,0xD2,0xD7,0x4F,0x92,0x08,
+               0xBE,0x25,0x8F,0xF3,0x24,0x94,0x33,0x28,0xF6,0x72,0x2D,0x9E,
+               0xE1,0x00,0x3E,0x5C,0x50,0xB1,0xDF,0x82,0xCC,0x6D,0x24,0x1B,
+               0x0E,0x2A,0xE9,0xCD,0x34,0x8B,0x1F,0xD4,0x7E,0x92,0x67,0xAF,
+               0xC1,0xB2,0xAE,0x91,0xEE,0x51,0xD6,0xCB,0x0E,0x31,0x79,0xAB,
+               0x10,0x42,0xA9,0x5D,0xCF,0x6A,0x94,0x83,0xB8,0x4B,0x4B,0x36,
+               0xB3,0x86,0x1A,0xA7,0x25,0x5E,0x4C,0x02,0x78,0xBA,0x36,0x04,
+               0x65,0x11,0xB9,0x93,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF
+       };
+       return BN_bin2bn(RFC3526_ORDER_1536, sizeof(RFC3526_ORDER_1536), NULL);
+}
+
+
 #ifdef OPENSSL_NO_SHA256
 #define NO_SHA256_WRAPPER
 #endif
@@ -518,12 +543,45 @@ int crypto_dh_init(u8 generator, const u8 *prime, size_t prime_len, u8 *privkey,
 
 
 int crypto_dh_derive_secret(u8 generator, const u8 *prime, size_t prime_len,
+                           const u8 *order, size_t order_len,
                            const u8 *privkey, size_t privkey_len,
                            const u8 *pubkey, size_t pubkey_len,
                            u8 *secret, size_t *len)
 {
-       return crypto_mod_exp(pubkey, pubkey_len, privkey, privkey_len,
-                             prime, prime_len, secret, len);
+       BIGNUM *pub, *p;
+       int res = -1;
+
+       pub = BN_bin2bn(pubkey, pubkey_len, NULL);
+       p = BN_bin2bn(prime, prime_len, NULL);
+       if (!pub || !p || BN_is_zero(pub) || BN_is_one(pub) ||
+           BN_cmp(pub, p) >= 0)
+               goto fail;
+
+       if (order) {
+               BN_CTX *ctx;
+               BIGNUM *q, *tmp;
+               int failed;
+
+               /* verify: pubkey^q == 1 mod p */
+               q = BN_bin2bn(order, order_len, NULL);
+               ctx = BN_CTX_new();
+               tmp = BN_new();
+               failed = !q || !ctx || !tmp ||
+                       !BN_mod_exp(tmp, pub, q, p, ctx) ||
+                       !BN_is_one(tmp);
+               BN_clear(q);
+               BN_clear(tmp);
+               BN_CTX_free(ctx);
+               if (failed)
+                       goto fail;
+       }
+
+       res = crypto_mod_exp(pubkey, pubkey_len, privkey, privkey_len,
+                            prime, prime_len, secret, len);
+fail:
+       BN_clear(pub);
+       BN_clear(p);
+       return res;
 }
 
 
@@ -709,6 +767,10 @@ void * dh5_init(struct wpabuf **priv, struct wpabuf **publ)
        if (dh->p == NULL)
                goto err;
 
+       dh->q = get_group5_order();
+       if (!dh->q)
+               goto err;
+
        if (DH_generate_key(dh) != 1)
                goto err;
 
@@ -737,7 +799,7 @@ err:
        DH *dh;
        struct wpabuf *pubkey = NULL, *privkey = NULL;
        size_t publen, privlen;
-       BIGNUM *p = NULL, *g;
+       BIGNUM *p, *g, *q;
        const BIGNUM *priv_key = NULL, *pub_key = NULL;
 
        *priv = NULL;
@@ -750,10 +812,12 @@ err:
 
        g = BN_new();
        p = get_group5_prime();
-       if (!g || BN_set_word(g, 2) != 1 || !p ||
-           DH_set0_pqg(dh, p, NULL, g) != 1)
+       q = get_group5_order();
+       if (!g || BN_set_word(g, 2) != 1 || !p || !q ||
+           DH_set0_pqg(dh, p, q, g) != 1)
                goto err;
        p = NULL;
+       q = NULL;
        g = NULL;
 
        if (DH_generate_key(dh) != 1)
@@ -778,6 +842,7 @@ err:
 
 err:
        BN_free(p);
+       BN_free(q);
        BN_free(g);
        wpabuf_clear_free(pubkey);
        wpabuf_clear_free(privkey);

diff --git a/src/crypto/crypto_wolfssl.c b/src/crypto/crypto_wolfssl.c
index b5a1e3fa31bc1ba58cd4e50e691876d2f4b78686..10cdae6048670d0c316adb164b4278cb48db27b1 100644 (file)
--- a/src/crypto/crypto_wolfssl.c
+++ b/src/crypto/crypto_wolfssl.c
@@ -826,6 +826,7 @@ done:
 
 
 int crypto_dh_derive_secret(u8 generator, const u8 *prime, size_t prime_len,
+                           const u8 *order, size_t order_len,
                            const u8 *privkey, size_t privkey_len,
                            const u8 *pubkey, size_t pubkey_len,
                            u8 *secret, size_t *len)

diff --git a/src/crypto/dh_groups.c b/src/crypto/dh_groups.c
index a9b770ec1f160bf0d283efd2510849506a6ecab7..5e421b24f5a561edbb77c5029ceab56eb31bee13 100644 (file)
--- a/src/crypto/dh_groups.c
+++ b/src/crypto/dh_groups.c
@@ -1249,6 +1249,7 @@ struct wpabuf * dh_derive_shared(const struct wpabuf *peer_public,
        if (shared == NULL)
                return NULL;
        if (crypto_dh_derive_secret(*dh->generator, dh->prime, dh->prime_len,
+                                   dh->order, dh->order_len,
                                    wpabuf_head(own_private),
                                    wpabuf_len(own_private),
                                    wpabuf_head(peer_public),

diff --git a/src/eap_common/eap_eke_common.c b/src/eap_common/eap_eke_common.c
index bfe8811e33f27f367449f31ca5b53b6caf3d79ab..438baf190be71a2f8284434aa08f4442633ec515 100644 (file)
--- a/src/eap_common/eap_eke_common.c
+++ b/src/eap_common/eap_eke_common.c
@@ -399,7 +399,7 @@ int eap_eke_shared_secret(struct eap_eke_session *sess, const u8 *key,
        /* SharedSecret = prf(0+, g ^ (x_s * x_p) (mod p)) */
        len = dh->prime_len;
        if (crypto_dh_derive_secret(*dh->generator, dh->prime, dh->prime_len,
-                                   dhpriv, dh->prime_len, peer_pub,
+                                   NULL, 0, dhpriv, dh->prime_len, peer_pub,
                                    dh->prime_len, modexp, &len) < 0)
                return -1;
        if (len < dh->prime_len) {