[ovs-dev] [PATCH] stream-ssl: Remove unsafe 1024 bit dh params

Jaime Caamaño Ruiz jcaamano at redhat.com
Wed Jun 16 16:04:33 UTC 2021


Using 1024 bit params for DH is considered unsafe [1]. Additionally,
from [2]:

"Modern servers that do not support export ciphersuites are advised to
either use SSL_CTX_set_tmp_dh() or alternatively, use the callback but
ignore keylength and is_export and simply supply at least 2048-bit
parameters in the callback."

Additionally, using 1024 bit dh params may block clients running on
recent openssl version from connecting to servers running on old
openssl versions given the stricter default security requirements of
those new openssl versions that clients may not be willing to forfeit.
Recent openssl peers are more likely to negotiate TLS 1.3 protocol
and use Elliptic-curve DH instead of fixed param DH.

[1] https://weakdh.org/
[2] https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_tmp_dh_callback.html

Signed-off-by: Jaime Caamaño Ruiz <jcaamano at redhat.com>
---
 build-aux/generate-dhparams-c |  3 +-
 lib/automake.mk               |  1 -
 lib/dh1024.pem                | 10 -----
 lib/dhparams.c                | 76 +++++++----------------------------
 lib/dhparams.h                |  1 -
 lib/stream-ssl.c              |  3 +-
 6 files changed, 16 insertions(+), 78 deletions(-)
 delete mode 100644 lib/dh1024.pem

diff --git a/build-aux/generate-dhparams-c b/build-aux/generate-dhparams-c
index dfbdb1f2e..1884c99e1 100755
--- a/build-aux/generate-dhparams-c
+++ b/build-aux/generate-dhparams-c
@@ -22,8 +22,7 @@ my_DH_set0_pqg(DH *dh, BIGNUM *p, const BIGNUM **q OVS_UNUSED, BIGNUM *g)
 #endif
 }
 EOF
