diff options
author | Matt Caswell <matt@openssl.org> | 2018-03-13 10:36:03 +0000 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2018-03-14 09:51:20 +0000 |
commit | 27e462f1b0c8d6295c745611e36beb5774de6688 (patch) | |
tree | abb6f14f8acdd950662a5a1c032d370ae8b53e4c /ssl | |
parent | 3295d2423889496e0933b3f9af6dc692c9f9a8f2 (diff) |
Only allow supported_versions in a TLSv1.3 ServerHello
As per the latest text in TLSv1.3 draft-26
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/5604)
Diffstat (limited to 'ssl')
-rw-r--r-- | ssl/statem/extensions.c | 5 | ||||
-rw-r--r-- | ssl/statem/extensions_clnt.c | 24 | ||||
-rw-r--r-- | ssl/statem/extensions_srvr.c | 8 |
3 files changed, 20 insertions, 17 deletions
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 0641a253d3..3dc4e8ed94 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -307,9 +307,8 @@ static const EXTENSION_DEFINITION ext_defs[] = { }, { TLSEXT_TYPE_supported_versions, - SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO - | SSL_EXT_TLS1_3_SERVER_HELLO | SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST - | SSL_EXT_TLS_IMPLEMENTATION_ONLY, + SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_SERVER_HELLO + | SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST | SSL_EXT_TLS_IMPLEMENTATION_ONLY, NULL, /* Processed inline as part of version selection */ NULL, tls_parse_stoc_supported_versions, diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index 9bf2d1cb38..bd025d7c02 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -1780,20 +1780,20 @@ int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context, if (version == TLS1_3_VERSION_DRAFT) version = TLS1_3_VERSION; + /* + * The only protocol version we support which is valid in this extension in + * a ServerHello is TLSv1.3 therefore we shouldn't be getting anything else. + */ + if (version != TLS1_3_VERSION) { + SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, + SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS, + SSL_R_BAD_PROTOCOL_VERSION_NUMBER); + return 0; + } + /* We ignore this extension for HRRs except to sanity check it */ - if (context == SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) { - /* - * The only protocol version we support which has an HRR message is - * TLSv1.3, therefore we shouldn't be getting an HRR for anything else. - */ - if (version != TLS1_3_VERSION) { - SSLfatal(s, SSL_AD_PROTOCOL_VERSION, - SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS, - SSL_R_BAD_HRR_VERSION); - return 0; - } + if (context == SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) return 1; - } /* We just set it here. We validate it in ssl_choose_client_version */ s->version = version; diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 425cd80efe..a1f92b076d 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -1572,8 +1572,12 @@ EXT_RETURN tls_construct_stoc_supported_versions(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx) { - if (!SSL_IS_TLS13(s)) - return EXT_RETURN_NOT_SENT; + if (!ossl_assert(SSL_IS_TLS13(s))) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS, + ERR_R_INTERNAL_ERROR); + return EXT_RETURN_FAIL; + } if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions) || !WPACKET_start_sub_packet_u16(pkt) |