From 597c51bc980ba6d7470dd8de747ac12a6c7a442b Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 5 Dec 2017 10:14:35 +0000 Subject: [PATCH] Merge HRR into ServerHello Reviewed-by: Ben Kaduk (Merged from https://github.com/openssl/openssl/pull/4701) --- apps/s_cb.c | 1 - crypto/err/openssl.txt | 1 + include/openssl/ssl.h | 2 - include/openssl/ssl3.h | 1 - include/openssl/sslerr.h | 1 + ssl/ssl_err.c | 2 + ssl/ssl_stat.c | 8 -- ssl/statem/statem_clnt.c | 153 ++++++++++-------------- ssl/statem/statem_lib.c | 25 +++- ssl/statem/statem_locl.h | 2 + ssl/statem/statem_srvr.c | 106 ++++++---------- ssl/t1_trce.c | 34 +----- test/recipes/70-test_key_share.t | 14 ++- test/recipes/70-test_sslrecords.t | 3 +- test/recipes/70-test_tls13cookie.t | 2 +- test/recipes/70-test_tls13kexmodes.t | 4 +- test/recipes/70-test_tls13messages.t | 6 +- util/perl/TLSProxy/HelloRetryRequest.pm | 150 ----------------------- util/perl/TLSProxy/Message.pm | 11 -- util/perl/TLSProxy/Proxy.pm | 1 - util/perl/checkhandshake.pm | 68 +++++++++-- 21 files changed, 201 insertions(+), 394 deletions(-) delete mode 100644 util/perl/TLSProxy/HelloRetryRequest.pm diff --git a/apps/s_cb.c b/apps/s_cb.c index 79d29199ea..c7c9ecb170 100644 --- a/apps/s_cb.c +++ b/apps/s_cb.c @@ -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}, diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index de7c50a0a0..af826cd10f 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -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 diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 48779fa6d2..98a106bb77 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -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, diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h index e9d56a8385..b781f61dab 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -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 diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h index 9c86220336..9564023044 100644 --- a/include/openssl/sslerr.h +++ b/include/openssl/sslerr.h @@ -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 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 3322685c19..08f6696163 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -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), ""}, diff --git a/ssl/ssl_stat.c b/ssl/ssl_stat.c index 5dd71d2294..179513b1a3 100644 --- a/ssl/ssl_stat.c +++ b/ssl/ssl_stat.c @@ -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: diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 37c198e713..af9e1dcd7d 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -22,7 +22,7 @@ #include #include -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; } diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 7b1811543c..c38c1337b5 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -19,6 +19,13 @@ #include #include +/* 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, diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h index ad5b028da7..5e0ce7e997 100644 --- a/ssl/statem/statem_locl.h +++ b/ssl/statem/statem_locl.h @@ -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 */ diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 43ad4a4623..3ebe76530a 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -24,7 +24,6 @@ #include 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) { diff --git a/ssl/t1_trce.c b/ssl/t1_trce.c index 6a8314bf73..59d0efc036 100644 --- a/ssl/t1_trce.c +++ b/ssl/t1_trce.c @@ -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)) diff --git a/test/recipes/70-test_key_share.t b/test/recipes/70-test_key_share.t index ae0a2b0c29..e2cdf0972f 100644 --- a/test/recipes/70-test_key_share.t +++ b/test/recipes/70-test_key_share.t @@ -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(); - } + } } } diff --git a/test/recipes/70-test_sslrecords.t b/test/recipes/70-test_sslrecords.t index ef4679261d..94dd11eef2 100644 --- a/test/recipes/70-test_sslrecords.t +++ b/test/recipes/70-test_sslrecords.t @@ -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); } diff --git a/test/recipes/70-test_tls13cookie.t b/test/recipes/70-test_tls13cookie.t index 3d3a10fcbf..289e589897 100644 --- a/test/recipes/70-test_tls13cookie.t +++ b/test/recipes/70-test_tls13cookie.t @@ -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); diff --git a/test/recipes/70-test_tls13kexmodes.t b/test/recipes/70-test_tls13kexmodes.t index dcc51aecdd..908ca4a21d 100644 --- a/test/recipes/70-test_tls13kexmodes.t +++ b/test/recipes/70-test_tls13kexmodes.t @@ -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, diff --git a/test/recipes/70-test_tls13messages.t b/test/recipes/70-test_tls13messages.t index 9319e8492d..4b0552c5ca 100644 --- a/test/recipes/70-test_tls13messages.t +++ b/test/recipes/70-test_tls13messages.t @@ -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 index c4125b7a16..0000000000 --- a/util/perl/TLSProxy/HelloRetryRequest.pm +++ /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; diff --git a/util/perl/TLSProxy/Message.pm b/util/perl/TLSProxy/Message.pm index 1c2bd2044b..5bb4050786 100644 --- a/util/perl/TLSProxy/Message.pm +++ b/util/perl/TLSProxy/Message.pm @@ -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, diff --git a/util/perl/TLSProxy/Proxy.pm b/util/perl/TLSProxy/Proxy.pm index 83a6494933..99b0dedd5b 100644 --- a/util/perl/TLSProxy/Proxy.pm +++ b/util/perl/TLSProxy/Proxy.pm @@ -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; diff --git a/util/perl/checkhandshake.pm b/util/perl/checkhandshake.pm index 65c5135a1e..e1667d5705 100644 --- a/util/perl/checkhandshake.pm +++ b/util/perl/checkhandshake.pm @@ -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]}), -- 2.25.1