From c3043dcd55d81617408025b1cdb8241ef753b805 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 22 Mar 2017 11:50:32 +0000 Subject: [PATCH] Add client side support for TLSv1.3 downgrade mechanism Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3022) --- ssl/ssl_locl.h | 3 +- ssl/statem/statem_clnt.c | 27 ++++++++------- ssl/statem/statem_lib.c | 73 +++++++++++++++++++++++++++++++++++----- 3 files changed, 80 insertions(+), 23 deletions(-) diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 98f77ceac9..4a9e599818 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -2175,7 +2175,8 @@ __owur int ssl_check_version_downgrade(SSL *s); __owur int ssl_set_version_bound(int method_version, int version, int *bound); __owur int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd); -__owur int ssl_choose_client_version(SSL *s, int version); +__owur int ssl_choose_client_version(SSL *s, int version, int checkdgrd, + int *al); int ssl_get_client_min_max_version(const SSL *s, int *min_version, int *max_version); diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 04908130f7..1342272a78 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1317,10 +1317,20 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) goto f_err; } - /* We do this immediately so we know what format the ServerHello is in */ - protverr = ssl_choose_client_version(s, sversion); + /* load the server random */ + if (!PACKET_copy_bytes(pkt, s->s3->server_random, SSL3_RANDOM_SIZE)) { + al = SSL_AD_DECODE_ERROR; + SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_LENGTH_MISMATCH); + goto f_err; + } + + /* + * We do this immediately so we know what format the ServerHello is in. + * Must be done after reading the random data so we can check for the + * TLSv1.3 downgrade sentinels + */ + protverr = ssl_choose_client_version(s, sversion, 1, &al); if (protverr != 0) { - al = SSL_AD_PROTOCOL_VERSION; SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, protverr); goto f_err; } @@ -1335,14 +1345,6 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) goto f_err; } - /* load the server hello data */ - /* load the server random */ - if (!PACKET_copy_bytes(pkt, s->s3->server_random, SSL3_RANDOM_SIZE)) { - al = SSL_AD_DECODE_ERROR; - SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_LENGTH_MISMATCH); - goto f_err; - } - /* Get the session-id. */ if (!SSL_IS_TLS13(s)) { if (!PACKET_get_length_prefixed_1(pkt, &session_id)) { @@ -1609,9 +1611,8 @@ static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt) s->hello_retry_request = 1; /* This will fail if it doesn't choose TLSv1.3+ */ - errorcode = ssl_choose_client_version(s, sversion); + errorcode = ssl_choose_client_version(s, sversion, 0, &al); if (errorcode != 0) { - al = SSL_AD_PROTOCOL_VERSION; SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, errorcode); goto f_err; } diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 41a375104d..f098213869 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -1681,22 +1681,32 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd) * * @s: client SSL handle. * @version: The proposed version from the server's HELLO. + * @checkdgrd: Whether to check the downgrade sentinels in the server_random + * @al: Where to store any alert value that may be generated * * Returns 0 on success or an SSL error reason number on failure. */ -int ssl_choose_client_version(SSL *s, int version) +int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al) { const version_info *vent; const version_info *table; + int highver = 0; /* TODO(TLS1.3): Remove this before release */ if (version == TLS1_3_VERSION_DRAFT) version = TLS1_3_VERSION; + if (s->hello_retry_request && version != TLS1_3_VERSION) { + *al = SSL_AD_PROTOCOL_VERSION; + return SSL_R_WRONG_SSL_VERSION; + } + switch (s->method->version) { default: - if (version != s->version) + if (version != s->version) { + *al = SSL_AD_PROTOCOL_VERSION; return SSL_R_WRONG_SSL_VERSION; + } /* * If this SSL handle is not from a version flexible method we don't * (and never did) check min/max, FIPS or Suite B constraints. Hope @@ -1716,23 +1726,68 @@ int ssl_choose_client_version(SSL *s, int version) for (vent = table; vent->version != 0; ++vent) { const SSL_METHOD *method; int err; +#ifndef OPENSSL_NO_TLS13DOWNGRADE + static const unsigned char tls11downgrade[] = { + 0x44, 0x4f, 0x57, 0x4e, 0x47, 0x52, 0x44, 0x00 + }; + static const unsigned char tls12downgrade[] = { + 0x44, 0x4f, 0x57, 0x4e, 0x47, 0x52, 0x44, 0x01 + }; +#endif - if (version != vent->version) - continue; if (vent->cmeth == NULL) - break; - if (s->hello_retry_request && version != TLS1_3_VERSION) - return SSL_R_WRONG_SSL_VERSION; + continue; + + if (highver != 0 && version != vent->version) + continue; method = vent->cmeth(); err = ssl_method_error(s, method); - if (err != 0) - return err; + if (err != 0) { + if (version == vent->version) { + *al = SSL_AD_PROTOCOL_VERSION; + return err; + } + + continue; + } + if (highver == 0) + highver = vent->version; + + if (version != vent->version) + continue; + +#ifndef OPENSSL_NO_TLS13DOWNGRADE + /* Check for downgrades */ + if (checkdgrd) { + if (version == TLS1_2_VERSION && highver > version) { + if (memcmp(tls12downgrade, + s->s3->server_random + SSL3_RANDOM_SIZE + - sizeof(tls12downgrade), + sizeof(tls12downgrade)) == 0) { + *al = SSL_AD_ILLEGAL_PARAMETER; + return SSL_R_INAPPROPRIATE_FALLBACK; + } + } else if (!SSL_IS_DTLS(s) + && version < TLS1_2_VERSION + && highver > version) { + if (memcmp(tls11downgrade, + s->s3->server_random + SSL3_RANDOM_SIZE + - sizeof(tls11downgrade), + sizeof(tls11downgrade)) == 0) { + *al = SSL_AD_ILLEGAL_PARAMETER; + return SSL_R_INAPPROPRIATE_FALLBACK; + } + } + } +#endif + s->method = method; s->version = version; return 0; } + *al = SSL_AD_PROTOCOL_VERSION; return SSL_R_UNSUPPORTED_PROTOCOL; } -- 2.40.0