Correct pointer to be freed
[openssl.git] / ssl / statem / statem_srvr.c
index 3c4d6ee7681a8b9346a8b3bce1762fa33f936c64..2e76b80b8647296d1de67d8247d82ca542893417 100644 (file)
@@ -61,6 +61,7 @@
 #include <openssl/bn.h>
 #include <openssl/md5.h>
 
+static int tls_construct_encrypted_extensions(SSL *s, WPACKET *pkt);
 static STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,
                                                       PACKET *cipher_suites,
                                                       STACK_OF(SSL_CIPHER)
@@ -94,15 +95,15 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt)
     default:
         break;
 
-    case TLS_ST_SW_SRVR_DONE:
+    case TLS_ST_SW_FINISHED:
         if (s->s3->tmp.cert_request) {
             if (mt == SSL3_MT_CERTIFICATE) {
                 st->hand_state = TLS_ST_SR_CERT;
                 return 1;
             }
         } else {
-            if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
-                st->hand_state = TLS_ST_SR_CHANGE;
+            if (mt == SSL3_MT_FINISHED) {
+                st->hand_state = TLS_ST_SR_FINISHED;
                 return 1;
             }
         }
@@ -110,8 +111,8 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt)
 
     case TLS_ST_SR_CERT:
         if (s->session->peer == NULL) {
-            if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
-                st->hand_state = TLS_ST_SR_CHANGE;
+            if (mt == SSL3_MT_FINISHED) {
+                st->hand_state = TLS_ST_SR_FINISHED;
                 return 1;
             }
         } else {
@@ -123,25 +124,11 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt)
         break;
 
     case TLS_ST_SR_CERT_VRFY:
-        if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
-            st->hand_state = TLS_ST_SR_CHANGE;
-            return 1;
-        }
-        break;
-
-    case TLS_ST_SR_CHANGE:
         if (mt == SSL3_MT_FINISHED) {
             st->hand_state = TLS_ST_SR_FINISHED;
             return 1;
         }
         break;
-
-    case TLS_ST_SW_FINISHED:
-        if (mt == SSL3_MT_CHANGE_CIPHER_SPEC) {
-            st->hand_state = TLS_ST_SR_CHANGE;
-            return 1;
-        }
-        break;
     }
 
     /* No valid transition found */
