stream-ssl: Support protocol ranges.

The NO options are deprecated since OpenSSL 1.1.0:
  * SSL_OP_NO_SSLv3
  * SSL_OP_NO_TLSv1
  * SSL_OP_NO_TLSv1_1
  * SSL_OP_NO_TLSv1_2

SSL_CTX_set_min/max_proto_version API should be used instead.

Change the "ssl-protocols" configuration option to parse values and
enable ranges with this new API instead.  This means that we'll start
enabling protocols that may not be enabled by the user, e.g.
--ssl-protocols="TLSv1,TLSv1.2" will now enable TLSv1.1 as well.
But it's probably not a big deal, and there will be no way to turn off
one protocol in the middle in the future anyway, since the OpenSSL
API required to do so is deprecated.  And such configurations are
very unlikely to be used in practice.  At least, that was one of the
reasons for OpenSSL to change the API in the first place.

While at it, allow users to configure simple ranges, instead of lists.
For example, OVS will now allow values like "TLSv1-TLSv1.2" to enable
all versions between TLSv1 and TLSv1.2, or "TLSv1.1+" to allow TLSv1.1
or any later version.  The option still accepts a list of protocols or
exactly one range.

Signed-off-by: Ilya Maximets <[email protected]>

NEWS

@@ -15,6 +15,11 @@ Post-v3.4.0
        on OpenFlow and database connections.  Use --ssl-protocols to turn
        them back on.  Support will be fully removed in the next release.
      * OpenSSL 1.1.1 or newer is now required for SSL/TLS support.
+     * The protocol list in --ssl-protocols or corresponding database column
+       now supports specifying simple protocol ranges like:
+         - "TLSv1-TLSv1.2" to enable all protocols between TLSv1 and TLSv1.2.
+         - "TLSv1.2+" to enable protocol TLSv1.2 and later.
+       The value must be a list of protocols or exactly one protocol range.
    - Userspace datapath:
      * The default zone limit, if set, is now inherited by any zone
        that does not have a specific value defined, rather than being

lib/ssl-connect.man

@@ -1,8 +1,16 @@
 .IP "\fB\-\-ssl\-protocols=\fIprotocols\fR"
