Provider a better error message if we fail to copy parameters
[openssl.git] / ssl / statem / extensions_clnt.c
index 1fbf9f6e0e6580ae56851861456ef7dabb9319ce..dbdedad1abc196f36778f073d87e4f6e48fc04fa 100644 (file)
@@ -1,16 +1,20 @@
 /*
- * Copyright 2016-2017 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2016-2020 The OpenSSL Project Authors. All Rights Reserved.
  *
- * Licensed under the OpenSSL license (the "License").  You may not use
+ * Licensed under the Apache License 2.0 (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
  */
 
 #include <openssl/ocsp.h>
-#include "../ssl_locl.h"
+#include "../ssl_local.h"
 #include "internal/cryptlib.h"
-#include "statem_locl.h"
+#include "statem_local.h"
+
+DEFINE_STACK_OF(SRTP_PROTECTION_PROFILE)
+DEFINE_STACK_OF_CONST(SSL_CIPHER)
+DEFINE_STACK_OF(OCSP_RESPID)
 
 EXT_RETURN tls_construct_ctos_renegotiate(SSL *s, WPACKET *pkt,
                                           unsigned int context, X509 *x,
@@ -22,8 +26,8 @@ EXT_RETURN tls_construct_ctos_renegotiate(SSL *s, WPACKET *pkt,
 
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_renegotiate)
             || !WPACKET_start_sub_packet_u16(pkt)
-            || !WPACKET_sub_memcpy_u8(pkt, s->s3->previous_client_finished,
-                               s->s3->previous_client_finished_len)
+            || !WPACKET_sub_memcpy_u8(pkt, s->s3.previous_client_finished,
+                               s->s3.previous_client_finished_len)
             || !WPACKET_close(pkt)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_RENEGOTIATE,
                  ERR_R_INTERNAL_ERROR);
@@ -113,17 +117,19 @@ EXT_RETURN tls_construct_ctos_srp(SSL *s, WPACKET *pkt, unsigned int context,
 #endif
 
 #ifndef OPENSSL_NO_EC
-static int use_ecc(SSL *s)
+static int use_ecc(SSL *s, int min_version, int max_version)
 {
-    int i, end;
+    int i, end, ret = 0;
     unsigned long alg_k, alg_a;
     STACK_OF(SSL_CIPHER) *cipher_stack = NULL;
+    const uint16_t *pgroups = NULL;
+    size_t num_groups, j;
 
     /* See if we support any ECC ciphersuites */
     if (s->version == SSL3_VERSION)
         return 0;
 
-    cipher_stack = SSL_get_ciphers(s);
+    cipher_stack = SSL_get1_supported_ciphers(s);
     end = sk_SSL_CIPHER_num(cipher_stack);
     for (i = 0; i < end; i++) {
         const SSL_CIPHER *c = sk_SSL_CIPHER_value(cipher_stack, i);
@@ -132,7 +138,22 @@ static int use_ecc(SSL *s)
         alg_a = c->algorithm_auth;
         if ((alg_k & (SSL_kECDHE | SSL_kECDHEPSK))
                 || (alg_a & SSL_aECDSA)
-                || c->min_tls >= TLS1_3_VERSION)
+                || c->min_tls >= TLS1_3_VERSION) {
+            ret = 1;
+            break;
+        }
+    }
+    sk_SSL_CIPHER_free(cipher_stack);
+    if (!ret)
+        return 0;
+
+    /* Check we have at least one EC supported group */
+    tls1_get_supported_groups(s, &pgroups, &num_groups);
+    for (j = 0; j < num_groups; j++) {
+        uint16_t ctmp = pgroups[j];
+
+        if (tls_valid_group(s, ctmp, min_version, max_version)
+                && tls_group_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED))
             return 1;
     }
 
