Don't accept early_data if we are going to issue a HelloRetryRequest

Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2737)

ssl/ssl_locl.h

@@ -1783,7 +1783,6 @@ typedef enum tlsext_index_en {
     TLSEXT_IDX_server_name,
     TLSEXT_IDX_srp,
     TLSEXT_IDX_early_data_info,
-    TLSEXT_IDX_early_data,
     TLSEXT_IDX_ec_point_formats,
     TLSEXT_IDX_supported_groups,
     TLSEXT_IDX_session_ticket,
@@ -1799,6 +1798,7 @@ typedef enum tlsext_index_en {
     TLSEXT_IDX_psk_kex_modes,
     TLSEXT_IDX_key_share,
     TLSEXT_IDX_cryptopro_bug,
+    TLSEXT_IDX_early_data,
     TLSEXT_IDX_padding,
     TLSEXT_IDX_psk
 } TLSEXT_INDEX;

ssl/statem/extensions.c

@@ -43,6 +43,7 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al);
 static int init_srtp(SSL *s, unsigned int context);
 #endif
 static int final_sig_algs(SSL *s, unsigned int context, int sent, int *al);
+static int final_early_data(SSL *s, unsigned int context, int sent, int *al);
 
 /* Structure to define a built-in extension */
 typedef struct extensions_definition_st {
@@ -135,12 +136,6 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         NULL, NULL, tls_parse_stoc_early_data_info,
         tls_construct_stoc_early_data_info, NULL, NULL
     },
-    {
-        TLSEXT_TYPE_early_data,
-        EXT_CLIENT_HELLO | EXT_TLS1_3_ENCRYPTED_EXTENSIONS,
-        NULL, tls_parse_ctos_early_data, tls_parse_stoc_early_data,
-        tls_construct_stoc_early_data, tls_construct_ctos_early_data, NULL
-    },
 #ifndef OPENSSL_NO_EC
     {
         TLSEXT_TYPE_ec_point_formats,
@@ -282,6 +277,13 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         EXT_TLS1_2_SERVER_HELLO | EXT_TLS1_2_AND_BELOW_ONLY,
         NULL, NULL, NULL, tls_construct_stoc_cryptopro_bug, NULL, NULL
     },
+    {
+        TLSEXT_TYPE_early_data,
+        EXT_CLIENT_HELLO | EXT_TLS1_3_ENCRYPTED_EXTENSIONS,
+        NULL, tls_parse_ctos_early_data, tls_parse_stoc_early_data,
+        tls_construct_stoc_early_data, tls_construct_ctos_early_data,
+        final_early_data
+    },
     {
         /* Must be immediately before pre_shared_key */
         /* TODO(TLS1.3): Fix me */
@@ -1229,3 +1231,28 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
 
     return ret;
 }
+
+static int final_early_data(SSL *s, unsigned int context, int sent, int *al)
+{
+    if (!s->server || !sent)
+        return 1;
+
+    if (s->max_early_data == 0
+            || !s->hit
+            || s->session->ext.tick_identity != 0
+            || s->early_data_state != SSL_EARLY_DATA_ACCEPTING
+            || !s->ext.early_data_ok
+            || s->hello_retry_request) {
+        s->ext.early_data = SSL_EARLY_DATA_REJECTED;
+    } else {
+        s->ext.early_data = SSL_EARLY_DATA_ACCEPTED;
+
+        if (!tls13_change_cipher_state(s,
+                    SSL3_CC_EARLY | SSL3_CHANGE_CIPHER_SERVER_READ)) {
+            *al = SSL_AD_INTERNAL_ERROR;
+            return 0;
+        }
+    }
+
+    return 1;
+}

ssl/statem/extensions_clnt.c

@@ -108,32 +108,6 @@ static int use_ecc(SSL *s)
     return i < end;
 }
 
