From 73cc84a132a08a02253ae168600fc4d16cd400d8 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 14 May 2018 18:35:30 +0100 Subject: [PATCH] Suport TLSv1.3 draft 28 Also retains support for drafts 27 and 26 Fixes #6257 Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/6258) --- include/openssl/tls1.h | 10 +++++++--- ssl/ssl_locl.h | 2 ++ ssl/statem/extensions_clnt.c | 8 ++++++-- ssl/statem/extensions_srvr.c | 4 ++-- ssl/statem/statem_lib.c | 17 ++++++++++++++++- ssl/t1_trce.c | 14 ++++++++++++-- test/recipes/70-test_sslversions.t | 7 ++++--- util/perl/TLSProxy/Record.pm | 2 +- 8 files changed, 50 insertions(+), 14 deletions(-) diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h index b9f0918f3e..37bdc7da43 100644 --- a/include/openssl/tls1.h +++ b/include/openssl/tls1.h @@ -30,9 +30,13 @@ extern "C" { # define TLS1_3_VERSION 0x0304 # define TLS_MAX_VERSION TLS1_3_VERSION -/* TODO(TLS1.3) REMOVE ME: Version indicator for draft -26 */ -# define TLS1_3_VERSION_DRAFT 0x7f1a -# define TLS1_3_VERSION_DRAFT_TXT "TLS 1.3 (draft 26)" +/* TODO(TLS1.3) REMOVE ME: Version indicators for draft version */ +# define TLS1_3_VERSION_DRAFT_26 0x7f1a +# define TLS1_3_VERSION_DRAFT_27 0x7f1b +# define TLS1_3_VERSION_DRAFT 0x7f1c +# define TLS1_3_VERSION_DRAFT_TXT_26 "TLS 1.3 (draft 26)" +# define TLS1_3_VERSION_DRAFT_TXT_27 "TLS 1.3 (draft 27)" +# define TLS1_3_VERSION_DRAFT_TXT "TLS 1.3 (draft 28)" /* Special value for method supporting multiple versions */ # define TLS_ANY_VERSION 0x10000 diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index c066e14b70..e02f5a1839 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1057,6 +1057,8 @@ struct ssl_st { * DTLS1_VERSION) */ int version; + /* TODO(TLS1.3): Remove this before release */ + int version_draft; /* SSLv3 */ const SSL_METHOD *method; /* diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index e4a5b3cddc..cc4563b357 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -538,7 +538,9 @@ EXT_RETURN tls_construct_ctos_supported_versions(SSL *s, WPACKET *pkt, for (currv = max_version; currv >= min_version; currv--) { /* TODO(TLS1.3): Remove this first if clause prior to release!! */ if (currv == TLS1_3_VERSION) { - if (!WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT)) { + if (!WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT) + || !WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT_27) + || !WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT_26)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS, ERR_R_INTERNAL_ERROR); @@ -1789,7 +1791,9 @@ int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context, } /* TODO(TLS1.3): Remove this before release */ - if (version == TLS1_3_VERSION_DRAFT) + if (version == TLS1_3_VERSION_DRAFT + || version == TLS1_3_VERSION_DRAFT_27 + || version == TLS1_3_VERSION_DRAFT_26) version = TLS1_3_VERSION; /* diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index ec4e1b8139..65b9d3b3d4 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -892,7 +892,7 @@ int tls_parse_ctos_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x, if (!WPACKET_put_bytes_u16(&hrrpkt, TLSEXT_TYPE_supported_versions) || !WPACKET_start_sub_packet_u16(&hrrpkt) /* TODO(TLS1.3): Fix this before release */ - || !WPACKET_put_bytes_u16(&hrrpkt, TLS1_3_VERSION_DRAFT) + || !WPACKET_put_bytes_u16(&hrrpkt, s->version_draft) || !WPACKET_close(&hrrpkt)) { WPACKET_cleanup(&hrrpkt); SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_CTOS_COOKIE, @@ -1606,7 +1606,7 @@ EXT_RETURN tls_construct_stoc_supported_versions(SSL *s, WPACKET *pkt, if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions) || !WPACKET_start_sub_packet_u16(pkt) /* TODO(TLS1.3): Update to remove the TLSv1.3 draft indicator */ - || !WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT) + || !WPACKET_put_bytes_u16(pkt, s->version_draft) || !WPACKET_close(pkt)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS, diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 5db5c80a88..91d304e2b4 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -1695,6 +1695,8 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd) unsigned int best_vers = 0; const SSL_METHOD *best_method = NULL; PACKET versionslist; + /* TODO(TLS1.3): Remove this before release */ + unsigned int orig_candidate = 0; suppversions->parsed = 1; @@ -1705,8 +1707,18 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd) while (PACKET_get_net_2(&versionslist, &candidate_vers)) { /* TODO(TLS1.3): Remove this before release */ - if (candidate_vers == TLS1_3_VERSION_DRAFT) + if (candidate_vers == TLS1_3_VERSION_DRAFT + || candidate_vers == TLS1_3_VERSION_DRAFT_27 + || candidate_vers == TLS1_3_VERSION_DRAFT_26) { + if (best_vers == TLS1_3_VERSION + && orig_candidate > candidate_vers) + continue; + orig_candidate = candidate_vers; candidate_vers = TLS1_3_VERSION; + } else if (candidate_vers == TLS1_3_VERSION) { + /* Don't actually accept real TLSv1.3 */ + continue; + } /* * TODO(TLS1.3): There is some discussion on the TLS list about * whether to ignore versions version = best_vers; + /* TODO(TLS1.3): Remove this before release */ + if (best_vers == TLS1_3_VERSION) + s->version_draft = orig_candidate; s->method = best_method; return 0; } diff --git a/ssl/t1_trce.c b/ssl/t1_trce.c index 5287326e5f..4d052d0705 100644 --- a/ssl/t1_trce.c +++ b/ssl/t1_trce.c @@ -65,7 +65,9 @@ static const ssl_trace_tbl ssl_version_tbl[] = { {TLS1_1_VERSION, "TLS 1.1"}, {TLS1_2_VERSION, "TLS 1.2"}, {TLS1_3_VERSION, "TLS 1.3"}, - /* TODO(TLS1.3): Remove this line before release */ + /* TODO(TLS1.3): Remove these lines before release */ + {TLS1_3_VERSION_DRAFT_26, TLS1_3_VERSION_DRAFT_TXT_26}, + {TLS1_3_VERSION_DRAFT_27, TLS1_3_VERSION_DRAFT_TXT_27}, {TLS1_3_VERSION_DRAFT, TLS1_3_VERSION_DRAFT_TXT}, {DTLS1_VERSION, "DTLS 1.0"}, {DTLS1_2_VERSION, "DTLS 1.2"}, @@ -642,7 +644,15 @@ static int ssl_print_version(BIO *bio, int indent, const char *name, vers = ((*pmsg)[0] << 8) | (*pmsg)[1]; if (version != NULL) { /* TODO(TLS1.3): Remove the draft conditional here before release */ - *version = (vers == TLS1_3_VERSION_DRAFT) ? TLS1_3_VERSION : vers; + switch(vers) { + case TLS1_3_VERSION_DRAFT_26: + case TLS1_3_VERSION_DRAFT_27: + case TLS1_3_VERSION_DRAFT: + *version = TLS1_3_VERSION; + break; + default: + *version = vers; + } } BIO_indent(bio, indent, 80); BIO_printf(bio, "%s=0x%x (%s)\n", diff --git a/test/recipes/70-test_sslversions.t b/test/recipes/70-test_sslversions.t index ab5a0a2682..5c9ee6c67f 100644 --- a/test/recipes/70-test_sslversions.t +++ b/test/recipes/70-test_sslversions.t @@ -138,7 +138,8 @@ sub modify_supported_versions_filter $ext = pack "C5", 0x04, # Length 0x03, 0x03, #TLSv1.2 - 0x03, 0x04; #TLSv1.3 + #TODO(TLS1.3): Fix before release + 0x7f, 0x1c; #TLSv1.3 (draft 28) } elsif ($testtype == UNRECOGNISED_VERSIONS) { $ext = pack "C5", 0x04, # Length @@ -152,8 +153,8 @@ sub modify_supported_versions_filter } elsif ($testtype == WITH_TLS1_4) { $ext = pack "C5", 0x04, # Length - 0x03, 0x05, #TLSv1.4 - 0x03, 0x04; #TLSv1.3 + #TODO(TLS1.3): Fix before release + 0x7f, 0x1c; #TLSv1.3 (draft 28) } if ($testtype == REVERSE_ORDER_VERSIONS || $testtype == UNRECOGNISED_VERSIONS diff --git a/util/perl/TLSProxy/Record.pm b/util/perl/TLSProxy/Record.pm index 8ff948b82f..9de51b371a 100644 --- a/util/perl/TLSProxy/Record.pm +++ b/util/perl/TLSProxy/Record.pm @@ -36,7 +36,7 @@ my %record_type = ( use constant { VERS_TLS_1_4 => 0x0305, - VERS_TLS_1_3_DRAFT => 0x7f1a, + VERS_TLS_1_3_DRAFT => 0x7f1c, VERS_TLS_1_3 => 0x0304, VERS_TLS_1_2 => 0x0303, VERS_TLS_1_1 => 0x0302, -- 2.25.1