Merge HRR into ServerHello
authorMatt Caswell <matt@openssl.org>
Tue, 5 Dec 2017 10:14:35 +0000 (10:14 +0000)
committerMatt Caswell <matt@openssl.org>
Thu, 14 Dec 2017 15:06:37 +0000 (15:06 +0000)
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)

21 files changed:
apps/s_cb.c
crypto/err/openssl.txt
include/openssl/ssl.h
include/openssl/ssl3.h
include/openssl/sslerr.h
ssl/ssl_err.c
ssl/ssl_stat.c
ssl/statem/statem_clnt.c
ssl/statem/statem_lib.c
ssl/statem/statem_locl.h
ssl/statem/statem_srvr.c
ssl/t1_trce.c
test/recipes/70-test_key_share.t
test/recipes/70-test_sslrecords.t
test/recipes/70-test_tls13cookie.t
test/recipes/70-test_tls13kexmodes.t
test/recipes/70-test_tls13messages.t
util/perl/TLSProxy/HelloRetryRequest.pm [deleted file]
util/perl/TLSProxy/Message.pm
util/perl/TLSProxy/Proxy.pm
util/perl/checkhandshake.pm

index 79d2919..c7c9ecb 100644 (file)
@@ -525,7 +525,6 @@ static STRINT_PAIR handshakes[] = {
     {", HelloVerifyRequest", DTLS1_MT_HELLO_VERIFY_REQUEST},
     {", NewSessionTicket", SSL3_MT_NEWSESSION_TICKET},
     {", EndOfEarlyData", SSL3_MT_END_OF_EARLY_DATA},
-    {", HelloRetryRequest", SSL3_MT_HELLO_RETRY_REQUEST},
     {", EncryptedExtensions", SSL3_MT_ENCRYPTED_EXTENSIONS},
     {", Certificate", SSL3_MT_CERTIFICATE},
     {", ServerKeyExchange", SSL3_MT_SERVER_KEY_EXCHANGE},
index de7c50a..af826cd 100644 (file)
@@ -1340,6 +1340,7 @@ SSL_F_TLS_POST_PROCESS_CLIENT_HELLO:378:tls_post_process_client_hello
 SSL_F_TLS_POST_PROCESS_CLIENT_KEY_EXCHANGE:384:\
        tls_post_process_client_key_exchange
 SSL_F_TLS_PREPARE_CLIENT_CERTIFICATE:360:tls_prepare_client_certificate
+SSL_F_TLS_PROCESS_AS_HELLO_RETRY_REQUEST:610:tls_process_as_hello_retry_request
 SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST:361:tls_process_certificate_request
 SSL_F_TLS_PROCESS_CERT_STATUS:362:*
 SSL_F_TLS_PROCESS_CERT_STATUS_BODY:495:tls_process_cert_status_body
index 48779fa..98a106b 100644 (file)
@@ -987,8 +987,6 @@ typedef enum {
     TLS_ST_CR_CERT_VRFY,
     TLS_ST_SW_CERT_VRFY,
     TLS_ST_CR_HELLO_REQ,
-    TLS_ST_SW_HELLO_RETRY_REQUEST,
-    TLS_ST_CR_HELLO_RETRY_REQUEST,
     TLS_ST_SW_KEY_UPDATE,
     TLS_ST_CW_KEY_UPDATE,
     TLS_ST_SR_KEY_UPDATE,
index e9d56a8..b781f61 100644 (file)
@@ -289,7 +289,6 @@ extern "C" {
 # define SSL3_MT_SERVER_HELLO                    2
 # define SSL3_MT_NEWSESSION_TICKET               4
 # define SSL3_MT_END_OF_EARLY_DATA               5
-# define SSL3_MT_HELLO_RETRY_REQUEST             6
 # define SSL3_MT_ENCRYPTED_EXTENSIONS            8
 # define SSL3_MT_CERTIFICATE                     11
 # define SSL3_MT_SERVER_KEY_EXCHANGE             12
index 9c86220..9564023 100644 (file)
@@ -385,6 +385,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_POST_PROCESS_CLIENT_HELLO              378
 # define SSL_F_TLS_POST_PROCESS_CLIENT_KEY_EXCHANGE       384
 # define SSL_F_TLS_PREPARE_CLIENT_CERTIFICATE             360
+# define SSL_F_TLS_PROCESS_AS_HELLO_RETRY_REQUEST         610
 # define SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST            361
 # define SSL_F_TLS_PROCESS_CERT_STATUS                    362
 # define SSL_F_TLS_PROCESS_CERT_STATUS_BODY               495
index 3322685..08f6696 100644 (file)
@@ -605,6 +605,8 @@ static const ERR_STRING_DATA SSL_str_functs[] = {
      "tls_post_process_client_key_exchange"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_PREPARE_CLIENT_CERTIFICATE, 0),
      "tls_prepare_client_certificate"},
+    {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_PROCESS_AS_HELLO_RETRY_REQUEST, 0),
+     "tls_process_as_hello_retry_request"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_PROCESS_CERTIFICATE_REQUEST, 0),
      "tls_process_certificate_request"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_PROCESS_CERT_STATUS, 0), ""},
index 5dd71d2..179513b 100644 (file)
@@ -97,10 +97,6 @@ const char *SSL_state_string_long(const SSL *s)
         return "TLSv1.3 write server certificate verify";
     case TLS_ST_CR_HELLO_REQ:
         return "SSLv3/TLS read hello request";
-    case TLS_ST_SW_HELLO_RETRY_REQUEST:
-        return "TLSv1.3 write hello retry request";
-    case TLS_ST_CR_HELLO_RETRY_REQUEST:
-        return "TLSv1.3 read hello retry request";
     case TLS_ST_SW_KEY_UPDATE:
         return "TLSv1.3 write server key update";
     case TLS_ST_CW_KEY_UPDATE:
@@ -208,10 +204,6 @@ const char *SSL_state_string(const SSL *s)
         return "TRSCV";
     case TLS_ST_CR_HELLO_REQ:
         return "TRHR";
