Skip to content

Commit db14bc7

Browse files
committed
src/internal.c: defer ticket SNI/ALPN binding check until after parsing
The early checks in DoClientTicketCheck and DoClientTicket ran before the corresponding extensions were parsed, so the computed current hash was zero while the ticket's stored hash was non-zero, rejecting valid resumptions in the nginx, haproxy, grpc and CPython integration tests. * TLS 1.3: DoTls13ClientHello processes pre_shared_key before ALPN_Select, so TLSX_ALPN_GetRequest returned WOLFSSL_ALPN_NOT_FOUND. * TLS 1.2: ClientHello extensions are parsed in wire order; clients that put SessionTicket before server_name / ALPN hit the same problem with both bindings. Consolidate the verification into a single VerifyTicketBinding() function, called once on the server after ALPN_Select (in both DoTls13ClientHello and DoClientHello). DoClientTicketFinalize copies the ticket's stored bindings onto ssl->session so the deferred check has the values to compare. The early per-call sites are removed. VerifyTicketBinding returns WOLFSSL_FATAL_ERROR on mismatch; the caller currently aborts the handshake. Behaviour on mismatch (error vs fallback to a fresh handshake) can be revisited from this single point.
1 parent 66074e2 commit db14bc7

3 files changed

Lines changed: 58 additions & 51 deletions

File tree

src/internal.c

Lines changed: 51 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -38468,6 +38468,11 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3846838468
if((ret=ALPN_Select(ssl)))
3846938469
goto out;
3847038470
#endif
38471+
#if defined(HAVE_SESSION_TICKET) && \
38472+
(defined(HAVE_SNI) || defined(HAVE_ALPN))
38473+
if((ret=VerifyTicketBinding(ssl)))
38474+
goto out;
38475+
#endif
3847138476

3847238477
i += totalExtSz;
3847338478
#else
@@ -39250,8 +39255,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3925039255
}
3925139256

3925239257
#ifdef HAVE_SNI
39253-
/* Hash the server-selected SNI into dst (TICKET_BINDING_HASH_SZ bytes).
39254-
* Zeros dst when no SNI is present. */
39258+
/* Hash server-selected SNI; zeros dst when none. */
3925539259
static int TicketSniHash(WOLFSSL* ssl, byte* dst)
3925639260
{
3925739261
char* name = NULL;
@@ -39271,8 +39275,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3927139275
#endif
3927239276

3927339277
#ifdef HAVE_ALPN
39274-
/* Hash the negotiated ALPN protocol into dst (TICKET_BINDING_HASH_SZ
39275-
* bytes). Zeros dst when no ALPN was negotiated. */
39278+
/* Hash negotiated ALPN; zeros dst when none. */
3927639279
static int TicketAlpnHash(WOLFSSL* ssl, byte* dst)
3927739280
{
3927839281
char* proto = NULL;
@@ -39290,6 +39293,38 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3929039293
}
3929139294
#endif
3929239295

39296+
#if defined(HAVE_SNI) || defined(HAVE_ALPN)
39297+
/* Server-side: verify the SNI/ALPN bindings carried on a resumed
39298+
* session match what was negotiated for the current connection.
39299+
* Must be called after extension parsing and ALPN_Select.
39300+
* Returns 0 on match, WOLFSSL_FATAL_ERROR on mismatch. */
39301+
int VerifyTicketBinding(WOLFSSL* ssl)
39302+
{
39303+
byte curHash[TICKET_BINDING_HASH_SZ];
39304+
39305+
if (!ssl->options.resuming || !ssl->options.useTicket)
39306+
return 0;
39307+
39308+
#ifdef HAVE_SNI
39309+
if (TicketSniHash(ssl, curHash) != 0 ||
39310+
XMEMCMP(curHash, ssl->session->sniHash,
39311+
TICKET_BINDING_HASH_SZ) != 0) {
39312+
WOLFSSL_MSG("Ticket SNI mismatch");
39313+
return WOLFSSL_FATAL_ERROR;
39314+
}
39315+
#endif
39316+
#ifdef HAVE_ALPN
39317+
if (TicketAlpnHash(ssl, curHash) != 0 ||
39318+
XMEMCMP(curHash, ssl->session->alpnHash,
39319+
TICKET_BINDING_HASH_SZ) != 0) {
39320+
WOLFSSL_MSG("Ticket ALPN mismatch");
39321+
return WOLFSSL_FATAL_ERROR;
39322+
}
39323+
#endif
39324+
return 0;
39325+
}
39326+
#endif
39327+
3929339328
/* create a new session ticket, 0 on success
3929439329
* Do any kind of setup in SetupTicket */
3929539330
int CreateTicket(WOLFSSL* ssl)
@@ -39755,28 +39790,8 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3975539790
ssl->sessionCtxSz) != 0))
3975639791
return WOLFSSL_FATAL_ERROR;
3975739792
#endif
39758-
#ifdef HAVE_SNI
39759-
{
39760-
byte curHash[TICKET_BINDING_HASH_SZ];
39761-
if (TicketSniHash((WOLFSSL*)ssl, curHash) != 0 ||
39762-
XMEMCMP(curHash, psk->it->sniHash,
39763-
TICKET_BINDING_HASH_SZ) != 0) {
39764-
WOLFSSL_MSG("Ticket SNI mismatch");
39765-
return WOLFSSL_FATAL_ERROR;
39766-
}
39767-
}
39768-
#endif
39769-
#ifdef HAVE_ALPN
39770-
{
39771-
byte curHash[TICKET_BINDING_HASH_SZ];
39772-
if (TicketAlpnHash((WOLFSSL*)ssl, curHash) != 0 ||
39773-
XMEMCMP(curHash, psk->it->alpnHash,
39774-
TICKET_BINDING_HASH_SZ) != 0) {
39775-
WOLFSSL_MSG("Ticket ALPN mismatch");
39776-
return WOLFSSL_FATAL_ERROR;
39777-
}
39778-
}
39779-
#endif
39793+
/* SNI/ALPN binding is verified after ALPN_Select via
39794+
* VerifyTicketBinding(). */
3978039795
return 0;
3978139796
}
3978239797
#endif /* WOLFSSL_SLT13 */
@@ -39872,6 +39887,14 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
3987239887
}
3987339888
}
3987439889
#endif
39890+
/* Carry the ticket bindings on the session for the deferred
39891+
* VerifyTicketBinding() check. */
39892+
#ifdef HAVE_SNI
39893+
XMEMCPY(ssl->session->sniHash, it->sniHash, TICKET_BINDING_HASH_SZ);
39894+
#endif
39895+
#ifdef HAVE_ALPN
39896+
XMEMCPY(ssl->session->alpnHash, it->alpnHash, TICKET_BINDING_HASH_SZ);
39897+
#endif
3987539898

