From: Sergey Kandaurov Date: Wed, 28 May 2025 17:58:26 +0000 (+0400) Subject: Preserve connection custom extensions in SSL_set_SSL_CTX() X-Git-Tag: openssl-3.5.1~13 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=45bd3c3798ab2f1f143f26c27b6d080c72b70eea;p=thirdparty%2Fopenssl.git Preserve connection custom extensions in SSL_set_SSL_CTX() 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 https://github.com/nginx/nginx/issues/711 Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/27706) (cherry picked from commit 403ba31a02e47d37070036529966d5a94d98c6fd) --- diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 18ed42e5040..34cde494964 100644 --- 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 index 597b4d4b639..c53ebc36ae9 100644 --- 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 index 2a52e2b67ed..aa352529c4c 100644 --- 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 index b5db25ce8d4..b83dd6c552d 100644 --- 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);