Skip to content

Commit

Permalink
Merge pull request #6415 from julek-wolfssl/issue/6408
Browse files Browse the repository at this point in the history 
Ignore session ID's shorter than 32 bytes instead of erroring out
  • Loading branch information
@SparkiDev
SparkiDev authored Jun 28, 2023
2 parents dcfa410 + 291c538 commit d029ba4
Show file tree
Hide file tree
Showing 6 changed files with 189 additions and 95 deletions.
35 changes: 12 additions & 23 deletions src/internal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33226,19 +33226,14 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
* session ticket validation check in TLS1.2 and below, define
* WOLFSSL_NO_TICKET_EXPIRE.
*/
int HandleTlsResumption(WOLFSSL* ssl, int bogusID, Suites* clSuites)
int HandleTlsResumption(WOLFSSL* ssl, Suites* clSuites)
{
int ret = 0;
WOLFSSL_SESSION* session;
(void)bogusID;
#ifdef HAVE_SESSION_TICKET
if (ssl->options.useTicket == 1) {
session = ssl->session;
}
else if (bogusID == 1 && ssl->options.rejectTicket == 0) {
WOLFSSL_MSG("Bogus session ID without session ticket");
return BUFFER_ERROR;
}
else
#endif
{
Expand Down Expand Up @@ -33347,7 +33342,6 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
word32 helloSz)
{
byte b;
byte bogusID = 0; /* flag for a bogus session id */
ProtocolVersion pv;
#ifdef WOLFSSL_SMALL_STACK
Suites* clSuites = NULL;
Expand Down Expand Up @@ -33582,30 +33576,25 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,

/* session id */
b = input[i++];

#ifdef HAVE_SESSION_TICKET
if (b > 0 && b < ID_LEN) {
bogusID = 1;
WOLFSSL_MSG("Client sent bogus session id, let's allow for echo");
if (b > ID_LEN) {
WOLFSSL_MSG("Invalid session ID size");
ret = BUFFER_ERROR; /* session ID greater than 32 bytes long */
goto out;
}
#endif

if (b == ID_LEN || bogusID) {
else if (b > 0) {
if ((i - begin) + b > helloSz) {
ret = BUFFER_ERROR;
goto out;
}

/* Always save session ID in case we want to echo it. */
XMEMCPY(ssl->arrays->sessionID, input + i, b);
ssl->arrays->sessionIDSz = b;
i += b;
ssl->options.resuming = 1; /* client wants to resume */

if (b == ID_LEN)
ssl->options.resuming = 1; /* client wants to resume */
WOLFSSL_MSG("Client wants to resume session");
}
else if (b) {
WOLFSSL_MSG("Invalid session ID size");
ret = BUFFER_ERROR; /* session ID nor 0 neither 32 bytes long */
goto out;
i += b;
}

#ifdef WOLFSSL_DTLS
Expand Down Expand Up @@ -33885,7 +33874,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,

/* ProcessOld uses same resume code */
if (ssl->options.resuming) {
ret = HandleTlsResumption(ssl, bogusID, clSuites);
ret = HandleTlsResumption(ssl, clSuites);
if (ret != 0)
goto out;

Expand Down
2 changes: 1 addition & 1 deletion src/quic.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ static int quic_record_append(WOLFSSL *ssl, QuicRecord *qr, const uint8_t *data,

qr->len = qr_length(qr->data, qr->end);
if (qr->len > qr->capacity) {
uint8_t *ndata = (uint8_t*)XREALLOC(qr->data, qr->len, ssl->head,
uint8_t *ndata = (uint8_t*)XREALLOC(qr->data, qr->len, ssl->heap,
DYNAMIC_TYPE_TMP_BUFFER);
if (!ndata) {
ret = WOLFSSL_FAILURE;
Expand Down
103 changes: 60 additions & 43 deletions src/tls13.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3984,6 +3984,49 @@ static int EchHashHelloInner(WOLFSSL* ssl, WOLFSSL_ECH* ech)
}
#endif

static void GetTls13SessionId(WOLFSSL* ssl, byte* output, word32* idx)
{
if (ssl->session->sessionIDSz > 0) {
/* Session resumption for old versions of protocol. */
if (ssl->session->sessionIDSz <= ID_LEN) {
if (output != NULL)
output[*idx] = ssl->session->sessionIDSz;
(*idx)++;
if (output != NULL) {
XMEMCPY(output + *idx, ssl->session->sessionID,
ssl->session->sessionIDSz);
}
*idx += ssl->session->sessionIDSz;
}
else {
/* Invalid session ID length. Reset it. */
ssl->session->sessionIDSz = 0;
if (output != NULL)
output[*idx] = 0;
(*idx)++;
}
}
else {
#ifdef WOLFSSL_TLS13_MIDDLEBOX_COMPAT
if (ssl->options.tls13MiddleBoxCompat) {
if (output != NULL)
output[*idx] = ID_LEN;
(*idx)++;
if (output != NULL)
XMEMCPY(output + *idx, ssl->arrays->clientRandom, ID_LEN);
*idx += ID_LEN;
}
else
#endif /* WOLFSSL_TLS13_MIDDLEBOX_COMPAT */
{
/* TLS v1.3 does not use session id - 0 length. */
if (output != NULL)
output[*idx] = 0;
(*idx)++;
}
}
}

/* handle generation of TLS 1.3 client_hello (1) */
/* Send a ClientHello message to the server.
* Include the information required to start a handshake with servers using
Expand Down Expand Up @@ -4092,6 +4135,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)
switch (ssl->options.asyncState) {
case TLS_ASYNC_BEGIN:
{
word32 sessIdSz = 0;

args->idx = RECORD_HEADER_SZ + HANDSHAKE_HEADER_SZ;

Expand All @@ -4100,26 +4144,21 @@ int SendTls13ClientHello(WOLFSSL* ssl)
args->idx += DTLS_RECORD_EXTRA + DTLS_HANDSHAKE_EXTRA;
#endif /* WOLFSSL_DTLS13 */

/* Version | Random | Session Id | Cipher Suites | Compression */
args->length = VERSION_SZ + RAN_LEN + ENUM_LEN + suites->suiteSz +
/* Version | Random | Cipher Suites | Compression */
args->length = VERSION_SZ + RAN_LEN + suites->suiteSz +
SUITE_LEN + COMP_LEN + ENUM_LEN;
#if defined(WOLFSSL_TLS13_MIDDLEBOX_COMPAT)
ssl->options.tls13MiddleBoxCompat = 1;
#endif
#ifdef WOLFSSL_QUIC
if (WOLFSSL_IS_QUIC(ssl)) {
/* RFC 9001 ch. 8.4 sessionID in ClientHello MUST be 0 length */
ssl->session->sessionIDSz = 0;
ssl->options.tls13MiddleBoxCompat = 0;
}
else
#endif
#if defined(WOLFSSL_TLS13_MIDDLEBOX_COMPAT)
{
args->length += ID_LEN;
ssl->options.tls13MiddleBoxCompat = 1;
}
#else
if (ssl->options.resuming && ssl->session->sessionIDSz > 0)
args->length += ssl->session->sessionIDSz;
#endif
GetTls13SessionId(ssl, NULL, &sessIdSz);
args->length += (word16)sessIdSz;

#ifdef WOLFSSL_DTLS13
if (ssl->options.dtls) {
Expand Down Expand Up @@ -4259,33 +4298,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)

args->idx += RAN_LEN;

if (ssl->session->sessionIDSz > 0) {
/* Session resumption for old versions of protocol. */
if (ssl->options.resuming) {
args->output[args->idx++] = ID_LEN;
XMEMCPY(args->output + args->idx, ssl->session->sessionID,
ssl->session->sessionIDSz);
args->idx += ID_LEN;
}
else {
/* Not resuming, zero length session ID */
args->output[args->idx++] = 0;
}
}
else {
#ifdef WOLFSSL_TLS13_MIDDLEBOX_COMPAT
if (ssl->options.tls13MiddleBoxCompat) {
args->output[args->idx++] = ID_LEN;
XMEMCPY(args->output + args->idx, ssl->arrays->clientRandom, ID_LEN);
args->idx += ID_LEN;
}
else
#endif /* WOLFSSL_TLS13_MIDDLEBOX_COMPAT */
{
/* TLS v1.3 does not use session id - 0 length. */
args->output[args->idx++] = 0;
}
}
GetTls13SessionId(ssl, args->output, &args->idx);

#ifdef WOLFSSL_DTLS13
if (ssl->options.dtls) {
Expand Down Expand Up @@ -6536,18 +6549,22 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
#endif

sessIdSz = input[args->idx++];
if (sessIdSz != ID_LEN && sessIdSz != 0) {
#ifndef WOLFSSL_TLS13_MIDDLEBOX_COMPAT
if (sessIdSz > ID_LEN)
#else
if (sessIdSz != ID_LEN && sessIdSz != 0)
#endif
{
ERROR_OUT(INVALID_PARAMETER, exit_dch);
}

if (sessIdSz + args->idx > helloSz)
ERROR_OUT(BUFFER_ERROR, exit_dch);

ssl->session->sessionIDSz = sessIdSz;
if (sessIdSz == ID_LEN) {
if (sessIdSz > 0)
XMEMCPY(ssl->session->sessionID, input + args->idx, sessIdSz);
args->idx += ID_LEN;
}
args->idx += sessIdSz;

#ifdef WOLFSSL_DTLS13
/* legacy_cookie */
Expand Down
Loading

0 comments on commit d029ba4

Please sign in to comment.