-    case TLS_ST_SW_HELLO_RETRY_REQUEST:
-        return "TWHRR";
-    case TLS_ST_CR_HELLO_RETRY_REQUEST:
-        return "TRHRR";
     case TLS_ST_SW_KEY_UPDATE:
         return "TWSKU";
     case TLS_ST_CW_KEY_UPDATE:
index 37c198e..af9e1dc 100644 (file)
@@ -22,7 +22,7 @@
 #include <openssl/bn.h>
 #include <openssl/engine.h>
 
-static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt);
+static MSG_PROCESS_RETURN tls_process_as_hello_retry_request(SSL *s, PACKET *pkt);
 static MSG_PROCESS_RETURN tls_process_encrypted_extensions(SSL *s, PACKET *pkt);
 
 static ossl_inline int cert_req_allowed(SSL *s);
@@ -206,11 +206,6 @@ int ossl_statem_client_read_transition(SSL *s, int mt)
                 st->hand_state = DTLS_ST_CR_HELLO_VERIFY_REQUEST;
                 return 1;
             }
-        } else {
-            if (mt == SSL3_MT_HELLO_RETRY_REQUEST) {
-                st->hand_state = TLS_ST_CR_HELLO_RETRY_REQUEST;
-                return 1;
-            }
         }
         break;
 
@@ -224,10 +219,6 @@ int ossl_statem_client_read_transition(SSL *s, int mt)
             st->hand_state = TLS_ST_CR_SRVR_HELLO;
             return 1;
         }
-        if (mt == SSL3_MT_HELLO_RETRY_REQUEST) {
-            st->hand_state = TLS_ST_CR_HELLO_RETRY_REQUEST;
-            return 1;
-        }
         break;
 
     case TLS_ST_CR_SRVR_HELLO:
@@ -506,7 +497,8 @@ WRITE_TRAN ossl_statem_client_write_transition(SSL *s)
          */
         return WRITE_TRAN_FINISHED;
 
-    case TLS_ST_CR_HELLO_RETRY_REQUEST:
+    case TLS_ST_CR_SRVR_HELLO:
+        /* We only get here in TLSv1.3 */
         st->hand_state = TLS_ST_CW_CLNT_HELLO;
         return WRITE_TRAN_CONTINUE;
 
@@ -912,9 +904,6 @@ size_t ossl_statem_client_max_message_size(SSL *s)
     case DTLS_ST_CR_HELLO_VERIFY_REQUEST:
         return HELLO_VERIFY_REQUEST_MAX_LENGTH;
 
-    case TLS_ST_CR_HELLO_RETRY_REQUEST:
-        return HELLO_RETRY_REQUEST_MAX_LENGTH;
-
     case TLS_ST_CR_CERT:
         return s->max_cert_list;
 
@@ -978,9 +967,6 @@ MSG_PROCESS_RETURN ossl_statem_client_process_message(SSL *s, PACKET *pkt)
     case DTLS_ST_CR_HELLO_VERIFY_REQUEST:
         return dtls_process_hello_verify(s, pkt);
 
-    case TLS_ST_CR_HELLO_RETRY_REQUEST:
-        return tls_process_hello_retry_request(s, pkt);
-
     case TLS_ST_CR_CERT:
         return tls_process_server_certificate(s, pkt);
 
@@ -1353,6 +1339,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     PACKET session_id, extpkt;
     size_t session_id_len;
     const unsigned char *cipherchars;
+    int hrr = 0;
     unsigned int compression;
     unsigned int sversion;
     unsigned int context;
@@ -1369,10 +1356,22 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     }
 
     /* load the server random */
-    if (!PACKET_copy_bytes(pkt, s->s3->server_random, SSL3_RANDOM_SIZE)) {
-        SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
-                 SSL_R_LENGTH_MISMATCH);
-        goto err;
+    if (s->version == TLS1_3_VERSION
+            && sversion == TLS1_2_VERSION
+            && PACKET_remaining(pkt) >= SSL3_RANDOM_SIZE
+            && memcmp(hrrrandom, PACKET_data(pkt), SSL3_RANDOM_SIZE) == 0) {
+        s->hello_retry_request = hrr = 1;
+        if (!PACKET_forward(pkt, SSL3_RANDOM_SIZE)) {
+            SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
+                     SSL_R_LENGTH_MISMATCH);
+            goto err;
+        }
+    } else {
+        if (!PACKET_copy_bytes(pkt, s->s3->server_random, SSL3_RANDOM_SIZE)) {
+            SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO,
+                     SSL_R_LENGTH_MISMATCH);
+            goto err;
+        }
     }
 
     /* Get the session-id. */
@@ -1402,7 +1401,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     }
 
     /* TLS extensions */
