From 2db44043faf27c89829448d4abff14fffd1e113e Mon Sep 17 00:00:00 2001 From: sebastian-carpenter Date: Fri, 12 Jun 2026 17:31:46 -0600 Subject: [PATCH] prevent public SNI from being installed on ctx --- src/tls.c | 23 +++-- tests/api.c | 244 +++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 208 insertions(+), 59 deletions(-) diff --git a/src/tls.c b/src/tls.c index ace0f7f979..cdaaf5c0c1 100644 --- a/src/tls.c +++ b/src/tls.c @@ -16737,9 +16737,9 @@ static int TLSX_EchChangeSNI(WOLFSSL* ssl, TLSX** pEchX, } if (serverNameX == NULL && ssl->ctx && ssl->ctx->extensions) { + /* do not set extensions to ctx->extensions otherwise the shared + * list would be mutated */ serverNameX = TLSX_Find(ssl->ctx->extensions, TLSX_SERVER_NAME); - if (serverNameX != NULL) - extensions = &ssl->ctx->extensions; } /* ECH requires an inner SNI to be present for ClientHelloInner. @@ -16759,14 +16759,21 @@ static int TLSX_EchChangeSNI(WOLFSSL* ssl, TLSX** pEchX, XMEMCPY(serverName, hostName, hostNameSz); } - /* only swap the SNI if one was found; extensions is non-NULL if an - * SNI entry was found on ssl->extensions or ctx->extensions */ - if (ret == 0 && extensions != NULL) { - /* remove the inner server name */ - TLSX_Remove(extensions, TLSX_SERVER_NAME, ssl->heap); + /* only swap the SNI if one was found, but only install onto + * ssl->extensions */ + if (ret == 0) { + if (extensions != NULL) { + /* remove the inner server name */ + TLSX_Remove(extensions, TLSX_SERVER_NAME, ssl->heap); + } + else { + /* inner SNI stays on the ctx; nothing to restore on ssl */ + serverNameX = NULL; + extensions = &ssl->extensions; + } /* set the public name as the server name */ - if ((ret = TLSX_UseSNI(extensions, WOLFSSL_SNI_HOST_NAME, + if ((ret = TLSX_UseSNI(&ssl->extensions, WOLFSSL_SNI_HOST_NAME, ((WOLFSSL_ECH*)echX->data)->echConfig->publicName, XSTRLEN(((WOLFSSL_ECH*)echX->data)->echConfig->publicName), ssl->heap)) == WOLFSSL_SUCCESS) diff --git a/tests/api.c b/tests/api.c index d34a5afef8..9b6e3a7bf6 100644 --- a/tests/api.c +++ b/tests/api.c @@ -14061,6 +14061,63 @@ static word16 echCbTestKemID = 0; static word16 echCbTestKdfID = 0; static word16 echCbTestAeadID = 0; +/* return the index of the first extension + * extLen will be updated with the length of the extensions field */ +static int ech_seek_extensions(byte* buf, word16* extLen) +{ + word16 idx; + byte sessionIdLen; + word16 cipherSuitesLen; + byte compressionLen; + + idx = OPAQUE16_LEN + RAN_LEN; + + sessionIdLen = buf[idx++]; + idx += sessionIdLen; + + ato16(buf + idx, &cipherSuitesLen); + idx += OPAQUE16_LEN + cipherSuitesLen; + + compressionLen = buf[idx++]; + idx += compressionLen; + + ato16(buf + idx, extLen); + idx += OPAQUE16_LEN; + + return idx; +} + +/* locate a particular extension: + * idx_p is updated with the location of that extension + * -> idx_p should start just after the handshake header + * 0 returned on success, error otherwise */ +static int ech_find_extension(byte* buf, word16* idx_p, word16 extType) +{ + word16 idx; + word16 extIdx; + word16 extLen; + + extIdx = ech_seek_extensions(buf + *idx_p, &extLen) + *idx_p; + idx = extIdx; + + while (idx - extIdx < extLen) { + word16 type; + word16 len; + + ato16(buf + idx, &type); + if (type == extType) { + *idx_p = idx; + return 0; + } + + idx += OPAQUE16_LEN; + ato16(buf + idx, &len); + idx += OPAQUE16_LEN + len; + } + + return BAD_FUNC_ARG; +} + /* the arg is whether the client has ech enabled or not */ static int test_ech_server_sni_callback(WOLFSSL* ssl, int* ad, void* arg) { @@ -14999,6 +15056,141 @@ static int test_wolfSSL_Tls13_ECH_disable_conn(void) return EXPECT_RESULT(); } +/* The public name must be visible and the private name must not be visible. + * useCtx installs the inner SNI on the shared ctx instead of the per-connection + * ssl, to exercise both branches of TLSX_EchChangeSNI. */ +static int test_wolfSSL_Tls13_ECH_wire_sni_ex(int accept, int useCtx) +{ + EXPECT_DECLS; + test_ssl_memio_ctx test_ctx; + word16 idx; + word16 nameLen; + word16 publicLen = (word16)XSTRLEN(echCbTestPublicName); + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + test_ctx.s_cb.method = wolfTLSv1_3_server_method; + test_ctx.c_cb.method = wolfTLSv1_3_client_method; + + test_ctx.s_cb.ctx_ready = test_ech_server_ctx_ready; + test_ctx.s_cb.ssl_ready = test_ech_server_ssl_ready; + + ExpectIntEQ(test_ssl_memio_setup(&test_ctx), TEST_SUCCESS); + + /* Accept path uses the correct configs; reject path installs bad configs + * (with the correct public name) so the client still offers ECH. */ + if (accept) { + ExpectIntEQ(wolfSSL_SetEchConfigs(test_ctx.c_ssl, echCbTestConfigs, + echCbTestConfigsLen), WOLFSSL_SUCCESS); + } + else { + WOLFSSL_CTX* tempCtx = NULL; + byte badConfig[128]; + word32 badConfigLen = sizeof(badConfig); + + ExpectNotNull(tempCtx = wolfSSL_CTX_new(wolfTLSv1_3_server_method())); + ExpectIntEQ(wolfSSL_CTX_GenerateEchConfig(tempCtx, echCbTestPublicName, + 0, 0, 0), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_CTX_GetEchConfigs(tempCtx, badConfig, + &badConfigLen), WOLFSSL_SUCCESS); + wolfSSL_CTX_free(tempCtx); + ExpectIntEQ(wolfSSL_SetEchConfigs(test_ctx.c_ssl, badConfig, + badConfigLen), WOLFSSL_SUCCESS); + } + + /* install the private (inner) SNI on the shared ctx or the per-connection + * ssl */ + if (useCtx) { + ExpectIntEQ(wolfSSL_CTX_UseSNI(test_ctx.c_ctx, WOLFSSL_SNI_HOST_NAME, + echCbTestPrivateName, (word16)XSTRLEN(echCbTestPrivateName)), + WOLFSSL_SUCCESS); + } + else { + ExpectIntEQ(wolfSSL_UseSNI(test_ctx.c_ssl, WOLFSSL_SNI_HOST_NAME, + echCbTestPrivateName, (word16)XSTRLEN(echCbTestPrivateName)), + WOLFSSL_SUCCESS); + } + + /* force HelloRetryRequest */ + ExpectIntEQ(wolfSSL_NoKeyShares(test_ctx.c_ssl), WOLFSSL_SUCCESS); + + /* On reject, client aborts with ech_required and won't send a cert. */ + if (!accept) { + wolfSSL_set_verify(test_ctx.s_ssl, WOLFSSL_VERIFY_NONE, NULL); + wolfSSL_set_verify(test_ctx.c_ssl, WOLFSSL_VERIFY_PEER, NULL); + } + + /* client writes CH1 into s_buff */ + ExpectIntEQ(wolfSSL_connect(test_ctx.c_ssl), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(test_ctx.c_ssl, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + + /* check sent SNI is correct */ + idx = RECORD_HEADER_SZ + HANDSHAKE_HEADER_SZ; + ExpectIntEQ(ech_find_extension(test_ctx.s_buff, &idx, TLSXT_SERVER_NAME), + 0); + ExpectIntEQ(test_ctx.s_buff[idx + 6], WOLFSSL_SNI_HOST_NAME); + ato16(test_ctx.s_buff + idx + 7, &nameLen); + ExpectIntEQ(nameLen, publicLen); + ExpectIntEQ(XMEMCMP(test_ctx.s_buff + idx + 9, echCbTestPublicName, + publicLen), 0); + + /* server consumes CH1 and writes HRR into c_buff */ + ExpectIntEQ(wolfSSL_accept(test_ctx.s_ssl), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(test_ctx.s_ssl, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + ExpectIntEQ(test_ctx.s_ssl->options.serverState, + SERVER_HELLO_RETRY_REQUEST_COMPLETE); + + /* client reads HRR from c_buff and writes CH2 into s_buff */ + ExpectIntEQ(wolfSSL_connect(test_ctx.c_ssl), WOLFSSL_FATAL_ERROR); + ExpectIntEQ(wolfSSL_get_error(test_ctx.c_ssl, WOLFSSL_FATAL_ERROR), + WOLFSSL_ERROR_WANT_READ); + + /* check sent SNI is correct */ + idx = RECORD_HEADER_SZ + HANDSHAKE_HEADER_SZ; + ExpectIntEQ(ech_find_extension(test_ctx.s_buff, &idx, TLSXT_SERVER_NAME), + 0); + ExpectIntEQ(test_ctx.s_buff[idx + 6], WOLFSSL_SNI_HOST_NAME); + ato16(test_ctx.s_buff + idx + 7, &nameLen); + ExpectIntEQ(nameLen, publicLen); + ExpectIntEQ(XMEMCMP(test_ctx.s_buff + idx + 9, echCbTestPublicName, + publicLen), 0); + + /* sanity check: finish the handshake and verify ECH acceptance */ + if (accept) { + ExpectIntEQ(test_ssl_memio_do_handshake(&test_ctx, 10, NULL), + TEST_SUCCESS); + ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.c_ssl), + WOLFSSL_ECH_STATUS_ACCEPTED); + ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.s_ssl), + WOLFSSL_ECH_STATUS_ACCEPTED); + } + else { + ExpectIntNE(test_ssl_memio_do_handshake(&test_ctx, 10, NULL), + TEST_SUCCESS); + ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.c_ssl), + WOLFSSL_ECH_STATUS_REJECTED); + ExpectIntEQ(wolfSSL_GetEchStatus(test_ctx.s_ssl), + WOLFSSL_ECH_STATUS_REJECTED); + } + + test_ssl_memio_cleanup(&test_ctx); + + return EXPECT_RESULT(); +} + +static int test_wolfSSL_Tls13_ECH_wire_sni(void) +{ + EXPECT_DECLS; + /* {reject, accept} x inner SNI on {ssl, ctx} */ + ExpectIntEQ(test_wolfSSL_Tls13_ECH_wire_sni_ex(0, 0), TEST_SUCCESS); + ExpectIntEQ(test_wolfSSL_Tls13_ECH_wire_sni_ex(1, 0), TEST_SUCCESS); + ExpectIntEQ(test_wolfSSL_Tls13_ECH_wire_sni_ex(0, 1), TEST_SUCCESS); + ExpectIntEQ(test_wolfSSL_Tls13_ECH_wire_sni_ex(1, 1), TEST_SUCCESS); + return EXPECT_RESULT(); +} + /* Regression test: an inner SNI hostname >= MAX_PUBLIC_NAME_SZ (256) bytes * must not cause a stack-buffer-overflow in TLSX_EchRestoreSNI. Before the * fix, the truncated copy omitted the NUL terminator and XSTRLEN read past @@ -15045,57 +15237,6 @@ static int test_wolfSSL_Tls13_ECH_long_SNI(void) return EXPECT_RESULT(); } -static int ech_seek_extensions(byte* buf, word16* innerExtLen) -{ - word16 idx; - byte sessionIdLen; - word16 cipherSuitesLen; - byte compressionLen; - - idx = OPAQUE16_LEN + RAN_LEN; - - sessionIdLen = buf[idx++]; - idx += sessionIdLen; - - ato16(buf + idx, &cipherSuitesLen); - idx += OPAQUE16_LEN + cipherSuitesLen; - - compressionLen = buf[idx++]; - idx += compressionLen; - - ato16(buf + idx, innerExtLen); - idx += OPAQUE16_LEN; - - return idx; -} - -static int ech_find_extension(byte* buf, word16* idx_p, word16 extType) -{ - word16 idx; - word16 innerExtIdx; - word16 innerExtLen; - - innerExtIdx = ech_seek_extensions(buf + *idx_p, &innerExtLen) + *idx_p; - idx = innerExtIdx; - - while (idx - innerExtIdx < innerExtLen) { - word16 type; - word16 len; - - ato16(buf + idx, &type); - if (type == extType) { - *idx_p = idx; - return 0; - } - - idx += OPAQUE16_LEN; - ato16(buf + idx, &len); - idx += OPAQUE16_LEN + len; - } - - return BAD_FUNC_ARG; -} - /* Test the HRR ECH rejection fallback path: * client offers ECH, HRR is triggered, server sends HRR without ECH extension, * client falls back to the outer transcript, then aborts with ech_required. @@ -35242,6 +35383,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_Tls13_ECH_trial_decrypt), TEST_DECL(test_wolfSSL_Tls13_ECH_GREASE), TEST_DECL(test_wolfSSL_Tls13_ECH_disable_conn), + TEST_DECL(test_wolfSSL_Tls13_ECH_wire_sni), TEST_DECL(test_wolfSSL_Tls13_ECH_long_SNI), TEST_DECL(test_wolfSSL_Tls13_ECH_HRR_rejection), TEST_DECL(test_wolfSSL_Tls13_ECH_ch2_no_ech),