-(openssl dhparam -C -in lib/dh1024.pem -noout &&
-openssl dhparam -C -in lib/dh2048.pem -noout &&
+(openssl dhparam -C -in lib/dh2048.pem -noout &&
 openssl dhparam -C -in lib/dh4096.pem -noout) | sed '
     s/^static DH/DH/
     s/\(get_dh[0-9]*\)()/\1(void)/
diff --git a/lib/automake.mk b/lib/automake.mk
index db9017591..1980bbeef 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -528,7 +528,6 @@ pkgconfig_DATA += \
 	lib/libsflow.pc
 
 EXTRA_DIST += \
-	lib/dh1024.pem \
 	lib/dh2048.pem \
 	lib/dh4096.pem \
 	lib/common.xml \
diff --git a/lib/dh1024.pem b/lib/dh1024.pem
deleted file mode 100644
index 6eaeca9b8..000000000
--- a/lib/dh1024.pem
+++ /dev/null
@@ -1,10 +0,0 @@
------BEGIN DH PARAMETERS-----
-MIGHAoGBAPSI/VhOSdvNILSd5JEHNmszbDgNRR0PfIizHHxbLY7288kjwEPwpVsY
-jY67VYy4XTjTNP18F1dDox0YbN4zISy1Kv884bEpQBgRjXyEpwpy1obEAxnIByl6
-ypUM2Zafq9AKUJsCRtMIPWakXUGfnHy9iUsiGSa6q6Jew1XpL3jHAgEC
------END DH PARAMETERS-----
-
-These are the 1024 bit DH parameters from "Assigned Number for SKIP Protocols"
-(http://www.skip-vpn.org/spec/numbers.html).
-See there for how they were generated.
-Note that g is not a generator, but this is not a problem since p is a safe prime.
diff --git a/lib/dhparams.c b/lib/dhparams.c
index 903bf1f35..85123863f 100644
--- a/lib/dhparams.c
+++ b/lib/dhparams.c
@@ -18,50 +18,6 @@ my_DH_set0_pqg(DH *dh, BIGNUM *p, const BIGNUM **q OVS_UNUSED, BIGNUM *g)
     return DH_set0_pqg(dh, p, NULL, g);
 #endif
 }
-#ifndef HEADER_DH_H
-# include <openssl/dh.h>
-#endif
-
-DH *get_dh1024(void)
-{
-    static unsigned char dhp_1024[] = {
-        0xF4, 0x88, 0xFD, 0x58, 0x4E, 0x49, 0xDB, 0xCD, 0x20, 0xB4,
-        0x9D, 0xE4, 0x91, 0x07, 0x36, 0x6B, 0x33, 0x6C, 0x38, 0x0D,
-        0x45, 0x1D, 0x0F, 0x7C, 0x88, 0xB3, 0x1C, 0x7C, 0x5B, 0x2D,
-        0x8E, 0xF6, 0xF3, 0xC9, 0x23, 0xC0, 0x43, 0xF0, 0xA5, 0x5B,
-        0x18, 0x8D, 0x8E, 0xBB, 0x55, 0x8C, 0xB8, 0x5D, 0x38, 0xD3,
-        0x34, 0xFD, 0x7C, 0x17, 0x57, 0x43, 0xA3, 0x1D, 0x18, 0x6C,
-        0xDE, 0x33, 0x21, 0x2C, 0xB5, 0x2A, 0xFF, 0x3C, 0xE1, 0xB1,
-        0x29, 0x40, 0x18, 0x11, 0x8D, 0x7C, 0x84, 0xA7, 0x0A, 0x72,
-        0xD6, 0x86, 0xC4, 0x03, 0x19, 0xC8, 0x07, 0x29, 0x7A, 0xCA,
-        0x95, 0x0C, 0xD9, 0x96, 0x9F, 0xAB, 0xD0, 0x0A, 0x50, 0x9B,
-        0x02, 0x46, 0xD3, 0x08, 0x3D, 0x66, 0xA4, 0x5D, 0x41, 0x9F,
-        0x9C, 0x7C, 0xBD, 0x89, 0x4B, 0x22, 0x19, 0x26, 0xBA, 0xAB,
-        0xA2, 0x5E, 0xC3, 0x55, 0xE9, 0x2F, 0x78, 0xC7
-    };
-    static unsigned char dhg_1024[] = {
-        0x02
-    };
-    DH *dh = DH_new();
-    BIGNUM *dhp_bn, *dhg_bn;
-
-    if (dh == NULL)
-        return NULL;
-    dhp_bn = BN_bin2bn(dhp_1024, sizeof (dhp_1024), NULL);
-    dhg_bn = BN_bin2bn(dhg_1024, sizeof (dhg_1024), NULL);
-    if (dhp_bn == NULL || dhg_bn == NULL
-            || !my_DH_set0_pqg(dh, dhp_bn, NULL, dhg_bn)) {
-        DH_free(dh);
-        BN_free(dhp_bn);
-        BN_free(dhg_bn);
-        return NULL;
-    }
-    return dh;
-}
-#ifndef HEADER_DH_H
-# include <openssl/dh.h>
-#endif
-
 DH *get_dh2048(void)
 {
     static unsigned char dhp_2048[] = {
@@ -96,25 +52,21 @@ DH *get_dh2048(void)
         0x02
     };
     DH *dh = DH_new();
-    BIGNUM *dhp_bn, *dhg_bn;
+    BIGNUM *p, *g;
 
     if (dh == NULL)
         return NULL;
-    dhp_bn = BN_bin2bn(dhp_2048, sizeof (dhp_2048), NULL);
-    dhg_bn = BN_bin2bn(dhg_2048, sizeof (dhg_2048), NULL);
-    if (dhp_bn == NULL || dhg_bn == NULL
-            || !my_DH_set0_pqg(dh, dhp_bn, NULL, dhg_bn)) {
+    p = BN_bin2bn(dhp_2048, sizeof(dhp_2048), NULL);
+    g = BN_bin2bn(dhg_2048, sizeof(dhg_2048), NULL);
+    if (p == NULL || g == NULL
+            || !my_DH_set0_pqg(dh, p, NULL, g)) {
         DH_free(dh);
-        BN_free(dhp_bn);
-        BN_free(dhg_bn);
+        BN_free(p);
+        BN_free(g);
         return NULL;
     }
     return dh;
 }
-#ifndef HEADER_DH_H
-# include <openssl/dh.h>
-#endif
-
 DH *get_dh4096(void)
 {
     static unsigned char dhp_4096[] = {
@@ -175,17 +127,17 @@ DH *get_dh4096(void)
         0x02
     };
     DH *dh = DH_new();
-    BIGNUM *dhp_bn, *dhg_bn;
+    BIGNUM *p, *g;
 
     if (dh == NULL)
         return NULL;
-    dhp_bn = BN_bin2bn(dhp_4096, sizeof (dhp_4096), NULL);
-    dhg_bn = BN_bin2bn(dhg_4096, sizeof (dhg_4096), NULL);
-    if (dhp_bn == NULL || dhg_bn == NULL
-            || !my_DH_set0_pqg(dh, dhp_bn, NULL, dhg_bn)) {
+    p = BN_bin2bn(dhp_4096, sizeof(dhp_4096), NULL);
+    g = BN_bin2bn(dhg_4096, sizeof(dhg_4096), NULL);
+    if (p == NULL || g == NULL
+            || !my_DH_set0_pqg(dh, p, NULL, g)) {
         DH_free(dh);
-        BN_free(dhp_bn);
-        BN_free(dhg_bn);
+        BN_free(p);
+        BN_free(g);
         return NULL;
     }
     return dh;
diff --git a/lib/dhparams.h b/lib/dhparams.h
index 9bf03e51e..0eca99a6f 100644
--- a/lib/dhparams.h
+++ b/lib/dhparams.h
@@ -20,7 +20,6 @@
 #include <inttypes.h>
 #include <openssl/dh.h>
 
-DH *get_dh1024(void);
 DH *get_dh2048(void);
 DH *get_dh4096(void);
 
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 078fcbc3a..0ea3f2c08 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -1087,7 +1087,6 @@ tmp_dh_callback(SSL *ssl OVS_UNUSED, int is_export OVS_UNUSED, int keylength)
     };
 
     static struct dh dh_table[] = {
-        {1024, NULL, get_dh1024},
         {2048, NULL, get_dh2048},
         {4096, NULL, get_dh4096},
     };
@@ -1095,7 +1094,7 @@ tmp_dh_callback(SSL *ssl OVS_UNUSED, int is_export OVS_UNUSED, int keylength)
     struct dh *dh;
 
     for (dh = dh_table; dh < &dh_table[ARRAY_SIZE(dh_table)]; dh++) {
-        if (dh->keylength == keylength) {
+        if (dh->keylength >= keylength) {
             if (!dh->dh) {
                 dh->dh = dh->constructor();
                 if (!dh->dh) {
-- 
2.31.1



More information about the dev mailing list