From 50d0dd040ea05a2167aaa570df7472f6d8020431 Mon Sep 17 00:00:00 2001 From: Ilya Maximets Date: Mon, 9 Dec 2024 17:38:53 +0100 Subject: [PATCH] stream-ssl: Add explicit support for configuring TLSv1.3. TLSv1.3 is currently only supported implicitly, if the --ssl-protocols are not provided. Or with the recent range support like "TLSv1.2+". However, it is not possible to explicitly ask for TLSv1.3 or set a custom list of ciphersuites for it. Fix that by adding TLSv1.3 to the list of available protocols and adding a new --ssl-ciphersuites option. The new option is necessary, because --ssl-ciphers translates into SSL_CTX_set_cipher_list() that configures ciphers for TLSv1.2 and earlier. SSL_CTX_set_ciphersuites() sets ciphersuites for TLSv1.3 and later. Tests updated to exercise new options and to reduce the use of deprecated TLSv1 and TLSv1.1. TLSv1.3 support was introduced in OpenSSL 1.1.1. Acked-by: Eelco Chaudron Signed-off-by: Ilya Maximets --- NEWS | 5 +++ lib/ssl-connect-syn.man | 2 + lib/ssl-connect.man | 29 ++++++++------ lib/stream-nossl.c | 7 ++++ lib/stream-ssl.c | 21 +++++++++- lib/stream-ssl.h | 11 +++++- lib/stream.c | 7 +++- ovsdb/ovsdb-server.c | 9 +++++ tests/ovsdb-server.at | 85 ++++++++++++++++++++++++++++------------- 9 files changed, 131 insertions(+), 45 deletions(-) diff --git a/NEWS b/NEWS index 2f9c8032e95..ab664ef7eca 100644 --- a/NEWS +++ b/NEWS @@ -20,6 +20,11 @@ Post-v3.4.0 - "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. + * Added explicit support for TLSv1.3. It can now be enabled via + --ssl-protocols (TLSv1.3 was supported in earlier versions only when + this option was not set). TLS ciphersuites for TLSv1.3 and later can + be configured via --ssl-ciphersuites (--ssl-ciphers only applies to + TLSv1.2 and earlier). * ovs-pki now generates 3072-bit keys by default. - Userspace datapath: * The default zone limit, if set, is now inherited by any zone diff --git a/lib/ssl-connect-syn.man b/lib/ssl-connect-syn.man index a5ca3466229..f6c30d3e5ca 100644 --- a/lib/ssl-connect-syn.man +++ b/lib/ssl-connect-syn.man @@ -3,3 +3,5 @@ .br [\fB\-\-ssl\-ciphers=\fIciphers\fR] .br +[\fB\-\-ssl\-ciphersuites=\fIciphersuites\fR] +.br diff --git a/lib/ssl-connect.man b/lib/ssl-connect.man index 772386d11e8..2e4d899bbf7 100644 --- a/lib/ssl-connect.man +++ b/lib/ssl-connect.man @@ -2,20 +2,25 @@ 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. 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. +\fBTLSv1.2\fR and \fBTLSv1.3\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.1-TLSv1.3\fR to allow \fBTLSv1.1\fR, +\fBTLSv1.2\fR and \fBTLSv1.3\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.1,TLSv1.3\fR, the +\fBTLSv1.2\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. . .IP "\fB\-\-ssl\-ciphers=\fIciphers\fR" -Specifies, in OpenSSL cipher string format, the ciphers \fB\*(PN\fR will -support for SSL/TLS connections. The default when this option is omitted is -\fBDEFAULT:@SECLEVEL=2\fR. +Specifies, in OpenSSL cipher string format, the ciphers \fB\*(PN\fR will +support for SSL/TLS connections with TLSv1.2 and earlier. The default when +this option is omitted is \fBDEFAULT:@SECLEVEL=2\fR. +. +.IP "\fB\-\-ssl\-ciphersuites=\fIciphersuites\fR" +Specifies, in OpenSSL ciphersuite string format, the ciphersuites +\fB\*(PN\fR will support for SSL/TLS connections with TLSv1.3 and later. +Default value from OpenSSL will be used when this option is omitted. diff --git a/lib/stream-nossl.c b/lib/stream-nossl.c index 71ef2361ff9..105ac377a97 100644 --- a/lib/stream-nossl.c +++ b/lib/stream-nossl.c @@ -89,3 +89,10 @@ stream_ssl_set_ciphers(const char *arg OVS_UNUSED) /* Ignore this option since it seems harmless to set SSL/TLS ciphers if * SSL/TLS won't be used. */ } + +void +stream_ssl_set_ciphersuites(const char *arg OVS_UNUSED) +{ + /* Ignore this option since it seems harmless to set TLS ciphersuites if + * SSL/TLS won't be used. */ +} diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c index 1d956c86751..27cb2fb35c0 100644 --- a/lib/stream-ssl.c +++ b/lib/stream-ssl.c @@ -166,6 +166,7 @@ static struct ssl_config_file certificate; static struct ssl_config_file ca_cert; static char *ssl_protocols = "TLSv1.2+"; static char *ssl_ciphers = "DEFAULT:@SECLEVEL=2"; +static char *ssl_ciphersuites = ""; /* Using default ones, unless specified. */ /* Ordinarily, the SSL client and server verify each other's certificates using * a CA certificate. Setting this to false disables this behavior. (This is a @@ -1220,8 +1221,8 @@ stream_ssl_set_key_and_cert(const char *private_key_file, } } -/* Sets SSL/TLS ciphers based on string input. Aborts with an error message - * if 'arg' is invalid. */ +/* Sets SSL/TLS ciphers for TLSv1.2 and earlier based on string input. + * Aborts with an error message if 'arg' is not valid. */ void stream_ssl_set_ciphers(const char *arg) { @@ -1235,6 +1236,21 @@ stream_ssl_set_ciphers(const char *arg) ssl_ciphers = xstrdup(arg); } +/* Sets TLS ciphersuites for TLSv1.3 and later based on string input. + * Aborts with an error message if 'arg' is not valid. */ +void +stream_ssl_set_ciphersuites(const char *arg) +{ + if (ssl_init() || !arg || !strcmp(ssl_ciphersuites, arg)) { + return; + } + if (SSL_CTX_set_ciphersuites(ctx, arg) == 0) { + VLOG_ERR("SSL_CTX_set_ciphersuites: %s", + ERR_error_string(ERR_get_error(), NULL)); + } + ssl_ciphersuites = xstrdup(arg); +} + /* Set SSL/TLS protocols based on the string input. Aborts with an error * message if 'arg' is invalid. */ void @@ -1254,6 +1270,7 @@ stream_ssl_set_protocols(const char *arg) {"TLSv1", TLS1_VERSION, true }, {"TLSv1.1", TLS1_1_VERSION, true }, {"TLSv1.2", TLS1_2_VERSION, false}, + {"TLSv1.3", TLS1_3_VERSION, false}, }; char *dash = strchr(arg, '-'); bool or_later = false; diff --git a/lib/stream-ssl.h b/lib/stream-ssl.h index 937f7c653ba..7036b176dbb 100644 --- a/lib/stream-ssl.h +++ b/lib/stream-ssl.h @@ -27,17 +27,20 @@ void stream_ssl_set_key_and_cert(const char *private_key_file, const char *certificate_file); void stream_ssl_set_protocols(const char *arg); void stream_ssl_set_ciphers(const char *arg); +void stream_ssl_set_ciphersuites(const char *arg); #define SSL_OPTION_ENUMS \ OPT_SSL_PROTOCOLS, \ - OPT_SSL_CIPHERS + OPT_SSL_CIPHERS, \ + OPT_SSL_CIPHERSUITES #define STREAM_SSL_LONG_OPTIONS \ {"private-key", required_argument, NULL, 'p'}, \ {"certificate", required_argument, NULL, 'c'}, \ {"ca-cert", required_argument, NULL, 'C'}, \ {"ssl-protocols", required_argument, NULL, OPT_SSL_PROTOCOLS}, \ - {"ssl-ciphers", required_argument, NULL, OPT_SSL_CIPHERS} + {"ssl-ciphers", required_argument, NULL, OPT_SSL_CIPHERS}, \ + {"ssl-ciphersuites", required_argument, NULL, OPT_SSL_CIPHERSUITES} #define STREAM_SSL_OPTION_HANDLERS \ case 'p': \ @@ -58,6 +61,10 @@ void stream_ssl_set_ciphers(const char *arg); \ case OPT_SSL_CIPHERS: \ stream_ssl_set_ciphers(optarg); \ + break; \ + \ + case OPT_SSL_CIPHERSUITES: \ + stream_ssl_set_ciphersuites(optarg); \ break; #define STREAM_SSL_CASES \ diff --git a/lib/stream.c b/lib/stream.c index 3fb4d11cffd..bbfbc489b8d 100644 --- a/lib/stream.c +++ b/lib/stream.c @@ -159,8 +159,11 @@ stream_usage(const char *name, bool active, bool passive, "to read or create\n"); } printf("SSL/TLS options:\n" - " --ssl-protocols=PROTOS range of SSL/TLS protocols to enable\n" - " --ssl-ciphers=CIPHERS list of SSL/TLS ciphers 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" + " with TLSv1.2 and earlier\n" + " --ssl-ciphersuites=SUITES list of SSL/TLS ciphersuites to\n" + " enable with TLSv1.3 and later\n"); #endif } diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index 13c7543cce4..fbc7ad5efe3 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -73,6 +73,7 @@ static char *certificate_file; static char *ca_cert_file; static char *ssl_protocols; static char *ssl_ciphers; +static char *ssl_ciphersuites; static bool bootstrap_ca_cert; /* Try to reclaim heap memory back to system after DB compaction. */ @@ -1792,17 +1793,21 @@ reconfigure_ssl(const struct shash *all_dbs) const char *resolved_ca_cert; const char *resolved_ssl_protocols; const char *resolved_ssl_ciphers; + const char *resolved_ssl_ciphersuites; resolved_private_key = query_db_string(all_dbs, private_key_file, &errors); resolved_certificate = query_db_string(all_dbs, certificate_file, &errors); resolved_ca_cert = query_db_string(all_dbs, ca_cert_file, &errors); resolved_ssl_protocols = query_db_string(all_dbs, ssl_protocols, &errors); resolved_ssl_ciphers = query_db_string(all_dbs, ssl_ciphers, &errors); + resolved_ssl_ciphersuites = query_db_string(all_dbs, ssl_ciphersuites, + &errors); stream_ssl_set_key_and_cert(resolved_private_key, resolved_certificate); stream_ssl_set_ca_cert_file(resolved_ca_cert, bootstrap_ca_cert); stream_ssl_set_protocols(resolved_ssl_protocols); stream_ssl_set_ciphers(resolved_ssl_ciphers); + stream_ssl_set_ciphersuites(resolved_ssl_ciphersuites); return errors.string; } @@ -2699,6 +2704,10 @@ parse_options(int argc, char *argv[], ssl_ciphers = optarg; break; + case OPT_SSL_CIPHERSUITES: + ssl_ciphersuites = optarg; + break; + case OPT_BOOTSTRAP_CA_CERT: ca_cert_file = optarg; bootstrap_ca_cert = true; diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at index 88f11ac823b..b71947511ce 100644 --- a/tests/ovsdb-server.at +++ b/tests/ovsdb-server.at @@ -842,11 +842,14 @@ AT_DATA([schema], "certificate": {"type": "string"}, "ca_cert": {"type": "string"}, "ssl_protocols" : {"type": "string"}, - "ssl_ciphers" : {"type" : "string"}}}}} + "ssl_ciphers" : {"type" : "string"}, + "ssl_ciphersuites" : {"type": "string"} + }}}} ]]) AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore]) # The !ECDHE-ECDSA-AES256-GCM-SHA384 in the ssl_ciphers is so that -# a cipher negotiation failure can be tested for later. +# a cipher negotiation failure can be tested for later. Same for +# the TLS_CHACHA20_POLY1305_SHA256 ciphersuite. AT_CHECK( [[ovsdb-tool transact db \ '["mydb", @@ -855,8 +858,10 @@ AT_CHECK( "row": {"private_key": "'"$PKIDIR/testpki-privkey2.pem"'", "certificate": "'"$PKIDIR/testpki-cert2.pem"'", "ca_cert": "'"$PKIDIR/testpki-cacert.pem"'", - "ssl_protocols": "'"TLSv1.2,TLSv1.1"'", - "ssl_ciphers": "'"DEFAULT:@SECLEVEL=2:!ECDHE-ECDSA-AES256-GCM-SHA384"'"}}]']], + "ssl_protocols": "'"TLSv1.3,TLSv1.2"'", + "ssl_ciphers": "'"DEFAULT:@SECLEVEL=2:!ECDHE-ECDSA-AES256-GCM-SHA384"'", + "ssl_ciphersuites": "TLS_CHACHA20_POLY1305_SHA256" + }}]']], [0], [ignore], [ignore]) on_exit 'kill `cat *.pid`' AT_CHECK( @@ -866,11 +871,12 @@ AT_CHECK( --ca-cert=db:mydb,SSL,ca_cert \ --ssl-protocols=db:mydb,SSL,ssl_protocols \ --ssl-ciphers=db:mydb,SSL,ssl_ciphers \ + --ssl-ciphersuites=db:mydb,SSL,ssl_ciphersuites \ --remote=pssl:0:127.0.0.1 db], [0], [ignore], [ignore]) PARSE_LISTENING_PORT([ovsdb-server.log], [SSL_PORT]) -# SSL_OVSDB_CLIENT(PROTOCOL, [CIPHERS]) +# SSL_OVSDB_CLIENT(PROTOCOL, [CIPHERS], [CIPHERSUITES]) m4_define([SSL_OVSDB_CLIENT], [dnl ovsdb-client -vconsole:stream_ssl:dbg \ --private-key=$PKIDIR/testpki-privkey.pem \ @@ -878,6 +884,7 @@ m4_define([SSL_OVSDB_CLIENT], [dnl --ca-cert=$PKIDIR/testpki-cacert.pem \ --ssl-protocols=[$1] \ m4_if([$2], [], [], [--ssl-ciphers=$2]) \ + m4_if([$3], [], [], [--ssl-ciphersuites=$3]) \ transact ssl:127.0.0.1:$SSL_PORT \ '[[["mydb", {"op": "select", @@ -885,26 +892,40 @@ m4_define([SSL_OVSDB_CLIENT], [dnl "where": [], "columns": ["private_key"]}]]]']) -AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.2,TLSv1.1]), [0], [stdout], [ignore]) -cat stdout >> output +AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.3,TLSv1.2]), [0], [stdout], [ignore]) +AT_CHECK_UNQUOTED( + [cat stdout], [0], + [[@<:@{"rows":@<:@{"private_key":"$PKIDIR/testpki-privkey2.pem"}@:>@}@:>@ +]], [ignore]) + +# Check that connection over TLSv1.2 works. +AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.2]), [0], [stdout], [ignore]) AT_CHECK_UNQUOTED( - [cat output], [0], + [cat stdout], [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(SSL_OVSDB_CLIENT([TLSv1]), [1], [stdout], [stderr]) + +# Check that connection over TLSv1.3 works. +AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.3]), [0], [stdout], [ignore]) +AT_CHECK_UNQUOTED( + [cat stdout], [0], + [[@<:@{"rows":@<:@{"private_key":"$PKIDIR/testpki-privkey2.pem"}@:>@}@:>@ +]], [ignore]) + +# Check that, when the server has TLSv1.2+ and the client has +# TLSv1.1, connection fails. +AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.1]), [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 +AT_CHECK([grep -q 'TLSv1.1 protocol is deprecated' output]) +AT_CHECK([grep -q 'Enabled protocol range: TLSv1.1 - TLSv1.1' stderr]) +# Check that when ciphers are not compatible, a negotiation # failure occurs. -AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.2,TLSv1.1], [ECDHE-ECDSA-AES256-GCM-SHA384]), +AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.2], [ECDHE-ECDSA-AES256-GCM-SHA384]), [1], [stdout], [stderr]) cat stderr > output AT_CHECK_UNQUOTED( @@ -912,7 +933,6 @@ AT_CHECK_UNQUOTED( [ovsdb-client: failed to connect to "ssl:127.0.0.1:$SSL_PORT" ], [ignore]) -AT_CHECK([grep -q 'TLSv1.1 protocol is deprecated' output]) # The error message for being unable to negotiate a shared ciphersuite # is 'sslv3 alert handshake failure'. This is not the clearest message. # In openssl 3.2.0 all the error messages were updated to replace 'sslv3' @@ -922,22 +942,33 @@ AT_CHECK_UNQUOTED( [stdout], [ignore]) +# Check that when ciphersuites are not compatible, a negotiation +# failure occurs. +AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.3], [DEFAULT], [TLS_AES_256_GCM_SHA384]), + [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 -E "(sslv3|ssl/tls) alert handshake failure" output]) + # 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(SSL_OVSDB_CLIENT([TLSv1.1,TLSv1.3]), [0], [stdout], [stderr]) +AT_CHECK([grep -q 'Enabled protocol range: TLSv1.1 - TLSv1.3' 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]) + 'TLSv1.2 is not configured, but will be enabled anyway' stderr]) +AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.1-TLSv1.3]), [0], [stdout], [stderr]) +AT_CHECK([grep -q 'Enabled protocol range: TLSv1.1 - TLSv1.3' 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(SSL_OVSDB_CLIENT([TLSv1.3-TLSv1.1]), [0], [stdout], [stderr]) +AT_CHECK([grep -q 'Enabled protocol range: TLSv1.1 - TLSv1.3' 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(SSL_OVSDB_CLIENT([TLSv1.3+]), [0], [stdout], [stderr]) +AT_CHECK([grep -q 'Enabled protocol range: TLSv1.3 or later' stderr]) +AT_CHECK(SSL_OVSDB_CLIENT([TLSv1.2-TLSv1.3,TLSv1.3+]), [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(SSL_OVSDB_CLIENT([TLSv1.2+TLSv1.3]), [0], [stdout], [stderr]) AT_CHECK([grep -q 'SSL/TLS protocol not recognized' stderr]) OVSDB_SERVER_SHUTDOWN(["