Skip to content

Commit 764245a

Browse files
douzzerdouzzer
authored
Merge pull request #10489 from holtrop-wolfssl/zd21798
Check SNI/ALPN in TLS 1.2/1.3 session resumptions
2 parents d5560b0 + 492603f commit 764245a

6 files changed

Lines changed: 445 additions & 37 deletions

File tree

‎src/internal.c‎

Lines changed: 91 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -38173,6 +38173,32 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3817338173
ssl->options.resuming = 0;
3817438174
return ret;
3817538175
}
38176+
#if defined(HAVE_SESSION_TICKET) && \
38177+
(defined(HAVE_SNI) || defined(HAVE_ALPN))
38178+
/* Do not resume session if sniHash/alpnHash do not match. */
38179+
if (!ssl->options.useTicket) {
38180+
byte curHash[TICKET_BINDING_HASH_SZ];
38181+
#ifdef HAVE_SNI
38182+
if (TicketSniHash(ssl, curHash) != 0 ||
38183+
XMEMCMP(curHash, session->sniHash,
38184+
TICKET_BINDING_HASH_SZ) != 0) {
38185+
WOLFSSL_MSG("Resumed session SNI mismatch, full handshake");
38186+
ssl->options.resuming = 0;
38187+
return ret;
38188+
}
38189+
#endif
38190+
#ifdef HAVE_ALPN
38191+
if (ssl->options.resuming &&
38192+
(TicketAlpnHash(ssl, curHash) != 0 ||
38193+
XMEMCMP(curHash, session->alpnHash,
38194+
TICKET_BINDING_HASH_SZ) != 0)) {
38195+
WOLFSSL_MSG("Resumed session ALPN mismatch, full handshake");
38196+
ssl->options.resuming = 0;
38197+
return ret;
38198+
}
38199+
#endif
38200+
}
38201+
#endif /* HAVE_SESSION_TICKET && (HAVE_SNI || HAVE_ALPN) */
3817638202
#if !defined(WOLFSSL_NO_TICKET_EXPIRE) && !defined(NO_ASN_TIME)
3817738203
/* check if the ticket is valid */
3817838204
if (LowResTimer() > session->bornOn + ssl->timeout) {
@@ -38754,8 +38780,22 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3875438780
#endif
3875538781
#if defined(HAVE_SESSION_TICKET) && \
3875638782
(defined(HAVE_SNI) || defined(HAVE_ALPN))
38757-
if((ret=VerifyTicketBinding(ssl)))
38758-
goto out;
38783+
/* Only verify here for TLS 1.2 ticket-based resumption.
38784+
* For stateful (session-ID) resumption ssl->session is
38785+
* not loaded until HandleTlsResumption runs below, which
38786+
* performs its own binding check against the cached
38787+
* session. On mismatch decline the resumption (RFC 6066
38788+
* Section 3) but proceed with a full handshake; leave
38789+
* useTicket set so the server still issues a fresh
38790+
* ticket to the client. */
38791+
if (ssl->options.useTicket &&
38792+
VerifyTicketBinding(ssl) != 0) {
38793+
WOLFSSL_MSG("Ticket binding mismatch, "
38794+
"declining resumption and falling back "
38795+
"to full handshake");
38796+
ssl->options.resuming = 0;
38797+
ssl->options.peerAuthGood = 0;
38798+
}
3875938799
#endif
3876038800

3876138801
i += totalExtSz;
@@ -39537,7 +39577,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3953739577

3953839578
#ifdef HAVE_SNI
3953939579
/* Hash server-selected SNI; zeros dst when none. */
39540-
static int TicketSniHash(WOLFSSL* ssl, byte* dst)
39580+
int TicketSniHash(WOLFSSL* ssl, byte* dst)
3954139581
{
3954239582
char* name = NULL;
3954339583
word16 nameLen;
@@ -39557,16 +39597,23 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3955739597

3955839598
#ifdef HAVE_ALPN
3955939599
/* Hash negotiated ALPN; zeros dst when none. */
39560-
static int TicketAlpnHash(WOLFSSL* ssl, byte* dst)
39600+
int TicketAlpnHash(WOLFSSL* ssl, byte* dst)
3956139601
{
39562-
char* proto = NULL;
39563-
word16 protoLen = 0;
39602+
TLSX* extension;
39603+
ALPN* alpn;
3956439604

39565-
if (TLSX_ALPN_GetRequest(ssl->extensions, (void**)&proto,
39566-
&protoLen) == WOLFSSL_SUCCESS &&
39567-
proto != NULL && protoLen > 0) {
39568-
return wc_Hash(TICKET_BINDING_HASH_TYPE, (const byte*)proto,
39569-
protoLen, dst, TICKET_BINDING_HASH_SZ);
39605+
extension = TLSX_Find(ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL);
39606+
if (extension != NULL) {
39607+
alpn = (ALPN*)extension->data;
39608+
if (alpn != NULL && alpn->negotiated == 1 &&
39609+
alpn->protocol_name != NULL) {
39610+
word32 protoLen = (word32)XSTRLEN(alpn->protocol_name);
39611+
if (protoLen > 0) {
39612+
return wc_Hash(TICKET_BINDING_HASH_TYPE,
39613+
(const byte*)alpn->protocol_name,
39614+
protoLen, dst, TICKET_BINDING_HASH_SZ);
39615+
}
39616+
}
3957039617
}
3957139618

3957239619
XMEMSET(dst, 0, TICKET_BINDING_HASH_SZ);
@@ -39575,15 +39622,30 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3957539622
#endif
3957639623

3957739624
#if defined(HAVE_SNI) || defined(HAVE_ALPN)
39578-
/* Server-side: verify the SNI/ALPN bindings carried on a resumed
39579-
* session match what was negotiated for the current connection.
39580-
* Must be called after extension parsing and ALPN_Select.
39581-
* Returns 0 on match, WOLFSSL_FATAL_ERROR on mismatch. */
39625+
/* Server-side TLS 1.2 ticket-resumption binding check. Confirms the
39626+
* SNI/ALPN bound to the resumed session matches what was negotiated
39627+
* for the current connection. Must be called after extension
39628+
* parsing and ALPN_Select so the negotiated values are available,
39629+
* and only once DoClientTicketFinalize has populated
39630+
* ssl->session->sniHash/alpnHash from the decrypted ticket.
39631+
*
39632+
* Other resumption paths handle the same check themselves and do
39633+
* not use this function:
39634+
* - TLS 1.2 session-ID (stateful): HandleTlsResumption compares
39635+
* against the cached session at lookup time.
39636+
* - TLS 1.3 PSK: DoPreSharedKeys compares against each candidate
39637+
* ticket's bound hashes before committing, allowing the server
39638+
* to skip mismatching PSKs and pick the next one.
39639+
*
39640+
* Returns 0 on match, WOLFSSL_FATAL_ERROR on mismatch. The caller
39641+
* is responsible for the policy on mismatch -- RFC 6066 Section 3
39642+
* mandates declining the resumption and proceeding with a full
39643+
* handshake rather than aborting. */
3958239644
int VerifyTicketBinding(WOLFSSL* ssl)
3958339645
{
3958439646
byte curHash[TICKET_BINDING_HASH_SZ];
3958539647

39586-
if (!ssl->options.resuming || !ssl->options.useTicket)
39648+
if (!ssl->options.resuming)
3958739649
return 0;
3958839650

3958939651
#ifdef HAVE_SNI
@@ -40092,8 +40154,9 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
4009240154
ssl->sessionCtxSz) != 0))
4009340155
return WOLFSSL_FATAL_ERROR;
4009440156
#endif
40095-
/* SNI/ALPN binding is verified after ALPN_Select via
40096-
* VerifyTicketBinding(). */
40157+
/* SNI/ALPN binding is checked by the per-PSK loop in
40158+
* DoPreSharedKeys, not here, so that mismatching PSKs can be
40159+
* skipped in favor of the next candidate. */
4009740160
return 0;
4009840161
}
4009940162
#endif /* WOLFSSL_SLT13 */
@@ -40189,8 +40252,13 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
4018940252
}
4019040253
}
4019140254
#endif
40192-
/* Carry the ticket bindings on the session for the deferred
40193-
* VerifyTicketBinding() check. */
40255+
/* Carry the ticket bindings on the session. TLS 1.2 uses these
40256+
* for the deferred VerifyTicketBinding() check in DoClientHello
40257+
* (SNI/ALPN aren't known when DoClientTicket runs during
40258+
* extension parsing). TLS 1.3 checks bindings per-PSK before
40259+
* reaching this point, but still copies them so a subsequent
40260+
* SetupSession on a resumed session preserves them in the cache
40261+
* for future resumptions. */
4019440262
#ifdef HAVE_SNI
4019540263
XMEMCPY(ssl->session->sniHash, it->sniHash, TICKET_BINDING_HASH_SZ);
4019640264
#endif
@@ -40556,8 +40624,9 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
4055640624
goto cleanup;
4055740625
}
4055840626

