]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Revert "Preserve connection custom extensions in SSL_set_SSL_CTX()"
authorMatt Caswell <matt@openssl.foundation>
Tue, 12 May 2026 13:19:35 +0000 (14:19 +0100)
committerTomas Mraz <tomas@openssl.foundation>
Wed, 13 May 2026 16:23:16 +0000 (18:23 +0200)
This reverts commit 403ba31a02e47d37070036529966d5a94d98c6fd.

PR #27706 (that this PR reverts) was intended to fix nginx/nginx#711

The problem was that when calling SSL_set_SSL_CTX() from an SNI callback
when using a QUIC object, the QUIC custom extensions were not being
handled correctly. The fix attempted to resolve this to make sure that
they were correctly being copied.

However, in reality there was a bug in the SNI callback code that meant when
we called it from a QUIC connection we were passing the *inner* TLS
object instead of the real QUIC one. The *inner* TLS object should be
entirely internal and not exposed to user callbacks. This bug was fixed in
dc84829cc5.

Once the above fix was in place `SSL_set_SSL_CTX()` immediately fails when
called with a QUIC object via the SNI callback. This was always the
intended behaviour - its use with a QUIC object was blocked since the very
beginning - but the fact that we passed the inner TLS object by mistake
circumvented the check when it was invoked from the SNI callback.

