Skip to content

Commit

Permalink
Merge pull request #8097 from julek-wolfssl/zd/18822
Browse files Browse the repository at this point in the history 
Fix TLS v1.2 session resumption edge cases
  • Loading branch information
@douzzer
douzzer authored Oct 23, 2024
2 parents 830c5da + 25e32c2 commit e7e2053
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 51 deletions.
101 changes: 55 additions & 46 deletions src/internal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17471,6 +17471,18 @@ int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx,
case certificate_request:
case server_hello_done:
if (ssl->options.resuming) {
/* Client requested resumption, but server is doing a
* full handshake */

/* The server's decision to resume isn't known until after the
* "server_hello". If subsequent handshake messages like
* "certificate" or "server_key_exchange" are recevied then we
* are doing a full handshake */

/* If the server included a session id then we
* treat this as a fatal error, since the server said it was
* doing resumption, but did not. */

/* https://www.rfc-editor.org/rfc/rfc5077.html#section-3.4
* Alternatively, the client MAY include an empty Session ID
* in the ClientHello. In this case, the client ignores the
Expand All @@ -17479,7 +17491,7 @@ int DoHandShakeMsgType(WOLFSSL* ssl, byte* input, word32* inOutIdx,
* messages.
*/
#ifndef WOLFSSL_WPAS
if (ssl->session->sessionIDSz != 0) {
if (ssl->arrays->sessionIDSz != 0) {
/* Fatal error. Only try to send an alert. RFC 5246 does not
* allow for reverting back to a full handshake after the
* server has indicated the intention to do a resumption. */
Expand Down Expand Up @@ -34510,6 +34522,29 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,

#ifndef WOLFSSL_NO_TLS12

static int getSessionID(WOLFSSL* ssl)
{
int sessIdSz = 0;
(void)ssl;
#ifndef NO_SESSION_CACHE
/* if no session cache don't send a session ID */
if (!ssl->options.sessionCacheOff)
sessIdSz = ID_LEN;
#endif
#ifdef HAVE_SESSION_TICKET
/* we may be echoing an ID as part of session tickets */
if (ssl->options.useTicket) {
/* echo session id sz can be 0,32 or bogus len in between */
sessIdSz = ssl->arrays->sessionIDSz;
if (sessIdSz > ID_LEN) {
WOLFSSL_MSG("Bad bogus session id len");
return BUFFER_ERROR;
}
}
#endif /* HAVE_SESSION_TICKET */
return sessIdSz;
}

/* handle generation of server_hello (2) */
int SendServerHello(WOLFSSL* ssl)
{
Expand All @@ -34518,63 +34553,31 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
word16 length;
word32 idx = RECORD_HEADER_SZ + HANDSHAKE_HEADER_SZ;
int sendSz;
byte sessIdSz = ID_LEN;
#if defined(HAVE_TLS_EXTENSIONS) && defined(HAVE_SESSION_TICKET)
byte echoId = 0; /* ticket echo id flag */
#endif
byte cacheOff = 0; /* session cache off flag */
byte sessIdSz;

WOLFSSL_START(WC_FUNC_SERVER_HELLO_SEND);
WOLFSSL_ENTER("SendServerHello");

ret = getSessionID(ssl);
if (ret < 0)
return ret;
sessIdSz = (byte)ret;

length = VERSION_SZ + RAN_LEN
+ ID_LEN + ENUM_LEN
+ ENUM_LEN + sessIdSz
+ SUITE_LEN
+ ENUM_LEN;

#ifdef HAVE_TLS_EXTENSIONS
ret = TLSX_GetResponseSize(ssl, server_hello, &length);
if (ret != 0)
return ret;
#ifdef HAVE_SESSION_TICKET
if (ssl->options.useTicket) {
/* echo session id sz can be 0,32 or bogus len in between */
sessIdSz = ssl->arrays->sessionIDSz;
if (sessIdSz > ID_LEN) {
WOLFSSL_MSG("Bad bogus session id len");
return BUFFER_ERROR;
}
if (!IsAtLeastTLSv1_3(ssl->version))
length -= (ID_LEN - sessIdSz); /* adjust ID_LEN assumption */
echoId = 1;
}
#endif /* HAVE_SESSION_TICKET */
#else
if (ssl->options.haveEMS) {
length += HELLO_EXT_SZ_SZ + HELLO_EXT_SZ;
}
#endif

/* is the session cache off at build or runtime */
#ifdef NO_SESSION_CACHE
cacheOff = 1;
#else
if (ssl->options.sessionCacheOff == 1) {
cacheOff = 1;
}
#endif

/* if no session cache don't send a session ID unless we're echoing
* an ID as part of session tickets */
if (cacheOff == 1
#if defined(HAVE_TLS_EXTENSIONS) && defined(HAVE_SESSION_TICKET)
&& echoId == 0
#endif
) {
length -= ID_LEN; /* adjust ID_LEN assumption */
sessIdSz = 0;
}

sendSz = length + HANDSHAKE_HEADER_SZ + RECORD_HEADER_SZ;
#ifdef WOLFSSL_DTLS
if (ssl->options.dtls) {
Expand Down Expand Up @@ -34605,18 +34608,15 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,

/* then random and session id */
if (!ssl->options.resuming) {
/* generate random part and session id */
ret = wc_RNG_GenerateBlock(ssl->rng, output + idx,
RAN_LEN + sizeof(sessIdSz) + sessIdSz);
if (ret != 0)
return ret;
word32 genRanLen = RAN_LEN;

#ifdef WOLFSSL_TLS13
if (TLSv1_3_Capable(ssl)) {
/* TLS v1.3 capable server downgraded. */
XMEMCPY(output + idx + RAN_LEN - (TLS13_DOWNGRADE_SZ + 1),
tls13Downgrade, TLS13_DOWNGRADE_SZ);
output[idx + RAN_LEN - 1] = (byte)IsAtLeastTLSv1_2(ssl);
genRanLen -= TLS13_DOWNGRADE_SZ + 1;
}
else
#endif
Expand All @@ -34628,12 +34628,21 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
XMEMCPY(output + idx + RAN_LEN - (TLS13_DOWNGRADE_SZ + 1),
tls13Downgrade, TLS13_DOWNGRADE_SZ);
output[idx + RAN_LEN - 1] = 0;
genRanLen -= TLS13_DOWNGRADE_SZ + 1;
}

/* store info in SSL for later */
/* generate random part */
ret = wc_RNG_GenerateBlock(ssl->rng, output + idx, genRanLen);
if (ret != 0)
return ret;
XMEMCPY(ssl->arrays->serverRandom, output + idx, RAN_LEN);
idx += RAN_LEN;

/* generate session id */
output[idx++] = sessIdSz;
ret = wc_RNG_GenerateBlock(ssl->rng, output + idx, sessIdSz);
if (ret != 0)
return ret;
XMEMCPY(ssl->arrays->sessionID, output + idx, sessIdSz);
ssl->arrays->sessionIDSz = sessIdSz;
}
Expand Down
21 changes: 16 additions & 5 deletions src/tls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5905,14 +5905,25 @@ static int TLSX_SessionTicket_Parse(WOLFSSL* ssl, const byte* input,
/* SERVER: ticket is peer auth. */
ssl->options.peerAuthGood = 1;
}
} else if (ret == WOLFSSL_TICKET_RET_REJECT) {
} else if (ret == WOLFSSL_TICKET_RET_REJECT ||
ret == WC_NO_ERR_TRACE(VERSION_ERROR)) {
WOLFSSL_MSG("Process client ticket rejected, not using");
ssl->options.rejectTicket = 1;
if (ret == WC_NO_ERR_TRACE(VERSION_ERROR))
WOLFSSL_MSG("\tbad TLS version");
ret = 0; /* not fatal */
} else if (ret == WC_NO_ERR_TRACE(VERSION_ERROR)) {
WOLFSSL_MSG("Process client ticket rejected, bad TLS version");

ssl->options.rejectTicket = 1;
ret = 0; /* not fatal */
/* If we have session tickets enabled then send a new ticket */
if (!TLSX_CheckUnsupportedExtension(ssl, TLSX_SESSION_TICKET)) {
ret = TLSX_UseSessionTicket(&ssl->extensions, NULL,
ssl->heap);
if (ret == WOLFSSL_SUCCESS) {
ret = 0;
TLSX_SetResponse(ssl, TLSX_SESSION_TICKET);
ssl->options.createTicket = 1;
ssl->options.useTicket = 1;
}
}
} else if (ret == WOLFSSL_TICKET_RET_FATAL) {
WOLFSSL_MSG("Process client ticket fatal error, not using");
} else if (ret < 0) {
Expand Down

0 comments on commit e7e2053

Please sign in to comment.