Add client side support for parsing Hello Retry Request

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2341)

5 files changed, 199 insertions, 52 deletions

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 705252c763..ea5a7634bd 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -20,6 +20,7 @@
 
 static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_ADD_CLIENT_KEY_SHARE_EXT), "add_client_key_share_ext"},
+    {ERR_FUNC(SSL_F_ADD_KEY_SHARE), "add_key_share"},
     {ERR_FUNC(SSL_F_CHECK_SUITEB_CIPHER_LIST), "check_suiteb_cipher_list"},
     {ERR_FUNC(SSL_F_CT_MOVE_SCTS), "ct_move_scts"},
     {ERR_FUNC(SSL_F_CT_STRICT), "ct_strict"},
@@ -412,6 +413,8 @@ static ERR_STRING_DATA SSL_str_functs[] = {
      "tls_process_encrypted_extensions"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_FINISHED), "tls_process_finished"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_HELLO_REQ), "tls_process_hello_req"},
+    {ERR_FUNC(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST),
+     "tls_process_hello_retry_request"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_INITIAL_SERVER_FLIGHT),
      "tls_process_initial_server_flight"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_KEY_EXCHANGE), "tls_process_key_exchange"},
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 003874507d..2516ab9562 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -528,12 +528,57 @@ int tls_construct_ctos_psk_kex_modes(SSL *s, WPACKET *pkt, unsigned int context,
     return 1;
 }
 
+#ifndef OPENSSL_NO_TLS1_3
+static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
+{
+    unsigned char *encodedPoint = NULL;
+    EVP_PKEY *key_share_key = NULL;
+    size_t encodedlen;
+
+    key_share_key = ssl_generate_pkey_curve(curve_id);
+    if (key_share_key == NULL) {
+        SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EVP_LIB);
+        return 0;
+    }
+
+    /* Encode the public key. */
+    encodedlen = EVP_PKEY_get1_tls_encodedpoint(key_share_key,
+                                                &encodedPoint);
+    if (encodedlen == 0) {
+        SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EC_LIB);
+        EVP_PKEY_free(key_share_key);
+        return 0;
+    }
+
+    /* Create KeyShareEntry */
+    if (!WPACKET_put_bytes_u16(pkt, curve_id)
+            || !WPACKET_sub_memcpy_u16(pkt, encodedPoint, encodedlen)) {
+        SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_INTERNAL_ERROR);
+        EVP_PKEY_free(key_share_key);
+        OPENSSL_free(encodedPoint);
+        return 0;
+    }
+
+    /*
+     * TODO(TLS1.3): When changing to send more than one key_share we're
+     * going to need to be able to save more than one EVP_PKEY. For now
+     * we reuse the existing tmp.pkey
+     */
+    s->s3->tmp.pkey = key_share_key;
+    s->s3->group_id = curve_id;
+    OPENSSL_free(encodedPoint);
+
+    return 1;
+}
+#endif
+
 int tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, unsigned int context,
                                  X509 *x, size_t chainidx, int *al)
 {
 #ifndef OPENSSL_NO_TLS1_3
-    size_t i, sharessent = 0, num_curves = 0;
+    size_t i, num_curves = 0;
     const unsigned char *pcurves = NULL;
+    unsigned int curve_id = 0;
 
     /* key_share extension */
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share)
@@ -551,62 +596,37 @@ int tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, unsigned int context,
         return 0;
     }
 
+    if (s->s3->tmp.pkey != NULL) {
+        /* Shouldn't happen! */
+        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
     /*
      * TODO(TLS1.3): Make the number of key_shares sent configurable. For
      * now, just send one
      */
-    for (i = 0; i < num_curves && sharessent < 1; i++, pcurves += 2) {
-        unsigned char *encodedPoint = NULL;
-        unsigned int curve_id = 0;
-        EVP_PKEY *key_share_key = NULL;
-        size_t encodedlen;
-
-        if (!tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED))
-            continue;
-
-        if (s->s3->tmp.pkey != NULL) {
-            /* Shouldn't happen! */
-            SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR);
-            return 0;
-        }
+    if (s->s3->group_id != 0) {
+        curve_id = s->s3->group_id;
+    } else {
+        for (i = 0; i < num_curves; i++, pcurves += 2) {
 
-        /* Generate a key for this key_share */
-        curve_id = (pcurves[0] << 8) | pcurves[1];
-        key_share_key = ssl_generate_pkey_curve(curve_id);
-        if (key_share_key == NULL) {
-            SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_EVP_LIB);
-            return 0;
-        }
+            if (!tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED))
+                continue;
 
