From f9666595e147cdb9efcc680cc39d9653f3a61f4c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 6 May 2019 18:56:30 +0200 Subject: [PATCH] Implement and test the new key identifier range Only allow creating keys in the application (user) range. Allow opening keys in the implementation (vendor) range as well. Compared with what the implementation allowed, which was undocumented: 0 is now allowed; values from 0x40000000 to 0xfffeffff are now forbidden. --- library/psa_crypto.c | 2 +- library/psa_crypto_slot_management.c | 29 +++++++----- library/psa_crypto_slot_management.h | 5 ++- library/psa_crypto_storage.h | 2 +- ...test_suite_psa_crypto_slot_management.data | 45 +++++++++++++------ 5 files changed, 54 insertions(+), 29 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 9cf90ddaf..fa459a176 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1401,7 +1401,7 @@ static psa_status_t psa_start_key_creation( if( attributes->lifetime != PSA_KEY_LIFETIME_VOLATILE ) { status = psa_validate_persistent_key_parameters( attributes->lifetime, - attributes->id ); + attributes->id, 1 ); if( status != PSA_SUCCESS ) return( status ); slot->persistent_storage_id = attributes->id; diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 30cc05bd3..2ef70db59 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -176,20 +176,23 @@ exit: * is provided. * * \param file_id The key identifier to check. + * \param vendor_ok Nonzero to allow key ids in the vendor range. + * 0 to allow only key ids in the application range. * * \return 1 if \p file_id is acceptable, otherwise 0. */ -static int psa_is_key_id_valid( psa_key_file_id_t file_id ) +static int psa_is_key_id_valid( psa_key_file_id_t file_id, + int vendor_ok ) { psa_app_key_id_t key_id = PSA_KEY_FILE_GET_KEY_ID( file_id ); - /* Reject id=0 because by general library conventions, 0 is an invalid - * value wherever possible. */ - if( key_id == 0 ) - return( 0 ); /* Reject high values because the file names are reserved for the * library's internal use. */ if( key_id > PSA_MAX_PERSISTENT_KEY_IDENTIFIER ) return( 0 ); + /* Applications may only create keys in the range + * 0..PSA_KEY_ID_USER_MAX. */ + if( ! vendor_ok && key_id > PSA_KEY_ID_USER_MAX ) + return( 0 ); return( 1 ); } @@ -231,13 +234,14 @@ static psa_status_t psa_internal_make_key_persistent( psa_key_handle_t handle, psa_status_t psa_validate_persistent_key_parameters( psa_key_lifetime_t lifetime, - psa_key_file_id_t id ) + psa_key_file_id_t id, + int creating ) { if( lifetime != PSA_KEY_LIFETIME_PERSISTENT ) return( PSA_ERROR_INVALID_ARGUMENT ); #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) - if( ! psa_is_key_id_valid( id ) ) + if( ! psa_is_key_id_valid( id, ! creating ) ) return( PSA_ERROR_INVALID_ARGUMENT ); return( PSA_SUCCESS ); @@ -250,13 +254,15 @@ psa_status_t psa_validate_persistent_key_parameters( static psa_status_t persistent_key_setup( psa_key_lifetime_t lifetime, psa_key_file_id_t id, psa_key_handle_t *handle, - psa_status_t wanted_load_status ) + int creating ) { psa_status_t status; + psa_status_t wanted_load_status = + ( creating ? PSA_ERROR_DOES_NOT_EXIST : PSA_SUCCESS ); *handle = 0; - status = psa_validate_persistent_key_parameters( lifetime, id ); + status = psa_validate_persistent_key_parameters( lifetime, id, creating ); if( status != PSA_SUCCESS ) return( status ); @@ -281,7 +287,7 @@ static psa_status_t persistent_key_setup( psa_key_lifetime_t lifetime, psa_status_t psa_open_key( psa_key_file_id_t id, psa_key_handle_t *handle ) { return( persistent_key_setup( PSA_KEY_LIFETIME_PERSISTENT, - id, handle, PSA_SUCCESS ) ); + id, handle, 0 ) ); } psa_status_t psa_create_key( psa_key_lifetime_t lifetime, @@ -290,8 +296,7 @@ psa_status_t psa_create_key( psa_key_lifetime_t lifetime, { psa_status_t status; - status = persistent_key_setup( lifetime, id, handle, - PSA_ERROR_DOES_NOT_EXIST ); + status = persistent_key_setup( lifetime, id, handle, 1 ); switch( status ) { case PSA_SUCCESS: return( PSA_ERROR_ALREADY_EXISTS ); diff --git a/library/psa_crypto_slot_management.h b/library/psa_crypto_slot_management.h index 914e2d507..2e459d1a7 100644 --- a/library/psa_crypto_slot_management.h +++ b/library/psa_crypto_slot_management.h @@ -64,6 +64,8 @@ void psa_wipe_all_key_slots( void ); * * \param lifetime The lifetime to test. * \param id The key id to test. + * \param creating 0 if attempting to open an existing key. + * Nonzero if attempting to create a key. * * \retval PSA_SUCCESS * The given parameters are valid. @@ -74,7 +76,8 @@ void psa_wipe_all_key_slots( void ); */ psa_status_t psa_validate_persistent_key_parameters( psa_key_lifetime_t lifetime, - psa_key_file_id_t id ); + psa_key_file_id_t id, + int creating ); #endif /* PSA_CRYPTO_SLOT_MANAGEMENT_H */ diff --git a/library/psa_crypto_storage.h b/library/psa_crypto_storage.h index 5434d0529..2af624a0c 100644 --- a/library/psa_crypto_storage.h +++ b/library/psa_crypto_storage.h @@ -59,7 +59,7 @@ extern "C" { * This limitation will probably become moot when we implement client * separation for key storage. */ -#define PSA_MAX_PERSISTENT_KEY_IDENTIFIER 0xfffeffff +#define PSA_MAX_PERSISTENT_KEY_IDENTIFIER PSA_KEY_ID_VENDOR_MAX /** * \brief Checks if persistent data is stored for the given key slot number diff --git a/tests/suites/test_suite_psa_crypto_slot_management.data b/tests/suites/test_suite_psa_crypto_slot_management.data index c5afdfa95..519e81ec7 100644 --- a/tests/suites/test_suite_psa_crypto_slot_management.data +++ b/tests/suites/test_suite_psa_crypto_slot_management.data @@ -7,14 +7,23 @@ transient_slot_lifecycle:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789ab Transient slot, check after restart transient_slot_lifecycle:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_SHUTDOWN -Persistent slot, check after closing -persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:1:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_CLOSE +Persistent slot, check after closing, id=min +persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MIN:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_CLOSE -Persistent slot, check after destroying -persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:1:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_DESTROY +Persistent slot, check after destroying, id=min +persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MIN:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_DESTROY -Persistent slot, check after restart -persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:1:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_SHUTDOWN +Persistent slot, check after restart, id=min +persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MIN:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_SHUTDOWN + +Persistent slot, check after closing, id=max +persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MAX:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_CLOSE + +Persistent slot, check after destroying, id=max +persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MAX:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_DESTROY + +Persistent slot, check after restart, id=max +persistent_slot_lifecycle:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MAX:0:0:PSA_KEY_TYPE_RAW_DATA:"0123456789abcdef0123456789abcdef":CLOSE_BY_SHUTDOWN Attempt to overwrite: close before create_existent:PSA_KEY_LIFETIME_PERSISTENT:1:CLOSE_BEFORE @@ -25,14 +34,18 @@ create_existent:PSA_KEY_LIFETIME_PERSISTENT:1:CLOSE_AFTER Attempt to overwrite: keep open create_existent:PSA_KEY_LIFETIME_PERSISTENT:1:KEEP_OPEN -Open failure: invalid identifier (0) -depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C -open_fail:0:PSA_ERROR_INVALID_ARGUMENT - Open failure: invalid identifier (random seed UID) depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C open_fail:PSA_CRYPTO_ITS_RANDOM_SEED_UID:PSA_ERROR_INVALID_ARGUMENT +Open failure: invalid identifier (reserved range) +depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C +open_fail:PSA_KEY_ID_VENDOR_MAX + 1:PSA_ERROR_INVALID_ARGUMENT + +Open failure: invalid identifier (implementation range) +depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C +open_fail:PSA_KEY_ID_USER_MAX + 1:PSA_ERROR_DOES_NOT_EXIST + Open failure: non-existent identifier depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C open_fail:1:PSA_ERROR_DOES_NOT_EXIST @@ -40,14 +53,18 @@ open_fail:1:PSA_ERROR_DOES_NOT_EXIST Create failure: invalid lifetime create_fail:0x7fffffff:0:PSA_ERROR_INVALID_ARGUMENT -Create failure: invalid key id (0) -depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C -create_fail:PSA_KEY_LIFETIME_PERSISTENT:0:PSA_ERROR_INVALID_ARGUMENT - Create failure: invalid key id (random seed UID) depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C create_fail:PSA_KEY_LIFETIME_PERSISTENT:PSA_CRYPTO_ITS_RANDOM_SEED_UID:PSA_ERROR_INVALID_ARGUMENT +Create failure: invalid key id (reserved range) +depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C +create_fail:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_VENDOR_MAX + 1:PSA_ERROR_INVALID_ARGUMENT + +Create failure: invalid key id (implementation range) +depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C +create_fail:PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_ID_USER_MAX + 1:PSA_ERROR_INVALID_ARGUMENT + Open not supported depends_on:!MBEDTLS_PSA_CRYPTO_STORAGE_C open_fail:1:PSA_ERROR_NOT_SUPPORTED