40559-
/* SNI/ALPN binding is verified after ALPN_Select via
40560-
* VerifyTicketBinding(). */
40627+
/* SNI/ALPN binding is verified later in DoClientHello via
40628+
* VerifyTicketBinding(), once extension parsing and ALPN_Select
40629+
* have run and the negotiated values are available. */
4056140630
DoClientTicketFinalize(ssl, it, NULL);
4056240631

4056340632
cleanup:

‎src/ssl_sess.c‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2715,6 +2715,16 @@ int wolfSSL_i2d_SSL_SESSION(WOLFSSL_SESSION* sess, unsigned char** p)
27152715
#ifdef HAVE_SESSION_TICKET
27162716
/* ticket len | ticket */
27172717
size += OPAQUE16_LEN + sess->ticketLen;
2718+
#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS)
2719+
#ifdef HAVE_SNI
2720+
/* sniHash */
2721+
size += TICKET_BINDING_HASH_SZ;
2722+
#endif
2723+
#ifdef HAVE_ALPN
2724+
/* alpnHash */
2725+
size += TICKET_BINDING_HASH_SZ;
2726+
#endif
2727+
#endif /* !NO_WOLFSSL_SERVER && !NO_TLS */
27182728
#endif
27192729

27202730
if (p != NULL) {
@@ -2800,6 +2810,16 @@ int wolfSSL_i2d_SSL_SESSION(WOLFSSL_SESSION* sess, unsigned char** p)
28002810
c16toa(sess->ticketLen, data + idx); idx += OPAQUE16_LEN;
28012811
XMEMCPY(data + idx, sess->ticket, sess->ticketLen);
28022812
idx += sess->ticketLen;
2813+
#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS)
2814+
#ifdef HAVE_SNI
2815+
XMEMCPY(data + idx, sess->sniHash, TICKET_BINDING_HASH_SZ);
2816+
idx += TICKET_BINDING_HASH_SZ;
2817+
#endif
2818+
#ifdef HAVE_ALPN
2819+
XMEMCPY(data + idx, sess->alpnHash, TICKET_BINDING_HASH_SZ);
2820+
idx += TICKET_BINDING_HASH_SZ;
2821+
#endif
2822+
#endif /* !NO_WOLFSSL_SERVER && !NO_TLS */
28032823
#endif
28042824
}
28052825
#endif
@@ -3086,6 +3106,26 @@ WOLFSSL_SESSION* wolfSSL_d2i_SSL_SESSION(WOLFSSL_SESSION** sess,
30863106
goto end;
30873107
}
30883108
XMEMCPY(s->ticket, data + idx, s->ticketLen); idx += s->ticketLen;
3109+
#if !defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS)
3110+
#ifdef HAVE_SNI
3111+
/* sniHash - SNI binding for stateful resumption (RFC 6066 section 3) */
3112+
if (i - idx < TICKET_BINDING_HASH_SZ) {
3113+
ret = BUFFER_ERROR;
3114+
goto end;
3115+
}
3116+
XMEMCPY(s->sniHash, data + idx, TICKET_BINDING_HASH_SZ);
3117+
idx += TICKET_BINDING_HASH_SZ;
3118+
#endif
3119+
#ifdef HAVE_ALPN
3120+
/* alpnHash - ALPN binding for stateful resumption */
3121+
if (i - idx < TICKET_BINDING_HASH_SZ) {
3122+
ret = BUFFER_ERROR;
3123+
goto end;
3124+
}
3125+
XMEMCPY(s->alpnHash, data + idx, TICKET_BINDING_HASH_SZ);
3126+
idx += TICKET_BINDING_HASH_SZ;
3127+
#endif
3128+
#endif /* !NO_WOLFSSL_SERVER && !NO_TLS */
30893129
#endif
30903130
(void)idx;
30913131