-        /* Encode the public key. */
-        encodedlen = EVP_PKEY_get1_tls_encodedpoint(key_share_key,
-                                                    &encodedPoint);
-        if (encodedlen == 0) {
-            SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_EC_LIB);
-            EVP_PKEY_free(key_share_key);
-            return 0;
-        }
-
-        /* Create KeyShareEntry */
-        if (!WPACKET_put_bytes_u16(pkt, curve_id)
-                || !WPACKET_sub_memcpy_u16(pkt, encodedPoint, encodedlen)) {
-            SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR);
-            EVP_PKEY_free(key_share_key);
-            OPENSSL_free(encodedPoint);
-            return 0;
+            curve_id = (pcurves[0] << 8) | pcurves[1];
+            break;
         }
+    }
 
-        /*
-         * TODO(TLS1.3): When changing to send more than one key_share we're
-         * going to need to be able to save more than one EVP_PKEY. For now
-         * we reuse the existing tmp.pkey
-         */
-        s->s3->group_id = curve_id;
-        s->s3->tmp.pkey = key_share_key;
-        sharessent++;
-        OPENSSL_free(encodedPoint);
+    if (curve_id == 0) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE);
+        return 0;
     }
 
+    if (!add_key_share(s, pkt, curve_id))
+        return 0;
+
     if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR);
         return 0;
@@ -1188,6 +1208,49 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
         return 0;
     }
 
