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)

diff --git a/CHANGES b/CHANGES
index 3507e350e3c2bdeefa0dff1d76e1ebf8df8e039e..acaa0995182fd0e00639510cd1f71964012454d5 100644 (file)
--- 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 cbde2601454e1b7fe7b42361c8d1c1c97374b1a4..69f1452441ea3530c581a269d519e6c9cb4b9484 100644 (file)
--- 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 6e98b59d858f372e238a991c052bece10fd1a99b..76d6ad018e5e33542059d5dfa28b5d99066cdabc 100644 (file)
--- 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 0d6b04de200762e2541d3e171ee507e70188a13b..8731cc2c7315a14246bc03f91df045e7b752a1f7 100644 (file)
--- 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 f0247b8d7d31b4bb8cbbe1c8d161f07a22b08893..a9041e946256350b9c06dd78e75fa89772af4160 100644 (file)
--- 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 d88e98993e1ea06fea9794f755de5ddb118d8b91..ede1c57a7bb074aeca0de571d18a2c24cff0ce05 100644 (file)
--- 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 dd871b3b4865b9c1d0d631d9a4d44056672ef47c..c51bbaa63e77e7b08a6ba88d6722a63ab6086c98 100644 (file)
--- 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 1e3451bbe6d1afb313475bcc50ea4ea9659bd391..13bd0361e0ea93135d718f115a4d2f3c14d3db16 100644 (file)
--- 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 f80d5b3f4d745fc21fe66f784b42977e4300ccb0..662a4f32eb4841a7db8306f0f525c1157a39f0c1 100644 (file)
--- 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;