Convert the state machine code to use SSLfatal()

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

9 files changed, 1897 insertions, 1630 deletions

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index a72859c873..062fc0438b 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -12,14 +12,11 @@
 #include "../ssl_locl.h"
 #include "statem_locl.h"
 
-static int final_renegotiate(SSL *s, unsigned int context, int sent,
-                                     int *al);
+static int final_renegotiate(SSL *s, unsigned int context, int sent);
 static int init_server_name(SSL *s, unsigned int context);
-static int final_server_name(SSL *s, unsigned int context, int sent,
-                                     int *al);
+static int final_server_name(SSL *s, unsigned int context, int sent);
 #ifndef OPENSSL_NO_EC
-static int final_ec_pt_formats(SSL *s, unsigned int context, int sent,
-                                       int *al);
+static int final_ec_pt_formats(SSL *s, unsigned int context, int sent);
 #endif
 static int init_session_ticket(SSL *s, unsigned int context);
 #ifndef OPENSSL_NO_OCSP
@@ -29,33 +26,32 @@ static int init_status_request(SSL *s, unsigned int context);
 static int init_npn(SSL *s, unsigned int context);
 #endif
 static int init_alpn(SSL *s, unsigned int context);
-static int final_alpn(SSL *s, unsigned int context, int sent, int *al);
+static int final_alpn(SSL *s, unsigned int context, int sent);
 static int init_sig_algs(SSL *s, unsigned int context);
 static int init_certificate_authorities(SSL *s, unsigned int context);
 static EXT_RETURN tls_construct_certificate_authorities(SSL *s, WPACKET *pkt,
                                                         unsigned int context,
                                                         X509 *x,
-                                                        size_t chainidx,
-                                                        int *al);
+                                                        size_t chainidx);
 static int tls_parse_certificate_authorities(SSL *s, PACKET *pkt,
                                              unsigned int context, X509 *x,
-                                             size_t chainidx, int *al);
+                                             size_t chainidx);
 #ifndef OPENSSL_NO_SRP
 static int init_srp(SSL *s, unsigned int context);
 #endif
 static int init_etm(SSL *s, unsigned int context);
 static int init_ems(SSL *s, unsigned int context);
-static int final_ems(SSL *s, unsigned int context, int sent, int *al);
+static int final_ems(SSL *s, unsigned int context, int sent);
 static int init_psk_kex_modes(SSL *s, unsigned int context);
 #ifndef OPENSSL_NO_EC
-static int final_key_share(SSL *s, unsigned int context, int sent, int *al);
+static int final_key_share(SSL *s, unsigned int context, int sent);
 #endif
 #ifndef OPENSSL_NO_SRTP
 static int init_srtp(SSL *s, unsigned int context);
 #endif
-static int final_sig_algs(SSL *s, unsigned int context, int sent, int *al);
-static int final_early_data(SSL *s, unsigned int context, int sent, int *al);
-static int final_maxfragmentlen(SSL *s, unsigned int context, int sent, int *al);
+static int final_sig_algs(SSL *s, unsigned int context, int sent);
+static int final_early_data(SSL *s, unsigned int context, int sent);
+static int final_maxfragmentlen(SSL *s, unsigned int context, int sent);
 
 /* Structure to define a built-in extension */
 typedef struct extensions_definition_st {
@@ -73,22 +69,22 @@ typedef struct extensions_definition_st {
     int (*init)(SSL *s, unsigned int context);
     /* Parse extension sent from client to server */
     int (*parse_ctos)(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
-                      size_t chainidx, int *al);
+                      size_t chainidx);
     /* Parse extension send from server to client */
     int (*parse_stoc)(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
-                      size_t chainidx, int *al);
+                      size_t chainidx);
     /* Construct extension sent from server to client */
     EXT_RETURN (*construct_stoc)(SSL *s, WPACKET *pkt, unsigned int context,
-                                 X509 *x, size_t chainidx, int *al);
+                                 X509 *x, size_t chainidx);
     /* Construct extension sent from client to server */
     EXT_RETURN (*construct_ctos)(SSL *s, WPACKET *pkt, unsigned int context,
-                                 X509 *x, size_t chainidx, int *al);
+                                 X509 *x, size_t chainidx);
     /*
      * Finalise extension after parsing. Always called where an extensions was
      * initialised even if the extension was not present. |sent| is set to 1 if
      * the extension was seen, or 0 otherwise.
      */
