Update state machine to send CCS based on whether we did an HRR

The CCS may be sent at different times based on whether or not we
sent an HRR earlier. In order to make that decision this commit
also updates things to make sure we remember whether an HRR was
used or not.

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

6 files changed, 47 insertions, 33 deletions

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 988e919044..026126d4d8 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1235,7 +1235,7 @@ static int final_key_share(SSL *s, unsigned int context, int sent)
      */
     if (s->server && s->s3->peer_tmp == NULL) {
         /* No suitable share */
-        if (s->hello_retry_request == 0 && sent
+        if (s->hello_retry_request == SSL_HRR_NONE && sent
                 && (!s->hit
                     || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE)
                        != 0)) {
@@ -1260,7 +1260,7 @@ static int final_key_share(SSL *s, unsigned int context, int sent)
             if (i < num_groups) {
                 /* A shared group exists so send a HelloRetryRequest */
                 s->s3->group_id = group_id;
-                s->hello_retry_request = 1;
+                s->hello_retry_request = SSL_HRR_PENDING;
                 return 1;
             }
         }
@@ -1275,8 +1275,8 @@ static int final_key_share(SSL *s, unsigned int context, int sent)
     }
 
     /* We have a key_share so don't send any more HelloRetryRequest messages */
-    if (s->server)
-        s->hello_retry_request = 0;
+    if (s->server && s->hello_retry_request == SSL_HRR_PENDING)
+        s->hello_retry_request = SSL_HRR_COMPLETE;
 
     /*
      * For a client side resumption with no key_share we need to generate
@@ -1405,7 +1405,7 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
      * following a HelloRetryRequest then this includes the hash of the first
      * ClientHello and the HelloRetryRequest itself.
      */