-    if (PACKET_remaining(pkt) == 0) {
+    if (PACKET_remaining(pkt) == 0 && !hrr) {
         PACKET_null_init(&extpkt);
     } else if (!PACKET_as_length_prefixed_2(pkt, &extpkt)
                || PACKET_remaining(pkt) != 0) {
@@ -1411,17 +1410,45 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
         goto err;
     }
 
-    if (!tls_collect_extensions(s, &extpkt,
-                                SSL_EXT_TLS1_2_SERVER_HELLO
-                                | SSL_EXT_TLS1_3_SERVER_HELLO,
-                                &extensions, NULL, 1)) {
-        /* SSLfatal() already called */
-        goto err;
+    if (!hrr) {
+        if (!tls_collect_extensions(s, &extpkt,
+                                    SSL_EXT_TLS1_2_SERVER_HELLO
+                                    | SSL_EXT_TLS1_3_SERVER_HELLO,
+                                    &extensions, NULL, 1)) {
+            /* SSLfatal() already called */
+            goto err;
+        }
+
+        if (!ssl_choose_client_version(s, sversion, extensions)) {
+            /* SSLfatal() already called */
+            goto err;
+        }
     }
 
-    if (!ssl_choose_client_version(s, sversion, extensions)) {
-        /* SSLfatal() already called */
-        goto err;
+    if (SSL_IS_TLS13(s) || hrr) {
+        if (compression != 0) {
+            SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
+                     SSL_F_TLS_PROCESS_SERVER_HELLO,
+                     SSL_R_INVALID_COMPRESSION_ALGORITHM);
+            goto err;
+        }
+
+        if (session_id_len != s->tmp_session_id_len
+                || memcmp(PACKET_data(&session_id), s->tmp_session_id,
+                          session_id_len) != 0) {
+            SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
+                     SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_INVALID_SESSION_ID);
+            goto err;
+        }
+    }
+
+    if (hrr) {
+        if (!set_client_ciphersuite(s, cipherchars)) {
+            /* SSLfatal() already called */
+            goto err;
+        }
+
+        return tls_process_as_hello_retry_request(s, &extpkt);
     }
 
     /*
@@ -1450,21 +1477,6 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
             goto err;
         }
 
-        if (compression != 0) {
-            SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
-                     SSL_F_TLS_PROCESS_SERVER_HELLO,
-                     SSL_R_INVALID_COMPRESSION_ALGORITHM);
-            goto err;
-        }
-
-        if (session_id_len != s->tmp_session_id_len
-                || memcmp(PACKET_data(&session_id), s->tmp_session_id,
-                          session_id_len) != 0) {
-            SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
-                     SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_INVALID_SESSION_ID);
-            goto err;
-        }
-
         /* This will set s->hit if we are resuming */
         if (!tls_parse_extension(s, TLSEXT_IDX_psk,
                                  SSL_EXT_TLS1_3_SERVER_HELLO,
@@ -1670,28 +1682,10 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     return MSG_PROCESS_ERROR;
 }
 
-static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt)
+static MSG_PROCESS_RETURN tls_process_as_hello_retry_request(SSL *s,
+                                                             PACKET *extpkt)
 {
-    unsigned int sversion;
-    const unsigned char *cipherchars;
     RAW_EXTENSION *extensions = NULL;
-    PACKET extpkt;
-
-    if (!PACKET_get_net_2(pkt, &sversion)) {
-        SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST,
-                 SSL_R_LENGTH_MISMATCH);
-        goto err;
-    }
-
-    /* TODO(TLS1.3): Remove the TLS1_3_VERSION_DRAFT clause before release */
-    if (sversion != TLS1_3_VERSION && sversion != TLS1_3_VERSION_DRAFT) {
-        SSLfatal(s, SSL_AD_PROTOCOL_VERSION,
-                 SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST,
-                 SSL_R_WRONG_SSL_VERSION);
-        goto err;
-    }
-
-    s->hello_retry_request = 1;
 
     /*
      * If we were sending early_data then the enc_write_ctx is now invalid and
@@ -1700,28 +1694,7 @@ static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt)
     EVP_CIPHER_CTX_free(s->enc_write_ctx);
     s->enc_write_ctx = NULL;
 
-    if (!PACKET_get_bytes(pkt, &cipherchars, TLS_CIPHER_LEN)) {
-        SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST,
-                 SSL_R_LENGTH_MISMATCH);
-        goto err;
-    }
-
-    if (!set_client_ciphersuite(s, cipherchars)) {
-        /* SSLfatal() already called */
-        goto err;
-    }
-
-    if (!PACKET_as_length_prefixed_2(pkt, &extpkt)
-               /* Must have a non-empty extensions block */
-            || PACKET_remaining(&extpkt) == 0
-               /* Must be no trailing data after extensions */
-            || PACKET_remaining(pkt) != 0) {
-        SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST,
-                 SSL_R_BAD_LENGTH);
-        goto err;
-    }
-
-    if (!tls_collect_extensions(s, &extpkt, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST,
+    if (!tls_collect_extensions(s, extpkt, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST,
                                 &extensions, NULL, 1)
             || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST,
                                          extensions, NULL, 0, 1)) {
@@ -1742,8 +1715,8 @@ static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt)
          * ClientHello will not change
          */
         SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
-                 SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST,
-                  SSL_R_NO_CHANGE_FOLLOWING_HRR);
+                 SSL_F_TLS_PROCESS_AS_HELLO_RETRY_REQUEST,
+                 SSL_R_NO_CHANGE_FOLLOWING_HRR);
         goto err;
     }
 
index 7b18115..c38c133 100644 (file)
 #include <openssl/evp.h>
 #include <openssl/x509.h>
 
+/* Fixed value used in the ServerHello random field to identify an HRR */
+const unsigned char hrrrandom[] = {
+    0xcf, 0x21, 0xad, 0x74, 0xe5, 0x9a, 0x61, 0x11, 0xbe, 0x1d, 0x8c, 0x02,
+    0x1e, 0x65, 0xb8, 0x91, 0xc2, 0xa2, 0x11, 0x16, 0x7a, 0xbb, 0x8c, 0x5e,
+    0x07, 0x9e, 0x09, 0xe2, 0xc8, 0xa8, 0x33, 0x9c
+};
+
 /*
  * send s->init_buf in records of type 'type' (SSL3_RT_HANDSHAKE or
  * SSL3_RT_CHANGE_CIPHER_SPEC)
@@ -1238,12 +1245,18 @@ int tls_get_message_body(SSL *s, size_t *len)
          * We defer feeding in the HRR until later. We'll do it as part of
          * processing the message
          */
