From: Matt Caswell Date: Sat, 22 Oct 2016 23:41:11 +0000 (+0100) Subject: Add server side support for supported_versions extension X-Git-Tag: OpenSSL_1_1_1-pre1~3129 X-Git-Url: https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff_plain;h=cd99883755f428ac47e8e2ccb21333b675ec22d9 Add server side support for supported_versions extension Reviewed-by: Rich Salz --- diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 0c7aeedfeb..31e1fd52dd 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -2077,6 +2077,9 @@ __owur int dtls1_process_heartbeat(SSL *s, unsigned char *p, size_t length); # endif +__owur RAW_EXTENSION *tls_get_extension_by_type(RAW_EXTENSION *exts, + size_t numexts, + unsigned int type); __owur int tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello, SSL_SESSION **ret); __owur int tls_check_client_ems_support(SSL *s, const CLIENTHELLO_MSG *hello); diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 6a05b9dd24..b8bca0e4dc 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -779,8 +779,13 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt) * TLS 1.0 and renegotiating with TLS 1.2. We do this by using * client_version in client hello and not resetting it to * the negotiated version. + * + * For TLS 1.3 we always set the ClientHello version to 1.2 and rely on the + * supported_versions extension for the reall supported versions. */ - if (!WPACKET_put_bytes_u16(pkt, s->client_version) + if (!WPACKET_put_bytes_u16(pkt, + (!SSL_IS_DTLS(s) && s->client_version >= TLS1_3_VERSION) + ? TLS1_2_VERSION : s->client_version) || !WPACKET_memcpy(pkt, s->s3->client_random, SSL3_RANDOM_SIZE)) { SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR); return 0; diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index a3a31bc740..a7f2a0f5d7 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -996,6 +996,7 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello) const version_info *vent; const version_info *table; int disabled = 0; + RAW_EXTENSION *suppversions; s->client_version = client_version; @@ -1019,6 +1020,73 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello) break; } + suppversions = tls_get_extension_by_type(hello->pre_proc_exts, + hello->num_extensions, + TLSEXT_TYPE_supported_versions); + + /* + * TODO(TLS1.3): We only look at this if our max protocol version is TLS1.3 + * or above. Should we allow it for lower versions too? + */ + if (suppversions != NULL && !SSL_IS_DTLS(s) + && (s->max_proto_version == 0 + || TLS1_3_VERSION <= s->max_proto_version)) { + unsigned int candidate_vers = 0; + unsigned int best_vers = 0; + const SSL_METHOD *best_method = NULL; + PACKET versionslist; + + if (!PACKET_get_length_prefixed_1(&suppversions->data, &versionslist) + || PACKET_remaining(&suppversions->data) != 0) { + /* Trailing or invalid data? */ + return SSL_R_LENGTH_MISMATCH; + } + + while (PACKET_get_net_2(&versionslist, &candidate_vers)) { + /* TODO(TLS1.3): Remove this before release */ + if (candidate_vers == TLS1_3_VERSION_DRAFT) + candidate_vers = TLS1_3_VERSION; + if ((int)candidate_vers > s->client_version) + s->client_version = candidate_vers; + if (version_cmp(s, candidate_vers, best_vers) <= 0) + continue; + for (vent = table; + vent->version != 0 && vent->version != (int)candidate_vers; + ++vent); + if (vent->version != 0) { + const SSL_METHOD *method; + + method = vent->smeth(); + if (ssl_method_error(s, method) == 0) { + best_vers = candidate_vers; + best_method = method; + } + } + } + if (PACKET_remaining(&versionslist) != 0) { + /* Trailing data? */ + return SSL_R_LENGTH_MISMATCH; + } + + if (best_vers > 0) { + s->version = best_vers; + s->method = best_method; + return 0; + } + return SSL_R_UNSUPPORTED_PROTOCOL; + } + + /* + * If the supported versions extension isn't present, then the highest + * version we can negotiate is TLSv1.2 + */ + if (version_cmp(s, client_version, TLS1_3_VERSION) >= 0) + client_version = TLS1_2_VERSION; + + /* + * No supported versions extension, so we just use the version supplied in + * the ClientHello. + */ for (vent = table; vent->version != 0; ++vent) { const SSL_METHOD *method; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 515b4e33af..2122726213 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -2826,8 +2826,8 @@ int ssl_parse_serverhello_tlsext(SSL *s, PACKET *pkt) * * Returns a pointer to the found RAW_EXTENSION data, or NULL if not found. */ -static RAW_EXTENSION *get_extension_by_type(RAW_EXTENSION *exts, size_t numexts, - unsigned int type) +RAW_EXTENSION *tls_get_extension_by_type(RAW_EXTENSION *exts, size_t numexts, + unsigned int type) { size_t loop; @@ -2885,9 +2885,9 @@ int tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello, if (s->version <= SSL3_VERSION || !tls_use_ticket(s)) return 0; - ticketext = get_extension_by_type(hello->pre_proc_exts, - hello->num_extensions, - TLSEXT_TYPE_session_ticket); + ticketext = tls_get_extension_by_type(hello->pre_proc_exts, + hello->num_extensions, + TLSEXT_TYPE_session_ticket); if (ticketext == NULL) return 0; @@ -2948,8 +2948,9 @@ int tls_check_client_ems_support(SSL *s, const CLIENTHELLO_MSG *hello) if (s->version <= SSL3_VERSION) return 1; - emsext = get_extension_by_type(hello->pre_proc_exts, hello->num_extensions, - TLSEXT_TYPE_extended_master_secret); + emsext = tls_get_extension_by_type(hello->pre_proc_exts, + hello->num_extensions, + TLSEXT_TYPE_extended_master_secret); /* * No extensions is a success - we have successfully discovered that the diff --git a/test/recipes/70-test_sslvertol.t b/test/recipes/70-test_sslvertol.t index f8c94e91df..46fc9b6cc7 100755 --- a/test/recipes/70-test_sslvertol.t +++ b/test/recipes/70-test_sslvertol.t @@ -34,15 +34,18 @@ my $proxy = TLSProxy::Proxy->new( (!$ENV{HARNESS_ACTIVE} || $ENV{HARNESS_VERBOSE}) ); -#Test 1: Asking for TLS1.3 should pass -my $client_version = TLSProxy::Record::VERS_TLS_1_3; +#Test 1: Asking for TLS1.4 should pass +my $client_version = TLSProxy::Record::VERS_TLS_1_4; +#We don't want the supported versions extension for this test +$proxy->clientflags("-no_tls1_3"); $proxy->start() or plan skip_all => "Unable to start up Proxy for tests"; plan tests => 2; -ok(TLSProxy::Message->success(), "Version tolerance test, TLS 1.3"); +ok(TLSProxy::Message->success(), "Version tolerance test, TLS 1.4"); #Test 2: Testing something below SSLv3 should fail $client_version = TLSProxy::Record::VERS_SSL_3_0 - 1; $proxy->clear(); +$proxy->clientflags("-no_tls1_3"); $proxy->start(); ok(TLSProxy::Message->fail(), "Version tolerance test, SSL < 3.0"); diff --git a/test/ssl-tests/17-renegotiate.conf b/test/ssl-tests/17-renegotiate.conf index c47a9567dd..fffb572a47 100644 --- a/test/ssl-tests/17-renegotiate.conf +++ b/test/ssl-tests/17-renegotiate.conf @@ -18,6 +18,7 @@ client = 0-renegotiate-client-no-resume-client [0-renegotiate-client-no-resume-server] Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem CipherString = DEFAULT +MaxProtocol = TLSv1.2 Options = NoResumptionOnRenegotiation PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem @@ -45,6 +46,7 @@ client = 1-renegotiate-client-resume-client [1-renegotiate-client-resume-server] Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem CipherString = DEFAULT +MaxProtocol = TLSv1.2 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [1-renegotiate-client-resume-client] @@ -71,6 +73,7 @@ client = 2-renegotiate-server-no-resume-client [2-renegotiate-server-no-resume-server] Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem CipherString = DEFAULT +MaxProtocol = TLSv1.2 Options = NoResumptionOnRenegotiation PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem @@ -98,6 +101,7 @@ client = 3-renegotiate-server-resume-client [3-renegotiate-server-resume-server] Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem CipherString = DEFAULT +MaxProtocol = TLSv1.2 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [3-renegotiate-server-resume-client] diff --git a/test/ssl-tests/17-renegotiate.conf.in b/test/ssl-tests/17-renegotiate.conf.in index a081617724..ab581eca72 100644 --- a/test/ssl-tests/17-renegotiate.conf.in +++ b/test/ssl-tests/17-renegotiate.conf.in @@ -19,7 +19,8 @@ our @tests = ( { name => "renegotiate-client-no-resume", server => { - "Options" => "NoResumptionOnRenegotiation" + "Options" => "NoResumptionOnRenegotiation", + "MaxProtocol" => "TLSv1.2" }, client => {}, test => { @@ -31,7 +32,9 @@ our @tests = ( }, { name => "renegotiate-client-resume", - server => {}, + server => { + "MaxProtocol" => "TLSv1.2" + }, client => {}, test => { "Method" => "TLS", @@ -43,7 +46,8 @@ our @tests = ( { name => "renegotiate-server-no-resume", server => { - "Options" => "NoResumptionOnRenegotiation" + "Options" => "NoResumptionOnRenegotiation", + "MaxProtocol" => "TLSv1.2" }, client => {}, test => { @@ -55,7 +59,9 @@ our @tests = ( }, { name => "renegotiate-server-resume", - server => {}, + server => { + "MaxProtocol" => "TLSv1.2" + }, client => {}, test => { "Method" => "TLS", diff --git a/util/TLSProxy/Record.pm b/util/TLSProxy/Record.pm index a4e7adcf82..bf6de439ad 100644 --- a/util/TLSProxy/Record.pm +++ b/util/TLSProxy/Record.pm @@ -35,6 +35,7 @@ my %record_type = ( ); use constant { + VERS_TLS_1_4 => 773, VERS_TLS_1_3 => 772, VERS_TLS_1_2 => 771, VERS_TLS_1_1 => 770,