From e0a83c328da6b11cce15f70cbfa95acf03432454 Mon Sep 17 00:00:00 2001 From: Nadav0077 <18245584+Nadav0077@users.noreply.github.com> Date: Fri, 22 May 2026 13:07:17 +0300 Subject: [PATCH] Harden serialized data deserialization Reject malformed TLS 1.3 serialized sessions instead of accepting unterminated strings or extra trailing data, and reject serialized SSL contexts whose DTLS Connection ID length exceeds the maximum supported size instead of overflowing the internal buffer. Backport of #10715 (development). Signed-off-by: Nadav0077 <18245584+Nadav0077@users.noreply.github.com> --- .../serialized-data-load-hardening.txt | 5 + library/ssl_tls.c | 31 +++- tests/suites/test_suite_ssl.data | 16 ++ tests/suites/test_suite_ssl.function | 169 ++++++++++++++++++ 4 files changed, 217 insertions(+), 4 deletions(-) create mode 100644 ChangeLog.d/serialized-data-load-hardening.txt diff --git a/ChangeLog.d/serialized-data-load-hardening.txt b/ChangeLog.d/serialized-data-load-hardening.txt new file mode 100644 index 0000000000..e068988dac --- /dev/null +++ b/ChangeLog.d/serialized-data-load-hardening.txt @@ -0,0 +1,5 @@ +Bugfix + * Reject malformed TLS 1.3 serialized sessions instead of accepting + unterminated strings or extra trailing data. + * Reject serialized SSL contexts whose DTLS Connection ID length exceeds + the maximum supported size, instead of overflowing the internal buffer. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e8a44f834c..cb14275d1e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3634,6 +3634,12 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, } if (alpn_len > 0) { + /* The data is about to be used as a null-terminated string, so + * check that it actually is one. */ + if (p[alpn_len - 1] != '\0') { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + int ret = mbedtls_ssl_session_set_ticket_alpn(session, (char *) p); if (ret != 0) { return ret; @@ -3659,11 +3665,16 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } if (hostname_len > 0) { - session->hostname = mbedtls_calloc(1, hostname_len); - if (session->hostname == NULL) { - return MBEDTLS_ERR_SSL_ALLOC_FAILED; + /* The data is about to be used as a null-terminated string, so + * check that it actually is one. */ + if (p[hostname_len - 1] != '\0') { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + + int ret = mbedtls_ssl_session_set_hostname(session, (const char *) p); + if (ret != 0) { + return ret; } - memcpy(session->hostname, p, hostname_len); p += hostname_len; } #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ @@ -3701,6 +3712,10 @@ static int ssl_tls13_session_load(mbedtls_ssl_session *session, } #endif /* MBEDTLS_SSL_CLI_C */ + if (p != end) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + return 0; } @@ -4999,6 +5014,10 @@ static int ssl_context_load(mbedtls_ssl_context *ssl, return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } + if (ssl->transform->in_cid_len > sizeof(ssl->transform->in_cid)) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + memcpy(ssl->transform->in_cid, p, ssl->transform->in_cid_len); p += ssl->transform->in_cid_len; @@ -5008,6 +5027,10 @@ static int ssl_context_load(mbedtls_ssl_context *ssl, return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } + if (ssl->transform->out_cid_len > sizeof(ssl->transform->out_cid)) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + memcpy(ssl->transform->out_cid, p, ssl->transform->out_cid_len); p += ssl->transform->out_cid_len; #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 881d502ed5..aba17ff145 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3028,6 +3028,22 @@ TLS 1.3: SRV: Session serialization, load buffer size depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_SRV_C ssl_serialize_session_load_buf_size:0:"":MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_VERSION_TLS1_3 +TLS 1.3: Session serialization rejects trailing data +depends_on:MBEDTLS_SSL_CLI_C +ssl_tls13_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_IS_CLIENT:TEST_TLS13_SESSION_LOAD_TRAILING_DATA + +TLS 1.3: Session serialization rejects hostname without terminator +depends_on:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SERVER_NAME_INDICATION +ssl_tls13_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_IS_CLIENT:TEST_TLS13_SESSION_LOAD_HOSTNAME_NO_NUL + +TLS 1.3: Session serialization rejects ALPN without terminator +depends_on:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_EARLY_DATA:MBEDTLS_SSL_ALPN +ssl_tls13_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_IS_SERVER:TEST_TLS13_SESSION_LOAD_ALPN_NO_NUL + +SSL context serialization rejects out-of-bounds DTLS CID length +depends_on:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_PROTO_DTLS:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_SSL_CONTEXT_SERIALIZATION:PSA_HAVE_ALG_SOME_RSA_SIGN:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_256:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:TEST_GCM_OR_CHACHAPOLY_ENABLED +ssl_context_load_rejects_oob_cid_length: + Test configuration of EC groups through mbedtls_ssl_conf_groups() conf_group: diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 39457c9047..2000eddc3c 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -25,6 +25,11 @@ #define TEST_EARLY_DATA_NO_INITIAL_ALPN 6 #define TEST_EARLY_DATA_NO_LATER_ALPN 7 +/* Mutations for ssl_tls13_session_load_rejects_bad_serialized_data */ +#define TEST_TLS13_SESSION_LOAD_TRAILING_DATA 0 +#define TEST_TLS13_SESSION_LOAD_HOSTNAME_NO_NUL 1 +#define TEST_TLS13_SESSION_LOAD_ALPN_NO_NUL 2 + #if (!defined(MBEDTLS_SSL_PROTO_TLS1_2)) && \ defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_CLI_C) && \ defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_DEBUG_C) && \ @@ -2733,6 +2738,170 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS */ +void ssl_tls13_session_load_rejects_bad_serialized_data(int endpoint_type, + int mutation) +{ + mbedtls_ssl_session session, restored; + unsigned char *buf = NULL; + size_t len, bad_len, i; + unsigned char *field = NULL; + const unsigned char hostname[] = "hostname example"; + const unsigned char alpn[] = "ALPNExample"; + const unsigned char *string_to_corrupt = NULL; + size_t string_len = 0; + + mbedtls_ssl_session_init(&session); + mbedtls_ssl_session_init(&restored); + USE_PSA_INIT(); + + TEST_EQUAL(mbedtls_test_ssl_tls13_populate_session( + &session, 42, endpoint_type), 0); + + TEST_EQUAL(mbedtls_ssl_session_save(&session, NULL, 0, &len), + MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL); + TEST_CALLOC(buf, len + 1); + TEST_EQUAL(mbedtls_ssl_session_save(&session, buf, len, &len), 0); + + /* The serialized data must load successfully before we corrupt it, + * otherwise the test below would not prove anything. */ + TEST_EQUAL(mbedtls_ssl_session_load(&restored, buf, len), 0); + mbedtls_ssl_session_free(&restored); + mbedtls_ssl_session_init(&restored); + + bad_len = len; + switch (mutation) { + case TEST_TLS13_SESSION_LOAD_TRAILING_DATA: + bad_len = len + 1; + buf[len] = 0; + break; + + case TEST_TLS13_SESSION_LOAD_HOSTNAME_NO_NUL: + string_to_corrupt = hostname; + string_len = sizeof(hostname); + break; + + case TEST_TLS13_SESSION_LOAD_ALPN_NO_NUL: + string_to_corrupt = alpn; + string_len = sizeof(alpn); + break; + + default: + TEST_ASSERT(0); + } + + if (string_to_corrupt != NULL) { + for (i = 0; i + string_len <= len; i++) { + if (memcmp(buf + i, string_to_corrupt, string_len) == 0) { + field = buf + i; + break; + } + } + TEST_ASSERT(field != NULL); + field[string_len - 1] = 'X'; + } + + TEST_EQUAL(mbedtls_ssl_session_load(&restored, buf, bad_len), + MBEDTLS_ERR_SSL_BAD_INPUT_DATA); + +exit: + mbedtls_ssl_session_free(&session); + mbedtls_ssl_session_free(&restored); + mbedtls_free(buf); + USE_PSA_DONE(); +} +/* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_PROTO_DTLS:MBEDTLS_SSL_DTLS_CONNECTION_ID:MBEDTLS_SSL_CONTEXT_SERIALIZATION:PSA_HAVE_ALG_SOME_RSA_SIGN:PSA_WANT_ECC_SECP_R1_384:PSA_WANT_ALG_SHA_256:MBEDTLS_CAN_HANDLE_RSA_TEST_KEY:TEST_GCM_OR_CHACHAPOLY_ENABLED */ +void ssl_context_load_rejects_oob_cid_length(void) +{ + /* Sentinel CID so we can find its location inside the serialized buffer. */ + const unsigned char server_cid[] = { + 0xAB, 0xCD, 0xEF, 0x12, 0x34, 0x56, 0x78, 0x9A + }; + const unsigned char client_cid[] = { + 0x9A, 0x78, 0x56, 0x34, 0x12, 0xEF, 0xCD, 0xAB + }; + + mbedtls_test_handshake_test_options options; + mbedtls_test_ssl_endpoint client_ep, server_ep; + unsigned char *context_buf = NULL; + size_t context_buf_len, i; + unsigned char *cid_len_p = NULL; + + memset(&client_ep, 0, sizeof(client_ep)); + memset(&server_ep, 0, sizeof(server_ep)); + + MD_OR_USE_PSA_INIT(); + mbedtls_test_init_handshake_options(&options); + + options.dtls = 1; + options.expected_negotiated_version = MBEDTLS_SSL_VERSION_TLS1_2; + + TEST_EQUAL(mbedtls_test_ssl_endpoint_init( + &client_ep, MBEDTLS_SSL_IS_CLIENT, &options), 0); + TEST_EQUAL(mbedtls_test_ssl_endpoint_init( + &server_ep, MBEDTLS_SSL_IS_SERVER, &options), 0); + + /* Configure CID with unique patterns so we can locate the length byte in + * the serialized context. Both peers must agree on the CID length via + * conf_cid() before set_cid() will accept the value. */ + TEST_EQUAL(mbedtls_ssl_conf_cid(&client_ep.conf, sizeof(client_cid), + MBEDTLS_SSL_UNEXPECTED_CID_FAIL), 0); + TEST_EQUAL(mbedtls_ssl_conf_cid(&server_ep.conf, sizeof(server_cid), + MBEDTLS_SSL_UNEXPECTED_CID_FAIL), 0); + TEST_EQUAL(mbedtls_ssl_set_cid(&client_ep.ssl, MBEDTLS_SSL_CID_ENABLED, + client_cid, sizeof(client_cid)), 0); + TEST_EQUAL(mbedtls_ssl_set_cid(&server_ep.ssl, MBEDTLS_SSL_CID_ENABLED, + server_cid, sizeof(server_cid)), 0); + + TEST_EQUAL(mbedtls_test_ssl_dtls_join_endpoints(&client_ep, &server_ep), 0); + + TEST_ASSERT(mbedtls_test_ssl_perform_connection(&options, &client_ep, + &server_ep)); + + /* Serialize the post-handshake server context. */ + TEST_EQUAL(mbedtls_ssl_context_save(&server_ep.ssl, NULL, 0, + &context_buf_len), + MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL); + TEST_CALLOC(context_buf, context_buf_len); + TEST_EQUAL(mbedtls_ssl_context_save(&server_ep.ssl, context_buf, + context_buf_len, &context_buf_len), 0); + + /* The serialized format writes [in_cid_len][in_cid bytes]; locate the + * length byte by searching for our sentinel CID. */ + for (i = 1; i + sizeof(server_cid) <= context_buf_len; i++) { + if (context_buf[i - 1] == (unsigned char) sizeof(server_cid) && + memcmp(context_buf + i, server_cid, sizeof(server_cid)) == 0) { + cid_len_p = context_buf + i - 1; + break; + } + } + TEST_ASSERT(cid_len_p != NULL); + + /* Corrupt the length to the smallest value strictly greater than + * sizeof(in_cid), so we exercise the new bounds check rather than the + * pre-existing buffer-size check that would also trip on huge values. */ + *cid_len_p = (unsigned char) (MBEDTLS_SSL_CID_IN_LEN_MAX + 1); + + /* Reinitialise the server SSL context so we can load into a fresh one. */ + mbedtls_ssl_free(&server_ep.ssl); + mbedtls_ssl_init(&server_ep.ssl); + TEST_EQUAL(mbedtls_ssl_setup(&server_ep.ssl, &server_ep.conf), 0); + + TEST_EQUAL(mbedtls_ssl_context_load(&server_ep.ssl, context_buf, + context_buf_len), + MBEDTLS_ERR_SSL_BAD_INPUT_DATA); + +exit: + mbedtls_test_ssl_endpoint_free(&client_ep); + mbedtls_test_ssl_endpoint_free(&server_ep); + mbedtls_test_free_handshake_options(&options); + mbedtls_free(context_buf); + MD_OR_USE_PSA_DONE(); +} +/* END_CASE */ + /* BEGIN_CASE */ void ssl_session_serialize_version_check(int corrupt_major, int corrupt_minor,