@@ -3664,6 +3704,16 @@ void SetupSession(WOLFSSL* ssl)
36643704
session->sessionCtxSz = ssl->sessionCtxSz;
36653705
}
36663706
#endif
3707+
#if defined(HAVE_SESSION_TICKET) && \
3708+
!defined(NO_WOLFSSL_SERVER) && !defined(NO_TLS)
3709+
/* Bind the current SNI/ALPN to the session to verify on later resumption */
3710+
#ifdef HAVE_SNI
3711+
(void)TicketSniHash(ssl, session->sniHash);
3712+
#endif
3713+
#ifdef HAVE_ALPN
3714+
(void)TicketAlpnHash(ssl, session->alpnHash);
3715+
#endif
3716+
#endif /* HAVE_SESSION_TICKET && !NO_WOLFSSL_SERVER && !NO_TLS */
36673717
session->timeout = ssl->timeout;
36683718
#ifndef NO_ASN_TIME
36693719
session->bornOn = LowResTimer();

‎src/tls13.c‎

Lines changed: 64 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6455,6 +6455,37 @@ static int DoPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 inputSz,
64556455
}
64566456
#endif
64576457
ret = DoClientTicketCheck(ssl, current, ssl->timeout, suite);
6458+
#if defined(HAVE_SNI) || defined(HAVE_ALPN)
6459+
if (ret == 0) {
6460+
/* Decline this PSK if the SNI/ALPN bound to the ticket
6461+
* does not match the current connection. RFC 6066 Sect.
6462+
* 3 mandates this for SNI; wolfSSL applies the same
6463+
* policy to ALPN as defense in depth. Skipping the PSK
6464+
* (rather than aborting) lets the server try the next
6465+
* candidate or fall back to a full handshake naturally
6466+
* without unwinding committed PSK state. ALPN_Select
6467+
* has already run earlier in DoTls13ClientHello so the
6468+
* negotiated ALPN is available to TicketAlpnHash. */
6469+
byte curHash[TICKET_BINDING_HASH_SZ];
6470+
#ifdef HAVE_SNI
6471+
if (TicketSniHash(ssl, curHash) != 0 ||
6472+
XMEMCMP(curHash, current->it->sniHash,
6473+
TICKET_BINDING_HASH_SZ) != 0) {
6474+
WOLFSSL_MSG("Ticket SNI mismatch, skipping PSK");
6475+
ret = WOLFSSL_FATAL_ERROR;
6476+
}
6477+
#endif
6478+
#ifdef HAVE_ALPN
6479+
if (ret == 0 &&
6480+
(TicketAlpnHash(ssl, curHash) != 0 ||
6481+
XMEMCMP(curHash, current->it->alpnHash,
6482+
TICKET_BINDING_HASH_SZ) != 0)) {
6483+
WOLFSSL_MSG("Ticket ALPN mismatch, skipping PSK");
6484+
ret = WOLFSSL_FATAL_ERROR;
6485+
}
6486+
#endif
6487+
}
6488+
#endif
64586489
if (ret == 0)
64596490
DoClientTicketFinalize(ssl, current->it, current->sess);
64606491
if (current->sess_free_cb != NULL) {
@@ -6606,11 +6637,11 @@ static int CheckPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 helloSz,
66066637
return ret;
66076638
}
66086639

