From e3dac4aaa15c19bc56fc2513cea7f8289931e24b Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 10 Jun 2022 17:21:51 +0200 Subject: [PATCH 1/6] tls13: Add Certificate msg parsing tests with invalid vector lengths Signed-off-by: Ronald Cron --- library/ssl_tls13_generic.c | 22 ++-- library/ssl_tls13_invasive.h | 4 + tests/scripts/all.sh | 12 ++ tests/suites/test_suite_ssl.data | 3 + tests/suites/test_suite_ssl.function | 175 +++++++++++++++++++++++++++ 5 files changed, 207 insertions(+), 9 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index c7e00a98a8..2ddefc3d44 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -31,6 +31,7 @@ #include #include "ssl_misc.h" +#include "ssl_tls13_invasive.h" #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" @@ -391,9 +392,10 @@ cleanup: /* Parse certificate chain send by the server. */ MBEDTLS_CHECK_RETURN_CRITICAL -static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) +MBEDTLS_STATIC_TESTABLE +int mbedtls_ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t certificate_request_context_len = 0; @@ -524,9 +526,10 @@ exit: } #else MBEDTLS_CHECK_RETURN_CRITICAL -static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) +MBEDTLS_STATIC_TESTABLE +int mbedtls_ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { ((void) ssl); ((void) buf); @@ -657,7 +660,8 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) * with details encoded in the verification flags. All other kinds * of error codes, including those from the user provided f_vrfy * functions, are treated as fatal and lead to a failure of - * ssl_tls13_parse_certificate even if verification was optional. */ + * mbedtls_ssl_tls13_parse_certificate even if verification was optional. + */ if( authmode == MBEDTLS_SSL_VERIFY_OPTIONAL && ( ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED || ret == MBEDTLS_ERR_SSL_BAD_CERTIFICATE ) ) @@ -735,8 +739,8 @@ int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ) &buf, &buf_len ) ); /* Parse the certificate chain sent by the peer. */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate( ssl, buf, - buf + buf_len ) ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_parse_certificate( ssl, buf, + buf + buf_len ) ); /* Validate the certificate chain and set the verification results. */ MBEDTLS_SSL_PROC_CHK( ssl_tls13_validate_certificate( ssl ) ); diff --git a/library/ssl_tls13_invasive.h b/library/ssl_tls13_invasive.h index 4e39f90cfb..55fc95836c 100644 --- a/library/ssl_tls13_invasive.h +++ b/library/ssl_tls13_invasive.h @@ -80,6 +80,10 @@ psa_status_t mbedtls_psa_hkdf_expand( psa_algorithm_t hash_alg, const unsigned char *info, size_t info_len, unsigned char *okm, size_t okm_len ); +MBEDTLS_CHECK_RETURN_CRITICAL +int mbedtls_ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ); #endif /* MBEDTLS_TEST_HOOKS */ #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 6144c2fadc..27c211fbf0 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2904,6 +2904,18 @@ component_test_tls13_only () { if_build_succeeded tests/ssl-opt.sh } +component_test_tls13_only_with_hooks () { + msg "build: default config with MBEDTLS_SSL_PROTO_TLS1_3 and MBEDTLS_TEST_HOOKS, without MBEDTLS_SSL_PROTO_TLS1_2" + scripts/config.py set MBEDTLS_TEST_HOOKS + make CFLAGS="'-DMBEDTLS_USER_CONFIG_FILE=\"../tests/configs/tls13-only.h\"'" + + msg "test: default config with MBEDTLS_SSL_PROTO_TLS1_3 enabled, without MBEDTLS_SSL_PROTO_TLS1_2" + if_build_succeeded make test + + msg "ssl-opt.sh (TLS 1.3)" + if_build_succeeded tests/ssl-opt.sh +} + component_test_tls13 () { msg "build: default config with MBEDTLS_SSL_PROTO_TLS1_3 enabled, without padding" scripts/config.py set MBEDTLS_SSL_PROTO_TLS1_3 diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index e82c66ed43..8ce81d0a1f 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3381,3 +3381,6 @@ cookie_parsing:"16fefd0000000000000000002f010000de000000000000011efefd7b72727272 Cookie parsing: one byte overread cookie_parsing:"16fefd0000000000000000002F010000de000000000000011efefd7b7272727272727272727272727272727272727272727272727272727272727d0001":MBEDTLS_ERR_SSL_DECODE_ERROR + +TLS 1.3 srv Certificate msg - wrong vector lengths +tls13_server_certificate_msg_invalid_vector_len diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 2ab2aaa7c7..29a065b263 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -105,6 +105,7 @@ void init_handshake_options( handshake_test_options *opts ) opts->cli_log_fun = NULL; opts->resize_buffers = 1; } + /* * Buffer structure for custom I/O callbacks. */ @@ -2307,6 +2308,103 @@ exit: } #endif /* MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_ENTROPY_C && MBEDTLS_CTR_DRBG_C */ +/* + * Tweak vector lengths in a TLS 1.3 Certificate message + * + * /param[in] buf Buffer containing the Certificate message to tweak + * /param[in]]out] end End of the buffer to parse + * /param tweak Tweak identifier (from 1 to the number of tweaks). + * /param[out] expected_result Error code expected from the parsing function + * /param[out] args Arguments of the MBEDTLS_SSL_CHK_BUF_READ_PTR call that + * is expected to fail. All zeroes if no + * MBEDTLS_SSL_CHK_BUF_READ_PTR failure is expected. + */ +int tweak_tls13_certificate_msg_vector_len( + unsigned char *buf, unsigned char **end, int tweak, int *expected_result ) +{ +/* + * The definition of the tweaks assume that the certificate list contains only + * one certificate. + */ + +/* + * struct { + * opaque cert_data<1..2^24-1>; + * Extension extensions<0..2^16-1>; + * } CertificateEntry; + * + * struct { + * opaque certificate_request_context<0..2^8-1>; + * CertificateEntry certificate_list<0..2^24-1>; + * } Certificate; + */ + unsigned char *p_certificate_request_context_len = buf; + size_t certificate_request_context_len = buf[0]; + + unsigned char *p_certificate_list_len = buf + 1 + certificate_request_context_len; + unsigned char *certificate_list = p_certificate_list_len + 3; + size_t certificate_list_len = MBEDTLS_GET_UINT24_BE( p_certificate_list_len, 0 ); + + unsigned char *p_cert_data_len = certificate_list; + unsigned char *cert_data = p_cert_data_len + 3; + size_t cert_data_len = MBEDTLS_GET_UINT24_BE( p_cert_data_len, 0 ); + + unsigned char *p_extensions_len = cert_data + cert_data_len; + unsigned char *extensions = p_extensions_len + 2; + size_t extensions_len = MBEDTLS_GET_UINT16_BE( p_extensions_len, 0 ); + + *expected_result = MBEDTLS_ERR_SSL_DECODE_ERROR; + + switch( tweak ) + { + case 1: + /* Failure when checking if the certificate request context length and + * certificate list length can be read + */ + *end = buf + 3; + break; + + case 2: + /* Invalid certificate request context length. + */ + *p_certificate_request_context_len = + certificate_request_context_len + 1; + break; + + case 3: + /* Failure when checking if certificate_list data can be read. */ + MBEDTLS_PUT_UINT24_BE( certificate_list_len + 1, + p_certificate_list_len, 0 ); + break; + + case 4: + /* Failure when checking if the cert_data length can be read. */ + MBEDTLS_PUT_UINT24_BE( 2, p_certificate_list_len, 0 ); + break; + + case 5: + /* Failure when checking if cert_data data can be read. */ + MBEDTLS_PUT_UINT24_BE( certificate_list_len - 3 + 1, + p_cert_data_len, 0 ); + break; + + case 6: + /* Failure when checking if the extensions length can be read. */ + MBEDTLS_PUT_UINT24_BE( certificate_list_len - extensions_len - 1, + p_certificate_list_len, 0 ); + break; + + case 7: + /* Failure when checking if extensions data can be read. */ + MBEDTLS_PUT_UINT16_BE( extensions_len + 1, p_extensions_len, 0 ); + break; + + default: + return( -1 ); + } + + return( 0 ); +} /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -5708,3 +5806,80 @@ exit: USE_PSA_DONE( ); } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS:MBEDTLS_SSL_PROTO_TLS1_3:!MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SRV_C:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED */ +void tls13_server_certificate_msg_invalid_vector_len( ) +{ + int ret = -1; + mbedtls_endpoint client_ep, server_ep; + unsigned char *buf, *end; + size_t buf_len; + int step = 0; + int expected_result; + + /* + * Test set-up + */ + USE_PSA_INIT( ); + + ret = mbedtls_endpoint_init( &client_ep, MBEDTLS_SSL_IS_CLIENT, + MBEDTLS_PK_ECDSA, NULL, NULL, NULL, NULL ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_endpoint_init( &server_ep, MBEDTLS_SSL_IS_SERVER, + MBEDTLS_PK_ECDSA, NULL, NULL, NULL, NULL ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_mock_socket_connect( &(client_ep.socket), + &(server_ep.socket), 1024 ); + TEST_EQUAL( ret, 0 ); + + while( 1 ) + { + mbedtls_test_set_step( ++step ); + + ret = mbedtls_move_handshake_to_state( &(server_ep.ssl), + &(client_ep.ssl), + MBEDTLS_SSL_CERTIFICATE_VERIFY ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_ssl_flush_output( &(server_ep.ssl) ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_move_handshake_to_state( &(client_ep.ssl), + &(server_ep.ssl), + MBEDTLS_SSL_SERVER_CERTIFICATE ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_ssl_tls13_fetch_handshake_msg( &(client_ep.ssl), + MBEDTLS_SSL_HS_CERTIFICATE, + &buf, &buf_len ); + TEST_EQUAL( ret, 0 ); + + end = buf + buf_len; + + /* + * Tweak server Certificate message and parse it. + */ + + ret = tweak_tls13_certificate_msg_vector_len( + buf, &end, step, &expected_result ); + + if( ret != 0 ) + break; + + ret = mbedtls_ssl_tls13_parse_certificate( &(client_ep.ssl), buf, end ); + TEST_EQUAL( ret, expected_result ); + + ret = mbedtls_ssl_session_reset( &(client_ep.ssl) ); + TEST_EQUAL( ret, 0 ); + + ret = mbedtls_ssl_session_reset( &(server_ep.ssl) ); + TEST_EQUAL( ret, 0 ); + } + +exit: + mbedtls_endpoint_free( &client_ep, NULL ); + mbedtls_endpoint_free( &server_ep, NULL ); + USE_PSA_DONE( ); +} +/* END_CASE */ From ad8c17b9c6214a076b48ecdb2d27bf55cafb7490 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 10 Jun 2022 17:18:09 +0200 Subject: [PATCH 2/6] tls: Add overread/overwrite check failure tracking Signed-off-by: Ronald Cron --- library/ssl_misc.h | 25 +++++++++++++++++++++++++ library/ssl_tls.c | 26 ++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 1280241dcb..816beea31e 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -381,11 +381,36 @@ static inline size_t mbedtls_ssl_get_input_buflen( const mbedtls_ssl_context *ct * \return Zero if the needed space is available in the buffer, non-zero * otherwise. */ +#if ! defined(MBEDTLS_TEST_HOOKS) static inline int mbedtls_ssl_chk_buf_ptr( const uint8_t *cur, const uint8_t *end, size_t need ) { return( ( cur > end ) || ( need > (size_t)( end - cur ) ) ); } +#else +typedef struct +{ + const uint8_t *cur; + const uint8_t *end; + size_t need; +} mbedtls_ssl_chk_buf_ptr_args; + +void mbedtls_ssl_set_chk_buf_ptr_fail_args( + const uint8_t *cur, const uint8_t *end, size_t need ); +void mbedtls_ssl_reset_chk_buf_ptr_fail_args( void ); +int mbedtls_ssl_cmp_chk_buf_ptr_fail_args( mbedtls_ssl_chk_buf_ptr_args *args ); + +static inline int mbedtls_ssl_chk_buf_ptr( const uint8_t *cur, + const uint8_t *end, size_t need ) +{ + if( ( cur > end ) || ( need > (size_t)( end - cur ) ) ) + { + mbedtls_ssl_set_chk_buf_ptr_fail_args( cur, end, need ); + return( 1 ); + } + return( 0 ); +} +#endif /** * \brief This macro checks if the remaining size in a buffer is diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 668b5ecae3..55b7f85cec 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -58,6 +58,30 @@ #include "mbedtls/oid.h" #endif +#if defined(MBEDTLS_TEST_HOOKS) +static mbedtls_ssl_chk_buf_ptr_args chk_buf_ptr_fail_args; + +void mbedtls_ssl_set_chk_buf_ptr_fail_args( + const uint8_t *cur, const uint8_t *end, size_t need ) +{ + chk_buf_ptr_fail_args.cur = cur; + chk_buf_ptr_fail_args.end = end; + chk_buf_ptr_fail_args.need = need; +} + +void mbedtls_ssl_reset_chk_buf_ptr_fail_args( void ) +{ + memset( &chk_buf_ptr_fail_args, 0, sizeof( chk_buf_ptr_fail_args ) ); +} + +int mbedtls_ssl_cmp_chk_buf_ptr_fail_args( mbedtls_ssl_chk_buf_ptr_args *args ) +{ + return( ( chk_buf_ptr_fail_args.cur != args->cur ) || + ( chk_buf_ptr_fail_args.end != args->end ) || + ( chk_buf_ptr_fail_args.need != args->need ) ); +} +#endif /* MBEDTLS_TEST_HOOKS */ + #if defined(MBEDTLS_SSL_PROTO_DTLS) #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) @@ -1103,6 +1127,8 @@ void mbedtls_ssl_session_reset_msg_layer( mbedtls_ssl_context *ssl, memset( ssl->in_buf, 0, in_buf_len ); } + ssl->send_alert = 0; + /* Reset outgoing message writing */ ssl->out_msgtype = 0; ssl->out_msglen = 0; From e7b9b6b3805a1217b7c8922891f29d48dd9ec041 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 10 Jun 2022 17:24:31 +0200 Subject: [PATCH 3/6] tls13: Add checks of overread check failures In Certificate message parsing tests with invalid vector lengths, add checks that the parsing failed on the expected overread check. Signed-off-by: Ronald Cron --- tests/suites/test_suite_ssl.function | 43 ++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 29a065b263..97eccbd24d 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -106,6 +106,22 @@ void init_handshake_options( handshake_test_options *opts ) opts->resize_buffers = 1; } +#if defined(MBEDTLS_TEST_HOOKS) +static void set_chk_buf_ptr_args( + mbedtls_ssl_chk_buf_ptr_args *args, + unsigned char *cur, unsigned char *end, size_t need ) +{ + args->cur = cur; + args->end = end; + args->need = need; +} + +static void reset_chk_buf_ptr_args( mbedtls_ssl_chk_buf_ptr_args *args ) +{ + memset( args, 0, sizeof( *args ) ); +} +#endif /* MBEDTLS_TEST_HOOKS */ + /* * Buffer structure for custom I/O callbacks. */ @@ -2308,6 +2324,7 @@ exit: } #endif /* MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_ENTROPY_C && MBEDTLS_CTR_DRBG_C */ +#if defined(MBEDTLS_TEST_HOOKS) /* * Tweak vector lengths in a TLS 1.3 Certificate message * @@ -2320,7 +2337,8 @@ exit: * MBEDTLS_SSL_CHK_BUF_READ_PTR failure is expected. */ int tweak_tls13_certificate_msg_vector_len( - unsigned char *buf, unsigned char **end, int tweak, int *expected_result ) + unsigned char *buf, unsigned char **end, int tweak, + int *expected_result, mbedtls_ssl_chk_buf_ptr_args *args ) { /* * The definition of the tweaks assume that the certificate list contains only @@ -2362,6 +2380,7 @@ int tweak_tls13_certificate_msg_vector_len( * certificate list length can be read */ *end = buf + 3; + set_chk_buf_ptr_args( args, buf, *end, 4 ); break; case 2: @@ -2369,34 +2388,46 @@ int tweak_tls13_certificate_msg_vector_len( */ *p_certificate_request_context_len = certificate_request_context_len + 1; + reset_chk_buf_ptr_args( args ); break; case 3: /* Failure when checking if certificate_list data can be read. */ MBEDTLS_PUT_UINT24_BE( certificate_list_len + 1, p_certificate_list_len, 0 ); + set_chk_buf_ptr_args( args, certificate_list, *end, + certificate_list_len + 1 ); break; case 4: /* Failure when checking if the cert_data length can be read. */ MBEDTLS_PUT_UINT24_BE( 2, p_certificate_list_len, 0 ); + set_chk_buf_ptr_args( args, p_cert_data_len, certificate_list + 2, 3 ); break; case 5: /* Failure when checking if cert_data data can be read. */ MBEDTLS_PUT_UINT24_BE( certificate_list_len - 3 + 1, p_cert_data_len, 0 ); + set_chk_buf_ptr_args( args, cert_data, + certificate_list + certificate_list_len, + certificate_list_len - 3 + 1 ); break; case 6: /* Failure when checking if the extensions length can be read. */ MBEDTLS_PUT_UINT24_BE( certificate_list_len - extensions_len - 1, p_certificate_list_len, 0 ); + set_chk_buf_ptr_args( args, p_extensions_len, + certificate_list + certificate_list_len - extensions_len - 1, 2 ); break; case 7: /* Failure when checking if extensions data can be read. */ MBEDTLS_PUT_UINT16_BE( extensions_len + 1, p_extensions_len, 0 ); + + set_chk_buf_ptr_args( args, extensions, + certificate_list + certificate_list_len, extensions_len + 1 ); break; default: @@ -2405,6 +2436,7 @@ int tweak_tls13_certificate_msg_vector_len( return( 0 ); } +#endif /* MBEDTLS_TEST_HOOKS */ /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -5815,6 +5847,7 @@ void tls13_server_certificate_msg_invalid_vector_len( ) size_t buf_len; int step = 0; int expected_result; + mbedtls_ssl_chk_buf_ptr_args expected_chk_buf_ptr_args; /* * Test set-up @@ -5862,7 +5895,7 @@ void tls13_server_certificate_msg_invalid_vector_len( ) */ ret = tweak_tls13_certificate_msg_vector_len( - buf, &end, step, &expected_result ); + buf, &end, step, &expected_result, &expected_chk_buf_ptr_args ); if( ret != 0 ) break; @@ -5870,6 +5903,11 @@ void tls13_server_certificate_msg_invalid_vector_len( ) ret = mbedtls_ssl_tls13_parse_certificate( &(client_ep.ssl), buf, end ); TEST_EQUAL( ret, expected_result ); + TEST_ASSERT( mbedtls_ssl_cmp_chk_buf_ptr_fail_args( + &expected_chk_buf_ptr_args ) == 0 ); + + mbedtls_ssl_reset_chk_buf_ptr_fail_args( ); + ret = mbedtls_ssl_session_reset( &(client_ep.ssl) ); TEST_EQUAL( ret, 0 ); @@ -5878,6 +5916,7 @@ void tls13_server_certificate_msg_invalid_vector_len( ) } exit: + mbedtls_ssl_reset_chk_buf_ptr_fail_args( ); mbedtls_endpoint_free( &client_ep, NULL ); mbedtls_endpoint_free( &server_ep, NULL ); USE_PSA_DONE( ); From 2b1a43c1011a03ae0050273b390cccd6d78853fc Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 10 Jun 2022 17:03:54 +0200 Subject: [PATCH 4/6] tls13: Add missing overread check in Certificate msg parsing. Signed-off-by: Ronald Cron --- library/ssl_tls13_generic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 2ddefc3d44..7d0559bc22 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -446,6 +446,7 @@ int mbedtls_ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, mbedtls_x509_crt_init( ssl->session_negotiate->peer_cert ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, certificate_list_len ); certificate_list_end = p + certificate_list_len; while( p < certificate_list_end ) { From e0d7367a9ed3e6096914686c19f8175d00de44a5 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 17 Jun 2022 15:38:26 +0200 Subject: [PATCH 5/6] Add change log Signed-off-by: Ronald Cron --- ChangeLog.d/tls13-add-missing-overread-check.txt | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 ChangeLog.d/tls13-add-missing-overread-check.txt diff --git a/ChangeLog.d/tls13-add-missing-overread-check.txt b/ChangeLog.d/tls13-add-missing-overread-check.txt new file mode 100644 index 0000000000..4552cd735f --- /dev/null +++ b/ChangeLog.d/tls13-add-missing-overread-check.txt @@ -0,0 +1,8 @@ +Security + * Fix a buffer overread in TLS 1.3 Certificate parsing. An unauthenticated + client or server could cause an MbedTLS server or client to overread up + to 64 kBytes of data and potentially overread the input buffer by that + amount minus the size of the input buffer. As overread data undergoes + various checks, the likelihood of reaching the boundary of the input + buffer is rather small but increases as its size + MBEDTLS_SSL_IN_CONTENT_LEN decreases. From cf600bc07c19bbe104636baee09fe73612d615d0 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 17 Jun 2022 15:54:16 +0200 Subject: [PATCH 6/6] Comment fixes Signed-off-by: Ronald Cron --- library/ssl_misc.h | 2 +- tests/suites/test_suite_ssl.function | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 816beea31e..c17fa20f3d 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -410,7 +410,7 @@ static inline int mbedtls_ssl_chk_buf_ptr( const uint8_t *cur, } return( 0 ); } -#endif +#endif /* MBEDTLS_TEST_HOOKS */ /** * \brief This macro checks if the remaining size in a buffer is diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 97eccbd24d..59c69c6afe 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -2328,11 +2328,11 @@ exit: /* * Tweak vector lengths in a TLS 1.3 Certificate message * - * /param[in] buf Buffer containing the Certificate message to tweak - * /param[in]]out] end End of the buffer to parse - * /param tweak Tweak identifier (from 1 to the number of tweaks). - * /param[out] expected_result Error code expected from the parsing function - * /param[out] args Arguments of the MBEDTLS_SSL_CHK_BUF_READ_PTR call that + * \param[in] buf Buffer containing the Certificate message to tweak + * \param[in]]out] end End of the buffer to parse + * \param tweak Tweak identifier (from 1 to the number of tweaks). + * \param[out] expected_result Error code expected from the parsing function + * \param[out] args Arguments of the MBEDTLS_SSL_CHK_BUF_READ_PTR call that * is expected to fail. All zeroes if no * MBEDTLS_SSL_CHK_BUF_READ_PTR failure is expected. */