Merge pull request #5765 from leorosen/fix-some-resource-leaks
Fix resource leaks
diff --git a/ChangeLog.d/fix_some_resource_leaks.txt b/ChangeLog.d/fix_some_resource_leaks.txt
new file mode 100644
index 0000000..9761537
--- /dev/null
+++ b/ChangeLog.d/fix_some_resource_leaks.txt
@@ -0,0 +1,6 @@
+Bugfix
+ * Fix resource leaks in mbedtls_pk_parse_public_key() in low
+ memory conditions.
+Security
+ * Fix potential memory leak inside mbedtls_ssl_cache_set() with
+ an invalid session id length.
diff --git a/library/pkparse.c b/library/pkparse.c
index 68727ec..6f40968 100644
--- a/library/pkparse.c
+++ b/library/pkparse.c
@@ -1454,10 +1454,16 @@
{
p = pem.buf;
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 )
+ {
+ 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 fe4f30c..d198474 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -312,7 +312,11 @@
#endif
if( session_serialized != NULL )
+ {
mbedtls_platform_zeroize( session_serialized, session_serialized_len );
+ mbedtls_free( session_serialized );
+ session_serialized = NULL;
+ }
return( ret );
}
diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data
index 1b36d32..a851d5c 100644
--- a/tests/suites/test_suite_ssl.data
+++ b/tests/suites/test_suite_ssl.data
@@ -3426,3 +3426,6 @@
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 efec5ea..c65b565 100644
--- a/tests/suites/test_suite_ssl.function
+++ b/tests/suites/test_suite_ssl.function
@@ -9,6 +9,10 @@
#include <ssl_tls13_invasive.h>
#include "test/certs.h"
+#if defined(MBEDTLS_SSL_CACHE_C)
+#include "mbedtls/ssl_cache.h"
+#endif
+
#include <psa/crypto.h>
#include <constant_time_internal.h>
@@ -82,38 +86,58 @@
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 )
{
- 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 = NULL;
+ ASSERT_ALLOC( opts->cache, sizeof( mbedtls_ssl_cache_context ) );
+ mbedtls_ssl_cache_init( opts->cache );
+exit:
+ return;
+#endif
+}
+
+void free_handshake_options( handshake_test_options *opts )
+{
+#if defined(MBEDTLS_SSL_CACHE_C)
+ mbedtls_ssl_cache_free( opts->cache );
+ mbedtls_free( opts->cache );
+#else
+ (void) opts;
+#endif
}
/*
* Buffer structure for custom I/O callbacks.
@@ -918,9 +942,8 @@
*
* \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 +1025,15 @@
mbedtls_ssl_conf_authmode( &( ep->conf ), MBEDTLS_SSL_VERIFY_REQUIRED );
+#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,
+ mbedtls_ssl_cache_get,
+ mbedtls_ssl_cache_set );
+ }
+#endif
+
ret = mbedtls_ssl_setup( &( ep->ssl ), &( ep->conf ) );
TEST_ASSERT( ret == 0 );
@@ -1010,8 +1042,10 @@
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 );
@@ -1952,7 +1986,7 @@
#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];
@@ -1984,11 +2018,7 @@
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 +2030,7 @@
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 +2064,7 @@
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 +2076,8 @@
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 +4789,24 @@
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 +4816,21 @@
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 +4857,7 @@
}
exit:
+ free_handshake_options( &options );
mbedtls_endpoint_free( &base_ep, NULL );
mbedtls_endpoint_free( &second_ep, NULL );
USE_PSA_DONE( );
@@ -4857,6 +4883,9 @@
/* The goto below is used to avoid an "unused label" warning.*/
goto exit;
+
+exit:
+ free_handshake_options( &options );
}
/* END_CASE */
@@ -4875,6 +4904,9 @@
/* The goto below is used to avoid an "unused label" warning.*/
goto exit;
+
+exit:
+ free_handshake_options( &options );
}
/* END_CASE */
@@ -4909,6 +4941,9 @@
/* The goto below is used to avoid an "unused label" warning.*/
goto exit;
+
+exit:
+ free_handshake_options( &options );
}
/* END_CASE */
@@ -4931,8 +4966,12 @@
#endif
perform_handshake( &options );
+
/* The goto below is used to avoid an "unused label" warning.*/
goto exit;
+
+exit:
+ free_handshake_options( &options );
}
/* END_CASE */
@@ -4971,6 +5010,8 @@
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 +5048,9 @@
{
TEST_ASSERT( cli_pattern.counter >= 1 );
}
+
+exit:
+ free_handshake_options( &options );
}
/* END_CASE */
@@ -5021,8 +5065,11 @@
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 +5089,11 @@
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 */
@@ -5459,6 +5509,71 @@
}
/* END_CASE */
+/* 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 };
+ 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, NULL ) == 0 );
+
+ TEST_ASSERT( mbedtls_endpoint_init( &server, MBEDTLS_SSL_IS_SERVER,
+ &options, NULL, 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 );
+ }
+
+ /* 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( )
{
@@ -5558,22 +5673,26 @@
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),
@@ -5611,6 +5730,7 @@
exit:
mbedtls_endpoint_free( &client, NULL );
mbedtls_endpoint_free( &server, NULL );
+ free_handshake_options( &options );
USE_PSA_DONE( );
}