6609-
/* Extensions pushed on stack/list and PSK must be last. */
6610-
if (ssl->extensions != ext) {
6611-
WOLFSSL_ERROR_VERBOSE(PSK_KEY_ERROR);
6612-
return PSK_KEY_ERROR;
6613-
}
6640+
/* Wire-order check that PSK was the last extension in ClientHello is
6641+
* performed in DoTls13ClientHello immediately after TLSX_Parse, since
6642+
* post-parse code (e.g. ALPN_Select via TLSX_SetALPN) may legitimately
6643+
* prepend new entries to ssl->extensions before this point and would
6644+
* otherwise trip a head-of-list check here. */
66146645

66156646
/* Assume we are going to resume with a pre-shared key. */
66166647
ssl->options.resuming = 1;
@@ -7593,6 +7624,25 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
75937624
goto exit_dch;
75947625
}
75957626

7627+
#if (defined(HAVE_SESSION_TICKET) || !defined(NO_PSK)) && \
7628+
defined(HAVE_TLS_EXTENSIONS)
7629+
/* RFC 8446 Section 4.2.11: the pre_shared_key extension MUST be the
7630+
* last extension in the ClientHello. wolfSSL stores extensions in
7631+
* reverse wire order (TLSX_Push prepends), so a well-formed
7632+
* ClientHello with PSK leaves PSK at the head of ssl->extensions
7633+
* here, before any post-parse code (e.g. ALPN_Select) modifies the
7634+
* list. */
7635+
{
7636+
TLSX* pskExt = TLSX_Find(ssl->extensions, TLSX_PRE_SHARED_KEY);
7637+
if (pskExt != NULL && ssl->extensions != pskExt) {
7638+
WOLFSSL_MSG("pre_shared_key extension was not last in "
7639+
"ClientHello");
7640+
WOLFSSL_ERROR_VERBOSE(PSK_KEY_ERROR);
7641+
ERROR_OUT(PSK_KEY_ERROR, exit_dch);
7642+
}
7643+
}
7644+
#endif
7645+
75967646
#if defined(HAVE_ECH)
75977647
if (!ssl->options.echProcessingInner && echX != NULL &&
75987648
((WOLFSSL_ECH*)echX->data)->state == ECH_WRITE_NONE) {
@@ -7702,6 +7752,15 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
77027752
}
77037753
#endif
77047754

