Fix potential memory corruption on MAC/cipher setup failure
When psa_mac_start(), psa_encrypt_setup() or psa_cipher_setup()
failed, depending on when the failure happened, it was possible that
psa_mac_abort() or psa_cipher_abort() would crash because it would try
to call a free() function uninitialized data in the operation
structure. Refactor the functions so that they initialize the
operation structure before doing anything else.
Add non-regression tests and a few more positive and negative unit
tests for psa_mac_start() and psa_cipher_setup() (the latter via
psa_encrypt_setip()).
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index d75226c..535384c 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -1133,10 +1133,53 @@
}
}
+/* Initialize the MAC operation structure. Once this function has been
+ * called, psa_mac_abort can run and will do the right thing. */
+static psa_status_t psa_mac_init( psa_mac_operation_t *operation,
+ psa_algorithm_t alg )
+{
+ psa_status_t status = PSA_ERROR_NOT_SUPPORTED;
+
+ operation->alg = alg;
+ operation->key_set = 0;
+ operation->iv_set = 0;
+ operation->iv_required = 0;
+ operation->has_input = 0;
+ operation->key_usage_sign = 0;
+ operation->key_usage_verify = 0;
+
+#if defined(MBEDTLS_CMAC_C)
+ if( alg == PSA_ALG_CMAC )
+ {
+ operation->iv_required = 0;
+ mbedtls_cipher_init( &operation->ctx.cmac );
+ status = PSA_SUCCESS;
+ }
+ else
+#endif /* MBEDTLS_CMAC_C */
+#if defined(MBEDTLS_MD_C)
+ if( PSA_ALG_IS_HMAC( operation->alg ) )
+ {
+ status = psa_hash_start( &operation->ctx.hmac.hash_ctx,
+ PSA_ALG_HMAC_HASH( alg ) );
+ }
+ else
+#endif /* MBEDTLS_MD_C */
+ {
+ /* fall through with NOT_SUPPORTED */
+ }
+
+ if( status != PSA_SUCCESS )
+ memset( operation, 0, sizeof( *operation ) );
+ return( status );
+}
+
psa_status_t psa_mac_abort( psa_mac_operation_t *operation )
{
switch( operation->alg )
{
+ case 0:
+ return( PSA_SUCCESS );
#if defined(MBEDTLS_CMAC_C)
case PSA_ALG_CMAC:
mbedtls_cipher_free( &operation->ctx.cmac );
@@ -1165,6 +1208,8 @@
operation->iv_set = 0;
operation->iv_required = 0;
operation->has_input = 0;
+ operation->key_usage_sign = 0;
+ operation->key_usage_verify = 0;
return( PSA_SUCCESS );
}
@@ -1178,8 +1223,6 @@
int ret;
operation->mac_size = cipher_info->block_size;
- operation->iv_required = 0;
- mbedtls_cipher_init( &operation->ctx.cmac );
ret = mbedtls_cipher_setup( &operation->ctx.cmac, cipher_info );
if( ret != 0 )
@@ -1213,14 +1256,9 @@
if( key_type != PSA_KEY_TYPE_HMAC )
return( PSA_ERROR_INVALID_ARGUMENT );
- operation->iv_required = 0;
operation->mac_size = digest_size;
- status = psa_hash_start( &operation->ctx.hmac.hash_ctx,
- PSA_ALG_HMAC_HASH( alg ) );
- if( status != PSA_SUCCESS )
- return( status );
-
+ /* The hash was started earlier in psa_mac_init. */
if( key_length > block_size )
{
status = psa_hash_update( &operation->ctx.hmac.hash_ctx,
@@ -1274,13 +1312,9 @@
size_t key_bits;
const mbedtls_cipher_info_t *cipher_info = NULL;
- operation->alg = 0;
- operation->key_set = 0;
- operation->iv_set = 0;
- operation->iv_required = 1;
- operation->has_input = 0;
- operation->key_usage_sign = 0;
- operation->key_usage_verify = 0;
+ status = psa_mac_init( operation, alg );
+ if( status != PSA_SUCCESS )
+ return( status );
status = psa_get_key_information( key, &key_type, &key_bits );
if( status != PSA_SUCCESS )
@@ -1332,7 +1366,6 @@
else
{
- operation->alg = alg;
operation->key_set = 1;
}
return( status );
@@ -1872,6 +1905,21 @@
/* Symmetric cryptography */
/****************************************************************/
+/* Initialize the cipher operation structure. Once this function has been
+ * called, psa_cipher_abort can run and will do the right thing. */
+static psa_status_t psa_cipher_init( psa_cipher_operation_t *operation,
+ psa_algorithm_t alg )
+{
+ operation->alg = alg;
+ operation->key_set = 0;
+ operation->iv_set = 0;
+ operation->iv_required = 1;
+ operation->iv_size = 0;
+ operation->block_size = 0;
+ mbedtls_cipher_init( &operation->ctx.cipher );
+ return( PSA_SUCCESS );
+}
+
static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation,
psa_key_slot_t key,
psa_algorithm_t alg,
@@ -1884,12 +1932,9 @@
size_t key_bits;
const mbedtls_cipher_info_t *cipher_info = NULL;
- operation->alg = alg;
- operation->key_set = 0;
- operation->iv_set = 0;
- operation->iv_required = 1;
- operation->iv_size = 0;
- operation->block_size = 0;
+ status = psa_cipher_init( operation, alg );
+ if( status != PSA_SUCCESS )
+ return( status );
status = psa_get_key_information( key, &key_type, &key_bits );
if( status != PSA_SUCCESS )
@@ -1900,7 +1945,6 @@
if( cipher_info == NULL )
return( PSA_ERROR_NOT_SUPPORTED );
- mbedtls_cipher_init( &operation->ctx.cipher );
ret = mbedtls_cipher_setup( &operation->ctx.cipher, cipher_info );
if( ret != 0 )
{
@@ -1944,7 +1988,6 @@
#endif //MBEDTLS_CIPHER_MODE_WITH_PADDING
operation->key_set = 1;
- operation->alg = alg;
operation->block_size = ( PSA_ALG_IS_BLOCK_CIPHER( alg ) ?
PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ) :
1 );
@@ -2119,6 +2162,9 @@
psa_status_t psa_cipher_abort( psa_cipher_operation_t *operation )
{
+ if( operation->alg == 0 )
+ return( PSA_SUCCESS );
+
mbedtls_cipher_free( &operation->ctx.cipher );
operation->alg = 0;
diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data
index 9902a0e..552faf9 100644
--- a/tests/suites/test_suite_psa_crypto.data
+++ b/tests/suites/test_suite_psa_crypto.data
@@ -106,6 +106,14 @@
PSA key lifetime set: invalid key lifetime value
key_lifetime_set_fail:1:PSA_KEY_LIFETIME_PERSISTENT+1:PSA_ERROR_INVALID_ARGUMENT
+PSA hash setup: good, SHA-256
+depends_on:MBEDTLS_SHA256_C
+hash_setup:PSA_ALG_SHA_256:PSA_SUCCESS
+
+PSA hash setup: bad (unknown hash algorithm)
+depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C
+hash_setup:0x80000000 | PSA_ALG_SHA_256:PSA_ERROR_NOT_SUPPORTED
+
PSA hash finish: SHA-256
depends_on:MBEDTLS_SHA256_C
hash_finish:PSA_ALG_SHA_256:"bd":"68325720aabd7c82f30f554b313d0570c95accbb7dc4b5aae11204c08ffe732b"
@@ -114,6 +122,27 @@
depends_on:MBEDTLS_SHA256_C
hash_verify:PSA_ALG_SHA_256:"bd":"68325720aabd7c82f30f554b313d0570c95accbb7dc4b5aae11204c08ffe732b"
+PSA MAC setup: good, HMAC-SHA-256
+depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C
+mac_setup:PSA_KEY_TYPE_HMAC:"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f":PSA_ALG_HMAC(PSA_ALG_SHA_256):PSA_SUCCESS
+
+PSA MAC setup: good, AES-CMAC
+depends_on:MBEDTLS_AES_C:MBEDTLS_CMAC_C
+mac_setup:PSA_KEY_TYPE_AES:"000102030405060708090a0b0c0d0e0f":PSA_ALG_CMAC:PSA_SUCCESS
+
+PSA MAC setup: bad algorithm (unknown MAC algorithm)
+depends_on:MBEDTLS_MD_C
+mac_setup:PSA_KEY_TYPE_RAW_DATA:"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f":PSA_ALG_HMAC(0):PSA_ERROR_NOT_SUPPORTED
+
+PSA MAC setup: invalid key type, HMAC-SHA-256
+depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C
+mac_setup:PSA_KEY_TYPE_RAW_DATA:"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f":PSA_ALG_HMAC(PSA_ALG_SHA_256):PSA_ERROR_INVALID_ARGUMENT
+
+PSA MAC setup: incompatible key DES for CMAC
+depends_on:MBEDTLS_DES_C:MBEDTLS_CMAC_C
+# Either INVALID_ARGUMENT or NOT_SUPPORTED would be reasonable here
+mac_setup:PSA_KEY_TYPE_DES:"000102030405060708090a0b0c0d0e0f":PSA_ALG_CMAC:PSA_ERROR_NOT_SUPPORTED
+
PSA MAC verify: HMAC-SHA-256
depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C
mac_verify:PSA_KEY_TYPE_HMAC:"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f":PSA_ALG_HMAC(PSA_ALG_SHA_256):"53616d706c65206d65737361676520666f72206b65796c656e3d626c6f636b6c656e":"8bb9a1db9806f20df7f77b82138c7914d174d59e13dc4d0169c9057b133e1d62"
@@ -218,6 +247,24 @@
depends_on:MBEDTLS_CMAC_C:MBEDTLS_AES_C
mac_verify:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":PSA_ALG_CMAC:"6bc1bee22e409f96e93d7e117393172aae2d8a571e03ac9c9eb76fac45af8e5130c81c46a35ce411":"dfa66747de9ae63030ca32611497c827"
+PSA cipher setup: good, AES-CTR
+depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CTR
+cipher_setup:PSA_KEY_TYPE_AES:"000102030405060708090a0b0c0d0e0f":PSA_ALG_CTR:PSA_SUCCESS
+
+PSA cipher setup: bad algorithm (unknown cipher algorithm)
+depends_on:MBEDTLS_AES_C
+cipher_setup:PSA_KEY_TYPE_AES:"000102030405060708090a0b0c0d0e0f":PSA_ALG_CATEGORY_CIPHER:PSA_ERROR_NOT_SUPPORTED
+
+PSA cipher setup: invalid key type, CTR
+depends_on:MBEDTLS_CIPHER_MODE_CTR
+# Either INVALID_ARGUMENT or NOT_SUPPORTED would be reasonable here
+cipher_setup:PSA_KEY_TYPE_RAW_DATA:"000102030405060708090a0b0c0d0e0f":PSA_ALG_CTR:PSA_ERROR_NOT_SUPPORTED
+
+PSA cipher setup: incompatible key ARC4 for CTR
+depends_on:MBEDTLS_ARC4_C:MBEDTLS_CIPHER_MODE_CTR
+# Either INVALID_ARGUMENT or NOT_SUPPORTED would be reasonable here
+cipher_setup:PSA_KEY_TYPE_ARC4:"000102030405060708090a0b0c0d0e0f":PSA_ALG_CTR:PSA_ERROR_NOT_SUPPORTED
+
PSA symmetric encrypt: AES-CBC-nopad, 16 bytes, good
depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC
cipher_encrypt:PSA_ALG_CBC_BASE | PSA_ALG_BLOCK_CIPHER_PAD_NONE:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a":"a076ec9dfbe47d52afc357336f20743b":PSA_SUCCESS
diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function
index 1cd9c22..ee78132 100644
--- a/tests/suites/test_suite_psa_crypto.function
+++ b/tests/suites/test_suite_psa_crypto.function
@@ -375,6 +375,25 @@
/* END_CASE */
/* BEGIN_CASE */
+void hash_setup( int alg_arg,
+ int expected_status_arg )
+{
+ psa_algorithm_t alg = alg_arg;
+ psa_hash_operation_t operation;
+ psa_status_t status;
+
+ TEST_ASSERT( psa_crypto_init( ) == PSA_SUCCESS );
+
+ status = psa_hash_start( &operation, alg );
+ psa_hash_abort( &operation );
+ TEST_ASSERT( status == (psa_status_t) expected_status_arg );
+
+exit:
+ mbedtls_psa_crypto_free( );
+}
+/* END_CASE */
+
+/* BEGIN_CASE */
void hash_finish( int alg_arg, data_t *input, data_t *expected_hash )
{
psa_algorithm_t alg = alg_arg;
@@ -431,6 +450,40 @@
/* END_CASE */
/* BEGIN_CASE */
+void mac_setup( int key_type_arg,
+ data_t *key,
+ int alg_arg,
+ int expected_status_arg )
+{
+ int key_slot = 1;
+ psa_key_type_t key_type = key_type_arg;
+ psa_algorithm_t alg = alg_arg;
+ psa_mac_operation_t operation;
+ psa_key_policy_t policy;
+ psa_status_t status;
+
+ TEST_ASSERT( psa_crypto_init( ) == PSA_SUCCESS );
+
+ psa_key_policy_init( &policy );
+ psa_key_policy_set_usage( &policy,
+ PSA_KEY_USAGE_SIGN | PSA_KEY_USAGE_VERIFY,
+ alg );
+ TEST_ASSERT( psa_set_key_policy( key_slot, &policy ) == PSA_SUCCESS );
+
+ TEST_ASSERT( psa_import_key( key_slot, key_type,
+ key->x, key->len ) == PSA_SUCCESS );
+
+ status = psa_mac_start( &operation, key_slot, alg );
+ psa_mac_abort( &operation );
+ TEST_ASSERT( status == (psa_status_t) expected_status_arg );
+
+exit:
+ psa_destroy_key( key_slot );
+ mbedtls_psa_crypto_free( );
+}
+/* END_CASE */
+
+/* BEGIN_CASE */
void mac_verify( int key_type_arg,
data_t *key,
int alg_arg,
@@ -474,6 +527,38 @@
/* END_CASE */
/* BEGIN_CASE */
+void cipher_setup( int key_type_arg,
+ data_t *key,
+ int alg_arg,
+ int expected_status_arg )
+{
+ int key_slot = 1;
+ psa_key_type_t key_type = key_type_arg;
+ psa_algorithm_t alg = alg_arg;
+ psa_cipher_operation_t operation;
+ psa_key_policy_t policy;
+ psa_status_t status;
+
+ TEST_ASSERT( psa_crypto_init( ) == PSA_SUCCESS );
+
+ psa_key_policy_init( &policy );
+ psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_ENCRYPT, alg );
+ TEST_ASSERT( psa_set_key_policy( key_slot, &policy ) == PSA_SUCCESS );
+
+ TEST_ASSERT( psa_import_key( key_slot, key_type,
+ key->x, key->len ) == PSA_SUCCESS );
+
+ status = psa_encrypt_setup( &operation, key_slot, alg );
+ psa_cipher_abort( &operation );
+ TEST_ASSERT( status == (psa_status_t) expected_status_arg );
+
+exit:
+ psa_destroy_key( key_slot );
+ mbedtls_psa_crypto_free( );
+}
+/* END_CASE */
+
+/* BEGIN_CASE */
void cipher_encrypt( int alg_arg, int key_type_arg,
data_t *key,
data_t *input, data_t *expected_output,