-    int (*final)(SSL *s, unsigned int context, int sent, int *al);
+    int (*final)(SSL *s, unsigned int context, int sent);
 } EXTENSION_DEFINITION;
 
 /*
@@ -458,8 +454,7 @@ int extension_is_relevant(SSL *s, unsigned int extctx, unsigned int thisctx)
  * extensions that we know about. We ignore others.
  */
 int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
-                           RAW_EXTENSION **res, int *al, size_t *len,
-                           int init)
+                           RAW_EXTENSION **res, size_t *len, int init)
 {
     PACKET extensions = *packet;
     size_t i = 0;
@@ -480,8 +475,8 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
     num_exts = OSSL_NELEM(ext_defs) + (exts != NULL ? exts->meths_count : 0);
     raw_extensions = OPENSSL_zalloc(num_exts * sizeof(*raw_extensions));
     if (raw_extensions == NULL) {
-        *al = SSL_AD_INTERNAL_ERROR;
-        SSLerr(SSL_F_TLS_COLLECT_EXTENSIONS, ERR_R_MALLOC_FAILURE);
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_COLLECT_EXTENSIONS,
+                 ERR_R_MALLOC_FAILURE);
         return 0;
     }
 
@@ -493,8 +488,8 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
 
         if (!PACKET_get_net_2(&extensions, &type) ||
             !PACKET_get_length_prefixed_2(&extensions, &extension)) {
-            SSLerr(SSL_F_TLS_COLLECT_EXTENSIONS, SSL_R_BAD_EXTENSION);
-            *al = SSL_AD_DECODE_ERROR;
+            SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_COLLECT_EXTENSIONS,
+                     SSL_R_BAD_EXTENSION);
             goto err;
         }
         /*
@@ -507,8 +502,8 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
                 || (type == TLSEXT_TYPE_psk
                     && (context & SSL_EXT_CLIENT_HELLO) != 0
                     && PACKET_remaining(&extensions) != 0)) {
-            SSLerr(SSL_F_TLS_COLLECT_EXTENSIONS, SSL_R_BAD_EXTENSION);
-            *al = SSL_AD_ILLEGAL_PARAMETER;
+            SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_COLLECT_EXTENSIONS,
+                     SSL_R_BAD_EXTENSION);
             goto err;
         }
         idx = thisex - raw_extensions;
@@ -534,8 +529,8 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
                 && type != TLSEXT_TYPE_renegotiate
                 && type != TLSEXT_TYPE_signed_certificate_timestamp
                 && (s->ext.extflags[idx] & SSL_EXT_FLAG_SENT) == 0) {
-            SSLerr(SSL_F_TLS_COLLECT_EXTENSIONS, SSL_R_UNSOLICITED_EXTENSION);
-            *al = SSL_AD_UNSUPPORTED_EXTENSION;
+            SSLfatal(s, SSL_AD_UNSUPPORTED_EXTENSION,
+                     SSL_F_TLS_COLLECT_EXTENSIONS, SSL_R_UNSOLICITED_EXTENSION);
             goto err;
         }
         if (thisex != NULL) {
@@ -561,7 +556,7 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
             if (thisexd->init != NULL && (thisexd->context & context) != 0
                 && extension_is_relevant(s, thisexd->context, context)
                 && !thisexd->init(s, context)) {
-                *al = SSL_AD_INTERNAL_ERROR;
+                /* SSLfatal() already called */
                 goto err;
             }
         }
@@ -589,11 +584,11 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,
  * this counted as success.
  */
 int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context,
-                        RAW_EXTENSION *exts, X509 *x, size_t chainidx, int *al)
+                        RAW_EXTENSION *exts, X509 *x, size_t chainidx)
 {
     RAW_EXTENSION *currext = &exts[idx];
     int (*parser)(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
-                  size_t chainidx, int *al) = NULL;
+                  size_t chainidx) = NULL;
 
     /* Skip if the extension is not present */
     if (!currext->present)
@@ -616,7 +611,7 @@ int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context,
         parser = s->server ? extdef->parse_ctos : extdef->parse_stoc;
 
         if (parser != NULL)
-            return parser(s, &currext->data, context, x, chainidx, al);
+            return parser(s, &currext->data, context, x, chainidx);
 
         /*
          * If the parser is NULL we fall through to the custom extension
@@ -625,13 +620,10 @@ int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context,
     }
 
     /* Parse custom extensions */