-int tls_construct_ctos_early_data(SSL *s, WPACKET *pkt, unsigned int context,
-                                  X509 *x, size_t chainidx, int *al)
-{
-    if (s->early_data_state != SSL_EARLY_DATA_CONNECTING
-            || s->session->ext.max_early_data == 0) {
-        s->max_early_data = 0;
-        return 1;
-    }
-    s->max_early_data = s->session->ext.max_early_data;
-
-    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_early_data)
-            || !WPACKET_start_sub_packet_u16(pkt)
-            || !WPACKET_close(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA, ERR_R_INTERNAL_ERROR);
-        return 0;
-    }
-
-    /*
-     * We set this to rejected here. Later, if the server acknowledges the
-     * extension, we set it to accepted.
-     */
-    s->ext.early_data = SSL_EARLY_DATA_REJECTED;
-
-    return 1;
-}
-
 int tls_construct_ctos_ec_pt_formats(SSL *s, WPACKET *pkt, unsigned int context,
                                      X509 *x, size_t chainidx, int *al)
 {
@@ -662,6 +636,32 @@ int tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, unsigned int context,
     return 1;
 }
 
+int tls_construct_ctos_early_data(SSL *s, WPACKET *pkt, unsigned int context,
+                                  X509 *x, size_t chainidx, int *al)
+{
+    if (s->early_data_state != SSL_EARLY_DATA_CONNECTING
+            || s->session->ext.max_early_data == 0) {
+        s->max_early_data = 0;
+        return 1;
+    }
+    s->max_early_data = s->session->ext.max_early_data;
+
+    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_early_data)
+            || !WPACKET_start_sub_packet_u16(pkt)
+            || !WPACKET_close(pkt)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    /*
+     * We set this to rejected here. Later, if the server acknowledges the
+     * extension, we set it to accepted.
+     */
+    s->ext.early_data = SSL_EARLY_DATA_REJECTED;
+
+    return 1;
+}
+
 #define F5_WORKAROUND_MIN_MSG_LEN   0xff
 #define F5_WORKAROUND_MAX_MSG_LEN   0x200
 
@@ -923,30 +923,6 @@ int tls_parse_stoc_early_data_info(SSL *s, PACKET *pkt, unsigned int context,
     return 1;
 }
 
-int tls_parse_stoc_early_data(SSL *s, PACKET *pkt, unsigned int context,
-                              X509 *x, size_t chainidx, int *al)
-{
-    if (PACKET_remaining(pkt) != 0) {
-        *al = SSL_AD_DECODE_ERROR;
-        return 0;
-    }
-
-    if (s->ext.early_data != SSL_EARLY_DATA_REJECTED
-            || !s->hit
-            || s->session->ext.tick_identity != 0) {
-        /*
-         * If we get here then we didn't send early data, or we didn't resume
-         * using the first identity so the server should not be accepting it.
-         */
-        *al = SSL_AD_ILLEGAL_PARAMETER;
-        return 0;
-    }
-
-    s->ext.early_data = SSL_EARLY_DATA_ACCEPTED;
-
-    return 1;
-}
-
 #ifndef OPENSSL_NO_EC
 int tls_parse_stoc_ec_pt_formats(SSL *s, PACKET *pkt, unsigned int context,
                                  X509 *x, size_t chainidx, int *al)
@@ -1362,6 +1338,30 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     return 1;
 }
 
