PR Feedback

justsmth · Oct 7, 2024 · c9562cb · c9562cb
1 parent 3d0cebf
commit c9562cb
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 6 deletions.
diff --git a/crypto/evp_extra/evp_extra_test.cc b/crypto/evp_extra/evp_extra_test.cc
@@ -1816,13 +1816,15 @@ TEST(EVPExtraTest, DHKeygen) {
 }
 
 TEST(EVPExtraTest, DHParamgen) {
-  std::vector<std::pair<int, int>> test_data({ {1024, 3}, {512, 2}});
+  std::vector<std::pair<int, int>> test_data(
+      {{768, 3}, {512, DH_GENERATOR_2}, {256, DH_GENERATOR_5}});
 
   for (std::pair<int, int> plgen : test_data) {
     const int prime_len = plgen.first;
     const int generator = plgen.second;
     // Construct a EVP_PKEY_CTX
-    bssl::UniquePtr<EVP_PKEY_CTX> ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_DH, nullptr));
+    bssl::UniquePtr<EVP_PKEY_CTX> ctx(
+        EVP_PKEY_CTX_new_id(EVP_PKEY_DH, nullptr));
     ASSERT_TRUE(ctx);
     // Initialize for paramgen
     ASSERT_TRUE(EVP_PKEY_paramgen_init(ctx.get()));
@@ -1836,6 +1838,19 @@ TEST(EVPExtraTest, DHParamgen) {
     ASSERT_TRUE(EVP_PKEY_paramgen(ctx.get(), &raw_pkey));
     EVP_PKEY_free(raw_pkey);
   }
+
+  // Test error conditions
+  const int prime_len = 255;
+  const int generator = 1;
+  // Construct a EVP_PKEY_CTX
+  bssl::UniquePtr<EVP_PKEY_CTX> ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_DH, nullptr));
+  ASSERT_TRUE(ctx);
+  // Initialize for paramgen
+  ASSERT_TRUE(EVP_PKEY_paramgen_init(ctx.get()));
+  // Set the prime length
+  ASSERT_NE(EVP_PKEY_CTX_set_dh_paramgen_prime_len(ctx.get(), prime_len), 1);
+  // Set the generator
+  ASSERT_NE(EVP_PKEY_CTX_set_dh_paramgen_generator(ctx.get(), generator), 1);
 }
 
 // Test that |EVP_PKEY_keygen| works for Ed25519.

diff --git a/crypto/evp_extra/p_dh.c b/crypto/evp_extra/p_dh.c
@@ -31,8 +31,9 @@ static int pkey_dh_init(EVP_PKEY_CTX *ctx) {
   if (dctx == NULL) {
     return 0;
   }
+  // Default parameters
   dctx->prime_len = 2048;
-  dctx->generator = 2;
+  dctx->generator = DH_GENERATOR_2;
 
   ctx->data = dctx;
   return 1;
@@ -129,6 +130,9 @@ static int pkey_dh_ctrl(EVP_PKEY_CTX *ctx, int type, int p1, void *_p2) {
       return 1;
 
     case EVP_PKEY_CTRL_DH_PARAMGEN_GENERATOR:
+      if(p1 < 2) {
+        return -2;
+      }
       dctx->generator = p1;
       return 1;
 
@@ -140,15 +144,15 @@ static int pkey_dh_ctrl(EVP_PKEY_CTX *ctx, int type, int p1, void *_p2) {
 
 static int pkey_dh_paramgen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey) {
   DH_PKEY_CTX *dctx = ctx->data;
-
   DH *dh = DH_new();
   if (dh == NULL) {
     return 0;
   }
   int ret = DH_generate_parameters_ex(dh, dctx->prime_len, dctx->generator, NULL);
-  if (ret) {
+  if (ret == 1) {
     EVP_PKEY_assign_DH(pkey, dh);
   } else {
+    ret = 0;
     DH_free(dh);
   }
   return ret;

diff --git a/include/openssl/evp.h b/include/openssl/evp.h
@@ -183,11 +183,14 @@ OPENSSL_EXPORT DH *EVP_PKEY_get1_DH(const EVP_PKEY *pkey);
 
 // EVP_PKEY_CTX_set_dh_paramgen_prime_len sets the length of the DH prime
 // parameter p for DH parameter generation. If this function is not called,
-// the default length of 2048 is used.
+// the default length of 2048 is used. |pbits| must be greater than or equal
+// to 256. Returns 1 on success, otherwise returns a non-positive value.
 OPENSSL_EXPORT int EVP_PKEY_CTX_set_dh_paramgen_prime_len(EVP_PKEY_CTX *ctx, int pbits);
 
 // EVP_PKEY_CTX_set_dh_paramgen_generator sets the DH generator for DH parameter
 // generation. If this function is not called, the default value of 2 is used.
+// |gen| must be greater than 1. Returns 1 on success, otherwise returns a
+// non-positive value.
 OPENSSL_EXPORT int EVP_PKEY_CTX_set_dh_paramgen_generator(EVP_PKEY_CTX *ctx, int gen);
 
 #define EVP_PKEY_NONE NID_undef