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()).
This commit is contained in:
Gilles Peskine 2018-06-20 16:05:20 +02:00 committed by itayzafrir
parent 12313cd84c
commit 16c0f4f787
3 changed files with 202 additions and 24 deletions

View File

@ -1133,10 +1133,53 @@ static size_t psa_get_hash_block_size( psa_algorithm_t alg )
}
}
/* 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 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation )
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 @@ static int psa_cmac_start( psa_mac_operation_t *operation,
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 @@ static int psa_hmac_start( psa_mac_operation_t *operation,
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 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation,
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 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation,
else
{
operation->alg = alg;
operation->key_set = 1;
}
return( status );
@ -1872,6 +1905,21 @@ psa_status_t psa_asymmetric_decrypt( psa_key_slot_t key,
/* 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 @@ static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation,
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 @@ static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation,
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 @@ static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation,
#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_finish( psa_cipher_operation_t *operation,
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;

View File

@ -106,6 +106,14 @@ key_lifetime_set_fail:1:PSA_KEY_LIFETIME_WRITE_ONCE:PSA_ERROR_NOT_SUPPORTED
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 @@ PSA hash verify: SHA-256
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 @@ PSA MAC verify: CMAC-AES-128
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

View File

@ -374,6 +374,25 @@ exit:
}
/* 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 )
{
@ -430,6 +449,40 @@ exit:
}
/* 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,
@ -473,6 +526,38 @@ exit:
}
/* 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,