BUG/MEDIUM: ssl: fix an out-of-bounds read on SSL_CTX switch with a .sctl file

A certificate's .sctl sidecar registers extension 18 only on that
certificate's SSL_CTX. Connections start on initial_ctx without this
extension, then switch contexts in the ClientHello callback.

OpenSSL sizes the received-extensions array from the initial context but
parses it using the selected context's extension count. An unauthenticated
ClientHello selecting a certificate with a .sctl file can therefore trigger
an out-of-bounds read in tls_parse_all_extensions(). Depending on the heap
contents, the parser may also write past the array and read adjacent data.

Register the extension in the same order on every switchable frontend
context. The add callback omits it when the selected context has no SCT
list. Skip QUIC contexts because native OpenSSL's SSL_set_SSL_CTX() reads
freed memory when merging the transport parameters extension with custom
extensions. Skip registration on AWS-LC, which owns extension 18.

Workaround: remove "sctl" from "ssl-load-extra-files" or remove the .sctl files.

This should be backported to all supported versions.

Reported-by: Finlay Moss <fmoss2003@gmail.com>

diff --git a/include/haproxy/openssl-compat.h b/include/haproxy/openssl-compat.h
index 25a8f3cccffa3a7d471839e91a36df77223a3bbb..5c37d6584b3064f7803dd0eb489bc6a56aca72a2 100644 (file)
--- a/include/haproxy/openssl-compat.h
+++ b/include/haproxy/openssl-compat.h
@@ -123,7 +123,7 @@ enum ssl_encryption_level_t {
 #define HAVE_SSL_CLIENT_HELLO_CB
 #endif
 
-#if ((OPENSSL_VERSION_NUMBER >= 0x1000200fL) && !defined(OPENSSL_NO_TLSEXT) && !defined(LIBRESSL_VERSION_NUMBER) && !defined(OPENSSL_IS_BORINGSSL))
+#if ((OPENSSL_VERSION_NUMBER >= 0x1000200fL) && !defined(OPENSSL_NO_TLSEXT) && !defined(LIBRESSL_VERSION_NUMBER) && !defined(OPENSSL_IS_BORINGSSL) && !defined(OPENSSL_IS_AWSLC))
 #define HAVE_SSL_CTX_ADD_SERVER_CUSTOM_EXT
 #endif
 

diff --git a/include/haproxy/ssl_sock.h b/include/haproxy/ssl_sock.h
index 66675636d7868509368cae0157e4d0686b10e1e5..b8dbc9025f7ae8091ce151da9c7787ed759b2951 100644 (file)
--- a/include/haproxy/ssl_sock.h
+++ b/include/haproxy/ssl_sock.h
@@ -61,6 +61,7 @@ extern struct list openssl_providers;
 extern struct stats_module ssl_stats_module;
 
 uint64_t ssl_sock_sni_hash(const struct ist sni);
+int ssl_sock_ctx_add_sctl_ext(struct bind_conf *bind_conf, SSL_CTX *ctx);
 int ssl_sock_prep_ctx_and_inst(struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_conf,
                               SSL_CTX *ctx, struct ckch_inst *ckch_inst, char **err);
 int ssl_sock_prep_srv_ctx_and_inst(const struct server *srv, SSL_CTX *ctx,

diff --git a/src/ssl_gencert.c b/src/ssl_gencert.c
index 39e6fc7f7c6c1586f5613cae843d80ad0c08aa69..65db0f07755e5d93cab62dac6075b684848bc598 100644 (file)
--- a/src/ssl_gencert.c
+++ b/src/ssl_gencert.c
@@ -246,6 +246,10 @@ static SSL_CTX *ssl_sock_do_create_cert(const char *servername, struct bind_conf
        if (!(ssl_ctx = SSL_CTX_new(SSLv23_server_method())))
                goto mkcert_error;
 
+       /* the custom extensions must match the ones of the initial context */
+       if (!ssl_sock_ctx_add_sctl_ext(bind_conf, ssl_ctx))
+               goto mkcert_error;
+
        if (global_ssl.security_level > -1)
                SSL_CTX_set_security_level(ssl_ctx, global_ssl.security_level);
 

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 08968b3ae5bed35cbca53e005d0f83575e469673..4a70975d975836095b4998a700871bed32359770 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1679,9 +1679,17 @@ static int ssl_sock_load_ocsp(const char *path, SSL_CTX *ctx, struct ckch_data *
 
 int sctl_ex_index = -1;
 
+/* Returns 1 with the selected context's SCT list in <out>/<outlen>, or 0
+ * to omit the extension when that context has no list.
+ */
 static __maybe_unused int ssl_sock_sctl_add_cbk(SSL *ssl, unsigned ext_type, const unsigned char **out, size_t *outlen, int *al, void *add_arg)
 {
-       struct buffer *sctl = add_arg;
+       struct buffer *sctl;
+
+       /* All frontend contexts register the extension; only some have a list. */
+       sctl = SSL_CTX_get_ex_data(SSL_get_SSL_CTX(ssl), sctl_ex_index);
+       if (!sctl)
+               return 0;
 
        *out = (unsigned char *) sctl->area;
        *outlen = sctl->data;
@@ -1689,27 +1697,40 @@ static __maybe_unused int ssl_sock_sctl_add_cbk(SSL *ssl, unsigned ext_type, con
        return 1;
 }
 
+/* Accepts the client's SCT request without inspecting its payload. Returns 1. */
 static __maybe_unused int ssl_sock_sctl_parse_cbk(SSL *s, unsigned int ext_type, const unsigned char *in, size_t inlen, int *al, void *parse_arg)
 {
        return 1;
 }
 
+/* Attaches <sctl> to <ctx>. Returns 0 on success or -1 on failure. */
 static int ssl_sock_load_sctl(SSL_CTX *ctx, struct buffer *sctl)
 {
-       int ret = -1;
-
-       if (!SSL_CTX_add_server_custom_ext(ctx, CT_EXTENSION_TYPE, ssl_sock_sctl_add_cbk, NULL, sctl, ssl_sock_sctl_parse_cbk, NULL))
-               goto out;
-
-       SSL_CTX_set_ex_data(ctx, sctl_ex_index, sctl);
+       if (!SSL_CTX_set_ex_data(ctx, sctl_ex_index, sctl))
+               return -1;
+       return 0;
+}
 
-       ret = 0;
+#endif
 
-out:
-       return ret;
-}
+/* Registers SCTL on frontend <ctx> of <bind_conf>. Call after creating each
+ * context so extension arrays match across SSL_set_SSL_CTX() switches,
+ * avoiding out-of-bounds accesses in OpenSSL. Skip QUIC: native OpenSSL
+ * reads freed memory when merging its transport parameters extension.
+ * Returns 1 on success (including skipped contexts), 0 on failure.
+ */
+int ssl_sock_ctx_add_sctl_ext(struct bind_conf *bind_conf, SSL_CTX *ctx)
+{
+#ifdef HAVE_SSL_CTX_ADD_SERVER_CUSTOM_EXT
+       /* xprt is not yet set to QUIC when certificates are loaded */
+       if (bind_conf->options & BC_O_USE_SOCK_DGRAM)
+               return 1;
 
+       if (!SSL_CTX_add_server_custom_ext(ctx, CT_EXTENSION_TYPE, ssl_sock_sctl_add_cbk, NULL, NULL, ssl_sock_sctl_parse_cbk, NULL))
+               return 0;
 #endif
+       return 1;
+}
 
 static void ssl_sock_infocbk(const SSL *ssl, int where, int ret)
 {
@@ -3350,6 +3371,13 @@ int ckch_inst_new_load_store(const char *path, struct ckch_store *ckchs, struct
 
        SSL_CTX_set_ex_data(ctx, ssl_crtname_index, crtname);
 
+       if (!ssl_sock_ctx_add_sctl_ext(bind_conf, ctx)) {
+               memprintf(err, "%sunable to register the SCTL extension for cert '%s'.\n",
+                         err && *err ? *err : "", path);
+               errcode |= ERR_ALERT | ERR_FATAL;
+               goto error;
+       }
+
        if (global_ssl.security_level > -1)
                SSL_CTX_set_security_level(ctx, global_ssl.security_level);
 
@@ -4155,6 +4183,12 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf)
 
        bind_conf->initial_ctx = ctx;
 
+       if (!ssl_sock_ctx_add_sctl_ext(bind_conf, ctx)) {
+               cfgerr += 1;
+               ha_alert("Proxy '%s': failed to register the SCTL extension for bind '%s' at [%s:%d].\n",
+                        bind_conf->frontend->id, bind_conf->arg, bind_conf->file, bind_conf->line);
+       }
+
        if (global_ssl.security_level > -1)
                SSL_CTX_set_security_level(ctx, global_ssl.security_level);