Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

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

to review the following change.


Change subject: Check that tls-version-min is supported on startup
......................................................................

Check that tls-version-min is supported on startup

After disabling TLS 1.0 and 1.1 in the mbedtls build, the client would
error out during startup if tls-version-min was set to 1.0 or 1.1, but
the server would start up and only error out when a client attempts to
connect.

This commit checks that tls-version-min is supported during startup so
that both the client and server error out on startup.

Change-Id: Ifc589854816842e46969a76e2845084b3aaa962f
Signed-off-by: Max Fillinger <maximilian.fillin...@foxcrypto.com>
---
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
5 files changed, 31 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/83/683/1

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index f2c7536..f8287f2 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8933,7 +8933,7 @@
         ver = tls_version_parse(p[1], p[2]);
         if (ver == TLS_VER_BAD)
         {
-            msg(msglevel, "unknown tls-version-min parameter: %s", p[1]);
+            msg(msglevel, "unknown or unsupported tls-version-min parameter: 
%s", p[1]);
             goto err;
         }
         options->ssl_flags &=
@@ -8947,7 +8947,7 @@
         ver = tls_version_parse(p[1], NULL);
         if (ver == TLS_VER_BAD)
         {
-            msg(msglevel, "unknown tls-version-max parameter: %s", p[1]);
+            msg(msglevel, "unknown or unsupported tls-version-max parameter: 
%s", p[1]);
             goto err;
         }
         options->ssl_flags &=
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 2054eb4..1d3fdcd 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -405,20 +405,21 @@
 int
 tls_version_parse(const char *vstr, const char *extra)
 {
+    const int min_version = tls_version_min();
     const int max_version = tls_version_max();
-    if (!strcmp(vstr, "1.0") && TLS_VER_1_0 <= max_version)
+    if (!strcmp(vstr, "1.0") && min_version <= TLS_VER_1_0 && TLS_VER_1_0 <= 
max_version)
     {
         return TLS_VER_1_0;
     }
-    else if (!strcmp(vstr, "1.1") && TLS_VER_1_1 <= max_version)
+    else if (!strcmp(vstr, "1.1") && min_version <= TLS_VER_1_1 && TLS_VER_1_1 
<= max_version)
     {
         return TLS_VER_1_1;
     }
-    else if (!strcmp(vstr, "1.2") && TLS_VER_1_2 <= max_version)
+    else if (!strcmp(vstr, "1.2") && min_version <= TLS_VER_1_2 && TLS_VER_1_2 
<= max_version)
     {
         return TLS_VER_1_2;
     }
-    else if (!strcmp(vstr, "1.3") && TLS_VER_1_3 <= max_version)
+    else if (!strcmp(vstr, "1.3") && min_version <= TLS_VER_1_3 && TLS_VER_1_3 
<= max_version)
     {
         return TLS_VER_1_3;
     }
diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index 285705f..ef1347b 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -117,6 +117,14 @@
 int tls_version_max(void);

 /**
+ * Return the minimum TLS version (as a TLS_VER_x constant)
+ * supported by current SSL implementation
+ *
+ * @return              One of the TLS_VER_x constants (but not TLS_VER_BAD).
+ */
+int tls_version_min(void);
+
+/**
  * Initialise a library-specific TLS context for a server.
  *
  * @param ctx           TLS context to initialise
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index bb88da9..de7efed 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1049,6 +1049,16 @@
 #endif /* defined(MBEDTLS_SSL_PROTO_TLS1_2) */
 }

+int
+tls_version_min(void)
+{
+#if defined(MBEDTLS_SSL_PROTO_TLS1_2)
+    return TLS_VER_1_2;
+#else
+    #error "mbedtls is compiled without support for TLS 1.2."
+#endif /* defined(MBEDTLS_SSL_PROTO_TLS1_2) */
+}
+
 /**
  * Convert an OpenVPN tls-version variable to mbed TLS format
  *
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index e8a30a3..352f7fb 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -231,6 +231,12 @@
 #endif
 }

+int
+tls_version_min(void)
+{
+    return TLS_VER_1_0;
+}
+
 /** Convert internal version number to openssl version number */
 static int
 openssl_tls_version(int ver)

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/683?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: Ifc589854816842e46969a76e2845084b3aaa962f
Gerrit-Change-Number: 683
Gerrit-PatchSet: 1
Gerrit-Owner: MaxF <m...@max-fillinger.net>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to