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>

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

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,
+	gcry_mpi_t pub = NULL;
-			      prime, prime_len, secret, len);
+	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;
 }
 
 

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,
+	struct bignum *pub;
-			      prime, prime_len, secret, len);
+	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;
 }
 
 

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);
 }

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,
+	mpz_t pub;
-			      prime, prime_len, secret, len);
+	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;
 }
 
 

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,
+	BIGNUM *pub, *p;
-			      prime, prime_len, secret, len);
+	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 ||
+	q = get_group5_order();
-	    DH_set0_pqg(dh, p, NULL, g) != 1)
+	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);

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)

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

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