Convert key exchange to one shot call
[openssl.git] / ssl / statem / statem_srvr.c
index f6ecbf700654643553600d27243882cda95919de..6168b98ede6e2527121cf6010412360609888878 100644 (file)
@@ -1,5 +1,7 @@
 /*
  * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright (c) 2002, Oracle and/or its affiliates. All rights reserved
+ * Copyright 2005 Nokia. All rights reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -7,46 +9,6 @@
  * https://www.openssl.org/source/license.html
  */
 
-/* ====================================================================
- * Copyright 2002 Sun Microsystems, Inc. ALL RIGHTS RESERVED.
- *
- * Portions of the attached software ("Contribution") are developed by
- * SUN MICROSYSTEMS, INC., and are contributed to the OpenSSL project.
- *
- * The Contribution is licensed pursuant to the OpenSSL open source
- * license provided above.
- *
- * ECC cipher suite support in OpenSSL originally written by
- * Vipul Gupta and Sumit Gupta of Sun Microsystems Laboratories.
- *
- */
-/* ====================================================================
- * Copyright 2005 Nokia. All rights reserved.
- *
- * The portions of the attached software ("Contribution") is developed by
- * Nokia Corporation and is licensed pursuant to the OpenSSL open source
- * license.
- *
- * The Contribution, originally written by Mika Kousa and Pasi Eronen of
- * Nokia Corporation, consists of the "PSK" (Pre-Shared Key) ciphersuites
- * support (see RFC 4279) to OpenSSL.
- *
- * No patent licenses or other rights except those expressly stated in
- * the OpenSSL open source license shall be deemed granted or received
- * expressly, by implication, estoppel, or otherwise.
- *
- * No assurances are provided by Nokia that the Contribution does not
- * infringe the patent or other intellectual property rights of any third
- * party or that the license provides you with all the necessary rights
- * to make use of the Contribution.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" WITHOUT WARRANTY OF ANY KIND. IN
- * ADDITION TO THE DISCLAIMERS INCLUDED IN THE LICENSE, NOKIA
- * SPECIFICALLY DISCLAIMS ANY LIABILITY FOR CLAIMS BROUGHT BY YOU OR ANY
- * OTHER ENTITY BASED ON INFRINGEMENT OF INTELLECTUAL PROPERTY RIGHTS OR
- * OTHERWISE.
- */
-
 #include <stdio.h>
 #include "../ssl_locl.h"
 #include "statem_locl.h"
@@ -1175,6 +1137,7 @@ int dtls_construct_hello_verify_request(SSL *s, WPACKET *pkt)
  *   SNI,
  *   elliptic_curves
  *   ec_point_formats
+ *   signature_algorithms (for TLSv1.2 only)
  *
  * 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.
@@ -1245,6 +1208,10 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
     }
     /* Check if this is actually an unexpected renegotiation ClientHello */
     if (s->renegotiate == 0 && !SSL_IS_FIRST_HANDSHAKE(s)) {
+        if ((s->options & SSL_OP_NO_RENEGOTIATION)) {
+            ssl3_send_alert(s, SSL3_AL_WARNING, SSL_AD_NO_RENEGOTIATION);
+            goto err;
+        }
         s->renegotiate = 1;
         s->new_session = 1;
     }
@@ -1259,7 +1226,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
         unsigned int mt;
 
         if (!SSL_IS_FIRST_HANDSHAKE(s) || s->hello_retry_request) {
-            al = SSL_AD_HANDSHAKE_FAILURE;
+            al = SSL_AD_UNEXPECTED_MESSAGE;
             SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_UNEXPECTED_MESSAGE);
             goto f_err;
         }
@@ -1317,7 +1284,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
         }
 
         if (session_id_len > SSL_MAX_SSL_SESSION_ID_LENGTH) {
-            al = SSL_AD_DECODE_ERROR;
+            al = SSL_AD_ILLEGAL_PARAMETER;
             SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH);
             goto f_err;
         }
