From 0a233224316f85bcd30c40106579c3d35addf4c4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 14:50:28 +0200 Subject: [PATCH 1/5] Improve documentation of the allocate method --- include/psa/crypto_se_driver.h | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 69cdababa..cd57b065d 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -977,7 +977,21 @@ typedef psa_status_t (*psa_drv_se_generate_key_t)(psa_drv_se_context_t *drv_cont * If one of the functions is not implemented, it should be set to NULL. */ typedef struct { - /** Function that allocates a slot. */ + /** Function that allocates a slot for a key. + * + * The core calls this function to determine a slot number, then + * calls the actual creation function (such as + * psa_drv_se_key_management_t::p_import or + * psa_drv_se_key_management_t::p_generate). + * + * If this function succeeds, the next call that the core makes to the + * driver is either the creation function or + * psa_drv_se_key_management_t::p_destroy. Note that + * if the platform is reset after this function returns, the core + * may either subsequently call + * psa_drv_se_key_management_t::p_destroy or may behave as if the + * last call to this function had not taken place. + */ psa_drv_se_allocate_key_t p_allocate; /** Function that performs a key import operation */ psa_drv_se_import_key_t p_import; From ae9964d3ef2bc9f76a05eb43fcc216bdf5252c72 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 14:55:14 +0200 Subject: [PATCH 2/5] Add validate_slot_number method to SE drivers Pave the way for allowing the application to choose the slot number in a secure element, rather than always letting the driver choose. --- include/psa/crypto_se_driver.h | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index cd57b065d..127f17b5c 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -833,6 +833,30 @@ typedef psa_status_t (*psa_drv_se_allocate_key_t)( const psa_key_attributes_t *attributes, psa_key_slot_number_t *key_slot); +/** \brief A function that determines whether a slot number is valid + * for a key. + * + * \param[in,out] drv_context The driver context structure. + * \param[in] attributes Attributes of the key. + * \param[in] key_slot Slot where the key is to be stored. + * + * \retval #PSA_SUCCESS + * The given slot number is valid for a key with the given + * attributes. + * \retval #PSA_ERROR_INVALID_ARGUMENT + * The given slot number is not valid for a key with the + * given attributes. This includes the case where the slot + * number is not valid at all. + * \retval #PSA_ERROR_ALREADY_EXISTS + * There is already a key with the specified slot number. + * Drivers may choose to return this error from the key + * creation function instead. + */ +typedef psa_status_t (*psa_drv_se_validate_slot_number_t)( + psa_drv_se_context_t *drv_context, + const psa_key_attributes_t *attributes, + psa_key_slot_number_t key_slot); + /** \brief A function that imports a key into a secure element in binary format * * This function can support any output from psa_export_key(). Refer to the @@ -993,6 +1017,16 @@ typedef struct { * last call to this function had not taken place. */ psa_drv_se_allocate_key_t p_allocate; + /** Function that checks the validity of a slot for a key. + * + * The core calls this function instead of + * psa_drv_se_key_management_t::p_allocate to create + * a key in a specific slot. It then calls the actual creation function + * (such as psa_drv_se_key_management_t::p_import or + * psa_drv_se_key_management_t::p_generate) or + * psa_drv_se_key_management_t::p_destroy. + */ + psa_drv_se_validate_slot_number_t p_validate_slot_number; /** Function that performs a key import operation */ psa_drv_se_import_key_t p_import; /** Function that performs a generation */ From 46d9439a5ec82d09b76d007f785cef74924f969c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 14:55:50 +0200 Subject: [PATCH 3/5] Support slot_number attribute when creating a key Allow the application to choose the slot number in a secure element, rather than always letting the driver choose. With this commit, any application may request any slot. In an implementation with isolation, it's up to the service to filter key creation requests and apply policies to limit which applications can request which slot. --- library/psa_crypto.c | 4 -- library/psa_crypto_se.c | 37 +++++++--- .../test_suite_psa_crypto_se_driver_hal.data | 9 +++ ...st_suite_psa_crypto_se_driver_hal.function | 70 +++++++++++++++++++ 4 files changed, 105 insertions(+), 15 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 5cb88de7e..856d8622d 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1582,10 +1582,6 @@ static psa_status_t psa_start_key_creation( * we can roll back to a state where the key doesn't exist. */ if( *p_drv != NULL ) { - /* Choosing a slot number is not supported yet. */ - if( attributes->core.flags & MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER ) - return( PSA_ERROR_NOT_SUPPORTED ); - status = psa_find_se_slot_for_key( attributes, *p_drv, &slot->data.se.slot_number ); if( status != PSA_SUCCESS ) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index bc7325180..ca38e2065 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -201,7 +201,6 @@ psa_status_t psa_find_se_slot_for_key( psa_key_slot_number_t *slot_number ) { psa_status_t status; - psa_drv_se_allocate_key_t p_allocate = NULL; /* If the lifetime is wrong, it's a bug in the library. */ if( driver->lifetime != psa_get_key_lifetime( attributes ) ) @@ -210,17 +209,33 @@ psa_status_t psa_find_se_slot_for_key( /* If the driver doesn't support key creation in any way, give up now. */ if( driver->methods->key_management == NULL ) return( PSA_ERROR_NOT_SUPPORTED ); - p_allocate = driver->methods->key_management->p_allocate; - /* If the driver doesn't tell us how to allocate a slot, that's - * not supported for the time being. */ - if( p_allocate == NULL ) - return( PSA_ERROR_NOT_SUPPORTED ); - - status = p_allocate( &driver->context, - driver->internal.persistent_data, - attributes, - slot_number ); + if( psa_get_key_slot_number( attributes, slot_number ) == PSA_SUCCESS ) + { + /* The application wants to use a specific slot. Allow it if + * the driver supports it. On a system with isolation, + * the crypto service must check that the application is + * permitted to request this slot. */ + psa_drv_se_validate_slot_number_t p_validate_slot_number = + driver->methods->key_management->p_validate_slot_number; + if( p_validate_slot_number == NULL ) + return( PSA_ERROR_NOT_SUPPORTED ); + status = p_validate_slot_number( &driver->context, attributes, + *slot_number ); + } + else + { + /* The application didn't tell us which slot to use. Let the driver + * choose. This is the normal case. */ + psa_drv_se_allocate_key_t p_allocate = + driver->methods->key_management->p_allocate; + if( p_allocate == NULL ) + return( PSA_ERROR_NOT_SUPPORTED ); + status = p_allocate( &driver->context, + driver->internal.persistent_data, + attributes, + slot_number ); + } return( status ); } diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data index 6fb65f02a..57aa47f76 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -39,6 +39,15 @@ key_creation_import_export:0:1 SE key import-export, check after restart (slot 3) key_creation_import_export:3:1 +Key creation in a specific slot (0) +key_creation_in_chosen_slot:0:PSA_SUCCESS + +Key creation in a specific slot (max) +key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ) - 1:PSA_SUCCESS + +Key creation in a specific slot (too large) +key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ):PSA_ERROR_INVALID_ARGUMENT + Key creation smoke test: AES-CTR key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CTR:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 9a5746476..8924ae1e7 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -177,6 +177,18 @@ static psa_status_t ram_allocate( psa_drv_se_context_t *context, return( PSA_ERROR_INSUFFICIENT_STORAGE ); } +static psa_status_t ram_validate_slot_number( + psa_drv_se_context_t *context, + const psa_key_attributes_t *attributes, + psa_key_slot_number_t slot_number ) +{ + (void) context; + (void) attributes; + if( slot_number >= ARRAY_LENGTH( ram_slots ) ) + return( PSA_ERROR_INVALID_ARGUMENT ); + return( PSA_SUCCESS ); +} + /****************************************************************/ @@ -536,6 +548,64 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void key_creation_in_chosen_slot( int slot_arg, + int expected_status_arg ) +{ + psa_key_slot_number_t wanted_slot = slot_arg; + psa_status_t expected_status = expected_status_arg; + psa_status_t status; + psa_drv_se_t driver; + psa_drv_se_key_management_t key_management; + psa_key_lifetime_t lifetime = 2; + psa_key_id_t id = 1; + psa_key_handle_t handle = 0; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + const uint8_t key_material[3] = {0xfa, 0xca, 0xde}; + + memset( &driver, 0, sizeof( driver ) ); + memset( &key_management, 0, sizeof( key_management ) ); + driver.hal_version = PSA_DRV_SE_HAL_VERSION; + driver.key_management = &key_management; + driver.persistent_data_size = sizeof( ram_slot_usage_t ); + key_management.p_validate_slot_number = ram_validate_slot_number; + key_management.p_import = ram_import; + key_management.p_destroy = ram_destroy; + key_management.p_export = ram_export; + + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + PSA_ASSERT( psa_crypto_init( ) ); + + /* Create a key. */ + psa_set_key_id( &attributes, id ); + psa_set_key_lifetime( &attributes, lifetime ); + psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_EXPORT ); + psa_set_key_type( &attributes, PSA_KEY_TYPE_RAW_DATA ); + psa_set_key_slot_number( &attributes, wanted_slot ); + status = psa_import_key( &attributes, + key_material, sizeof( key_material ), + &handle ); + TEST_EQUAL( status, expected_status ); + + if( status == PSA_SUCCESS ) + { + /* Test that the key was created in the expected slot. */ + TEST_EQUAL( ram_slots[wanted_slot].type, PSA_KEY_TYPE_RAW_DATA ); + + /* Test that the key is reported with the correct attributes, + * including the expected slot. */ + PSA_ASSERT( psa_get_key_attributes( handle, &attributes ) ); + + PSA_ASSERT( psa_destroy_key( handle ) ); + } + +exit: + PSA_DONE( ); + ram_slots_reset( ); + psa_purge_storage( ); +} +/* END_CASE */ + /* BEGIN_CASE */ void key_creation_smoke( int type_arg, int alg_arg, data_t *key_material ) From 0a1104474b5a2ff2405d7d1807506c20a727333d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 14:59:15 +0200 Subject: [PATCH 4/5] Test restarting after creating a key in a specific slot --- .../test_suite_psa_crypto_se_driver_hal.data | 12 ++++++-- ...st_suite_psa_crypto_se_driver_hal.function | 28 +++++++++++++------ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data index 57aa47f76..e6482ddbc 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -40,13 +40,19 @@ SE key import-export, check after restart (slot 3) key_creation_import_export:3:1 Key creation in a specific slot (0) -key_creation_in_chosen_slot:0:PSA_SUCCESS +key_creation_in_chosen_slot:0:0:PSA_SUCCESS Key creation in a specific slot (max) -key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ) - 1:PSA_SUCCESS +key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ) - 1:0:PSA_SUCCESS + +Key creation in a specific slot (0, restart) +key_creation_in_chosen_slot:0:1:PSA_SUCCESS + +Key creation in a specific slot (max, restart) +key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ) - 1:1:PSA_SUCCESS Key creation in a specific slot (too large) -key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ):PSA_ERROR_INVALID_ARGUMENT +key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ):0:PSA_ERROR_INVALID_ARGUMENT Key creation smoke test: AES-CTR key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CTR:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 8924ae1e7..0fab0433f 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -550,6 +550,7 @@ exit: /* BEGIN_CASE */ void key_creation_in_chosen_slot( int slot_arg, + int restart, int expected_status_arg ) { psa_key_slot_number_t wanted_slot = slot_arg; @@ -587,18 +588,27 @@ void key_creation_in_chosen_slot( int slot_arg, &handle ); TEST_EQUAL( status, expected_status ); - if( status == PSA_SUCCESS ) + if( status != PSA_SUCCESS ) + goto exit; + + /* Maybe restart, to check that the information is saved correctly. */ + if( restart ) { - /* Test that the key was created in the expected slot. */ - TEST_EQUAL( ram_slots[wanted_slot].type, PSA_KEY_TYPE_RAW_DATA ); - - /* Test that the key is reported with the correct attributes, - * including the expected slot. */ - PSA_ASSERT( psa_get_key_attributes( handle, &attributes ) ); - - PSA_ASSERT( psa_destroy_key( handle ) ); + mbedtls_psa_crypto_free( ); + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + PSA_ASSERT( psa_crypto_init( ) ); + PSA_ASSERT( psa_open_key( id, &handle ) ); } + /* Test that the key was created in the expected slot. */ + TEST_EQUAL( ram_slots[wanted_slot].type, PSA_KEY_TYPE_RAW_DATA ); + + /* Test that the key is reported with the correct attributes, + * including the expected slot. */ + PSA_ASSERT( psa_get_key_attributes( handle, &attributes ) ); + + PSA_ASSERT( psa_destroy_key( handle ) ); + exit: PSA_DONE( ); ram_slots_reset( ); From 9d75202efb8267e34e871a023539b2d9050aedf9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 9 Aug 2019 11:33:48 +0200 Subject: [PATCH 5/5] Clarify and expand the documentation of the allocate/create sequence --- include/psa/crypto_se_driver.h | 74 +++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 127f17b5c..9a5d97da7 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -811,6 +811,42 @@ typedef struct { /**@{*/ /** \brief A function that allocates a slot for a key. + * + * To create a key in a specific slot in a secure element, the core + * first calls this function to determine a valid slot number, + * then calls a function to create the key material in that slot. + * For example, in nominal conditions (that is, if no error occurs), + * the effect of a call to psa_import_key() with a lifetime that places + * the key in a secure element is the following: + * -# The core calls psa_drv_se_key_management_t::p_allocate + * (or in some implementations + * psa_drv_se_key_management_t::p_validate_slot_number). The driver + * selects (or validates) a suitable slot number given the key attributes + * and the state of the secure element. + * -# The core calls psa_drv_se_key_management_t::p_import to import + * the key material in the selected slot. + * + * Other key creation methods lead to similar sequences. For example, the + * sequence for psa_generate_key() is the same except that the second step + * is a call to psa_drv_se_key_management_t::p_generate. + * + * In case of errors, other behaviors are possible. + * - If the PSA Cryptography subsystem dies after the first step, + * for example because the device has lost power abruptly, + * the second step may never happen, or may happen after a reset + * and re-initialization. Alternatively, after a reset and + * re-initialization, the core may call + * psa_drv_se_key_management_t::p_destroy on the slot number that + * was allocated (or validated) instead of calling a key creation function. + * - If an error occurs, the core may call + * psa_drv_se_key_management_t::p_destroy on the slot number that + * was allocated (or validated) instead of calling a key creation function. + * + * Errors and system resets also have an impact on the driver's persistent + * data. If a reset happens before the overall key creation process is + * completed (before or after the second step above), it is unspecified + * whether the persistent data after the reset is identical to what it + * was before or after the call to `p_allocate` (or `p_validate_slot_number`). * * \param[in,out] drv_context The driver context structure. * \param[in,out] persistent_data A pointer to the persistent data @@ -836,6 +872,18 @@ typedef psa_status_t (*psa_drv_se_allocate_key_t)( /** \brief A function that determines whether a slot number is valid * for a key. * + * To create a key in a specific slot in a secure element, the core + * first calls this function to validate the choice of slot number, + * then calls a function to create the key material in that slot. + * See the documentation of #psa_drv_se_allocate_key_t for more details. + * + * As of the PSA Cryptography API specification version 1.0, there is no way + * for applications to trigger a call to this function. However some + * implementations offer the capability to create or declare a key in + * a specific slot via implementation-specific means, generally for the + * sake of initial device provisioning or onboarding. Such a mechanism may + * be added to a future version of the PSA Cryptography API specification. + * * \param[in,out] drv_context The driver context structure. * \param[in] attributes Attributes of the key. * \param[in] key_slot Slot where the key is to be stored. @@ -1001,31 +1049,9 @@ typedef psa_status_t (*psa_drv_se_generate_key_t)(psa_drv_se_context_t *drv_cont * If one of the functions is not implemented, it should be set to NULL. */ typedef struct { - /** Function that allocates a slot for a key. - * - * The core calls this function to determine a slot number, then - * calls the actual creation function (such as - * psa_drv_se_key_management_t::p_import or - * psa_drv_se_key_management_t::p_generate). - * - * If this function succeeds, the next call that the core makes to the - * driver is either the creation function or - * psa_drv_se_key_management_t::p_destroy. Note that - * if the platform is reset after this function returns, the core - * may either subsequently call - * psa_drv_se_key_management_t::p_destroy or may behave as if the - * last call to this function had not taken place. - */ + /** Function that allocates a slot for a key. */ psa_drv_se_allocate_key_t p_allocate; - /** Function that checks the validity of a slot for a key. - * - * The core calls this function instead of - * psa_drv_se_key_management_t::p_allocate to create - * a key in a specific slot. It then calls the actual creation function - * (such as psa_drv_se_key_management_t::p_import or - * psa_drv_se_key_management_t::p_generate) or - * psa_drv_se_key_management_t::p_destroy. - */ + /** Function that checks the validity of a slot for a key. */ psa_drv_se_validate_slot_number_t p_validate_slot_number; /** Function that performs a key import operation */ psa_drv_se_import_key_t p_import;