From e3dac4aaa15c19bc56fc2513cea7f8289931e24b Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 10 Jun 2022 17:21:51 +0200 Subject: [PATCH] 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 c7e00a98a..2ddefc3d4 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 4e39f90cf..55fc95836 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 6144c2fad..27c211fbf 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 e82c66ed4..8ce81d0a1 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 2ab2aaa7c..29a065b26 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 */