Use as small dh key size as possible to support the security

Longer private key sizes unnecessarily raise the cycles needed to
compute the shared secret without any increase of the real security.

We use minimum key sizes as defined in RFC7919.

For arbitrary parameters we cannot know whether they are safe
primes (we could test but that would be too inefficient) we have
to keep generating large keys.

However we now set a small dh->length when we are generating safe prime
parameters because we know it is safe to use small keys with them.

That means users need to regenerate the parameters if they
want to take the performance advantage of small private key.

Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18480)

3 files changed, 38 insertions, 21 deletions

diff --git a/crypto/ffc/ffc_backend.c b/crypto/ffc/ffc_backend.c
index 9a013d95d3..dbd28b0e66 100644
--- a/crypto/ffc/ffc_backend.c
+++ b/crypto/ffc/ffc_backend.c
@@ -39,7 +39,7 @@ int ossl_ffc_params_fromdata(FFC_PARAMS *ffc, const OSSL_PARAM params[])
         if (prm->data_type != OSSL_PARAM_UTF8_STRING
             || prm->data == NULL
             || (group = ossl_ffc_name_to_dh_named_group(prm->data)) == NULL
-            || !ossl_ffc_named_group_set_pqg(ffc, group))
+            || !ossl_ffc_named_group_set(ffc, group))
 #endif
             goto err;
     }
diff --git a/crypto/ffc/ffc_dh.c b/crypto/ffc/ffc_dh.c
index 9a7e99cff6..df07e173bc 100644
--- a/crypto/ffc/ffc_dh.c
+++ b/crypto/ffc/ffc_dh.c
@@ -13,16 +13,18 @@
 
 #ifndef OPENSSL_NO_DH
 
-# define FFDHE(sz) {                                                        \
+# define FFDHE(sz, keylength) {                                             \
         SN_ffdhe##sz, NID_ffdhe##sz,                                        \
         sz,                                                                 \
+        keylength,                                                          \
         &ossl_bignum_ffdhe##sz##_p, &ossl_bignum_ffdhe##sz##_q,             \
         &ossl_bignum_const_2,                                               \
     }
 
-# define MODP(sz)  {                                                        \
+# define MODP(sz, keylength)  {                                             \
         SN_modp_##sz, NID_modp_##sz,                                        \
         sz,                                                                 \
+        keylength,                                                          \
         &ossl_bignum_modp_##sz##_p, &ossl_bignum_modp_##sz##_q,             \
         &ossl_bignum_const_2                                                \
     }
@@ -30,14 +32,15 @@
 # define RFC5114(name, uid, sz, tag) {                                      \
         name, uid,                                                          \
         sz,                                                                 \
+        0,                                                                  \
         &ossl_bignum_dh##tag##_p, &ossl_bignum_dh##tag##_q,                 \
         &ossl_bignum_dh##tag##_g                                            \
     }
 
 #else
 
-# define FFDHE(sz)                      { SN_ffdhe##sz, NID_ffdhe##sz }
-# define MODP(sz)                       { SN_modp_##sz, NID_modp_##sz }
+# define FFDHE(sz, keylength)           { SN_ffdhe##sz, NID_ffdhe##sz }
+# define MODP(sz, keylength)            { SN_modp_##sz, NID_modp_##sz }
 # define RFC5114(name, uid, sz, tag)    { name, uid }
 
 #endif
@@ -47,26 +50,32 @@ struct dh_named_group_st {
     int uid;
 #ifndef OPENSSL_NO_DH
     int32_t nbits;
+    int keylength;
     const BIGNUM *p;
     const BIGNUM *q;
     const BIGNUM *g;
 #endif
 };
 
+/*
+ * The private key length values are taken from RFC7919 with the values for
+ * MODP primes given the same lengths as the equivalent FFDHE.
+ * The MODP 1536 value is approximated.
+ */
 static const DH_NAMED_GROUP dh_named_groups[] = {
-    FFDHE(2048),
-    FFDHE(3072),
-    FFDHE(4096),
-    FFDHE(6144),
-    FFDHE(8192),
+    FFDHE(2048, 225),
+    FFDHE(3072, 275),
+    FFDHE(4096, 325),
+    FFDHE(6144, 375),
+    FFDHE(8192, 400),
 #ifndef FIPS_MODULE
-    MODP(1536),
+    MODP(1536, 200),
 #endif
-    MODP(2048),
-    MODP(3072),
-    MODP(4096),
-    MODP(6144),
-    MODP(8192),
+    MODP(2048, 225),
+    MODP(3072, 275),
+    MODP(4096, 325),
+    MODP(6144, 375),
+    MODP(8192, 400),
     /*
      * Additional dh named groups from RFC 5114 that have a different g.
      * The uid can be any unique identifier.
@@ -134,6 +143,13 @@ const char *ossl_ffc_named_group_get_name(const DH_NAMED_GROUP *group)
 }
 
 #ifndef OPENSSL_NO_DH
+int ossl_ffc_named_group_get_keylength(const DH_NAMED_GROUP *group)
+{
+    if (group == NULL)
+        return 0;
+    return group->keylength;
+}
+
 const BIGNUM *ossl_ffc_named_group_get_q(const DH_NAMED_GROUP *group)
 {
     if (group == NULL)
@@ -141,13 +157,14 @@ const BIGNUM *ossl_ffc_named_group_get_q(const DH_NAMED_GROUP *group)
     return group->q;
 }
 
-int ossl_ffc_named_group_set_pqg(FFC_PARAMS *ffc, const DH_NAMED_GROUP *group)
+int ossl_ffc_named_group_set(FFC_PARAMS *ffc, const DH_NAMED_GROUP *group)
 {
     if (ffc == NULL || group == NULL)
         return 0;
 
     ossl_ffc_params_set0_pqg(ffc, (BIGNUM *)group->p, (BIGNUM *)group->q,
                              (BIGNUM *)group->g);
+    ffc->keylength = group->keylength;
 
     /* flush the cached nid, The DH layer is responsible for caching */
     ffc->nid = NID_undef;
diff --git a/crypto/ffc/ffc_key_generate.c b/crypto/ffc/ffc_key_generate.c
index c18f349ee2..cb895f2abd 100644
--- a/crypto/ffc/ffc_key_generate.c
+++ b/crypto/ffc/ffc_key_generate.c
@@ -25,11 +25,11 @@ int ossl_ffc_generate_private_key(BN_CTX *ctx, const FFC_PARAMS *params,
     int ret = 0, qbits = BN_num_bits(params->q);
     BIGNUM *m, *two_powN = NULL;
 
-    /* Deal with the edge case where the value of N is not set */
-    if (N == 0)
-        N = qbits;
+    /* Deal with the edge cases where the value of N and/or s is not set */
     if (s == 0)
-        s = N / 2;
+        goto err;
+    if (N == 0)
+        N = params->keylength ? params->keylength : 2 * s;
 
     /* Step (2) : check range of N */
     if (N < 2 * s || N > qbits)