-    if (custom_ext_parse(s, context, currext->type,
-                         PACKET_data(&currext->data),
-                         PACKET_remaining(&currext->data),
-                         x, chainidx, al) <= 0)
-        return 0;
-
-    return 1;
+    return custom_ext_parse(s, context, currext->type,
+                            PACKET_data(&currext->data),
+                            PACKET_remaining(&currext->data),
+                            x, chainidx);
 }
 
 /*
@@ -643,7 +635,7 @@ int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context,
  * failure, |*al| is populated with a suitable alert code.
  */
 int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, X509 *x,
-                             size_t chainidx, int *al, int fin)
+                             size_t chainidx, int fin)
 {
     size_t i, numexts = OSSL_NELEM(ext_defs);
     const EXTENSION_DEFINITION *thisexd;
@@ -653,8 +645,10 @@ int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, X509 *x,
 
     /* Parse each extension in turn */
     for (i = 0; i < numexts; i++) {
-        if (!tls_parse_extension(s, i, context, exts, x, chainidx, al))
+        if (!tls_parse_extension(s, i, context, exts, x, chainidx)) {
+            /* SSLfatal() already called */
             return 0;
+        }
     }
 
     if (fin) {
@@ -665,8 +659,10 @@ int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, X509 *x,
         for (i = 0, thisexd = ext_defs; i < OSSL_NELEM(ext_defs);
              i++, thisexd++) {
             if (thisexd->final != NULL && (thisexd->context & context) != 0
-                && !thisexd->final(s, context, exts[i].present, al))
+                && !thisexd->final(s, context, exts[i].present)) {
+                /* SSLfatal() already called */
                 return 0;
+            }
         }
     }
 
@@ -702,23 +698,16 @@ int should_add_extension(SSL *s, unsigned int extctx, unsigned int thisctx,
  * them to |pkt|. If this is an extension for a Certificate in a Certificate
  * message, then |x| will be set to the Certificate we are handling, and
  * |chainidx| will indicate the position in the chainidx we are processing (with
- * 0 being the first in the chain). Returns 1 on success or 0 on failure. If a
- * failure occurs then |al| is populated with a suitable alert code. On a
+ * 0 being the first in the chain). Returns 1 on success or 0 on failure. On a
  * failure construction stops at the first extension to fail to construct.
  */
 int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