@@ -145,8 +166,15 @@ EXT_RETURN tls_construct_ctos_ec_pt_formats(SSL *s, WPACKET *pkt,
 {
     const unsigned char *pformats;
     size_t num_formats;
+    int reason, min_version, max_version;
 
-    if (!use_ecc(s))
+    reason = ssl_get_min_max_version(s, &min_version, &max_version, NULL);
+    if (reason != 0) {
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                 SSL_F_TLS_CONSTRUCT_CTOS_EC_PT_FORMATS, reason);
+        return EXT_RETURN_FAIL;
+    }
+    if (!use_ecc(s, min_version, max_version))
         return EXT_RETURN_NOT_SENT;
 
     /* Add TLS extension ECPointFormats to the ClientHello message */
@@ -164,43 +192,59 @@ EXT_RETURN tls_construct_ctos_ec_pt_formats(SSL *s, WPACKET *pkt,
 
     return EXT_RETURN_SENT;
 }
+#endif
 
+#if !defined(OPENSSL_NO_DH) || !defined(OPENSSL_NO_EC)
 EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
                                                unsigned int context, X509 *x,
                                                size_t chainidx)
 {
     const uint16_t *pgroups = NULL;
     size_t num_groups = 0, i;
+    int min_version, max_version, reason;
 
-    if (!use_ecc(s))
+    reason = ssl_get_min_max_version(s, &min_version, &max_version, NULL);
+    if (reason != 0) {
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                 SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS, reason);
+        return EXT_RETURN_FAIL;
+    }
+
+#if defined(OPENSSL_NO_EC)
+    if (max_version < TLS1_3_VERSION)
+        return EXT_RETURN_NOT_SENT;
+#else
+    if (!use_ecc(s, min_version, max_version) && max_version < TLS1_3_VERSION)
         return EXT_RETURN_NOT_SENT;
+#endif
 
     /*
      * Add TLS extension supported_groups to the ClientHello message
      */
-    /* TODO(TLS1.3): Add support for DHE groups */
     tls1_get_supported_groups(s, &pgroups, &num_groups);
 
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_groups)
                /* Sub-packet for supported_groups extension */
             || !WPACKET_start_sub_packet_u16(pkt)
-            || !WPACKET_start_sub_packet_u16(pkt)) {
+            || !WPACKET_start_sub_packet_u16(pkt)
+            || !WPACKET_set_flags(pkt, WPACKET_FLAGS_NON_ZERO_LENGTH)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                  SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS,
                  ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }
