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)

diff --git a/CHANGES.md b/CHANGES.md
index 3ee3b44976a7abac7b28451bfef9efc872b88b7e..c314581ce7038e6824c9e1c3cc5281aa33e2a7d4 100644 (file)
--- a/CHANGES.md
+++ b/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
 -----------
 

diff --git a/crypto/dh/dh_gen.c b/crypto/dh/dh_gen.c
index 66d1f94bc005b652e5f306ce3eed46b3b203e423..628410c0d3b88fe4ee0a4a2b5c1fbf09cb005cc6 100644 (file)
--- a/crypto/dh/dh_gen.c
+++ b/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:

diff --git a/crypto/dh/dh_group_params.c b/crypto/dh/dh_group_params.c
index 3f843fe9569d598559af135a0e1d2c72659bdde4..460bd8f009894dc3cde7d17a13d4895470a42ad5 100644 (file)
--- a/crypto/dh/dh_group_params.c
+++ b/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++;
     }
 }

diff --git a/crypto/ffc/ffc_backend.c b/crypto/ffc/ffc_backend.c
index 9a013d95d35916c7a3c3624e043474e12975b5b5..dbd28b0e66bd7ad49906572b3cb7fca5617f6a77 100644 (file)
--- 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 9a7e99cff670488c70e2bdecb1b57b36e7d3bd1d..df07e173bcb8c1f4039a5257e0ad5d9c9e179f71 100644 (file)
--- 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 c18f349ee2268cc12d141f456a6033cc492ffe9e..cb895f2abd532939014f2523823f9ba85c5356ab 100644 (file)
--- 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)

diff --git a/include/internal/ffc.h b/include/internal/ffc.h
index 79cb06aba3b84ffd5d198737198d49c94fc52142..50673efb89676c4ffe18f69834d1c1686b358697 100644 (file)
--- a/include/internal/ffc.h
+++ b/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 */

diff --git a/test/ffc_internal_test.c b/test/ffc_internal_test.c
index 026158d4ba24cf955c2b32a2bf249e5c8913b13c..051a31eac0f4af124c31238c04d2796825c91d45 100644 (file)
--- a/test/ffc_internal_test.c
+++ b/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;