-                             X509 *x, size_t chainidx, int *al)
+                             X509 *x, size_t chainidx)
 {
     size_t i;
-    int min_version, max_version = 0, reason, tmpal;
+    int min_version, max_version = 0, reason;
     const EXTENSION_DEFINITION *thisexd;
 
-    /*
-     * Normally if something goes wrong during construction it's an internal
-     * error. We can always override this later.
-     */
-    tmpal = SSL_AD_INTERNAL_ERROR;
-
     if (!WPACKET_start_sub_packet_u16(pkt)
                /*
                 * If extensions are of zero length then we don't even add the
@@ -729,15 +718,17 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
                  (SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO)) != 0
                 && !WPACKET_set_flags(pkt,
                                      WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH))) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_EXTENSIONS, ERR_R_INTERNAL_ERROR);
-        goto err;
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_EXTENSIONS,
+                 ERR_R_INTERNAL_ERROR);
+        return 0;
     }
 
     if ((context & SSL_EXT_CLIENT_HELLO) != 0) {
         reason = ssl_get_min_max_version(s, &min_version, &max_version);
         if (reason != 0) {
-            SSLerr(SSL_F_TLS_CONSTRUCT_EXTENSIONS, reason);
-            goto err;
+            SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_EXTENSIONS,
+                     reason);
+            return 0;
         }
     }
 
@@ -746,14 +737,14 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
         /* On the server side with initialise during ClientHello parsing */
         custom_ext_init(&s->cert->custext);
     }
-    if (!custom_ext_add(s, context, pkt, x, chainidx, max_version, &tmpal)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_EXTENSIONS, ERR_R_INTERNAL_ERROR);
-        goto err;
+    if (!custom_ext_add(s, context, pkt, x, chainidx, max_version)) {
+        /* SSLfatal() already called */
+        return 0;
     }
 
     for (i = 0, thisexd = ext_defs; i < OSSL_NELEM(ext_defs); i++, thisexd++) {
         EXT_RETURN (*construct)(SSL *s, WPACKET *pkt, unsigned int context,
-                                X509 *x, size_t chainidx, int *al);
+                                X509 *x, size_t chainidx);
         EXT_RETURN ret;
 
         /* Skip if not relevant for our context */
@@ -766,9 +757,11 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
         if (construct == NULL)
             continue;
 
-        ret = construct(s, pkt, context, x, chainidx, &tmpal);
-        if (ret == EXT_RETURN_FAIL)
-            goto err;
+        ret = construct(s, pkt, context, x, chainidx);
+        if (ret == EXT_RETURN_FAIL) {
+            /* SSLfatal() already called */
+            return 0;
+        }
         if (ret == EXT_RETURN_SENT
                 && (context & (SSL_EXT_CLIENT_HELLO
                                | SSL_EXT_TLS1_3_CERTIFICATE_REQUEST
@@ -777,15 +770,12 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
     }
 
     if (!WPACKET_close(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_EXTENSIONS, ERR_R_INTERNAL_ERROR);
-        goto err;
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_EXTENSIONS,
+                 ERR_R_INTERNAL_ERROR);
+        return 0;
     }
 
     return 1;
-
- err:
-    *al = tmpal;
-    return 0;
 }
 
 /*
@@ -796,8 +786,7 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
  * of a failure then |*al| is populated with a suitable error code.
  */
 
-static int final_renegotiate(SSL *s, unsigned int context, int sent,
-                                     int *al)
+static int final_renegotiate(SSL *s, unsigned int context, int sent)
 {
     if (!s->server) {
         /*
@@ -807,9 +796,8 @@ static int final_renegotiate(SSL *s, unsigned int context, int sent,
         if (!(s->options & SSL_OP_LEGACY_SERVER_CONNECT)
                 && !(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)
                 && !sent) {
-            *al = SSL_AD_HANDSHAKE_FAILURE;
-            SSLerr(SSL_F_FINAL_RENEGOTIATE,
-                   SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
+            SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_F_FINAL_RENEGOTIATE,
+                     SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
             return 0;
         }
 
@@ -820,9 +808,8 @@ static int final_renegotiate(SSL *s, unsigned int context, int sent,
     if (s->renegotiate
             && !(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)
             && !sent) {
-        *al = SSL_AD_HANDSHAKE_FAILURE;
-        SSLerr(SSL_F_FINAL_RENEGOTIATE,
-               SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
+        SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_F_FINAL_RENEGOTIATE,
+                 SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
         return 0;
     }
 
@@ -838,8 +825,7 @@ static int init_server_name(SSL *s, unsigned int context)
     return 1;
 }
 
-static int final_server_name(SSL *s, unsigned int context, int sent,
-                                     int *al)
+static int final_server_name(SSL *s, unsigned int context, int sent)
 {
     int ret = SSL_TLSEXT_ERR_NOACK, discard;
     int altmp = SSL_AD_UNRECOGNIZED_NAME;
@@ -890,23 +876,25 @@ static int final_server_name(SSL *s, unsigned int context, int sent,
                 ss->ext.tick_age_add = 0;
                 ss->ext.tick_identity = 0;
                 if (!ssl_generate_session_id(s, ss)) {
-                    ret = SSL_TLSEXT_ERR_ALERT_FATAL;
-                    altmp = SSL_AD_INTERNAL_ERROR;
+                    SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_FINAL_SERVER_NAME,
+                             ERR_R_INTERNAL_ERROR);
+                    return 0;
                 }
             } else {
-                ret = SSL_TLSEXT_ERR_ALERT_FATAL;
-                altmp = SSL_AD_INTERNAL_ERROR;
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_FINAL_SERVER_NAME,
+                         ERR_R_INTERNAL_ERROR);
+                return 0;
             }
         }
     }
 
     switch (ret) {
     case SSL_TLSEXT_ERR_ALERT_FATAL:
-        *al = altmp;
+        SSLfatal(s, altmp, SSL_F_FINAL_SERVER_NAME, SSL_R_CALLBACK_FAILED);
         return 0;
 
     case SSL_TLSEXT_ERR_ALERT_WARNING:
-        *al = altmp;
+        ssl3_send_alert(s, SSL3_AL_WARNING, altmp);
         return 1;
 
     case SSL_TLSEXT_ERR_NOACK:
@@ -919,8 +907,7 @@ static int final_server_name(SSL *s, unsigned int context, int sent,
 }
 
 #ifndef OPENSSL_NO_EC
-static int final_ec_pt_formats(SSL *s, unsigned int context, int sent,
-                                       int *al)
+static int final_ec_pt_formats(SSL *s, unsigned int context, int sent)
 {
     unsigned long alg_k, alg_a;
 
@@ -949,8 +936,8 @@ static int final_ec_pt_formats(SSL *s, unsigned int context, int sent,
                 break;
         }
         if (i == s->session->ext.ecpointformats_len) {
-            SSLerr(SSL_F_FINAL_EC_PT_FORMATS,
-                   SSL_R_TLS_INVALID_ECPOINTFORMAT_LIST);
+            SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_FINAL_EC_PT_FORMATS,
+                     SSL_R_TLS_INVALID_ECPOINTFORMAT_LIST);
             return 0;
         }
     }
@@ -1008,7 +995,7 @@ static int init_alpn(SSL *s, unsigned int context)
     return 1;
 }
 
-static int final_alpn(SSL *s, unsigned int context, int sent, int *al)
+static int final_alpn(SSL *s, unsigned int context, int sent)
 {
     if (!s->server && !sent && s->session->ext.alpn_selected != NULL)
             s->ext.early_data_ok = 0;
@@ -1022,8 +1009,10 @@ static int final_alpn(SSL *s, unsigned int context, int sent, int *al)
      * we also have to do this before we decide whether to accept early_data.
      * In TLSv1.3 we've already negotiated our cipher so we do this call now.
      * For < TLSv1.3 we defer it until after cipher negotiation.
+     * 
+     * On failure SSLfatal() already called.
      */
-    return tls_handle_alpn(s, al);
+    return tls_handle_alpn(s);
 }
 
 static int init_sig_algs(SSL *s, unsigned int context)
@@ -1060,7 +1049,7 @@ static int init_ems(SSL *s, unsigned int context)
     return 1;
 }
 
-static int final_ems(SSL *s, unsigned int context, int sent, int *al)
+static int final_ems(SSL *s, unsigned int context, int sent)
 {
     if (!s->server && s->hit) {
         /*
@@ -1069,8 +1058,8 @@ static int final_ems(SSL *s, unsigned int context, int sent, int *al)
          */
         if (!(s->s3->flags & TLS1_FLAGS_RECEIVED_EXTMS) !=
             !(s->session->flags & SSL_SESS_FLAG_EXTMS)) {
-            *al = SSL_AD_HANDSHAKE_FAILURE;
-            SSLerr(SSL_F_FINAL_EMS, SSL_R_INCONSISTENT_EXTMS);
+            SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_F_FINAL_EMS,
+                     SSL_R_INCONSISTENT_EXTMS);
             return 0;
         }
     }
@@ -1088,8 +1077,7 @@ static int init_certificate_authorities(SSL *s, unsigned int context)
 static EXT_RETURN tls_construct_certificate_authorities(SSL *s, WPACKET *pkt,
                                                         unsigned int context,
                                                         X509 *x,
-                                                        size_t chainidx,
-                                                        int *al)
+                                                        size_t chainidx)
 {
     const STACK_OF(X509_NAME) *ca_sk = SSL_get0_CA_list(s);
 
@@ -1097,25 +1085,37 @@ static EXT_RETURN tls_construct_certificate_authorities(SSL *s, WPACKET *pkt,
         return EXT_RETURN_NOT_SENT;
 
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_certificate_authorities)
-        || !WPACKET_start_sub_packet_u16(pkt)
-        || !construct_ca_names(s, pkt)
-        || !WPACKET_close(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CERTIFICATE_AUTHORITIES,
+        || !WPACKET_start_sub_packet_u16(pkt)) {
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                 SSL_F_TLS_CONSTRUCT_CERTIFICATE_AUTHORITIES,
                ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }
 
+    if (!construct_ca_names(s, pkt)) {
+        /* SSLfatal() already called */
+        return EXT_RETURN_FAIL;
+    }
+
+    if (!WPACKET_close(pkt)) {
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                 SSL_F_TLS_CONSTRUCT_CERTIFICATE_AUTHORITIES,
+                 ERR_R_INTERNAL_ERROR);
+        return EXT_RETURN_FAIL;
+    }
+
     return EXT_RETURN_SENT;
 }
 
 static int tls_parse_certificate_authorities(SSL *s, PACKET *pkt,
                                              unsigned int context, X509 *x,
-                                             size_t chainidx, int *al)
+                                             size_t chainidx)
 {
-    if (!parse_ca_names(s, pkt, al))
+    if (!parse_ca_names(s, pkt))
         return 0;
     if (PACKET_remaining(pkt) != 0) {
-        *al = SSL_AD_DECODE_ERROR;
+        SSLfatal(s, SSL_AD_DECODE_ERROR,
+                 SSL_F_TLS_PARSE_CERTIFICATE_AUTHORITIES, SSL_R_BAD_EXTENSION);
         return 0;
     }
     return 1;
@@ -1131,11 +1131,11 @@ static int init_srtp(SSL *s, unsigned int context)
 }
 #endif
 