+int tls_parse_stoc_early_data(SSL *s, PACKET *pkt, unsigned int context,
+                              X509 *x, size_t chainidx, int *al)
+{
+    if (PACKET_remaining(pkt) != 0) {
+        *al = SSL_AD_DECODE_ERROR;
+        return 0;
+    }
+
+    if (s->ext.early_data != SSL_EARLY_DATA_REJECTED
+            || !s->hit
+            || s->session->ext.tick_identity != 0) {
+        /*
+         * If we get here then we didn't send early data, or we didn't resume
+         * using the first identity so the server should not be accepting it.
+         */
+        *al = SSL_AD_ILLEGAL_PARAMETER;
+        return 0;
+    }
+
+    s->ext.early_data = SSL_EARLY_DATA_ACCEPTED;
+
+    return 1;
+}
+
 int tls_parse_stoc_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                        size_t chainidx, int *al)
 {

ssl/statem/extensions_srvr.c

@@ -162,31 +162,6 @@ int tls_parse_ctos_srp(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
 }
 #endif
 
-int tls_parse_ctos_early_data(SSL *s, PACKET *pkt, unsigned int context,
-                              X509 *x, size_t chainidx, int *al)
-{
-    if (PACKET_remaining(pkt) != 0) {
-        *al = SSL_AD_DECODE_ERROR;
-        return 0;
-    }
-
-    if (s->max_early_data == 0 || !s->hit || s->session->ext.tick_identity != 0
-            || s->early_data_state != SSL_EARLY_DATA_ACCEPTING
-            || !s->ext.early_data_ok) {
-        s->ext.early_data = SSL_EARLY_DATA_REJECTED;
-    } else {
-        s->ext.early_data = SSL_EARLY_DATA_ACCEPTED;
-
-        if (!tls13_change_cipher_state(s,
-                    SSL3_CC_EARLY | SSL3_CHANGE_CIPHER_SERVER_READ)) {
-            *al = SSL_AD_INTERNAL_ERROR;
-            return 0;
-        }
-    }
-
-    return 1;
-}
-
 #ifndef OPENSSL_NO_EC
 int tls_parse_ctos_ec_pt_formats(SSL *s, PACKET *pkt, unsigned int context,
                                  X509 *x, size_t chainidx, int *al)
@@ -686,6 +661,18 @@ int tls_parse_ctos_ems(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     return 1;
 }
 
+
+int tls_parse_ctos_early_data(SSL *s, PACKET *pkt, unsigned int context,
+                              X509 *x, size_t chainidx, int *al)
+{
+    if (PACKET_remaining(pkt) != 0) {
+        *al = SSL_AD_DECODE_ERROR;
+        return 0;
+    }
+
+    return 1;
+}
+
 int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                        size_t chainidx, int *al)
 {
@@ -871,22 +858,6 @@ int tls_construct_stoc_early_data_info(SSL *s, WPACKET *pkt,
     return 1;
 }
 
-int tls_construct_stoc_early_data(SSL *s, WPACKET *pkt, unsigned int context,
-                                  X509 *x, size_t chainidx, int *al)
-{
-    if (s->ext.early_data != SSL_EARLY_DATA_ACCEPTED)
-        return 1;
-
-    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_early_data)
-            || !WPACKET_start_sub_packet_u16(pkt)
-            || !WPACKET_close(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_STOC_EARLY_DATA, ERR_R_INTERNAL_ERROR);
-        return 0;
-    }
-
-    return 1;
-}
-
 #ifndef OPENSSL_NO_EC
 int tls_construct_stoc_ec_pt_formats(SSL *s, WPACKET *pkt, unsigned int context,
                                      X509 *x, size_t chainidx, int *al)
@@ -1176,6 +1147,22 @@ int tls_construct_stoc_cryptopro_bug(SSL *s, WPACKET *pkt, unsigned int context,
     return 1;
 }
 
+int tls_construct_stoc_early_data(SSL *s, WPACKET *pkt, unsigned int context,
+                                  X509 *x, size_t chainidx, int *al)
+{
+    if (s->ext.early_data != SSL_EARLY_DATA_ACCEPTED)
+        return 1;
+
+    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_early_data)
+            || !WPACKET_start_sub_packet_u16(pkt)
+            || !WPACKET_close(pkt)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_STOC_EARLY_DATA, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    return 1;
+}
+
 int tls_construct_stoc_psk(SSL *s, WPACKET *pkt, unsigned int context, X509 *x,
                            size_t chainidx, int *al)
 {