@@ -164,8 +151,11 @@ int ossl_statem_server_read_transition(SSL *s, int mt)
 {
     OSSL_STATEM *st = &s->statem;
 
-    if (s->method->version == TLS1_3_VERSION)
-        return ossl_statem_server13_read_transition(s, mt);
+    if (SSL_IS_TLS13(s)) {
+        if (!ossl_statem_server13_read_transition(s, mt))
+            goto err;
+        return 1;
+    }
 
     switch (st->hand_state) {
     default:
@@ -265,7 +255,7 @@ int ossl_statem_server_read_transition(SSL *s, int mt)
 
     case TLS_ST_SR_CHANGE:
 #ifndef OPENSSL_NO_NEXTPROTONEG
-        if (s->s3->next_proto_neg_seen) {
+        if (s->s3->npn_seen) {
             if (mt == SSL3_MT_NEXT_PROTO) {
                 st->hand_state = TLS_ST_SR_NEXT_PROTO;
                 return 1;
@@ -298,6 +288,7 @@ int ossl_statem_server_read_transition(SSL *s, int mt)
         break;
     }
 
+ err:
     /* No valid transition found */
     ssl3_send_alert(s, SSL3_AL_FATAL, SSL3_AD_UNEXPECTED_MESSAGE);
     SSLerr(SSL_F_OSSL_STATEM_SERVER_READ_TRANSITION, SSL_R_UNEXPECTED_MESSAGE);
@@ -362,7 +353,7 @@ static int send_certificate_request(SSL *s)
             * if SSL_VERIFY_CLIENT_ONCE is set, don't request cert
             * during re-negotiation:
             */
-           && ((s->session->peer == NULL) ||
+           && (s->s3->tmp.finish_md_len == 0 ||
                !(s->verify_mode & SSL_VERIFY_CLIENT_ONCE))
            /*
             * never request cert in anonymous ciphersuites (see
@@ -418,58 +409,35 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_SRVR_HELLO:
+        st->hand_state = TLS_ST_SW_ENCRYPTED_EXTENSIONS;
+        return WRITE_TRAN_CONTINUE;
+
+    case TLS_ST_SW_ENCRYPTED_EXTENSIONS:
         if (s->hit)
-            st->hand_state = s->tlsext_ticket_expected
-                                ? TLS_ST_SW_SESSION_TICKET : TLS_ST_SW_CHANGE;
+            st->hand_state = TLS_ST_SW_FINISHED;
+        else if (send_certificate_request(s))
+            st->hand_state = TLS_ST_SW_CERT_REQ;
         else
             st->hand_state = TLS_ST_SW_CERT;
 
         return WRITE_TRAN_CONTINUE;
 
-    case TLS_ST_SW_CERT:
-        if (s->tlsext_status_expected) {
-            st->hand_state = TLS_ST_SW_CERT_STATUS;
-            return WRITE_TRAN_CONTINUE;
-        }
-        /* Fall through */
-
-    case TLS_ST_SW_CERT_STATUS:
-        if (send_certificate_request(s)) {
-            st->hand_state = TLS_ST_SW_CERT_REQ;
-            return WRITE_TRAN_CONTINUE;
-        }
-        /* Fall through */
-
     case TLS_ST_SW_CERT_REQ:
-        st->hand_state = TLS_ST_SW_SRVR_DONE;
+        st->hand_state = TLS_ST_SW_CERT;
         return WRITE_TRAN_CONTINUE;
 
-    case TLS_ST_SW_SRVR_DONE:
-        return WRITE_TRAN_FINISHED;
-
-    case TLS_ST_SR_FINISHED:
-        if (s->hit) {
-            st->hand_state = TLS_ST_OK;
-            ossl_statem_set_in_init(s, 0);
-            return WRITE_TRAN_CONTINUE;
-        }
-
-        st->hand_state = s->tlsext_ticket_expected ? TLS_ST_SW_SESSION_TICKET
-                                                   : TLS_ST_SW_CHANGE;
-        return WRITE_TRAN_CONTINUE;
-
-    case TLS_ST_SW_SESSION_TICKET:
-        st->hand_state = TLS_ST_SW_CHANGE;
+    case TLS_ST_SW_CERT:
+        st->hand_state = TLS_ST_SW_CERT_VRFY;
         return WRITE_TRAN_CONTINUE;
 
-    case TLS_ST_SW_CHANGE:
+    case TLS_ST_SW_CERT_VRFY:
         st->hand_state = TLS_ST_SW_FINISHED;
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_FINISHED:
-        if (s->hit)
-            return WRITE_TRAN_FINISHED;
+        return WRITE_TRAN_FINISHED;
 
+    case TLS_ST_SR_FINISHED:
         st->hand_state = TLS_ST_OK;
         ossl_statem_set_in_init(s, 0);
         return WRITE_TRAN_CONTINUE;
@@ -489,7 +457,7 @@ WRITE_TRAN ossl_statem_server_write_transition(SSL *s)
      * to negotiate yet, so we don't take this branch until later
      */
 
-    if (s->method->version == TLS1_3_VERSION)
+    if (SSL_IS_TLS13(s))
         return ossl_statem_server13_write_transition(s);
 
     switch (st->hand_state) {
@@ -524,7 +492,7 @@ WRITE_TRAN ossl_statem_server_write_transition(SSL *s)
 
     case TLS_ST_SW_SRVR_HELLO:
         if (s->hit) {
-            if (s->tlsext_ticket_expected)
+            if (s->ext.ticket_expected)
                 st->hand_state = TLS_ST_SW_SESSION_TICKET;
             else
                 st->hand_state = TLS_ST_SW_CHANGE;
@@ -545,7 +513,7 @@ WRITE_TRAN ossl_statem_server_write_transition(SSL *s)
         return WRITE_TRAN_CONTINUE;
 
     case TLS_ST_SW_CERT:
-        if (s->tlsext_status_expected) {
+        if (s->ext.status_expected) {
             st->hand_state = TLS_ST_SW_CERT_STATUS;
             return WRITE_TRAN_CONTINUE;
         }
@@ -577,7 +545,7 @@ WRITE_TRAN ossl_statem_server_write_transition(SSL *s)
             st->hand_state = TLS_ST_OK;
             ossl_statem_set_in_init(s, 0);
             return WRITE_TRAN_CONTINUE;
-        } else if (s->tlsext_ticket_expected) {
+        } else if (s->ext.ticket_expected) {
             st->hand_state = TLS_ST_SW_SESSION_TICKET;
         } else {
             st->hand_state = TLS_ST_SW_CHANGE;
@@ -745,6 +713,20 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
                      sizeof(sctpauthkey), sctpauthkey);
         }
 #endif
+        /*
+         * 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
+         * alert, or an encrypted handshake message. We're going to need
+         * something clever in the record layer for this.
+         */
+        if (SSL_IS_TLS13(s)) {
+            if (!s->method->ssl3_enc->setup_key_block(s)
+                || !s->method->ssl3_enc->change_cipher_state(s,
+                        SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_SERVER_WRITE)
+                || !s->method->ssl3_enc->change_cipher_state(s,
+                        SSL3_CC_HANDSHAKE |SSL3_CHANGE_CIPHER_SERVER_READ))
+            return WORK_ERROR;
+        }
         break;
 
     case TLS_ST_SW_CHANGE:
@@ -787,6 +769,14 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
                      0, NULL);
         }
 #endif
+        if (SSL_IS_TLS13(s)) {
+            if (!s->method->ssl3_enc->generate_master_secret(s,
+                        s->session->master_key, s->handshake_secret, 0,
+                        &s->session->master_key_length)
+                || !s->method->ssl3_enc->change_cipher_state(s,
+                        SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_SERVER_WRITE))
+            return WORK_ERROR;
+        }
         break;
     }
 
@@ -840,6 +830,12 @@ int ossl_statem_server_construct_message(SSL *s, WPACKET *pkt,
         *mt = SSL3_MT_CERTIFICATE;
         break;
 
+    case TLS_ST_SW_CERT_VRFY:
+        *confunc = tls_construct_cert_verify;
+        *mt = SSL3_MT_CERTIFICATE_VERIFY;
+        break;
+
+
     case TLS_ST_SW_KEY_EXCH:
         *confunc = tls_construct_server_key_exchange;
         *mt = SSL3_MT_SERVER_KEY_EXCHANGE;
@@ -869,6 +865,11 @@ int ossl_statem_server_construct_message(SSL *s, WPACKET *pkt,
         *confunc = tls_construct_finished;
         *mt = SSL3_MT_FINISHED;
         break;
+
+    case TLS_ST_SW_ENCRYPTED_EXTENSIONS:
+        *confunc = tls_construct_encrypted_extensions;
+        *mt = SSL3_MT_ENCRYPTED_EXTENSIONS;
+        break;
     }
 
     return 1;
@@ -1006,7 +1007,7 @@ WORK_STATE ossl_statem_server_post_process_message(SSL *s, WORK_STATE wst)
 #endif
         return WORK_FINISHED_CONTINUE;
     }
-
+    return WORK_FINISHED_CONTINUE;
 }
 
 #ifndef OPENSSL_NO_SRP
@@ -1066,6 +1067,69 @@ int dtls_construct_hello_verify_request(SSL *s, WPACKET *pkt)
     return 1;
 }
 
+#ifndef OPENSSL_NO_EC
+/*-
+ * ssl_check_for_safari attempts to fingerprint Safari using OS X
+ * SecureTransport using the TLS extension block in |hello|.
+ * Safari, since 10.6, sends exactly these extensions, in this order:
+ *   SNI,
+ *   elliptic_curves
+ *   ec_point_formats
+ *
+ * We wish to fingerprint Safari because they broke ECDHE-ECDSA support in 10.8,
+ * but they advertise support. So enabling ECDHE-ECDSA ciphers breaks them.
+ * Sadly we cannot differentiate 10.6, 10.7 and 10.8.4 (which work), from
+ * 10.8..10.8.3 (which don't work).
+ */
+static void ssl_check_for_safari(SSL *s, const CLIENTHELLO_MSG *hello)
+{
+    static const unsigned char kSafariExtensionsBlock[] = {
+        0x00, 0x0a,             /* elliptic_curves extension */
+        0x00, 0x08,             /* 8 bytes */
+        0x00, 0x06,             /* 6 bytes of curve ids */
+        0x00, 0x17,             /* P-256 */
+        0x00, 0x18,             /* P-384 */
+        0x00, 0x19,             /* P-521 */
+
+        0x00, 0x0b,             /* ec_point_formats */
+        0x00, 0x02,             /* 2 bytes */
+        0x01,                   /* 1 point format */
+        0x00,                   /* uncompressed */
+        /* The following is only present in TLS 1.2 */
+        0x00, 0x0d,             /* signature_algorithms */
+        0x00, 0x0c,             /* 12 bytes */
+        0x00, 0x0a,             /* 10 bytes */
+        0x05, 0x01,             /* SHA-384/RSA */
+        0x04, 0x01,             /* SHA-256/RSA */
+        0x02, 0x01,             /* SHA-1/RSA */
+        0x04, 0x03,             /* SHA-256/ECDSA */
+        0x02, 0x03,             /* SHA-1/ECDSA */
+    };
+    /* Length of the common prefix (first two extensions). */
+    static const size_t kSafariCommonExtensionsLength = 18;
+    unsigned int type;
+    PACKET sni, tmppkt;
+    size_t ext_len;
+
+    tmppkt = hello->extensions;
+
+    if (!PACKET_forward(&tmppkt, 2)
+        || !PACKET_get_net_2(&tmppkt, &type)
+        || !PACKET_get_length_prefixed_2(&tmppkt, &sni)) {
+        return;
+    }
+
+    if (type != TLSEXT_TYPE_server_name)
+        return;
+
+    ext_len = TLS1_get_client_version(s) >= TLS1_2_VERSION ?
+        sizeof(kSafariExtensionsBlock) : kSafariCommonExtensionsLength;
+
+    s->s3->is_probably_safari = PACKET_equal(&tmppkt, kSafariExtensionsBlock,
+                                             ext_len);
+}
+#endif                          /* !OPENSSL_NO_EC */
+
 MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
 {
     int i, al = SSL_AD_INTERNAL_ERROR;
@@ -1253,8 +1317,8 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
 
     /* Preserve the raw extensions PACKET for later use */
     extensions = clienthello.extensions;
-    if (!tls_collect_extensions(&extensions, &clienthello.pre_proc_exts,
-                                  &clienthello.num_extensions, &al)) {
+    if (!tls_collect_extensions(s, &extensions, EXT_CLIENT_HELLO,
+                                &clienthello.pre_proc_exts, &al)) {
         /* SSLerr already been called */
         goto f_err;
     }
@@ -1339,9 +1403,9 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
     s->hit = 0;
 
     /* We need to do this before getting the session */
-    if (!tls_check_client_ems_support(s, &clienthello)) {
-        /* Only fails if the extension is malformed */
-        al = SSL_AD_DECODE_ERROR;
+    if (!tls_parse_extension(s, TLSEXT_IDX_extended_master_secret,
+                             EXT_CLIENT_HELLO,
+                             clienthello.pre_proc_exts, NULL, 0, &al)) {
         SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_CLIENTHELLO_TLSEXT);
         goto f_err;
     }
@@ -1438,14 +1502,20 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
         goto f_err;
     }
 
+#ifndef OPENSSL_NO_EC
+    if (s->options & SSL_OP_SAFARI_ECDHE_ECDSA_BUG)
+        ssl_check_for_safari(s, &clienthello);
+#endif                          /* !OPENSSL_NO_EC */
+
     /* TLS extensions */
-    if (!ssl_parse_clienthello_tlsext(s, &clienthello)) {
+    if (!tls_parse_all_extensions(s, EXT_CLIENT_HELLO,
+                                  clienthello.pre_proc_exts, NULL, 0, &al)) {
         SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_PARSE_TLSEXT);
-        goto err;
+        goto f_err;
     }
 
     /* Check we've got a key_share for TLSv1.3 */
-    if (s->version == TLS1_3_VERSION && s->s3->peer_tmp == NULL && !s->hit) {
+    if (SSL_IS_TLS13(s) && s->s3->peer_tmp == NULL && !s->hit) {
         /* No suitable share */
         /* TODO(TLS1.3): Send a HelloRetryRequest */
         al = SSL_AD_HANDSHAKE_FAILURE;
@@ -1467,7 +1537,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
         }
     }
 
-    if (!s->hit && s->version >= TLS1_VERSION && s->tls_session_secret_cb) {
+    if (!s->hit && s->version >= TLS1_VERSION && s->ext.session_secret_cb) {
         const SSL_CIPHER *pref_cipher = NULL;
         /*
          * s->session->master_key_length is a size_t, but this is an int for
@@ -1476,10 +1546,10 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
         int master_key_length;
 
         master_key_length = sizeof(s->session->master_key);
-        if (s->tls_session_secret_cb(s, s->session->master_key,
+        if (s->ext.session_secret_cb(s, s->session->master_key,
                                      &master_key_length, ciphers,
                                      &pref_cipher,
-                                     s->tls_session_secret_cb_arg)
+                                     s->ext.session_secret_cb_arg)
                 && master_key_length > 0) {
             s->session->master_key_length = master_key_length;
             s->hit = 1;
@@ -1625,6 +1695,55 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
     return MSG_PROCESS_ERROR;
 }
 
+/*
+ * Call the status request callback if needed. Upon success, returns 1.
+ * Upon failure, returns 0 and sets |*al| to the appropriate fatal alert.
+ */
+static int tls_handle_status_request(SSL *s, int *al)
+{
+    s->ext.status_expected = 0;
+
+    /*
+     * If status request then ask callback what to do. Note: this must be
+     * called after servername callbacks in case the certificate has changed,
+     * and must be called after the cipher has been chosen because this may
+     * influence which certificate is sent
+     */
+    if (s->ext.status_type != TLSEXT_STATUSTYPE_nothing && s->ctx != NULL
+            && s->ctx->ext.status_cb != NULL) {
+        int ret;
+        CERT_PKEY *certpkey = ssl_get_server_send_pkey(s);
+
+        /* If no certificate can't return certificate status */
+        if (certpkey != NULL) {
+            /*
+             * Set current certificate to one we will use so SSL_get_certificate
+             * et al can pick it up.
+             */
+            s->cert->key = certpkey;
+            ret = s->ctx->ext.status_cb(s, s->ctx->ext.status_arg);
+            switch (ret) {
+                /* We don't want to send a status request response */
+            case SSL_TLSEXT_ERR_NOACK:
+                s->ext.status_expected = 0;
+                break;
+                /* status request response should be sent */
+            case SSL_TLSEXT_ERR_OK:
+                if (s->ext.ocsp.resp)
+                    s->ext.status_expected = 1;
+                break;
+                /* something bad happened */
+            case SSL_TLSEXT_ERR_ALERT_FATAL:
+            default:
+                *al = SSL_AD_INTERNAL_ERROR;
+                return 0;
+            }
+        }
+    }
+
+    return 1;
+}
+
 WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
 {
     int al = SSL_AD_HANDSHAKE_FAILURE;
@@ -1658,11 +1777,13 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
             s->s3->tmp.new_cipher = cipher;
             /* check whether we should disable session resumption */
             if (s->not_resumable_session_cb != NULL)
-                s->session->not_resumable = s->not_resumable_session_cb(s,
-                                                                        ((cipher->algorithm_mkey & (SSL_kDHE | SSL_kECDHE)) != 0));
+                s->session->not_resumable =
+                    s->not_resumable_session_cb(s, ((cipher->algorithm_mkey
+                                                    & (SSL_kDHE | SSL_kECDHE))
+                                                   != 0));
             if (s->session->not_resumable)
                 /* do not send a session ticket */
-                s->tlsext_ticket_expected = 0;
+                s->ext.ticket_expected = 0;
         } else {
             /* Session-id reuse */
             s->s3->tmp.new_cipher = s->session->cipher;
@@ -1687,13 +1808,14 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
          * s->s3->tmp.new_cipher- the new cipher to use.
          */
 
-        /* Handles TLS extensions that we couldn't check earlier */
-        if (s->version >= SSL3_VERSION) {
-            if (!ssl_check_clienthello_tlsext_late(s, &al)) {
-                SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
-                       SSL_R_CLIENTHELLO_TLSEXT);
-                goto f_err;
-            }
+        /*
+         * Call status_request callback if needed. Has to be done after the
+         * certificate callbacks etc above.
+         */
+        if (!tls_handle_status_request(s, &al)) {
+            SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
+                   SSL_R_CLIENTHELLO_TLSEXT);
+            goto f_err;
         }
 
         wst = WORK_MORE_B;
@@ -1787,18 +1909,23 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
         compm = s->s3->tmp.new_compression->id;
 #endif
 
-    if (!WPACKET_sub_memcpy_u8(pkt, s->session->session_id, sl)
+    if ((!SSL_IS_TLS13(s)
+                && !WPACKET_sub_memcpy_u8(pkt, s->session->session_id, sl))
             || !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, pkt, &len)
-            || !WPACKET_put_bytes_u8(pkt, compm)
-            || !ssl_prepare_serverhello_tlsext(s)
-            || !ssl_add_serverhello_tlsext(s, pkt, &al)) {
+            || (!SSL_IS_TLS13(s)
+                && !WPACKET_put_bytes_u8(pkt, compm))
+            || !tls_construct_extensions(s, pkt,
+                                         SSL_IS_TLS13(s)
+                                            ? EXT_TLS1_3_SERVER_HELLO
+                                            : EXT_TLS1_2_SERVER_HELLO,
+                                         NULL, 0, &al)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
         goto err;
     }
 
     return 1;
  err:
-    ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+    ssl3_send_alert(s, SSL3_AL_FATAL, al);
     return 0;
 }
 
@@ -1829,6 +1956,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
     unsigned long type;
     const BIGNUM *r[4];
     EVP_MD_CTX *md_ctx = EVP_MD_CTX_new();
+    EVP_PKEY_CTX *pctx = NULL;
     size_t paramlen, paramoffset;
 
     if (!WPACKET_get_total_written(pkt, &paramoffset)) {
@@ -2085,7 +2213,8 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
          */
         if (md) {
             unsigned char *sigbytes1, *sigbytes2;
-            unsigned int siglen;
+            size_t siglen;
+            int ispss = 0;
 
             /* Get length of the parameters we have written above */
             if (!WPACKET_get_length(pkt, &paramlen)) {
@@ -2095,7 +2224,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
             }
             /* send signature algorithm */
             if (SSL_USE_SIGALGS(s)) {
-                if (!tls12_get_sigandhash(pkt, pkey, md)) {
+                if (!tls12_get_sigandhash(s, pkt, pkey, md, &ispss)) {
                     /* Should never happen */
                     SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
                            ERR_R_INTERNAL_ERROR);
@@ -2111,16 +2240,31 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
              * up front, and then properly allocate them in the WPACKET
              * afterwards.
              */
-            if (!WPACKET_sub_reserve_bytes_u16(pkt, EVP_PKEY_size(pkey),
-                                               &sigbytes1)
-                    || EVP_SignInit_ex(md_ctx, md, NULL) <= 0
-                    || EVP_SignUpdate(md_ctx, &(s->s3->client_random[0]),
-                                      SSL3_RANDOM_SIZE) <= 0
-                    || EVP_SignUpdate(md_ctx, &(s->s3->server_random[0]),
-                                      SSL3_RANDOM_SIZE) <= 0
-                    || EVP_SignUpdate(md_ctx, s->init_buf->data + paramoffset,
-                                      paramlen) <= 0
-                    || EVP_SignFinal(md_ctx, sigbytes1, &siglen, pkey) <= 0
+            siglen = EVP_PKEY_size(pkey);
+            if (!WPACKET_sub_reserve_bytes_u16(pkt, siglen, &sigbytes1)
+                    || EVP_DigestSignInit(md_ctx, &pctx, md, NULL, pkey) <= 0) {
+                SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
+                       ERR_R_INTERNAL_ERROR);
+                goto f_err;
+            }
+            if (ispss) {
+                if (EVP_PKEY_CTX_set_rsa_padding(pctx,
+                                                 RSA_PKCS1_PSS_PADDING) <= 0
+                           /* -1 here means set saltlen to the digest len */
+                        || EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, -1) <= 0) {
+                    SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
+                           ERR_R_EVP_LIB);
+                    goto f_err;
+                }
+            }
+            if (EVP_DigestSignUpdate(md_ctx, &(s->s3->client_random[0]),
+                                     SSL3_RANDOM_SIZE) <= 0
+                    || EVP_DigestSignUpdate(md_ctx, &(s->s3->server_random[0]),
+                                            SSL3_RANDOM_SIZE) <= 0
+                    || EVP_DigestSignUpdate(md_ctx,
+                                            s->init_buf->data + paramoffset,
+                                            paramlen) <= 0
+                    || EVP_DigestSignFinal(md_ctx, sigbytes1, &siglen) <= 0
                     || !WPACKET_sub_allocate_bytes_u16(pkt, siglen, &sigbytes2)
                     || sigbytes1 != sigbytes2) {
                 SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
@@ -2165,8 +2309,9 @@ int tls_construct_certificate_request(SSL *s, WPACKET *pkt)
     }
 
     if (SSL_USE_SIGALGS(s)) {
-        const unsigned char *psigs;
-        size_t nl = tls12_get_psigalgs(s, &psigs);
+        const unsigned int *psigs;
+        size_t nl = tls12_get_psigalgs(s, 1, &psigs);
+
         if (!WPACKET_start_sub_packet_u16(pkt)
                 || !tls12_copy_sigalgs(s, pkt, psigs, nl)
                 || !WPACKET_close(pkt)) {
@@ -2799,7 +2944,7 @@ WORK_STATE tls_post_process_client_key_exchange(SSL *s, WORK_STATE wst)
                                            sizeof(labelbuffer), NULL, 0,
                                            0) <= 0) {
                 ossl_statem_set_error(s);
-                return WORK_ERROR;;
+                return WORK_ERROR;
             }
 
             BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_SCTP_ADD_AUTH_KEY,
@@ -2857,163 +3002,6 @@ WORK_STATE tls_post_process_client_key_exchange(SSL *s, WORK_STATE wst)
     return WORK_FINISHED_CONTINUE;
 }
 
-MSG_PROCESS_RETURN tls_process_cert_verify(SSL *s, PACKET *pkt)
-{
-    EVP_PKEY *pkey = NULL;
-    const unsigned char *sig, *data;
-#ifndef OPENSSL_NO_GOST
-    unsigned char *gost_data = NULL;
-#endif
-    int al, ret = MSG_PROCESS_ERROR;
-    int type = 0, j;
-    unsigned int len;
-    X509 *peer;
-    const EVP_MD *md = NULL;
-    long hdatalen = 0;
-    void *hdata;
-
-    EVP_MD_CTX *mctx = EVP_MD_CTX_new();
-
-    if (mctx == NULL) {
-        SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, ERR_R_MALLOC_FAILURE);
-        al = SSL_AD_INTERNAL_ERROR;
-        goto f_err;
-    }
-
-    peer = s->session->peer;
-    pkey = X509_get0_pubkey(peer);
-    type = X509_certificate_type(peer, pkey);
-
-    if (!(type & EVP_PKT_SIGN)) {
-        SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY,
-               SSL_R_SIGNATURE_FOR_NON_SIGNING_CERTIFICATE);
-        al = SSL_AD_ILLEGAL_PARAMETER;
-        goto f_err;
-    }
-
-    /* Check for broken implementations of GOST ciphersuites */
-    /*
-     * If key is GOST and n is exactly 64, it is bare signature without
-     * length field (CryptoPro implementations at least till CSP 4.0)
-     */
-#ifndef OPENSSL_NO_GOST
-    if (PACKET_remaining(pkt) == 64
-        && EVP_PKEY_id(pkey) == NID_id_GostR3410_2001) {
-        len = 64;
-    } else
-#endif
-    {
-        if (SSL_USE_SIGALGS(s)) {
-            int rv;
-
-            if (!PACKET_get_bytes(pkt, &sig, 2)) {
-                al = SSL_AD_DECODE_ERROR;
-                goto f_err;
-            }
-            rv = tls12_check_peer_sigalg(&md, s, sig, pkey);
-            if (rv == -1) {
-                al = SSL_AD_INTERNAL_ERROR;
-                goto f_err;
-            } else if (rv == 0) {
-                al = SSL_AD_DECODE_ERROR;
-                goto f_err;
-            }
-#ifdef SSL_DEBUG
-            fprintf(stderr, "USING TLSv1.2 HASH %s\n", EVP_MD_name(md));
-#endif
-        } else {
-            /* Use default digest for this key type */
-            int idx = ssl_cert_type(NULL, pkey);
-            if (idx >= 0)
-                md = s->s3->tmp.md[idx];
-            if (md == NULL) {
-                al = SSL_AD_INTERNAL_ERROR;
-                goto f_err;
-            }
-        }
-
-        if (!PACKET_get_net_2(pkt, &len)) {
-            SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, SSL_R_LENGTH_MISMATCH);
-            al = SSL_AD_DECODE_ERROR;
-            goto f_err;
-        }
-    }
-    j = EVP_PKEY_size(pkey);
-    if (((int)len > j) || ((int)PACKET_remaining(pkt) > j)
-        || (PACKET_remaining(pkt) == 0)) {
-        SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, SSL_R_WRONG_SIGNATURE_SIZE);
-        al = SSL_AD_DECODE_ERROR;
-        goto f_err;
-    }
-    if (!PACKET_get_bytes(pkt, &data, len)) {
-        SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, SSL_R_LENGTH_MISMATCH);
-        al = SSL_AD_DECODE_ERROR;
-        goto f_err;
-    }
-
-    hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata);
-    if (hdatalen <= 0) {
-        SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, ERR_R_INTERNAL_ERROR);
-        al = SSL_AD_INTERNAL_ERROR;
-        goto f_err;
-    }
-
-#ifdef SSL_DEBUG
-    fprintf(stderr, "Using client verify alg %s\n", EVP_MD_name(md));
-#endif
-    if (!EVP_VerifyInit_ex(mctx, md, NULL)
-        || !EVP_VerifyUpdate(mctx, hdata, hdatalen)) {
-        SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, ERR_R_EVP_LIB);
-        al = SSL_AD_INTERNAL_ERROR;
-        goto f_err;
-    }
-#ifndef OPENSSL_NO_GOST
-    {
-        int pktype = EVP_PKEY_id(pkey);
-        if (pktype == NID_id_GostR3410_2001
-            || pktype == NID_id_GostR3410_2012_256
-            || pktype == NID_id_GostR3410_2012_512) {
-            if ((gost_data = OPENSSL_malloc(len)) == NULL) {
-                SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, ERR_R_MALLOC_FAILURE);
-                al = SSL_AD_INTERNAL_ERROR;
-                goto f_err;
-            }
-            BUF_reverse(gost_data, data, len);
-            data = gost_data;
-        }
-    }
-#endif
-
-    if (s->version == SSL3_VERSION
-        && !EVP_MD_CTX_ctrl(mctx, EVP_CTRL_SSL3_MASTER_SECRET,
-                            (int)s->session->master_key_length,
-                            s->session->master_key)) {
-        SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, ERR_R_EVP_LIB);
-        al = SSL_AD_INTERNAL_ERROR;
-        goto f_err;
-    }
-
-    if (EVP_VerifyFinal(mctx, data, len, pkey) <= 0) {
-        al = SSL_AD_DECRYPT_ERROR;
-        SSLerr(SSL_F_TLS_PROCESS_CERT_VERIFY, SSL_R_BAD_SIGNATURE);
-        goto f_err;
-    }
-
-    ret = MSG_PROCESS_CONTINUE_PROCESSING;
-    if (0) {
- f_err:
-        ssl3_send_alert(s, SSL3_AL_FATAL, al);
-        ossl_statem_set_error(s);
-    }
-    BIO_free(s->s3->handshake_buffer);
-    s->s3->handshake_buffer = NULL;
-    EVP_MD_CTX_free(mctx);
-#ifndef OPENSSL_NO_GOST
-    OPENSSL_free(gost_data);
-#endif
-    return ret;
-}
-
 MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
 {
     int i, al = SSL_AD_INTERNAL_ERROR, ret = MSG_PROCESS_ERROR;
@@ -3021,22 +3009,25 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
     unsigned long l, llen;
     const unsigned char *certstart, *certbytes;
     STACK_OF(X509) *sk = NULL;
-    PACKET spkt;
+    PACKET spkt, context;
+    size_t chainidx;
 
     if ((sk = sk_X509_new_null()) == NULL) {
         SSLerr(SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE, ERR_R_MALLOC_FAILURE);
         goto f_err;
     }
 
-    if (!PACKET_get_net_3(pkt, &llen)
-        || !PACKET_get_sub_packet(pkt, &spkt, llen)
-        || PACKET_remaining(pkt) != 0) {
+    /* TODO(TLS1.3): For now we ignore the context. We need to verify this */
+    if ((SSL_IS_TLS13(s) && !PACKET_get_length_prefixed_1(pkt, &context))
+            || !PACKET_get_net_3(pkt, &llen)
+            || !PACKET_get_sub_packet(pkt, &spkt, llen)
+            || PACKET_remaining(pkt) != 0) {
         al = SSL_AD_DECODE_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE, SSL_R_LENGTH_MISMATCH);
         goto f_err;
     }
 
-    while (PACKET_remaining(&spkt) > 0) {
+    for (chainidx = 0; PACKET_remaining(&spkt) > 0; chainidx++) {
         if (!PACKET_get_net_3(&spkt, &l)
             || !PACKET_get_bytes(&spkt, &certbytes, l)) {
             al = SSL_AD_DECODE_ERROR;
@@ -3057,6 +3048,26 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
                    SSL_R_CERT_LENGTH_MISMATCH);
             goto f_err;
         }
+
+        if (SSL_IS_TLS13(s)) {
+            RAW_EXTENSION *rawexts = NULL;
+            PACKET extensions;
+
+            if (!PACKET_get_length_prefixed_2(&spkt, &extensions)) {
+                al = SSL_AD_DECODE_ERROR;
+                SSLerr(SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE, SSL_R_BAD_LENGTH);
+                goto f_err;
+            }
+            if (!tls_collect_extensions(s, &extensions, EXT_TLS1_3_CERTIFICATE,
+                                        &rawexts, &al)
+                    || !tls_parse_all_extensions(s, EXT_TLS1_3_CERTIFICATE,
+                                                 rawexts, x, chainidx, &al)) {
+                OPENSSL_free(rawexts);
+                goto f_err;
+            }
+            OPENSSL_free(rawexts);
+        }
+
         if (!sk_X509_push(sk, x)) {
             SSLerr(SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE, ERR_R_MALLOC_FAILURE);
             goto f_err;
@@ -3129,6 +3140,17 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
      * certificate, while we do include it in statem_clnt.c
      */
     sk = NULL;
+
+    /* Save the current hash state for when we receive the CertificateVerify */
+    if (SSL_IS_TLS13(s)
+            && !ssl_handshake_hash(s, s->cert_verify_hash,
+                                   sizeof(s->cert_verify_hash),
+                                   &s->cert_verify_hash_len)) {
+        al = SSL_AD_INTERNAL_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE, ERR_R_INTERNAL_ERROR);
+        goto f_err;
+    }
+
     ret = MSG_PROCESS_CONTINUE_READING;
     goto done;
 
@@ -3144,6 +3166,7 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
 int tls_construct_server_certificate(SSL *s, WPACKET *pkt)
 {
     CERT_PKEY *cpk;
+    int al = SSL_AD_INTERNAL_ERROR;
 
     cpk = ssl_get_server_send_pkey(s);
     if (cpk == NULL) {
@@ -3151,8 +3174,14 @@ int tls_construct_server_certificate(SSL *s, WPACKET *pkt)
         return 0;
     }
 
-    if (!ssl3_output_cert_chain(s, pkt, cpk)) {
+    /*
+     * In TLSv1.3 the certificate chain is always preceded by a 0 length context
+     * for the server Certificate message
+     */
+    if ((SSL_IS_TLS13(s) && !WPACKET_put_bytes_u8(pkt, 0))
+            || !ssl3_output_cert_chain(s, pkt, cpk, &al)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_CERTIFICATE, ERR_R_INTERNAL_ERROR);
+        ssl3_send_alert(s, SSL3_AL_FATAL, al);
         return 0;
     }
 
@@ -3227,9 +3256,9 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt)
      * Initialize HMAC and cipher contexts. If callback present it does
      * all the work otherwise use generated values from parent ctx.
      */
-    if (tctx->tlsext_ticket_key_cb) {
+    if (tctx->ext.ticket_key_cb) {
         /* if 0 is returned, write an empty ticket */
-        int ret = tctx->tlsext_ticket_key_cb(s, key_name, iv, ctx,
+        int ret = tctx->ext.ticket_key_cb(s, key_name, iv, ctx,
                                              hctx, 1);
 
         if (ret == 0) {
@@ -3256,14 +3285,14 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt)
         if (RAND_bytes(iv, iv_len) <= 0)
             goto err;
         if (!EVP_EncryptInit_ex(ctx, cipher, NULL,
-                                tctx->tlsext_tick_aes_key, iv))
+                                tctx->ext.tick_aes_key, iv))
             goto err;
-        if (!HMAC_Init_ex(hctx, tctx->tlsext_tick_hmac_key,
-                          sizeof(tctx->tlsext_tick_hmac_key),
+        if (!HMAC_Init_ex(hctx, tctx->ext.tick_hmac_key,
+                          sizeof(tctx->ext.tick_hmac_key),
                           EVP_sha256(), NULL))
             goto err;
-        memcpy(key_name, tctx->tlsext_tick_key_name,
-               sizeof(tctx->tlsext_tick_key_name));
+        memcpy(key_name, tctx->ext.tick_key_name,
+               sizeof(tctx->ext.tick_key_name));
     }
 
     /*
@@ -3315,12 +3344,25 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt)
     return 0;
 }
 
+/*
+ * In TLSv1.3 this is called from the extensions code, otherwise it is used to
+ * create a separate message. Returns 1 on success or 0 on failure.
+ */
+int tls_construct_cert_status_body(SSL *s, WPACKET *pkt)
+{
+    if (!WPACKET_put_bytes_u8(pkt, s->ext.status_type)
+            || !WPACKET_sub_memcpy_u24(pkt, s->ext.ocsp.resp,
+                                       s->ext.ocsp.resp_len)) {
+        SSLerr(SSL_F_TLS_CONSTRUCT_CERT_STATUS_BODY, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    return 1;
+}
+
 int tls_construct_cert_status(SSL *s, WPACKET *pkt)
 {
-    if (!WPACKET_put_bytes_u8(pkt, s->tlsext_status_type)
-            || !WPACKET_sub_memcpy_u24(pkt, s->tlsext_ocsp_resp,
-                                       s->tlsext_ocsp_resplen)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CERT_STATUS, ERR_R_INTERNAL_ERROR);
+    if (!tls_construct_cert_status_body(s, pkt)) {
         ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
         return 0;
     }
@@ -3352,12 +3394,12 @@ MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt)
         goto err;
     }
 
-    if (!PACKET_memdup(&next_proto, &s->next_proto_negotiated, &next_proto_len)) {
-        s->next_proto_negotiated_len = 0;
+    if (!PACKET_memdup(&next_proto, &s->ext.npn, &next_proto_len)) {
+        s->ext.npn_len = 0;
         goto err;
     }
 
-    s->next_proto_negotiated_len = (unsigned char)next_proto_len;
+    s->ext.npn_len = (unsigned char)next_proto_len;
 
     return MSG_PROCESS_CONTINUE_READING;
  err:
@@ -3366,6 +3408,21 @@ MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt)
 }
 #endif
 
+static int tls_construct_encrypted_extensions(SSL *s, WPACKET *pkt)
+{
+    int al;
+
+    if (!tls_construct_extensions(s, pkt, EXT_TLS1_3_ENCRYPTED_EXTENSIONS,
+                                  NULL, 0, &al)) {
+        ssl3_send_alert(s, SSL3_AL_FATAL, al);
+        SSLerr(SSL_F_TLS_CONSTRUCT_ENCRYPTED_EXTENSIONS, ERR_R_INTERNAL_ERROR);
+        ssl3_send_alert(s, SSL3_AL_FATAL, al);
+        return 0;
+    }
+
+    return 1;
+}
+
 #define SSLV2_CIPHER_LEN    3
 
 STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,
@@ -3396,20 +3453,52 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,
         return NULL;
     }
 
-    if ((skp == NULL) || (*skp == NULL)) {
-        sk = sk_SSL_CIPHER_new_null(); /* change perhaps later */
-        if (sk == NULL) {
-            SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);
-            *al = SSL_AD_INTERNAL_ERROR;
-            return NULL;
-        }
-    } else {
-        sk = *skp;
-        sk_SSL_CIPHER_zero(sk);
+    sk = sk_SSL_CIPHER_new_null();
+    if (sk == NULL) {
+        SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);
+        *al = SSL_AD_INTERNAL_ERROR;
+        return NULL;
     }
 
-    if (!PACKET_memdup(cipher_suites, &s->s3->tmp.ciphers_raw,
-                       &s->s3->tmp.ciphers_rawlen)) {
+    if (sslv2format) {
+        size_t numciphers = PACKET_remaining(cipher_suites) / n;
+        PACKET sslv2ciphers = *cipher_suites;
+        unsigned int leadbyte;
+        unsigned char *raw;
+
+        /*
+         * We store the raw ciphers list in SSLv3+ format so we need to do some
+         * preprocessing to convert the list first. If there are any SSLv2 only
+         * ciphersuites with a non-zero leading byte then we are going to
+         * slightly over allocate because we won't store those. But that isn't a
+         * problem.
+         */
+        raw = OPENSSL_malloc(numciphers * TLS_CIPHER_LEN);
+        s->s3->tmp.ciphers_raw = raw;
+        if (raw == NULL) {
+            *al = SSL_AD_INTERNAL_ERROR;
+            goto err;
+        }
+        for (s->s3->tmp.ciphers_rawlen = 0;
+             PACKET_remaining(&sslv2ciphers) > 0;
+             raw += TLS_CIPHER_LEN) {
+            if (!PACKET_get_1(&sslv2ciphers, &leadbyte)
+                    || (leadbyte == 0
+                        && !PACKET_copy_bytes(&sslv2ciphers, raw,
+                                              TLS_CIPHER_LEN))
+                    || (leadbyte != 0
+                        && !PACKET_forward(&sslv2ciphers, TLS_CIPHER_LEN))) {
+                *al = SSL_AD_INTERNAL_ERROR;
+                OPENSSL_free(s->s3->tmp.ciphers_raw);
+                s->s3->tmp.ciphers_raw = NULL;
+                s->s3->tmp.ciphers_rawlen = 0;
+                goto err;
+            }
+            if (leadbyte == 0)
+                s->s3->tmp.ciphers_rawlen += TLS_CIPHER_LEN;
+        }
+    } else if (!PACKET_memdup(cipher_suites, &s->s3->tmp.ciphers_raw,
+                           &s->s3->tmp.ciphers_rawlen)) {
         *al = SSL_AD_INTERNAL_ERROR;
         goto err;
     }
@@ -3470,11 +3559,9 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,
         goto err;
     }
 
-    if (skp != NULL)
-        *skp = sk;
-    return (sk);
+    *skp = sk;
+    return sk;
  err:
-    if ((skp == NULL) || (*skp == NULL))
-        sk_SSL_CIPHER_free(sk);
+    sk_SSL_CIPHER_free(sk);
     return NULL;
 }