-static int final_sig_algs(SSL *s, unsigned int context, int sent, int *al)
+static int final_sig_algs(SSL *s, unsigned int context, int sent)
 {
     if (!sent && SSL_IS_TLS13(s) && !s->hit) {
-        *al = TLS13_AD_MISSING_EXTENSION;
-        SSLerr(SSL_F_FINAL_SIG_ALGS, SSL_R_MISSING_SIGALGS_EXTENSION);
+        SSLfatal(s, TLS13_AD_MISSING_EXTENSION, SSL_F_FINAL_SIG_ALGS,
+                 SSL_R_MISSING_SIGALGS_EXTENSION);
         return 0;
     }
 
@@ -1143,7 +1143,7 @@ static int final_sig_algs(SSL *s, unsigned int context, int sent, int *al)
 }
 
 #ifndef OPENSSL_NO_EC
-static int final_key_share(SSL *s, unsigned int context, int sent, int *al)
+static int final_key_share(SSL *s, unsigned int context, int sent)
 {
     if (!SSL_IS_TLS13(s))
         return 1;
@@ -1168,8 +1168,8 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al)
             && (!s->hit
                 || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) == 0)) {
         /* Nothing left we can do - just fail */
-        *al = SSL_AD_MISSING_EXTENSION;
-        SSLerr(SSL_F_FINAL_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE);
+        SSLfatal(s, SSL_AD_MISSING_EXTENSION, SSL_F_FINAL_KEY_SHARE,
+                 SSL_R_NO_SUITABLE_KEY_SHARE);
         return 0;
     }
     /*
@@ -1230,11 +1230,9 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al)
         if (!s->hit
                 || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) == 0) {
             /* Nothing left we can do - just fail */
