diff --git a/library/psa_crypto_storage.c b/library/psa_crypto_storage.c index 37820533f9..103c9bbb8e 100644 --- a/library/psa_crypto_storage.c +++ b/library/psa_crypto_storage.c @@ -174,7 +174,13 @@ static psa_status_t psa_crypto_storage_store( const psa_key_file_id_t key, exit: if( status != PSA_SUCCESS ) - psa_its_remove( data_identifier ); + { + /* Remove the file in case we managed to create it but something + * went wrong. It's ok if the file doesn't exist. If the file exists + * but the removal fails, we're already reporting an error so there's + * nothing else we can do. */ + (void) psa_its_remove( data_identifier ); + } return( status ); } diff --git a/library/psa_its_file.c b/library/psa_its_file.c index 34a75dc694..2fbff20ef9 100644 --- a/library/psa_its_file.c +++ b/library/psa_its_file.c @@ -233,7 +233,12 @@ exit: if( rename_replace_existing( PSA_ITS_STORAGE_TEMP, filename ) != 0 ) status = PSA_ERROR_STORAGE_FAILURE; } - remove( PSA_ITS_STORAGE_TEMP ); + /* The temporary file may still exist, but only in failure cases where + * we're already reporting an error. So there's nothing we can do on + * failure. If the function succeeded, and in some error cases, the + * temporary file doesn't exist and so remove() is expected to fail. + * Thus we just ignore the return status of remove(). */ + (void) remove( PSA_ITS_STORAGE_TEMP ); return( status ); } diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index d982f81f67..cd26017962 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -799,6 +799,10 @@ hash_compute_fail:PSA_ALG_ANY_HASH:"":32:PSA_ERROR_NOT_SUPPORTED PSA hash compute: bad algorithm (not a hash) hash_compute_fail:PSA_ALG_HMAC(PSA_ALG_SHA_256):"":32:PSA_ERROR_INVALID_ARGUMENT +PSA hash compute: output buffer empty +depends_on:MBEDTLS_SHA256_C +hash_compute_fail:PSA_ALG_SHA_256:"":0:PSA_ERROR_BUFFER_TOO_SMALL + PSA hash compute: output buffer too small depends_on:MBEDTLS_SHA256_C hash_compute_fail:PSA_ALG_SHA_256:"":31:PSA_ERROR_BUFFER_TOO_SMALL @@ -828,6 +832,10 @@ PSA hash compare: truncated hash depends_on:MBEDTLS_SHA256_C hash_compare_fail:PSA_ALG_SHA_256:"":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b8":PSA_ERROR_INVALID_SIGNATURE +PSA hash compare: empty hash +depends_on:MBEDTLS_SHA256_C +hash_compare_fail:PSA_ALG_SHA_256:"":"":PSA_ERROR_INVALID_SIGNATURE + PSA hash compare: good depends_on:MBEDTLS_SHA256_C hash_compare_fail:PSA_ALG_SHA_256:"":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855":PSA_SUCCESS diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index b0b4ed6a21..665580bfe5 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -3083,6 +3083,7 @@ void mac_sign( int key_type_arg, } exit: + psa_mac_abort( &operation ); psa_destroy_key( handle ); PSA_DONE( ); mbedtls_free( actual_mac ); @@ -3161,6 +3162,7 @@ void mac_verify( int key_type_arg, } exit: + psa_mac_abort( &operation ); psa_destroy_key( handle ); PSA_DONE( ); mbedtls_free( perturbed_mac ); @@ -3241,6 +3243,7 @@ void cipher_setup( int key_type_arg, #endif exit: + psa_cipher_abort( &operation ); PSA_DONE( ); } /* END_CASE */ @@ -3393,6 +3396,7 @@ void cipher_bad_order( ) PSA_ASSERT( psa_destroy_key( handle ) ); exit: + psa_cipher_abort( &operation ); PSA_DONE( ); } /* END_CASE */ @@ -3451,6 +3455,7 @@ void cipher_encrypt( int alg_arg, int key_type_arg, } exit: + psa_cipher_abort( &operation ); mbedtls_free( output ); psa_destroy_key( handle ); PSA_DONE( ); @@ -3519,6 +3524,7 @@ void cipher_encrypt_multipart( int alg_arg, int key_type_arg, output, total_output_length ); exit: + psa_cipher_abort( &operation ); mbedtls_free( output ); psa_destroy_key( handle ); PSA_DONE( ); @@ -3590,6 +3596,7 @@ void cipher_decrypt_multipart( int alg_arg, int key_type_arg, output, total_output_length ); exit: + psa_cipher_abort( &operation ); mbedtls_free( output ); psa_destroy_key( handle ); PSA_DONE( ); @@ -3651,6 +3658,7 @@ void cipher_decrypt( int alg_arg, int key_type_arg, } exit: + psa_cipher_abort( &operation ); mbedtls_free( output ); psa_destroy_key( handle ); PSA_DONE( ); @@ -3732,6 +3740,8 @@ void cipher_verify_output( int alg_arg, int key_type_arg, ASSERT_COMPARE( input->x, input->len, output2, output2_length ); exit: + psa_cipher_abort( &operation1 ); + psa_cipher_abort( &operation2 ); mbedtls_free( output1 ); mbedtls_free( output2 ); psa_destroy_key( handle ); @@ -3835,6 +3845,8 @@ void cipher_verify_output_multipart( int alg_arg, ASSERT_COMPARE( input->x, input->len, output2, output2_length ); exit: + psa_cipher_abort( &operation1 ); + psa_cipher_abort( &operation2 ); mbedtls_free( output1 ); mbedtls_free( output2 ); psa_destroy_key( handle ); @@ -5697,7 +5709,7 @@ exit: /* In case there was a test failure after creating the persistent key * but while it was not open, try to re-open the persistent key * to delete it. */ - psa_open_key( key_id, &handle ); + (void) psa_open_key( key_id, &handle ); } psa_destroy_key( handle ); PSA_DONE(); diff --git a/tests/suites/test_suite_psa_crypto_hash.function b/tests/suites/test_suite_psa_crypto_hash.function index 6c577c06af..1bc93313a9 100644 --- a/tests/suites/test_suite_psa_crypto_hash.function +++ b/tests/suites/test_suite_psa_crypto_hash.function @@ -31,6 +31,7 @@ void hash_finish( int alg_arg, data_t *input, data_t *expected_hash ) actual_hash, actual_hash_length ); exit: + psa_hash_abort( &operation ); PSA_DONE( ); } /* END_CASE */ @@ -52,6 +53,7 @@ void hash_verify( int alg_arg, data_t *input, data_t *expected_hash ) expected_hash->len ) ); exit: + psa_hash_abort( &operation ); PSA_DONE( ); } /* END_CASE */ @@ -95,6 +97,8 @@ void hash_multi_part( int alg_arg, data_t *input, data_t *expected_hash ) } while( len++ != input->len ); exit: + psa_hash_abort( &operation ); + psa_hash_abort( &operation2 ); PSA_DONE( ); } /* END_CASE */ diff --git a/tests/suites/test_suite_psa_crypto_metadata.function b/tests/suites/test_suite_psa_crypto_metadata.function index 1ba8466952..7c0929e299 100644 --- a/tests/suites/test_suite_psa_crypto_metadata.function +++ b/tests/suites/test_suite_psa_crypto_metadata.function @@ -57,8 +57,18 @@ TEST_ASSERT( PSA_##flag( alg ) == !! ( ( flags ) & flag ) ) /* Check the parity of value. - * Return 0 if value has even parity and a nonzero value otherwise. */ -int test_parity( uint32_t value ) + * + * There are several numerical encodings for which the PSA Cryptography API + * specification deliberately defines encodings that all have the same + * parity. This way, a data glitch that flips one bit in the data cannot + * possibly turn a valid encoding into another valid encoding. Here in + * the tests, we check that the values (including Mbed TLS vendor-specific + * values) have the expected parity. + * + * The expected parity is even so that 0 is considered a valid encoding. + * + * Return a nonzero value if value has even parity and 0 otherwise. */ +int has_even_parity( uint32_t value ) { value ^= value >> 16; value ^= value >> 8; @@ -66,7 +76,7 @@ int test_parity( uint32_t value ) return( 0x9669 & 1 << ( value & 0xf ) ); } #define TEST_PARITY( value ) \ - TEST_ASSERT( test_parity( value ) ) + TEST_ASSERT( has_even_parity( value ) ) void algorithm_classification( psa_algorithm_t alg, unsigned flags ) { @@ -497,7 +507,7 @@ void ecc_key_family( int curve_arg ) psa_key_type_t public_type = PSA_KEY_TYPE_ECC_PUBLIC_KEY( curve ); psa_key_type_t pair_type = PSA_KEY_TYPE_ECC_KEY_PAIR( curve ); - test_parity( curve ); + TEST_PARITY( curve ); test_key_type( public_type, KEY_TYPE_IS_ECC | KEY_TYPE_IS_PUBLIC_KEY ); test_key_type( pair_type, KEY_TYPE_IS_ECC | KEY_TYPE_IS_KEY_PAIR ); @@ -514,7 +524,7 @@ void dh_key_family( int group_arg ) psa_key_type_t public_type = PSA_KEY_TYPE_DH_PUBLIC_KEY( group ); psa_key_type_t pair_type = PSA_KEY_TYPE_DH_KEY_PAIR( group ); - test_parity( group ); + TEST_PARITY( group ); test_key_type( public_type, KEY_TYPE_IS_DH | KEY_TYPE_IS_PUBLIC_KEY ); test_key_type( pair_type, KEY_TYPE_IS_DH | KEY_TYPE_IS_KEY_PAIR ); diff --git a/tests/suites/test_suite_psa_its.function b/tests/suites/test_suite_psa_its.function index b6cc488a6f..a7ce7b1d41 100644 --- a/tests/suites/test_suite_psa_its.function +++ b/tests/suites/test_suite_psa_its.function @@ -40,16 +40,23 @@ static psa_storage_uid_t uid_max = 0; static void cleanup( void ) { + /* Call remove() on all the files that a test might have created. + * We ignore the error if the file exists but remove() fails because + * it can't be checked portably (except by attempting to open the file + * first, which is needlessly slow and complicated here). A failure of + * remove() on an existing file is very unlikely anyway and would not + * have significant consequences other than perhaps failing the next + * test case. */ char filename[PSA_ITS_STORAGE_FILENAME_LENGTH]; psa_storage_uid_t uid; for( uid = 0; uid < uid_max; uid++ ) { psa_its_fill_filename( uid, filename ); - remove( filename ); + (void) remove( filename ); } psa_its_fill_filename( (psa_storage_uid_t)( -1 ), filename ); - remove( filename ); - remove( PSA_ITS_STORAGE_TEMP ); + (void) remove( filename ); + (void) remove( PSA_ITS_STORAGE_TEMP ); uid_max = 0; }