From 116f50cd963b24534963c1bb0ffa788a43d6109a Mon Sep 17 00:00:00 2001 From: Leonid Rozenboim Date: Thu, 21 Apr 2022 13:05:10 -0700 Subject: [PATCH 01/19] Fix resource leaks These potential leaks were flagged by the Coverity static analyzer. Signed-off-by: Leonid Rozenboim --- library/pkparse.c | 4 +++- library/ssl_cache.c | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/library/pkparse.c b/library/pkparse.c index 68727ec7e..bb5824fc3 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1456,8 +1456,10 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, if( ( pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_RSA ) ) == NULL ) return( MBEDTLS_ERR_PK_UNKNOWN_PK_ALG ); - if( ( ret = mbedtls_pk_setup( ctx, pk_info ) ) != 0 ) + if( ( ret = mbedtls_pk_setup( ctx, pk_info ) ) != 0 ) { + mbedtls_pem_free( &pem ); return( ret ); + } if ( ( ret = pk_get_rsapubkey( &p, p + pem.buflen, mbedtls_pk_rsa( *ctx ) ) ) != 0 ) mbedtls_pk_free( ctx ); diff --git a/library/ssl_cache.c b/library/ssl_cache.c index fe4f30cf8..456f41cef 100644 --- a/library/ssl_cache.c +++ b/library/ssl_cache.c @@ -312,7 +312,11 @@ exit: #endif if( session_serialized != NULL ) + { mbedtls_platform_zeroize( session_serialized, session_serialized_len ); + mbedtls_free(session_serialized); + session_serialized = NULL; + } return( ret ); } From 56e01f37a866ef7c3df1f9abac6a7525447ad5f9 Mon Sep 17 00:00:00 2001 From: Leonid Rozenboim Date: Fri, 22 Apr 2022 16:36:24 -0700 Subject: [PATCH 02/19] Created customary ChangeLog.d entry. Signed-off-by: Leonid Rozenboim --- ChangeLog.d/fix_some_resource_leaks.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/fix_some_resource_leaks.txt diff --git a/ChangeLog.d/fix_some_resource_leaks.txt b/ChangeLog.d/fix_some_resource_leaks.txt new file mode 100644 index 000000000..352e2ba77 --- /dev/null +++ b/ChangeLog.d/fix_some_resource_leaks.txt @@ -0,0 +1,4 @@ +Bugfix + * Fixed resource leaks in mbedtls_pk_parse_public_key(), + mbedtls_ssl_cache_set() for certain error conditions, + flagged by a static analyzer. From 072d2b094d32e74cfb5150b8d9e95bc6fc2e8986 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 13 May 2022 17:08:36 +0100 Subject: [PATCH 03/19] Add pem_free() to other error paths in pk_parse_public_key() Signed-off-by: Paul Elliott --- library/pkparse.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index bb5824fc3..11afd0bdf 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1453,10 +1453,14 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, if( ret == 0 ) { p = pem.buf; - if( ( pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_RSA ) ) == NULL ) + if( ( pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_RSA )) == NULL ) + { + mbedtls_pem_free( &pem ); return( MBEDTLS_ERR_PK_UNKNOWN_PK_ALG ); + } - if( ( ret = mbedtls_pk_setup( ctx, pk_info ) ) != 0 ) { + if( ( ret = mbedtls_pk_setup( ctx, pk_info ) ) != 0 ) + { mbedtls_pem_free( &pem ); return( ret ); } From b7aba1a584c09bf1469632ba2b049ecc791221cc Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 13 May 2022 17:17:30 +0100 Subject: [PATCH 04/19] Improve Changelog Signed-off-by: Paul Elliott --- ChangeLog.d/fix_some_resource_leaks.txt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ChangeLog.d/fix_some_resource_leaks.txt b/ChangeLog.d/fix_some_resource_leaks.txt index 352e2ba77..fcc90869c 100644 --- a/ChangeLog.d/fix_some_resource_leaks.txt +++ b/ChangeLog.d/fix_some_resource_leaks.txt @@ -1,4 +1,6 @@ Bugfix - * Fixed resource leaks in mbedtls_pk_parse_public_key(), - mbedtls_ssl_cache_set() for certain error conditions, - flagged by a static analyzer. + * Fixed resource leaks in mbedtls_pk_parse_public_key() under + certain error conditions. +Security + * Fix potential memory leak inside mbedtls_ssl_cache_set() with + invalid session id len. From 780dc18f74a08a066f819867d86bcb3b5669192c Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Fri, 10 Jun 2022 08:57:19 -0400 Subject: [PATCH 05/19] Refactor test_suite_ssl tests to enable cache setting Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 111 +++++++++++++++++++-------- 1 file changed, 77 insertions(+), 34 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index efec5ea46..2101ed8ff 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -9,6 +9,10 @@ #include #include "test/certs.h" +#if defined(MBEDTLS_SSL_CACHE_C) +#include "mbedtls/ssl_cache.h" +#endif + #include #include @@ -82,6 +86,9 @@ typedef struct handshake_test_options void (*srv_log_fun)(void *, int, const char *, int, const char *); void (*cli_log_fun)(void *, int, const char *, int, const char *); int resize_buffers; +#if defined(MBEDTLS_SSL_CACHE_C) + mbedtls_ssl_cache_context* cache; +#endif } handshake_test_options; void init_handshake_options( handshake_test_options *opts ) @@ -114,6 +121,20 @@ void init_handshake_options( handshake_test_options *opts ) opts->srv_log_fun = NULL; opts->cli_log_fun = NULL; opts->resize_buffers = 1; +#if defined(MBEDTLS_SSL_CACHE_C) + opts->cache = malloc( sizeof( mbedtls_ssl_cache_context ) ); + mbedtls_ssl_cache_init( opts->cache ); +#endif +} + +void free_handshake_options( handshake_test_options *opts ) +{ +#if defined(MBEDTLS_SSL_CACHE_C) + mbedtls_ssl_cache_free( opts->cache ); + free( opts->cache ); +#else + (void) opts; +#endif } /* * Buffer structure for custom I/O callbacks. @@ -918,9 +939,8 @@ exit: * * \retval 0 on success, otherwise error code. */ - -int mbedtls_endpoint_init( mbedtls_endpoint *ep, int endpoint_type, int pk_alg, - int opaque_alg, int opaque_alg2, int opaque_usage, +int mbedtls_endpoint_init( mbedtls_endpoint *ep, int endpoint_type, + handshake_test_options* options, mbedtls_test_message_socket_context *dtls_context, mbedtls_test_message_queue *input_queue, mbedtls_test_message_queue *output_queue, @@ -1002,6 +1022,15 @@ int mbedtls_endpoint_init( mbedtls_endpoint *ep, int endpoint_type, int pk_alg, mbedtls_ssl_conf_authmode( &( ep->conf ), MBEDTLS_SSL_VERIFY_REQUIRED ); +#if defined(MBEDTLS_SSL_CACHE_C) + if( endpoint_type == MBEDTLS_SSL_IS_SERVER && options->cache != NULL ) + { + mbedtls_ssl_conf_session_cache( &( ep->conf ), options->cache, + mbedtls_ssl_cache_get, + mbedtls_ssl_cache_set ); + } +#endif + ret = mbedtls_ssl_setup( &( ep->ssl ), &( ep->conf ) ); TEST_ASSERT( ret == 0 ); @@ -1010,8 +1039,10 @@ int mbedtls_endpoint_init( mbedtls_endpoint *ep, int endpoint_type, int pk_alg, mbedtls_ssl_conf_dtls_cookies( &( ep->conf ), NULL, NULL, NULL ); #endif - ret = mbedtls_endpoint_certificate_init( ep, pk_alg, opaque_alg, - opaque_alg2, opaque_usage ); + ret = mbedtls_endpoint_certificate_init( ep, options->pk_alg, + options->opaque_alg, + options->opaque_alg2, + options->opaque_usage ); TEST_ASSERT( ret == 0 ); TEST_EQUAL( mbedtls_ssl_conf_get_user_data_n( &ep->conf ), user_data_n ); @@ -1984,11 +2015,7 @@ void perform_handshake( handshake_test_options* options ) if( options->dtls != 0 ) { TEST_ASSERT( mbedtls_endpoint_init( &client, MBEDTLS_SSL_IS_CLIENT, - options->pk_alg, - options->opaque_alg, - options->opaque_alg2, - options->opaque_usage, - &client_context, + options, &client_context, &client_queue, &server_queue, NULL ) == 0 ); #if defined(MBEDTLS_TIMING_C) @@ -2000,11 +2027,7 @@ void perform_handshake( handshake_test_options* options ) else { TEST_ASSERT( mbedtls_endpoint_init( &client, MBEDTLS_SSL_IS_CLIENT, - options->pk_alg, - options->opaque_alg, - options->opaque_alg2, - options->opaque_usage, - NULL, NULL, + options, NULL, NULL, NULL, NULL ) == 0 ); } @@ -2038,11 +2061,7 @@ void perform_handshake( handshake_test_options* options ) if( options->dtls != 0 ) { TEST_ASSERT( mbedtls_endpoint_init( &server, MBEDTLS_SSL_IS_SERVER, - options->pk_alg, - options->opaque_alg, - options->opaque_alg2, - options->opaque_usage, - &server_context, + options, &server_context, &server_queue, &client_queue, NULL ) == 0 ); #if defined(MBEDTLS_TIMING_C) @@ -2054,12 +2073,8 @@ void perform_handshake( handshake_test_options* options ) else { TEST_ASSERT( mbedtls_endpoint_init( &server, MBEDTLS_SSL_IS_SERVER, - options->pk_alg, - options->opaque_alg, - options->opaque_alg2, - options->opaque_usage, - NULL, NULL, - NULL, NULL ) == 0 ); + options, NULL, NULL, NULL, + NULL ) == 0 ); } mbedtls_ssl_conf_authmode( &server.conf, options->srv_auth_mode ); @@ -4771,20 +4786,24 @@ void mbedtls_endpoint_sanity( int endpoint_type ) enum { BUFFSIZE = 1024 }; mbedtls_endpoint ep; int ret = -1; + handshake_test_options options; + init_handshake_options( &options ); + options.pk_alg = MBEDTLS_PK_RSA; - ret = mbedtls_endpoint_init( NULL, endpoint_type, MBEDTLS_PK_RSA, - 0, 0, 0, NULL, NULL, NULL, NULL ); + ret = mbedtls_endpoint_init( NULL, endpoint_type, &options, + NULL, NULL, NULL, NULL ); TEST_ASSERT( MBEDTLS_ERR_SSL_BAD_INPUT_DATA == ret ); - ret = mbedtls_endpoint_certificate_init( NULL, MBEDTLS_PK_RSA, 0, 0, 0 ); + ret = mbedtls_endpoint_certificate_init( NULL, options.pk_alg, 0, 0, 0 ); TEST_ASSERT( MBEDTLS_ERR_SSL_BAD_INPUT_DATA == ret ); - ret = mbedtls_endpoint_init( &ep, endpoint_type, MBEDTLS_PK_RSA, 0, 0, 0, + ret = mbedtls_endpoint_init( &ep, endpoint_type, &options, NULL, NULL, NULL, NULL ); TEST_ASSERT( ret == 0 ); exit: mbedtls_endpoint_free( &ep, NULL ); + free_handshake_options( &options ); } /* END_CASE */ @@ -4794,18 +4813,21 @@ void move_handshake_to_state(int endpoint_type, int state, int need_pass) enum { BUFFSIZE = 1024 }; mbedtls_endpoint base_ep, second_ep; int ret = -1; + handshake_test_options options; + init_handshake_options( &options ); + options.pk_alg = MBEDTLS_PK_RSA; USE_PSA_INIT( ); - ret = mbedtls_endpoint_init( &base_ep, endpoint_type, MBEDTLS_PK_RSA, - 0, 0, 0, NULL, NULL, NULL, NULL ); + ret = mbedtls_endpoint_init( &base_ep, endpoint_type, &options, + NULL, NULL, NULL, NULL ); TEST_ASSERT( ret == 0 ); ret = mbedtls_endpoint_init( &second_ep, ( endpoint_type == MBEDTLS_SSL_IS_SERVER ) ? MBEDTLS_SSL_IS_CLIENT : MBEDTLS_SSL_IS_SERVER, - MBEDTLS_PK_RSA, 0, 0, 0, - NULL, NULL, NULL, NULL ); + &options, NULL, NULL, NULL, NULL ); + TEST_ASSERT( ret == 0 ); ret = mbedtls_mock_socket_connect( &(base_ep.socket), @@ -4832,6 +4854,7 @@ void move_handshake_to_state(int endpoint_type, int state, int need_pass) } exit: + free_handshake_options( &options ); mbedtls_endpoint_free( &base_ep, NULL ); mbedtls_endpoint_free( &second_ep, NULL ); USE_PSA_DONE( ); @@ -4931,8 +4954,12 @@ void app_data( int mfl, int cli_msg_len, int srv_msg_len, #endif perform_handshake( &options ); + /* The goto below is used to avoid an "unused label" warning.*/ goto exit; + +exit: + free_handshake_options( &options ); } /* END_CASE */ @@ -5007,6 +5034,9 @@ void handshake_fragmentation( int mfl, int expected_srv_hs_fragmentation, int ex { TEST_ASSERT( cli_pattern.counter >= 1 ); } + +exit: + free_handshake_options( &options ); } /* END_CASE */ @@ -5021,8 +5051,12 @@ void renegotiation( int legacy_renegotiation ) options.dtls = 1; perform_handshake( &options ); + /* The goto below is used to avoid an "unused label" warning.*/ goto exit; + +exit: + free_handshake_options( &options ); } /* END_CASE */ @@ -5042,8 +5076,11 @@ void resize_buffers( int mfl, int renegotiation, int legacy_renegotiation, options.resize_buffers = 1; perform_handshake( &options ); + /* The goto below is used to avoid an "unused label" warning.*/ goto exit; +exit: + free_handshake_options( &options ); } /* END_CASE */ @@ -5055,6 +5092,9 @@ void resize_buffers_serialize_mfl( int mfl ) /* The goto below is used to avoid an "unused label" warning.*/ goto exit; + +exit: + free_handshake_options( &options ); } /* END_CASE */ @@ -5066,6 +5106,9 @@ void resize_buffers_renegotiate_mfl( int mfl, int legacy_renegotiation, /* The goto below is used to avoid an "unused label" warning.*/ goto exit; + +exit: + free_handshake_options( &options ); } /* END_CASE */ From 514683abdc29602ef66f64b765121ca7fd568e74 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Fri, 10 Jun 2022 10:33:05 -0400 Subject: [PATCH 06/19] Add a test with a bad session_id_len that makes cache setting fail Force a bad session_id_len before handshake wrapup. This should result in a forced jump to a clean up of a serialized session. Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.data | 3 ++ tests/suites/test_suite_ssl.function | 68 ++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 1b36d324e..a851d5c74 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3426,3 +3426,6 @@ raw_key_agreement_fail:0 Raw key agreement: bad server key raw_key_agreement_fail:1 + +Force a bad session id length +force_bad_session_id_len diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 2101ed8ff..93202bfa5 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -5502,6 +5502,74 @@ void conf_group() } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_CACHE_C:MBEDTLS_DEBUG_C */ +void force_bad_session_id_len( ) +{ + enum { BUFFSIZE = 1024 }; + handshake_test_options options; + mbedtls_endpoint client, server; + log_pattern srv_pattern, cli_pattern; + mbedtls_test_message_socket_context server_context, client_context; + + srv_pattern.pattern = cli_pattern.pattern = "cache did not store session"; + srv_pattern.counter = 0; + init_handshake_options( &options ); + + options.srv_log_obj = &srv_pattern; + options.srv_log_fun = log_analyzer; + + USE_PSA_INIT( ); + + mbedtls_message_socket_init( &server_context ); + mbedtls_message_socket_init( &client_context ); + + TEST_ASSERT( mbedtls_endpoint_init( &client, MBEDTLS_SSL_IS_CLIENT, + &options, NULL, NULL, + NULL ) == 0 ); + + TEST_ASSERT( mbedtls_endpoint_init( &server, MBEDTLS_SSL_IS_SERVER, + &options, NULL, NULL, NULL ) == 0 ); + + mbedtls_debug_set_threshold( 1 ); + mbedtls_ssl_conf_dbg( &server.conf, options.srv_log_fun, + options.srv_log_obj ); + + TEST_ASSERT( mbedtls_mock_socket_connect( &(client.socket), + &(server.socket), + BUFFSIZE ) == 0 ); + + TEST_ASSERT( mbedtls_move_handshake_to_state( &(client.ssl), + &(server.ssl), + MBEDTLS_SSL_HANDSHAKE_WRAPUP ) + == 0 ); + /* Force a bad session_id_len that will be read by the server in + * mbedtls_ssl_cache_set. */ + server.ssl.session_negotiate->id_len = 33; + if( options.cli_msg_len != 0 || options.srv_msg_len != 0 ) + { + /* Start data exchanging test */ + TEST_ASSERT( mbedtls_exchange_data( &(client.ssl), options.cli_msg_len, + options.expected_cli_fragments, + &(server.ssl), options.srv_msg_len, + options.expected_srv_fragments ) + == 0 ); + } + TEST_ASSERT( mbedtls_ssl_conf_get_user_data_p( &client.conf ) == &client ); + TEST_ASSERT( mbedtls_ssl_get_user_data_p( &client.ssl ) == &client ); + TEST_ASSERT( mbedtls_ssl_conf_get_user_data_p( &server.conf ) == &server ); + TEST_ASSERT( mbedtls_ssl_get_user_data_p( &server.ssl ) == &server ); + + /* Make sure that the cache did not store the session */ + TEST_EQUAL( srv_pattern.counter, 1 ); +exit: + mbedtls_endpoint_free( &client, NULL ); + mbedtls_endpoint_free( &server, NULL ); + free_handshake_options( &options ); + mbedtls_debug_set_threshold( 0 ); + USE_PSA_DONE( ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_TIMING_C:MBEDTLS_HAVE_TIME */ void timing_final_delay_accessor( ) { From 9abad0c5efe731234200527a66393ec24f10c17b Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Fri, 10 Jun 2022 10:40:54 -0400 Subject: [PATCH 07/19] Improve the changelog message to contain more details Signed-off-by: Andrzej Kurek --- ChangeLog.d/fix_some_resource_leaks.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ChangeLog.d/fix_some_resource_leaks.txt b/ChangeLog.d/fix_some_resource_leaks.txt index fcc90869c..fb36561bb 100644 --- a/ChangeLog.d/fix_some_resource_leaks.txt +++ b/ChangeLog.d/fix_some_resource_leaks.txt @@ -1,6 +1,6 @@ Bugfix - * Fixed resource leaks in mbedtls_pk_parse_public_key() under - certain error conditions. + * Fixed resource leaks in mbedtls_pk_parse_public_key() in low + memory conditions. Security * Fix potential memory leak inside mbedtls_ssl_cache_set() with - invalid session id len. + an invalid session id length. From 626a931bb954ac6147597236b632f4ee701a4357 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Fri, 10 Jun 2022 11:07:39 -0400 Subject: [PATCH 08/19] test_suite_ssl: Add missing arguments in endpoint initialization Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 93202bfa5..3f19cf301 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -5525,10 +5525,11 @@ void force_bad_session_id_len( ) TEST_ASSERT( mbedtls_endpoint_init( &client, MBEDTLS_SSL_IS_CLIENT, &options, NULL, NULL, - NULL ) == 0 ); + NULL, NULL ) == 0 ); TEST_ASSERT( mbedtls_endpoint_init( &server, MBEDTLS_SSL_IS_SERVER, - &options, NULL, NULL, NULL ) == 0 ); + &options, NULL, NULL, NULL, + NULL ) == 0 ); mbedtls_debug_set_threshold( 1 ); mbedtls_ssl_conf_dbg( &server.conf, options.srv_log_fun, @@ -5669,22 +5670,26 @@ void raw_key_agreement_fail( int bad_server_ecdhe_key ) mbedtls_endpoint client, server; mbedtls_psa_stats_t stats; size_t free_slots_before = -1; + handshake_test_options options; uint16_t iana_tls_group_list[] = { MBEDTLS_SSL_IANA_TLS_GROUP_SECP256R1, MBEDTLS_SSL_IANA_TLS_GROUP_NONE }; USE_PSA_INIT( ); + init_handshake_options( &options ); + options.pk_alg = MBEDTLS_PK_ECDSA; + /* Client side, force SECP256R1 to make one key bitflip fail * the raw key agreement. Flipping the first byte makes the * required 0x04 identifier invalid. */ TEST_EQUAL( mbedtls_endpoint_init( &client, MBEDTLS_SSL_IS_CLIENT, - MBEDTLS_PK_ECDSA, 0, 0, 0, NULL, NULL, + &options, NULL, NULL, NULL, iana_tls_group_list ), 0 ); /* Server side */ TEST_EQUAL( mbedtls_endpoint_init( &server, MBEDTLS_SSL_IS_SERVER, - MBEDTLS_PK_ECDSA, 0, 0, 0, - NULL, NULL, NULL, NULL ), 0 ); + &options, NULL, NULL, + NULL, NULL ), 0 ); TEST_EQUAL( mbedtls_mock_socket_connect( &(client.socket), &(server.socket), @@ -5722,6 +5727,7 @@ void raw_key_agreement_fail( int bad_server_ecdhe_key ) exit: mbedtls_endpoint_free( &client, NULL ); mbedtls_endpoint_free( &server, NULL ); + free_handshake_options( &options ); USE_PSA_DONE( ); } From ed58b50ea61b365e986e85c76460bc4792ee4953 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Fri, 10 Jun 2022 19:24:05 -0400 Subject: [PATCH 09/19] test_suite_ssl: add missing MBEDTLS_SSL_SERVER_C dependency Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 3f19cf301..03cfe1551 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -1022,7 +1022,7 @@ int mbedtls_endpoint_init( mbedtls_endpoint *ep, int endpoint_type, mbedtls_ssl_conf_authmode( &( ep->conf ), MBEDTLS_SSL_VERIFY_REQUIRED ); -#if defined(MBEDTLS_SSL_CACHE_C) +#if defined(MBEDTLS_SSL_CACHE_C) && defined(MBEDTLS_SSL_SRV_C) if( endpoint_type == MBEDTLS_SSL_IS_SERVER && options->cache != NULL ) { mbedtls_ssl_conf_session_cache( &( ep->conf ), options->cache, From 6e518ab086024f3b1dcb7925d30843dba85130d2 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Sat, 11 Jun 2022 05:08:38 -0400 Subject: [PATCH 10/19] test_suite_ssl: add missing options cleanup Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 03cfe1551..e53b98099 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -4998,6 +4998,8 @@ void handshake_serialization( ) perform_handshake( &options ); /* The goto below is used to avoid an "unused label" warning.*/ goto exit; +exit: + free_handshake_options( &options ); } /* END_CASE */ From 456a109edbbc73d4094036c6ada105164c8e0837 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Sat, 11 Jun 2022 06:27:05 -0400 Subject: [PATCH 11/19] test_suite_ssl: add required dependencies for default handshake parameters Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index e53b98099..4a1b6af37 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -5504,7 +5504,7 @@ void conf_group() } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_CACHE_C:MBEDTLS_DEBUG_C */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_CACHE_C:MBEDTLS_DEBUG_C:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_PKCS1_V15:MBEDTLS_ENTROPY_C:MBEDTLS_CTR_DRBG_C */ void force_bad_session_id_len( ) { enum { BUFFSIZE = 1024 }; From e8ad0d7d42a7beeef3f037383369e2d76e5bbe88 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Sat, 11 Jun 2022 09:43:45 -0400 Subject: [PATCH 12/19] Disable bad session id length test in TLS 1.3 Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 4a1b6af37..14504e377 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -5504,7 +5504,7 @@ void conf_group() } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_CACHE_C:MBEDTLS_DEBUG_C:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_PKCS1_V15:MBEDTLS_ENTROPY_C:MBEDTLS_CTR_DRBG_C */ +/* BEGIN_CASE depends_on:MBEDTLS_SSL_SRV_C:MBEDTLS_SSL_CACHE_C:!MBEDTLS_SSL_PROTO_TLS1_3:MBEDTLS_DEBUG_C:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_PKCS1_V15:MBEDTLS_ENTROPY_C:MBEDTLS_CTR_DRBG_C */ void force_bad_session_id_len( ) { enum { BUFFSIZE = 1024 }; From e11acb2c9b7f4c8fe0a663e67d3ff482f0f6ced1 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 27 Jun 2022 06:11:34 -0400 Subject: [PATCH 13/19] test_suite_ssl: add proper cache cleanup Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 14504e377..09b9d8e66 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -4880,6 +4880,9 @@ void handshake_version( int dtls, int client_min_version, int client_max_version /* The goto below is used to avoid an "unused label" warning.*/ goto exit; + +exit: + free_handshake_options( &options ); } /* END_CASE */ @@ -4898,6 +4901,9 @@ void handshake_psk_cipher( char* cipher, int pk_alg, data_t *psk_str, int dtls ) /* The goto below is used to avoid an "unused label" warning.*/ goto exit; + + exit: + free_handshake_options( &options ); } /* END_CASE */ @@ -5056,9 +5062,6 @@ void renegotiation( int legacy_renegotiation ) /* The goto below is used to avoid an "unused label" warning.*/ goto exit; - -exit: - free_handshake_options( &options ); } /* END_CASE */ @@ -5094,9 +5097,6 @@ void resize_buffers_serialize_mfl( int mfl ) /* The goto below is used to avoid an "unused label" warning.*/ goto exit; - -exit: - free_handshake_options( &options ); } /* END_CASE */ From 92d7417d891bb58acb58eb5aaa5f25618457f8ed Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 28 Jun 2022 10:29:42 -0400 Subject: [PATCH 14/19] Formatting fixes Signed-off-by: Andrzej Kurek --- library/pkparse.c | 2 +- library/ssl_cache.c | 2 +- tests/suites/test_suite_ssl.function | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index 11afd0bdf..6f409689f 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1453,7 +1453,7 @@ int mbedtls_pk_parse_public_key( mbedtls_pk_context *ctx, if( ret == 0 ) { p = pem.buf; - if( ( pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_RSA )) == NULL ) + if( ( pk_info = mbedtls_pk_info_from_type( MBEDTLS_PK_RSA ) ) == NULL ) { mbedtls_pem_free( &pem ); return( MBEDTLS_ERR_PK_UNKNOWN_PK_ALG ); diff --git a/library/ssl_cache.c b/library/ssl_cache.c index 456f41cef..d19847428 100644 --- a/library/ssl_cache.c +++ b/library/ssl_cache.c @@ -314,7 +314,7 @@ exit: if( session_serialized != NULL ) { mbedtls_platform_zeroize( session_serialized, session_serialized_len ); - mbedtls_free(session_serialized); + mbedtls_free( session_serialized ); session_serialized = NULL; } diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 09b9d8e66..4224c413a 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -87,7 +87,7 @@ typedef struct handshake_test_options void (*cli_log_fun)(void *, int, const char *, int, const char *); int resize_buffers; #if defined(MBEDTLS_SSL_CACHE_C) - mbedtls_ssl_cache_context* cache; + mbedtls_ssl_cache_context *cache; #endif } handshake_test_options; @@ -940,7 +940,7 @@ exit: * \retval 0 on success, otherwise error code. */ int mbedtls_endpoint_init( mbedtls_endpoint *ep, int endpoint_type, - handshake_test_options* options, + handshake_test_options *options, mbedtls_test_message_socket_context *dtls_context, mbedtls_test_message_queue *input_queue, mbedtls_test_message_queue *output_queue, @@ -1983,7 +1983,7 @@ exit: #if defined(MBEDTLS_X509_CRT_PARSE_C) && \ defined(MBEDTLS_ENTROPY_C) && \ defined(MBEDTLS_CTR_DRBG_C) -void perform_handshake( handshake_test_options* options ) +void perform_handshake( handshake_test_options *options ) { /* forced_ciphersuite needs to last until the end of the handshake */ int forced_ciphersuite[2]; From 2e1a2322615910d85a8b9abffa34b7b3f820efac Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 28 Jun 2022 11:16:09 -0400 Subject: [PATCH 15/19] Fix changelog wording Signed-off-by: Andrzej Kurek --- ChangeLog.d/fix_some_resource_leaks.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.d/fix_some_resource_leaks.txt b/ChangeLog.d/fix_some_resource_leaks.txt index fb36561bb..9761537d8 100644 --- a/ChangeLog.d/fix_some_resource_leaks.txt +++ b/ChangeLog.d/fix_some_resource_leaks.txt @@ -1,5 +1,5 @@ Bugfix - * Fixed resource leaks in mbedtls_pk_parse_public_key() in low + * Fix resource leaks in mbedtls_pk_parse_public_key() in low memory conditions. Security * Fix potential memory leak inside mbedtls_ssl_cache_set() with From 3d0d501517ff1ac91a889d977d0726022a05cbda Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 4 Jul 2022 05:20:55 -0400 Subject: [PATCH 16/19] test_suite_ssl: prefer ASSERT_ALLOC over malloc Fix formatting for option initialization Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 64 ++++++++++++++-------------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 4224c413a..6408410db 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -93,37 +93,39 @@ typedef struct handshake_test_options void init_handshake_options( handshake_test_options *opts ) { - opts->cipher = ""; - opts->client_min_version = MBEDTLS_SSL_VERSION_UNKNOWN; - opts->client_max_version = MBEDTLS_SSL_VERSION_UNKNOWN; - opts->server_min_version = MBEDTLS_SSL_VERSION_UNKNOWN; - opts->server_max_version = MBEDTLS_SSL_VERSION_UNKNOWN; - opts->expected_negotiated_version = MBEDTLS_SSL_VERSION_TLS1_2; - opts->expected_handshake_result = 0; - opts->expected_ciphersuite = 0; - opts->pk_alg = MBEDTLS_PK_RSA; - opts->opaque_alg = 0; - opts->opaque_alg2 = 0; - opts->opaque_usage = 0; - opts->psk_str = NULL; - opts->dtls = 0; - opts->srv_auth_mode = MBEDTLS_SSL_VERIFY_NONE; - opts->serialize = 0; - opts->mfl = MBEDTLS_SSL_MAX_FRAG_LEN_NONE; - opts->cli_msg_len = 100; - opts->srv_msg_len = 100; - opts->expected_cli_fragments = 1; - opts->expected_srv_fragments = 1; - opts->renegotiate = 0; - opts->legacy_renegotiation = MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION; - opts->srv_log_obj = NULL; - opts->srv_log_obj = NULL; - opts->srv_log_fun = NULL; - opts->cli_log_fun = NULL; - opts->resize_buffers = 1; + opts->cipher = ""; + opts->client_min_version = MBEDTLS_SSL_VERSION_UNKNOWN; + opts->client_max_version = MBEDTLS_SSL_VERSION_UNKNOWN; + opts->server_min_version = MBEDTLS_SSL_VERSION_UNKNOWN; + opts->server_max_version = MBEDTLS_SSL_VERSION_UNKNOWN; + opts->expected_negotiated_version = MBEDTLS_SSL_VERSION_TLS1_2; + opts->expected_handshake_result = 0; + opts->expected_ciphersuite = 0; + opts->pk_alg = MBEDTLS_PK_RSA; + opts->opaque_alg = 0; + opts->opaque_alg2 = 0; + opts->opaque_usage = 0; + opts->psk_str = NULL; + opts->dtls = 0; + opts->srv_auth_mode = MBEDTLS_SSL_VERIFY_NONE; + opts->serialize = 0; + opts->mfl = MBEDTLS_SSL_MAX_FRAG_LEN_NONE; + opts->cli_msg_len = 100; + opts->srv_msg_len = 100; + opts->expected_cli_fragments = 1; + opts->expected_srv_fragments = 1; + opts->renegotiate = 0; + opts->legacy_renegotiation = MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION; + opts->srv_log_obj = NULL; + opts->srv_log_obj = NULL; + opts->srv_log_fun = NULL; + opts->cli_log_fun = NULL; + opts->resize_buffers = 1; #if defined(MBEDTLS_SSL_CACHE_C) - opts->cache = malloc( sizeof( mbedtls_ssl_cache_context ) ); - mbedtls_ssl_cache_init( opts->cache ); + ASSERT_ALLOC( opts->cache, sizeof( mbedtls_ssl_cache_context ) ); + mbedtls_ssl_cache_init( opts->cache ); +exit: + return; #endif } @@ -131,7 +133,7 @@ void free_handshake_options( handshake_test_options *opts ) { #if defined(MBEDTLS_SSL_CACHE_C) mbedtls_ssl_cache_free( opts->cache ); - free( opts->cache ); + mbedtls_free( opts->cache ); #else (void) opts; #endif From 1e085686ecc5c38c5e9f5c384f274404fe5ec836 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 4 Jul 2022 05:23:21 -0400 Subject: [PATCH 17/19] test_suite_ssl: remove unnecessary user data checks Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 6408410db..2c910e821 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -5559,10 +5559,6 @@ void force_bad_session_id_len( ) options.expected_srv_fragments ) == 0 ); } - TEST_ASSERT( mbedtls_ssl_conf_get_user_data_p( &client.conf ) == &client ); - TEST_ASSERT( mbedtls_ssl_get_user_data_p( &client.ssl ) == &client ); - TEST_ASSERT( mbedtls_ssl_conf_get_user_data_p( &server.conf ) == &server ); - TEST_ASSERT( mbedtls_ssl_get_user_data_p( &server.ssl ) == &server ); /* Make sure that the cache did not store the session */ TEST_EQUAL( srv_pattern.counter, 1 ); From 9dc4402afa30ccd86829e8a3cf7930cb3a642eda Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 4 Jul 2022 05:46:15 -0400 Subject: [PATCH 18/19] test_suite_ssl: zeroize the cache pointer in case if the struct memory gets reused Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index 2c910e821..f780fef3f 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -122,6 +122,7 @@ void init_handshake_options( handshake_test_options *opts ) opts->cli_log_fun = NULL; opts->resize_buffers = 1; #if defined(MBEDTLS_SSL_CACHE_C) + opts->cache = NULL; ASSERT_ALLOC( opts->cache, sizeof( mbedtls_ssl_cache_context ) ); mbedtls_ssl_cache_init( opts->cache ); exit: From ddb8cd601dcc18bb95f539f5bf0fabf34de1e278 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Mon, 4 Jul 2022 16:07:28 -0400 Subject: [PATCH 19/19] test_suite_ssl: Fix handshake options cleanup Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_ssl.function | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_ssl.function b/tests/suites/test_suite_ssl.function index f780fef3f..c65b56554 100644 --- a/tests/suites/test_suite_ssl.function +++ b/tests/suites/test_suite_ssl.function @@ -4905,7 +4905,7 @@ void handshake_psk_cipher( char* cipher, int pk_alg, data_t *psk_str, int dtls ) /* The goto below is used to avoid an "unused label" warning.*/ goto exit; - exit: +exit: free_handshake_options( &options ); } /* END_CASE */ @@ -4941,6 +4941,9 @@ void handshake_ciphersuite_select( char* cipher, int pk_alg, data_t *psk_str, /* The goto below is used to avoid an "unused label" warning.*/ goto exit; + +exit: + free_handshake_options( &options ); } /* END_CASE */ @@ -5065,6 +5068,8 @@ void renegotiation( int legacy_renegotiation ) /* The goto below is used to avoid an "unused label" warning.*/ goto exit; +exit: + free_handshake_options( &options ); } /* END_CASE */ @@ -5111,9 +5116,6 @@ void resize_buffers_renegotiate_mfl( int mfl, int legacy_renegotiation, /* The goto below is used to avoid an "unused label" warning.*/ goto exit; - -exit: - free_handshake_options( &options ); } /* END_CASE */