diff --git a/src/internal.c b/src/internal.c index 9d8a23a3b2..40c59beee7 100644 --- a/src/internal.c +++ b/src/internal.c @@ -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 { @@ -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; @@ -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 @@ -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; diff --git a/src/quic.c b/src/quic.c index a8a590bf2e..e371940b62 100644 --- a/src/quic.c +++ b/src/quic.c @@ -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; diff --git a/src/tls13.c b/src/tls13.c index 257750233d..3ff4b532cc 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -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 @@ -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; @@ -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) { @@ -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) { @@ -6536,7 +6549,12 @@ 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); } @@ -6544,10 +6562,9 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx, 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 */ diff --git a/tests/api.c b/tests/api.c index 2642f276c8..86410d4119 100644 --- a/tests/api.c +++ b/tests/api.c @@ -342,7 +342,7 @@ defined(HAVE_SESSION_TICKET) || (defined(OPENSSL_EXTRA) && \ defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_CERT_GEN)) || \ defined(WOLFSSL_TEST_STATIC_BUILD) || defined(WOLFSSL_DTLS) || \ - defined(HAVE_ECH) || defined(HAVE_EX_DATA) + defined(HAVE_ECH) || defined(HAVE_EX_DATA) || !defined(NO_SESSION_CACHE) /* for testing SSL_get_peer_cert_chain, or SESSION_TICKET_HINT_DEFAULT, * for setting authKeyIdSrc in WOLFSSL_X509, or testing DTLS sequence * number tracking */ @@ -7346,8 +7346,8 @@ static int test_wolfSSL_CTX_verifyDepth_ServerClient_1(void) test_ssl_cbf client_cbf; test_ssl_cbf server_cbf; - XMEMSET(&client_cbf, 0, sizeof(callback_functions)); - XMEMSET(&server_cbf, 0, sizeof(callback_functions)); + XMEMSET(&client_cbf, 0, sizeof(client_cbf)); + XMEMSET(&server_cbf, 0, sizeof(server_cbf)); #ifdef WOLFSSL_TLS13 client_cbf.method = wolfTLSv1_3_client_method; @@ -7387,8 +7387,8 @@ static int test_wolfSSL_CTX_verifyDepth_ServerClient_2(void) test_ssl_cbf client_cbf; test_ssl_cbf server_cbf; - XMEMSET(&client_cbf, 0, sizeof(callback_functions)); - XMEMSET(&server_cbf, 0, sizeof(callback_functions)); + XMEMSET(&client_cbf, 0, sizeof(client_cbf)); + XMEMSET(&server_cbf, 0, sizeof(server_cbf)); #ifdef WOLFSSL_TLS13 client_cbf.method = wolfTLSv1_3_client_method; @@ -7432,8 +7432,8 @@ static int test_wolfSSL_CTX_verifyDepth_ServerClient_3(void) test_ssl_cbf client_cbf; test_ssl_cbf server_cbf; - XMEMSET(&client_cbf, 0, sizeof(callback_functions)); - XMEMSET(&server_cbf, 0, sizeof(callback_functions)); + XMEMSET(&client_cbf, 0, sizeof(client_cbf)); + XMEMSET(&server_cbf, 0, sizeof(server_cbf)); #ifdef WOLFSSL_TLS13 client_cbf.method = wolfTLSv1_3_client_method; @@ -11494,8 +11494,8 @@ static int test_wolfSSL_X509_TLS_version_test_2(void) test_ssl_cbf func_cb_client; test_ssl_cbf func_cb_server; - XMEMSET(&func_cb_client, 0, sizeof(callback_functions)); - XMEMSET(&func_cb_server, 0, sizeof(callback_functions)); + XMEMSET(&func_cb_client, 0, sizeof(func_cb_client)); + XMEMSET(&func_cb_server, 0, sizeof(func_cb_server)); func_cb_client.ctx_ready = &test_set_x509_badversion; func_cb_server.ctx_ready = &test_set_override_x509; @@ -32872,8 +32872,8 @@ static int test_wolfSSL_Tls13_postauth(void) test_ssl_cbf client_cbf; /* test version failure doing post auth with TLS 1.2 connection */ - XMEMSET(&server_cbf, 0, sizeof(callback_functions)); - XMEMSET(&client_cbf, 0, sizeof(callback_functions)); + XMEMSET(&server_cbf, 0, sizeof(server_cbf)); + XMEMSET(&client_cbf, 0, sizeof(client_cbf)); server_cbf.method = wolfTLSv1_2_server_method; server_cbf.ssl_ready = set_post_auth_cb; server_cbf.on_result = post_auth_version_cb; @@ -32884,8 +32884,8 @@ static int test_wolfSSL_Tls13_postauth(void) &server_cbf, NULL), TEST_SUCCESS); /* tests on post auth with TLS 1.3 */ - XMEMSET(&server_cbf, 0, sizeof(callback_functions)); - XMEMSET(&client_cbf, 0, sizeof(callback_functions)); + XMEMSET(&server_cbf, 0, sizeof(server_cbf)); + XMEMSET(&client_cbf, 0, sizeof(client_cbf)); server_cbf.method = wolfTLSv1_3_server_method; server_cbf.ssl_ready = set_post_auth_cb; client_cbf.ssl_ready = set_post_auth_cb; @@ -34381,8 +34381,8 @@ static int test_wolfSSL_msgCb(void) test_ssl_cbf client_cb; test_ssl_cbf server_cb; - XMEMSET(&client_cb, 0, sizeof(callback_functions)); - XMEMSET(&server_cb, 0, sizeof(callback_functions)); + XMEMSET(&client_cb, 0, sizeof(client_cb)); + XMEMSET(&server_cb, 0, sizeof(server_cb)); #ifndef WOLFSSL_NO_TLS12 client_cb.method = wolfTLSv1_2_client_method; server_cb.method = wolfTLSv1_2_server_method; @@ -39006,8 +39006,8 @@ static int test_wolfSSL_cert_cb(void) test_ssl_cbf func_cb_client; test_ssl_cbf func_cb_server; - XMEMSET(&func_cb_client, 0, sizeof(callback_functions)); - XMEMSET(&func_cb_server, 0, sizeof(callback_functions)); + XMEMSET(&func_cb_client, 0, sizeof(func_cb_client)); + XMEMSET(&func_cb_server, 0, sizeof(func_cb_server)); func_cb_client.ctx_ready = clientCertSetupCb; func_cb_client.ssl_ready = clientCertClearCb; @@ -39376,7 +39376,7 @@ static int test_wolfSSL_CTX_sess_set_remove_cb(void) * session object */ test_ssl_cbf func_cb; - XMEMSET(&func_cb, 0, sizeof(callback_functions)); + XMEMSET(&func_cb, 0, sizeof(func_cb)); func_cb.ctx_ready = SessRemCtxSetupCb; func_cb.on_result = SessRemSslSetupCb; @@ -48660,8 +48660,8 @@ static int test_DhCallbacks(void) &func_cb_server, NULL), TEST_SUCCESS); /* Test fail */ - XMEMSET(&func_cb_client, 0, sizeof(callback_functions)); - XMEMSET(&func_cb_server, 0, sizeof(callback_functions)); + XMEMSET(&func_cb_client, 0, sizeof(func_cb_client)); + XMEMSET(&func_cb_server, 0, sizeof(func_cb_server)); /* set callbacks to use DH functions */ func_cb_client.ctx_ready = &test_dh_ctx_setup; @@ -58937,7 +58937,8 @@ static int test_TLS_13_ticket_different_ciphers(void) #if defined(WOLFSSL_EXTRA_ALERTS) && !defined(WOLFSSL_NO_TLS12) && \ defined(HAVE_IO_TESTS_DEPENDENCIES) -#define TEST_WRONG_CS_CLIENT "TLS_DHE_RSA_WITH_AES_128_CBC_SHA" +#define TEST_WRONG_CS_CLIENT "DHE-RSA-AES128-SHA" +/* AKA TLS_DHE_RSA_WITH_AES_128_CBC_SHA */ byte test_extra_alerts_wrong_cs_sh[] = { 0x16, 0x03, 0x03, 0x00, 0x56, 0x02, 0x00, 0x00, 0x52, 0x03, 0x03, 0xef, @@ -59283,7 +59284,8 @@ static int test_harden_no_secure_renegotiation(void) ExpectIntEQ(client_cbs.return_code, TEST_FAIL); ExpectIntEQ(client_cbs.last_err, SECURE_RENEGOTIATION_E); ExpectIntEQ(server_cbs.return_code, TEST_FAIL); - ExpectIntEQ(server_cbs.last_err, SOCKET_ERROR_E); + ExpectTrue(server_cbs.last_err == SOCKET_ERROR_E || + server_cbs.last_err == FATAL_ERROR); return EXPECT_RESULT(); } @@ -59469,6 +59471,89 @@ static int test_dtls13_bad_epoch_ch(void) } #endif +#if defined(HAVE_SSL_MEMIO_TESTS_DEPENDENCIES) && !defined(NO_SESSION_CACHE) +static int test_short_session_id_ssl_ready(WOLFSSL* ssl) +{ + EXPECT_DECLS; + WOLFSSL_SESSION *sess = NULL; + /* Setup the session to avoid errors */ + ssl->session->timeout = -1; + ssl->session->side = WOLFSSL_CLIENT_END; +#if defined(SESSION_CERTS) || (defined(WOLFSSL_TLS13) && \ + defined(HAVE_SESSION_TICKET)) + ssl->session->version = ssl->version; +#endif + /* Force a short session ID to be sent */ + ssl->session->sessionIDSz = 4; +#ifndef NO_SESSION_CACHE_REF + /* Allow the client cache to be used */ + ssl->session->idLen = 4; +#endif + ssl->session->isSetup = 1; + ExpectNotNull(sess = wolfSSL_get_session(ssl)); + ExpectIntEQ(wolfSSL_set_session(ssl, sess), WOLFSSL_SUCCESS); + return EXPECT_RESULT(); +} + +static int test_short_session_id(void) +{ + EXPECT_DECLS; + test_ssl_cbf client_cbf; + test_ssl_cbf server_cbf; + size_t i; + struct { + method_provider client_meth; + method_provider server_meth; + const char* tls_version; + } params[] = { +#if defined(WOLFSSL_TLS13) && !defined(WOLFSSL_NO_DEF_TICKET_ENC_CB) && \ + defined(HAVE_SESSION_TICKET) && defined(WOLFSSL_TICKET_HAVE_ID) && \ + !defined(WOLFSSL_TLS13_MIDDLEBOX_COMPAT) +/* With WOLFSSL_TLS13_MIDDLEBOX_COMPAT a short ID will result in an error */ + { wolfTLSv1_3_client_method, wolfTLSv1_3_server_method, "TLSv1_3" }, +#ifdef WOLFSSL_DTLS13 + { wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method, "DTLSv1_3" }, +#endif +#endif +#ifndef WOLFSSL_NO_TLS12 + { wolfTLSv1_2_client_method, wolfTLSv1_2_server_method, "TLSv1_2" }, +#ifdef WOLFSSL_DTLS + { wolfDTLSv1_2_client_method, wolfDTLSv1_2_server_method, "DTLSv1_2" }, +#endif +#endif +#if !defined(NO_OLD_TLS) && ((!defined(NO_AES) && !defined(NO_AES_CBC)) || \ + !defined(NO_DES3)) + { wolfTLSv1_1_client_method, wolfTLSv1_1_server_method, "TLSv1_1" }, +#ifdef WOLFSSL_DTLS + { wolfDTLSv1_client_method, wolfDTLSv1_server_method, "DTLSv1_0" }, +#endif +#endif + }; + + printf("\n"); + + for (i = 0; i < sizeof(params)/sizeof(*params) && !EXPECT_FAIL(); i++) { + XMEMSET(&client_cbf, 0, sizeof(client_cbf)); + XMEMSET(&server_cbf, 0, sizeof(server_cbf)); + + printf("\tTesting short ID with %s\n", params[i].tls_version); + + client_cbf.ssl_ready = test_short_session_id_ssl_ready; + client_cbf.method = params[i].client_meth; + server_cbf.method = params[i].server_meth; + + ExpectIntEQ(test_wolfSSL_client_server_nofail_memio(&client_cbf, + &server_cbf, NULL), TEST_SUCCESS); + } + + return EXPECT_RESULT(); +} +#else +static int test_short_session_id(void) +{ + return TEST_SKIPPED; +} +#endif #if defined(HAVE_NULL_CIPHER) && defined(HAVE_IO_TESTS_DEPENDENCIES) && \ defined(WOLFSSL_DTLS13) @@ -60902,10 +60987,12 @@ TEST_CASE testCases[] = { TEST_DECL(test_harden_no_secure_renegotiation), TEST_DECL(test_override_alt_cert_chain), TEST_DECL(test_dtls13_bad_epoch_ch), + TEST_DECL(test_short_session_id), TEST_DECL(test_wolfSSL_dtls13_null_cipher), /* Can't memory test as client/server hangs. */ TEST_DECL(test_dtls_msg_from_other_peer), TEST_DECL(test_dtls_ipv6_check), + /* This test needs to stay at the end to clean up any caches allocated. */ TEST_DECL(test_wolfSSL_Cleanup) }; @@ -61020,7 +61107,6 @@ int ApiTest(void) #ifndef WOLFSSL_UNIT_TEST_NO_TIMING double timeDiff; #endif - EXPECT_DECLS; printf(" Begin API Tests\n"); fflush(stdout); @@ -61042,6 +61128,8 @@ int ApiTest(void) if (res == 0) { for (i = 0; i < TEST_CASE_CNT; ++i) { + EXPECT_DECLS; + /* When not testing all cases then skip if not marked for running. */ if (!testAll && !testCases[i].run) { diff --git a/wolfssl/internal.h b/wolfssl/internal.h index c01913341e..447d04d31d 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -2052,8 +2052,7 @@ WOLFSSL_LOCAL int DoTls13Finished(WOLFSSL* ssl, const byte* input, word32* inOut WOLFSSL_LOCAL int DoApplicationData(WOLFSSL* ssl, byte* input, word32* inOutIdx, int sniff); /* TLS v1.3 needs these */ -WOLFSSL_LOCAL int HandleTlsResumption(WOLFSSL* ssl, int bogusID, - Suites* clSuites); +WOLFSSL_LOCAL int HandleTlsResumption(WOLFSSL* ssl, Suites* clSuites); #ifdef WOLFSSL_TLS13 WOLFSSL_LOCAL byte SuiteMac(const byte* suite); #endif diff --git a/wolfssl/wolfcrypt/types.h b/wolfssl/wolfcrypt/types.h index e42b410465..a342282448 100644 --- a/wolfssl/wolfcrypt/types.h +++ b/wolfssl/wolfcrypt/types.h @@ -507,13 +507,14 @@ typedef struct w64wrapper { #else /* just use plain C stdlib stuff if desired */ #include - #define XMALLOC(s, h, t) malloc((size_t)(s)) + #define XMALLOC(s, h, t) ((void)(h), (void)(t), malloc((size_t)(s))) #ifdef WOLFSSL_XFREE_NO_NULLNESS_CHECK - #define XFREE(p, h, t) free(xp) + #define XFREE(p, h, t) ((void)(h), (void)(t), free(p)) #else #define XFREE(p, h, t) {void* xp = (p); if (xp) free(xp);} #endif - #define XREALLOC(p, n, h, t) realloc((p), (size_t)(n)) + #define XREALLOC(p, n, h, t) \ + ((void)(h), (void)(t), realloc((p), (size_t)(n))) #endif #elif defined(WOLFSSL_LINUXKM)