Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ignore session ID's shorter than 32 bytes instead of erroring out #6415

Merged
merged 1 commit into from
Jun 28, 2023
+189 −95
Merged

Ignore session ID's shorter than 32 bytes instead of erroring out #6415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 12 additions & 23 deletions src/internal.c
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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;
SparkiDev marked this conversation as resolved.
Show resolved Hide resolved
#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