7755+
#ifdef HAVE_ALPN
7756+
/* Select the ALPN protocol before PSK selection so that the
7757+
* selected value is available to the per-PSK SNI/ALPN binding check
7758+
* inside CheckPreSharedKeys/DoPreSharedKeys. ALPN_Select itself
7759+
* only inspects ssl->extensions and the app callback; it does not
7760+
* depend on any state set during PSK validation. */
7761+
if ((ret = ALPN_Select(ssl)) != 0)
7762+
goto exit_dch;
7763+
#endif
77057764
#if (defined(HAVE_SESSION_TICKET) || !defined(NO_PSK)) && \
77067765
defined(HAVE_TLS_EXTENSIONS)
77077766
ret = CheckPreSharedKeys(ssl, input + args->begin, helloSz, ssl->clSuites,
@@ -7743,16 +7802,6 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
77437802
#endif
77447803
}
77457804

7746-
#ifdef HAVE_ALPN
7747-
/* With PSK and all other things validated, it's time to
7748-
* select the ALPN protocol, if so requested */
7749-
if ((ret = ALPN_Select(ssl)) != 0)
7750-
goto exit_dch;
7751-
#endif
7752-
#if defined(HAVE_SESSION_TICKET) && (defined(HAVE_SNI) || defined(HAVE_ALPN))
7753-
if ((ret = VerifyTicketBinding(ssl)) != 0)
7754-
goto exit_dch;
7755-
#endif
77567805
} /* case TLS_ASYNC_BEGIN */
77577806
FALL_THROUGH;
77587807

0 commit comments

Comments
 (0)