@@ -1375,8 +1342,8 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
             if (!PACKET_copy_all(&cookie, clienthello->dtls_cookie,
                                  DTLS1_COOKIE_LENGTH,
                                  &clienthello->dtls_cookie_len)) {
-                al = SSL_AD_DECODE_ERROR;
-                SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH);
+                al = SSL_AD_INTERNAL_ERROR;
+                SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
                 goto f_err;
             }
             /*
@@ -1406,7 +1373,8 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
         if (PACKET_remaining(pkt) == 0) {
             PACKET_null_init(&clienthello->extensions);
         } else {
-            if (!PACKET_get_length_prefixed_2(pkt, &clienthello->extensions)) {
+            if (!PACKET_get_length_prefixed_2(pkt, &clienthello->extensions)
+                    || PACKET_remaining(pkt) != 0) {
                 al = SSL_AD_DECODE_ERROR;
                 SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH);
                 goto f_err;
@@ -1417,8 +1385,8 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
     if (!PACKET_copy_all(&compression, clienthello->compressions,
                          MAX_COMPRESSIONS_SIZE,
                          &clienthello->compressions_len)) {
-        al = SSL_AD_DECODE_ERROR;
-        SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH);
+        al = SSL_AD_INTERNAL_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
         goto f_err;
     }
 
@@ -1438,7 +1406,8 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
  err:
     ossl_statem_set_error(s);
 
-    OPENSSL_free(clienthello->pre_proc_exts);
+    if (clienthello != NULL)
+        OPENSSL_free(clienthello->pre_proc_exts);
     OPENSSL_free(clienthello);
 
     return MSG_PROCESS_ERROR;
@@ -1559,6 +1528,69 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal)
 
     s->hit = 0;
 
+    if (!ssl_cache_cipherlist(s, &clienthello->ciphersuites,
+                              clienthello->isv2, &al) ||
+        !bytes_to_cipher_list(s, &clienthello->ciphersuites, &ciphers, &scsvs,
+                             clienthello->isv2, &al)) {
+        goto err;
+    }
+
+    s->s3->send_connection_binding = 0;
+    /* Check what signalling cipher-suite values were received. */
+    if (scsvs != NULL) {
+        for(i = 0; i < sk_SSL_CIPHER_num(scsvs); i++) {
+            c = sk_SSL_CIPHER_value(scsvs, i);
+            if (SSL_CIPHER_get_id(c) == SSL3_CK_SCSV) {
+                if (s->renegotiate) {
+                    /* SCSV is fatal if renegotiating */
+                    SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
+                           SSL_R_SCSV_RECEIVED_WHEN_RENEGOTIATING);
+                    al = SSL_AD_HANDSHAKE_FAILURE;
+                    goto err;
+                }
+                s->s3->send_connection_binding = 1;
+            } else if (SSL_CIPHER_get_id(c) == SSL3_CK_FALLBACK_SCSV &&
+                       !ssl_check_version_downgrade(s)) {
+                /*
+                 * This SCSV indicates that the client previously tried
+                 * a higher version.  We should fail if the current version
+                 * is an unexpected downgrade, as that indicates that the first
+                 * connection may have been tampered with in order to trigger
+                 * an insecure downgrade.
+                 */
+                SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
+                       SSL_R_INAPPROPRIATE_FALLBACK);
+                al = SSL_AD_INAPPROPRIATE_FALLBACK;
+                goto err;
+            }
+        }
+    }
+
+    /* For TLSv1.3 we must select the ciphersuite *before* session resumption */
+    if (SSL_IS_TLS13(s)) {
+        const SSL_CIPHER *cipher =
+            ssl3_choose_cipher(s, ciphers, SSL_get_ciphers(s));
+
+        if (cipher == NULL) {
+            SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
+                   SSL_R_NO_SHARED_CIPHER);
+            al = SSL_AD_HANDSHAKE_FAILURE;
+            goto err;
+        }
+        if (s->hello_retry_request
+                && (s->s3->tmp.new_cipher == NULL
+                    || s->s3->tmp.new_cipher->id != cipher->id)) {
+            /*
+             * A previous HRR picked a different ciphersuite to the one we
+             * just selected. Something must have changed.
+             */
+            al = SSL_AD_ILLEGAL_PARAMETER;
+            SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, SSL_R_BAD_CIPHER);
+            goto err;
+        }
+        s->s3->tmp.new_cipher = cipher;
+    }
+
     /* We need to do this before getting the session */
     if (!tls_parse_extension(s, TLSEXT_IDX_extended_master_secret,
                              SSL_EXT_CLIENT_HELLO,
@@ -1602,46 +1634,11 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal)
         }
     }
 
