Skip to content

Commit a66c422

Browse files
Check SNI/ALPN in TLS 1.2 stateful session ID resumption
1 parent 460a871 commit a66c422

5 files changed

Lines changed: 159 additions & 11 deletions

File tree

src/internal.c

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38153,6 +38153,38 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3815338153
ssl->options.resuming = 0;
3815438154
return ret;
3815538155
}
38156+
#if defined(HAVE_SESSION_TICKET) && \
38157+
(defined(HAVE_SNI) || defined(HAVE_ALPN))
38158+
/* RFC 6066 section 3: a server MUST NOT resume the session if the
38159+
* server_name in the current ClientHello differs from the one that
38160+
* was bound to the session. VerifyTicketBinding() covers the
38161+
* ticket path early in DoClientHello, but the session-ID cache
38162+
* path does not load the session until now - so the binding check
38163+
* must be applied here, after wolfSSL_GetSession(). On mismatch
38164+
* fall back to a full handshake. Gated on HAVE_SESSION_TICKET
38165+
* because session->sniHash/alpnHash and ssl->options.useTicket
38166+
* only exist in that configuration. */
38167+
if (!ssl->options.useTicket) {
38168+
byte curHash[TICKET_BINDING_HASH_SZ];
38169+
#ifdef HAVE_SNI
38170+
if (TicketSniHash(ssl, curHash) != 0 ||
38171+
XMEMCMP(curHash, session->sniHash,
38172+
TICKET_BINDING_HASH_SZ) != 0) {
38173+
WOLFSSL_MSG("Resumed session SNI mismatch, full handshake");
38174+
ssl->options.resuming = 0;
38175+
}
38176+
#endif
38177+
#ifdef HAVE_ALPN
38178+
if (ssl->options.resuming &&
38179+
(TicketAlpnHash(ssl, curHash) != 0 ||
38180+
XMEMCMP(curHash, session->alpnHash,
38181+
TICKET_BINDING_HASH_SZ) != 0)) {
38182+
WOLFSSL_MSG("Resumed session ALPN mismatch, full handshake");
38183+
ssl->options.resuming = 0;
38184+
}
38185+
#endif
38186+
}
38187+
#endif /* HAVE_SESSION_TICKET && (HAVE_SNI || HAVE_ALPN) */
3815638188
#if !defined(WOLFSSL_NO_TICKET_EXPIRE) && !defined(NO_ASN_TIME)
3815738189
/* check if the ticket is valid */
3815838190
if (LowResTimer() > session->bornOn + ssl->timeout) {
@@ -39520,7 +39552,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3952039552

3952139553
#ifdef HAVE_SNI
3952239554
/* Hash server-selected SNI; zeros dst when none. */
39523-
static int TicketSniHash(WOLFSSL* ssl, byte* dst)
39555+
int TicketSniHash(WOLFSSL* ssl, byte* dst)
3952439556
{
3952539557
char* name = NULL;
3952639558
word16 nameLen;
@@ -39539,17 +39571,28 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3953939571
#endif
3954039572

3954139573
#ifdef HAVE_ALPN
39542-
/* Hash negotiated ALPN; zeros dst when none. */
39543-
static int TicketAlpnHash(WOLFSSL* ssl, byte* dst)
39574+
/* Hash negotiated ALPN; zeros dst when none. Peeks at the extension
39575+
* directly rather than going through TLSX_ALPN_GetRequest(), which
39576+
* pushes WOLFSSL_ALPN_NOT_FOUND onto the error queue when no ALPN is
39577+
* present - undesirable for a silent binding probe called on every
39578+
* session setup. */
39579+
int TicketAlpnHash(WOLFSSL* ssl, byte* dst)
3954439580
{
39545-
char* proto = NULL;
39546-
word16 protoLen = 0;
39547-
39548-
if (TLSX_ALPN_GetRequest(ssl->extensions, (void**)&proto,
39549-
&protoLen) == WOLFSSL_SUCCESS &&
39550-
proto != NULL && protoLen > 0) {
39551-
return wc_Hash(TICKET_BINDING_HASH_TYPE, (const byte*)proto,
39552-
protoLen, dst, TICKET_BINDING_HASH_SZ);
39581+
TLSX* extension;
39582+
ALPN* alpn;
39583+
39584+
extension = TLSX_Find(ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL);
39585+
if (extension != NULL) {
39586+
alpn = (ALPN*)extension->data;
39587+
if (alpn != NULL && alpn->negotiated == 1 &&
39588+
alpn->protocol_name != NULL) {
39589+
word32 protoLen = (word32)XSTRLEN(alpn->protocol_name);
39590+
if (protoLen > 0) {
39591+
return wc_Hash(TICKET_BINDING_HASH_TYPE,
39592+
(const byte*)alpn->protocol_name,
39593+
protoLen, dst, TICKET_BINDING_HASH_SZ);
39594+
}
39595+
}
3955339596
}
3955439597

3955539598
XMEMSET(dst, 0, TICKET_BINDING_HASH_SZ);

src/ssl_sess.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3647,6 +3647,20 @@ void SetupSession(WOLFSSL* ssl)
36473647
session->sessionCtxSz = ssl->sessionCtxSz;
36483648
}
36493649
#endif
3650+
#ifdef HAVE_SESSION_TICKET
3651+
/* Bind the current SNI/ALPN to the session so that a later resumption
3652+
* via the stateful session-ID cache (no ticket) can be rejected when
3653+
* the resumed ClientHello carries a different SNI/ALPN. RFC 6066 section 3
3654+
* requires "MUST NOT accept the request to resume the session if the
3655+
* server_name extension contains a different name". CreateTicket()
3656+
* overwrites these with the same values for the ticket path. */
3657+
#ifdef HAVE_SNI
3658+
(void)TicketSniHash(ssl, session->sniHash);
3659+
#endif
3660+
#ifdef HAVE_ALPN
3661+
(void)TicketAlpnHash(ssl, session->alpnHash);
3662+
#endif
3663+
#endif /* HAVE_SESSION_TICKET */
36503664
session->timeout = ssl->timeout;
36513665
#ifndef NO_ASN_TIME
36523666
session->bornOn = LowResTimer();

tests/api/test_tls.c

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,89 @@ int test_tls12_etm_failed_resumption(void)
861861
return EXPECT_RESULT();
862862
}
863863

864+
/* RFC 6066 Section 3 requires:
865+
* "A server that implements this extension MUST NOT accept the request to
866+
* resume the session if the server_name extension contains a different
867+
* name. Instead, it proceeds with a full handshake to establish a new
868+
* session."
869+
*
870+
* wolfSSL's SNI/ALPN ticket-binding hardening (see VerifyTicketBinding,
871+
* added in PR #10279) covers the session ticket path but short-circuits on
872+
* !ssl->options.useTicket, so it does not apply to the TLS 1.2 stateful
873+
* session-ID cache resumption path. SetupSession() does not store the
874+
* original SNI on the cached WOLFSSL_SESSION, and TlsSessionCacheGetAndLock()
875+
* keys only on (sessionID, sessionIDSz, side). The result is that a session
876+
* established under one SNI can be resumed under a different SNI via the
877+
* session-ID cache, in violation of the MUST NOT above.
878+
*
879+
* This test forces the session-ID resumption path (no tickets) and offers a
880+
* different SNI on the resumption attempt. The server must NOT resume. */
881+
int test_tls12_session_id_resumption_sni_mismatch(void)
882+
{
883+
EXPECT_DECLS;
884+
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
885+
!defined(WOLFSSL_NO_TLS12) && defined(HAVE_SNI) && \
886+
defined(HAVE_SESSION_TICKET) && !defined(NO_SESSION_CACHE)
887+
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
888+
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
889+
WOLFSSL_SESSION *sess = NULL;
890+
struct test_memio_ctx test_ctx;
891+
const char* sniA = "public.example";
892+
const char* sniB = "admin.example";
893+
894+
/* Step 1: full TLS 1.2 handshake under SNI=public.example, with the
895+
* session ticket path disabled so resumption can only happen via the
896+
* server's session-ID cache. */
897+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
898+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
899+
wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0);
900+
ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS);
901+
ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_s), WOLFSSL_SUCCESS);
902+
ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME,
903+
sniA, (word16)XSTRLEN(sniA)), WOLFSSL_SUCCESS);
904+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
905+
/* Sanity: the first handshake was not a resumption. */
906+
ExpectIntEQ(wolfSSL_session_reused(ssl_s), 0);
907+
ExpectNotNull(sess = wolfSSL_get1_session(ssl_c));
908+
909+
wolfSSL_free(ssl_c); ssl_c = NULL;
910+
wolfSSL_free(ssl_s); ssl_s = NULL;
911+
912+
/* Step 2: new SSL objects on the SAME WOLFSSL_CTX (so the server's
913+
* session cache still holds the entry from step 1). The client offers
914+
* the saved session but advertises a *different* SNI. The server's
915+
* cache lookup will match by session ID, but per RFC 6066 Section 3 the
916+
* server MUST NOT resume because the SNI differs from the original. */
917+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
918+
ExpectNotNull(ssl_c = wolfSSL_new(ctx_c));
919+
wolfSSL_SetIOReadCtx(ssl_c, &test_ctx);
920+
wolfSSL_SetIOWriteCtx(ssl_c, &test_ctx);
921+
ExpectNotNull(ssl_s = wolfSSL_new(ctx_s));
922+
wolfSSL_SetIOReadCtx(ssl_s, &test_ctx);
923+
wolfSSL_SetIOWriteCtx(ssl_s, &test_ctx);
924+
ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS);
925+
ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_s), WOLFSSL_SUCCESS);
926+
ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME,
927+
sniB, (word16)XSTRLEN(sniB)), WOLFSSL_SUCCESS);
928+
ExpectIntEQ(wolfSSL_set_session(ssl_c, sess), WOLFSSL_SUCCESS);
929+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
930+
931+
/* Post-fix expected behavior: server falls back to a full handshake
932+
* because the SNI in the ClientHello does not match the SNI bound to
933+
* the cached session. Pre-fix, the server silently resumes - which is
934+
* the bug. Both sides should report no resumption. */
935+
ExpectIntEQ(wolfSSL_session_reused(ssl_s), 0);
936+
ExpectIntEQ(wolfSSL_session_reused(ssl_c), 0);
937+
938+
wolfSSL_SESSION_free(sess);
939+
wolfSSL_free(ssl_c);
940+
wolfSSL_free(ssl_s);
941+
wolfSSL_CTX_free(ctx_c);
942+
wolfSSL_CTX_free(ctx_s);
943+
#endif
944+
return EXPECT_RESULT();
945+
}
946+
864947
int test_tls_set_curves_list_ecc_fallback(void)
865948
{
866949
EXPECT_DECLS;

tests/api/test_tls.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ int test_tls_certreq_order(void);
3232
int test_tls12_bad_cv_sig_alg(void);
3333
int test_tls12_no_null_compression(void);
3434
int test_tls12_etm_failed_resumption(void);
35+
int test_tls12_session_id_resumption_sni_mismatch(void);
3536
int test_tls_set_curves_list_ecc_fallback(void);
3637
int test_tls12_corrupted_finished(void);
3738
int test_tls12_peerauth_failsafe(void);
@@ -47,6 +48,7 @@ int test_tls12_peerauth_failsafe(void);
4748
TEST_DECL_GROUP("tls", test_tls12_bad_cv_sig_alg), \
4849
TEST_DECL_GROUP("tls", test_tls12_no_null_compression), \
4950
TEST_DECL_GROUP("tls", test_tls12_etm_failed_resumption), \
51+
TEST_DECL_GROUP("tls", test_tls12_session_id_resumption_sni_mismatch), \
5052
TEST_DECL_GROUP("tls", test_tls_set_curves_list_ecc_fallback), \
5153
TEST_DECL_GROUP("tls", test_tls12_corrupted_finished), \
5254
TEST_DECL_GROUP("tls", test_tls12_peerauth_failsafe)

wolfssl/internal.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6821,6 +6821,12 @@ WOLFSSL_LOCAL int DoClientTicket_ex(const WOLFSSL* ssl, PreSharedKey* psk,
68216821
#endif
68226822

68236823
WOLFSSL_LOCAL int DoClientTicket(WOLFSSL* ssl, const byte* input, word32 len);
6824+
#ifdef HAVE_SNI
6825+
WOLFSSL_LOCAL int TicketSniHash(WOLFSSL* ssl, byte* dst);
6826+
#endif
6827+
#ifdef HAVE_ALPN
6828+
WOLFSSL_LOCAL int TicketAlpnHash(WOLFSSL* ssl, byte* dst);
6829+
#endif
68246830
#if defined(HAVE_SNI) || defined(HAVE_ALPN)
68256831
WOLFSSL_LOCAL int VerifyTicketBinding(WOLFSSL* ssl);
68266832
#endif

0 commit comments

Comments
 (0)