commit a2b97bdf3dbbd255ee24aa8a74cf88d4f7034898
author Matt Caswell <matt@openssl.org> Fri Oct 13 11:41:50 2017 +0100
committer Matt Caswell <matt@openssl.org> Mon Oct 16 15:52:18 2017 +0100
tree 0f9d0b6480413aceaa00c2789da69d137684fd33
parent aeb3e4abb6abab64342df44b2a7dc407f6508a11

Don't do version neg on an HRR

Previously if a client received an HRR then we would do version negotiation
immediately - because we know we are going to get TLSv1.3. However this
causes a problem when we emit the 2nd ClientHello because we start changing
a whole load of stuff to ommit things that aren't relevant for < TLSv1.3.
The spec requires that the 2nd ClientHello is the same except for changes
required from the HRR. Therefore the simplest thing to do is to defer the
version negotiation until we receive the ServerHello.

Fixes #4292

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4527)

ssl/statem/statem_clnt.c

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 88c0889..fdf5d45 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -391,10 +391,6 @@
         /* We only hit this in the case of HelloRetryRequest */
         return WRITE_TRAN_FINISHED;
 
-    case TLS_ST_CR_HELLO_RETRY_REQUEST:
-        st->hand_state = TLS_ST_CW_CLNT_HELLO;
-        return WRITE_TRAN_CONTINUE;
-
     case TLS_ST_CR_FINISHED:
         if (s->early_data_state == SSL_EARLY_DATA_WRITE_RETRY
                 || s->early_data_state == SSL_EARLY_DATA_FINISHED_WRITING)
@@ -500,6 +496,10 @@
          */
         return WRITE_TRAN_FINISHED;
 
+    case TLS_ST_CR_HELLO_RETRY_REQUEST:
+        st->hand_state = TLS_ST_CW_CLNT_HELLO;
+        return WRITE_TRAN_CONTINUE;
+
     case TLS_ST_EARLY_DATA:
         return WRITE_TRAN_FINISHED;
 
@@ -1558,7 +1558,6 @@
 static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt)
 {
     unsigned int sversion;
-    int errorcode;
     const unsigned char *cipherchars;
     RAW_EXTENSION *extensions = NULL;
     int al;
@@ -1579,13 +1578,6 @@
     EVP_CIPHER_CTX_free(s->enc_write_ctx);
     s->enc_write_ctx = NULL;
 
-    /* This will fail if it doesn't choose TLSv1.3+ */
-    errorcode = ssl_choose_client_version(s, sversion, 0, &al);
-    if (errorcode != 0) {
-        SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, errorcode);
-        goto f_err;
-    }
-
     if (!PACKET_get_bytes(pkt, &cipherchars, TLS_CIPHER_LEN)) {
         SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, SSL_R_LENGTH_MISMATCH);
         al = SSL_AD_DECODE_ERROR;