From 6de1fe90860ddfe768864838637f681537f3f108 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Mon, 22 Jul 2019 22:50:19 +0200 Subject: [PATCH] Enforce a minimum DH modulus size of 512 bits [extended tests] Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/9437) --- CHANGES | 3 +++ crypto/dh/dh_err.c | 1 + crypto/dh/dh_gen.c | 10 ++++++++++ crypto/dh/dh_key.c | 10 ++++++++++ crypto/dh/dh_locl.h | 2 ++ crypto/err/openssl.txt | 1 + doc/man1/dhparam.pod | 3 ++- include/openssl/dherr.h | 1 + test/dhtest.c | 25 ++++++------------------- 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 #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. 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; -- 2.25.1