diff --git a/crypto/dh_extra/dh_test.cc b/crypto/dh_extra/dh_test.cc index 7a17070276..e36880f9e0 100644 --- a/crypto/dh_extra/dh_test.cc +++ b/crypto/dh_extra/dh_test.cc @@ -112,6 +112,54 @@ TEST(DHTest, Basic) { EXPECT_GE(key1.size(), 4u); } +TEST(DHTest, OversizedModulus) { + bssl::UniquePtr a(DH_new()); + ASSERT_TRUE(a); + + const size_t LARGE_MOD_P = 4097; // OPENSSL_DH_CHECK_MAX_MODULUS_BITS / 8 + 1 + + // Create a BigNumber which will be interpreted as a big-endian value + auto number = std::unique_ptr>( + new uint8_t[LARGE_MOD_P]); + for (size_t i = 0; i < LARGE_MOD_P; i++) { + number[i] = 255; + } + + bssl::UniquePtr p(BN_bin2bn(number.get(), LARGE_MOD_P, nullptr)); + bssl::UniquePtr q(BN_new()); + bssl::UniquePtr g(BN_new()); + + // Q and G don't matter for this test, they just can't be null + ASSERT_TRUE(DH_set0_pqg(a.get(), p.release(), q.release(), g.release())); + + int check_result; + ASSERT_FALSE(DH_check(a.get(), &check_result)); + uint32_t error = ERR_get_error(); + ASSERT_EQ(ERR_LIB_DH, ERR_GET_LIB(error)); + ASSERT_EQ(DH_R_MODULUS_TOO_LARGE, ERR_GET_REASON(error)); +} + +TEST(DHTest, LargeQ) { + bssl::UniquePtr a(DH_new()); + ASSERT_TRUE(a); + ASSERT_TRUE(DH_generate_parameters_ex(a.get(), 64, DH_GENERATOR_5, nullptr)); + + bssl::UniquePtr q(BN_new()); + ASSERT_TRUE(q); + BN_set_word(q.get(), 2039L); + + a.get()->q = q.release(); + + ASSERT_TRUE(DH_generate_key(a.get())); + + ASSERT_TRUE(BN_copy(a.get()->q, a.get()->p)); + ASSERT_TRUE(BN_add(a.get()->q, a.get()->q, BN_value_one())); + + int check_result; + ASSERT_TRUE(DH_check(a.get(), &check_result)); + ASSERT_TRUE(check_result & DH_CHECK_INVALID_Q_VALUE); +} + // The following parameters are taken from RFC 5114, section 2.2. This is not a // safe prime. Do not use these parameters. static const uint8_t kRFC5114_2048_224P[] = { diff --git a/crypto/fipsmodule/dh/check.c b/crypto/fipsmodule/dh/check.c index 0c82c17f00..85519a5f6e 100644 --- a/crypto/fipsmodule/dh/check.c +++ b/crypto/fipsmodule/dh/check.c @@ -60,6 +60,7 @@ #include "internal.h" +#define OPENSSL_DH_CHECK_MAX_MODULUS_BITS 32768 int DH_check_pub_key(const DH *dh, const BIGNUM *pub_key, int *out_flags) { *out_flags = 0; @@ -119,12 +120,19 @@ int DH_check(const DH *dh, int *out_flags) { // for 3, p mod 12 == 5 // for 5, p mod 10 == 3 or 7 // should hold. - int ok = 0, r; + int ok = 0, r, q_good = 0; BN_CTX *ctx = NULL; BN_ULONG l; BIGNUM *t1 = NULL, *t2 = NULL; *out_flags = 0; + + /* Don't do any checks at all with an excessively large modulus */ + if (BN_num_bits(dh->p) > OPENSSL_DH_CHECK_MAX_MODULUS_BITS) { + OPENSSL_PUT_ERROR(DH, DH_R_MODULUS_TOO_LARGE); + return 0; + } + ctx = BN_CTX_new(); if (ctx == NULL) { goto err; @@ -140,6 +148,14 @@ int DH_check(const DH *dh, int *out_flags) { } if (dh->q) { + if (BN_ucmp(dh->p, dh->q) > 0) { + q_good = 1; + } else { + *out_flags |= DH_CHECK_INVALID_Q_VALUE; + } + } + + if (q_good) { if (BN_cmp(dh->g, BN_value_one()) <= 0) { *out_flags |= DH_CHECK_NOT_SUITABLE_GENERATOR; } else if (BN_cmp(dh->g, dh->p) >= 0) {