DH: stop setting the private key length arbitrarily
authorRichard Levitte <levitte@openssl.org>
Thu, 15 Oct 2020 05:14:16 +0000 (07:14 +0200)
committerRichard Levitte <levitte@openssl.org>
Tue, 27 Oct 2020 14:13:54 +0000 (15:13 +0100)
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
crypto/dh/dh_key.c
crypto/dh/dh_lib.c

index 1ce29e652d3038fc0b222530975d5c36d75ff660..cc8d064c4e590d0c1718e16aa7d9570b5bf1a474 100644 (file)
@@ -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;
 }
 
index 90802633a66c77bf99b6578da950a0bfffdc2429..930b33a33b1ffe636f5921df0605125a5b6ff43b 100644 (file)
@@ -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))
index 6280472ade4e7bc6abc112eb63671c729060a234..207e7b06c69186700dbd0437d952d3758bb7a714 100644 (file)
@@ -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++;