Attention is currently required from: flichtenheld, plaisthos.

Hello flichtenheld,

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/945?usp=email

to look at the new patch set (#2).

The following approvals got outdated and were removed:
Code-Review-1 by flichtenheld


Change subject: Make dh none behaviour default if not specified and add dh auto
......................................................................

Make dh none behaviour default if not specified and add dh auto

Nowadays ciphers that are using still DH and not ECDH are rarely chosen
as best cipher suite. Our man page even indicates that OpenSSL 1.0.1+
supports ECDH cipher suites. So it does not feel useful to force
specifying --dh anymore.

Custom generated Diffie Hellmann parameters are also discouraged
nowadays. The newest OpenSSL FIPS libraries even flat out reject them:

   FIPS 186-4 type domain parameters no longer allowed in FIPS mode,
   since the required validation routines were removed from FIPS 186-5

So add --dh auto since is very little extra code to let the TLS library
itself pick the parameter.

Change-Id: Ica02244c9f0ac9b4690a51f940fda9d900465289
---
M Changes.rst
M doc/man-sections/tls-options.rst
M src/openvpn/options.c
M src/openvpn/ssl.c
M src/openvpn/ssl_backend.h
M src/openvpn/ssl_mbedtls.c
M src/openvpn/ssl_openssl.c
7 files changed, 53 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/45/945/2

diff --git a/Changes.rst b/Changes.rst
index a4f5e57..a7ce8f8 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -103,6 +103,12 @@
 - ``--x509-username-field`` will no longer automatically convert fieldnames to
   uppercase. This is deprecated since OpenVPN 2.4, and has now been removed.

+- ``--dh none`` is now the default if ``--dh`` is specified. Modern TLS
+  implementations will prefer ECDH and other more modern algorithm anyway.
+
+- ``--dh auto`` instructs the TLS library - if supported - to use internal
+  Diffie-Hellmann parameters.
+
 Overview of changes in 2.6
 ==========================

diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst
index 0638d09..ad4c9cc 100644
--- a/doc/man-sections/tls-options.rst
+++ b/doc/man-sections/tls-options.rst
@@ -171,16 +171,22 @@


 --dh file
-  File containing Diffie Hellman parameters in .pem format (required for
+  File containing Diffie Hellman parameters in .pem format (used by
   ``--tls-server`` only).

   Set ``file`` to :code:`none` to disable Diffie Hellman key exchange (and
-  use ECDH only). Note that this requires peers to be using an SSL library
-  that supports ECDH TLS cipher suites (e.g. OpenSSL 1.0.1+, or
-  mbed TLS 2.0+).
+  use ECDH or newer hybrid key agreement algorithms like X25519MLKEM768).
+  Note that this requires peers to be using an SSL library that supports
+  ECDH TLS cipher suites (e.g. OpenSSL 1.0.1+, or mbed TLS 2.0+). Starting
+  with 2.7.0, this is the same as not specifying ``--dh`` at all.

-  Use ``openssl dhparam -out dh2048.pem 2048`` to generate 2048-bit DH
-  parameters. Diffie Hellman parameters may be considered public.
+  Set ``file`` to :code:`auto` to enable builtin Diffie Hellman
+  parameters of the TLS library, e.g. the ones defined in RFC 7919.
+
+  Diffie Hellman parameters can be generated using
+  ``openssl dhparam -out dh2048.pem 2048`` but it is recommended to
+  use the public well-known parameters or using ``none``.
+  Diffie Hellman parameters may be considered public.

 --ecdh-curve name
   Specify the curve to use for elliptic curve Diffie Hellman. Available
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 96119c4..e67371e 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3711,9 +3711,12 @@

     if (o->tls_server)
     {
-        /* Check that DH file is specified, or explicitly disabled */
-        notnull(o->dh_file, "DH file (--dh)");
-        if (streq(o->dh_file, "none"))
+        if (streq(o->dh_file, "auto"))
+        {
+            /* do not check existence of the "auto" file */
+            o->dh_file_inline = true;
+        }
+        else if (streq(o->dh_file, "none"))
         {
             o->dh_file = NULL;
         }
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 23f6423..d591bac 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -535,7 +535,11 @@
     {
         tls_ctx_server_new(new_ctx);

-        if (options->dh_file)
+        if (options->dh_file && !strcmp(options->dh_file, "auto"))
+        {
+            tls_ctx_use_dh_params_builtin(new_ctx);
+        }
+        else if  (options->dh_file)
         {
             tls_ctx_load_dh_params(new_ctx, options->dh_file,
                                    options->dh_file_inline);
diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index e25727f..326f78d 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -223,6 +223,13 @@
                             bool dh_file_inline);

 /**
+ * Instructs the TLS library to use builtin Diffie Hellman Parameters instead
+ * of loading specific ones (e.g. the ones from RFC 7919)
+ * @param ctx
+ */
+void tls_ctx_use_dh_params_builtin(struct tls_root_ctx *ctx);
+
+/**
  * Load Elliptic Curve Parameters, and load them into the library-specific
  * TLS context.
  *
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index e15c391..f1fada3 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -480,6 +480,13 @@
 }

 void
+tls_ctx_use_dh_params_builtin(struct tls_root_ctx *ctx)
+{
+    msg(M_FATAL, "mbed TLS does not support --dh auto. Use --dh none or "
+        "manually specify Diffie-Hellmann parameters");
+}
+
+void
 tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const char *curve_name
                          )
 {
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index f7be50c..f204007 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -714,6 +714,16 @@
 }

 void
+tls_ctx_use_dh_params_builtin(struct tls_root_ctx *ctx)
+{
+    if (!SSL_CTX_set_dh_auto(ctx->ctx, 1))
+    {
+        crypto_msg(M_FATAL, "SSL_CTX_set_dh_auto");
+    }
+
+}
+
+void
 tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const char *curve_name)
 {
 #if OPENSSL_VERSION_NUMBER >= 0x30000000L

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/945?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ica02244c9f0ac9b4690a51f940fda9d900465289
Gerrit-Change-Number: 945
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arne-open...@rfc2549.org>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-MessageType: newpatchset
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to