Send supported_versions in an HRR

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

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

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

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"},

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;
 

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,

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],
 

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],
 

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);