-Specifies, in a comma- or space-delimited list, the SSL/TLS protocols
+Specifies a range or a comma- or space-delimited list of the SSL/TLS protocols
 \fB\*(PN\fR will enable for SSL/TLS connections.  Supported
 \fIprotocols\fR include \fBTLSv1\fR (deprecated), \fBTLSv1.1\fR (deprecated),
-and \fBTLSv1.2\fR.
+and \fBTLSv1.2\fR.  Ranges can be provided in a form of two protocol names
+separated with a dash, or as a single protocol name with a plus sign.
+For example, use \fBTLSv1-TLSv1.2\fR to allow \fBTLSv1\fR, \fBTLSv1.1\fR
+and \fBTLSv1.2\fR.  Use \fBTLSv1.2+\fR to allow \fBTLSv1.2\fR and any later
+protocol.  The option accepts a list of protocols or exactly one range.
+The range is a preferred way of specifying protocols and the option always
+behaves as if the range between the minimum and the maximum specified version
+is provided, i.e., if the option is set to \fBTLSv1,TLSv1.2\fR, the
+\fBTLSv1.1\fR will also be enabled as if it was a range.
 Regardless of order, the highest protocol supported by both sides will
 be chosen when making the connection.  The default when this option is
 omitted is \fBTLSv1.2\fR or later.

lib/stream-ssl.c

@@ -32,6 +32,7 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include "bitmap.h"
 #include "coverage.h"
 #include "openvswitch/dynamic-string.h"
 #include "entropy.h"
@@ -42,6 +43,7 @@
 #include "openvswitch/shash.h"
 #include "socket-util.h"
 #include "util.h"
+#include "sset.h"
 #include "stream-provider.h"
 #include "stream.h"
 #include "timeval.h"
@@ -162,7 +164,7 @@ struct ssl_config_file {
 static struct ssl_config_file private_key;
 static struct ssl_config_file certificate;
 static struct ssl_config_file ca_cert;
-static char *ssl_protocols = "TLSv1.2";
+static char *ssl_protocols = "TLSv1.2+";
 static char *ssl_ciphers = "HIGH:!aNULL:!MD5";
 
 /* Ordinarily, the SSL client and server verify each other's certificates using
@@ -1061,12 +1063,13 @@ do_ssl_init(void)
         return ENOPROTOOPT;
     }
 
-    long options = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 |
-                   SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1;
 #ifdef SSL_OP_IGNORE_UNEXPECTED_EOF
-    options |= SSL_OP_IGNORE_UNEXPECTED_EOF;
+    SSL_CTX_set_options(ctx, SSL_OP_IGNORE_UNEXPECTED_EOF);
 #endif
-    SSL_CTX_set_options(ctx, options);
+
+    /* Only allow TLSv1.2 or later. */
+    SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION);
+    SSL_CTX_set_max_proto_version(ctx, 0);
 
 #if OPENSSL_VERSION_NUMBER < 0x3000000fL
     SSL_CTX_set_tmp_dh_callback(ctx, tmp_dh_callback);
@@ -1248,60 +1251,98 @@ stream_ssl_set_ciphers(const char *arg)
 void
 stream_ssl_set_protocols(const char *arg)
 {
-    if (ssl_init() || !arg || !strcmp(arg, ssl_protocols)){
+    if (ssl_init() || !arg || !strcmp(arg, ssl_protocols)) {
         return;
     }
 
-    /* Start with all the flags off and turn them on as requested. */
-    long protocol_flags = SSL_OP_NO_SSL_MASK;
+    struct sset set = SSET_INITIALIZER(&set);
     struct {
         const char *name;
-        long no_flag;
+        int version;
         bool deprecated;
     } protocols[] = {
-        {"TLSv1",   SSL_OP_NO_TLSv1,   true },
-        {"TLSv1.1", SSL_OP_NO_TLSv1_1, true },
-        {"TLSv1.2", SSL_OP_NO_TLSv1_2, false},
+        {"later",   0 /* any version */, false},
+        {"TLSv1",   TLS1_VERSION,        true },
+        {"TLSv1.1", TLS1_1_VERSION,      true },
+        {"TLSv1.2", TLS1_2_VERSION,      false},
     };
+    char *dash = strchr(arg, '-');
+    bool or_later = false;
+    int len = strlen(arg);
+
+    if (len && arg[len - 1] == '+') {
+        /* We only support full ranges, so more than one version or later "X+"
+         * doesn't make a lot of sense. */
+        sset_add_and_free(&set, xmemdup0(arg, len - 1));
+        or_later = true;
+    } else if (dash) {
+        /* Again, do not attempt to parse multiple ranges.  The range should
+         * always be a single "X-Y". */
+        sset_add_and_free(&set, xmemdup0(arg, dash - arg));
+        sset_add_and_free(&set, xstrdup(dash + 1));
+    } else {
+        /* Otherwise, it's a list that should not include ranges. */
+        sset_from_delimited_string(&set, arg, " ,\t");
+    }
 
-    char *s = xstrdup(arg);
-    char *save_ptr = NULL;
-    char *word = strtok_r(s, " ,\t", &save_ptr);
-    if (word == NULL) {
+    if (sset_is_empty(&set)) {
         VLOG_ERR("SSL/TLS protocol settings invalid");
         goto exit;
     }
-    while (word != NULL) {
-        long no_flag = 0;
 
-        for (size_t i = 0; i < ARRAY_SIZE(protocols); i++) {
-            if (!strcasecmp(word, protocols[i].name)) {
-                no_flag = protocols[i].no_flag;
-                if (protocols[i].deprecated) {
-                    VLOG_WARN("%s protocol is deprecated", word);
-                }
-                break;
+    size_t min_version = ARRAY_SIZE(protocols) + 1;
+    size_t max_version = 0;
+    unsigned long map = 0;
+
+    for (size_t i = 1; i < ARRAY_SIZE(protocols); i++) {
+        if (sset_contains(&set, protocols[i].name)) {
+            min_version = MIN(min_version, i);
+            max_version = MAX(max_version, i);
+            if (protocols[i].deprecated) {
+                VLOG_WARN("%s protocol is deprecated", protocols[i].name);
             }
+            bitmap_set1(&map, i);
+            sset_find_and_delete(&set, protocols[i].name);
         }
+    }
+
+    if (!sset_is_empty(&set)) {
+        const char *word;
 
-        if (!no_flag) {
+        SSET_FOR_EACH (word, &set) {
             VLOG_ERR("%s: SSL/TLS protocol not recognized", word);
-            goto exit;
         }
+        goto exit;
+    }
 
-        /* Reverse the no flag and mask it out in the flags
-         * to turn on that protocol. */
-        protocol_flags &= ~no_flag;
-        word = strtok_r(NULL, " ,\t", &save_ptr);
-    };
+    /* At his point we must have parsed at least one protocol. */
+    ovs_assert(min_version && min_version < ARRAY_SIZE(protocols));
+    ovs_assert(max_version && max_version < ARRAY_SIZE(protocols));
+    if (!or_later && !dash) {
+        for (size_t i = min_version + 1; i < max_version; i++) {
+            if (!bitmap_is_set(&map, i)) {
+                VLOG_WARN("SSL/TLS protocol %s"
+                          " is not configured, but will be enabled anyway.",
+                          protocols[i].name);
+            }
+        }
+    }
 
-    /* Set the actual options. */
-    SSL_CTX_set_options(ctx, protocol_flags);
+    if (or_later) {
+        ovs_assert(min_version == max_version);
+        max_version = 0;
+    }
 
+    /* Set the actual versions. */
+    SSL_CTX_set_min_proto_version(ctx, protocols[min_version].version);
+    SSL_CTX_set_max_proto_version(ctx, protocols[max_version].version);
+    VLOG_DBG("Enabled protocol range: %s%s%s", protocols[min_version].name,
+                                               max_version ? " - " : " or ",
+                                               protocols[max_version].name);
     ssl_protocols = xstrdup(arg);
 
 exit:
-    free(s);
+    sset_destroy(&set);
 }
 
 /* Reads the X509 certificate or certificates in file 'file_name'.  On success,

lib/stream.c

@@ -159,7 +159,7 @@ stream_usage(const char *name, bool active, bool passive,
                "to read or create\n");
     }
     printf("SSL/TLS options:\n"
-           "  --ssl-protocols=PROTOS  list of SSL/TLS protocols to enable\n"
+           "  --ssl-protocols=PROTOS  range of SSL/TLS protocols to enable\n"
            "  --ssl-ciphers=CIPHERS   list of SSL/TLS ciphers to enable\n");
 #endif
 }

tests/ovsdb-server.at

@@ -869,66 +869,43 @@ AT_CHECK(
         --remote=pssl:0:127.0.0.1 db],
   [0], [ignore], [ignore])
 PARSE_LISTENING_PORT([ovsdb-server.log], [SSL_PORT])
-AT_CHECK(
-  [[ovsdb-client \
-        --private-key=$PKIDIR/testpki-privkey.pem \
-        --certificate=$PKIDIR/testpki-cert.pem \
-        --ca-cert=$PKIDIR/testpki-cacert.pem \
-        --ssl-protocols=TLSv1.2,TLSv1.1 \
-        --ssl-ciphers=HIGH:!aNULL:!MD5 \
-        transact ssl:127.0.0.1:$SSL_PORT \
-        '["mydb",
-          {"op": "select",
-           "table": "SSL",
-           "where": [],
-           "columns": ["private_key"]}]']],
-  [0], [stdout], [ignore])
+
+# SSL_OVSDB_CLIENT(PROTOCOL, [CIPHERS])
+m4_define([SSL_OVSDB_CLIENT], [dnl
+  ovsdb-client -vconsole:stream_ssl:dbg \
+    --private-key=$PKIDIR/testpki-privkey.pem \
+    --certificate=$PKIDIR/testpki-cert.pem \
+    --ca-cert=$PKIDIR/testpki-cacert.pem \
+    --ssl-protocols=[$1] \
+    m4_if([$2], [], [], [--ssl-ciphers=$2]) \
+    transact ssl:127.0.0.1:$SSL_PORT \
+    '[[["mydb",
+      {"op": "select",
+       "table": "SSL",
+       "where": [],
+       "columns": ["private_key"]}]]]'])
+
+AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.2,TLSv1.1]), [0], [stdout], [ignore])
 cat stdout >> output
 AT_CHECK_UNQUOTED(
   [cat output], [0],
   [[@<:@{"rows":@<:@{"private_key":"$PKIDIR/testpki-privkey2.pem"}@:>@}@:>@
 ]], [ignore])
 # Check that when the server has TLSv1.1+ and the client has
 # TLSv1 that the connection fails.
-AT_CHECK(
-  [[ovsdb-client \
-        --private-key=$PKIDIR/testpki-privkey.pem \
-        --certificate=$PKIDIR/testpki-cert.pem \
-        --ca-cert=$PKIDIR/testpki-cacert.pem \
-        --ssl-protocols=TLSv1 \
-        --ssl-ciphers=HIGH:!aNULL:!MD5 \
-        transact ssl:127.0.0.1:$SSL_PORT \
-        '["mydb",
-          {"op": "select",
-           "table": "SSL",
-           "where": [],
-           "columns": ["private_key"]}]']], 
-  [1], [stdout], 
-  [stderr])
+AT_CHECK(SSL_OVSDB_CLIENT([TLSv1]), [1], [stdout], [stderr])
 cat stderr > output
 AT_CHECK_UNQUOTED(
   [sed -n "/failed to connect/s/ (.*)//p" output], [0],
   [ovsdb-client: failed to connect to "ssl:127.0.0.1:$SSL_PORT"
 ], 
   [ignore])
 AT_CHECK([grep -q 'TLSv1 protocol is deprecated' output])
+AT_CHECK([grep -q 'Enabled protocol range: TLSv1 - TLSv1' stderr])
 # Check that when ciphers are not compatible, that a negotiation
 # failure occurs.
-AT_CHECK(
-  [[ovsdb-client \
-        --private-key=$PKIDIR/testpki-privkey.pem \
-        --certificate=$PKIDIR/testpki-cert.pem \
-        --ca-cert=$PKIDIR/testpki-cacert.pem \
-        --ssl-protocols=TLSv1.2,TLSv1.1 \
-        --ssl-ciphers=ECDHE-ECDSA-AES256-GCM-SHA384 \
-        transact ssl:127.0.0.1:$SSL_PORT \
-        '["mydb",
-          {"op": "select",
-           "table": "SSL",
-           "where": [],
-           "columns": ["private_key"]}]']], 
-  [1], [stdout], 
-  [stderr])
+AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.2,TLSv1.1], [ECDHE-ECDSA-AES256-GCM-SHA384]),
+         [1], [stdout], [stderr])
 cat stderr > output
 AT_CHECK_UNQUOTED(
   [sed -n "/failed to connect/s/ (.*)//p" output], [0],
@@ -944,6 +921,25 @@ AT_CHECK_UNQUOTED(
   [grep -E "(sslv3|ssl/tls) alert handshake failure" output], [0],
   [stdout],
   [ignore])
+
+# Checking parsing of different protocol ranges.
+AT_CHECK(SSL_OVSDB_CLIENT([TLSv1,TLSv1.2]), [0], [stdout], [stderr])
+AT_CHECK([grep -q 'Enabled protocol range: TLSv1 - TLSv1.2' stderr])
+AT_CHECK([grep -q \
+            'TLSv1.1 is not configured, but will be enabled anyway' stderr])
+AT_CHECK(SSL_OVSDB_CLIENT([TLSv1-TLSv1.2]), [0], [stdout], [stderr])
+AT_CHECK([grep -q 'Enabled protocol range: TLSv1 - TLSv1.2' stderr])
+AT_CHECK([grep -q 'will be enabled anyway' stderr], [1])
+AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.2-TLSv1]), [0], [stdout], [stderr])
+AT_CHECK([grep -q 'Enabled protocol range: TLSv1 - TLSv1.2' stderr])
+AT_CHECK([grep -q 'will be enabled anyway' stderr], [1])
+AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.1+]), [0], [stdout], [stderr])
+AT_CHECK([grep -q 'Enabled protocol range: TLSv1.1 or later' stderr])
+AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.1-TLSv1.2,TLSv1.2+]), [0], [stdout], [stderr])
+AT_CHECK([grep -q 'SSL/TLS protocol not recognized' stderr])
+AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.1+TLSv1.2]), [0], [stdout], [stderr])
+AT_CHECK([grep -q 'SSL/TLS protocol not recognized' stderr])
+
 OVSDB_SERVER_SHUTDOWN(["
   /stream_ssl|WARN/d
   /Protocol error/d