-    if (s->hello_retry_request) {
+    if (s->hello_retry_request == SSL_HRR_PENDING) {
         size_t hdatalen;
         void *hdata;
 
@@ -1516,7 +1516,7 @@ static int final_early_data(SSL *s, unsigned int context, int sent)
             || s->session->ext.tick_identity != 0
             || s->early_data_state != SSL_EARLY_DATA_ACCEPTING
             || !s->ext.early_data_ok
-            || s->hello_retry_request) {
+            || s->hello_retry_request != SSL_HRR_NONE) {
         s->ext.early_data = SSL_EARLY_DATA_REJECTED;
     } else {
         s->ext.early_data = SSL_EARLY_DATA_ACCEPTED;
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 2640756134..1fbf9f6e0e 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -599,7 +599,7 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
     size_t encodedlen;
 
     if (s->s3->tmp.pkey != NULL) {
-        if (!ossl_assert(s->hello_retry_request)) {
+        if (!ossl_assert(s->hello_retry_request == SSL_HRR_PENDING)) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_ADD_KEY_SHARE,
                      ERR_R_INTERNAL_ERROR);
             return 0;
@@ -749,7 +749,7 @@ EXT_RETURN tls_construct_ctos_early_data(SSL *s, WPACKET *pkt,
     SSL_SESSION *edsess = NULL;
     const EVP_MD *handmd = NULL;
 
-    if (s->hello_retry_request)
+    if (s->hello_retry_request == SSL_HRR_PENDING)
         handmd = ssl_handshake_md(s);
 
     if (s->psk_use_session_cb != NULL
@@ -961,7 +961,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
             || (s->session->ext.ticklen == 0 && s->psksession == NULL))
         return EXT_RETURN_NOT_SENT;
 
-    if (s->hello_retry_request)
+    if (s->hello_retry_request == SSL_HRR_PENDING)
         handmd = ssl_handshake_md(s);
 
     if (s->session->ext.ticklen != 0) {
@@ -980,7 +980,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
             goto dopsksess;
         }
 
-        if (s->hello_retry_request && mdres != handmd) {
+        if (s->hello_retry_request == SSL_HRR_PENDING && mdres != handmd) {
             /*
              * Selected ciphersuite hash does not match the hash for the session
              * so we can't use it.
@@ -1044,7 +1044,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
             return EXT_RETURN_FAIL;
         }
 
-        if (s->hello_retry_request && mdpsk != handmd) {
+        if (s->hello_retry_request == SSL_HRR_PENDING && mdpsk != handmd) {
             /*
              * Selected ciphersuite hash does not match the hash for the PSK
              * session. This is an application bug.
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 93ac98f116..d34a7c5ee5 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -704,7 +704,7 @@ int tls_parse_ctos_early_data(SSL *s, PACKET *pkt, unsigned int context,
         return 0;
     }
 
-    if (s->hello_retry_request) {
+    if (s->hello_retry_request != SSL_HRR_NONE) {
         SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
                  SSL_F_TLS_PARSE_CTOS_EARLY_DATA, SSL_R_BAD_EXTENSION);
         return 0;
@@ -1245,7 +1245,7 @@ EXT_RETURN tls_construct_stoc_key_share(SSL *s, WPACKET *pkt,
 
     if (ckey == NULL) {
         /* No key_share received from client */
-        if (s->hello_retry_request) {
+        if (s->hello_retry_request == SSL_HRR_PENDING) {
             if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share)
                     || !WPACKET_start_sub_packet_u16(pkt)
                     || !WPACKET_put_bytes_u16(pkt, s->s3->group_id)
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index af9e1dcd7d..80148fa531 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -387,7 +387,7 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s)
                 || s->early_data_state == SSL_EARLY_DATA_FINISHED_WRITING)
             st->hand_state = TLS_ST_PENDING_EARLY_DATA_END;
         else if ((s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0
-                    && !s->hello_retry_request)
+                 && s->hello_retry_request == SSL_HRR_NONE)
             st->hand_state = TLS_ST_CW_CHANGE;
         else
             st->hand_state = (s->s3->tmp.cert_req != 0) ? TLS_ST_CW_CERT
@@ -1055,7 +1055,8 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
     if (sess == NULL
             || !ssl_version_supported(s, sess->ssl_version)
             || !SSL_SESSION_is_resumable(sess)) {
-        if (!s->hello_retry_request && !ssl_get_new_session(s, 0)) {
+        if (s->hello_retry_request == SSL_HRR_NONE
+                && !ssl_get_new_session(s, 0)) {
             /* SSLfatal() already called */
             return 0;
         }
@@ -1078,7 +1079,7 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
             }
         }
     } else {
-        i = s->hello_retry_request == 0;
+        i = (s->hello_retry_request == SSL_HRR_NONE);
     }
 
     if (i && ssl_fill_hello_random(s, 0, p, sizeof(s->s3->client_random),
@@ -1136,7 +1137,7 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
             sess_id_len = sizeof(s->tmp_session_id);
             s->tmp_session_id_len = sess_id_len;
             session_id = s->tmp_session_id;
-            if (!s->hello_retry_request
+            if (s->hello_retry_request == SSL_HRR_NONE
                     && ssl_randbytes(s, s->tmp_session_id,
                                      sess_id_len) <= 0) {
                 SSLfatal(s, SSL_AD_INTERNAL_ERROR,
@@ -1360,7 +1361,8 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
             && 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;
+        s->hello_retry_request = SSL_HRR_PENDING;
+        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);
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index d64ddffffd..b65dfa1587 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1656,7 +1656,7 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd)
     suppversions = &hello->pre_proc_exts[TLSEXT_IDX_supported_versions];
 
     /* If we did an HRR then supported versions is mandatory */
-    if (!suppversions->present && s->hello_retry_request)
+    if (!suppversions->present && s->hello_retry_request != SSL_HRR_NONE)
         return SSL_R_UNSUPPORTED_PROTOCOL;
 
     if (suppversions->present && !SSL_IS_DTLS(s)) {
@@ -1703,7 +1703,7 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd)
         }
 
         if (best_vers > 0) {
-            if (s->hello_retry_request) {
+            if (s->hello_retry_request != SSL_HRR_NONE) {
                 /*
                  * This is after a HelloRetryRequest so we better check that we
                  * negotiated TLSv1.3
@@ -1779,7 +1779,8 @@ int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions)
         return 0;
     }
 
-    if (s->hello_retry_request && s->version != TLS1_3_VERSION) {
+    if (s->hello_retry_request != SSL_HRR_NONE
+            && s->version != TLS1_3_VERSION) {
         s->version = origv;
         SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION,
                  SSL_R_WRONG_SSL_VERSION);
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 3ebe76530a..7d1d15dcc1 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -48,7 +48,7 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt)
         break;
 
     case TLS_ST_EARLY_DATA:
-        if (s->hello_retry_request) {
+        if (s->hello_retry_request == SSL_HRR_PENDING) {
             if (mt == SSL3_MT_CLIENT_HELLO) {
                 st->hand_state = TLS_ST_SR_CLNT_HELLO;
                 return 1;
@@ -395,16 +395,20 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_SRVR_HELLO:
-        if (s->hello_retry_request)
-            st->hand_state = TLS_ST_EARLY_DATA;
-        else if ((s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0)
+        if ((s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0
+                && s->hello_retry_request != SSL_HRR_COMPLETE)
             st->hand_state = TLS_ST_SW_CHANGE;
+        else if (s->hello_retry_request == SSL_HRR_PENDING)
+            st->hand_state = TLS_ST_EARLY_DATA;
         else
             st->hand_state = TLS_ST_SW_ENCRYPTED_EXTENSIONS;
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_CHANGE:
-        st->hand_state = TLS_ST_SW_ENCRYPTED_EXTENSIONS;
+        if (s->hello_retry_request == SSL_HRR_PENDING)
+            st->hand_state = TLS_ST_EARLY_DATA;
+        else
+            st->hand_state = TLS_ST_SW_ENCRYPTED_EXTENSIONS;
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_ENCRYPTED_EXTENSIONS:
@@ -664,6 +668,8 @@ WORK_STATE ossl_statem_server_pre_work(SSL *s, WORK_STATE wst)
         break;
 
     case TLS_ST_SW_CHANGE:
+        if (SSL_IS_TLS13(s))
+            break;
         s->session->cipher = s->s3->tmp.new_cipher;
         if (!s->method->ssl3_enc->setup_key_block(s)) {
             /* SSLfatal() already called */
@@ -733,7 +739,7 @@ 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 (SSL_IS_TLS13(s) && s->hello_retry_request == SSL_HRR_PENDING) {
             if (statem_flush(s) != 1)
                 return WORK_MORE_A;
             break;
@@ -765,11 +771,14 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
         }
 #endif
         if (!SSL_IS_TLS13(s)
-                || (s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0)
+                || ((s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0
+                    && s->hello_retry_request != SSL_HRR_COMPLETE))
             break;
         /* Fall through */
 
     case TLS_ST_SW_CHANGE:
+        if (s->hello_retry_request == SSL_HRR_PENDING)
+            break;
         /*
          * TODO(TLS1.3): This actually causes a problem. We don't yet know
          * whether the next record we are going to receive is an unencrypted
@@ -1267,7 +1276,8 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
     if (clienthello->isv2) {
         unsigned int mt;
 
-        if (!SSL_IS_FIRST_HANDSHAKE(s) || s->hello_retry_request) {
+        if (!SSL_IS_FIRST_HANDSHAKE(s)
+                || s->hello_retry_request != SSL_HRR_NONE) {
             SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE,
                      SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_UNEXPECTED_MESSAGE);
             goto err;
@@ -1624,7 +1634,7 @@ static int tls_early_post_process_client_hello(SSL *s)
                      SSL_R_NO_SHARED_CIPHER);
             goto err;
         }
-        if (s->hello_retry_request
+        if (s->hello_retry_request == SSL_HRR_PENDING
                 && (s->s3->tmp.new_cipher == NULL
                     || s->s3->tmp.new_cipher->id != cipher->id)) {
             /*
@@ -2200,7 +2210,7 @@ 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;
+    int usetls13 = SSL_IS_TLS13(s) || s->hello_retry_request == SSL_HRR_PENDING;
 
     version = usetls13 ? TLS1_2_VERSION : s->version;
     if (!WPACKET_put_bytes_u16(pkt, version)
@@ -2209,7 +2219,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
                 * tls_process_client_hello()
                 */
             || !WPACKET_memcpy(pkt,
-                               s->hello_retry_request
+                               s->hello_retry_request == SSL_HRR_PENDING
                                    ? hrrrandom : s->s3->server_random,
                                SSL3_RANDOM_SIZE)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_SERVER_HELLO,
@@ -2269,6 +2279,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
             || !WPACKET_put_bytes_u8(pkt, compm)
             || !tls_construct_extensions(s, pkt,
                                          s->hello_retry_request
+                                            == SSL_HRR_PENDING
                                             ? SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST
                                             : (SSL_IS_TLS13(s)
                                                 ? SSL_EXT_TLS1_3_SERVER_HELLO
@@ -2278,7 +2289,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
         return 0;
     }
 
-    if (s->hello_retry_request) {
+    if (s->hello_retry_request == SSL_HRR_PENDING) {
         /* Ditch the session. We'll create a new one next time around */
         SSL_SESSION_free(s->session);
         s->session = NULL;