-    if (!ssl_cache_cipherlist(s, &clienthello->ciphersuites,
-                              clienthello->isv2, &al) ||
-        !bytes_to_cipher_list(s, &clienthello->ciphersuites, &ciphers, &scsvs,
-                             clienthello->isv2, &al)) {
-        goto err;
-    }
-
-    s->s3->send_connection_binding = 0;
-    /* Check what signalling cipher-suite values were received. */
-    if (scsvs != NULL) {
-        for(i = 0; i < sk_SSL_CIPHER_num(scsvs); i++) {
-            c = sk_SSL_CIPHER_value(scsvs, i);
-            if (SSL_CIPHER_get_id(c) == SSL3_CK_SCSV) {
-                if (s->renegotiate) {
-                    /* SCSV is fatal if renegotiating */
-                    SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
-                           SSL_R_SCSV_RECEIVED_WHEN_RENEGOTIATING);
-                    al = SSL_AD_HANDSHAKE_FAILURE;
-                    goto err;
-                }
-                s->s3->send_connection_binding = 1;
-            } else if (SSL_CIPHER_get_id(c) == SSL3_CK_FALLBACK_SCSV &&
-                       !ssl_check_version_downgrade(s)) {
-                /*
-                 * This SCSV indicates that the client previously tried
-                 * a higher version.  We should fail if the current version
-                 * is an unexpected downgrade, as that indicates that the first
-                 * connection may have been tampered with in order to trigger
-                 * an insecure downgrade.
-                 */
-                SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
-                       SSL_R_INAPPROPRIATE_FALLBACK);
-                al = SSL_AD_INAPPROPRIATE_FALLBACK;
-                goto err;
-            }
-        }
-    }
-
-    /* If it is a hit, check that the cipher is in the list */
-    if (s->hit) {
+    /*
+     * If it is a hit, check that the cipher is in the list. In TLSv1.3 we check
+     * ciphersuite compatibility with the session as part of resumption.
+     */
+    if (!SSL_IS_TLS13(s) && s->hit) {
         j = 0;
         id = s->session->cipher->id;
 
@@ -1709,7 +1706,11 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal)
         }
     }
 
-    if (!s->hit && s->version >= TLS1_VERSION && s->ext.session_secret_cb) {
+    if (!s->hit
+            && s->version >= TLS1_VERSION
+            && !SSL_IS_TLS13(s)
+            && !SSL_IS_DTLS(s)
+            && 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
@@ -1754,9 +1755,22 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal)
      * algorithms from the client, starting at q.
      */
     s->s3->tmp.new_compression = NULL;
+    if (SSL_IS_TLS13(s)) {
+        /*
+         * We already checked above that the NULL compression method appears in
+         * the list. Now we check there aren't any others (which is illegal in
+         * a TLSv1.3 ClientHello.
+         */
+        if (clienthello->compressions_len != 1) {
+            al = SSL_AD_ILLEGAL_PARAMETER;
+            SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO,
+                   SSL_R_INVALID_COMPRESSION_ALGORITHM);
+            goto err;
+        }
+    }
 #ifndef OPENSSL_NO_COMP
     /* This only happens if we have a cache hit */
-    if (s->session->compress_meth != 0 && !SSL_IS_TLS13(s)) {
+    else if (s->session->compress_meth != 0) {
         int m, comp_id = s->session->compress_meth;
         unsigned int k;
         /* Perform sanity checks on resumed compression algorithm */
@@ -1792,8 +1806,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal)
         }
     } else if (s->hit) {
         comp = NULL;
-    } else if (ssl_allow_compression(s) && s->ctx->comp_methods
-                   && !SSL_IS_TLS13(s)) {
+    } else if (ssl_allow_compression(s) && s->ctx->comp_methods) {
         /* See if we have a match */
         int m, nn, v, done = 0;
         unsigned int o;
@@ -1831,7 +1844,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal)
      * Given s->session->ciphers and SSL_get_ciphers, we must pick a cipher
      */
 
