aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGilles Peskine <gilles.peskine@arm.com>2020-08-26 13:18:27 +0200
committerGitHub <noreply@github.com>2020-08-26 13:18:27 +0200
commitd4b913385013071a7f4479c833797cb7267e1596 (patch)
tree13e01953a448a869e7d9cdbd794f1015f60f34ad
parent9e4d4387f07326fff227a40f76c25e5181b1b1e2 (diff)
parenta2e518daf51de2cb548786b37a7d6e55ac8ffd67 (diff)
downloadmbed-tls-d4b913385013071a7f4479c833797cb7267e1596.tar.gz
Merge pull request #3611 from gilles-peskine-arm/psa-coverity-cleanups-202008
Minor fixes in PSA code and tests
-rw-r--r--library/psa_crypto_storage.c8
-rw-r--r--library/psa_its_file.c7
-rw-r--r--tests/suites/test_suite_psa_crypto.data8
-rw-r--r--tests/suites/test_suite_psa_crypto.function14
-rw-r--r--tests/suites/test_suite_psa_crypto_hash.function4
-rw-r--r--tests/suites/test_suite_psa_crypto_metadata.function20
-rw-r--r--tests/suites/test_suite_psa_its.function13
7 files changed, 63 insertions, 11 deletions
diff --git a/library/psa_crypto_storage.c b/library/psa_crypto_storage.c
index 37820533f..103c9bbb8 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 34a75dc69..2fbff20ef 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 d982f81f6..cd2601796 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 b0b4ed6a2..665580bfe 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 6c577c06a..1bc93313a 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 1ba846695..7c0929e29 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 b6cc488a6..a7ce7b1d4 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;
}