Enforce a minimum DH modulus size of 512 bits

[extended tests]

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9437)

9 files changed, 36 insertions, 20 deletions

diff --git a/CHANGES b/CHANGES
index 3507e350e3..acaa099518 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,9 @@
 
  Changes between 1.1.1 and 3.0.0 [xx XXX xxxx]
 
+  *) Enforce a minimum DH modulus size of 512 bits.
+     [Bernd Edlinger]
+
   *) Changed DH parameters to generate the order q subgroup instead of 2q.
      Previously generated DH parameters are still accepted by DH_check
      but DH_generate_key works around that by clearing bit 0 of the
diff --git a/crypto/dh/dh_err.c b/crypto/dh/dh_err.c
index cbde260145..69f1452441 100644
--- a/crypto/dh/dh_err.c
+++ b/crypto/dh/dh_err.c
@@ -41,6 +41,7 @@ static const ERR_STRING_DATA DH_str_reasons[] = {
     {ERR_PACK(ERR_LIB_DH, 0, DH_R_KEYS_NOT_SET), "keys not set"},
     {ERR_PACK(ERR_LIB_DH, 0, DH_R_MISSING_PUBKEY), "missing pubkey"},
     {ERR_PACK(ERR_LIB_DH, 0, DH_R_MODULUS_TOO_LARGE), "modulus too large"},
+    {ERR_PACK(ERR_LIB_DH, 0, DH_R_MODULUS_TOO_SMALL), "modulus too small"},
     {ERR_PACK(ERR_LIB_DH, 0, DH_R_NOT_SUITABLE_GENERATOR),
     "not suitable generator"},
     {ERR_PACK(ERR_LIB_DH, 0, DH_R_NO_PARAMETERS_SET), "no parameters set"},
diff --git a/crypto/dh/dh_gen.c b/crypto/dh/dh_gen.c
index 6e98b59d85..76d6ad018e 100644
--- a/crypto/dh/dh_gen.c
+++ b/crypto/dh/dh_gen.c
@@ -61,6 +61,16 @@ static int dh_builtin_genparams(DH *ret, int prime_len, int generator,
     int g, ok = -1;
     BN_CTX *ctx = NULL;
 
+    if (prime_len > OPENSSL_DH_MAX_MODULUS_BITS) {
+        DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_MODULUS_TOO_LARGE);
+        return 0;
+    }
+
+    if (prime_len < DH_MIN_MODULUS_BITS) {
+        DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_MODULUS_TOO_SMALL);
+        return 0;
+    }
+
     ctx = BN_CTX_new();
     if (ctx == NULL)
         goto err;
diff --git a/crypto/dh/dh_key.c b/crypto/dh/dh_key.c
index 0d6b04de20..8731cc2c73 100644
--- a/crypto/dh/dh_key.c
+++ b/crypto/dh/dh_key.c
@@ -87,6 +87,11 @@ static int generate_key(DH *dh)
         return 0;
     }
 
+    if (BN_num_bits(dh->p) < DH_MIN_MODULUS_BITS) {
+        DHerr(DH_F_GENERATE_KEY, DH_R_MODULUS_TOO_SMALL);
+        return 0;
+    }
+
     ctx = BN_CTX_new();
     if (ctx == NULL)
         goto err;
@@ -181,6 +186,11 @@ static int compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh)
         goto err;
     }
 
+    if (BN_num_bits(dh->p) < DH_MIN_MODULUS_BITS) {
+        DHerr(DH_F_COMPUTE_KEY, DH_R_MODULUS_TOO_SMALL);
+        return 0;
+    }
+
     ctx = BN_CTX_new();
     if (ctx == NULL)
         goto err;
diff --git a/crypto/dh/dh_locl.h b/crypto/dh/dh_locl.h
index f0247b8d7d..a9041e9462 100644
--- a/crypto/dh/dh_locl.h
+++ b/crypto/dh/dh_locl.h
@@ -10,6 +10,8 @@
 #include <openssl/dh.h>
 #include "internal/refcount.h"
 
