Preserve connection custom extensions in SSL_set_SSL_CTX()

pluknet · mattcaswell · commit 45bd3c3798ab · 2025-06-20T15:56:58.000+01:00
The SSL_set_SSL_CTX() function is used to switch SSL contexts for the given SSL object. If contexts differ, this includes updating a cert structure with custom extensions from the new context. This however overwrites connection custom extensions previously set on top of inherited from the old context. The fix is to preserve connection custom extensions using a newly introduced flag SSL_EXT_FLAG_CONN in custom_ext_copy_conn(). Similar to custom_ext_copy(), it is a no-op if there are no custom extensions to copy. The only such consumer is ossl_quic_tls_configure() used to set the "quic_transport_parameters" extension. Before this change, context switch resulted in transport parameters not being sent due to the missing extension. Initially reported at nginx/nginx#711 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #27706) (cherry picked from commit 403ba31)
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
@@ -5476,6 +5476,8 @@ 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;
 
diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
@@ -2067,6 +2067,11 @@ 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;
@@ -2998,6 +3003,8 @@ __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);
diff --git a/ssl/statem/extensions_cust.c b/ssl/statem/extensions_cust.c
@@ -107,7 +107,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 = 0;
+        meth->ext_flags &= ~(SSL_EXT_FLAG_SENT | SSL_EXT_FLAG_RECEIVED);
 }
 
 /* Pass received custom extension data to the application for parsing. */
@@ -330,6 +330,58 @@ 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;
@@ -417,6 +469,7 @@ 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++;
diff --git a/test/sslapitest.c b/test/sslapitest.c
@@ -12866,10 +12866,11 @@ 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, *cctx = NULL;
+    SSL_CTX *sctx = NULL, *sctx2 = NULL, *cctx = NULL;
     SSL *serverssl = NULL, *clientssl = NULL;
     int testresult = 0;
     OSSL_DISPATCH qtdis[] = {
@@ -12897,6 +12898,7 @@ 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));
@@ -12911,6 +12913,18 @@ 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;
@@ -12951,6 +12965,12 @@ 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)
@@ -12992,6 +13012,7 @@ 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);
 
@@ -13573,7 +13594,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, 5);
+    ADD_ALL_TESTS(test_quic_tls, 6);
     ADD_TEST(test_quic_tls_early_data);
 #endif
     ADD_ALL_TESTS(test_no_renegotiation, 2);