-    if (!s->hit || s->hello_retry_request) {
+    if (!s->hit || SSL_IS_TLS13(s)) {
         sk_SSL_CIPHER_free(s->session->ciphers);
         s->session->ciphers = ciphers;
         if (ciphers == NULL) {
@@ -1937,9 +1950,9 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
         wst = WORK_MORE_B;
     }
     if (wst == WORK_MORE_B) {
-        if (!s->hit || s->hello_retry_request) {
+        if (!s->hit || SSL_IS_TLS13(s)) {
             /* Let cert callback update server certificates if required */
-            if (s->cert->cert_cb) {
+            if (!s->hit && s->cert->cert_cb != NULL) {
                 int rv = s->cert->cert_cb(s, s->cert->cert_cb_arg);
                 if (rv == 0) {
                     al = SSL_AD_INTERNAL_ERROR;
@@ -1953,34 +1966,28 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
                 }
                 s->rwstate = SSL_NOTHING;
             }
-            cipher =
-                ssl3_choose_cipher(s, s->session->ciphers, SSL_get_ciphers(s));
 
-            if (cipher == NULL) {
-                SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
-                       SSL_R_NO_SHARED_CIPHER);
-                goto f_err;
-            }
-            if (SSL_IS_TLS13(s) && s->s3->tmp.new_cipher != NULL
-                    && s->s3->tmp.new_cipher->id != cipher->id) {
-                /*
-                 * A previous HRR picked a different ciphersuite to the one we
-                 * just selected. Something must have changed.
-                 */
-                al = SSL_AD_ILLEGAL_PARAMETER;
-                SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO, SSL_R_BAD_CIPHER);
-                goto f_err;
+            /* In TLSv1.3 we selected the ciphersuite before resumption */
+            if (!SSL_IS_TLS13(s)) {
+                cipher =
+                    ssl3_choose_cipher(s, s->session->ciphers, SSL_get_ciphers(s));
+
+                if (cipher == NULL) {
+                    SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
+                           SSL_R_NO_SHARED_CIPHER);
+                    goto f_err;
+                }
+                s->s3->tmp.new_cipher = cipher;
             }
-            s->s3->tmp.new_cipher = cipher;
             if (!s->hit) {
                 if (!tls_choose_sigalg(s, &al))
                     goto f_err;
                 /* 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->not_resumable_session_cb(s,
+                            ((s->s3->tmp.new_cipher->algorithm_mkey
+                              & (SSL_kDHE | SSL_kECDHE)) != 0));
                 if (s->session->not_resumable)
                     /* do not send a session ticket */
                     s->ext.ticket_expected = 0;
@@ -2207,7 +2214,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
             pkdhp = pkdh;
         }
         if (pkdhp == NULL) {
-            al = SSL_AD_HANDSHAKE_FAILURE;
+            al = SSL_AD_INTERNAL_ERROR;
             SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
                    SSL_R_MISSING_TMP_DH_KEY);
             goto f_err;
@@ -2300,7 +2307,7 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
     } else
 #endif
     {
-        al = SSL_AD_HANDSHAKE_FAILURE;
+        al = SSL_AD_INTERNAL_ERROR;
         SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
                SSL_R_UNKNOWN_KEY_EXCHANGE_TYPE);
         goto f_err;
@@ -2403,9 +2410,10 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
     /* not anonymous */
     if (lu != NULL) {
         EVP_PKEY *pkey = s->s3->tmp.cert->privatekey;
-        const EVP_MD *md = ssl_md(lu->hash_idx);
-        unsigned char *sigbytes1, *sigbytes2;
-        size_t siglen;
+        const EVP_MD *md;
+        unsigned char *sigbytes1, *sigbytes2, *tbs;
+        size_t siglen, tbslen;
+        int rv;
 
         if (pkey == NULL || md == NULL) {
             /* Should never happen */
@@ -2449,15 +2457,17 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt)
                 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)
+        tbslen = construct_key_exchange_tbs(s, &tbs,
+                                            s->init_buf->data + paramoffset,
+                                            paramlen);
+        if (tbslen == 0) {
+            SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
+                   ERR_R_MALLOC_FAILURE);
+            goto f_err;
+        }
+        rv = EVP_DigestSign(md_ctx, sigbytes1, &siglen, tbs, tbslen);
+        OPENSSL_free(tbs);
+        if (rv <= 0 || !WPACKET_sub_allocate_bytes_u16(pkt, siglen, &sigbytes2)
             || sigbytes1 != sigbytes2) {
             SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE,
                    ERR_R_INTERNAL_ERROR);
@@ -2617,7 +2627,7 @@ static int tls_process_cke_rsa(SSL *s, PACKET *pkt, int *al)
 
     rsa = EVP_PKEY_get0_RSA(s->cert->pkeys[SSL_PKEY_RSA].privatekey);
     if (rsa == NULL) {
-        *al = SSL_AD_HANDSHAKE_FAILURE;
+        *al = SSL_AD_INTERNAL_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_CKE_RSA, SSL_R_MISSING_RSA_CERTIFICATE);
         return 0;
     }
@@ -2780,20 +2790,20 @@ static int tls_process_cke_dhe(SSL *s, PACKET *pkt, int *al)
     int ret = 0;
 
     if (!PACKET_get_net_2(pkt, &i) || PACKET_remaining(pkt) != i) {
-        *al = SSL_AD_HANDSHAKE_FAILURE;
+        *al = SSL_AD_DECODE_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_CKE_DHE,
                SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG);
         goto err;
     }
     skey = s->s3->tmp.pkey;
     if (skey == NULL) {
-        *al = SSL_AD_HANDSHAKE_FAILURE;
+        *al = SSL_AD_INTERNAL_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, SSL_R_MISSING_TMP_DH_KEY);
         goto err;
     }
 
     if (PACKET_remaining(pkt) == 0L) {
-        *al = SSL_AD_HANDSHAKE_FAILURE;
+        *al = SSL_AD_DECODE_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_CKE_DHE, SSL_R_MISSING_TMP_DH_KEY);
         goto err;
     }