-    /* Copy curve ID if supported */
+    /* Copy group ID if supported */
     for (i = 0; i < num_groups; i++) {
         uint16_t ctmp = pgroups[i];
 
-        if (tls_curve_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) {
+        if (tls_valid_group(s, ctmp, min_version, max_version)
+                && tls_group_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) {
             if (!WPACKET_put_bytes_u16(pkt, ctmp)) {
-                    SSLfatal(s, SSL_AD_INTERNAL_ERROR,
-                             SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS,
-                             ERR_R_INTERNAL_ERROR);
-                    return EXT_RETURN_FAIL;
-                }
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                         SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS,
+                         ERR_R_INTERNAL_ERROR);
+                return EXT_RETURN_FAIL;
+            }
         }
     }
     if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) {
@@ -384,7 +428,7 @@ EXT_RETURN tls_construct_ctos_npn(SSL *s, WPACKET *pkt, unsigned int context,
 EXT_RETURN tls_construct_ctos_alpn(SSL *s, WPACKET *pkt, unsigned int context,
                                    X509 *x, size_t chainidx)
 {
-    s->s3->alpn_sent = 0;
+    s->s3.alpn_sent = 0;
 
     if (s->ext.alpn == NULL || !SSL_IS_FIRST_HANDSHAKE(s))
         return EXT_RETURN_NOT_SENT;
@@ -399,7 +443,7 @@ EXT_RETURN tls_construct_ctos_alpn(SSL *s, WPACKET *pkt, unsigned int context,
                  ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }
-    s->s3->alpn_sent = 1;
+    s->s3.alpn_sent = 1;
 
     return EXT_RETURN_SENT;
 }
@@ -491,6 +535,9 @@ EXT_RETURN tls_construct_ctos_sct(SSL *s, WPACKET *pkt, unsigned int context,
 EXT_RETURN tls_construct_ctos_ems(SSL *s, WPACKET *pkt, unsigned int context,
                                   X509 *x, size_t chainidx)
 {
+    if (s->options & SSL_OP_NO_EXTENDED_MASTER_SECRET)
+        return EXT_RETURN_NOT_SENT;
+
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_extended_master_secret)
             || !WPACKET_put_bytes_u16(pkt, 0)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_EMS,
@@ -507,7 +554,7 @@ EXT_RETURN tls_construct_ctos_supported_versions(SSL *s, WPACKET *pkt,
 {
     int currv, min_version, max_version, reason;
 
-    reason = ssl_get_min_max_version(s, &min_version, &max_version);
+    reason = ssl_get_min_max_version(s, &min_version, &max_version, NULL);
     if (reason != 0) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                  SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS, reason);
@@ -530,21 +577,8 @@ EXT_RETURN tls_construct_ctos_supported_versions(SSL *s, WPACKET *pkt,
         return EXT_RETURN_FAIL;
     }
 
-    /*
-     * TODO(TLS1.3): There is some discussion on the TLS list as to whether
-     * we should include versions <TLS1.2. For the moment we do. To be
-     * reviewed later.
-     */
     for (currv = max_version; currv >= min_version; currv--) {
-        /* TODO(TLS1.3): Remove this first if clause prior to release!! */
-        if (currv == TLS1_3_VERSION) {
-            if (!WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT)) {
-                SSLfatal(s, SSL_AD_INTERNAL_ERROR,
-                         SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS,
-                         ERR_R_INTERNAL_ERROR);
-                return EXT_RETURN_FAIL;
-            }
-        } else if (!WPACKET_put_bytes_u16(pkt, currv)) {
+        if (!WPACKET_put_bytes_u16(pkt, currv)) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                      SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS,
                      ERR_R_INTERNAL_ERROR);
@@ -598,7 +632,7 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
     EVP_PKEY *key_share_key = NULL;
     size_t encodedlen;
 
-    if (s->s3->tmp.pkey != NULL) {
+    if (s->s3.tmp.pkey != NULL) {
         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);
@@ -607,7 +641,7 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
         /*
          * Could happen if we got an HRR that wasn't requesting a new key_share
          */
-        key_share_key = s->s3->tmp.pkey;
+        key_share_key = s->s3.tmp.pkey;
     } else {
         key_share_key = ssl_generate_pkey_group(s, curve_id);
         if (key_share_key == NULL) {
@@ -637,13 +671,13 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
      * 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;
+    s->s3.tmp.pkey = key_share_key;
+    s->s3.group_id = curve_id;
     OPENSSL_free(encoded_point);
 
     return 1;
  err:
-    if (s->s3->tmp.pkey == NULL)
+    if (s->s3.tmp.pkey == NULL)
         EVP_PKEY_free(key_share_key);
     OPENSSL_free(encoded_point);
     return 0;
@@ -676,12 +710,12 @@ EXT_RETURN tls_construct_ctos_key_share(SSL *s, WPACKET *pkt,
      * TODO(TLS1.3): Make the number of key_shares sent configurable. For
      * now, just send one
      */
-    if (s->s3->group_id != 0) {
-        curve_id = s->s3->group_id;
+    if (s->s3.group_id != 0) {
+        curve_id = s->s3.group_id;
     } else {
         for (i = 0; i < num_groups; i++) {
 
-            if (!tls_curve_allowed(s, pgroups[i], SSL_SECOP_CURVE_SUPPORTED))
+            if (!tls_group_allowed(s, pgroups[i], SSL_SECOP_CURVE_SUPPORTED))
                 continue;
 
             curve_id = pgroups[i];
@@ -705,9 +739,10 @@ EXT_RETURN tls_construct_ctos_key_share(SSL *s, WPACKET *pkt,
                  ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }
-#endif
-
     return EXT_RETURN_SENT;
+#else
+    return EXT_RETURN_NOT_SENT;
+#endif
 }
 
 EXT_RETURN tls_construct_ctos_cookie(SSL *s, WPACKET *pkt, unsigned int context,
@@ -743,6 +778,9 @@ EXT_RETURN tls_construct_ctos_early_data(SSL *s, WPACKET *pkt,
                                          unsigned int context, X509 *x,
                                          size_t chainidx)
 {
+#ifndef OPENSSL_NO_PSK
+    char identity[PSK_MAX_IDENTITY_LEN + 1];
+#endif  /* OPENSSL_NO_PSK */
     const unsigned char *id = NULL;
     size_t idlen = 0;
     SSL_SESSION *psksess = NULL;
@@ -762,6 +800,60 @@ EXT_RETURN tls_construct_ctos_early_data(SSL *s, WPACKET *pkt,
         return EXT_RETURN_FAIL;
     }
 
+#ifndef OPENSSL_NO_PSK
+    if (psksess == NULL && s->psk_client_callback != NULL) {
+        unsigned char psk[PSK_MAX_PSK_LEN];
+        size_t psklen = 0;
+
+        memset(identity, 0, sizeof(identity));
+        psklen = s->psk_client_callback(s, NULL, identity, sizeof(identity) - 1,
+                                        psk, sizeof(psk));
+
+        if (psklen > PSK_MAX_PSK_LEN) {
+            SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE,
+                     SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA, ERR_R_INTERNAL_ERROR);
+            return EXT_RETURN_FAIL;
+        } else if (psklen > 0) {
+            const unsigned char tls13_aes128gcmsha256_id[] = { 0x13, 0x01 };
+            const SSL_CIPHER *cipher;
+
+            idlen = strlen(identity);
+            if (idlen > PSK_MAX_IDENTITY_LEN) {
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                         SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA,
+                         ERR_R_INTERNAL_ERROR);
+                return EXT_RETURN_FAIL;
+            }
+            id = (unsigned char *)identity;
+
+            /*
+             * We found a PSK using an old style callback. We don't know
+             * the digest so we default to SHA256 as per the TLSv1.3 spec
+             */
+            cipher = SSL_CIPHER_find(s, tls13_aes128gcmsha256_id);
+            if (cipher == NULL) {
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                         SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA,
+                         ERR_R_INTERNAL_ERROR);
+                return EXT_RETURN_FAIL;
+            }
+
+            psksess = SSL_SESSION_new();
+            if (psksess == NULL
+                    || !SSL_SESSION_set1_master_key(psksess, psk, psklen)
+                    || !SSL_SESSION_set_cipher(psksess, cipher)
+                    || !SSL_SESSION_set_protocol_version(psksess, TLS1_3_VERSION)) {
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                         SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA,
+                         ERR_R_INTERNAL_ERROR);
+                OPENSSL_cleanse(psk, psklen);
+                return EXT_RETURN_FAIL;
+            }
+            OPENSSL_cleanse(psk, psklen);
+        }
+    }
+#endif  /* OPENSSL_NO_PSK */
+
     SSL_SESSION_free(s->psksession);
     s->psksession = psksess;
     if (psksess != NULL) {
@@ -893,7 +985,7 @@ EXT_RETURN tls_construct_ctos_padding(SSL *s, WPACKET *pkt,
     if (s->session->ssl_version == TLS1_3_VERSION
             && s->session->ext.ticklen != 0
             && s->session->cipher != NULL) {
-        const EVP_MD *md = ssl_md(s->session->cipher->algorithm2);
+        const EVP_MD *md = ssl_md(s->ctx, s->session->cipher->algorithm2);
 
         if (md != NULL) {
             /*
@@ -945,7 +1037,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
     const EVP_MD *handmd = NULL, *mdres = NULL, *mdpsk = NULL;
     int dores = 0;
 
-    s->session->ext.tick_identity = TLSEXT_PSK_BAD_IDENTITY;
+    s->ext.tick_identity = 0;
 
     /*
      * Note: At this stage of the code we only support adding a single
@@ -971,7 +1063,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
                      ERR_R_INTERNAL_ERROR);
             return EXT_RETURN_FAIL;
         }
-        mdres = ssl_md(s->session->cipher->algorithm2);
+        mdres = ssl_md(s->ctx, s->session->cipher->algorithm2);
         if (mdres == NULL) {
             /*
              * Don't recognize this cipher so we can't use the session.
@@ -998,6 +1090,16 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
          */
         now = (uint32_t)time(NULL);
         agesec = now - (uint32_t)s->session->time;
+        /*
+         * We calculate the age in seconds but the server may work in ms. Due to
+         * rounding errors we could overestimate the age by up to 1s. It is
+         * better to underestimate it. Otherwise, if the RTT is very short, when
+         * the server calculates the age reported by the client it could be
+         * bigger than the age calculated on the server - which should never
+         * happen.
+         */
+        if (agesec > 0)
+            agesec--;
 
         if (s->session->ext.tick_lifetime_hint < agesec) {
             /* Ticket is too old. Ignore it. */
@@ -1025,6 +1127,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
         agems += s->session->ext.tick_age_add;
 
         reshashsize = EVP_MD_size(mdres);
+        s->ext.tick_identity++;
         dores = 1;
     }
 
@@ -1033,7 +1136,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
         return EXT_RETURN_NOT_SENT;
 
     if (s->psksession != NULL) {
-        mdpsk = ssl_md(s->psksession->cipher->algorithm2);
+        mdpsk = ssl_md(s->ctx, s->psksession->cipher->algorithm2);
         if (mdpsk == NULL) {
             /*
              * Don't recognize this cipher so we can't use the session.
@@ -1084,6 +1187,7 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
                      ERR_R_INTERNAL_ERROR);
             return EXT_RETURN_FAIL;
         }
+        s->ext.tick_identity++;
     }
 
     if (!WPACKET_close(pkt)
@@ -1122,10 +1226,31 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
         return EXT_RETURN_FAIL;
     }
 
-    if (dores)
-        s->session->ext.tick_identity = 0;
-    if (s->psksession != NULL)
-        s->psksession->ext.tick_identity = (dores ? 1 : 0);
+    return EXT_RETURN_SENT;
+#else
+    return EXT_RETURN_NOT_SENT;
+#endif
+}
+
+EXT_RETURN tls_construct_ctos_post_handshake_auth(SSL *s, WPACKET *pkt,
+                                                  unsigned int context,
+                                                  X509 *x, size_t chainidx)
+{
+#ifndef OPENSSL_NO_TLS1_3
+    if (!s->pha_enabled)
+        return EXT_RETURN_NOT_SENT;
+
+    /* construct extension - 0 length, no contents */
+    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_post_handshake_auth)
+            || !WPACKET_start_sub_packet_u16(pkt)
+            || !WPACKET_close(pkt)) {
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                 SSL_F_TLS_CONSTRUCT_CTOS_POST_HANDSHAKE_AUTH,
+                 ERR_R_INTERNAL_ERROR);
+        return EXT_RETURN_FAIL;
+    }
+
+    s->post_handshake_auth = SSL_PHA_EXT_SENT;
 
     return EXT_RETURN_SENT;
 #else
@@ -1133,22 +1258,23 @@ EXT_RETURN tls_construct_ctos_psk(SSL *s, WPACKET *pkt, unsigned int context,
 #endif
 }
 
+
 /*
  * Parse the server's renegotiation binding and abort if it's not right
  */
 int tls_parse_stoc_renegotiate(SSL *s, PACKET *pkt, unsigned int context,
                                X509 *x, size_t chainidx)
 {
-    size_t expected_len = s->s3->previous_client_finished_len
-        + s->s3->previous_server_finished_len;
+    size_t expected_len = s->s3.previous_client_finished_len
+        + s->s3.previous_server_finished_len;
     size_t ilen;
     const unsigned char *data;
 
     /* Check for logic errors */
     if (!ossl_assert(expected_len == 0
-                     || s->s3->previous_client_finished_len != 0)
+                     || s->s3.previous_client_finished_len != 0)
         || !ossl_assert(expected_len == 0
-                        || s->s3->previous_server_finished_len != 0)) {
+                        || s->s3.previous_server_finished_len != 0)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_STOC_RENEGOTIATE,
                  ERR_R_INTERNAL_ERROR);
         return 0;
@@ -1175,22 +1301,22 @@ int tls_parse_stoc_renegotiate(SSL *s, PACKET *pkt, unsigned int context,
         return 0;
     }
 
-    if (!PACKET_get_bytes(pkt, &data, s->s3->previous_client_finished_len)
-        || memcmp(data, s->s3->previous_client_finished,
-                  s->s3->previous_client_finished_len) != 0) {
+    if (!PACKET_get_bytes(pkt, &data, s->s3.previous_client_finished_len)
+        || memcmp(data, s->s3.previous_client_finished,
+                  s->s3.previous_client_finished_len) != 0) {
         SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PARSE_STOC_RENEGOTIATE,
                  SSL_R_RENEGOTIATION_MISMATCH);
         return 0;
     }
 
-    if (!PACKET_get_bytes(pkt, &data, s->s3->previous_server_finished_len)
-        || memcmp(data, s->s3->previous_server_finished,
-                  s->s3->previous_server_finished_len) != 0) {
+    if (!PACKET_get_bytes(pkt, &data, s->s3.previous_server_finished_len)
+        || memcmp(data, s->s3.previous_server_finished,
+                  s->s3.previous_server_finished_len) != 0) {
         SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PARSE_STOC_RENEGOTIATE,
                  SSL_R_RENEGOTIATION_MISMATCH);
         return 0;
     }
-    s->s3->send_connection_binding = 1;
+    s->s3.send_connection_binding = 1;
 
     return 1;
 }
@@ -1202,7 +1328,7 @@ int tls_parse_stoc_maxfragmentlen(SSL *s, PACKET *pkt, unsigned int context,
     unsigned int value;
 
     if (PACKET_remaining(pkt) != 1 || !PACKET_get_1(pkt, &value)) {
-        SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PARSE_STOC_MAXFRAGMENTLEN, 
+        SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PARSE_STOC_MAXFRAGMENTLEN,
                  SSL_R_BAD_EXTENSION);
         return 0;
     }
@@ -1273,7 +1399,7 @@ int tls_parse_stoc_server_name(SSL *s, PACKET *pkt, unsigned int context,
 int tls_parse_stoc_ec_pt_formats(SSL *s, PACKET *pkt, unsigned int context,
                                  X509 *x, size_t chainidx)
 {
-    unsigned int ecpointformats_len;
+    size_t ecpointformats_len;
     PACKET ecptformatlist;
 
     if (!PACKET_as_length_prefixed_1(pkt, &ecptformatlist)) {
@@ -1283,20 +1409,25 @@ int tls_parse_stoc_ec_pt_formats(SSL *s, PACKET *pkt, unsigned int context,
     }
     if (!s->hit) {
         ecpointformats_len = PACKET_remaining(&ecptformatlist);
-        s->session->ext.ecpointformats_len = 0;
+        if (ecpointformats_len == 0) {
+            SSLfatal(s, SSL_AD_DECODE_ERROR,
+                     SSL_F_TLS_PARSE_STOC_EC_PT_FORMATS, SSL_R_BAD_LENGTH);
+            return 0;
+        }
 
-        OPENSSL_free(s->session->ext.ecpointformats);
-        s->session->ext.ecpointformats = OPENSSL_malloc(ecpointformats_len);
-        if (s->session->ext.ecpointformats == NULL) {
+        s->ext.peer_ecpointformats_len = 0;
+        OPENSSL_free(s->ext.peer_ecpointformats);
+        s->ext.peer_ecpointformats = OPENSSL_malloc(ecpointformats_len);
+        if (s->ext.peer_ecpointformats == NULL) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                      SSL_F_TLS_PARSE_STOC_EC_PT_FORMATS, ERR_R_INTERNAL_ERROR);
             return 0;
         }
 
-        s->session->ext.ecpointformats_len = ecpointformats_len;
+        s->ext.peer_ecpointformats_len = ecpointformats_len;
 
         if (!PACKET_copy_bytes(&ecptformatlist,
-                               s->session->ext.ecpointformats,
+                               s->ext.peer_ecpointformats,
                                ecpointformats_len)) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                      SSL_F_TLS_PARSE_STOC_EC_PT_FORMATS, ERR_R_INTERNAL_ERROR);
@@ -1340,6 +1471,12 @@ int tls_parse_stoc_session_ticket(SSL *s, PACKET *pkt, unsigned int context,
 int tls_parse_stoc_status_request(SSL *s, PACKET *pkt, unsigned int context,
                                   X509 *x, size_t chainidx)
 {
+    if (context == SSL_EXT_TLS1_3_CERTIFICATE_REQUEST) {
+        /* We ignore this if the server sends a CertificateRequest */
+        /* TODO(TLS1.3): Add support for this */
+        return 1;
+    }
+
     /*
      * MUST only be sent if we've requested a status
      * request message. In TLS <= 1.2 it must also be empty.
@@ -1378,6 +1515,12 @@ int tls_parse_stoc_status_request(SSL *s, PACKET *pkt, unsigned int context,
 int tls_parse_stoc_sct(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                        size_t chainidx)
 {
+    if (context == SSL_EXT_TLS1_3_CERTIFICATE_REQUEST) {
+        /* We ignore this if the server sends it in a CertificateRequest */
+        /* TODO(TLS1.3): Add support for this */
+        return 1;
+    }
+
     /*
      * Only take it if we asked for it - i.e if there is no CT validation
      * callback set, then a custom extension MAY be processing it, so we
@@ -1500,7 +1643,7 @@ int tls_parse_stoc_npn(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
 
     memcpy(s->ext.npn, selected, selected_len);
     s->ext.npn_len = selected_len;
-    s->s3->npn_seen = 1;
+    s->s3.npn_seen = 1;
 
     return 1;
 }
@@ -1512,7 +1655,7 @@ int tls_parse_stoc_alpn(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     size_t len;
 
     /* We must have requested it. */
-    if (!s->s3->alpn_sent) {
+    if (!s->s3.alpn_sent) {
         SSLfatal(s, SSL_AD_UNSUPPORTED_EXTENSION, SSL_F_TLS_PARSE_STOC_ALPN,
                  SSL_R_BAD_EXTENSION);
         return 0;
@@ -1530,37 +1673,45 @@ int tls_parse_stoc_alpn(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                  SSL_R_BAD_EXTENSION);
         return 0;
     }
-    OPENSSL_free(s->s3->alpn_selected);
-    s->s3->alpn_selected = OPENSSL_malloc(len);
-    if (s->s3->alpn_selected == NULL) {
+    OPENSSL_free(s->s3.alpn_selected);
+    s->s3.alpn_selected = OPENSSL_malloc(len);
+    if (s->s3.alpn_selected == NULL) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_STOC_ALPN,
                  ERR_R_INTERNAL_ERROR);
         return 0;
     }
-    if (!PACKET_copy_bytes(pkt, s->s3->alpn_selected, len)) {
+    if (!PACKET_copy_bytes(pkt, s->s3.alpn_selected, len)) {
         SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PARSE_STOC_ALPN,
                  SSL_R_BAD_EXTENSION);
         return 0;
     }
-    s->s3->alpn_selected_len = len;
+    s->s3.alpn_selected_len = len;
 
     if (s->session->ext.alpn_selected == NULL
             || s->session->ext.alpn_selected_len != len
-            || memcmp(s->session->ext.alpn_selected, s->s3->alpn_selected, len)
+            || memcmp(s->session->ext.alpn_selected, s->s3.alpn_selected, len)
                != 0) {
         /* ALPN not consistent with the old session so cannot use early_data */
         s->ext.early_data_ok = 0;
     }
     if (!s->hit) {
-        /* If a new session then update it with the selected ALPN */
+        /*
+         * This is a new session and so alpn_selected should have been
+         * initialised to NULL. We should update it with the selected ALPN.
+         */
+        if (!ossl_assert(s->session->ext.alpn_selected == NULL)) {
+            SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_STOC_ALPN,
+                     ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
         s->session->ext.alpn_selected =
-            OPENSSL_memdup(s->s3->alpn_selected, s->s3->alpn_selected_len);
+            OPENSSL_memdup(s->s3.alpn_selected, s->s3.alpn_selected_len);
         if (s->session->ext.alpn_selected == NULL) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_STOC_ALPN,
                      ERR_R_INTERNAL_ERROR);
             return 0;
         }
-        s->session->ext.alpn_selected_len = s->s3->alpn_selected_len;
+        s->session->ext.alpn_selected_len = s->s3.alpn_selected_len;
     }
 
     return 1;
@@ -1623,8 +1774,8 @@ int tls_parse_stoc_etm(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
 {
     /* Ignore if inappropriate ciphersuite */
     if (!(s->options & SSL_OP_NO_ENCRYPT_THEN_MAC)
-            && s->s3->tmp.new_cipher->algorithm_mac != SSL_AEAD
-            && s->s3->tmp.new_cipher->algorithm_enc != SSL_RC4)
+            && s->s3.tmp.new_cipher->algorithm_mac != SSL_AEAD
+            && s->s3.tmp.new_cipher->algorithm_enc != SSL_RC4)
         s->ext.use_etm = 1;
 
     return 1;
@@ -1633,7 +1784,9 @@ int tls_parse_stoc_etm(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
 int tls_parse_stoc_ems(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                        size_t chainidx)
 {
-    s->s3->flags |= TLS1_FLAGS_RECEIVED_EXTMS;
+    if (s->options & SSL_OP_NO_EXTENDED_MASTER_SECRET)
+        return 1;
+    s->s3.flags |= TLS1_FLAGS_RECEIVED_EXTMS;
     if (!s->hit)
         s->session->flags |= SSL_SESS_FLAG_EXTMS;
 
@@ -1653,9 +1806,20 @@ int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context,
         return 0;
     }
 
-    /* TODO(TLS1.3): Remove this before release */
-    if (version == TLS1_3_VERSION_DRAFT)
-        version = TLS1_3_VERSION;
+    /*
+     * The only protocol version we support which is valid in this extension in
+     * a ServerHello is TLSv1.3 therefore we shouldn't be getting anything else.
+     */
+    if (version != TLS1_3_VERSION) {
+        SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
+                 SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS,
+                 SSL_R_BAD_PROTOCOL_VERSION_NUMBER);
+        return 0;
+    }
+
+    /* We ignore this extension for HRRs except to sanity check it */
+    if (context == SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST)
+        return 1;
 
     /* We just set it here. We validate it in ssl_choose_client_version */
     s->version = version;
@@ -1669,10 +1833,10 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
 #ifndef OPENSSL_NO_TLS1_3
     unsigned int group_id;
     PACKET encoded_pt;
-    EVP_PKEY *ckey = s->s3->tmp.pkey, *skey = NULL;
+    EVP_PKEY *ckey = s->s3.tmp.pkey, *skey = NULL;
 
     /* Sanity check */
-    if (ckey == NULL || s->s3->peer_tmp != NULL) {
+    if (ckey == NULL || s->s3.peer_tmp != NULL) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_STOC_KEY_SHARE,
                  ERR_R_INTERNAL_ERROR);
         return 0;
@@ -1698,7 +1862,7 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
          * 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) {
+        if (group_id == s->s3.group_id) {
             SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
                      SSL_F_TLS_PARSE_STOC_KEY_SHARE, SSL_R_BAD_KEY_SHARE);
             return 0;
@@ -1711,19 +1875,19 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                 break;
         }
         if (i >= num_groups
-                || !tls_curve_allowed(s, group_id, SSL_SECOP_CURVE_SUPPORTED)) {
+                || !tls_group_allowed(s, group_id, SSL_SECOP_CURVE_SUPPORTED)) {
             SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
                      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;
+        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) {
+    if (group_id != s->s3.group_id) {
         /*
          * This isn't for the group that we sent in the original
          * key_share!
@@ -1740,12 +1904,13 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
         return 0;
     }
 
-    skey = ssl_generate_pkey(ckey);
-    if (skey == NULL) {
+    skey = EVP_PKEY_new();
+    if (skey == NULL || EVP_PKEY_copy_parameters(skey, ckey) <= 0) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_STOC_KEY_SHARE,
-                 ERR_R_MALLOC_FAILURE);
+                 SSL_R_COPY_PARAMETERS_FAILED);
         return 0;
     }
+
     if (!EVP_PKEY_set1_tls_encodedpoint(skey, PACKET_data(&encoded_pt),
                                         PACKET_remaining(&encoded_pt))) {
         SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PARSE_STOC_KEY_SHARE,
@@ -1759,7 +1924,7 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
         EVP_PKEY_free(skey);
         return 0;
     }
-    s->s3->peer_tmp = skey;
+    s->s3.peer_tmp = skey;
 #endif
 
     return 1;
@@ -1806,8 +1971,7 @@ int tls_parse_stoc_early_data(SSL *s, PACKET *pkt, unsigned int context,
     }
 
     if (!s->ext.early_data_ok
-            || !s->hit
-            || s->session->ext.tick_identity != 0) {
+            || !s->hit) {
         /*
          * If we get here then we didn't send early data, or we didn't resume
          * using the first identity, or the SNI/ALPN is not consistent so the
@@ -1835,17 +1999,28 @@ int tls_parse_stoc_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
         return 0;
     }
 
-    if (s->session->ext.tick_identity == (int)identity) {
+    if (identity >= (unsigned int)s->ext.tick_identity) {
+        SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PARSE_STOC_PSK,
+                 SSL_R_BAD_PSK_IDENTITY);
+        return 0;
+    }
+
+    /*
+     * Session resumption tickets are always sent before PSK tickets. If the
+     * ticket index is 0 then it must be for a session resumption ticket if we
+     * sent two tickets, or if we didn't send a PSK ticket.
+     */
+    if (identity == 0 && (s->psksession == NULL || s->ext.tick_identity == 2)) {
         s->hit = 1;
         SSL_SESSION_free(s->psksession);
         s->psksession = NULL;
         return 1;
     }
 
-    if (s->psksession == NULL
-            || s->psksession->ext.tick_identity != (int)identity) {
-        SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PARSE_STOC_PSK,
-                 SSL_R_BAD_PSK_IDENTITY);
+    if (s->psksession == NULL) {
+        /* Should never happen */
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_STOC_PSK,
+                 ERR_R_INTERNAL_ERROR);
         return 0;
     }
 
@@ -1864,6 +2039,9 @@ int tls_parse_stoc_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     s->session = s->psksession;
     s->psksession = NULL;
     s->hit = 1;
+    /* Early data is only allowed if we used the first ticket */
+    if (identity != 0)
+        s->ext.early_data_ok = 0;
 #endif
 
     return 1;