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)

CHANGES.md

@@ -133,6 +133,11 @@ OpenSSL 3.1
 
    *Hugo Landau*
 
+ * When generating safe-prime DH parameters set the recommended private key
+   length equivalent to minimum key lengths as in RFC 7919.
+
+   *Tomáš Mráz*
+
 OpenSSL 3.0
 -----------
 

crypto/dh/dh_gen.c

@@ -28,6 +28,7 @@
 #include <openssl/bn.h>
 #include <openssl/sha.h>
 #include "crypto/dh.h"
+#include "crypto/security_bits.h"
 #include "dh_local.h"
 
 #ifndef FIPS_MODULE
@@ -219,6 +220,9 @@ static int dh_builtin_genparams(DH *ret, int prime_len, int generator,
         goto err;
     if (!BN_set_word(ret->params.g, g))
         goto err;
+    /* We are using safe prime p, set key length equivalent to RFC 7919 */
+    ret->length = (2 * ossl_ifc_ffc_compute_security_bits(prime_len)
+                   + 24) / 25 * 25;
     ret->dirty_cnt++;
     ok = 1;
  err:

crypto/dh/dh_group_params.c

@@ -31,7 +31,7 @@ static DH *dh_param_init(OSSL_LIB_CTX *libctx, const DH_NAMED_GROUP *group)
     if (dh == NULL)
         return NULL;
 
-    ossl_ffc_named_group_set_pqg(&dh->params, group);
+    ossl_ffc_named_group_set(&dh->params, group);
     dh->params.nid = ossl_ffc_named_group_get_uid(group);
     dh->dirty_cnt++;
     return dh;
@@ -72,8 +72,9 @@ void ossl_dh_cache_named_group(DH *dh)
                                                     dh->params.g)) != NULL) {
         if (dh->params.q == NULL)
             dh->params.q = (BIGNUM *)ossl_ffc_named_group_get_q(group);
-        /* cache the nid */
+        /* cache the nid and default key length */
         dh->params.nid = ossl_ffc_named_group_get_uid(group);
+        dh->params.keylength = ossl_ffc_named_group_get_keylength(group);
         dh->dirty_cnt++;
     }
 }

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;
     }

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;

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)

include/internal/ffc.h

@@ -112,6 +112,8 @@ typedef struct ffc_params_st {
      */
     const char *mdname;
     const char *mdprops;
+    /* Default key length for known named groups according to RFC7919 */
+    int keylength;
 } FFC_PARAMS;
 
 void ossl_ffc_params_init(FFC_PARAMS *params);
@@ -205,8 +207,9 @@ const DH_NAMED_GROUP *ossl_ffc_numbers_to_dh_named_group(const BIGNUM *p,
 int ossl_ffc_named_group_get_uid(const DH_NAMED_GROUP *group);
 const char *ossl_ffc_named_group_get_name(const DH_NAMED_GROUP *);
 #ifndef OPENSSL_NO_DH
+int ossl_ffc_named_group_get_keylength(const DH_NAMED_GROUP *group);
 const BIGNUM *ossl_ffc_named_group_get_q(const DH_NAMED_GROUP *group);
-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);
 #endif
 
 #endif /* OSSL_INTERNAL_FFC_H */

test/ffc_internal_test.c

@@ -27,6 +27,7 @@
 #include "testutil.h"
 
 #include "internal/ffc.h"
+#include "crypto/security_bits.h"
 
 #ifndef OPENSSL_NO_DSA
 static const unsigned char dsa_2048_224_sha224_p[] = {
@@ -598,6 +599,9 @@ static int ffc_private_gen_test(int index)
     /* fail since N > len(q) */
     if (!TEST_false(ossl_ffc_generate_private_key(ctx, params, N + 1, 112, priv)))
         goto err;
+    /* s must be always set */
+    if (!TEST_false(ossl_ffc_generate_private_key(ctx, params, N, 0, priv)))
+        goto err;
     /* pass since 2s <= N <= len(q) */
     if (!TEST_true(ossl_ffc_generate_private_key(ctx, params, N, 112, priv)))
         goto err;
@@ -609,9 +613,10 @@ static int ffc_private_gen_test(int index)
         goto err;
     if (!TEST_true(ossl_ffc_validate_private_key(params->q, priv, &res)))
         goto err;
-
-    /* N and s are ignored in this case */
-    if (!TEST_true(ossl_ffc_generate_private_key(ctx, params, 0, 0, priv)))
+    /* N is ignored in this case */
+    if (!TEST_true(ossl_ffc_generate_private_key(ctx, params, 0,
+                                                 ossl_ifc_ffc_compute_security_bits(BN_num_bits(params->p)),
+                                                 priv)))
         goto err;
     if (!TEST_true(ossl_ffc_validate_private_key(params->q, priv, &res)))
         goto err;