Skip to content

Commit

Permalink
stream-ssl: Add explicit support for configuring TLSv1.3.
Browse files Browse the repository at this point in the history 
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.

Signed-off-by: Ilya Maximets <[email protected]>
  • Loading branch information
@igsilya
igsilya committed Dec 9, 2024
1 parent 9d0e552 commit f0f9e9d
Show file tree
Hide file tree
Showing 9 changed files with 118 additions and 32 deletions.
5 changes: 5 additions & 0 deletions NEWS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/ssl-connect-syn.man
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@
.br
[\fB\-\-ssl\-ciphers=\fIciphers\fR]
.br
[\fB\-\-ssl\-ciphersuites=\fIciphersuites\fR]
.br
11 changes: 8 additions & 3 deletions lib/ssl-connect.man
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ 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.
7 changes: 7 additions & 0 deletions lib/stream-nossl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
}
21 changes: 19 additions & 2 deletions lib/stream-ssl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,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
Expand Down Expand Up @@ -1223,8 +1224,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)
{
Expand All @@ -1238,6 +1239,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
Expand All @@ -1257,6 +1273,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;
Expand Down
11 changes: 9 additions & 2 deletions lib/stream-ssl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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': \
Expand All @@ -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 \
Expand Down
7 changes: 5 additions & 2 deletions lib/stream.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 list of SSL/TLS protocols to enable\n"
" --ssl-ciphers=CIPHERS list of SSL/TLS ciphers to enable\n");
" --ssl-protocols=PROTOS list 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
}

Expand Down
9 changes: 9 additions & 0 deletions ovsdb/ovsdb-server.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
77 changes: 54 additions & 23 deletions tests/ovsdb-server.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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(
Expand All @@ -866,53 +871,68 @@ 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 \
--certificate=$PKIDIR/testpki-cert.pem \
--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",
"table": "SSL",
"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(
[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.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'
Expand All @@ -922,20 +942,31 @@ 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])
'TLSv1.2 is not configured, but will be enabled anyway' stderr])
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 \
'TLSv1.1 is not configured, but will be enabled anyway' stderr])
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(["
Expand Down

0 comments on commit f0f9e9d

Please sign in to comment.