From 28e1d588f14404d480cc2bd38827ecd587625643 Mon Sep 17 00:00:00 2001
From: Richard Levitte <levitte@openssl.org>
Date: Thu, 15 Oct 2020 07:14:16 +0200
Subject: [PATCH] DH: stop setting the private key length arbitrarily

The private key length is supposed to be a user settable parameter.
We do check if it's set or not, and if not, we do apply defaults.

Fixes #12071

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13140)
---
 crypto/dh/dh_backend.c |  2 +-
 crypto/dh/dh_key.c     |  5 ++++-
 crypto/dh/dh_lib.c     | 13 -------------
 3 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/crypto/dh/dh_backend.c b/crypto/dh/dh_backend.c
index 1ce29e652d3..cc8d064c4e5 100644
--- a/crypto/dh/dh_backend.c
+++ b/crypto/dh/dh_backend.c
@@ -30,7 +30,7 @@ static int dh_ffc_params_fromdata(DH *dh, const OSSL_PARAM params[])
 
     ret = ossl_ffc_params_fromdata(ffc, params);
     if (ret)
-        dh_cache_named_group(dh); /* This increments dh->dirt_cnt */
+        dh_cache_named_group(dh); /* This increments dh->dirty_cnt */
     return ret;
 }
 
diff --git a/crypto/dh/dh_key.c b/crypto/dh/dh_key.c
index 90802633a66..930b33a33b1 100644
--- a/crypto/dh/dh_key.c
+++ b/crypto/dh/dh_key.c
@@ -277,7 +277,10 @@ static int generate_key(DH *dh)
                 goto err;
 #else
             if (dh->params.q == NULL) {
-                /* secret exponent length */
+                /* secret exponent length, must satisfy 2^(l-1) <= p */
+                if (dh->length != 0
+                    && dh->length >= BN_num_bits(dh->params.p))
+                    goto err;
                 l = dh->length ? dh->length : BN_num_bits(dh->params.p) - 1;
                 if (!BN_priv_rand_ex(priv_key, l, BN_RAND_TOP_ONE,
                                      BN_RAND_BOTTOM_ANY, ctx))
diff --git a/crypto/dh/dh_lib.c b/crypto/dh/dh_lib.c
index 6280472ade4..207e7b06c69 100644
--- a/crypto/dh/dh_lib.c
+++ b/crypto/dh/dh_lib.c
@@ -219,18 +219,6 @@ int DH_set0_pqg(DH *dh, BIGNUM *p, BIGNUM *q, BIGNUM *g)
 
     ossl_ffc_params_set0_pqg(&dh->params, p, q, g);
     dh_cache_named_group(dh);
-    if (q != NULL)
-        dh->length = BN_num_bits(q);
-    /*
-     * Check if this is a named group. If it finds a named group then the
-     * 'q' and 'length' value are either already set or are set by the
-     * call.
-     */
-    if (DH_get_nid(dh) == NID_undef) {
-        /* If its not a named group then set the 'length' if q is not NULL */
-        if (q != NULL)
-            dh->length = BN_num_bits(q);
-    }
     dh->dirty_cnt++;
     return 1;
 }
@@ -264,7 +252,6 @@ int DH_set0_key(DH *dh, BIGNUM *pub_key, BIGNUM *priv_key)
     if (priv_key != NULL) {
         BN_clear_free(dh->priv_key);
         dh->priv_key = priv_key;
-        dh->length = BN_num_bits(priv_key);
     }
 
     dh->dirty_cnt++;
-- 
2.47.2