Skip to content

Commit d029ba4

Browse files
SparkiDevSparkiDev
authored
Merge pull request #6415 from julek-wolfssl/issue/6408
Ignore session ID's shorter than 32 bytes instead of erroring out
2 parents dcfa410 + 291c538 commit d029ba4

6 files changed

Lines changed: 189 additions & 95 deletions

File tree

‎src/internal.c‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -33226,19 +33226,14 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
3322633226
* session ticket validation check in TLS1.2 and below, define
3322733227
* WOLFSSL_NO_TICKET_EXPIRE.
3322833228
*/
33229-
int HandleTlsResumption(WOLFSSL* ssl, int bogusID, Suites* clSuites)
33229+
int HandleTlsResumption(WOLFSSL* ssl, Suites* clSuites)
3323033230
{
3323133231
int ret = 0;
3323233232
WOLFSSL_SESSION* session;
33233-
(void)bogusID;
3323433233
#ifdef HAVE_SESSION_TICKET
3323533234
if (ssl->options.useTicket == 1) {
3323633235
session = ssl->session;
3323733236
}
33238-
else if (bogusID == 1 && ssl->options.rejectTicket == 0) {
33239-
WOLFSSL_MSG("Bogus session ID without session ticket");
33240-
return BUFFER_ERROR;
33241-
}
3324233237
else
3324333238
#endif
3324433239
{
@@ -33347,7 +33342,6 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
3334733342
word32 helloSz)
3334833343
{
3334933344
byte b;
33350-
byte bogusID = 0; /* flag for a bogus session id */
3335133345
ProtocolVersion pv;
3335233346
#ifdef WOLFSSL_SMALL_STACK
3335333347
Suites* clSuites = NULL;
@@ -33582,30 +33576,25 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
3358233576

3358333577
/* session id */
3358433578
b = input[i++];
33585-
33586-
#ifdef HAVE_SESSION_TICKET
33587-
if (b > 0 && b < ID_LEN) {
33588-
bogusID = 1;
33589-
WOLFSSL_MSG("Client sent bogus session id, let's allow for echo");
33579+
if (b > ID_LEN) {
33580+
WOLFSSL_MSG("Invalid session ID size");
33581+
ret = BUFFER_ERROR; /* session ID greater than 32 bytes long */
33582+
goto out;
3359033583
}
33591-
#endif
33592-
33593-
if (b == ID_LEN || bogusID) {
33584+
else if (b > 0) {
3359433585
if ((i - begin) + b > helloSz) {
3359533586
ret = BUFFER_ERROR;
3359633587
goto out;
3359733588
}
3359833589

33590+
/* Always save session ID in case we want to echo it. */
3359933591
XMEMCPY(ssl->arrays->sessionID, input + i, b);
3360033592
ssl->arrays->sessionIDSz = b;
33601-
i += b;
33602-
ssl->options.resuming = 1; /* client wants to resume */
33593+
33594+
if (b == ID_LEN)
33595+
ssl->options.resuming = 1; /* client wants to resume */
3360333596
WOLFSSL_MSG("Client wants to resume session");
33604-
}
33605-
else if (b) {
33606-
WOLFSSL_MSG("Invalid session ID size");
33607-
ret = BUFFER_ERROR; /* session ID nor 0 neither 32 bytes long */
33608-
goto out;
33597+
i += b;
3360933598
}
3361033599

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

3388633875
/* ProcessOld uses same resume code */
3388733876
if (ssl->options.resuming) {
33888-
ret = HandleTlsResumption(ssl, bogusID, clSuites);
33877+
ret = HandleTlsResumption(ssl, clSuites);
3388933878
if (ret != 0)
3389033879
goto out;
3389133880

‎src/quic.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ static int quic_record_append(WOLFSSL *ssl, QuicRecord *qr, const uint8_t *data,
130130

131131
qr->len = qr_length(qr->data, qr->end);
132132
if (qr->len > qr->capacity) {
133-
uint8_t *ndata = (uint8_t*)XREALLOC(qr->data, qr->len, ssl->head,
133+
uint8_t *ndata = (uint8_t*)XREALLOC(qr->data, qr->len, ssl->heap,
134134
DYNAMIC_TYPE_TMP_BUFFER);
135135
if (!ndata) {
136136
ret = WOLFSSL_FAILURE;

‎src/tls13.c‎

Lines changed: 60 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3984,6 +3984,49 @@ static int EchHashHelloInner(WOLFSSL* ssl, WOLFSSL_ECH* ech)
39843984
}
39853985
#endif
39863986

3987+
static void GetTls13SessionId(WOLFSSL* ssl, byte* output, word32* idx)
3988+
{
3989+
if (ssl->session->sessionIDSz > 0) {
3990+
/* Session resumption for old versions of protocol. */
3991+
if (ssl->session->sessionIDSz <= ID_LEN) {
3992+
if (output != NULL)
3993+
output[*idx] = ssl->session->sessionIDSz;
3994+
(*idx)++;
3995+
if (output != NULL) {
3996+
XMEMCPY(output + *idx, ssl->session->sessionID,
3997+
ssl->session->sessionIDSz);
3998+
}
3999+
*idx += ssl->session->sessionIDSz;
4000+
}
4001+
else {
4002+
/* Invalid session ID length. Reset it. */
4003+
ssl->session->sessionIDSz = 0;
4004+
if (output != NULL)
4005+
output[*idx] = 0;
4006+
(*idx)++;
4007+
}
4008+
}
4009+
else {
4010+
#ifdef WOLFSSL_TLS13_MIDDLEBOX_COMPAT
4011+
if (ssl->options.tls13MiddleBoxCompat) {
4012+
if (output != NULL)
4013+
output[*idx] = ID_LEN;
4014+
(*idx)++;
4015+
if (output != NULL)
4016+
XMEMCPY(output + *idx, ssl->arrays->clientRandom, ID_LEN);
4017+
*idx += ID_LEN;
4018+
}
4019+
else
4020+
#endif /* WOLFSSL_TLS13_MIDDLEBOX_COMPAT */
4021+
{
4022+
/* TLS v1.3 does not use session id - 0 length. */
4023+
if (output != NULL)
4024+
output[*idx] = 0;
4025+
(*idx)++;
4026+
}
4027+
}
4028+
}
4029+
39874030
/* handle generation of TLS 1.3 client_hello (1) */
39884031
/* Send a ClientHello message to the server.
39894032
* Include the information required to start a handshake with servers using
@@ -4092,6 +4135,7 @@ int SendTls13ClientHello(WOLFSSL* ssl)
40924135
switch (ssl->options.asyncState) {
40934136
case TLS_ASYNC_BEGIN:
40944137
{
4138+
word32 sessIdSz = 0;
40954139

40964140
args->idx = RECORD_HEADER_SZ + HANDSHAKE_HEADER_SZ;
40974141

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

4103-
/* Version | Random | Session Id | Cipher Suites | Compression */
4104-
args->length = VERSION_SZ + RAN_LEN + ENUM_LEN + suites->suiteSz +
4147+
/* Version | Random | Cipher Suites | Compression */
4148+
args->length = VERSION_SZ + RAN_LEN + suites->suiteSz +
41054149
SUITE_LEN + COMP_LEN + ENUM_LEN;
4150+
#if defined(WOLFSSL_TLS13_MIDDLEBOX_COMPAT)
4151+
ssl->options.tls13MiddleBoxCompat = 1;
4152+
#endif
41064153
#ifdef WOLFSSL_QUIC
41074154
if (WOLFSSL_IS_QUIC(ssl)) {
41084155
/* RFC 9001 ch. 8.4 sessionID in ClientHello MUST be 0 length */
41094156
ssl->session->sessionIDSz = 0;
41104157
ssl->options.tls13MiddleBoxCompat = 0;
41114158
}
4112-
else
4113-
#endif
4114-
#if defined(WOLFSSL_TLS13_MIDDLEBOX_COMPAT)
4115-
{
4116-
args->length += ID_LEN;
4117-
ssl->options.tls13MiddleBoxCompat = 1;
4118-
}
4119-
#else
4120-
if (ssl->options.resuming && ssl->session->sessionIDSz > 0)
4121-
args->length += ssl->session->sessionIDSz;
41224159
#endif
4160+
GetTls13SessionId(ssl, NULL, &sessIdSz);
4161+
args->length += (word16)sessIdSz;
41234162

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

42604299
args->idx += RAN_LEN;
42614300

4262-
if (ssl->session->sessionIDSz > 0) {
4263-
/* Session resumption for old versions of protocol. */
4264-
if (ssl->options.resuming) {
4265-
args->output[args->idx++] = ID_LEN;
4266-
XMEMCPY(args->output + args->idx, ssl->session->sessionID,
4267-
ssl->session->sessionIDSz);
4268-
args->idx += ID_LEN;
4269-
}
4270-
else {
4271-
/* Not resuming, zero length session ID */
4272-
args->output[args->idx++] = 0;
4273-
}
4274-
}
4275-
else {
4276-
#ifdef WOLFSSL_TLS13_MIDDLEBOX_COMPAT
4277-
if (ssl->options.tls13MiddleBoxCompat) {
4278-
args->output[args->idx++] = ID_LEN;
4279-
XMEMCPY(args->output + args->idx, ssl->arrays->clientRandom, ID_LEN);
4280-
args->idx += ID_LEN;
4281-
}
4282-
else
4283-
#endif /* WOLFSSL_TLS13_MIDDLEBOX_COMPAT */
4284-
{
4285-
/* TLS v1.3 does not use session id - 0 length. */
4286-
args->output[args->idx++] = 0;
4287-
}
4288-
}
4301+
GetTls13SessionId(ssl, args->output, &args->idx);
42894302

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

65386551
sessIdSz = input[args->idx++];
6539-
if (sessIdSz != ID_LEN && sessIdSz != 0) {
6552+
#ifndef WOLFSSL_TLS13_MIDDLEBOX_COMPAT
6553+
if (sessIdSz > ID_LEN)
6554+
#else
6555+
if (sessIdSz != ID_LEN && sessIdSz != 0)
6556+
#endif
6557+
{
65406558
ERROR_OUT(INVALID_PARAMETER, exit_dch);
65416559
}
65426560

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

65466564
ssl->session->sessionIDSz = sessIdSz;
6547-
if (sessIdSz == ID_LEN) {
6565+
if (sessIdSz > 0)
65486566
XMEMCPY(ssl->session->sessionID, input + args->idx, sessIdSz);
6549-
args->idx += ID_LEN;
6550-
}
6567+
args->idx += sessIdSz;
65516568

65526569
#ifdef WOLFSSL_DTLS13
65536570
/* legacy_cookie */

0 commit comments

Comments
 (0)