Skip to content

Commit 9a67c0d

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

5 files changed

Lines changed: 225 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: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2698,6 +2698,16 @@ int wolfSSL_i2d_SSL_SESSION(WOLFSSL_SESSION* sess, unsigned char** p)
26982698
#ifdef HAVE_SESSION_TICKET
26992699
/* ticket len | ticket */
27002700
size += OPAQUE16_LEN + sess->ticketLen;
2701+
#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS)
2702+
#ifdef HAVE_SNI
2703+
/* sniHash */
2704+
size += TICKET_BINDING_HASH_SZ;
2705+
#endif
2706+
#ifdef HAVE_ALPN
2707+
/* alpnHash */
2708+
size += TICKET_BINDING_HASH_SZ;
2709+
#endif
2710+
#endif /* !NO_WOLFSSL_SERVER && !NO_TLS */
27012711
#endif
27022712

27032713
if (p != NULL) {
@@ -2783,6 +2793,16 @@ int wolfSSL_i2d_SSL_SESSION(WOLFSSL_SESSION* sess, unsigned char** p)
27832793
c16toa(sess->ticketLen, data + idx); idx += OPAQUE16_LEN;
27842794
XMEMCPY(data + idx, sess->ticket, sess->ticketLen);
27852795
idx += sess->ticketLen;
2796+
#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS)
2797+
#ifdef HAVE_SNI
2798+
XMEMCPY(data + idx, sess->sniHash, TICKET_BINDING_HASH_SZ);
2799+
idx += TICKET_BINDING_HASH_SZ;
2800+
#endif
2801+
#ifdef HAVE_ALPN
2802+
XMEMCPY(data + idx, sess->alpnHash, TICKET_BINDING_HASH_SZ);
2803+
idx += TICKET_BINDING_HASH_SZ;
2804+
#endif
2805+
#endif /* !NO_WOLFSSL_SERVER && !NO_TLS */
27862806
#endif
27872807
}
27882808
#endif
@@ -3069,6 +3089,26 @@ WOLFSSL_SESSION* wolfSSL_d2i_SSL_SESSION(WOLFSSL_SESSION** sess,
30693089
goto end;
30703090
}
30713091
XMEMCPY(s->ticket, data + idx, s->ticketLen); idx += s->ticketLen;
3092+
#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS)
3093+
#ifdef HAVE_SNI
3094+
/* sniHash - SNI binding for stateful resumption (RFC 6066 section 3) */
3095+
if (i - idx < TICKET_BINDING_HASH_SZ) {
3096+
ret = BUFFER_ERROR;
3097+
goto end;
3098+
}
3099+
XMEMCPY(s->sniHash, data + idx, TICKET_BINDING_HASH_SZ);
3100+
idx += TICKET_BINDING_HASH_SZ;
3101+
#endif
3102+
#ifdef HAVE_ALPN
3103+
/* alpnHash - ALPN binding for stateful resumption */
3104+
if (i - idx < TICKET_BINDING_HASH_SZ) {
3105+
ret = BUFFER_ERROR;
3106+
goto end;
3107+
}
3108+
XMEMCPY(s->alpnHash, data + idx, TICKET_BINDING_HASH_SZ);
3109+
idx += TICKET_BINDING_HASH_SZ;
3110+
#endif
3111+
#endif /* !NO_WOLFSSL_SERVER && !NO_TLS */
30723112
#endif
30733113
(void)idx;
30743114