-        if (s->s3->tmp.message_type != SSL3_MT_HELLO_RETRY_REQUEST
-                && !ssl3_finish_mac(s, (unsigned char *)s->init_buf->data,
-                                    s->init_num + SSL3_HM_HEADER_LENGTH)) {
-            /* SSLfatal() already called */
-            *len = 0;
-            return 0;
+#define SERVER_HELLO_RANDOM_OFFSET  (SSL3_HM_HEADER_LENGTH + 2)
+        if (s->s3->tmp.message_type != SSL3_MT_SERVER_HELLO
+                || s->init_num < SERVER_HELLO_RANDOM_OFFSET + SSL3_RANDOM_SIZE
+                || memcmp(hrrrandom,
+                          s->init_buf->data + SERVER_HELLO_RANDOM_OFFSET,
+                          SSL3_RANDOM_SIZE) != 0) {
+            if (!ssl3_finish_mac(s, (unsigned char *)s->init_buf->data,
+                                 s->init_num + SSL3_HM_HEADER_LENGTH)) {
+                /* SSLfatal() already called */
+                *len = 0;
+                return 0;
+            }
         }
         if (s->msg_callback)
             s->msg_callback(0, s->version, SSL3_RT_HANDSHAKE, s->init_buf->data,
index ad5b028..5e0ce7e 100644 (file)
@@ -35,6 +35,8 @@
 /* Dummy message type */
 #define SSL3_MT_DUMMY   -1
 
+extern const unsigned char hrrrandom[];
+
 /* Message processing return codes */
 typedef enum {
     /* Something bad happened */
index 43ad4a4..3ebe765 100644 (file)
@@ -24,7 +24,6 @@
 #include <openssl/md5.h>
 
 static int tls_construct_encrypted_extensions(SSL *s, WPACKET *pkt);
-static int tls_construct_hello_retry_request(SSL *s, WPACKET *pkt);
 
 /*
  * ossl_statem_server13_read_transition() encapsulates the logic for the allowed
@@ -392,18 +391,13 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)
         return WRITE_TRAN_FINISHED;
 
     case TLS_ST_SR_CLNT_HELLO:
-        if (s->hello_retry_request)
-            st->hand_state = TLS_ST_SW_HELLO_RETRY_REQUEST;
-        else
-            st->hand_state = TLS_ST_SW_SRVR_HELLO;
-        return WRITE_TRAN_CONTINUE;
-
-    case TLS_ST_SW_HELLO_RETRY_REQUEST:
-        st->hand_state = TLS_ST_EARLY_DATA;
+        st->hand_state = TLS_ST_SW_SRVR_HELLO;
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_SRVR_HELLO:
-        if ((s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0)
+        if (s->hello_retry_request)
+            st->hand_state = TLS_ST_EARLY_DATA;
+        else if ((s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0)
             st->hand_state = TLS_ST_SW_CHANGE;
         else
             st->hand_state = TLS_ST_SW_ENCRYPTED_EXTENSIONS;
@@ -714,11 +708,6 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
         /* No post work to be done */
         break;
 
-    case TLS_ST_SW_HELLO_RETRY_REQUEST:
-        if (statem_flush(s) != 1)
-            return WORK_MORE_A;
-        break;
-
     case TLS_ST_SW_HELLO_REQ:
         if (statem_flush(s) != 1)
             return WORK_MORE_A;
@@ -744,6 +733,11 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
         break;
 
     case TLS_ST_SW_SRVR_HELLO:
+        if (SSL_IS_TLS13(s) && s->hello_retry_request) {
+            if (statem_flush(s) != 1)
+                return WORK_MORE_A;
+            break;
+        }
 #ifndef OPENSSL_NO_SCTP
         if (SSL_IS_DTLS(s) && s->hit) {
             unsigned char sctpauthkey[64];
@@ -963,11 +957,6 @@ int ossl_statem_server_construct_message(SSL *s, WPACKET *pkt,
         *mt = SSL3_MT_ENCRYPTED_EXTENSIONS;
         break;
 
-    case TLS_ST_SW_HELLO_RETRY_REQUEST:
-        *confunc = tls_construct_hello_retry_request;
-        *mt = SSL3_MT_HELLO_RETRY_REQUEST;
-        break;
-
     case TLS_ST_SW_KEY_UPDATE:
         *confunc = tls_construct_key_update;
         *mt = SSL3_MT_KEY_UPDATE;
@@ -2211,14 +2200,18 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
     size_t sl, len;
     int version;
     unsigned char *session_id;
+    int usetls13 = SSL_IS_TLS13(s) || s->hello_retry_request;
 
-    version = SSL_IS_TLS13(s) ? TLS1_2_VERSION : s->version;
+    version = usetls13 ? TLS1_2_VERSION : s->version;
     if (!WPACKET_put_bytes_u16(pkt, version)
                /*
                 * Random stuff. Filling of the server_random takes place in
                 * tls_process_client_hello()
                 */
-            || !WPACKET_memcpy(pkt, s->s3->server_random, SSL3_RANDOM_SIZE)) {
+            || !WPACKET_memcpy(pkt,
+                               s->hello_retry_request
+                                   ? hrrrandom : s->s3->server_random,
+                               SSL3_RANDOM_SIZE)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_SERVER_HELLO,
                  ERR_R_INTERNAL_ERROR);
         return 0;
@@ -2247,7 +2240,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
          && !s->hit))
         s->session->session_id_length = 0;
 
-    if (SSL_IS_TLS13(s)) {
+    if (usetls13) {
         sl = s->tmp_session_id_len;
         session_id = s->tmp_session_id;
     } else {
@@ -2265,7 +2258,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
 #ifdef OPENSSL_NO_COMP
     compm = 0;
 #else
-    if (SSL_IS_TLS13(s) || s->s3->tmp.new_compression == NULL)
+    if (usetls13 || s->s3->tmp.new_compression == NULL)
         compm = 0;
     else
         compm = s->s3->tmp.new_compression->id;
@@ -2275,16 +2268,32 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
             || !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, pkt, &len)
             || !WPACKET_put_bytes_u8(pkt, compm)
             || !tls_construct_extensions(s, pkt,
-                                         SSL_IS_TLS13(s)
-                                            ? SSL_EXT_TLS1_3_SERVER_HELLO
-                                            : SSL_EXT_TLS1_2_SERVER_HELLO,
+                                         s->hello_retry_request
+                                            ? SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST
+                                            : (SSL_IS_TLS13(s)
+                                                ? SSL_EXT_TLS1_3_SERVER_HELLO
+                                                : SSL_EXT_TLS1_2_SERVER_HELLO),
                                          NULL, 0)) {
         /* SSLfatal() already called */
         return 0;
     }
 
-    if (!(s->verify_mode & SSL_VERIFY_PEER)
-            && !ssl3_digest_cached_records(s, 0)) {
+    if (s->hello_retry_request) {
+        /* Ditch the session. We'll create a new one next time around */
+        SSL_SESSION_free(s->session);
+        s->session = NULL;
+        s->hit = 0;
+
+        /*
+         * Re-initialise the Transcript Hash. We're going to prepopulate it with
+         * a synthetic message_hash in place of ClientHello1.
+         */
+        if (!create_synthetic_message_hash(s)) {
+            /* SSLfatal() already called */
+            return 0;
+        }
+    } else if (!(s->verify_mode & SSL_VERIFY_PEER)
+                && !ssl3_digest_cached_records(s, 0)) {
         /* SSLfatal() already called */;
         return 0;
     }
@@ -3856,45 +3865,6 @@ static int tls_construct_encrypted_extensions(SSL *s, WPACKET *pkt)
     return 1;
 }
 
-static int tls_construct_hello_retry_request(SSL *s, WPACKET *pkt)
-{
-    size_t len = 0;
-
-    /*
-     * TODO(TLS1.3): Remove the DRAFT version before release
-     * (should be s->version)
-     */
-    if (!WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT)
-            || !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, pkt,
-                                              &len)) {
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
-                 SSL_F_TLS_CONSTRUCT_HELLO_RETRY_REQUEST, ERR_R_INTERNAL_ERROR);
-       return 0;
-    }
-
-    if (!tls_construct_extensions(s, pkt, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST,
-                                  NULL, 0)) {
-        /* SSLfatal() already called */
-        return 0;
-    }
-
-    /* Ditch the session. We'll create a new one next time around */
-    SSL_SESSION_free(s->session);
-    s->session = NULL;
-    s->hit = 0;
-
-    /*
-     * Re-initialise the Transcript Hash. We're going to prepopulate it with
-     * a synthetic message_hash in place of ClientHello1.
-     */
-    if (!create_synthetic_message_hash(s)) {
-        /* SSLfatal() already called */
-        return 0;
-    }
-
-    return 1;
-}
-
 MSG_PROCESS_RETURN tls_process_end_of_early_data(SSL *s, PACKET *pkt)
 {
     if (PACKET_remaining(pkt) != 0) {
index 6a8314b..59d0efc 100644 (file)
@@ -87,7 +87,6 @@ static const ssl_trace_tbl ssl_handshake_tbl[] = {
     {DTLS1_MT_HELLO_VERIFY_REQUEST, "HelloVerifyRequest"},
     {SSL3_MT_NEWSESSION_TICKET, "NewSessionTicket"},
     {SSL3_MT_END_OF_EARLY_DATA, "EndOfEarlyData"},
-    {SSL3_MT_HELLO_RETRY_REQUEST, "HelloRetryRequest"},
     {SSL3_MT_ENCRYPTED_EXTENSIONS, "EncryptedExtensions"},
     {SSL3_MT_CERTIFICATE, "Certificate"},
     {SSL3_MT_SERVER_KEY_EXCHANGE, "ServerKeyExchange"},
@@ -783,11 +782,10 @@ static int ssl_print_extension(BIO *bio, int indent, int server,
         break;
 
     case TLSEXT_TYPE_key_share:
-        if (mt == SSL3_MT_HELLO_RETRY_REQUEST) {
+        if (server && extlen == 2) {
             int group_id;
 
-            if (extlen != 2)
-                return 0;
+            /* We assume this is an HRR, otherwise this is an invalid key_share */
             group_id = (ext[0] << 8) | ext[1];
             BIO_indent(bio, indent + 4, 80);
             BIO_printf(bio, "NamedGroup: %s (%d)\n",
@@ -1015,29 +1013,6 @@ static int ssl_print_server_hello(BIO *bio, int indent,
     return 1;
 }
 
-static int ssl_print_hello_retry_request(BIO *bio, int indent,
-                                         const unsigned char *msg,
-                                         size_t msglen)
-{
-    unsigned int cs;
-
-    if (!ssl_print_version(bio, indent, "server_version", &msg, &msglen, NULL))
-        return 0;
-
-    cs = (msg[0] << 8) | msg[1];
-    BIO_indent(bio, indent, 80);
-    BIO_printf(bio, "cipher_suite {0x%02X, 0x%02X} %s\n",
-               msg[0], msg[1], ssl_trace_str(cs, ssl_ciphers_tbl));
-    msg += 2;
-    msglen -= 2;
-
-    if (!ssl_print_extensions(bio, indent, 1, SSL3_MT_HELLO_RETRY_REQUEST, &msg,
-                              &msglen))
-        return 0;
-
-    return 1;
-}
-
 static int ssl_get_keyex(const char **pname, const SSL *ssl)
 {
     unsigned long alg_k = ssl->s3->tmp.new_cipher->algorithm_mkey;
@@ -1471,11 +1446,6 @@ static int ssl_print_handshake(BIO *bio, const SSL *ssl, int server,
             return 0;
         break;
 
-    case SSL3_MT_HELLO_RETRY_REQUEST:
-        if (!ssl_print_hello_retry_request(bio, indent + 2, msg, msglen))
-            return 0;
-        break;
-
     case SSL3_MT_ENCRYPTED_EXTENSIONS:
         if (!ssl_print_extensions(bio, indent + 2, 1,
                                   SSL3_MT_ENCRYPTED_EXTENSIONS, &msg, &msglen))
index ae0a2b0..e2cdf09 100644 (file)
@@ -223,6 +223,7 @@ ok(TLSProxy::Message->success(), "Ignore key_share for TLS<=1.2 server");
 #Test 22: The server sending an HRR but not requesting a new key_share should
 #         fail
 $proxy->clear();
+$direction = SERVER_TO_CLIENT;
 $testtype = NO_KEY_SHARES_IN_HRR;
 $proxy->serverflags("-curves X25519");
 $proxy->start();
@@ -341,6 +342,12 @@ sub modify_key_shares_filter
             if ($testtype == LOOK_ONLY) {
                 return;
             }
+            if ($testtype == NO_KEY_SHARES_IN_HRR) {
+                $message->delete_extension(TLSProxy::Message::EXT_KEY_SHARE);
+                $message->set_extension(TLSProxy::Message::EXT_UNKNOWN, "");
+                $message->repack();
+                return;
+            }
             if ($testtype == SELECT_X25519) {
                 $ext = pack "C4H64",
                     0x00, 0x1d, #x25519
@@ -370,12 +377,7 @@ sub modify_key_shares_filter
             $message->set_extension(TLSProxy::Message::EXT_KEY_SHARE, $ext);
 
             $message->repack();
-        } elsif ($message->mt == TLSProxy::Message::MT_HELLO_RETRY_REQUEST
-                 && $testtype == NO_KEY_SHARES_IN_HRR) {
-            $message->delete_extension(TLSProxy::Message::EXT_KEY_SHARE);
-            $message->set_extension(TLSProxy::Message::EXT_UNKNOWN, "");
-            $message->repack();
-         }
+        }
     }
 }
 
index ef46792..94dd11e 100644 (file)
@@ -485,7 +485,8 @@ sub change_outer_record_type
     for ($i = 0; ${$proxy->record_list}[$i]->flight() < 1; $i++) {
         next;
     }
-    $i++;
+    #Skip CCS and ServerHello
+    $i += 2;
     ${$proxy->record_list}[$i]->outer_content_type(TLSProxy::Record::RT_HANDSHAKE);
 }
 
index 3d3a10f..289e589 100644 (file)
@@ -74,7 +74,7 @@ sub cookie_filter
         0x04, 0x05;
 
     foreach my $message (@{$proxy->message_list}) {
-        if ($message->mt == TLSProxy::Message::MT_HELLO_RETRY_REQUEST
+        if ($message->mt == TLSProxy::Message::MT_SERVER_HELLO
                 && ${$message->records}[0]->flight == 1) {
             $message->delete_extension(TLSProxy::Message::EXT_KEY_SHARE)
                 if ($testtype == COOKIE_ONLY);
index dcc51ae..908ca4a 100644 (file)
@@ -35,7 +35,7 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf");
 @handmessages = (
     [TLSProxy::Message::MT_CLIENT_HELLO,
         checkhandshake::ALL_HANDSHAKES],
-    [TLSProxy::Message::MT_HELLO_RETRY_REQUEST,
+    [TLSProxy::Message::MT_SERVER_HELLO,
         checkhandshake::HRR_HANDSHAKE | checkhandshake::HRR_RESUME_HANDSHAKE],
     [TLSProxy::Message::MT_CLIENT_HELLO,
         checkhandshake::HRR_HANDSHAKE | checkhandshake::HRR_RESUME_HANDSHAKE],
@@ -90,7 +90,7 @@ $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_HELLO_RETRY_REQUEST, TLSProxy::Message::EXT_KEY_SHARE,
+    [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_KEY_SHARE,
         checkhandshake::KEY_SHARE_HRR_EXTENSION],
 
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SERVER_NAME,
index 9319e84..4b0552c 100644 (file)
@@ -35,7 +35,7 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf");
 @handmessages = (
     [TLSProxy::Message::MT_CLIENT_HELLO,
         checkhandshake::ALL_HANDSHAKES],
-    [TLSProxy::Message::MT_HELLO_RETRY_REQUEST,
+    [TLSProxy::Message::MT_SERVER_HELLO,
         checkhandshake::HRR_HANDSHAKE | checkhandshake::HRR_RESUME_HANDSHAKE],
     [TLSProxy::Message::MT_CLIENT_HELLO,
         checkhandshake::HRR_HANDSHAKE | checkhandshake::HRR_RESUME_HANDSHAKE],
@@ -90,7 +90,7 @@ $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_HELLO_RETRY_REQUEST, TLSProxy::Message::EXT_KEY_SHARE,
+    [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_KEY_SHARE,
         checkhandshake::KEY_SHARE_HRR_EXTENSION],
 
     [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_SERVER_NAME,
@@ -324,6 +324,6 @@ $proxy->start();
 checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
                checkhandshake::DEFAULT_EXTENSIONS
                | checkhandshake::SUPPORTED_GROUPS_SRV_EXTENSION,
-               "Default handshake test");
+               "Acceptable but non preferred key_share");
 
 unlink $session;
diff --git a/util/perl/TLSProxy/HelloRetryRequest.pm b/util/perl/TLSProxy/HelloRetryRequest.pm
deleted file mode 100644 (file)
index c4125b7..0000000
+++ /dev/null
@@ -1,150 +0,0 @@
-# Copyright 2016 The OpenSSL Project Authors. All Rights Reserved.
-#
-# Licensed under the OpenSSL license (the "License").  You may not use
-# this file except in compliance with the License.  You can obtain a copy
-# in the file LICENSE in the source distribution or at
-# https://www.openssl.org/source/license.html
-
-use strict;
-
-package TLSProxy::HelloRetryRequest;
-
-use vars '@ISA';
-push @ISA, 'TLSProxy::Message';
-
-sub new
-{
-    my $class = shift;
-    my ($server,
-        $data,
-        $records,
-        $startoffset,
-        $message_frag_lens) = @_;
-
-    my $self = $class->SUPER::new(
-        $server,
-        TLSProxy::Message::MT_HELLO_RETRY_REQUEST,
-        $data,
-        $records,
-        $startoffset,
-        $message_frag_lens);
-
-    $self->{extension_data} = "";
-
-    return $self;
-}
-
-sub parse
-{
-    my $self = shift;
-    my $ptr = 2;
-
-    TLSProxy::Proxy->is_tls13(1);
-
-    my ($server_version) = unpack('n', $self->data);
-    # TODO(TLS1.3): Replace this reference to draft version before release
-    if ($server_version == TLSProxy::Record::VERS_TLS_1_3_DRAFT) {
-        $server_version = TLSProxy::Record::VERS_TLS_1_3;
-    }
-
-    my $ciphersuite = unpack('n', substr($self->data, $ptr));
-    $ptr += 2;
-
-    my $extensions_len = unpack('n', substr($self->data, $ptr));
-    if (!defined $extensions_len) {
-        $extensions_len = 0;
-    }
-
-    $ptr += 2;
-    my $extension_data;
-    if ($extensions_len != 0) {
-        $extension_data = substr($self->data, $ptr);
-
-        if (length($extension_data) != $extensions_len) {
-            die "Invalid extension length\n";
-        }
-    } else {
-        if (length($self->data) != 2) {
-            die "Invalid extension length\n";
-        }
-        $extension_data = "";
-    }
-    my %extensions = ();
-    while (length($extension_data) >= 4) {
-        my ($type, $size) = unpack("nn", $extension_data);
-        my $extdata = substr($extension_data, 4, $size);
-        $extension_data = substr($extension_data, 4 + $size);
-        $extensions{$type} = $extdata;
-    }
-
-    $self->server_version($server_version);
-    $self->ciphersuite($ciphersuite);
-    $self->extension_data(\%extensions);
-
-    print "    Server Version:".$server_version."\n";
-    print "    Ciphersuite:".$ciphersuite."\n";
-    print "    Extensions Len:".$extensions_len."\n";
-}
-
-#Reconstruct the on-the-wire message data following changes
-sub set_message_contents
-{
-    my $self = shift;
-    my $data;
-    my $extensions = "";
-
-    foreach my $key (keys %{$self->extension_data}) {
-        my $extdata = ${$self->extension_data}{$key};
-        $extensions .= pack("n", $key);
-        $extensions .= pack("n", length($extdata));
-        $extensions .= $extdata;
-        if ($key == TLSProxy::Message::EXT_DUPLICATE_EXTENSION) {
-            $extensions .= pack("n", $key);
-            $extensions .= pack("n", length($extdata));
-            $extensions .= $extdata;
-        }
-    }
-
-    $data = pack('n', $self->server_version);
-    $data .= pack('n', $self->ciphersuite);
-    $data .= pack('n', length($extensions));
-    $data .= $extensions;
-    $self->data($data);
-}
-
-#Read/write accessors
-sub server_version
-{
-    my $self = shift;
-    if (@_) {
-      $self->{server_version} = shift;
-    }
-    return $self->{server_version};
-}
-sub ciphersuite
-{
-    my $self = shift;
-    if (@_) {
-      $self->{ciphersuite} = shift;
-    }
-    return $self->{ciphersuite};
-}
-sub extension_data
-{
-    my $self = shift;
-    if (@_) {
-        $self->{extension_data} = shift;
-    }
-    return $self->{extension_data};
-}
-sub set_extension
-{
-    my ($self, $ext_type, $ext_data) = @_;
-    $self->{extension_data}{$ext_type} = $ext_data;
-}
-sub delete_extension
-{
-    my ($self, $ext_type) = @_;
-    delete $self->{extension_data}{$ext_type};
-}
-1;
index 1c2bd20..5bb4050 100644 (file)
@@ -17,7 +17,6 @@ use constant {
     MT_CLIENT_HELLO => 1,
     MT_SERVER_HELLO => 2,
     MT_NEW_SESSION_TICKET => 4,
-    MT_HELLO_RETRY_REQUEST => 6,
     MT_ENCRYPTED_EXTENSIONS => 8,
     MT_CERTIFICATE => 11,
     MT_SERVER_KEY_EXCHANGE => 12,
@@ -48,7 +47,6 @@ my %message_type = (
     MT_CLIENT_HELLO, "ClientHello",
     MT_SERVER_HELLO, "ServerHello",
     MT_NEW_SESSION_TICKET, "NewSessionTicket",
-    MT_HELLO_RETRY_REQUEST, "HelloRetryRequest",
     MT_ENCRYPTED_EXTENSIONS, "EncryptedExtensions",
     MT_CERTIFICATE, "Certificate",
     MT_SERVER_KEY_EXCHANGE, "ServerKeyExchange",
@@ -296,15 +294,6 @@ sub create_message
             [@message_frag_lens]
         );
         $message->parse();
-    } elsif ($mt == MT_HELLO_RETRY_REQUEST) {
-        $message = TLSProxy::HelloRetryRequest->new(
-            $server,
-            $data,
-            [@message_rec_list],
-            $startoffset,
-            [@message_frag_lens]
-        );
-        $message->parse();
     } elsif ($mt == MT_SERVER_HELLO) {
         $message = TLSProxy::ServerHello->new(
             $server,
index 83a6494..99b0ded 100644 (file)
@@ -16,7 +16,6 @@ use IO::Select;
 use TLSProxy::Record;
 use TLSProxy::Message;
 use TLSProxy::ClientHello;
-use TLSProxy::HelloRetryRequest;
 use TLSProxy::ServerHello;
 use TLSProxy::EncryptedExtensions;
 use TLSProxy::Certificate;
index 65c5135..e1667d5 100644 (file)
@@ -69,10 +69,33 @@ sub checkhandshake($$$$)
         my $extcount;
         my $clienthelloseen = 0;
 
+        my $lastmt = 0;
+        my $numsh = 0;
+        if (TLSProxy::Proxy::is_tls13()) {
+            #How many ServerHellos are we expecting?
+            for ($numtests = 0; $handmessages[$loop][1] != 0; $loop++) {
+                next if (($handmessages[$loop][1] & $handtype) == 0);
+                $numsh++ if ($lastmt != TLSProxy::Message::MT_SERVER_HELLO
+                             && $handmessages[$loop][0] == TLSProxy::Message::MT_SERVER_HELLO);
+                $lastmt = $handmessages[$loop][0];
+            }
+        }
+
         #First count the number of tests
         my $nextmess = 0;
         my $message = undef;
         my $chnum = 0;
+        my $shnum = 0;
+        if (!TLSProxy::Proxy::is_tls13()) {
+            # In non-TLSv1.3 we always treat reneg CH and SH like the first CH
+            # and SH
+            $chnum = 1;
+            $shnum = 1;
+        }
+        #If we're only expecting one ServerHello out of two then we skip the
+        #first ServerHello in the list completely
+        $shnum++ if ($numsh == 1 && TLSProxy::Proxy::is_tls13());
+        $loop = 0;
         for ($numtests = 0; $handmessages[$loop][1] != 0; $loop++) {
             next if (($handmessages[$loop][1] & $handtype) == 0);
             if (scalar @{$proxy->message_list} > $nextmess) {
@@ -84,10 +107,11 @@ sub checkhandshake($$$$)
             $numtests++;
 
             next if (!defined $message);
-            $chnum = 1 if $message->mt() != TLSProxy::Message::MT_CLIENT_HELLO
-                          && TLSProxy::Proxy::is_tls13();
+            if (TLSProxy::Proxy::is_tls13()) {
+                $chnum++ if $message->mt() == TLSProxy::Message::MT_CLIENT_HELLO;
+                $shnum++ if $message->mt() == TLSProxy::Message::MT_SERVER_HELLO;
+            }
             next if ($message->mt() != TLSProxy::Message::MT_CLIENT_HELLO
-                    && $message->mt() != TLSProxy::Message::MT_HELLO_RETRY_REQUEST
                     && $message->mt() != TLSProxy::Message::MT_SERVER_HELLO
                     && $message->mt() !=
                        TLSProxy::Message::MT_ENCRYPTED_EXTENSIONS
@@ -96,14 +120,19 @@ sub checkhandshake($$$$)
             next if $message->mt() == TLSProxy::Message::MT_CERTIFICATE
                     && !TLSProxy::Proxy::is_tls13();
 
-            my $extchnum = 0;
+            my $extchnum = 1;
+            my $extshnum = 1;
             for (my $extloop = 0;
                     $extensions[$extloop][2] != 0;
                     $extloop++) {
-                $extchnum = 1 if $extensions[$extloop][0] != TLSProxy::Message::MT_CLIENT_HELLO
+                $extchnum = 2 if $extensions[$extloop][0] != TLSProxy::Message::MT_CLIENT_HELLO
                                  && TLSProxy::Proxy::is_tls13();
+                $extshnum = 2 if $extensions[$extloop][0] != TLSProxy::Message::MT_SERVER_HELLO
+                                 && $extchnum == 2;
                 next if $extensions[$extloop][0] == TLSProxy::Message::MT_CLIENT_HELLO
                                  && $extchnum != $chnum;
+                next if $extensions[$extloop][0] == TLSProxy::Message::MT_SERVER_HELLO
+                                 && $extshnum != $shnum;
                 next if ($message->mt() != $extensions[$extloop][0]);
                 $numtests++;
             }
@@ -114,7 +143,18 @@ sub checkhandshake($$$$)
 
         $nextmess = 0;
         $message = undef;
-        $chnum = 0;
+        if (TLSProxy::Proxy::is_tls13()) {
+            $chnum = 0;
+            $shnum = 0;
+        } else {
+            # In non-TLSv1.3 we always treat reneg CH and SH like the first CH
+            # and SH
+            $chnum = 1;
+            $shnum = 1;
+        }
+        #If we're only expecting one ServerHello out of two then we skip the
+        #first ServerHello in the list completely
+        $shnum++ if ($numsh == 1 && TLSProxy::Proxy::is_tls13());
         for ($loop = 0; $handmessages[$loop][1] != 0; $loop++) {
             next if (($handmessages[$loop][1] & $handtype) == 0);
             if (scalar @{$proxy->message_list} > $nextmess) {
@@ -132,11 +172,12 @@ sub checkhandshake($$$$)
                    "Message type check. Got ".$message->mt
                    .", expected ".$handmessages[$loop][0]);
             }
-            $chnum = 1 if $message->mt() != TLSProxy::Message::MT_CLIENT_HELLO
-                          && TLSProxy::Proxy::is_tls13();
+            if (TLSProxy::Proxy::is_tls13()) {
+                $chnum++ if $message->mt() == TLSProxy::Message::MT_CLIENT_HELLO;
+                $shnum++ if $message->mt() == TLSProxy::Message::MT_SERVER_HELLO;
+            }
 
             next if ($message->mt() != TLSProxy::Message::MT_CLIENT_HELLO
-                    && $message->mt() != TLSProxy::Message::MT_HELLO_RETRY_REQUEST
                     && $message->mt() != TLSProxy::Message::MT_SERVER_HELLO
                     && $message->mt() !=
                        TLSProxy::Message::MT_ENCRYPTED_EXTENSIONS
@@ -153,16 +194,21 @@ sub checkhandshake($$$$)
             }
             #Now check that we saw the extensions we expected
             my $msgexts = $message->extension_data();
-            my $extchnum = 0;
+            my $extchnum = 1;
+            my $extshnum = 1;
             for (my $extloop = 0, $extcount = 0; $extensions[$extloop][2] != 0;
                                 $extloop++) {
                 #In TLSv1.3 we can have two ClientHellos if there has been a
                 #HelloRetryRequest, and they may have different extensions. Skip
                 #if these are extensions for a different ClientHello
-                $extchnum = 1 if $extensions[$extloop][0] != TLSProxy::Message::MT_CLIENT_HELLO
+                $extchnum = 2 if $extensions[$extloop][0] != TLSProxy::Message::MT_CLIENT_HELLO
                                  && TLSProxy::Proxy::is_tls13();
+                $extshnum = 2 if $extensions[$extloop][0] != TLSProxy::Message::MT_SERVER_HELLO
+                                 && $extchnum == 2;
                 next if $extensions[$extloop][0] == TLSProxy::Message::MT_CLIENT_HELLO
                                  && $extchnum != $chnum;
+                next if $extensions[$extloop][0] == TLSProxy::Message::MT_SERVER_HELLO
+                                 && $extshnum != $shnum;
                 next if ($message->mt() != $extensions[$extloop][0]);
                 ok (($extensions[$extloop][2] & $exttype) == 0
                       || defined ($msgexts->{$extensions[$extloop][1]}),