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 | 5 ++ lib/ssl-connect.man | 12 ++++- lib/stream-ssl.c | 111 +++++++++++++++++++++++++++++------------- lib/stream.c | 2 +- tests/ovsdb-server.at | 84 +++++++++++++++----------------- 5 files changed, 132 insertions(+), 82 deletions(-) diff --git a/NEWS b/NEWS index 655d2911c..75d25222a 100644 --- a/NEWS +++ b/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 diff --git a/lib/ssl-connect.man b/lib/ssl-connect.man index 108850da5..e731885c1 100644 --- a/lib/ssl-connect.man +++ b/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. diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c index 8b0208e92..1af2703c8 100644 --- a/lib/stream-ssl.c +++ b/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 @@ -1057,12 +1059,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); @@ -1244,60 +1247,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, diff --git a/lib/stream.c b/lib/stream.c index baf01a801..3fb4d11cf 100644 --- a/lib/stream.c +++ b/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 } diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at index 320a8c6a9..3b9aa0c74 100644 --- a/tests/ovsdb-server.at +++ b/tests/ovsdb-server.at @@ -869,20 +869,23 @@ 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], @@ -890,21 +893,7 @@ AT_CHECK_UNQUOTED( ]], [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], @@ -912,23 +901,11 @@ AT_CHECK_UNQUOTED( ], [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 -- 2.47.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
