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>
This commit is contained in:
Jouni Malinen 2019-03-05 17:05:03 +02:00 committed by Jouni Malinen
parent 4a9531a755
commit b11fa98bcb
9 changed files with 190 additions and 12 deletions

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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