diff --git a/ChangeLog.d/serialized-data-load-hardening.txt b/ChangeLog.d/serialized-data-load-hardening.txt new file mode 100644 index 0000000000..c06e156fc5 --- /dev/null +++ b/ChangeLog.d/serialized-data-load-hardening.txt @@ -0,0 +1,4 @@ +Bugfix + * Reject serialized TLS 1.2 sessions whose session ID length exceeds 32, + instead of accepting an out-of-range length that is later used to read + past the end of the 32-byte session ID buffer. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0195576213..c37263c1cd 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3254,6 +3254,9 @@ static int ssl_tls12_session_load(mbedtls_ssl_session *session, } session->id_len = *p++; + if (session->id_len > sizeof(session->id)) { + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } memcpy(session->id, p, 32); p += 32; diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index f7876d993e..a2703e1027 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3043,21 +3043,29 @@ depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_SRV_ 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 +depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_CLI_C +ssl_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_SSL_IS_CLIENT:TEST_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 +depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SERVER_NAME_INDICATION +ssl_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_VERSION_TLS1_3: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 +depends_on:MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_SSL_SESSION_TICKETS:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_EARLY_DATA:MBEDTLS_SSL_ALPN +ssl_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_VERSION_TLS1_3: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: +TLS 1.2: Session serialization rejects out-of-bounds session ID length (client) +depends_on:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_CLI_C +ssl_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_VERSION_TLS1_2:MBEDTLS_SSL_IS_CLIENT:TEST_TLS12_SESSION_LOAD_OOB_ID_LEN + +TLS 1.2: Session serialization rejects out-of-bounds session ID length (server) +depends_on:MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_SRV_C +ssl_session_load_rejects_bad_serialized_data:MBEDTLS_SSL_VERSION_TLS1_2:MBEDTLS_SSL_IS_SERVER:TEST_TLS12_SESSION_LOAD_OOB_ID_LEN + 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 c1cf9bab86..4c23a45fbc 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -26,10 +26,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 +/* Mutations for ssl_session_load_rejects_bad_serialized_data */ +#define TEST_SESSION_LOAD_TRAILING_DATA 0 #define TEST_TLS13_SESSION_LOAD_HOSTNAME_NO_NUL 1 #define TEST_TLS13_SESSION_LOAD_ALPN_NO_NUL 2 +#define TEST_TLS12_SESSION_LOAD_OOB_ID_LEN 3 #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_SSL_SESSION_TICKETS) && \ @@ -2918,9 +2919,10 @@ 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) +/* BEGIN_CASE */ +void ssl_session_load_rejects_bad_serialized_data(int tls_version, + int endpoint_type, + int mutation) { mbedtls_ssl_session session, restored; unsigned char *buf = NULL; @@ -2931,12 +2933,34 @@ void ssl_tls13_session_load_rejects_bad_serialized_data(int endpoint_type, const unsigned char *string_to_corrupt = NULL; size_t string_len = 0; + (void) hostname; + (void) alpn; + 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); + switch (tls_version) { +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + case MBEDTLS_SSL_VERSION_TLS1_2: + TEST_EQUAL(mbedtls_test_ssl_tls12_populate_session( + &session, 0, endpoint_type, NULL), 0); + /* Sentinel session ID so we can locate the length byte later. + * The detection code below relies on id_len being exactly + * sizeof(session.id), so force it. */ + memset(session.id, 0x5A, sizeof(session.id)); + session.id_len = sizeof(session.id); + break; +#endif +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS) + case MBEDTLS_SSL_VERSION_TLS1_3: + TEST_EQUAL(mbedtls_test_ssl_tls13_populate_session( + &session, 42, endpoint_type), 0); + break; +#endif + default: + TEST_FAIL("unsupported TLS version"); + } TEST_EQUAL(mbedtls_ssl_session_save(&session, NULL, 0, &len), MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL); @@ -2951,7 +2975,7 @@ void ssl_tls13_session_load_rejects_bad_serialized_data(int endpoint_type, bad_len = len; switch (mutation) { - case TEST_TLS13_SESSION_LOAD_TRAILING_DATA: + case TEST_SESSION_LOAD_TRAILING_DATA: bad_len = len + 1; buf[len] = 0; break; @@ -2966,6 +2990,28 @@ void ssl_tls13_session_load_rejects_bad_serialized_data(int endpoint_type, string_len = sizeof(alpn); break; + case TEST_TLS12_SESSION_LOAD_OOB_ID_LEN: + /* The format writes [id_len][32 id bytes]; find the length byte + * that precedes the sentinel run and push it out of range. */ + for (i = 1; i + sizeof(session.id) <= len; i++) { + size_t k = 0; + if (buf[i - 1] != (unsigned char) sizeof(session.id)) { + continue; + } + while (k < sizeof(session.id) && buf[i + k] == 0x5A) { + k++; + } + if (k == sizeof(session.id)) { + field = buf + i - 1; + break; + } + } + TEST_ASSERT(field != NULL); + /* Smallest value strictly greater than sizeof(id). */ + *field = (unsigned char) (sizeof(session.id) + 1); + field = NULL; + break; + default: TEST_ASSERT(0); }