From 1bb574f3f2e758124b2a7acac6550ec0c63d9970 Mon Sep 17 00:00:00 2001
From: Sean McGrail <549813+skmcgrail@users.noreply.github.com>
Date: Tue, 1 Aug 2023 10:02:01 -0700
Subject: [PATCH] Fix Excessive time spent checking DH q parameter value
 (#1121)

---
 crypto/dh_extra/dh_test.cc   | 21 +++++++++++++++++++++
 crypto/fipsmodule/dh/check.c | 10 +++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/crypto/dh_extra/dh_test.cc b/crypto/dh_extra/dh_test.cc
index 4a9c9c3fcc..f9151f3313 100644
--- a/crypto/dh_extra/dh_test.cc
+++ b/crypto/dh_extra/dh_test.cc
@@ -140,6 +140,27 @@ TEST(DHTest, OversizedModulus) {
   ASSERT_EQ(DH_R_MODULUS_TOO_LARGE, ERR_GET_REASON(error));
 }
 
+TEST(DHTest, LargeQ) {
+  bssl::UniquePtr<DH> a(DH_new());
+  ASSERT_TRUE(a);
+  ASSERT_TRUE(DH_generate_parameters_ex(a.get(), 64, DH_GENERATOR_5, nullptr));
+
+  bssl::UniquePtr<BIGNUM> 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 8a7c142581..85519a5f6e 100644
--- a/crypto/fipsmodule/dh/check.c
+++ b/crypto/fipsmodule/dh/check.c
@@ -120,7 +120,7 @@ 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;
@@ -148,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) {