3987639899
if (!IsAtLeastTLSv1_3(ssl->version)) {
3987739900
if (ssl->arrays == NULL)
@@ -40231,31 +40254,8 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl)
4023140254
goto cleanup;
4023240255
}
4023340256

40234-
#ifdef HAVE_SNI
40235-
{
40236-
byte curHash[TICKET_BINDING_HASH_SZ];
40237-
if (TicketSniHash(ssl, curHash) != 0 ||
40238-
XMEMCMP(curHash, it->sniHash,
40239-
TICKET_BINDING_HASH_SZ) != 0) {
40240-
WOLFSSL_MSG("Ticket SNI mismatch");
40241-
decryptRet = WOLFSSL_TICKET_RET_REJECT;
40242-
goto cleanup;
40243-
}
40244-
}
40245-
#endif
40246-
#ifdef HAVE_ALPN
40247-
{
40248-
byte curHash[TICKET_BINDING_HASH_SZ];
40249-
if (TicketAlpnHash(ssl, curHash) != 0 ||
40250-
XMEMCMP(curHash, it->alpnHash,
40251-
TICKET_BINDING_HASH_SZ) != 0) {
40252-
WOLFSSL_MSG("Ticket ALPN mismatch");
40253-
decryptRet = WOLFSSL_TICKET_RET_REJECT;
40254-
goto cleanup;
40255-
}
40256-
}
40257-
#endif
40258-
40257+
/* SNI/ALPN binding is verified after ALPN_Select via
40258+
* VerifyTicketBinding(). */
4025940259
DoClientTicketFinalize(ssl, it, NULL);
4026040260

4026140261
cleanup:

src/tls13.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7555,6 +7555,10 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
75557555
* select the ALPN protocol, if so requested */
75567556
if ((ret = ALPN_Select(ssl)) != 0)
75577557
goto exit_dch;
7558+
#endif
7559+
#if defined(HAVE_SESSION_TICKET) && (defined(HAVE_SNI) || defined(HAVE_ALPN))
7560+
if ((ret = VerifyTicketBinding(ssl)) != 0)
7561+
goto exit_dch;
75587562
#endif
75597563
} /* case TLS_ASYNC_BEGIN */
75607564
FALL_THROUGH;

wolfssl/internal.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6790,6 +6790,9 @@ WOLFSSL_LOCAL int DoClientTicket_ex(const WOLFSSL* ssl, PreSharedKey* psk,
67906790
#endif
67916791

67926792
WOLFSSL_LOCAL int DoClientTicket(WOLFSSL* ssl, const byte* input, word32 len);
6793+
#if defined(HAVE_SNI) || defined(HAVE_ALPN)
6794+
WOLFSSL_LOCAL int VerifyTicketBinding(WOLFSSL* ssl);
6795+
#endif
67936796
#endif /* HAVE_SESSION_TICKET */
67946797
WOLFSSL_LOCAL int SendData(WOLFSSL* ssl, const void* data, size_t sz);
67956798
#ifdef WOLFSSL_THREADED_CRYPT

0 commit comments

Comments
 (0)