The fix in dc84829cc5 actually landed *before* the commit that this PR
reverts. So, in reality the nginx bug was already "fixed" by the time
that PR #27706 was merged (fixed in the sense that the invocation of
`SSL_set_SSL_CTX()` fails gracefully). The code that it introduced can not
be reached (and never could be) because calling `SSL_set_SSL_CTX()` is
explicitly blocked when using a QUIC object. Therefore we should remove
this dead code.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
MergeDate: Wed May 13 16:23:24 2026
(Merged from https://github.com/openssl/openssl/pull/31159)

ssl/ssl_lib.c
ssl/ssl_local.h
ssl/statem/extensions_cust.c
test/sslapitest.c

index 8fb76e96ad60987bf07fc039393cac9205d60663..b7673a0b71f0ac7614b80e1a45c746aa22dc6eb2 100644 (file)
@@ -5703,8 +5703,6 @@ SSL_CTX *SSL_set_SSL_CTX(SSL *ssl, SSL_CTX *ctx)
     new_cert = ssl_cert_dup(ctx->cert);
     if (new_cert == NULL)
         goto err;
-    if (!custom_exts_copy_conn(&new_cert->custext, &sc->cert->custext))
-        goto err;
     if (!custom_exts_copy_flags(&new_cert->custext, &sc->cert->custext))
         goto err;
 
index 7edef0bd402a206f4fc0a58a5bd285a334463e4c..86a9237da0393fca1f62913c77cdaf0db3ad9f6e 100644 (file)
@@ -2102,11 +2102,6 @@ typedef struct {
  * corresponding ServerHello extension.
  */
 #define SSL_EXT_FLAG_SENT 0x2
-/*
- * Indicates an extension that was set on SSL object and needs to be
- * preserved when switching SSL contexts.
- */
-#define SSL_EXT_FLAG_CONN 0x4
 
 typedef struct {
     custom_ext_method *meths;
@@ -2920,8 +2915,6 @@ __owur int custom_ext_add(SSL_CONNECTION *s, int context, WPACKET *pkt, X509 *x,
 
 __owur int custom_exts_copy(custom_ext_methods *dst,
     const custom_ext_methods *src);
-__owur int custom_exts_copy_conn(custom_ext_methods *dst,
-    const custom_ext_methods *src);
 __owur int custom_exts_copy_flags(custom_ext_methods *dst,
     const custom_ext_methods *src);
 void custom_exts_free(custom_ext_methods *exts);
index 881e42be07144a474baae37e1c7c1405bf8cb006..c4be3d5318aa3b807f033ecaf9f05d2cef4bc82a 100644 (file)
@@ -106,7 +106,7 @@ void custom_ext_init(custom_ext_methods *exts)
     custom_ext_method *meth = exts->meths;
 
     for (i = 0; i < exts->meths_count; i++, meth++)
-        meth->ext_flags &= ~(SSL_EXT_FLAG_SENT | SSL_EXT_FLAG_RECEIVED);
+        meth->ext_flags = 0;
 }
 
 /* Pass received custom extension data to the application for parsing. */
@@ -390,56 +390,6 @@ int custom_exts_copy(custom_ext_methods *dst, const custom_ext_methods *src)
     return 1;
 }
 
-/* Copy custom extensions that were set on connection */
-int custom_exts_copy_conn(custom_ext_methods *dst,
-    const custom_ext_methods *src)
-{
-    size_t i;
-    int err = 0;
-
-    if (src->meths_count > 0) {
-        size_t meths_count = 0;
-
-        for (i = 0; i < src->meths_count; i++)
-            if ((src->meths[i].ext_flags & SSL_EXT_FLAG_CONN) != 0)
-                meths_count++;
-
-        if (meths_count > 0) {
-            custom_ext_method *methdst = OPENSSL_realloc(dst->meths,
-                (dst->meths_count + meths_count) * sizeof(custom_ext_method));
-
-            if (methdst == NULL)
-                return 0;
-
-            for (i = 0; i < dst->meths_count; i++)
-                custom_ext_copy_old_cb(&methdst[i], &dst->meths[i], &err);
-
-            dst->meths = methdst;
-            methdst += dst->meths_count;
-
-            for (i = 0; i < src->meths_count; i++) {
-                custom_ext_method *methsrc = &src->meths[i];
-
-                if ((methsrc->ext_flags & SSL_EXT_FLAG_CONN) == 0)
-                    continue;
-
-                memcpy(methdst, methsrc, sizeof(custom_ext_method));
-                custom_ext_copy_old_cb(methdst, methsrc, &err);
-                methdst++;
-            }
-
-            dst->meths_count += meths_count;
-        }
-    }
-
-    if (err) {
-        custom_exts_free(dst);
-        return 0;
-    }
-
-    return 1;
-}
-
 void custom_exts_free(custom_ext_methods *exts)
 {
     size_t i;
@@ -528,7 +478,6 @@ int ossl_tls_add_custom_ext_intern(SSL_CTX *ctx, custom_ext_methods *exts,
     meth->add_cb = add_cb;
     meth->free_cb = free_cb;
     meth->ext_type = ext_type;
-    meth->ext_flags = (ctx == NULL) ? SSL_EXT_FLAG_CONN : 0;
     meth->add_arg = add_arg;
     meth->parse_arg = parse_arg;
     exts->meths_count++;
index 38faa22bbf091f6925022ae1c43f655686d481ee..b17318dfbe1f0035feee9a5aaa91ab35cae50616 100644 (file)
@@ -13841,11 +13841,10 @@ static int alert_cb(SSL *s, unsigned char alert_code, void *arg)
  * Test 1: Force a failure
  * Test 3: Use a CCM based ciphersuite
  * Test 4: fail yield_secret_cb to see double free
- * Test 5: Normal run with SNI
  */
 static int test_quic_tls(int idx)
 {
-    SSL_CTX *sctx = NULL, *sctx2 = NULL, *cctx = NULL;
+    SSL_CTX *sctx = NULL, *cctx = NULL;
     SSL *serverssl = NULL, *clientssl = NULL;
     int testresult = 0;
     OSSL_DISPATCH qtdis[] = {
@@ -13873,7 +13872,6 @@ static int test_quic_tls(int idx)
     if (idx == 4)
         qtdis[3].function = (void (*)(void))yield_secret_cb_fail;
 
-    snicb = 0;
     memset(secret_history, 0, sizeof(secret_history));
     secret_history_idx = 0;
     memset(&sdata, 0, sizeof(sdata));
@@ -13888,18 +13886,6 @@ static int test_quic_tls(int idx)
             &sctx, &cctx, cert, privkey)))
         goto end;
 
-    if (idx == 5) {
-        if (!TEST_true(create_ssl_ctx_pair(libctx, TLS_server_method(), NULL,
-                TLS1_3_VERSION, 0,
-                &sctx2, NULL, cert, privkey)))
-            goto end;
-
-        /* Set up SNI */
-        if (!TEST_true(SSL_CTX_set_tlsext_servername_callback(sctx, sni_cb))
-            || !TEST_true(SSL_CTX_set_tlsext_servername_arg(sctx, sctx2)))
-            goto end;
-    }
-
     if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, NULL,
             NULL)))
         goto end;
@@ -13940,12 +13926,6 @@ static int test_quic_tls(int idx)
         goto end;
     }
 
-    /* We should have had the SNI callback called exactly once */
-    if (idx == 5) {
-        if (!TEST_int_eq(snicb, 1))
-            goto end;
-    }
-
     /* Check no problems during the handshake */
     if (!TEST_false(sdata.alert)
         || !TEST_false(cdata.alert)
@@ -13987,7 +13967,6 @@ static int test_quic_tls(int idx)
 end:
     SSL_free(serverssl);
     SSL_free(clientssl);
-    SSL_CTX_free(sctx2);
     SSL_CTX_free(sctx);
     SSL_CTX_free(cctx);
 
@@ -15037,7 +15016,7 @@ int setup_tests(void)
 #endif
     ADD_ALL_TESTS(test_alpn, 4);
 #if !defined(OSSL_NO_USABLE_TLS1_3)
-    ADD_ALL_TESTS(test_quic_tls, 6);
+    ADD_ALL_TESTS(test_quic_tls, 5);
     ADD_TEST(test_quic_tls_early_data);
 #endif
     ADD_ALL_TESTS(test_no_renegotiation, 2);