+    if ((context & EXT_TLS1_3_HELLO_RETRY_REQUEST) != 0) {
+        unsigned const char *pcurves = NULL;
+        size_t i, num_curves;
+
+        if (PACKET_remaining(pkt) != 0) {
+            *al = SSL_AD_HANDSHAKE_FAILURE;
+            SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, SSL_R_LENGTH_MISMATCH);
+            return 0;
+        }
+
+        /*
+         * It is an error if the HelloRetryRequest wants a key_share that we
+         * already sent in the first ClientHello
+         */
+        if (group_id == s->s3->group_id) {
+            *al = SSL_AD_ILLEGAL_PARAMETER;
+            SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, SSL_R_BAD_KEY_SHARE);
+            return 0;
+        }
+
+        /* Validate the selected group is one we support */
+        pcurves = s->ext.supportedgroups;
+        if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) {
+            SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
+        for (i = 0; i < num_curves; i++, pcurves += 2) {
+            if (group_id == (unsigned int)((pcurves[0] << 8) | pcurves[1]))
+                break;
+        }
+        if (i >= num_curves
+                || !tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED)) {
+            *al = SSL_AD_ILLEGAL_PARAMETER;
+            SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, SSL_R_BAD_KEY_SHARE);
+            return 0;
+        }
+
+        s->s3->group_id = group_id;
+        EVP_PKEY_free(s->s3->tmp.pkey);
+        s->s3->tmp.pkey = NULL;
+        return 1;
+    }
+
     if (group_id != s->s3->group_id) {
         /*
          * This isn't for the group that we sent in the original
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index e5c60aee78..5860a2307d 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -60,6 +60,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_encrypted_extensions(SSL *s, PACKET *pkt);
 
 static ossl_inline int cert_req_allowed(SSL *s);
@@ -137,6 +138,17 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt)
     default:
         break;
 
+    case TLS_ST_CW_CLNT_HELLO:
+        /*
+         * This must a ClientHello following a HelloRetryRequest, so the only
+         * thing we can get now is a ServerHello.
+         */
+        if (mt == SSL3_MT_SERVER_HELLO) {
+            st->hand_state = TLS_ST_CR_SRVR_HELLO;
+            return 1;
+        }
+        break;
+
     case TLS_ST_CR_SRVR_HELLO:
         if (mt == SSL3_MT_ENCRYPTED_EXTENSIONS) {
             st->hand_state = TLS_ST_CR_ENCRYPTED_EXTENSIONS;
@@ -210,8 +222,8 @@ int ossl_statem_client_read_transition(SSL *s, int mt)
     int ske_expected;
 
     /*
-     * Note that after a ClientHello we don't know what version we are going
-     * to negotiate yet, so we don't take this branch until later
+     * Note that after writing the first ClientHello we don't know what version
+     * we are going to negotiate yet, so we don't take this branch until later.
      */
     if (SSL_IS_TLS13(s)) {
         if (!ossl_statem_client13_read_transition(s, mt))
@@ -234,6 +246,11 @@ 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;
 
@@ -390,15 +407,23 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s)
      */
 
     /*
-     * Note: There are no cases for TLS_ST_BEFORE or TLS_ST_CW_CLNT_HELLO,
-     * because we haven't negotiated TLSv1.3 yet at that point. They are
-     * handled by ossl_statem_client_write_transition().
+     * Note: There are no cases for TLS_ST_BEFORE because we haven't negotiated
+     * TLSv1.3 yet at that point. They are handled by
+     * ossl_statem_client_write_transition().
      */
     switch (st->hand_state) {
     default:
         /* Shouldn't happen */
         return WRITE_TRAN_ERROR;
 
+    case TLS_ST_CW_CLNT_HELLO:
+        /* 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:
         st->hand_state = (s->s3->tmp.cert_req != 0) ? TLS_ST_CW_CERT
                                                     : TLS_ST_CW_FINISHED;
@@ -779,6 +804,9 @@ 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;
 
@@ -836,6 +864,9 @@ 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);
 
@@ -1432,6 +1463,52 @@ 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)
+{
+    unsigned int sversion;
+    int protverr;
+    RAW_EXTENSION *extensions = NULL;
+    int al;
+    PACKET extpkt;
+
+    if (!PACKET_get_net_2(pkt, &sversion)) {
+        al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, SSL_R_LENGTH_MISMATCH);
+        goto f_err;
+    }
+
+    s->hello_retry_request = 1;
+
+    /* This will fail if it doesn't choose TLSv1.3+ */
+    protverr = ssl_choose_client_version(s, sversion);
+    if (protverr != 0) {
+        al = SSL_AD_PROTOCOL_VERSION;
+        SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, protverr);
+        goto f_err;
+    }
+
+    if (!PACKET_as_length_prefixed_2(pkt, &extpkt)) {
+        al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, SSL_R_BAD_LENGTH);
+        goto f_err;
+    }
+
+    if (!tls_collect_extensions(s, &extpkt, EXT_TLS1_3_HELLO_RETRY_REQUEST,
+                                &extensions, &al)
+            || !tls_parse_all_extensions(s, EXT_TLS1_3_HELLO_RETRY_REQUEST,
+                                         extensions, NULL, 0, &al))
+        goto f_err;
+
+    OPENSSL_free(extensions);
+
+    return MSG_PROCESS_FINISHED_READING;
+ f_err:
+    ssl3_send_alert(s, SSL3_AL_FATAL, al);
+    ossl_statem_set_error(s);
+    OPENSSL_free(extensions);
+    return MSG_PROCESS_ERROR;
+}
+
 MSG_PROCESS_RETURN tls_process_server_certificate(SSL *s, PACKET *pkt)
 {
     int al, i, ret = MSG_PROCESS_ERROR, exp_idx;
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 13174abb17..c3dd31a9ad 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -1595,6 +1595,9 @@ int ssl_choose_client_version(SSL *s, int version)
             continue;
         if (vent->cmeth == NULL)
             break;
+        if (s->hello_retry_request && version != TLS1_3_VERSION)
+            return SSL_R_WRONG_SSL_VERSION;
+
         method = vent->cmeth();
         err = ssl_method_error(s, method);
         if (err != 0)
diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 35d96cd78c..429d5c703f 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -19,6 +19,7 @@
 /* The spec allows for a longer length than this, but we limit it */
 #define HELLO_VERIFY_REQUEST_MAX_LENGTH 258
 #define SERVER_HELLO_MAX_LENGTH         20000
+#define HELLO_RETRY_REQUEST_MAX_LENGTH  20000
 #define ENCRYPTED_EXTENSIONS_MAX_LENGTH 20000
 #define SERVER_KEY_EXCH_MAX_LENGTH      102400
 #define SERVER_HELLO_DONE_MAX_LENGTH    0