@@ -3647,6 +3687,23 @@ void SetupSession(WOLFSSL* ssl)
36473687
session->sessionCtxSz = ssl->sessionCtxSz;
36483688
}
36493689
#endif
3690+
#if defined(HAVE_SESSION_TICKET) && \
3691+
!defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS)
3692+
/* Bind the current SNI/ALPN to the session so that a later resumption
3693+
* via the stateful session-ID cache (no ticket) can be rejected when
3694+
* the resumed ClientHello carries a different SNI/ALPN. RFC 6066 section 3
3695+
* requires "MUST NOT accept the request to resume the session if the
3696+
* server_name extension contains a different name". CreateTicket()
3697+
* overwrites these with the same values for the ticket path. The
3698+
* NO_WOLFSSL_SERVER/NO_TLS gates match the gates around TicketSniHash /
3699+
* TicketAlpnHash in internal.c. */
3700+
#ifdef HAVE_SNI
3701+
(void)TicketSniHash(ssl, session->sniHash);
3702+
#endif
3703+
#ifdef HAVE_ALPN
3704+
(void)TicketAlpnHash(ssl, session->alpnHash);
3705+
#endif
3706+
#endif /* HAVE_SESSION_TICKET && !NO_WOLFSSL_SERVER && !NO_TLS */
36503707
session->timeout = ssl->timeout;
36513708
#ifndef NO_ASN_TIME
36523709
session->bornOn = LowResTimer();

tests/api/test_tls.c

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