-            if (!sent)
-                *al = SSL_AD_MISSING_EXTENSION;
-            else
-                *al = SSL_AD_HANDSHAKE_FAILURE;
-            SSLerr(SSL_F_FINAL_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE);
+            SSLfatal(s,
+                     sent ? SSL_AD_HANDSHAKE_FAILURE : SSL_AD_MISSING_EXTENSION,
+                     SSL_F_FINAL_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE);
             return 0;
         }
     }
@@ -1249,8 +1247,8 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al)
      * processing).
      */
     if (!sent && !s->server && !tls13_generate_handshake_secret(s, NULL, 0)) {
-        *al = SSL_AD_INTERNAL_ERROR;
-        SSLerr(SSL_F_FINAL_KEY_SHARE, ERR_R_INTERNAL_ERROR);
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_FINAL_KEY_SHARE,
+                 ERR_R_INTERNAL_ERROR);
         return 0;
     }
 
@@ -1442,7 +1440,7 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
     return ret;
 }
 
-static int final_early_data(SSL *s, unsigned int context, int sent, int *al)
+static int final_early_data(SSL *s, unsigned int context, int sent)
 {
     if (!sent)
         return 1;
@@ -1456,7 +1454,8 @@ static int final_early_data(SSL *s, unsigned int context, int sent, int *al)
              * later realised that it shouldn't have done (e.g. inconsistent
              * ALPN)
              */
-            *al = SSL_AD_ILLEGAL_PARAMETER;
+            SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_FINAL_EARLY_DATA,
+                     SSL_R_BAD_EARLY_DATA);
             return 0;
         }
 
