From 9a67c0d419f9eb60bd4322a0691537e76805035d Mon Sep 17 00:00:00 2001 From: Josh Holtrop Date: Thu, 14 May 2026 15:46:38 -0400 Subject: [PATCH] Check SNI/ALPN in TLS 1.2 stateful session ID resumption --- src/internal.c | 65 +++++++++++++++++++++++----- src/ssl_sess.c | 57 ++++++++++++++++++++++++ tests/api/test_tls.c | 100 +++++++++++++++++++++++++++++++++++++++++++ tests/api/test_tls.h | 2 + wolfssl/internal.h | 12 ++++++ 5 files changed, 225 insertions(+), 11 deletions(-) diff --git a/src/internal.c b/src/internal.c index f19d8fa10d6..dce8a8113a2 100644 --- a/src/internal.c +++ b/src/internal.c @@ -38153,6 +38153,38 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) ssl->options.resuming = 0; return ret; } +#if defined(HAVE_SESSION_TICKET) && \ + (defined(HAVE_SNI) || defined(HAVE_ALPN)) + /* RFC 6066 section 3: a server MUST NOT resume the session if the + * server_name in the current ClientHello differs from the one that + * was bound to the session. VerifyTicketBinding() covers the + * ticket path early in DoClientHello, but the session-ID cache + * path does not load the session until now - so the binding check + * must be applied here, after wolfSSL_GetSession(). On mismatch + * fall back to a full handshake. Gated on HAVE_SESSION_TICKET + * because session->sniHash/alpnHash and ssl->options.useTicket + * only exist in that configuration. */ + if (!ssl->options.useTicket) { + byte curHash[TICKET_BINDING_HASH_SZ]; +#ifdef HAVE_SNI + if (TicketSniHash(ssl, curHash) != 0 || + XMEMCMP(curHash, session->sniHash, + TICKET_BINDING_HASH_SZ) != 0) { + WOLFSSL_MSG("Resumed session SNI mismatch, full handshake"); + ssl->options.resuming = 0; + } +#endif +#ifdef HAVE_ALPN + if (ssl->options.resuming && + (TicketAlpnHash(ssl, curHash) != 0 || + XMEMCMP(curHash, session->alpnHash, + TICKET_BINDING_HASH_SZ) != 0)) { + WOLFSSL_MSG("Resumed session ALPN mismatch, full handshake"); + ssl->options.resuming = 0; + } +#endif + } +#endif /* HAVE_SESSION_TICKET && (HAVE_SNI || HAVE_ALPN) */ #if !defined(WOLFSSL_NO_TICKET_EXPIRE) && !defined(NO_ASN_TIME) /* check if the ticket is valid */ if (LowResTimer() > session->bornOn + ssl->timeout) { @@ -39520,7 +39552,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) #ifdef HAVE_SNI /* Hash server-selected SNI; zeros dst when none. */ - static int TicketSniHash(WOLFSSL* ssl, byte* dst) + int TicketSniHash(WOLFSSL* ssl, byte* dst) { char* name = NULL; word16 nameLen; @@ -39539,17 +39571,28 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) #endif #ifdef HAVE_ALPN - /* Hash negotiated ALPN; zeros dst when none. */ - static int TicketAlpnHash(WOLFSSL* ssl, byte* dst) + /* Hash negotiated ALPN; zeros dst when none. Peeks at the extension + * directly rather than going through TLSX_ALPN_GetRequest(), which + * pushes WOLFSSL_ALPN_NOT_FOUND onto the error queue when no ALPN is + * present - undesirable for a silent binding probe called on every + * session setup. */ + int TicketAlpnHash(WOLFSSL* ssl, byte* dst) { - char* proto = NULL; - word16 protoLen = 0; - - if (TLSX_ALPN_GetRequest(ssl->extensions, (void**)&proto, - &protoLen) == WOLFSSL_SUCCESS && - proto != NULL && protoLen > 0) { - return wc_Hash(TICKET_BINDING_HASH_TYPE, (const byte*)proto, - protoLen, dst, TICKET_BINDING_HASH_SZ); + TLSX* extension; + ALPN* alpn; + + extension = TLSX_Find(ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL); + if (extension != NULL) { + alpn = (ALPN*)extension->data; + if (alpn != NULL && alpn->negotiated == 1 && + alpn->protocol_name != NULL) { + word32 protoLen = (word32)XSTRLEN(alpn->protocol_name); + if (protoLen > 0) { + return wc_Hash(TICKET_BINDING_HASH_TYPE, + (const byte*)alpn->protocol_name, + protoLen, dst, TICKET_BINDING_HASH_SZ); + } + } } XMEMSET(dst, 0, TICKET_BINDING_HASH_SZ); diff --git a/src/ssl_sess.c b/src/ssl_sess.c index 1a6a6b5f357..b7ac3f55add 100644 --- a/src/ssl_sess.c +++ b/src/ssl_sess.c @@ -2698,6 +2698,16 @@ int wolfSSL_i2d_SSL_SESSION(WOLFSSL_SESSION* sess, unsigned char** p) #ifdef HAVE_SESSION_TICKET /* ticket len | ticket */ size += OPAQUE16_LEN + sess->ticketLen; +#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS) +#ifdef HAVE_SNI + /* sniHash */ + size += TICKET_BINDING_HASH_SZ; +#endif +#ifdef HAVE_ALPN + /* alpnHash */ + size += TICKET_BINDING_HASH_SZ; +#endif +#endif /* !NO_WOLFSSL_SERVER && !NO_TLS */ #endif if (p != NULL) { @@ -2783,6 +2793,16 @@ int wolfSSL_i2d_SSL_SESSION(WOLFSSL_SESSION* sess, unsigned char** p) c16toa(sess->ticketLen, data + idx); idx += OPAQUE16_LEN; XMEMCPY(data + idx, sess->ticket, sess->ticketLen); idx += sess->ticketLen; +#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS) +#ifdef HAVE_SNI + XMEMCPY(data + idx, sess->sniHash, TICKET_BINDING_HASH_SZ); + idx += TICKET_BINDING_HASH_SZ; +#endif +#ifdef HAVE_ALPN + XMEMCPY(data + idx, sess->alpnHash, TICKET_BINDING_HASH_SZ); + idx += TICKET_BINDING_HASH_SZ; +#endif +#endif /* !NO_WOLFSSL_SERVER && !NO_TLS */ #endif } #endif @@ -3069,6 +3089,26 @@ WOLFSSL_SESSION* wolfSSL_d2i_SSL_SESSION(WOLFSSL_SESSION** sess, goto end; } XMEMCPY(s->ticket, data + idx, s->ticketLen); idx += s->ticketLen; +#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS) +#ifdef HAVE_SNI + /* sniHash - SNI binding for stateful resumption (RFC 6066 section 3) */ + if (i - idx < TICKET_BINDING_HASH_SZ) { + ret = BUFFER_ERROR; + goto end; + } + XMEMCPY(s->sniHash, data + idx, TICKET_BINDING_HASH_SZ); + idx += TICKET_BINDING_HASH_SZ; +#endif +#ifdef HAVE_ALPN + /* alpnHash - ALPN binding for stateful resumption */ + if (i - idx < TICKET_BINDING_HASH_SZ) { + ret = BUFFER_ERROR; + goto end; + } + XMEMCPY(s->alpnHash, data + idx, TICKET_BINDING_HASH_SZ); + idx += TICKET_BINDING_HASH_SZ; +#endif +#endif /* !NO_WOLFSSL_SERVER && !NO_TLS */ #endif (void)idx; @@ -3647,6 +3687,23 @@ void SetupSession(WOLFSSL* ssl) session->sessionCtxSz = ssl->sessionCtxSz; } #endif +#if defined(HAVE_SESSION_TICKET) && \ + !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS) + /* Bind the current SNI/ALPN to the session so that a later resumption + * via the stateful session-ID cache (no ticket) can be rejected when + * the resumed ClientHello carries a different SNI/ALPN. RFC 6066 section 3 + * requires "MUST NOT accept the request to resume the session if the + * server_name extension contains a different name". CreateTicket() + * overwrites these with the same values for the ticket path. The + * NO_WOLFSSL_SERVER/NO_TLS gates match the gates around TicketSniHash / + * TicketAlpnHash in internal.c. */ +#ifdef HAVE_SNI + (void)TicketSniHash(ssl, session->sniHash); +#endif +#ifdef HAVE_ALPN + (void)TicketAlpnHash(ssl, session->alpnHash); +#endif +#endif /* HAVE_SESSION_TICKET && !NO_WOLFSSL_SERVER && !NO_TLS */ session->timeout = ssl->timeout; #ifndef NO_ASN_TIME session->bornOn = LowResTimer(); diff --git a/tests/api/test_tls.c b/tests/api/test_tls.c index aedae4f7031..ea59a393656 100644 --- a/tests/api/test_tls.c +++ b/tests/api/test_tls.c @@ -861,6 +861,106 @@ int test_tls12_etm_failed_resumption(void) return EXPECT_RESULT(); } +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(HAVE_SNI) && \ + defined(HAVE_SESSION_TICKET) && !defined(NO_SESSION_CACHE) +/* Accept-all SNI callback used by test_tls12_session_id_resumption_sni_mismatch. + * Registering any sniRecvCb causes the server to keep the client-provided + * SNI in ssl->extensions (see TLSX_SNI_Parse, "Forcing SSL object to store + * SNI parameter"), which is what the binding code reads. */ +static int accept_any_sni_cb(WOLFSSL* ssl, int* ret, void* arg) +{ + (void)ssl; (void)ret; (void)arg; + return 0; /* accept */ +} +#endif + +/* RFC 6066 Section 3 requires: + * "A server that implements this extension MUST NOT accept the request to + * resume the session if the server_name extension contains a different + * name. Instead, it proceeds with a full handshake to establish a new + * session." + * + * wolfSSL's SNI/ALPN ticket-binding hardening (see VerifyTicketBinding, + * added in PR #10279) covers the session ticket path but short-circuits on + * !ssl->options.useTicket, so it does not apply to the TLS 1.2 stateful + * session-ID cache resumption path. SetupSession() does not store the + * original SNI on the cached WOLFSSL_SESSION, and TlsSessionCacheGetAndLock() + * keys only on (sessionID, sessionIDSz, side). The result is that a session + * established under one SNI can be resumed under a different SNI via the + * session-ID cache, in violation of the MUST NOT above. + * + * This test forces the session-ID resumption path (no tickets) and offers a + * different SNI on the resumption attempt. The server must NOT resume. */ +int test_tls12_session_id_resumption_sni_mismatch(void) +{ + EXPECT_DECLS; +#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \ + !defined(WOLFSSL_NO_TLS12) && defined(HAVE_SNI) && \ + defined(HAVE_SESSION_TICKET) && !defined(NO_SESSION_CACHE) + WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL; + WOLFSSL *ssl_c = NULL, *ssl_s = NULL; + WOLFSSL_SESSION *sess = NULL; + struct test_memio_ctx test_ctx; + const char* sniA = "public.example"; + const char* sniB = "admin.example"; + + /* Step 1: full TLS 1.2 handshake under SNI=public.example, with the + * session ticket path disabled so resumption can only happen via the + * server's session-ID cache. The server-side SNI callback ensures + * ssl->extensions retains the client's SNI in builds that don't + * compile in WOLFSSL_ALWAYS_KEEP_SNI. */ + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s, + wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0); + wolfSSL_CTX_set_servername_callback(ctx_s, accept_any_sni_cb); + ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_s), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, + sniA, (word16)XSTRLEN(sniA)), WOLFSSL_SUCCESS); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + /* Sanity: the first handshake was not a resumption. */ + ExpectIntEQ(wolfSSL_session_reused(ssl_s), 0); + ExpectNotNull(sess = wolfSSL_get1_session(ssl_c)); + + wolfSSL_free(ssl_c); ssl_c = NULL; + wolfSSL_free(ssl_s); ssl_s = NULL; + + /* Step 2: new SSL objects on the SAME WOLFSSL_CTX (so the server's + * session cache still holds the entry from step 1). The client offers + * the saved session but advertises a *different* SNI. The server's + * cache lookup will match by session ID, but per RFC 6066 Section 3 the + * server MUST NOT resume because the SNI differs from the original. */ + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + ExpectNotNull(ssl_c = wolfSSL_new(ctx_c)); + wolfSSL_SetIOReadCtx(ssl_c, &test_ctx); + wolfSSL_SetIOWriteCtx(ssl_c, &test_ctx); + ExpectNotNull(ssl_s = wolfSSL_new(ctx_s)); + wolfSSL_SetIOReadCtx(ssl_s, &test_ctx); + wolfSSL_SetIOWriteCtx(ssl_s, &test_ctx); + ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_s), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME, + sniB, (word16)XSTRLEN(sniB)), WOLFSSL_SUCCESS); + ExpectIntEQ(wolfSSL_set_session(ssl_c, sess), WOLFSSL_SUCCESS); + ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0); + + /* Post-fix expected behavior: server falls back to a full handshake + * because the SNI in the ClientHello does not match the SNI bound to + * the cached session. Pre-fix, the server silently resumes - which is + * the bug. Both sides should report no resumption. */ + ExpectIntEQ(wolfSSL_session_reused(ssl_s), 0); + ExpectIntEQ(wolfSSL_session_reused(ssl_c), 0); + + wolfSSL_SESSION_free(sess); + wolfSSL_free(ssl_c); + wolfSSL_free(ssl_s); + wolfSSL_CTX_free(ctx_c); + wolfSSL_CTX_free(ctx_s); +#endif + return EXPECT_RESULT(); +} + int test_tls_set_curves_list_ecc_fallback(void) { EXPECT_DECLS; diff --git a/tests/api/test_tls.h b/tests/api/test_tls.h index c0f74f21505..86db576150f 100644 --- a/tests/api/test_tls.h +++ b/tests/api/test_tls.h @@ -32,6 +32,7 @@ int test_tls_certreq_order(void); int test_tls12_bad_cv_sig_alg(void); int test_tls12_no_null_compression(void); int test_tls12_etm_failed_resumption(void); +int test_tls12_session_id_resumption_sni_mismatch(void); int test_tls_set_curves_list_ecc_fallback(void); int test_tls12_corrupted_finished(void); int test_tls12_peerauth_failsafe(void); @@ -47,6 +48,7 @@ int test_tls12_peerauth_failsafe(void); TEST_DECL_GROUP("tls", test_tls12_bad_cv_sig_alg), \ TEST_DECL_GROUP("tls", test_tls12_no_null_compression), \ TEST_DECL_GROUP("tls", test_tls12_etm_failed_resumption), \ + TEST_DECL_GROUP("tls", test_tls12_session_id_resumption_sni_mismatch), \ TEST_DECL_GROUP("tls", test_tls_set_curves_list_ecc_fallback), \ TEST_DECL_GROUP("tls", test_tls12_corrupted_finished), \ TEST_DECL_GROUP("tls", test_tls12_peerauth_failsafe) diff --git a/wolfssl/internal.h b/wolfssl/internal.h index a2da2e6de61..06d279267d3 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -6821,9 +6821,21 @@ WOLFSSL_LOCAL int DoClientTicket_ex(const WOLFSSL* ssl, PreSharedKey* psk, #endif WOLFSSL_LOCAL int DoClientTicket(WOLFSSL* ssl, const byte* input, word32 len); +/* TicketSniHash, TicketAlpnHash, and VerifyTicketBinding are defined in + * internal.c only when !NO_WOLFSSL_SERVER && !NO_TLS - gate the + * declarations to match so client-only or no-TLS builds don't compile in + * call sites that would fail to link. */ +#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS) +#ifdef HAVE_SNI +WOLFSSL_LOCAL int TicketSniHash(WOLFSSL* ssl, byte* dst); +#endif +#ifdef HAVE_ALPN +WOLFSSL_LOCAL int TicketAlpnHash(WOLFSSL* ssl, byte* dst); +#endif #if defined(HAVE_SNI) || defined(HAVE_ALPN) WOLFSSL_LOCAL int VerifyTicketBinding(WOLFSSL* ssl); #endif +#endif /* !NO_WOLFSSL_SERVER && !NO_TLS */ #endif /* HAVE_SESSION_TICKET */ WOLFSSL_LOCAL int SendData(WOLFSSL* ssl, const void* data, size_t sz); #ifdef WOLFSSL_THREADED_CRYPT