864+
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
865+
!defined(WOLFSSL_NO_TLS12) && defined(HAVE_SNI) && \
866+
defined(HAVE_SESSION_TICKET) && !defined(NO_SESSION_CACHE)
867+
/* Accept-all SNI callback used by test_tls12_session_id_resumption_sni_mismatch.
868+
* Registering any sniRecvCb causes the server to keep the client-provided
869+
* SNI in ssl->extensions (see TLSX_SNI_Parse, "Forcing SSL object to store
870+
* SNI parameter"), which is what the binding code reads. */
871+
static int accept_any_sni_cb(WOLFSSL* ssl, int* ret, void* arg)
872+
{
873+
(void)ssl; (void)ret; (void)arg;
874+
return 0; /* accept */
875+
}
876+
#endif
877+
878+
/* RFC 6066 Section 3 requires:
879+
* "A server that implements this extension MUST NOT accept the request to
880+
* resume the session if the server_name extension contains a different
881+
* name. Instead, it proceeds with a full handshake to establish a new
882+
* session."
883+
*
884+
* wolfSSL's SNI/ALPN ticket-binding hardening (see VerifyTicketBinding,
885+
* added in PR #10279) covers the session ticket path but short-circuits on
886+
* !ssl->options.useTicket, so it does not apply to the TLS 1.2 stateful
887+
* session-ID cache resumption path. SetupSession() does not store the
888+
* original SNI on the cached WOLFSSL_SESSION, and TlsSessionCacheGetAndLock()
889+
* keys only on (sessionID, sessionIDSz, side). The result is that a session
890+
* established under one SNI can be resumed under a different SNI via the
891+
* session-ID cache, in violation of the MUST NOT above.
892+
*
893+
* This test forces the session-ID resumption path (no tickets) and offers a
894+
* different SNI on the resumption attempt. The server must NOT resume. */
895+
int test_tls12_session_id_resumption_sni_mismatch(void)
896+
{
897+
EXPECT_DECLS;
898+
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
899+
!defined(WOLFSSL_NO_TLS12) && defined(HAVE_SNI) && \
900+
defined(HAVE_SESSION_TICKET) && !defined(NO_SESSION_CACHE)
901+
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
902+
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
903+
WOLFSSL_SESSION *sess = NULL;
904+
struct test_memio_ctx test_ctx;
905+
const char* sniA = "public.example";
906+
const char* sniB = "admin.example";
907+
908+
/* Step 1: full TLS 1.2 handshake under SNI=public.example, with the
909+
* session ticket path disabled so resumption can only happen via the
910+
* server's session-ID cache. The server-side SNI callback ensures
911+
* ssl->extensions retains the client's SNI in builds that don't
912+
* compile in WOLFSSL_ALWAYS_KEEP_SNI. */
913+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
914+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
915+
wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0);
916+
wolfSSL_CTX_set_servername_callback(ctx_s, accept_any_sni_cb);
917+
ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS);
918+
ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_s), WOLFSSL_SUCCESS);
919+
ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME,
920+
sniA, (word16)XSTRLEN(sniA)), WOLFSSL_SUCCESS);
921+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
922+
/* Sanity: the first handshake was not a resumption. */
923+
ExpectIntEQ(wolfSSL_session_reused(ssl_s), 0);
924+
ExpectNotNull(sess = wolfSSL_get1_session(ssl_c));
925+
926+
wolfSSL_free(ssl_c); ssl_c = NULL;
927+
wolfSSL_free(ssl_s); ssl_s = NULL;
928+
929+
/* Step 2: new SSL objects on the SAME WOLFSSL_CTX (so the server's
930+
* session cache still holds the entry from step 1). The client offers
931+
* the saved session but advertises a *different* SNI. The server's
932+
* cache lookup will match by session ID, but per RFC 6066 Section 3 the
933+
* server MUST NOT resume because the SNI differs from the original. */
934+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
935+
ExpectNotNull(ssl_c = wolfSSL_new(ctx_c));
936+
wolfSSL_SetIOReadCtx(ssl_c, &test_ctx);
937+
wolfSSL_SetIOWriteCtx(ssl_c, &test_ctx);
938+
ExpectNotNull(ssl_s = wolfSSL_new(ctx_s));
939+
wolfSSL_SetIOReadCtx(ssl_s, &test_ctx);
940+
wolfSSL_SetIOWriteCtx(ssl_s, &test_ctx);
941+
ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_c), WOLFSSL_SUCCESS);
942+
ExpectIntEQ(wolfSSL_NoTicketTLSv12(ssl_s), WOLFSSL_SUCCESS);
943+
ExpectIntEQ(wolfSSL_UseSNI(ssl_c, WOLFSSL_SNI_HOST_NAME,
944+
sniB, (word16)XSTRLEN(sniB)), WOLFSSL_SUCCESS);
945+
ExpectIntEQ(wolfSSL_set_session(ssl_c, sess), WOLFSSL_SUCCESS);
946+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
947+
948+
/* Post-fix expected behavior: server falls back to a full handshake
949+
* because the SNI in the ClientHello does not match the SNI bound to
950+
* the cached session. Pre-fix, the server silently resumes - which is
951+
* the bug. Both sides should report no resumption. */
952+
ExpectIntEQ(wolfSSL_session_reused(ssl_s), 0);
953+
ExpectIntEQ(wolfSSL_session_reused(ssl_c), 0);
954+
955+
wolfSSL_SESSION_free(sess);
956+
wolfSSL_free(ssl_c);
957+
wolfSSL_free(ssl_s);
958+
wolfSSL_CTX_free(ctx_c);
959+
wolfSSL_CTX_free(ctx_s);
960+
#endif
961+
return EXPECT_RESULT();
962+
}
963+
864964
int test_tls_set_curves_list_ecc_fallback(void)
865965
{
866966
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: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6821,9 +6821,21 @@ 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+
/* TicketSniHash, TicketAlpnHash, and VerifyTicketBinding are defined in
6825+
* internal.c only when !NO_WOLFSSL_SERVER && !NO_TLS - gate the
6826+
* declarations to match so client-only or no-TLS builds don't compile in
6827+
* call sites that would fail to link. */
6828+
#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS)
6829+
#ifdef HAVE_SNI
6830+
WOLFSSL_LOCAL int TicketSniHash(WOLFSSL* ssl, byte* dst);
6831+
#endif
6832+
#ifdef HAVE_ALPN
6833+
WOLFSSL_LOCAL int TicketAlpnHash(WOLFSSL* ssl, byte* dst);
6834+
#endif
68246835
#if defined(HAVE_SNI) || defined(HAVE_ALPN)
68256836
WOLFSSL_LOCAL int VerifyTicketBinding(WOLFSSL* ssl);
68266837
#endif
6838+
#endif /* !NO_WOLFSSL_SERVER && !NO_TLS */
68276839
#endif /* HAVE_SESSION_TICKET */
68286840
WOLFSSL_LOCAL int SendData(WOLFSSL* ssl, const void* data, size_t sz);
68296841
#ifdef WOLFSSL_THREADED_CRYPT

0 commit comments

Comments
 (0)