From 3a4f0e3cc4bb8a289b724f415c10dd83cd2b9ffe Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 19 Nov 2020 17:55:23 +0100 Subject: [PATCH] tests: psa: Reset key attributes where needed After a call to psa_get_key_attributes() to retrieve the attributes of a key into a psa_key_attributes_t structure, a call to psa_reset_key_attributes() is mandated to free the resources that may be referenced by the psa_key_attributes_t structure. Not calling psa_reset_key_attributes() may result in a memory leak. When a test function calls psa_get_key_parameters() the associated key attributes are systematically reset in the clean-up part of the function with a comment to emphasize the need for the reset and make it more visible. Signed-off-by: Ronald Cron --- tests/suites/test_suite_pk.function | 12 ++ tests/suites/test_suite_psa_crypto.function | 148 ++++++++++++++++-- ...t_suite_psa_crypto_persistent_key.function | 10 ++ ...st_suite_psa_crypto_se_driver_hal.function | 23 +++ ..._suite_psa_crypto_slot_management.function | 45 ++++++ 5 files changed, 226 insertions(+), 12 deletions(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 9803f9051..98016c652 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -200,6 +200,12 @@ void pk_psa_utils( ) TEST_ASSERT( PSA_SUCCESS == psa_destroy_key( key ) ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &attributes ); + mbedtls_pk_free( &pk ); /* redundant except upon error */ mbedtls_pk_free( &pk2 ); PSA_DONE( ); @@ -1289,6 +1295,12 @@ void pk_psa_sign( int grpid_arg, hash, sizeof hash, sig, sig_len ) == 0 ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &attributes ); + mbedtls_pk_free( &pk ); PSA_DONE( ); } diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index b03df3d4b..8e71610ac 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -292,7 +292,12 @@ int check_key_attributes_sanity( mbedtls_svc_key_id_t key ) ok = 1; exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + return( ok ); } @@ -445,6 +450,7 @@ static int exercise_cipher_key( mbedtls_svc_key_id_t key, iv_length = PSA_BLOCK_CIPHER_BLOCK_SIZE( psa_get_key_type( &attributes ) ); maybe_invalid_padding = ! PSA_ALG_IS_STREAM_CIPHER( alg ); + psa_reset_key_attributes( &attributes ); } PSA_ASSERT( psa_cipher_decrypt_setup( &operation, key, alg ) ); PSA_ASSERT( psa_cipher_set_iv( &operation, @@ -717,8 +723,13 @@ static psa_status_t key_agreement_with_self( operation, PSA_KEY_DERIVATION_INPUT_SECRET, key, public_key, public_key_length ); exit: - mbedtls_free( public_key ); + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + + mbedtls_free( public_key ); return( status ); } @@ -754,8 +765,13 @@ static psa_status_t raw_key_agreement_with_self( psa_algorithm_t alg, public_key, public_key_length, output, sizeof( output ), &output_length ); exit: - mbedtls_free( public_key ); + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + + mbedtls_free( public_key ); return( status ); } @@ -1033,8 +1049,13 @@ static int exercise_export_key( mbedtls_svc_key_id_t key, exported, exported_length ); exit: - mbedtls_free( exported ); + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + + mbedtls_free( exported ); return( ok ); } @@ -1069,8 +1090,13 @@ static int exercise_export_public_key( mbedtls_svc_key_id_t key ) exported, exported_length ); exit: - mbedtls_free( exported ); + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + + mbedtls_free( exported ); return( ok ); } @@ -1205,7 +1231,12 @@ static int test_operations_on_invalid_key( mbedtls_svc_key_id_t key ) ok = 1; exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + return( ok ); } @@ -1479,8 +1510,13 @@ void import_with_policy( int type_arg, test_operations_on_invalid_key( key ); exit: - psa_destroy_key( key ); + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &got_attributes ); + + psa_destroy_key( key ); PSA_DONE( ); } /* END_CASE */ @@ -1518,8 +1554,13 @@ void import_with_data( data_t *data, int type_arg, test_operations_on_invalid_key( key ); exit: - psa_destroy_key( key ); + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &got_attributes ); + + psa_destroy_key( key ); PSA_DONE( ); } /* END_CASE */ @@ -1567,6 +1608,12 @@ void import_large_key( int type_arg, int byte_size_arg, } exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &attributes ); + psa_destroy_key( key ); PSA_DONE( ); mbedtls_free( buffer ); @@ -1696,9 +1743,14 @@ destroy: test_operations_on_invalid_key( key ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &got_attributes ); + mbedtls_free( exported ); mbedtls_free( reexported ); - psa_reset_key_attributes( &got_attributes ); PSA_DONE( ); } /* END_CASE */ @@ -1749,9 +1801,14 @@ void import_export_public_key( data_t *data, } exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &attributes ); + mbedtls_free( exported ); psa_destroy_key( key ); - psa_reset_key_attributes( &attributes ); PSA_DONE( ); } /* END_CASE */ @@ -1792,8 +1849,14 @@ void import_and_exercise_key( data_t *data, test_operations_on_invalid_key( key ); exit: - psa_destroy_key( key ); + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &got_attributes ); + + psa_reset_key_attributes( &attributes ); + psa_destroy_key( key ); PSA_DONE( ); } /* END_CASE */ @@ -1832,8 +1895,13 @@ void effective_key_attributes( int type_arg, int expected_type_arg, TEST_EQUAL( psa_get_key_algorithm( &attributes ), expected_alg ); exit: - psa_destroy_key( key ); + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + + psa_destroy_key( key ); PSA_DONE( ); } /* END_CASE */ @@ -2087,8 +2155,13 @@ void asymmetric_encryption_key_policy( int policy_usage, TEST_EQUAL( status, PSA_ERROR_NOT_PERMITTED ); exit: - psa_destroy_key( key ); + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + + psa_destroy_key( key ); PSA_DONE( ); mbedtls_free( buffer ); } @@ -2265,6 +2338,12 @@ void key_policy_alg2( int key_type_arg, data_t *key_data, goto exit; exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &got_attributes ); + psa_destroy_key( key ); PSA_DONE( ); } @@ -2385,8 +2464,13 @@ void copy_success( int source_usage_arg, PSA_ASSERT( psa_destroy_key( target_key ) ); exit: + /* + * Source and target key attributes may have been returned by + * psa_get_key_attributes() thus reset them as required. + */ psa_reset_key_attributes( &source_attributes ); psa_reset_key_attributes( &target_attributes ); + PSA_DONE( ); mbedtls_free( export_buffer ); } @@ -4138,7 +4222,12 @@ void sign_deterministic( int key_type_arg, data_t *key_data, #endif /* MBEDTLS_TEST_DEPRECATED */ exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + psa_destroy_key( key ); mbedtls_free( signature ); PSA_DONE( ); @@ -4259,7 +4348,12 @@ void sign_verify( int key_type_arg, data_t *key_data, } exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + psa_destroy_key( key ); mbedtls_free( signature ); PSA_DONE( ); @@ -4409,7 +4503,12 @@ void asymmetric_encrypt( int key_type_arg, } exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + psa_destroy_key( key ); mbedtls_free( output ); PSA_DONE( ); @@ -4473,7 +4572,12 @@ void asymmetric_encrypt_decrypt( int key_type_arg, output2, output2_length ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + psa_destroy_key( key ); mbedtls_free( output ); mbedtls_free( output2 ); @@ -5080,8 +5184,13 @@ void derive_key_exercise( int alg_arg, goto exit; exit: - psa_key_derivation_abort( &operation ); + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &got_attributes ); + + psa_key_derivation_abort( &operation ); psa_destroy_key( base_key ); psa_destroy_key( derived_key ); PSA_DONE( ); @@ -5511,7 +5620,12 @@ void generate_key( int type_arg, goto exit; exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &got_attributes ); + psa_destroy_key( key ); PSA_DONE( ); } @@ -5612,7 +5726,12 @@ void generate_key_rsa( int bits_arg, } exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() or + * set by psa_set_key_domain_parameters() thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + psa_destroy_key( key ); PSA_DONE( ); mbedtls_free( e_read_buffer ); @@ -5741,7 +5860,12 @@ void persistent_key_load_key_from_storage( data_t *data, goto exit; exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + mbedtls_free( first_export ); mbedtls_free( second_export ); psa_key_derivation_abort( &operation ); diff --git a/tests/suites/test_suite_psa_crypto_persistent_key.function b/tests/suites/test_suite_psa_crypto_persistent_key.function index c4c2b75f6..8e10158f6 100644 --- a/tests/suites/test_suite_psa_crypto_persistent_key.function +++ b/tests/suites/test_suite_psa_crypto_persistent_key.function @@ -240,7 +240,12 @@ void persistent_key_import( int owner_id_arg, int key_id_arg, int type_arg, PSA_ASSERT( psa_destroy_key( key_id ) ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + psa_destroy_persistent_key( key_id ); PSA_DONE(); } @@ -308,7 +313,12 @@ void import_export_persistent_key( data_t *data, int type_arg, TEST_EQUAL( psa_is_key_present_in_storage( key_id ), 0 ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + mbedtls_free( exported ); PSA_DONE( ); psa_destroy_persistent_key( key_id ); diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 04aecb6b7..1add9b4a7 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -578,6 +578,12 @@ static int check_key_attributes( ok = 1; exit: + /* + * Actual key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &actual_attributes ); + return( ok ); } @@ -753,7 +759,12 @@ static int smoke_test_key( mbedtls_svc_key_id_t key ) ok = 1; exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ psa_reset_key_attributes( &attributes ); + return( ok ); } @@ -1080,6 +1091,12 @@ void key_creation_in_chosen_slot( int slot_arg, TEST_EQUAL( psa_open_key( id, &handle ), PSA_ERROR_DOES_NOT_EXIST ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &attributes ); + PSA_DONE( ); ram_slots_reset( ); psa_purge_storage( ); @@ -1431,6 +1448,12 @@ void sign_verify( int flow, PSA_ERROR_INVALID_SIGNATURE ); exit: + /* + * Driver key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &drv_attributes ); + psa_destroy_key( id ); psa_destroy_key( sw_key ); PSA_DONE( ); diff --git a/tests/suites/test_suite_psa_crypto_slot_management.function b/tests/suites/test_suite_psa_crypto_slot_management.function index edc1886fe..57d478982 100644 --- a/tests/suites/test_suite_psa_crypto_slot_management.function +++ b/tests/suites/test_suite_psa_crypto_slot_management.function @@ -226,6 +226,12 @@ void transient_slot_lifecycle( int owner_id_arg, TEST_EQUAL( psa_close_key( key ), PSA_ERROR_DOES_NOT_EXIST ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &attributes ); + PSA_DONE( ); } /* END_CASE */ @@ -369,6 +375,13 @@ void persistent_slot_lifecycle( int lifetime_arg, int owner_id_arg, int id_arg, } exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &attributes ); + psa_reset_key_attributes( &read_attributes ); + PSA_DONE( ); psa_purge_key_storage( ); mbedtls_free( reexported ); @@ -437,6 +450,12 @@ void create_existent( int lifetime_arg, int owner_id_arg, int id_arg, PSA_ASSERT( psa_close_key( id ) ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &attributes ); + PSA_DONE( ); psa_purge_key_storage( ); } @@ -626,6 +645,13 @@ void copy_across_lifetimes( int source_lifetime_arg, int source_owner_id_arg, PSA_ASSERT( psa_destroy_key( returned_target_id ) ); exit: + /* + * Source and target key attributes may have been returned by + * psa_get_key_attributes() thus reset them as required. + */ + psa_reset_key_attributes( &source_attributes ); + psa_reset_key_attributes( &target_attributes ); + PSA_DONE( ); mbedtls_free( export_buffer ); #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) @@ -737,6 +763,13 @@ void copy_to_occupied( int source_lifetime_arg, int source_id_arg, PSA_ASSERT( psa_destroy_key( returned_target_id ) ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &attributes1 ); + psa_reset_key_attributes( &attributes2 ); + PSA_DONE( ); mbedtls_free( export_buffer ); #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) @@ -824,6 +857,12 @@ void invalid_handle( int handle_construction, PSA_ASSERT( psa_close_key( valid_handle ) ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &attributes ); + PSA_DONE( ); } /* END_CASE */ @@ -1059,6 +1098,12 @@ void non_reusable_key_slots_integrity_in_case_of_key_slot_starvation( ) ASSERT_COMPARE( exported, exported_length, (uint8_t *) &persistent_key, sizeof( persistent_key ) ); exit: + /* + * Key attributes may have been returned by psa_get_key_attributes() + * thus reset them as required. + */ + psa_reset_key_attributes( &attributes ); + psa_destroy_key( persistent_key ); PSA_DONE( ); mbedtls_free( keys );