@@ -2872,7 +2882,7 @@ static int tls_process_cke_ecdhe(SSL *s, PACKET *pkt, int *al)
             goto err;
         }
         if (EVP_PKEY_set1_tls_encodedpoint(ckey, data, i) == 0) {
-            *al = SSL_AD_HANDSHAKE_FAILURE;
+            *al = SSL_AD_ILLEGAL_PARAMETER;
             SSLerr(SSL_F_TLS_PROCESS_CKE_ECDHE, ERR_R_EC_LIB);
             goto err;
         }
@@ -2912,6 +2922,7 @@ static int tls_process_cke_srp(SSL *s, PACKET *pkt, int *al)
         return 0;
     }
     if ((s->srp_ctx.A = BN_bin2bn(data, i, NULL)) == NULL) {
+        *al = SSL_AD_INTERNAL_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_CKE_SRP, ERR_R_BN_LIB);
         return 0;
     }
@@ -3056,7 +3067,7 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt)
     if (alg_k & SSL_kPSK) {
         /* Identity extracted earlier: should be nothing left */
         if (PACKET_remaining(pkt) != 0) {
-            al = SSL_AD_HANDSHAKE_FAILURE;
+            al = SSL_AD_DECODE_ERROR;
             SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE,
                    SSL_R_LENGTH_MISMATCH);
             goto err;
@@ -3083,7 +3094,7 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt)
         if (!tls_process_cke_gost(s, pkt, &al))
             goto err;
     } else {
-        al = SSL_AD_HANDSHAKE_FAILURE;
+        al = SSL_AD_INTERNAL_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE,
                SSL_R_UNKNOWN_CIPHER_TYPE);
         goto err;
@@ -3218,9 +3229,9 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
             if (!tls_collect_extensions(s, &extensions,
                                         SSL_EXT_TLS1_3_CERTIFICATE, &rawexts,
                                         &al, NULL, chainidx == 0)
-                    || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE,
-                                                 rawexts, x, chainidx, &al,
-                                                 !PACKET_remaining(&spkt))) {
+                || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE,
+                                             rawexts, x, chainidx, &al,
+                                             PACKET_remaining(&spkt) == 0)) {
                 OPENSSL_free(rawexts);
                 goto f_err;
             }
@@ -3570,6 +3581,7 @@ MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt)
 {
     PACKET next_proto, padding;
     size_t next_proto_len;
+    int al = SSL_AD_INTERNAL_ERROR;
 
     /*-
      * The payload looks like:
@@ -3581,6 +3593,7 @@ MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt)
     if (!PACKET_get_length_prefixed_1(pkt, &next_proto)
         || !PACKET_get_length_prefixed_1(pkt, &padding)
         || PACKET_remaining(pkt) > 0) {
+        al = SSL_AD_DECODE_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_NEXT_PROTO, SSL_R_LENGTH_MISMATCH);
         goto err;
     }
@@ -3594,6 +3607,7 @@ MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt)
 
     return MSG_PROCESS_CONTINUE_READING;
  err:
+    ssl3_send_alert(s, SSL3_AL_FATAL, al);
     ossl_statem_set_error(s);
     return MSG_PROCESS_ERROR;
 }
@@ -3607,7 +3621,6 @@ static int tls_construct_encrypted_extensions(SSL *s, WPACKET *pkt)
                                   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;
     }