]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: ssl: 'tune.ssl.default-dh-param' value ignored with openssl > 1.1.1
authorEmeric Brun <ebrun@haproxy.com>
Thu, 17 Oct 2019 12:53:03 +0000 (14:53 +0200)
committerWilly Tarreau <w@1wt.eu>
Fri, 18 Oct 2019 13:18:52 +0000 (15:18 +0200)
If openssl 1.1.1 is used, c2aae74f0 commit mistakenly enables DH automatic
feature from openssl instead of ECDH automatic feature. There is no impact for
the ECDH one because the feature is always enabled for that version. But doing
this, the 'tune.ssl.default-dh-param' was completely ignored for DH parameters.

This patch fix the bug calling 'SSL_CTX_set_ecdh_auto' instead of
'SSL_CTX_set_dh_auto'.

Currently some users may use a 2048 DH bits parameter, thinking they're using a
1024 bits one. Doing this, they may experience performance issue on light hardware.

This patch warns the user if haproxy fails to configure the given DH parameter.
In this case and if openssl version is > 1.1.0, haproxy will let openssl to
automatically choose a default DH parameter. For other openssl versions, the DH
ciphers won't be usable.

A commonly case of failure is due to the security level of openssl.cnf
which could refuse a 1024 bits DH parameter for a 2048 bits key:

 $ cat /etc/ssl/openssl.cnf
 ...

 [system_default_sect]
 MinProtocol = TLSv1
 CipherString = DEFAULT@SECLEVEL=2

This should be backport into any branch containing the commit c2aae74f0.
It requires all or part of the previous CLEANUP series.

This addresses github issue #324.

src/ssl_sock.c

index 173e91ac7312e00463df1f7d92f0611d3f81fd3b..6d80bbeea88e538f17065846b3d1d02f2e87924f 100644 (file)
@@ -2795,7 +2795,20 @@ static int ssl_sock_load_dh_params(SSL_CTX *ctx, const struct cert_key_and_chain
 
        if (ckch && ckch->dh) {
                dh = ckch->dh;
-               SSL_CTX_set_tmp_dh(ctx, dh);
+               if (!SSL_CTX_set_tmp_dh(ctx, dh)) {
+                       memprintf(err, "%sunable to load the DH parameter specified in '%s'",
+                                 err && *err ? *err : "", path);
+#if defined(SSL_CTX_set_dh_auto)
+                       SSL_CTX_set_dh_auto(ctx, 1);
+                       memprintf(err, "%s, SSL library will use an automatically generated DH parameter.\n",
+                                 err && *err ? *err : "");
+#else
+                       memprintf(err, "%s, DH ciphers won't be available.\n",
+                                 err && *err ? *err : "");
+#endif
+                       ret |= ERR_WARN;
+                       goto end;
+               }
 
                if (ssl_dh_ptr_index >= 0) {
                        /* store a pointer to the DH params to avoid complaining about
@@ -2804,7 +2817,20 @@ static int ssl_sock_load_dh_params(SSL_CTX *ctx, const struct cert_key_and_chain
                }
        }
        else if (global_dh) {
-               SSL_CTX_set_tmp_dh(ctx, global_dh);
+               if (!SSL_CTX_set_tmp_dh(ctx, global_dh)) {
+                       memprintf(err, "%sunable to use the global DH parameter for certificate '%s'",
+                                 err && *err ? *err : "", path);
+#if defined(SSL_CTX_set_dh_auto)
+                       SSL_CTX_set_dh_auto(ctx, 1);
+                       memprintf(err, "%s, SSL library will use an automatically generated DH parameter.\n",
+                                 err && *err ? *err : "");
+#else
+                       memprintf(err, "%s, DH ciphers won't be available.\n",
+                                 err && *err ? *err : "");
+#endif
+                       ret |= ERR_WARN;
+                       goto end;
+               }
        }
        else {
                /* Clear openssl global errors stack */
@@ -2822,7 +2848,20 @@ static int ssl_sock_load_dh_params(SSL_CTX *ctx, const struct cert_key_and_chain
                                goto end;
                        }
 
-                       SSL_CTX_set_tmp_dh(ctx, local_dh_1024);
+                       if (!SSL_CTX_set_tmp_dh(ctx, local_dh_1024)) {
+                               memprintf(err, "%sunable to load default 1024 bits DH parameter for certificate '%s'.\n",
+                                         err && *err ? *err : "", path);
+#if defined(SSL_CTX_set_dh_auto)
+                               SSL_CTX_set_dh_auto(ctx, 1);
+                               memprintf(err, "%s, SSL library will use an automatically generated DH parameter.\n",
+                                         err && *err ? *err : "");
+#else
+                               memprintf(err, "%s, DH ciphers won't be available.\n",
+                                         err && *err ? *err : "");
+#endif
+                               ret |= ERR_WARN;
+                               goto end;
+                       }
                }
                else {
                        SSL_CTX_set_tmp_dh_callback(ctx, ssl_get_tmp_dh);
@@ -4673,7 +4712,7 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_
                         NULL);
 
                if (ecdhe == NULL) {
-                       SSL_CTX_set_dh_auto(ctx, 1);
+                       SSL_CTX_set_ecdh_auto(ctx, 1);
                        return cfgerr;
                }
 #else