@@ -1475,7 +1474,7 @@ static int final_early_data(SSL *s, unsigned int context, int sent, int *al)
 
         if (!tls13_change_cipher_state(s,
                     SSL3_CC_EARLY | SSL3_CHANGE_CIPHER_SERVER_READ)) {
-            *al = SSL_AD_INTERNAL_ERROR;
+            /* SSLfatal() already called */
             return 0;
         }
     }
@@ -1483,24 +1482,27 @@ static int final_early_data(SSL *s, unsigned int context, int sent, int *al)
     return 1;
 }
 
-static int final_maxfragmentlen(SSL *ssl, unsigned int context, int sent, int *al)
+static int final_maxfragmentlen(SSL *s, unsigned int context, int sent)
 {
     /*
      * Session resumption on server-side with MFL extension active
      *  BUT MFL extension packet was not resent (i.e. sent == 0)
      */
-    if (ssl->server && ssl->hit && USE_MAX_FRAGMENT_LENGTH_EXT(ssl->session)
+    if (s->server && s->hit && USE_MAX_FRAGMENT_LENGTH_EXT(s->session)
             && !sent ) {
-        *al = SSL_AD_MISSING_EXTENSION;
+        SSLfatal(s, SSL_AD_MISSING_EXTENSION, SSL_F_FINAL_MAXFRAGMENTLEN,
+                 SSL_R_BAD_EXTENSION);
         return 0;
     }
 
     /* Current SSL buffer is lower than requested MFL */
-    if (ssl->session && USE_MAX_FRAGMENT_LENGTH_EXT(ssl->session)
-            && ssl->max_send_fragment < GET_MAX_FRAGMENT_LENGTH(ssl->session))
+    if (s->session && USE_MAX_FRAGMENT_LENGTH_EXT(s->session)
+            && s->max_send_fragment < GET_MAX_FRAGMENT_LENGTH(s->session))
         /* trigger a larger buffer reallocation */
-        if (!ssl3_setup_buffers(ssl))   
+        if (!ssl3_setup_buffers(s)) {
+            /* SSLfatal() already called */
             return 0;
+        }
 
     return 1;
 }
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 0dc1c49734..326d77eb56 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -14,7 +14,7 @@
 
 EXT_RETURN tls_construct_ctos_renegotiate(SSL *s, WPACKET *pkt,
                                           unsigned int context, X509 *x,
-                                          size_t chainidx, int *al)
+                                          size_t chainidx)
 {
     /* Add RI if renegotiating */
     if (!s->renegotiate)
@@ -25,7 +25,8 @@ EXT_RETURN tls_construct_ctos_renegotiate(SSL *s, WPACKET *pkt,
             || !WPACKET_sub_memcpy_u8(pkt, s->s3->previous_client_finished,
                                s->s3->previous_client_finished_len)
             || !WPACKET_close(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_RENEGOTIATE, ERR_R_INTERNAL_ERROR);
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_RENEGOTIATE,
+                 ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }
 
@@ -34,7 +35,7 @@ EXT_RETURN tls_construct_ctos_renegotiate(SSL *s, WPACKET *pkt,
 
 EXT_RETURN tls_construct_ctos_server_name(SSL *s, WPACKET *pkt,
                                           unsigned int context, X509 *x,
-                                          size_t chainidx, int *al)
+                                          size_t chainidx)
 {
     if (s->ext.hostname == NULL)
         return EXT_RETURN_NOT_SENT;
@@ -50,7 +51,8 @@ EXT_RETURN tls_construct_ctos_server_name(SSL *s, WPACKET *pkt,
                                        strlen(s->ext.hostname))
             || !WPACKET_close(pkt)
             || !WPACKET_close(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SERVER_NAME, ERR_R_INTERNAL_ERROR);
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_SERVER_NAME,
+                 ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }
 
@@ -60,7 +62,7 @@ EXT_RETURN tls_construct_ctos_server_name(SSL *s, WPACKET *pkt,
 /* Push a Max Fragment Len extension into ClientHello */
 EXT_RETURN tls_construct_ctos_maxfragmentlen(SSL *s, WPACKET *pkt,
                                              unsigned int context, X509 *x,
-                                             size_t chainidx, int *al)
+                                             size_t chainidx)
 {
     if (s->ext.max_fragment_len_mode == TLSEXT_max_fragment_length_DISABLED)
         return EXT_RETURN_NOT_SENT;
@@ -75,7 +77,8 @@ EXT_RETURN tls_construct_ctos_maxfragmentlen(SSL *s, WPACKET *pkt,
             || !WPACKET_start_sub_packet_u16(pkt)
             || !WPACKET_put_bytes_u8(pkt, s->ext.max_fragment_len_mode)
             || !WPACKET_close(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_MAXFRAGMENTLEN, ERR_R_INTERNAL_ERROR);
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                 SSL_F_TLS_CONSTRUCT_CTOS_MAXFRAGMENTLEN, ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }
 
@@ -84,7 +87,7 @@ EXT_RETURN tls_construct_ctos_maxfragmentlen(SSL *s, WPACKET *pkt,
 
 #ifndef OPENSSL_NO_SRP
 EXT_RETURN tls_construct_ctos_srp(SSL *s, WPACKET *pkt, unsigned int context,
-                                  X509 *x, size_t chainidx, int *al)
+                                  X509 *x, size_t chainidx)
 {
     /* Add SRP username if there is one */
     if (s->srp_ctx.login == NULL)
@@ -100,7 +103,8 @@ EXT_RETURN tls_construct_ctos_srp(SSL *s, WPACKET *pkt, unsigned int context,
                                strlen(s->srp_ctx.login))
             || !WPACKET_close(pkt)
             || !WPACKET_close(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SRP, ERR_R_INTERNAL_ERROR);
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_CTOS_SRP,
+                 ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }
 
@@ -137,7 +141,7 @@ static int use_ecc(SSL *s)
 
 EXT_RETURN tls_construct_ctos_ec_pt_formats(SSL *s, WPACKET *pkt,
                                             unsigned int context, X509 *x,
-                                            size_t chainidx, int *al)
+                                            size_t chainidx)
 {
     const unsigned char *pformats;
     size_t num_formats;
@@ -153,7 +157,8 @@ EXT_RETURN tls_construct_ctos_ec_pt_formats(SSL *s, WPACKET *pkt,
             || !WPACKET_start_sub_packet_u16(pkt)
             || !WPACKET_sub_memcpy_u8(pkt, pformats, num_formats)
             || !WPACKET_close(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_EC_PT_FORMATS, ERR_R_INTERNAL_ERROR);
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                 SSL_F_TLS_CONSTRUCT_CTOS_EC_PT_FORMATS, ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }
 
@@ -162,7 +167,7 @@ EXT_RETURN tls_construct_ctos_ec_pt_formats(SSL *s, WPACKET *pkt,
 
 EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
                                                unsigned int context, X509 *x,
-                                               size_t chainidx, int *al)
+                                               size_t chainidx)
 {
     const uint16_t *pgroups = NULL;
     size_t num_groups = 0, i;
@@ -180,8 +185,9 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
                /* Sub-packet for supported_groups extension */
             || !WPACKET_start_sub_packet_u16(pkt)
             || !WPACKET_start_sub_packet_u16(pkt)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS,
-               ERR_R_INTERNAL_ERROR);
+        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 */
@@ -208,7 +214,7 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
 
 EXT_RETURN tls_construct_ctos_session_ticket(SSL *s, WPACKET *pkt,
                                              unsigned int context, X509 *x,
-                                             size_t chainidx, int *al)
+                                             size_t chainidx)
 {
     size_t ticklen;
 
@@ -224,8 +230,9 @@ EXT_RETURN tls_construct_ctos_session_ticket(SSL *s, WPACKET *pkt,
         ticklen = s->ext.session_ticket->length;
         s->session->ext.tick = OPENSSL_malloc(ticklen);
         if (s->session->ext.tick == NULL) {
-            SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SESSION_TICKET,
-                   ERR_R_INTERNAL_ERROR);
+            SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                     SSL_F_TLS_CONSTRUCT_CTOS_SESSION_TICKET,
+                     ERR_R_INTERNAL_ERROR);
             return EXT_RETURN_FAIL;
         }
         memcpy(s->session->ext.tick,
@@ -241,7 +248,8 @@ EXT_RETURN tls_construct_ctos_session_ticket(SSL *s, WPACKET *pkt,
 
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_session_ticket)
             || !WPACKET_sub_memcpy_u16(pkt, s->session->ext.tick, ticklen)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SESSION_TICKET, ERR_R_INTERNAL_ERROR);