Send supported_versions in an HRR

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index af826cd10f73a00e077af532bb76a75922032978..872016f35023c80a4444304f55e109e26daaa447 100644 (file)
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -2361,6 +2361,7 @@ SSL_R_BAD_EXTENSION:110:bad extension
 SSL_R_BAD_HANDSHAKE_LENGTH:332:bad handshake length
 SSL_R_BAD_HANDSHAKE_STATE:236:bad handshake state
 SSL_R_BAD_HELLO_REQUEST:105:bad hello request
+SSL_R_BAD_HRR_VERSION:263:bad hrr version
 SSL_R_BAD_KEY_SHARE:108:bad key share
 SSL_R_BAD_KEY_UPDATE:122:bad key update
 SSL_R_BAD_LENGTH:271:bad length

diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h
index 9564023044816ccc1ec85f7e356adbd0b8b9fafa..2986be84ad5f1ee434dae15a4aa5d05878f2928a 100644 (file)
--- a/include/openssl/sslerr.h
+++ b/include/openssl/sslerr.h
@@ -444,6 +444,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_R_BAD_HANDSHAKE_LENGTH                       332
 # define SSL_R_BAD_HANDSHAKE_STATE                        236
 # define SSL_R_BAD_HELLO_REQUEST                          105
+# define SSL_R_BAD_HRR_VERSION                            263
 # define SSL_R_BAD_KEY_SHARE                              108
 # define SSL_R_BAD_KEY_UPDATE                             122
 # define SSL_R_BAD_LENGTH                                 271

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 08f66961637d0dcffe1f55fa8f88768b9276054b..1e3eb2cc7237336577258047212e7c04b1b8b7c3 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -706,6 +706,7 @@ static const ERR_STRING_DATA SSL_str_reasons[] = {
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_HANDSHAKE_STATE),
     "bad handshake state"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_HELLO_REQUEST), "bad hello request"},
+    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_HRR_VERSION), "bad hrr version"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_KEY_SHARE), "bad key share"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_KEY_UPDATE), "bad key update"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_BAD_LENGTH), "bad length"},

diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 1fbf9f6e0e6580ae56851861456ef7dabb9319ce..f357396d81a4eb8a8e761d60112a74e16ab6dc91 100644 (file)
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -1657,6 +1657,21 @@ int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context,
     if (version == TLS1_3_VERSION_DRAFT)
         version = TLS1_3_VERSION;
 
+    /* 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) {
+            *al = SSL_AD_PROTOCOL_VERSION;
+            SSLerr(SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS,
+                   SSL_R_BAD_HRR_VERSION);
+            return 0;
+        }
+        return 1;
+    }
+
     /* We just set it here. We validate it in ssl_choose_client_version */
     s->version = version;
 

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 7d1d15dcc1e442ccb21a7cf1a61c2c744183a052..4f0487cc0f3d37e21cf1f49d2c21ce5f23779a52 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -2274,7 +2274,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
         compm = s->s3->tmp.new_compression->id;
 #endif
 
-    if (!WPACKET_sub_memcpy_u8(pkt,     session_id, sl)
+    if (!WPACKET_sub_memcpy_u8(pkt, session_id, sl)
             || !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, pkt, &len)
             || !WPACKET_put_bytes_u8(pkt, compm)
             || !tls_construct_extensions(s, pkt,

diff --git a/test/recipes/70-test_tls13kexmodes.t b/test/recipes/70-test_tls13kexmodes.t
index 908ca4a21d9f6d91569749edcaad9fb1ad073873..7afb56092bab9c6e38636491a100373008976faf 100644 (file)
--- a/test/recipes/70-test_tls13kexmodes.t
+++ b/test/recipes/70-test_tls13kexmodes.t
@@ -90,6 +90,8 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf");
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_PSK,
         checkhandshake::PSK_CLI_EXTENSION],
 
+    [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS,
+        checkhandshake::DEFAULT_EXTENSIONS],
     [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_KEY_SHARE,
         checkhandshake::KEY_SHARE_HRR_EXTENSION],
 

diff --git a/test/recipes/70-test_tls13messages.t b/test/recipes/70-test_tls13messages.t
index 4b0552c5ca31225c4102599e39eb827fbd806b39..2cf822aca08f1188e8a3e3c684ab615b9f9d207e 100644 (file)
--- a/test/recipes/70-test_tls13messages.t
+++ b/test/recipes/70-test_tls13messages.t
@@ -90,6 +90,8 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf");
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_PSK,
         checkhandshake::PSK_CLI_EXTENSION],
 
+    [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS,
+        checkhandshake::DEFAULT_EXTENSIONS],
     [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_KEY_SHARE,
         checkhandshake::KEY_SHARE_HRR_EXTENSION],
 

diff --git a/util/perl/TLSProxy/ServerHello.pm b/util/perl/TLSProxy/ServerHello.pm
index 693a652b6cd99d9f6b8a9ce06377311adc992d42..934eaf4dea328d88c6ab2d31787c07b4ee3a47ab 100644 (file)
--- a/util/perl/TLSProxy/ServerHello.pm
+++ b/util/perl/TLSProxy/ServerHello.pm
@@ -50,6 +50,7 @@ sub parse
     my $self = shift;
     my $ptr = 2;
     my ($server_version) = unpack('n', $self->data);
+    my $neg_version = $server_version;
 
     my $random = substr($self->data, $ptr, 32);
     $ptr += 32;
@@ -94,15 +95,15 @@ sub parse
         $extension_data = substr($extension_data, 4 + $size);
         $extensions{$type} = $extdata;
         if ($type == TLSProxy::Message::EXT_SUPPORTED_VERSIONS) {
-            $server_version = unpack('n', $extdata);
+            $neg_version = unpack('n', $extdata);
         }
     }
 
     if ($random eq $hrrrandom) {
         TLSProxy::Proxy->is_tls13(1);
         # TODO(TLS1.3): Replace this reference to draft version before release
-    } elsif ($server_version == TLSProxy::Record::VERS_TLS_1_3_DRAFT) {
-        $server_version = TLSProxy::Record::VERS_TLS_1_3;
+    } elsif ($neg_version == TLSProxy::Record::VERS_TLS_1_3_DRAFT) {
+        $neg_version = TLSProxy::Record::VERS_TLS_1_3;
         TLSProxy::Proxy->is_tls13(1);
 
         TLSProxy::Record->server_encrypting(1);