+#define DH_MIN_MODULUS_BITS     512
+
 struct dh_st {
     /*
      * This first argument is used to pick up errors when a DH is passed
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index d88e98993e..ede1c57a7b 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -2276,6 +2276,7 @@ DH_R_KDF_PARAMETER_ERROR:112:kdf parameter error
 DH_R_KEYS_NOT_SET:108:keys not set
 DH_R_MISSING_PUBKEY:125:missing pubkey
 DH_R_MODULUS_TOO_LARGE:103:modulus too large
+DH_R_MODULUS_TOO_SMALL:126:modulus too small
 DH_R_NOT_SUITABLE_GENERATOR:120:not suitable generator
 DH_R_NO_PARAMETERS_SET:107:no parameters set
 DH_R_NO_PRIVATE_VALUE:100:no private value
diff --git a/doc/man1/dhparam.pod b/doc/man1/dhparam.pod
index dd871b3b48..c51bbaa63e 100644
--- a/doc/man1/dhparam.pod
+++ b/doc/man1/dhparam.pod
@@ -103,8 +103,9 @@ This can be used with a subsequent B<-rand> flag.
 This option specifies that a parameter set should be generated of size
 I<numbits>. It must be the last option. If this option is present then
 the input file is ignored and parameters are generated instead. If
-this option is not present but a generator (B<-2> or B<-5>) is
+this option is not present but a generator (B<-2>, B<-3> or B<-5>) is
 present, parameters are generated with a default length of 2048 bits.
+The minimim length is 512 bits. The maximum length is 10000 bits.
 
 =item B<-noout>
 
diff --git a/include/openssl/dherr.h b/include/openssl/dherr.h
index 1e3451bbe6..13bd0361e0 100644
--- a/include/openssl/dherr.h
+++ b/include/openssl/dherr.h
@@ -80,6 +80,7 @@ int ERR_load_DH_strings(void);
 #  define DH_R_KEYS_NOT_SET                                108
 #  define DH_R_MISSING_PUBKEY                              125
 #  define DH_R_MODULUS_TOO_LARGE                           103
+#  define DH_R_MODULUS_TOO_SMALL                           126
 #  define DH_R_NOT_SUITABLE_GENERATOR                      120
 #  define DH_R_NO_PARAMETERS_SET                           107
 #  define DH_R_NO_PRIVATE_VALUE                            100
diff --git a/test/dhtest.c b/test/dhtest.c
index f80d5b3f4d..662a4f32eb 100644
--- a/test/dhtest.c
+++ b/test/dhtest.c
@@ -103,25 +103,12 @@ static int dh_test(void)
         || !TEST_ptr_eq(DH_get0_priv_key(dh), priv_key2))
         goto err3;
 
-    /* now generate a key pair ... */
-    if (!DH_generate_key(dh))
+    /* now generate a key pair (expect failure since modulus is too small) */
+    if (!TEST_false(DH_generate_key(dh)))
         goto err3;
 
-    /* ... and check whether the private key was reused: */
-
-    /* test it with the combined getter for pub_key and priv_key */
-    DH_get0_key(dh, &pub_key2, &priv_key2);
-    if (!TEST_ptr(pub_key2)
-        || !TEST_ptr_eq(priv_key2, priv_key))
-        goto err3;
-
-    /* test it the simple getters for pub_key and priv_key */
-    if (!TEST_ptr_eq(DH_get0_pub_key(dh), pub_key2)
-        || !TEST_ptr_eq(DH_get0_priv_key(dh), priv_key2))
-        goto err3;
-
-    /* check whether the public key was calculated correctly */
-    TEST_uint_eq(BN_get_word(pub_key2), 3331L);
+    /* We'll have a stale error on the queue from the above test so clear it */
+    ERR_clear_error();
 
     /*
      * II) key generation
@@ -132,7 +119,7 @@ static int dh_test(void)
         goto err3;
     BN_GENCB_set(_cb, &cb, NULL);
     if (!TEST_ptr(a = DH_new())
-            || !TEST_true(DH_generate_parameters_ex(a, 64,
+            || !TEST_true(DH_generate_parameters_ex(a, 512,
                                                     DH_GENERATOR_5, _cb)))
         goto err3;
 
@@ -192,7 +179,7 @@ static int dh_test(void)
             || !TEST_true((cout = DH_compute_key(cbuf, apub_key, c)) != -1))
         goto err3;
 
-    if (!TEST_true(aout >= 4)
+    if (!TEST_true(aout >= 20)
             || !TEST_mem_eq(abuf, aout, bbuf, bout)
             || !TEST_mem_eq(abuf, aout, cbuf, cout))
         goto err3;