yuzu/mbedtls
1
0
Fork 0
mirror of https://github.com/yuzu-emu/mbedtls.git synced 2025-03-24 22:25:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

Clean up status code handling inside psa_destroy_key

Browse source 
Adopt a simple method for tracking whether there was a failure: each
fallible operation sets overall_status, unless overall_status is
already non-successful. Thus in case of multiple failures, the
function always reports whatever failed first. This may not always be
the right thing, but it's simple.

This revealed a bug whereby if the only failure was the call to
psa_destroy_se_key(), i.e. if the driver reported a failure or if the
driver lacked support for destroying keys, psa_destroy_key() would
ignore that failure.

For a key in a secure element, if creating a transaction file fails,
don't touch storage, but close the key in memory. This may not be
right, but it's no wronger than it was before. Tracked in
https://github.com/ARMmbed/mbed-crypto/issues/215
This commit is contained in:
Gilles Peskine 2019-08-13 15:58:36 +02:00
parent 9ce31c466d
commit 4b7f340fbf
1 changed files with 29 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
library/psa_crypto.c
View file

@ -1014,8 +1014,8 @@ psa_status_t psa_wipe_key_slot( psa_key_slot_t *slot )
psa_status_t psa_destroy_key( psa_key_handle_t handle ) psa_status_t psa_destroy_key( psa_key_handle_t handle )
{ {
psa_key_slot_t *slot; psa_key_slot_t *slot;
psa_status_t status = PSA_SUCCESS; psa_status_t status; /* status of the last operation */
psa_status_t storage_status = PSA_SUCCESS; psa_status_t overall_status = PSA_SUCCESS;
#if defined(MBEDTLS_PSA_CRYPTO_SE_C) #if defined(MBEDTLS_PSA_CRYPTO_SE_C)
psa_se_drv_table_entry_t *driver; psa_se_drv_table_entry_t *driver;
#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ #endif /* MBEDTLS_PSA_CRYPTO_SE_C */
@ -1041,18 +1041,30 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle )
if( status != PSA_SUCCESS ) if( status != PSA_SUCCESS )
{ {
(void) psa_crypto_stop_transaction( ); (void) psa_crypto_stop_transaction( );
/* TOnogrepDO: destroy what can be destroyed anyway */ /* We should still try to destroy the key in the secure
return( status ); * element and the key metadata in storage. This is especially
* important if the error is that the storage is full.
* But how to do it exactly without risking an inconsistent
* state after a reset?
* https://github.com/ARMmbed/mbed-crypto/issues/215
*/
overall_status = status;
goto exit;
} }
status = psa_destroy_se_key( driver, slot->data.se.slot_number ); status = psa_destroy_se_key( driver, slot->data.se.slot_number );
if( overall_status == PSA_SUCCESS )
overall_status = status;
} }
#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ #endif /* MBEDTLS_PSA_CRYPTO_SE_C */
#if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C)
if( slot->attr.lifetime != PSA_KEY_LIFETIME_VOLATILE ) if( slot->attr.lifetime != PSA_KEY_LIFETIME_VOLATILE )
{ {
storage_status = psa_destroy_persistent_key( slot->attr.id ); status = psa_destroy_persistent_key( slot->attr.id );
if( overall_status == PSA_SUCCESS )
overall_status = status;
/* TODO: other slots may have a copy of the same key. We should /* TODO: other slots may have a copy of the same key. We should
* invalidate them. * invalidate them.
* https://github.com/ARMmbed/mbed-crypto/issues/214 * https://github.com/ARMmbed/mbed-crypto/issues/214
@ -1063,23 +1075,23 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle )
#if defined(MBEDTLS_PSA_CRYPTO_SE_C) #if defined(MBEDTLS_PSA_CRYPTO_SE_C)
if( driver != NULL ) if( driver != NULL )
{ {
psa_status_t status2;
status = psa_save_se_persistent_data( driver ); status = psa_save_se_persistent_data( driver );
status2 = psa_crypto_stop_transaction( ); if( overall_status == PSA_SUCCESS )
if( status == PSA_SUCCESS ) overall_status = status;
status = status2; status = psa_crypto_stop_transaction( );
if( status != PSA_SUCCESS ) if( overall_status == PSA_SUCCESS )
{ overall_status = status;
/* TOnogrepDO: destroy what can be destroyed anyway */
return( status );
}
} }
#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ #endif /* MBEDTLS_PSA_CRYPTO_SE_C */
#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
exit:
#endif /* MBEDTLS_PSA_CRYPTO_SE_C */
status = psa_wipe_key_slot( slot ); status = psa_wipe_key_slot( slot );
if( status != PSA_SUCCESS ) /* Prioritize CORRUPTION_DETECTED from wiping over a storage error */
return( status ); if( overall_status == PSA_SUCCESS )
return( storage_status ); overall_status = status;
return( overall_status );
} }
void psa_reset_key_attributes( psa_key_attributes_t *attributes ) void psa_reset_key_attributes( psa_key_attributes_t *attributes )