From e62b74e68f4d2cf79a6218ae3abf4fb910643fb7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 25 Jun 2019 15:25:09 +0200 Subject: [PATCH 01/50] Add public-key export method --- include/psa/crypto_se_driver.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 85247051e6..fc0d96162c 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -819,6 +819,8 @@ typedef struct { psa_drv_se_destroy_key_t p_destroy; /** Function that performs a key export operation */ psa_drv_se_export_key_t p_export; + /** Function that performs a public key export operation */ + psa_drv_se_export_key_t p_export_public; } psa_drv_se_key_management_t; /**@}*/ From f989dbe6d8b2f355303f1aaff5644b11349a18ca Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Jun 2019 18:18:12 +0200 Subject: [PATCH 02/50] SE driver lookup functions Expose the type of an entry in the SE driver table as an opaque type to other library modules. Soon, driver table entries will have state, and callers will need to be able to access this state through functions using this opaque type. Provide functions to look up a driver by its lifetime and to retrieve the method table from an entry. --- library/psa_crypto_se.c | 52 ++++++++++++++++++++++++++++++++++++++--- library/psa_crypto_se.h | 38 ++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index 688d4e7c8e..70e3a16802 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -32,13 +32,53 @@ #include "psa_crypto_se.h" -typedef struct +/****************************************************************/ +/* Driver lookup */ +/****************************************************************/ + +typedef struct psa_se_drv_table_entry_s { psa_key_lifetime_t lifetime; const psa_drv_se_t *methods; -} method_table_entry_t; +} psa_se_drv_table_entry_t; -static method_table_entry_t driver_table[PSA_MAX_SE_DRIVERS]; +static psa_se_drv_table_entry_t driver_table[PSA_MAX_SE_DRIVERS]; + +const psa_se_drv_table_entry_t *psa_get_se_driver_entry( + psa_key_lifetime_t lifetime ) +{ + size_t i; + if( lifetime == 0 ) + return( NULL ); + for( i = 0; i < PSA_MAX_SE_DRIVERS; i++ ) + { + if( driver_table[i].lifetime == lifetime ) + return( &driver_table[i] ); + } + return( NULL ); +} + +const psa_drv_se_t *psa_get_se_driver_methods( + const psa_se_drv_table_entry_t *drv ) +{ + return( drv->methods ); +} + +const psa_drv_se_t *psa_get_se_driver( psa_key_lifetime_t lifetime ) +{ + const psa_se_drv_table_entry_t *drv = psa_get_se_driver_entry( lifetime ); + if( drv == NULL ) + return( NULL ); + else + return( drv->methods ); +} + + + + +/****************************************************************/ +/* Driver registration */ +/****************************************************************/ psa_status_t psa_register_se_driver( psa_key_lifetime_t lifetime, @@ -83,4 +123,10 @@ void psa_unregister_all_se_drivers( void ) memset( driver_table, 0, sizeof( driver_table ) ); } + + +/****************************************************************/ +/* The end */ +/****************************************************************/ + #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ diff --git a/library/psa_crypto_se.h b/library/psa_crypto_se.h index e99bd25763..88b0127c68 100644 --- a/library/psa_crypto_se.h +++ b/library/psa_crypto_se.h @@ -42,4 +42,42 @@ */ void psa_unregister_all_se_drivers( void ); +/** A structure that describes a registered secure element driver. + * + * A secure element driver table entry contains a pointer to the + * driver's method table and a pointer to the driver's slot usage + * structure. + */ +typedef struct psa_se_drv_table_entry_s psa_se_drv_table_entry_t; + +/** Return the secure element driver table entry for a lifetime value. + * + * \param lifetime The lifetime value to query. + * + * \return The driver table entry for \p lifetime, or + * \p NULL if \p lifetime does not correspond to a registered driver. + */ +const psa_se_drv_table_entry_t *psa_get_se_driver_entry( + psa_key_lifetime_t lifetime ); + +/** Return the method table for a secure element driver. + * + * \param[in] drv The driver table entry to access. + * + * \return The driver table entry for \p lifetime, or + * \p NULL if \p lifetime does not correspond to a registered driver. + */ +const psa_drv_se_t *psa_get_se_driver_methods( + const psa_se_drv_table_entry_t *drv ); + +/** Return the secure element driver method table for a lifetime value. + * + * \param lifetime The lifetime value to query. + * + * \return The driver method table for \p lifetime, or + * \p NULL if \p lifetime does not correspond to a registered driver. + */ +const psa_drv_se_t *psa_get_se_driver( + psa_key_lifetime_t lifetime ); + #endif /* PSA_CRYPTO_SE_H */ From 6e59c42d1d2453919500ba75f2a71a9c3cc1d2f5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Jun 2019 19:06:52 +0200 Subject: [PATCH 03/50] Split the secure element driver method table memory layout Instead of having one giant table containing all possible methods, represent a driver's method table as a structure containing pointers to substructures. This way a driver that doesn't implement a certain class of operations can use NULL for this class as a whole instead of storing NULL for each method. --- include/psa/crypto_se_driver.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index fc0d96162c..87a9354752 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -980,12 +980,12 @@ typedef struct { * Use #PSA_DRV_SE_HAL_VERSION. */ uint32_t hal_version; - psa_drv_se_key_management_t key_management; - psa_drv_se_mac_t mac; - psa_drv_se_cipher_t cipher; - psa_drv_se_aead_t aead; - psa_drv_se_asymmetric_t asymmetric; - psa_drv_se_key_derivation_t derivation; + const psa_drv_se_key_management_t *key_management; + const psa_drv_se_mac_t *mac; + const psa_drv_se_cipher_t *cipher; + const psa_drv_se_aead_t *aead; + const psa_drv_se_asymmetric_t *asymmetric; + const psa_drv_se_key_derivation_t *derivation; } psa_drv_se_t; /** The current version of the secure element driver HAL. From 011e4284a1709bd8306c016ee9040922af45a24b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Jun 2019 18:34:38 +0200 Subject: [PATCH 04/50] Look up the SE driver when creating a key When creating a key with a lifetime that places it in a secure element, retrieve the appropriate driver table entry. This commit doesn't yet achieve behavior: so far the code only retrieves the driver, it doesn't call the driver. --- library/psa_crypto.c | 71 ++++++++++++++++++++-------- library/psa_crypto_slot_management.c | 14 +++++- library/psa_crypto_slot_management.h | 30 ++++++++++++ 3 files changed, 93 insertions(+), 22 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 5245e61bf4..65e1e2a74e 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1291,9 +1291,11 @@ static psa_status_t psa_set_key_policy_internal( * In case of failure at any step, stop the sequence and call * psa_fail_key_creation(). * - * \param attributes Key attributes for the new key. - * \param handle On success, a handle for the allocated slot. - * \param p_slot On success, a pointer to the prepared slot. + * \param[in] attributes Key attributes for the new key. + * \param[out] handle On success, a handle for the allocated slot. + * \param[out] p_slot On success, a pointer to the prepared slot. + * \param[out] p_drv On any return, the driver for the key, if any. + * NULL for a transparent key. * * \retval #PSA_SUCCESS * The key slot is ready to receive key material. @@ -1303,11 +1305,14 @@ static psa_status_t psa_set_key_policy_internal( static psa_status_t psa_start_key_creation( const psa_key_attributes_t *attributes, psa_key_handle_t *handle, - psa_key_slot_t **p_slot ) + psa_key_slot_t **p_slot, + const psa_se_drv_table_entry_t **p_drv ) { psa_status_t status; psa_key_slot_t *slot; + *p_drv = NULL; + status = psa_internal_allocate_key_slot( handle, p_slot ); if( status != PSA_SUCCESS ) return( status ); @@ -1317,10 +1322,12 @@ static psa_status_t psa_start_key_creation( if( status != PSA_SUCCESS ) return( status ); slot->lifetime = attributes->lifetime; + if( attributes->lifetime != PSA_KEY_LIFETIME_VOLATILE ) { status = psa_validate_persistent_key_parameters( attributes->lifetime, - attributes->id, 1 ); + attributes->id, + p_drv, 1 ); if( status != PSA_SUCCESS ) return( status ); slot->persistent_storage_id = attributes->id; @@ -1338,17 +1345,22 @@ static psa_status_t psa_start_key_creation( * See the documentation of psa_start_key_creation() for the intended use * of this function. * - * \param slot Pointer to the slot with key material. + * \param[in,out] slot Pointer to the slot with key material. + * \param[in] driver The secure element driver for the key, + * or NULL for a transparent key. * * \retval #PSA_SUCCESS * The key was successfully created. The handle is now valid. * \return If this function fails, the key slot is an invalid state. * You must call psa_fail_key_creation() to wipe and free the slot. */ -static psa_status_t psa_finish_key_creation( psa_key_slot_t *slot ) +static psa_status_t psa_finish_key_creation( + psa_key_slot_t *slot, + const psa_se_drv_table_entry_t *driver ) { psa_status_t status = PSA_SUCCESS; (void) slot; + (void) driver; #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) if( slot->lifetime == PSA_KEY_LIFETIME_PERSISTENT ) @@ -1390,12 +1402,25 @@ static psa_status_t psa_finish_key_creation( psa_key_slot_t *slot ) * See the documentation of psa_start_key_creation() for the intended use * of this function. * - * \param slot Pointer to the slot with key material. + * \param[in,out] slot Pointer to the slot with key material. + * \param[in] driver The secure element driver for the key, + * or NULL for a transparent key. */ -static void psa_fail_key_creation( psa_key_slot_t *slot ) +static void psa_fail_key_creation( psa_key_slot_t *slot, + const psa_se_drv_table_entry_t *driver ) { + (void) driver; + if( slot == NULL ) return; + +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + /* TOnogrepDO: If the key has already been created in the secure + * element, and the failure happened later (when saving metadata + * to internal storage), we need to destroy the key in the secure + * element. */ +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + psa_wipe_key_slot( slot ); } @@ -1458,8 +1483,9 @@ psa_status_t psa_import_key( const psa_key_attributes_t *attributes, { psa_status_t status; psa_key_slot_t *slot = NULL; + const psa_se_drv_table_entry_t *driver = NULL; - status = psa_start_key_creation( attributes, handle, &slot ); + status = psa_start_key_creation( attributes, handle, &slot, &driver ); if( status != PSA_SUCCESS ) goto exit; @@ -1470,11 +1496,11 @@ psa_status_t psa_import_key( const psa_key_attributes_t *attributes, if( status != PSA_SUCCESS ) goto exit; - status = psa_finish_key_creation( slot ); + status = psa_finish_key_creation( slot, driver ); exit: if( status != PSA_SUCCESS ) { - psa_fail_key_creation( slot ); + psa_fail_key_creation( slot, driver ); *handle = 0; } return( status ); @@ -1514,6 +1540,7 @@ psa_status_t psa_copy_key( psa_key_handle_t source_handle, psa_key_slot_t *source_slot = NULL; psa_key_slot_t *target_slot = NULL; psa_key_attributes_t actual_attributes = *specified_attributes; + const psa_se_drv_table_entry_t *driver = NULL; status = psa_get_key_from_slot( source_handle, &source_slot, PSA_KEY_USAGE_COPY, 0 ); @@ -1530,7 +1557,7 @@ psa_status_t psa_copy_key( psa_key_handle_t source_handle, goto exit; status = psa_start_key_creation( &actual_attributes, - target_handle, &target_slot ); + target_handle, &target_slot, &driver ); if( status != PSA_SUCCESS ) goto exit; @@ -1538,11 +1565,11 @@ psa_status_t psa_copy_key( psa_key_handle_t source_handle, if( status != PSA_SUCCESS ) goto exit; - status = psa_finish_key_creation( target_slot ); + status = psa_finish_key_creation( target_slot, driver ); exit: if( status != PSA_SUCCESS ) { - psa_fail_key_creation( target_slot ); + psa_fail_key_creation( target_slot, driver ); *target_handle = 0; } return( status ); @@ -4437,7 +4464,8 @@ psa_status_t psa_key_derivation_output_key( const psa_key_attributes_t *attribut { psa_status_t status; psa_key_slot_t *slot = NULL; - status = psa_start_key_creation( attributes, handle, &slot ); + const psa_se_drv_table_entry_t *driver = NULL; + status = psa_start_key_creation( attributes, handle, &slot, &driver ); if( status == PSA_SUCCESS ) { status = psa_generate_derived_key_internal( slot, @@ -4445,10 +4473,10 @@ psa_status_t psa_key_derivation_output_key( const psa_key_attributes_t *attribut operation ); } if( status == PSA_SUCCESS ) - status = psa_finish_key_creation( slot ); + status = psa_finish_key_creation( slot, driver ); if( status != PSA_SUCCESS ) { - psa_fail_key_creation( slot ); + psa_fail_key_creation( slot, driver ); *handle = 0; } return( status ); @@ -5467,7 +5495,8 @@ psa_status_t psa_generate_key( const psa_key_attributes_t *attributes, { psa_status_t status; psa_key_slot_t *slot = NULL; - status = psa_start_key_creation( attributes, handle, &slot ); + const psa_se_drv_table_entry_t *driver = NULL; + status = psa_start_key_creation( attributes, handle, &slot, &driver ); if( status == PSA_SUCCESS ) { status = psa_generate_key_internal( @@ -5475,10 +5504,10 @@ psa_status_t psa_generate_key( const psa_key_attributes_t *attributes, attributes->domain_parameters, attributes->domain_parameters_size ); } if( status == PSA_SUCCESS ) - status = psa_finish_key_creation( slot ); + status = psa_finish_key_creation( slot, driver ); if( status != PSA_SUCCESS ) { - psa_fail_key_creation( slot ); + psa_fail_key_creation( slot, driver ); *handle = 0; } return( status ); diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 900aa41a5f..eb24b6b4ca 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -168,8 +168,20 @@ static int psa_is_key_id_valid( psa_key_file_id_t file_id, psa_status_t psa_validate_persistent_key_parameters( psa_key_lifetime_t lifetime, psa_key_file_id_t id, + const psa_se_drv_table_entry_t **p_drv, int creating ) { + if( p_drv != NULL ) + *p_drv = NULL; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( psa_key_lifetime_is_external( lifetime ) ) + { + *p_drv = psa_get_se_driver_entry( lifetime ); + if( *p_drv == NULL ) + return( PSA_ERROR_INVALID_ARGUMENT ); + } + else +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ if( lifetime != PSA_KEY_LIFETIME_PERSISTENT ) return( PSA_ERROR_INVALID_ARGUMENT ); @@ -194,7 +206,7 @@ psa_status_t psa_open_key( psa_key_file_id_t id, psa_key_handle_t *handle ) *handle = 0; status = psa_validate_persistent_key_parameters( - PSA_KEY_LIFETIME_PERSISTENT, id, 0 ); + PSA_KEY_LIFETIME_PERSISTENT, id, NULL, 0 ); if( status != PSA_SUCCESS ) return( status ); diff --git a/library/psa_crypto_slot_management.h b/library/psa_crypto_slot_management.h index 5c1bde146e..8111c4a62b 100644 --- a/library/psa_crypto_slot_management.h +++ b/library/psa_crypto_slot_management.h @@ -22,6 +22,9 @@ #ifndef PSA_CRYPTO_SLOT_MANAGEMENT_H #define PSA_CRYPTO_SLOT_MANAGEMENT_H +#include "psa/crypto.h" +#include "psa_crypto_se.h" + /* Number of key slots (plus one because 0 is not used). * The value is a compile-time constant for now, for simplicity. */ #define PSA_KEY_SLOT_COUNT 32 @@ -71,6 +74,24 @@ void psa_wipe_all_key_slots( void ); psa_status_t psa_internal_allocate_key_slot( psa_key_handle_t *handle, psa_key_slot_t **p_slot ); +/** Test whether a lifetime designates a key in an external cryptoprocessor. + * + * \param lifetime The lifetime to test. + * + * \retval 1 + * The lifetime designates an external key. There should be a + * registered driver for this lifetime, otherwise the key cannot + * be created or manipulated. + * \retval 0 + * The lifetime designates a key that is volatile or in internal + * storage. + */ +static inline int psa_key_lifetime_is_external( psa_key_lifetime_t lifetime ) +{ + return( lifetime != PSA_KEY_LIFETIME_VOLATILE && + lifetime != PSA_KEY_LIFETIME_PERSISTENT ); +} + /** Test whether the given parameters are acceptable for a persistent key. * * This function does not access the storage in any way. It only tests @@ -78,8 +99,16 @@ psa_status_t psa_internal_allocate_key_slot( psa_key_handle_t *handle, * It does not test whether the a file by the given id exists or could be * created. * + * If the key is in external storage, this function returns the corresponding + * driver. + * * \param lifetime The lifetime to test. * \param id The key id to test. + * \param[out] p_drv On output, if \p lifetime designates a key + * in an external processor, \c *p_drv is a pointer + * to the driver table entry fot this lifetime. + * If \p lifetime designates a transparent key, + * \c *p_drv is \c NULL. * \param creating 0 if attempting to open an existing key. * Nonzero if attempting to create a key. * @@ -93,6 +122,7 @@ psa_status_t psa_internal_allocate_key_slot( psa_key_handle_t *handle, psa_status_t psa_validate_persistent_key_parameters( psa_key_lifetime_t lifetime, psa_key_file_id_t id, + const psa_se_drv_table_entry_t **p_drv, int creating ); From f03143a4d1b9eed8674d61884a09ba773e4572a9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:18:29 +0200 Subject: [PATCH 05/50] Change driver key slot numbers to 64 bits This slightly increases storage requirements, but works in more use cases. In particular, it allows drivers to treat choose slot numbers with a monotonic counter that is incremented each time a key is created, without worrying about overflow in practice. --- include/psa/crypto_se_driver.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 87a9354752..c53b34ade8 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -43,7 +43,7 @@ extern "C" { /** An internal designation of a key slot between the core part of the * PSA Crypto implementation and the driver. The meaning of this value * is driver-dependent. */ -typedef uint32_t psa_key_slot_number_t; // Change this to psa_key_slot_t after psa_key_slot_t is removed from Mbed crypto +typedef uint64_t psa_key_slot_number_t; /** \defgroup se_mac Secure Element Message Authentication Codes * Generation and authentication of Message Authentication Codes (MACs) using From 7a86da1d428fd7041cfed572c1ae6dc1a0777f53 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:25:38 +0200 Subject: [PATCH 06/50] Define a driver context structure type Define a structure that is to be instantiated once per driver instance. Define a driver initialization method and pass it the driver context. --- include/psa/crypto_se_driver.h | 89 ++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index c53b34ade8..58515a1881 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -40,11 +40,84 @@ extern "C" { #endif +/** \defgroup se_init Secure element driver initialization + */ +/**@{*/ + +/** \brief Driver context structure + * + * Driver functions receive a pointer to this structure. + * Each registered driver has one instance of this structure. + * + * Implementations must include the fields specified here and + * may include other fields. + */ +typedef struct { + /** A read-only pointer to the driver's persistent data. + * + * The PSA Cryptography core saves the persistent data from one + * session to the next. + * + * The core allocates a memory buffer for the persistent data. + * The pointer is guaranteed to be suitably alignedfor any data type, + * like a pointer returned by `malloc` (but the core can use any + * method to allocate the buffer, not necessarily `malloc`). + * + * The size of this buffer is given by psa_drv_se_t::persistent_data_size + * when the driver is registered, and this value is also recorded in the + * ::persistent_data_size field of this structure. + * + * Before the driver is initialized for the first time, the content of + * the persistent data is all-bits-zero. After a driver upgrade, if the + * size of the persistent data has increased, the original data is padded + * on the right with zeros; if the size has decreased, the original data + * is truncated to the new size. + * + * This pointer is to read-only data. Only a few driver functions are + * allowed to modify the persistent data. These functions receive a + * writable pointer. + */ + const void *const persistent_data; + + /** The size of \c persistent_data in bytes. + * + * This is always equal to the value of + * psa_drv_se_t::persistent_data_size when the driver is registered. + */ + const size_t persistent_data_size; + + /** Driver transient data. + * + * The core initializes this value to 0 and does not read or modify it + * afterwards. The driver may store whatever it wants in this field. + */ + uintptr_t transient_data; +} psa_drv_se_context_t; + +/** \brief A driver initialization function. + * + * \param[in,out] drv_context The driver context structure. + * \param lifetime The lifetime value for which this driver + * is registered. + * + * \retval #PSA_SUCCESS + * The driver is operational. + * The core will update the persistent data in storage. + * \return + * Any other return value prevents the driver from being used in + * this session. + * The core will NOT update the persistent data in storage. + */ +typedef psa_status_t (*psa_drv_se_init_t)(psa_drv_se_context_t *drv_context, + psa_key_lifetime_t lifetime); + /** An internal designation of a key slot between the core part of the * PSA Crypto implementation and the driver. The meaning of this value * is driver-dependent. */ typedef uint64_t psa_key_slot_number_t; +/**@}*/ + /** \defgroup se_mac Secure Element Message Authentication Codes * Generation and authentication of Message Authentication Codes (MACs) using * a secure element can be done either as a single function call (via the @@ -980,6 +1053,22 @@ typedef struct { * Use #PSA_DRV_SE_HAL_VERSION. */ uint32_t hal_version; + + /** The size of the driver's persistent data in bytes. */ + size_t persistent_data_size; + + /** The driver initialization function. + * + * This function is called once during the initialization of the + * PSA Cryptography subsystem, before any other function of the + * driver is called. If this function returns a failure status, + * the driver will be unusable, at least until the next system reset. + * + * If this field is \c NULL, it is equivalent to a function that does + * nothing and returns #PSA_SUCCESS. + */ + psa_drv_se_init_t p_init; + const psa_drv_se_key_management_t *key_management; const psa_drv_se_mac_t *mac; const psa_drv_se_cipher_t *cipher; From 8597bc13e7250db36f55e9521f9e7c56b468bab0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:28:46 +0200 Subject: [PATCH 07/50] Pass the driver context to most driver methods Pass the driver context to all driver methods except the ones that operate on an already-setup operation context. Rename `p_context` arguments to `op_context` to avoid confusion between contexts. --- include/psa/crypto_se_driver.h | 115 +++++++++++++++++++++------------ 1 file changed, 74 insertions(+), 41 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 58515a1881..7e1d3573d5 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -138,7 +138,8 @@ typedef uint64_t psa_key_slot_number_t; /** \brief A function that starts a secure element MAC operation for a PSA * Crypto Driver implementation * - * \param[in,out] p_context A structure that will contain the + * \param[in,out] drv_context The driver context structure. + * \param[in,out] op_context A structure that will contain the * hardware-specific MAC context * \param[in] key_slot The slot of the key to be used for the * operation @@ -148,28 +149,29 @@ typedef uint64_t psa_key_slot_number_t; * \retval PSA_SUCCESS * Success. */ -typedef psa_status_t (*psa_drv_se_mac_setup_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_mac_setup_t)(psa_drv_se_context_t *drv_context, + void *op_context, psa_key_slot_number_t key_slot, psa_algorithm_t algorithm); /** \brief A function that continues a previously started secure element MAC * operation * - * \param[in,out] p_context A hardware-specific structure for the + * \param[in,out] op_context A hardware-specific structure for the * previously-established MAC operation to be * updated * \param[in] p_input A buffer containing the message to be appended * to the MAC operation * \param[in] input_length The size in bytes of the input message buffer */ -typedef psa_status_t (*psa_drv_se_mac_update_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_mac_update_t)(void *op_context, const uint8_t *p_input, size_t input_length); /** \brief a function that completes a previously started secure element MAC * operation by returning the resulting MAC. * - * \param[in,out] p_context A hardware-specific structure for the + * \param[in,out] op_context A hardware-specific structure for the * previously started MAC operation to be * finished * \param[out] p_mac A buffer where the generated MAC will be @@ -182,7 +184,7 @@ typedef psa_status_t (*psa_drv_se_mac_update_t)(void *p_context, * \retval PSA_SUCCESS * Success. */ -typedef psa_status_t (*psa_drv_se_mac_finish_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_mac_finish_t)(void *op_context, uint8_t *p_mac, size_t mac_size, size_t *p_mac_length); @@ -190,7 +192,7 @@ typedef psa_status_t (*psa_drv_se_mac_finish_t)(void *p_context, /** \brief A function that completes a previously started secure element MAC * operation by comparing the resulting MAC against a provided value * - * \param[in,out] p_context A hardware-specific structure for the previously + * \param[in,out] op_context A hardware-specific structure for the previously * started MAC operation to be fiinished * \param[in] p_mac The MAC value against which the resulting MAC will * be compared against @@ -203,21 +205,22 @@ typedef psa_status_t (*psa_drv_se_mac_finish_t)(void *p_context, * The operation completed successfully, but the calculated MAC did * not match the provided MAC */ -typedef psa_status_t (*psa_drv_se_mac_finish_verify_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_mac_finish_verify_t)(void *op_context, const uint8_t *p_mac, size_t mac_length); /** \brief A function that aborts a previous started secure element MAC * operation * - * \param[in,out] p_context A hardware-specific structure for the previously + * \param[in,out] op_context A hardware-specific structure for the previously * started MAC operation to be aborted */ -typedef psa_status_t (*psa_drv_se_mac_abort_t)(void *p_context); +typedef psa_status_t (*psa_drv_se_mac_abort_t)(void *op_context); /** \brief A function that performs a secure element MAC operation in one * command and returns the calculated MAC * + * \param[in,out] drv_context The driver context structure. * \param[in] p_input A buffer containing the message to be MACed * \param[in] input_length The size in bytes of `p_input` * \param[in] key_slot The slot of the key to be used @@ -232,7 +235,8 @@ typedef psa_status_t (*psa_drv_se_mac_abort_t)(void *p_context); * \retval PSA_SUCCESS * Success. */ -typedef psa_status_t (*psa_drv_se_mac_generate_t)(const uint8_t *p_input, +typedef psa_status_t (*psa_drv_se_mac_generate_t)(psa_drv_se_context_t *drv_context, + const uint8_t *p_input, size_t input_length, psa_key_slot_number_t key_slot, psa_algorithm_t alg, @@ -243,6 +247,7 @@ typedef psa_status_t (*psa_drv_se_mac_generate_t)(const uint8_t *p_input, /** \brief A function that performs a secure element MAC operation in one * command and compares the resulting MAC against a provided value * + * \param[in,out] drv_context The driver context structure. * \param[in] p_input A buffer containing the message to be MACed * \param[in] input_length The size in bytes of `input` * \param[in] key_slot The slot of the key to be used @@ -259,7 +264,8 @@ typedef psa_status_t (*psa_drv_se_mac_generate_t)(const uint8_t *p_input, * The operation completed successfully, but the calculated MAC did * not match the provided MAC */ -typedef psa_status_t (*psa_drv_se_mac_verify_t)(const uint8_t *p_input, +typedef psa_status_t (*psa_drv_se_mac_verify_t)(psa_drv_se_context_t *drv_context, + const uint8_t *p_input, size_t input_length, psa_key_slot_number_t key_slot, psa_algorithm_t alg, @@ -336,7 +342,8 @@ typedef struct { /** \brief A function that provides the cipher setup function for a * secure element driver * - * \param[in,out] p_context A structure that will contain the + * \param[in,out] drv_context The driver context structure. + * \param[in,out] op_context A structure that will contain the * hardware-specific cipher context. * \param[in] key_slot The slot of the key to be used for the * operation @@ -348,7 +355,8 @@ typedef struct { * \retval PSA_SUCCESS * \retval PSA_ERROR_NOT_SUPPORTED */ -typedef psa_status_t (*psa_drv_se_cipher_setup_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_cipher_setup_t)(psa_drv_se_context_t *drv_context, + void *op_context, psa_key_slot_number_t key_slot, psa_algorithm_t algorithm, psa_encrypt_or_decrypt_t direction); @@ -361,21 +369,21 @@ typedef psa_status_t (*psa_drv_se_cipher_setup_t)(void *p_context, * generate function is not necessary for the drivers to implement as the PSA * Crypto implementation can do the generation using its RNG features. * - * \param[in,out] p_context A structure that contains the previously set up + * \param[in,out] op_context A structure that contains the previously set up * hardware-specific cipher context * \param[in] p_iv A buffer containing the initialization vector * \param[in] iv_length The size (in bytes) of the `p_iv` buffer * * \retval PSA_SUCCESS */ -typedef psa_status_t (*psa_drv_se_cipher_set_iv_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_cipher_set_iv_t)(void *op_context, const uint8_t *p_iv, size_t iv_length); /** \brief A function that continues a previously started secure element cipher * operation * - * \param[in,out] p_context A hardware-specific structure for the + * \param[in,out] op_context A hardware-specific structure for the * previously started cipher operation * \param[in] p_input A buffer containing the data to be * encrypted/decrypted @@ -390,7 +398,7 @@ typedef psa_status_t (*psa_drv_se_cipher_set_iv_t)(void *p_context, * * \retval PSA_SUCCESS */ -typedef psa_status_t (*psa_drv_se_cipher_update_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_cipher_update_t)(void *op_context, const uint8_t *p_input, size_t input_size, uint8_t *p_output, @@ -400,7 +408,7 @@ typedef psa_status_t (*psa_drv_se_cipher_update_t)(void *p_context, /** \brief A function that completes a previously started secure element cipher * operation * - * \param[in,out] p_context A hardware-specific structure for the + * \param[in,out] op_context A hardware-specific structure for the * previously started cipher operation * \param[out] p_output The caller-allocated buffer where the output * will be placed @@ -411,7 +419,7 @@ typedef psa_status_t (*psa_drv_se_cipher_update_t)(void *p_context, * * \retval PSA_SUCCESS */ -typedef psa_status_t (*psa_drv_se_cipher_finish_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_cipher_finish_t)(void *op_context, uint8_t *p_output, size_t output_size, size_t *p_output_length); @@ -419,10 +427,10 @@ typedef psa_status_t (*psa_drv_se_cipher_finish_t)(void *p_context, /** \brief A function that aborts a previously started secure element cipher * operation * - * \param[in,out] p_context A hardware-specific structure for the + * \param[in,out] op_context A hardware-specific structure for the * previously started cipher operation */ -typedef psa_status_t (*psa_drv_se_cipher_abort_t)(void *p_context); +typedef psa_status_t (*psa_drv_se_cipher_abort_t)(void *op_context); /** \brief A function that performs the ECB block mode for secure element * cipher operations @@ -430,6 +438,7 @@ typedef psa_status_t (*psa_drv_se_cipher_abort_t)(void *p_context); * Note: this function should only be used with implementations that do not * provide a needed higher-level operation. * + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot The slot of the key to be used for the operation * \param[in] algorithm The algorithm to be used in the cipher operation * \param[in] direction Indicates whether the operation is an encrypt or @@ -446,7 +455,8 @@ typedef psa_status_t (*psa_drv_se_cipher_abort_t)(void *p_context); * \retval PSA_SUCCESS * \retval PSA_ERROR_NOT_SUPPORTED */ -typedef psa_status_t (*psa_drv_se_cipher_ecb_t)(psa_key_slot_number_t key_slot, +typedef psa_status_t (*psa_drv_se_cipher_ecb_t)(psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot, psa_algorithm_t algorithm, psa_encrypt_or_decrypt_t direction, const uint8_t *p_input, @@ -500,6 +510,7 @@ typedef struct { * \brief A function that signs a hash or short message with a private key in * a secure element * + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Key slot of an asymmetric key pair * \param[in] alg A signature algorithm that is compatible * with the type of `key` @@ -512,7 +523,8 @@ typedef struct { * * \retval PSA_SUCCESS */ -typedef psa_status_t (*psa_drv_se_asymmetric_sign_t)(psa_key_slot_number_t key_slot, +typedef psa_status_t (*psa_drv_se_asymmetric_sign_t)(psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot, psa_algorithm_t alg, const uint8_t *p_hash, size_t hash_length, @@ -524,6 +536,7 @@ typedef psa_status_t (*psa_drv_se_asymmetric_sign_t)(psa_key_slot_number_t key_s * \brief A function that verifies the signature a hash or short message using * an asymmetric public key in a secure element * + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Key slot of a public key or an asymmetric key * pair * \param[in] alg A signature algorithm that is compatible with @@ -536,7 +549,8 @@ typedef psa_status_t (*psa_drv_se_asymmetric_sign_t)(psa_key_slot_number_t key_s * \retval PSA_SUCCESS * The signature is valid. */ -typedef psa_status_t (*psa_drv_se_asymmetric_verify_t)(psa_key_slot_number_t key_slot, +typedef psa_status_t (*psa_drv_se_asymmetric_verify_t)(psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot, psa_algorithm_t alg, const uint8_t *p_hash, size_t hash_length, @@ -547,6 +561,7 @@ typedef psa_status_t (*psa_drv_se_asymmetric_verify_t)(psa_key_slot_number_t key * \brief A function that encrypts a short message with an asymmetric public * key in a secure element * + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Key slot of a public key or an asymmetric key * pair * \param[in] alg An asymmetric encryption algorithm that is @@ -572,7 +587,8 @@ typedef psa_status_t (*psa_drv_se_asymmetric_verify_t)(psa_key_slot_number_t key * * \retval PSA_SUCCESS */ -typedef psa_status_t (*psa_drv_se_asymmetric_encrypt_t)(psa_key_slot_number_t key_slot, +typedef psa_status_t (*psa_drv_se_asymmetric_encrypt_t)(psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot, psa_algorithm_t alg, const uint8_t *p_input, size_t input_length, @@ -586,6 +602,7 @@ typedef psa_status_t (*psa_drv_se_asymmetric_encrypt_t)(psa_key_slot_number_t ke * \brief A function that decrypts a short message with an asymmetric private * key in a secure element. * + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Key slot of an asymmetric key pair * \param[in] alg An asymmetric encryption algorithm that is * compatible with the type of `key` @@ -610,7 +627,8 @@ typedef psa_status_t (*psa_drv_se_asymmetric_encrypt_t)(psa_key_slot_number_t ke * * \retval PSA_SUCCESS */ -typedef psa_status_t (*psa_drv_se_asymmetric_decrypt_t)(psa_key_slot_number_t key_slot, +typedef psa_status_t (*psa_drv_se_asymmetric_decrypt_t)(psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot, psa_algorithm_t alg, const uint8_t *p_input, size_t input_length, @@ -654,6 +672,7 @@ typedef struct { /** \brief A function that performs a secure element authenticated encryption * operation * + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Slot containing the key to use. * \param[in] algorithm The AEAD algorithm to compute * (\c PSA_ALG_XXX value such that @@ -681,7 +700,8 @@ typedef struct { * \retval #PSA_SUCCESS * Success. */ -typedef psa_status_t (*psa_drv_se_aead_encrypt_t)(psa_key_slot_number_t key_slot, +typedef psa_status_t (*psa_drv_se_aead_encrypt_t)(psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot, psa_algorithm_t algorithm, const uint8_t *p_nonce, size_t nonce_length, @@ -695,6 +715,7 @@ typedef psa_status_t (*psa_drv_se_aead_encrypt_t)(psa_key_slot_number_t key_slot /** A function that peforms a secure element authenticated decryption operation * + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Slot containing the key to use * \param[in] algorithm The AEAD algorithm to compute * (\c PSA_ALG_XXX value such that @@ -721,7 +742,8 @@ typedef psa_status_t (*psa_drv_se_aead_encrypt_t)(psa_key_slot_number_t key_slot * \retval #PSA_SUCCESS * Success. */ -typedef psa_status_t (*psa_drv_se_aead_decrypt_t)(psa_key_slot_number_t key_slot, +typedef psa_status_t (*psa_drv_se_aead_decrypt_t)(psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot, psa_algorithm_t algorithm, const uint8_t *p_nonce, size_t nonce_length, @@ -763,6 +785,7 @@ typedef struct { * This function can support any output from psa_export_key(). Refer to the * documentation of psa_export_key() for the format for each key type. * + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Slot where the key will be stored * This must be a valid slot for a key of the chosen * type. It must be unoccupied. @@ -776,7 +799,8 @@ typedef struct { * \retval #PSA_SUCCESS * Success. */ -typedef psa_status_t (*psa_drv_se_import_key_t)(psa_key_slot_number_t key_slot, +typedef psa_status_t (*psa_drv_se_import_key_t)(psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot, psa_key_lifetime_t lifetime, psa_key_type_t type, psa_algorithm_t algorithm, @@ -794,12 +818,15 @@ typedef psa_status_t (*psa_drv_se_import_key_t)(psa_key_slot_number_t key_slot, * * This function returns the specified slot to its default state. * - * \param[in] key_slot The key slot to erase. + * \param[in,out] drv_context The driver context structure. + * \param key_slot The key slot to erase. * * \retval #PSA_SUCCESS * The slot's content, if any, has been erased. */ -typedef psa_status_t (*psa_drv_se_destroy_key_t)(psa_key_slot_number_t key_slot); +typedef psa_status_t (*psa_drv_se_destroy_key_t)( + psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot); /** * \brief A function that exports a secure element key in binary format @@ -816,6 +843,7 @@ typedef psa_status_t (*psa_drv_se_destroy_key_t)(psa_key_slot_number_t key_slot) * `psa_export_key()` does. Refer to the * documentation of `psa_export_key()` for the format for each key type. * + * \param[in,out] drv_context The driver context structure. * \param[in] key Slot whose content is to be exported. This must * be an occupied key slot. * \param[out] p_data Buffer where the key data is to be written. @@ -831,7 +859,8 @@ typedef psa_status_t (*psa_drv_se_destroy_key_t)(psa_key_slot_number_t key_slot) * \retval #PSA_ERROR_HARDWARE_FAILURE * \retval #PSA_ERROR_CORRUPTION_DETECTED */ -typedef psa_status_t (*psa_drv_se_export_key_t)(psa_key_slot_number_t key, +typedef psa_status_t (*psa_drv_se_export_key_t)(psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key, uint8_t *p_data, size_t data_size, size_t *p_data_length); @@ -845,6 +874,7 @@ typedef psa_status_t (*psa_drv_se_export_key_t)(psa_key_slot_number_t key, * The format of the public key information will match the format specified for * the psa_export_key() function for the key type. * + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Slot where the generated key will be placed * \param[in] type The type of the key to be generated * \param[in] usage The prescribed usage of the generated key @@ -864,7 +894,8 @@ typedef psa_status_t (*psa_drv_se_export_key_t)(psa_key_slot_number_t key, * \param[out] p_pubkey_length Upon successful completion, will contain the * size of the data placed in `p_pubkey_out`. */ -typedef psa_status_t (*psa_drv_se_generate_key_t)(psa_key_slot_number_t key_slot, +typedef psa_status_t (*psa_drv_se_generate_key_t)(psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot, psa_key_type_t type, psa_key_usage_t usage, size_t bits, @@ -950,7 +981,8 @@ typedef struct { /** \brief A function that Sets up a secure element key derivation operation by * specifying the algorithm and the source key sot * - * \param[in,out] p_context A hardware-specific structure containing any + * \param[in,out] drv_context The driver context structure. + * \param[in,out] op_context A hardware-specific structure containing any * context information for the implementation * \param[in] kdf_alg The algorithm to be used for the key derivation * \param[in] source_key The key to be used as the source material for the @@ -958,7 +990,8 @@ typedef struct { * * \retval PSA_SUCCESS */ -typedef psa_status_t (*psa_drv_se_key_derivation_setup_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_key_derivation_setup_t)(psa_drv_se_context_t *drv_context, + void *op_context, psa_algorithm_t kdf_alg, psa_key_slot_number_t source_key); @@ -969,7 +1002,7 @@ typedef psa_status_t (*psa_drv_se_key_derivation_setup_t)(void *p_context, * expeced that this function may be called multiple times for the same * operation, each with a different algorithm-specific `collateral_id` * - * \param[in,out] p_context A hardware-specific structure containing any + * \param[in,out] op_context A hardware-specific structure containing any * context information for the implementation * \param[in] collateral_id An ID for the collateral being provided * \param[in] p_collateral A buffer containing the collateral data @@ -977,7 +1010,7 @@ typedef psa_status_t (*psa_drv_se_key_derivation_setup_t)(void *p_context, * * \retval PSA_SUCCESS */ -typedef psa_status_t (*psa_drv_se_key_derivation_collateral_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_key_derivation_collateral_t)(void *op_context, uint32_t collateral_id, const uint8_t *p_collateral, size_t collateral_size); @@ -985,14 +1018,14 @@ typedef psa_status_t (*psa_drv_se_key_derivation_collateral_t)(void *p_context, /** \brief A function that performs the final secure element key derivation * step and place the generated key material in a slot * - * \param[in,out] p_context A hardware-specific structure containing any + * \param[in,out] op_context A hardware-specific structure containing any * context information for the implementation * \param[in] dest_key The slot where the generated key material * should be placed * * \retval PSA_SUCCESS */ -typedef psa_status_t (*psa_drv_se_key_derivation_derive_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_key_derivation_derive_t)(void *op_context, psa_key_slot_number_t dest_key); /** \brief A function that performs the final step of a secure element key @@ -1006,7 +1039,7 @@ typedef psa_status_t (*psa_drv_se_key_derivation_derive_t)(void *p_context, * * \retval PSA_SUCCESS */ -typedef psa_status_t (*psa_drv_se_key_derivation_export_t)(void *p_context, +typedef psa_status_t (*psa_drv_se_key_derivation_export_t)(void *op_context, uint8_t *p_output, size_t output_size, size_t *p_output_length); From f2223c868db35192472b82fcd57cfd34c1e8e227 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:33:02 +0200 Subject: [PATCH 08/50] New driver method: allocate Add a driver method to allocate a key slot for a key that is about to be created. --- include/psa/crypto_se_driver.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 7e1d3573d5..4458562d12 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -780,6 +780,30 @@ typedef struct { */ /**@{*/ +/* This type is documented in crypto.h. As far as drivers are concerned, + * this is an opaque type. */ +typedef struct psa_key_attributes_s psa_key_attributes_t; + +/** \brief A function that allocates a slot for a key. + * + * \param[in,out] drv_context The driver context structure. + * \param[in] attributes Attributes of the key. + * \param[out] key_slot Slot where the key will be stored. + * This must be a valid slot for a key of the + * chosen type. It must be unoccupied. + * + * \retval #PSA_SUCCESS + * Success. + * The core will record \c *key_slot as the key slot where the key + * is stored and will update the persistent data in storage. + * \retval #PSA_ERROR_NOT_SUPPORTED + * \retval #PSA_ERROR_INSUFFICIENT_STORAGE + */ +typedef psa_status_t (*psa_drv_se_allocate_key_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 @@ -915,6 +939,8 @@ 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. */ + psa_drv_se_allocate_key_t p_allocate; /** Function that performs a key import operation */ psa_drv_se_import_key_t p_import; /** Function that performs a generation */ From 94cc42c28f3f1e4d3c63ef0da8b767255c5d8118 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:34:20 +0200 Subject: [PATCH 09/50] Pass a writable pointer to the persistent data when needed Most driver methods are not allowed to modify the persistent data, so the driver context structure contains a const pointer to it. Pass a non-const pointer to the persstent data to the driver methods that need it: init, allocate, destroy. --- include/psa/crypto_se_driver.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 4458562d12..bdc038e880 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -97,6 +97,8 @@ typedef struct { /** \brief A driver initialization function. * * \param[in,out] drv_context The driver context structure. + * \param[in,out] persistent_data A pointer to the persistent data + * that allows writing. * \param lifetime The lifetime value for which this driver * is registered. * @@ -109,6 +111,7 @@ typedef struct { * The core will NOT update the persistent data in storage. */ typedef psa_status_t (*psa_drv_se_init_t)(psa_drv_se_context_t *drv_context, + void *persistent_data, psa_key_lifetime_t lifetime); /** An internal designation of a key slot between the core part of the @@ -787,6 +790,8 @@ typedef struct psa_key_attributes_s psa_key_attributes_t; /** \brief A function that allocates a slot for a key. * * \param[in,out] drv_context The driver context structure. + * \param[in,out] persistent_data A pointer to the persistent data + * that allows writing. * \param[in] attributes Attributes of the key. * \param[out] key_slot Slot where the key will be stored. * This must be a valid slot for a key of the @@ -801,6 +806,7 @@ typedef struct psa_key_attributes_s psa_key_attributes_t; */ typedef psa_status_t (*psa_drv_se_allocate_key_t)( psa_drv_se_context_t *drv_context, + void *persistent_data, const psa_key_attributes_t *attributes, psa_key_slot_number_t *key_slot); @@ -843,6 +849,8 @@ typedef psa_status_t (*psa_drv_se_import_key_t)(psa_drv_se_context_t *drv_contex * This function returns the specified slot to its default state. * * \param[in,out] drv_context The driver context structure. + * \param[in,out] persistent_data A pointer to the persistent data + * that allows writing. * \param key_slot The key slot to erase. * * \retval #PSA_SUCCESS @@ -850,6 +858,7 @@ typedef psa_status_t (*psa_drv_se_import_key_t)(psa_drv_se_context_t *drv_contex */ typedef psa_status_t (*psa_drv_se_destroy_key_t)( psa_drv_se_context_t *drv_context, + void *persistent_data, psa_key_slot_number_t key_slot); /** From 5243a202c3e1d9193e02d2725fb516aa179159bc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:38:19 +0200 Subject: [PATCH 10/50] Driver context manipulation functions Create the driver context when registering the driver. Implement some helper functions to access driver information. --- library/psa_crypto_se.c | 102 ++++++++++++++++++++++++++++++++++++---- library/psa_crypto_se.h | 61 +++++++++++++++++++----- 2 files changed, 141 insertions(+), 22 deletions(-) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index 70e3a16802..b95b2a5d5f 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -28,23 +28,49 @@ #if defined(MBEDTLS_PSA_CRYPTO_SE_C) #include +#include #include +#include "psa/crypto_se_driver.h" + #include "psa_crypto_se.h" +#include "mbedtls/platform.h" +#if !defined(MBEDTLS_PLATFORM_C) +#define mbedtls_calloc calloc +#define mbedtls_free free +#endif + + + /****************************************************************/ /* Driver lookup */ /****************************************************************/ +/* This structure is identical to psa_drv_se_context_t declared in + * `crypto_se_driver.h`, except that some parts are writable here + * (non-const, or pointer to non-const). */ +typedef struct +{ + void *persistent_data; + size_t persistent_data_size; + uintptr_t transient_data; +} psa_drv_se_internal_context_t; + typedef struct psa_se_drv_table_entry_s { psa_key_lifetime_t lifetime; const psa_drv_se_t *methods; + union + { + psa_drv_se_internal_context_t internal; + psa_drv_se_context_t context; + }; } psa_se_drv_table_entry_t; static psa_se_drv_table_entry_t driver_table[PSA_MAX_SE_DRIVERS]; -const psa_se_drv_table_entry_t *psa_get_se_driver_entry( +psa_se_drv_table_entry_t *psa_get_se_driver_entry( psa_key_lifetime_t lifetime ) { size_t i; @@ -59,20 +85,50 @@ const psa_se_drv_table_entry_t *psa_get_se_driver_entry( } const psa_drv_se_t *psa_get_se_driver_methods( - const psa_se_drv_table_entry_t *drv ) + const psa_se_drv_table_entry_t *driver ) { - return( drv->methods ); + return( driver->methods ); } -const psa_drv_se_t *psa_get_se_driver( psa_key_lifetime_t lifetime ) +psa_drv_se_context_t *psa_get_se_driver_context( + psa_se_drv_table_entry_t *driver ) { - const psa_se_drv_table_entry_t *drv = psa_get_se_driver_entry( lifetime ); - if( drv == NULL ) - return( NULL ); - else - return( drv->methods ); + return( &driver->context ); } +int psa_get_se_driver( psa_key_lifetime_t lifetime, + const psa_drv_se_t **p_methods, + psa_drv_se_context_t **p_drv_context) +{ + psa_se_drv_table_entry_t *driver = psa_get_se_driver_entry( lifetime ); + if( p_methods != NULL ) + *p_methods = ( driver ? driver->methods : NULL ); + if( p_drv_context != NULL ) + *p_drv_context = ( driver ? &driver->context : NULL ); + return( driver != NULL ); +} + + + +/****************************************************************/ +/* Persistent data management */ +/****************************************************************/ + +psa_status_t psa_load_se_persistent_data( + const psa_se_drv_table_entry_t *driver ) +{ + /*TODO*/ + (void) driver; + return( PSA_SUCCESS ); +} + +psa_status_t psa_save_se_persistent_data( + const psa_se_drv_table_entry_t *driver ) +{ + /*TODO*/ + (void) driver; + return( PSA_SUCCESS ); +} @@ -85,6 +141,7 @@ psa_status_t psa_register_se_driver( const psa_drv_se_t *methods) { size_t i; + psa_status_t status; if( methods->hal_version != PSA_DRV_SE_HAL_VERSION ) return( PSA_ERROR_NOT_SUPPORTED ); @@ -115,11 +172,38 @@ psa_status_t psa_register_se_driver( driver_table[i].lifetime = lifetime; driver_table[i].methods = methods; + + if( methods->persistent_data_size != 0 ) + { + driver_table[i].internal.persistent_data = + mbedtls_calloc( 1, methods->persistent_data_size ); + if( driver_table[i].internal.persistent_data == NULL ) + { + status = PSA_ERROR_INSUFFICIENT_MEMORY; + goto error; + } + status = psa_load_se_persistent_data( &driver_table[i] ); + if( status != PSA_SUCCESS ) + goto error; + } + driver_table[i].internal.persistent_data_size = + methods->persistent_data_size; + return( PSA_SUCCESS ); + +error: + memset( &driver_table[i], 0, sizeof( driver_table[i] ) ); + return( status ); } void psa_unregister_all_se_drivers( void ) { + size_t i; + for( i = 0; i < PSA_MAX_SE_DRIVERS; i++ ) + { + if( driver_table[i].internal.persistent_data != NULL ) + mbedtls_free( driver_table[i].internal.persistent_data ); + } memset( driver_table, 0, sizeof( driver_table ) ); } diff --git a/library/psa_crypto_se.h b/library/psa_crypto_se.h index 88b0127c68..a9951e6617 100644 --- a/library/psa_crypto_se.h +++ b/library/psa_crypto_se.h @@ -45,11 +45,30 @@ void psa_unregister_all_se_drivers( void ); /** A structure that describes a registered secure element driver. * * A secure element driver table entry contains a pointer to the - * driver's method table and a pointer to the driver's slot usage - * structure. + * driver's method table as well as the driver context structure. */ typedef struct psa_se_drv_table_entry_s psa_se_drv_table_entry_t; +/** Return the secure element driver information for a lifetime value. + * + * \param lifetime The lifetime value to query. + * \param[out] p_methods On output, if there is a driver, + * \c *methods points to its method table. + * Otherwise \c *methods is \c NULL. + * \param[out] p_drv_context On output, if there is a driver, + * \c *drv_context points to its context + * structure. + * Otherwise \c *drv_context is \c NULL. + * + * \retval 1 + * \p lifetime corresponds to a registered driver. + * \retval 0 + * \p lifetime does not correspond to a registered driver. + */ +int psa_get_se_driver( psa_key_lifetime_t lifetime, + const psa_drv_se_t **p_methods, + psa_drv_se_context_t **p_drv_context); + /** Return the secure element driver table entry for a lifetime value. * * \param lifetime The lifetime value to query. @@ -57,27 +76,43 @@ typedef struct psa_se_drv_table_entry_s psa_se_drv_table_entry_t; * \return The driver table entry for \p lifetime, or * \p NULL if \p lifetime does not correspond to a registered driver. */ -const psa_se_drv_table_entry_t *psa_get_se_driver_entry( +psa_se_drv_table_entry_t *psa_get_se_driver_entry( psa_key_lifetime_t lifetime ); /** Return the method table for a secure element driver. * - * \param[in] drv The driver table entry to access. + * \param[in] driver The driver table entry to access, or \c NULL. * - * \return The driver table entry for \p lifetime, or - * \p NULL if \p lifetime does not correspond to a registered driver. + * \return The driver's method table. + * \c NULL if \p driver is \c NULL. */ const psa_drv_se_t *psa_get_se_driver_methods( - const psa_se_drv_table_entry_t *drv ); + const psa_se_drv_table_entry_t *driver ); -/** Return the secure element driver method table for a lifetime value. +/** Return the context of a secure element driver. * - * \param lifetime The lifetime value to query. + * \param[in] driver The driver table entry to access, or \c NULL. * - * \return The driver method table for \p lifetime, or - * \p NULL if \p lifetime does not correspond to a registered driver. + * \return A pointer to the driver context. + * \c NULL if \p driver is \c NULL. */ -const psa_drv_se_t *psa_get_se_driver( - psa_key_lifetime_t lifetime ); +psa_drv_se_context_t *psa_get_se_driver_context( + psa_se_drv_table_entry_t *driver ); + +/** Load the persistent data of a secure element driver. + * + * \param driver The driver table entry containing the persistent + * data to load from storage. + */ +psa_status_t psa_load_se_persistent_data( + const psa_se_drv_table_entry_t *driver ); + +/** Save the persistent data of a secure element driver. + * + * \param[in] driver The driver table entry containing the persistent + * data to save to storage. + */ +psa_status_t psa_save_se_persistent_data( + const psa_se_drv_table_entry_t *driver ); #endif /* PSA_CRYPTO_SE_H */ From 8abe6a2d5ca502f0f451d29f45563fc0235aae90 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:40:35 +0200 Subject: [PATCH 11/50] Driver table entries are now mutable Since driver table entries contain the driver context, which is mutable, they can't be const anymore. --- library/psa_crypto.c | 14 +++++++------- library/psa_crypto_slot_management.c | 2 +- library/psa_crypto_slot_management.h | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 65e1e2a74e..db6a11fb24 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1306,7 +1306,7 @@ static psa_status_t psa_start_key_creation( const psa_key_attributes_t *attributes, psa_key_handle_t *handle, psa_key_slot_t **p_slot, - const psa_se_drv_table_entry_t **p_drv ) + psa_se_drv_table_entry_t **p_drv ) { psa_status_t status; psa_key_slot_t *slot; @@ -1356,7 +1356,7 @@ static psa_status_t psa_start_key_creation( */ static psa_status_t psa_finish_key_creation( psa_key_slot_t *slot, - const psa_se_drv_table_entry_t *driver ) + psa_se_drv_table_entry_t *driver ) { psa_status_t status = PSA_SUCCESS; (void) slot; @@ -1407,7 +1407,7 @@ static psa_status_t psa_finish_key_creation( * or NULL for a transparent key. */ static void psa_fail_key_creation( psa_key_slot_t *slot, - const psa_se_drv_table_entry_t *driver ) + psa_se_drv_table_entry_t *driver ) { (void) driver; @@ -1483,7 +1483,7 @@ psa_status_t psa_import_key( const psa_key_attributes_t *attributes, { psa_status_t status; psa_key_slot_t *slot = NULL; - const psa_se_drv_table_entry_t *driver = NULL; + psa_se_drv_table_entry_t *driver = NULL; status = psa_start_key_creation( attributes, handle, &slot, &driver ); if( status != PSA_SUCCESS ) @@ -1540,7 +1540,7 @@ psa_status_t psa_copy_key( psa_key_handle_t source_handle, psa_key_slot_t *source_slot = NULL; psa_key_slot_t *target_slot = NULL; psa_key_attributes_t actual_attributes = *specified_attributes; - const psa_se_drv_table_entry_t *driver = NULL; + psa_se_drv_table_entry_t *driver = NULL; status = psa_get_key_from_slot( source_handle, &source_slot, PSA_KEY_USAGE_COPY, 0 ); @@ -4464,7 +4464,7 @@ psa_status_t psa_key_derivation_output_key( const psa_key_attributes_t *attribut { psa_status_t status; psa_key_slot_t *slot = NULL; - const psa_se_drv_table_entry_t *driver = NULL; + psa_se_drv_table_entry_t *driver = NULL; status = psa_start_key_creation( attributes, handle, &slot, &driver ); if( status == PSA_SUCCESS ) { @@ -5495,7 +5495,7 @@ psa_status_t psa_generate_key( const psa_key_attributes_t *attributes, { psa_status_t status; psa_key_slot_t *slot = NULL; - const psa_se_drv_table_entry_t *driver = NULL; + psa_se_drv_table_entry_t *driver = NULL; status = psa_start_key_creation( attributes, handle, &slot, &driver ); if( status == PSA_SUCCESS ) { diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index eb24b6b4ca..40e9683e53 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -168,7 +168,7 @@ static int psa_is_key_id_valid( psa_key_file_id_t file_id, psa_status_t psa_validate_persistent_key_parameters( psa_key_lifetime_t lifetime, psa_key_file_id_t id, - const psa_se_drv_table_entry_t **p_drv, + psa_se_drv_table_entry_t **p_drv, int creating ) { if( p_drv != NULL ) diff --git a/library/psa_crypto_slot_management.h b/library/psa_crypto_slot_management.h index 8111c4a62b..049520d4b0 100644 --- a/library/psa_crypto_slot_management.h +++ b/library/psa_crypto_slot_management.h @@ -122,7 +122,7 @@ static inline int psa_key_lifetime_is_external( psa_key_lifetime_t lifetime ) psa_status_t psa_validate_persistent_key_parameters( psa_key_lifetime_t lifetime, psa_key_file_id_t id, - const psa_se_drv_table_entry_t **p_drv, + psa_se_drv_table_entry_t **p_drv, int creating ); From 73167e128f44727333502c5709e5aad2d554fdbc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:44:37 +0200 Subject: [PATCH 12/50] SE keys: store the slot number in the memory slot --- library/psa_crypto.c | 14 ++++++++++++++ library/psa_crypto_core.h | 9 +++++++++ 2 files changed, 23 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index db6a11fb24..84b10df3e1 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -363,6 +363,13 @@ static psa_status_t mbedtls_to_psa_error( int ret ) /* Key management */ /****************************************************************/ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +static inline int psa_key_slot_is_external( const psa_key_slot_t *slot ) +{ + return( psa_key_lifetime_is_external( slot->lifetime ) ); +} +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + #if defined(MBEDTLS_ECP_C) static psa_ecc_curve_t mbedtls_ecc_group_to_psa( mbedtls_ecp_group_id grpid ) { @@ -867,6 +874,13 @@ static psa_status_t psa_get_key_from_slot( psa_key_handle_t handle, /** Wipe key data from a slot. Preserve metadata such as the policy. */ static psa_status_t psa_remove_key_data_from_memory( psa_key_slot_t *slot ) { +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( psa_key_slot_is_external( slot ) ) + { + /* No key material to clean. */ + } + else +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ if( slot->type == PSA_KEY_TYPE_NONE ) { /* No key material to clean. */ diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index 5958972572..6096810f40 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -29,6 +29,7 @@ #endif #include "psa/crypto.h" +#include "psa/crypto_se_driver.h" #include "mbedtls/ecp.h" #include "mbedtls/rsa.h" @@ -45,17 +46,25 @@ typedef struct unsigned allocated : 1; union { + /* Raw-data key (key_type_is_raw_bytes() in psa_crypto.c) */ struct raw_data { uint8_t *data; size_t bytes; } raw; #if defined(MBEDTLS_RSA_C) + /* RSA public key or key pair */ mbedtls_rsa_context *rsa; #endif /* MBEDTLS_RSA_C */ #if defined(MBEDTLS_ECP_C) + /* EC public key or key pair */ mbedtls_ecp_keypair *ecp; #endif /* MBEDTLS_ECP_C */ + /* Any key type in a secure element */ + struct se + { + psa_key_slot_number_t slot_number; + } se; } data; } psa_key_slot_t; From cbaff467efd1f81cc09dd81ae10c48e872f32360 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:46:04 +0200 Subject: [PATCH 13/50] SE keys: allocate a slot before creating the key --- library/psa_crypto.c | 24 ++++++++++++++++++++++++ library/psa_crypto_se.c | 29 +++++++++++++++++++++++++++++ library/psa_crypto_se.h | 15 +++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 84b10df3e1..93c9ce4448 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1348,6 +1348,18 @@ static psa_status_t psa_start_key_creation( } slot->type = attributes->type; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + /* Find a slot number. Don't yet mark it as allocated in case + * the key creation fails or there is a power failure. */ + if( *p_drv != NULL ) + { + status = psa_find_se_slot_for_key( attributes, *p_drv, + &slot->data.se.slot_number ); + if( status != PSA_SUCCESS ) + return( status ); + } +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + return( status ); } @@ -1405,6 +1417,18 @@ static psa_status_t psa_finish_key_creation( } #endif /* defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( driver != NULL ) + { + status = psa_save_se_persistent_data( driver ); + if( status != PSA_SUCCESS ) + { + psa_destroy_persistent_key( slot->persistent_storage_id ); + return( status ); + } + } +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + return( status ); } diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index b95b2a5d5f..fb57fc9625 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -130,6 +130,35 @@ psa_status_t psa_save_se_persistent_data( return( PSA_SUCCESS ); } +psa_status_t psa_find_se_slot_for_key( + const psa_key_attributes_t *attributes, + psa_se_drv_table_entry_t *driver, + 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 != attributes->lifetime ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + + /* 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 ); + return( status ); +} + /****************************************************************/ diff --git a/library/psa_crypto_se.h b/library/psa_crypto_se.h index a9951e6617..02819d9b3a 100644 --- a/library/psa_crypto_se.h +++ b/library/psa_crypto_se.h @@ -99,6 +99,21 @@ const psa_drv_se_t *psa_get_se_driver_methods( psa_drv_se_context_t *psa_get_se_driver_context( psa_se_drv_table_entry_t *driver ); +/** Find a free slot for a key that is to be created. + * + * This function calls the relevant method in the driver to find a suitable + * slot for a key with the given attributes. + * + * \param[in] attributes Metadata about the key that is about to be created. + * \param[in] driver The driver table entry to query. + * \param[out] slot_number On success, a slot number that is free in this + * secure element. + */ +psa_status_t psa_find_se_slot_for_key( + const psa_key_attributes_t *attributes, + psa_se_drv_table_entry_t *driver, + psa_key_slot_number_t *slot_number ); + /** Load the persistent data of a secure element driver. * * \param driver The driver table entry containing the persistent From 354f7671f48945ffa9e68e0a4564e7f16279a152 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:46:38 +0200 Subject: [PATCH 14/50] SE keys: support destroy When destroying a key in a secure element, call the driver's destroy method and update the driver's persistent data in storage. --- library/psa_crypto.c | 11 +++++++++++ library/psa_crypto_se.c | 16 ++++++++++++++++ library/psa_crypto_se.h | 8 ++++++++ 3 files changed, 35 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 93c9ce4448..70ef9be0de 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -939,10 +939,20 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle ) psa_key_slot_t *slot; psa_status_t status = PSA_SUCCESS; psa_status_t storage_status = PSA_SUCCESS; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + psa_se_drv_table_entry_t *driver; +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ status = psa_get_key_slot( handle, &slot ); if( status != PSA_SUCCESS ) return( status ); + +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + driver = psa_get_se_driver_entry( slot->lifetime ); + if( driver != NULL ) + status = psa_destroy_se_key( driver, slot->data.se.slot_number ); +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) if( slot->lifetime == PSA_KEY_LIFETIME_PERSISTENT ) { @@ -950,6 +960,7 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle ) psa_destroy_persistent_key( slot->persistent_storage_id ); } #endif /* defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */ + status = psa_wipe_key_slot( slot ); if( status != PSA_SUCCESS ) return( status ); diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index fb57fc9625..7287ac0d76 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -159,6 +159,22 @@ psa_status_t psa_find_se_slot_for_key( return( status ); } +psa_status_t psa_destroy_se_key( psa_se_drv_table_entry_t *driver, + psa_key_slot_number_t slot_number ) +{ + psa_status_t status; + psa_status_t storage_status; + if( driver->methods->key_management == NULL || + driver->methods->key_management->p_destroy == NULL ) + return( PSA_ERROR_NOT_PERMITTED ); + status = driver->methods->key_management->p_destroy( + &driver->context, + driver->internal.persistent_data, + slot_number ); + storage_status = psa_save_se_persistent_data( driver ); + return( status == PSA_SUCCESS ? storage_status : status ); +} + /****************************************************************/ diff --git a/library/psa_crypto_se.h b/library/psa_crypto_se.h index 02819d9b3a..f1d7e7c367 100644 --- a/library/psa_crypto_se.h +++ b/library/psa_crypto_se.h @@ -114,6 +114,14 @@ psa_status_t psa_find_se_slot_for_key( psa_se_drv_table_entry_t *driver, psa_key_slot_number_t *slot_number ); +/** Destoy a key in a secure element. + * + * This function calls the relevant driver method to destroy a key + * and updates the driver's persistent data. + */ +psa_status_t psa_destroy_se_key( psa_se_drv_table_entry_t *driver, + psa_key_slot_number_t slot_number ); + /** Load the persistent data of a secure element driver. * * \param driver The driver table entry containing the persistent From 5d309672af3a0ceb873a8c641682ff2948edf9fa Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:47:28 +0200 Subject: [PATCH 15/50] SE keys: support import and export --- library/psa_crypto.c | 55 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 70ef9be0de..77acf2edd3 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1133,11 +1133,33 @@ static psa_status_t psa_internal_export_key( const psa_key_slot_t *slot, size_t *data_length, int export_public_key ) { +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + const psa_drv_se_t *drv; + psa_drv_se_context_t *drv_context; +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + *data_length = 0; if( export_public_key && ! PSA_KEY_TYPE_IS_ASYMMETRIC( slot->type ) ) return( PSA_ERROR_INVALID_ARGUMENT ); +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( psa_get_se_driver( slot->lifetime, &drv, &drv_context ) ) + { + psa_drv_se_export_key_t method; + if( drv->key_management == NULL ) + return( PSA_ERROR_NOT_SUPPORTED ); + method = ( export_public_key ? + drv->key_management->p_export_public : + drv->key_management->p_export ); + if( method == NULL ) + return( PSA_ERROR_NOT_SUPPORTED ); + return( ( *method )( drv_context, + slot->data.se.slot_number, + data, data_size, data_length ) ); + } +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + if( key_type_is_raw_bytes( slot->type ) ) { if( slot->data.raw.bytes > data_size ) @@ -1538,12 +1560,33 @@ psa_status_t psa_import_key( const psa_key_attributes_t *attributes, if( status != PSA_SUCCESS ) goto exit; - status = psa_import_key_into_slot( slot, data, data_length ); - if( status != PSA_SUCCESS ) - goto exit; - status = psa_check_key_slot_attributes( slot, attributes ); - if( status != PSA_SUCCESS ) - goto exit; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( driver != NULL ) + { + const psa_drv_se_t *drv = psa_get_se_driver_methods( driver ); + if( drv->key_management == NULL || + drv->key_management->p_import == NULL ) + { + status = PSA_ERROR_NOT_SUPPORTED; + goto exit; + } + status = drv->key_management->p_import( + psa_get_se_driver_context( driver ), + slot->data.se.slot_number, + slot->lifetime, slot->type, slot->policy.alg, slot->policy.usage, + data, data_length ); + /* TOnogrepDO: psa_check_key_slot_attributes? */ + } + else +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + { + status = psa_import_key_into_slot( slot, data, data_length ); + if( status != PSA_SUCCESS ) + goto exit; + status = psa_check_key_slot_attributes( slot, attributes ); + if( status != PSA_SUCCESS ) + goto exit; + } status = psa_finish_key_creation( slot, driver ); exit: From 5dc742c36a8c5487e602ebe0a9632215b8d933e8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 12 Jul 2019 23:47:47 +0200 Subject: [PATCH 16/50] SE keys: smoke test import, export, destroy --- .../test_suite_psa_crypto_se_driver_hal.data | 6 + ...st_suite_psa_crypto_se_driver_hal.function | 166 +++++++++++++++++- 2 files changed, 171 insertions(+), 1 deletion(-) 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 c04b70d962..28c7d7583f 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -26,3 +26,9 @@ register_twice:3 Register SE driver: maximum number of drivers register_max: + +Key creation smoke test (p_allocate allows all slots) +key_creation_import_export:0 + +Key creation smoke test (p_allocate allows 1 slot) +key_creation_import_export:ARRAY_LENGTH( ram_slots ) - 1 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 b9d0a1f0a8..2e2a6480f0 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -4,9 +4,117 @@ #include "psa_crypto_se.h" -/* The minimum valid lifetime value for a secure element driver. */ +/** The minimum valid lifetime value for a secure element driver. */ #define MIN_DRIVER_LIFETIME 2 +/** The driver detected a condition that shouldn't happen. + * This is probably a bug in the library. */ +#define PSA_ERROR_DETECTED_BY_DRIVER ((psa_status_t)( -500 )) + +/** Like #TEST_ASSERT for use in a driver method. + * + * Use this macro to assert on guarantees provided by the core. + */ +#define DRIVER_ASSERT( TEST ) \ + do { \ + if( ! (TEST) ) \ + { \ + test_fail( #TEST, __LINE__, __FILE__ ); \ + return( PSA_ERROR_DETECTED_BY_DRIVER ); \ + } \ + } while( 0 ) + +#define RAM_MAX_KEY_SIZE 64 +typedef struct +{ + psa_key_lifetime_t lifetime; + psa_key_type_t type; + size_t bits; + uint8_t content[RAM_MAX_KEY_SIZE]; +} ram_slot_t; +static ram_slot_t ram_slots[16]; + +/* A type with at least ARRAY_LENGTH(ram_slots) bits, containing a + * bit vector indicating which slots are in use. */ +typedef uint16_t ram_slot_usage_t; + +static uint8_t ram_min_slot = 0; + +static void ram_slots_reset( void ) +{ + memset( ram_slots, 0, sizeof( ram_slots ) ); + ram_min_slot = 0; +} + +static psa_status_t ram_import( psa_drv_se_context_t *context, + psa_key_slot_number_t slot_number, + psa_key_lifetime_t lifetime, + psa_key_type_t type, + psa_algorithm_t algorithm, + psa_key_usage_t usage, + const uint8_t *p_data, + size_t data_length ) +{ + (void) context; + DRIVER_ASSERT( slot_number < ARRAY_LENGTH( ram_slots ) ); + if( data_length > sizeof( ram_slots[slot_number].content ) ) + return( PSA_ERROR_INSUFFICIENT_STORAGE ); + ram_slots[slot_number].lifetime = lifetime; + ram_slots[slot_number].type = type; + ram_slots[slot_number].bits = PSA_BYTES_TO_BITS( data_length ); + (void) algorithm; + (void) usage; + memcpy( ram_slots[slot_number].content, p_data, data_length ); + return( PSA_SUCCESS ); +} + +psa_status_t ram_export( psa_drv_se_context_t *context, + psa_key_slot_number_t slot_number, + uint8_t *p_data, + size_t data_size, + size_t *p_data_length ) +{ + size_t actual_size; + (void) context; + DRIVER_ASSERT( slot_number < ARRAY_LENGTH( ram_slots ) ); + actual_size = PSA_BITS_TO_BYTES( ram_slots[slot_number].bits ); + if( actual_size > data_size ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + *p_data_length = actual_size; + memcpy( p_data, ram_slots[slot_number].content, actual_size ); + return( PSA_SUCCESS ); +} + +psa_status_t ram_destroy( psa_drv_se_context_t *context, + void *persistent_data, + psa_key_slot_number_t slot_number ) +{ + ram_slot_usage_t *slot_usage = persistent_data; + DRIVER_ASSERT( context->persistent_data_size == sizeof( ram_slot_usage_t ) ); + DRIVER_ASSERT( slot_number < ARRAY_LENGTH( ram_slots ) ); + memset( &ram_slots[slot_number], 0, sizeof( ram_slots[slot_number] ) ); + *slot_usage &= ~(ram_slot_usage_t)( 1 << slot_number ); + return( PSA_SUCCESS ); +} + +psa_status_t ram_allocate( psa_drv_se_context_t *context, + void *persistent_data, + const psa_key_attributes_t *attributes, + psa_key_slot_number_t *slot_number ) +{ + ram_slot_usage_t *slot_usage = persistent_data; + (void) attributes; + DRIVER_ASSERT( context->persistent_data_size == sizeof( ram_slot_usage_t ) ); + for( *slot_number = ram_min_slot; + *slot_number < ARRAY_LENGTH( ram_slots ); + ++( *slot_number ) ) + { + if( ! ( *slot_usage & 1 << *slot_number ) ) + return( PSA_SUCCESS ); + } + return( PSA_ERROR_INSUFFICIENT_STORAGE ); +} + /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -78,3 +186,59 @@ exit: PSA_DONE( ); } /* END_CASE */ + +/* BEGIN_CASE */ +void key_creation_import_export( int min_slot ) +{ + 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}; + uint8_t exported[sizeof( key_material )]; + size_t exported_length; + + 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_allocate = ram_allocate; + key_management.p_import = ram_import; + key_management.p_destroy = ram_destroy; + key_management.p_export = ram_export; + ram_min_slot = min_slot; + + 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_ASSERT( psa_import_key( &attributes, + key_material, sizeof( key_material ), + &handle ) ); + + /* Test that the key was created in the expected slot. */ + TEST_ASSERT( ram_slots[min_slot].type == PSA_KEY_TYPE_RAW_DATA ); + + PSA_ASSERT( psa_export_key( handle, + exported, sizeof( exported ), + &exported_length ) ); + ASSERT_COMPARE( key_material, sizeof( key_material ), + exported, exported_length ); + + PSA_ASSERT( psa_destroy_key( handle ) ); + + /* Test that the key has been erased from the designated slot. */ + TEST_ASSERT( ram_slots[min_slot].type == 0 ); + +exit: + PSA_DONE( ); + ram_slots_reset( ); +} +/* END_CASE */ From c8336cb8f95f96740d5cfdc5b90758624550c08d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 22 Jul 2019 19:26:12 +0200 Subject: [PATCH 17/50] Implement a transaction record storage for resilience Implement a transaction record that can be used for actions that modify more than one piece of persistent data (whether in the persistent storage or elsewhere such as in a secure element). While performing a transaction, the transaction file is present in storage. If the system starts with an ongoing transaction, it must complete the transaction (not implemented yet). --- library/psa_crypto_storage.c | 66 +++++++++++++++++ library/psa_crypto_storage.h | 133 +++++++++++++++++++++++++++++++++++ 2 files changed, 199 insertions(+) diff --git a/library/psa_crypto_storage.c b/library/psa_crypto_storage.c index cd36bb9101..d07bdc580a 100644 --- a/library/psa_crypto_storage.c +++ b/library/psa_crypto_storage.c @@ -50,6 +50,12 @@ #define mbedtls_free free #endif + + +/****************************************************************/ +/* Key storage */ +/****************************************************************/ + /* Determine a file name (ITS file identifier) for the given key file * identifier. The file name must be distinct from any file that is used * for a purpose other than storing a key. Currently, the only such file @@ -399,6 +405,60 @@ exit: return( status ); } + + +/****************************************************************/ +/* Transactions */ +/****************************************************************/ + +#if defined(PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS) + +psa_crypto_transaction_t psa_crypto_transaction; + +psa_status_t psa_crypto_save_transaction( void ) +{ + struct psa_storage_info_t p_info; + psa_status_t status; + status = psa_its_get_info( PSA_CRYPTO_ITS_RANDOM_SEED_UID, &p_info ); + if( status == PSA_SUCCESS ) + { + /* This shouldn't happen: we're trying to start a transaction while + * there is still a transaction that hasn't been replayed. */ + return( PSA_ERROR_CORRUPTION_DETECTED ); + } + else if( status != PSA_ERROR_DOES_NOT_EXIST ) + return( status ); + return( psa_its_set( PSA_CRYPTO_ITS_TRANSACTION_UID, + sizeof( psa_crypto_transaction ), + &psa_crypto_transaction, + 0 ) ); +} + +psa_status_t psa_crypto_load_transaction( void ) +{ + return( psa_its_get( PSA_CRYPTO_ITS_TRANSACTION_UID, 0, + sizeof( psa_crypto_transaction ), + &psa_crypto_transaction ) ); +} + +psa_status_t psa_crypto_stop_transaction( void ) +{ + psa_status_t status = psa_its_remove( PSA_CRYPTO_ITS_TRANSACTION_UID ); + /* Whether or not updating the storage succeeded, the transaction is + * finished now. It's too late to go back, so zero out the in-memory + * data. */ + memset( &psa_crypto_transaction, 0, sizeof( psa_crypto_transaction ) ); + return( status ); +} + +#endif /* PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS */ + + + +/****************************************************************/ +/* Random generator state */ +/****************************************************************/ + #if defined(MBEDTLS_PSA_INJECT_ENTROPY) psa_status_t mbedtls_psa_storage_inject_entropy( const unsigned char *seed, size_t seed_size ) @@ -421,4 +481,10 @@ psa_status_t mbedtls_psa_storage_inject_entropy( const unsigned char *seed, } #endif /* MBEDTLS_PSA_INJECT_ENTROPY */ + + +/****************************************************************/ +/* The end */ +/****************************************************************/ + #endif /* MBEDTLS_PSA_CRYPTO_STORAGE_C */ diff --git a/library/psa_crypto_storage.h b/library/psa_crypto_storage.h index 2af624a0cf..16f5d5cac4 100644 --- a/library/psa_crypto_storage.h +++ b/library/psa_crypto_storage.h @@ -39,6 +39,7 @@ extern "C" { #include "psa/crypto.h" #include +#include /* Limit the maximum key size to 30kB (just in case someone tries to * inadvertently store an obscene amount of data) */ @@ -203,6 +204,138 @@ psa_status_t psa_parse_key_data_from_storage( const uint8_t *storage_data, psa_key_type_t *type, psa_key_policy_t *policy ); +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +/** This symbol is defined if transaction support is required. */ +#define PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS +#endif + +#if defined(PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS) + +/** The type of transaction that is in progress. + */ +/* This is an integer type rather than an enum for two reasons: to support + * unknown values when loading a transaction file, and to ensure that the + * type has a known size. + */ +typedef uint16_t psa_crypto_transaction_type_t; + +/** No transaction is in progress. + */ +#define PSA_CRYPTO_TRANSACTION_NONE ( (psa_crypto_transaction_type_t) 0x0000 ) + +/** Transaction data. + * + * This type is designed to be serialized by writing the memory representation + * and reading it back on the same device. + * + * \note The transaction mechanism is designed for a single active transaction + * at a time. The transaction object is #psa_crypto_transaction. + * + * \note If an API call starts a transaction, it must complete this transaction + * before returning to the application. + * + * The lifetime of a transaction is the following (note that only one + * transaction may be active at a time): + * + * -# Call psa_crypto_prepare_transaction() to initialize the transaction + * object in memory and declare the type of transaction that is starting. + * -# Fill in the type-specific fields of #psa_crypto_transaction. + * -# Call psa_crypto_save_transaction() to start the transaction. This + * saves the transaction data to internal storage. + * -# If there are intermediate stages in the transaction, update + * the fields of #psa_crypto_transaction and call + * psa_crypto_save_transaction() again when each stage is reached. + * -# When the transaction is over, whether it has been committed or aborted, + * call psa_crypto_stop_transaction() to remove the transaction data in + * storage and in memory. + * + * If the system crashes while a transaction is in progress, psa_crypto_init() + * calls psa_crypto_load_transaction() and takes care of completing or + * rewinding the transaction. + */ +typedef union +{ + /* Each element of this union must have the following properties + * to facilitate serialization and deserialization: + * + * - The element is a struct. + * - The first field of the struct is `psa_crypto_transaction_type_t type`. + * - Elements of the struct are arranged such a way that there is + * no padding. + */ + struct psa_crypto_transaction_unknown_s + { + psa_crypto_transaction_type_t type; + } unknown; +} psa_crypto_transaction_t; + +/** The single active transaction. + */ +extern psa_crypto_transaction_t psa_crypto_transaction; + +/** Prepare for a transaction. + * + * There must not be an ongoing transaction. + * + * \param type The type of transaction to start. + */ +static inline void psa_crypto_prepare_transaction( + psa_crypto_transaction_type_t type ) +{ + psa_crypto_transaction.unknown.type = type; +} + +/** Save the transaction data to storage. + * + * You may call this function multiple times during a transaction to + * atomically update the transaction state. + * + * \retval #PSA_SUCCESS + * \retval #PSA_ERROR_INSUFFICIENT_STORAGE + * \retval #PSA_ERROR_STORAGE_FAILURE + */ +psa_status_t psa_crypto_save_transaction( void ); + +/** Load the transaction data from storage, if any. + * + * This function is meant to be called from psa_crypto_init() to recover + * in case a transaction was interrupted by a system crash. + * + * \retval #PSA_SUCCESS + * The data about the ongoing transaction has been loaded to + * #psa_crypto_transaction. + * \retval #PSA_ERROR_DOES_NOT_EXIST + * There is no ongoing transaction. + * \retval #PSA_ERROR_STORAGE_FAILURE + */ +psa_status_t psa_crypto_load_transaction( void ); + +/** Indicate that the current transaction is finished. + * + * Call this function at the very end of transaction processing, whether + * the transaction has been committed or aborted. + * + * This function erases the transaction data in storage (if any) and + * resets the transaction data in memory. + * + * \retval #PSA_SUCCESS + * There was transaction data in storage. + * \retval #PSA_ERROR_DOES_NOT_EXIST + * There was no transaction data in storage. + * \retval #PSA_ERROR_STORAGE_FAILURE + * It was impossible to determine whether there was transaction data + * in storage, or the transaction data could not be erased. + */ +psa_status_t psa_crypto_stop_transaction( void ); + +/** The ITS file identifier for the transaction data. + * + * 0xffffffNN = special file; 0x74 = 't' for transaction. + */ +#define PSA_CRYPTO_ITS_TRANSACTION_UID ( (psa_key_id_t) 0xffffff74 ) + +#endif /* PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS */ + #if defined(MBEDTLS_PSA_INJECT_ENTROPY) /** Backend side of mbedtls_psa_inject_entropy(). * From fc76265385420739fa671590a98d2bc3df07af45 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 22 Jul 2019 19:30:34 +0200 Subject: [PATCH 18/50] Do secure element key creation and destruction in a transaction Key creation and key destruction for a key in a secure element both require updating three pieces of data: the key data in the secure element, the key metadata in internal storage, and the SE driver's persistent data. Perform these actions in a transaction so that recovery is possible if the action is interrupted midway. --- library/psa_crypto.c | 59 ++++++++++++++++++++++++++++++++++-- library/psa_crypto_storage.h | 40 +++++++++++++++++++----- 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 77acf2edd3..c482747b75 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -950,7 +950,20 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle ) #if defined(MBEDTLS_PSA_CRYPTO_SE_C) driver = psa_get_se_driver_entry( slot->lifetime ); if( driver != NULL ) + { + psa_crypto_prepare_transaction( PSA_CRYPTO_TRANSACTION_DESTROY_KEY ); + psa_crypto_transaction.key.lifetime = slot->lifetime; + psa_crypto_transaction.key.slot = slot->data.se.slot_number; + psa_crypto_transaction.key.id = slot->persistent_storage_id; + status = psa_crypto_save_transaction( ); + if( status != PSA_SUCCESS ) + { + /* TOnogrepDO: destroy what can be destroyed anyway */ + return( status ); + } + status = psa_destroy_se_key( driver, slot->data.se.slot_number ); + } #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) @@ -961,6 +974,18 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle ) } #endif /* defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( driver != NULL ) + { + status = psa_crypto_stop_transaction( ); + if( status != PSA_SUCCESS ) + { + /* TOnogrepDO: destroy what can be destroyed anyway */ + return( status ); + } + } +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + status = psa_wipe_key_slot( slot ); if( status != PSA_SUCCESS ) return( status ); @@ -1382,8 +1407,10 @@ static psa_status_t psa_start_key_creation( slot->type = attributes->type; #if defined(MBEDTLS_PSA_CRYPTO_SE_C) - /* Find a slot number. Don't yet mark it as allocated in case - * the key creation fails or there is a power failure. */ + /* Find a slot number for the new key. Save the slot number in + * persistent storage, but do not yet save the driver's persistent + * state, so that if the power fails during the key creation process, + * we can roll back to a state where the key doesn't exist. */ if( *p_drv != NULL ) { status = psa_find_se_slot_for_key( attributes, *p_drv, @@ -1391,6 +1418,13 @@ static psa_status_t psa_start_key_creation( if( status != PSA_SUCCESS ) return( status ); } + psa_crypto_prepare_transaction( PSA_CRYPTO_TRANSACTION_CREATE_KEY ); + psa_crypto_transaction.key.lifetime = slot->lifetime; + psa_crypto_transaction.key.slot = slot->data.se.slot_number; + psa_crypto_transaction.key.id = slot->persistent_storage_id; + status = psa_crypto_save_transaction( ); + if( status != PSA_SUCCESS ) + return( status ); #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ return( status ); @@ -1459,6 +1493,9 @@ static psa_status_t psa_finish_key_creation( psa_destroy_persistent_key( slot->persistent_storage_id ); return( status ); } + status = psa_crypto_stop_transaction( ); + if( status != PSA_SUCCESS ) + return( status ); } #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ @@ -1490,6 +1527,11 @@ static void psa_fail_key_creation( psa_key_slot_t *slot, * element, and the failure happened later (when saving metadata * to internal storage), we need to destroy the key in the secure * element. */ + + /* Abort the ongoing transaction if any. We already did what it + * takes to undo any partial creation. All that's left is to update + * the transaction data itself. */ + (void) psa_crypto_stop_transaction( ); #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ psa_wipe_key_slot( slot ); @@ -5674,6 +5716,19 @@ psa_status_t psa_crypto_init( void ) if( status != PSA_SUCCESS ) goto exit; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + status = psa_crypto_load_transaction( ); + if( status == PSA_SUCCESS ) + { + /*TOnogrepDO: complete or abort the transaction*/ + } + else if( status == PSA_ERROR_DOES_NOT_EXIST ) + { + /* There's no transaction to complete. It's all good. */ + status = PSA_SUCCESS; + } +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + /* All done. */ global_data.initialized = 1; diff --git a/library/psa_crypto_storage.h b/library/psa_crypto_storage.h index 16f5d5cac4..2e4079f7db 100644 --- a/library/psa_crypto_storage.h +++ b/library/psa_crypto_storage.h @@ -29,15 +29,9 @@ extern "C" { #endif -/* Include the Mbed TLS configuration file, the way Mbed TLS does it - * in each of its header files. */ -#if defined(MBEDTLS_CONFIG_FILE) -#include MBEDTLS_CONFIG_FILE -#else -#include "mbedtls/config.h" -#endif - #include "psa/crypto.h" +#include "psa/crypto_se_driver.h" + #include #include @@ -223,6 +217,22 @@ typedef uint16_t psa_crypto_transaction_type_t; */ #define PSA_CRYPTO_TRANSACTION_NONE ( (psa_crypto_transaction_type_t) 0x0000 ) +/** A key creation transaction. + * + * This is only used for keys in an external cryptoprocessor (secure element). + * Keys in RAM or in internal storage are created atomically in storage + * (simple file creation), so they do not need a transaction mechanism. + */ +#define PSA_CRYPTO_TRANSACTION_CREATE_KEY ( (psa_crypto_transaction_type_t) 0x0001 ) + +/** A key destruction transaction. + * + * This is only used for keys in an external cryptoprocessor (secure element). + * Keys in RAM or in internal storage are destroyed atomically in storage + * (simple file deletion), so they do not need a transaction mechanism. + */ +#define PSA_CRYPTO_TRANSACTION_DESTROY_KEY ( (psa_crypto_transaction_type_t) 0x0002 ) + /** Transaction data. * * This type is designed to be serialized by writing the memory representation @@ -266,7 +276,21 @@ typedef union struct psa_crypto_transaction_unknown_s { psa_crypto_transaction_type_t type; + uint16_t unused1; + uint32_t unused2; + uint64_t unused3; + uint64_t unused4; } unknown; + /* ::type is #PSA_CRYPTO_TRANSACTION_CREATE_KEY or + * #PSA_CRYPTO_TRANSACTION_DESTROY_KEY. */ + struct psa_crypto_transaction_key_s + { + psa_crypto_transaction_type_t type; + uint16_t unused1; + psa_key_lifetime_t lifetime; + psa_key_slot_number_t slot; + psa_key_id_t id; + } key; } psa_crypto_transaction_t; /** The single active transaction. From 6032673b399746b356979d960bf30ccbb446881f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 22 Jul 2019 20:10:36 +0200 Subject: [PATCH 19/50] Fix Doxygen reference Pass doxygen.sh --- include/psa/crypto_se_driver.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index bdc038e880..e7fe00671d 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -65,7 +65,7 @@ typedef struct { * * The size of this buffer is given by psa_drv_se_t::persistent_data_size * when the driver is registered, and this value is also recorded in the - * ::persistent_data_size field of this structure. + * psa_drv_se_context_t::persistent_data_size field of this structure. * * Before the driver is initialized for the first time, the content of * the persistent data is all-bits-zero. After a driver upgrade, if the From 274a2637f21e13cf0d7075c241dd4a53ff0ad2ea Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Jul 2019 11:27:38 +0200 Subject: [PATCH 20/50] Make whitespace consistent --- library/psa_crypto_storage.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/library/psa_crypto_storage.c b/library/psa_crypto_storage.c index d07bdc580a..97b2481d49 100644 --- a/library/psa_crypto_storage.c +++ b/library/psa_crypto_storage.c @@ -227,7 +227,7 @@ static psa_status_t psa_crypto_storage_get_data_length( * 32-bit integer manipulation macros (little endian) */ #ifndef GET_UINT32_LE -#define GET_UINT32_LE(n,b,i) \ +#define GET_UINT32_LE( n, b, i ) \ { \ (n) = ( (uint32_t) (b)[(i) ] ) \ | ( (uint32_t) (b)[(i) + 1] << 8 ) \ @@ -237,7 +237,7 @@ static psa_status_t psa_crypto_storage_get_data_length( #endif #ifndef PUT_UINT32_LE -#define PUT_UINT32_LE(n,b,i) \ +#define PUT_UINT32_LE( n, b, i ) \ { \ (b)[(i) ] = (unsigned char) ( ( (n) ) & 0xFF ); \ (b)[(i) + 1] = (unsigned char) ( ( (n) >> 8 ) & 0xFF ); \ @@ -271,12 +271,12 @@ void psa_format_key_data_for_storage( const uint8_t *data, (psa_persistent_key_storage_format *) storage_data; memcpy( storage_format->magic, PSA_KEY_STORAGE_MAGIC_HEADER, PSA_KEY_STORAGE_MAGIC_HEADER_LENGTH ); - PUT_UINT32_LE(0, storage_format->version, 0); - PUT_UINT32_LE(type, storage_format->type, 0); - PUT_UINT32_LE(policy->usage, storage_format->policy, 0); - PUT_UINT32_LE(policy->alg, storage_format->policy, sizeof( uint32_t )); - PUT_UINT32_LE(policy->alg2, storage_format->policy, 2 * sizeof( uint32_t ) ); - PUT_UINT32_LE(data_length, storage_format->data_len, 0); + PUT_UINT32_LE( 0, storage_format->version, 0 ); + PUT_UINT32_LE( type, storage_format->type, 0 ); + PUT_UINT32_LE( policy->usage, storage_format->policy, 0 ); + PUT_UINT32_LE( policy->alg, storage_format->policy, sizeof( uint32_t ) ); + PUT_UINT32_LE( policy->alg2, storage_format->policy, 2 * sizeof( uint32_t ) ); + PUT_UINT32_LE( data_length, storage_format->data_len, 0 ); memcpy( storage_format->key_data, data, data_length ); } @@ -307,11 +307,11 @@ psa_status_t psa_parse_key_data_from_storage( const uint8_t *storage_data, if( status != PSA_SUCCESS ) return( status ); - GET_UINT32_LE(version, storage_format->version, 0); + GET_UINT32_LE( version, storage_format->version, 0 ); if( version != 0 ) return( PSA_ERROR_STORAGE_FAILURE ); - GET_UINT32_LE(*key_data_length, storage_format->data_len, 0); + GET_UINT32_LE( *key_data_length, storage_format->data_len, 0 ); if( *key_data_length > ( storage_data_length - sizeof(*storage_format) ) || *key_data_length > PSA_CRYPTO_MAX_STORAGE_SIZE ) return( PSA_ERROR_STORAGE_FAILURE ); @@ -328,10 +328,10 @@ psa_status_t psa_parse_key_data_from_storage( const uint8_t *storage_data, memcpy( *key_data, storage_format->key_data, *key_data_length ); } - GET_UINT32_LE(*type, storage_format->type, 0); - GET_UINT32_LE(policy->usage, storage_format->policy, 0); - GET_UINT32_LE(policy->alg, storage_format->policy, sizeof( uint32_t )); - GET_UINT32_LE(policy->alg2, storage_format->policy, 2 * sizeof( uint32_t )); + GET_UINT32_LE( *type, storage_format->type, 0 ); + GET_UINT32_LE( policy->usage, storage_format->policy, 0 ); + GET_UINT32_LE( policy->alg, storage_format->policy, sizeof( uint32_t ) ); + GET_UINT32_LE( policy->alg2, storage_format->policy, 2 * sizeof( uint32_t ) ); return( PSA_SUCCESS ); } From bfd322ff346f8aba5f7c560918cbc0dc1d307059 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Jul 2019 11:58:03 +0200 Subject: [PATCH 21/50] Use a key attribute structure in the internal storage interface Pass information via a key attribute structure rather than as separate parameters to psa_crypto_storage functions. This makes it easier to maintain the code when the metadata of a key evolves. This has negligible impact on code size (+4B with "gcc -Os" on x86_64). --- library/psa_crypto.c | 27 +++++++++--- library/psa_crypto_slot_management.c | 10 +++-- library/psa_crypto_storage.c | 37 +++++++--------- library/psa_crypto_storage.h | 44 ++++++++----------- ...t_suite_psa_crypto_persistent_key.function | 28 +++++++----- 5 files changed, 76 insertions(+), 70 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index c482747b75..e048e9f2bc 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1100,6 +1100,22 @@ exit: } #endif /* MBEDTLS_RSA_C */ +/** Retrieve the readily-accessible attributes of a key in a slot. + * + * This function does not compute attributes that are not directly + * stored in the slot, such as the bit size of a transparent key. + */ +static void psa_get_key_slot_attributes( psa_key_slot_t *slot, + psa_key_attributes_t *attributes ) +{ + attributes->id = slot->persistent_storage_id; + attributes->lifetime = slot->lifetime; + attributes->policy = slot->policy; + attributes->type = slot->type; +} + +/** Retrieve all the publicly-accessible attributes of a key. + */ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, psa_key_attributes_t *attributes ) { @@ -1112,10 +1128,7 @@ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, if( status != PSA_SUCCESS ) return( status ); - attributes->id = slot->persistent_storage_id; - attributes->lifetime = slot->lifetime; - attributes->policy = slot->policy; - attributes->type = slot->type; + psa_get_key_slot_attributes( slot, attributes ); attributes->bits = psa_get_key_slot_bits( slot ); switch( slot->type ) @@ -1473,9 +1486,9 @@ static psa_status_t psa_finish_key_creation( if( status == PSA_SUCCESS ) { - status = psa_save_persistent_key( slot->persistent_storage_id, - slot->type, &slot->policy, - buffer, length ); + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_get_key_slot_attributes( slot, &attributes ); + status = psa_save_persistent_key( &attributes, buffer, length ); } if( buffer_size != 0 ) diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 40e9683e53..5326fbd6ad 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -124,13 +124,15 @@ static psa_status_t psa_load_persistent_key_into_slot( psa_key_slot_t *p_slot ) psa_status_t status = PSA_SUCCESS; uint8_t *key_data = NULL; size_t key_data_length = 0; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - status = psa_load_persistent_key( p_slot->persistent_storage_id, - &( p_slot )->type, - &( p_slot )->policy, &key_data, - &key_data_length ); + psa_set_key_id( &attributes, p_slot->persistent_storage_id ); + status = psa_load_persistent_key( &attributes, + &key_data, &key_data_length ); if( status != PSA_SUCCESS ) goto exit; + p_slot->type = psa_get_key_type( &attributes ); + p_slot->policy = attributes.policy; status = psa_import_key_into_slot( p_slot, key_data, key_data_length ); exit: diff --git a/library/psa_crypto_storage.c b/library/psa_crypto_storage.c index 97b2481d49..a35808a61d 100644 --- a/library/psa_crypto_storage.c +++ b/library/psa_crypto_storage.c @@ -263,8 +263,7 @@ typedef struct { void psa_format_key_data_for_storage( const uint8_t *data, const size_t data_length, - const psa_key_type_t type, - const psa_key_policy_t *policy, + const psa_key_attributes_t *attributes, uint8_t *storage_data ) { psa_persistent_key_storage_format *storage_format = @@ -272,10 +271,10 @@ void psa_format_key_data_for_storage( const uint8_t *data, memcpy( storage_format->magic, PSA_KEY_STORAGE_MAGIC_HEADER, PSA_KEY_STORAGE_MAGIC_HEADER_LENGTH ); PUT_UINT32_LE( 0, storage_format->version, 0 ); - PUT_UINT32_LE( type, storage_format->type, 0 ); - PUT_UINT32_LE( policy->usage, storage_format->policy, 0 ); - PUT_UINT32_LE( policy->alg, storage_format->policy, sizeof( uint32_t ) ); - PUT_UINT32_LE( policy->alg2, storage_format->policy, 2 * sizeof( uint32_t ) ); + PUT_UINT32_LE( psa_get_key_type( attributes ), storage_format->type, 0 ); + PUT_UINT32_LE( psa_get_key_usage_flags( attributes ), storage_format->policy, 0 ); + PUT_UINT32_LE( psa_get_key_algorithm( attributes ), storage_format->policy, sizeof( uint32_t ) ); + PUT_UINT32_LE( psa_get_key_enrollment_algorithm( attributes ), storage_format->policy, 2 * sizeof( uint32_t ) ); PUT_UINT32_LE( data_length, storage_format->data_len, 0 ); memcpy( storage_format->key_data, data, data_length ); } @@ -292,8 +291,7 @@ psa_status_t psa_parse_key_data_from_storage( const uint8_t *storage_data, size_t storage_data_length, uint8_t **key_data, size_t *key_data_length, - psa_key_type_t *type, - psa_key_policy_t *policy ) + psa_key_attributes_t *attributes ) { psa_status_t status; const psa_persistent_key_storage_format *storage_format = @@ -328,17 +326,15 @@ psa_status_t psa_parse_key_data_from_storage( const uint8_t *storage_data, memcpy( *key_data, storage_format->key_data, *key_data_length ); } - GET_UINT32_LE( *type, storage_format->type, 0 ); - GET_UINT32_LE( policy->usage, storage_format->policy, 0 ); - GET_UINT32_LE( policy->alg, storage_format->policy, sizeof( uint32_t ) ); - GET_UINT32_LE( policy->alg2, storage_format->policy, 2 * sizeof( uint32_t ) ); + GET_UINT32_LE( attributes->type, storage_format->type, 0 ); + GET_UINT32_LE( attributes->policy.usage, storage_format->policy, 0 ); + GET_UINT32_LE( attributes->policy.alg, storage_format->policy, sizeof( uint32_t ) ); + GET_UINT32_LE( attributes->policy.alg2, storage_format->policy, 2 * sizeof( uint32_t ) ); return( PSA_SUCCESS ); } -psa_status_t psa_save_persistent_key( const psa_key_file_id_t key, - const psa_key_type_t type, - const psa_key_policy_t *policy, +psa_status_t psa_save_persistent_key( const psa_key_attributes_t *attributes, const uint8_t *data, const size_t data_length ) { @@ -354,10 +350,10 @@ psa_status_t psa_save_persistent_key( const psa_key_file_id_t key, if( storage_data == NULL ) return( PSA_ERROR_INSUFFICIENT_MEMORY ); - psa_format_key_data_for_storage( data, data_length, type, policy, + psa_format_key_data_for_storage( data, data_length, attributes, storage_data ); - status = psa_crypto_storage_store( key, + status = psa_crypto_storage_store( psa_get_key_id( attributes ), storage_data, storage_data_length ); mbedtls_free( storage_data ); @@ -374,15 +370,14 @@ void psa_free_persistent_key_data( uint8_t *key_data, size_t key_data_length ) mbedtls_free( key_data ); } -psa_status_t psa_load_persistent_key( psa_key_file_id_t key, - psa_key_type_t *type, - psa_key_policy_t *policy, +psa_status_t psa_load_persistent_key( psa_key_attributes_t *attributes, uint8_t **data, size_t *data_length ) { psa_status_t status = PSA_SUCCESS; uint8_t *loaded_data; size_t storage_data_length = 0; + psa_key_id_t key = psa_get_key_id( attributes ); status = psa_crypto_storage_get_data_length( key, &storage_data_length ); if( status != PSA_SUCCESS ) @@ -398,7 +393,7 @@ psa_status_t psa_load_persistent_key( psa_key_file_id_t key, goto exit; status = psa_parse_key_data_from_storage( loaded_data, storage_data_length, - data, data_length, type, policy ); + data, data_length, attributes ); exit: mbedtls_free( loaded_data ); diff --git a/library/psa_crypto_storage.h b/library/psa_crypto_storage.h index 2e4079f7db..25049b08de 100644 --- a/library/psa_crypto_storage.h +++ b/library/psa_crypto_storage.h @@ -83,12 +83,11 @@ int psa_is_key_present_in_storage( const psa_key_file_id_t key ); * already occupied non-persistent key, as well as validating the key data. * * - * \param key Persistent identifier of the key to be stored. This - * should be an unoccupied storage location. - * \param type Key type (a \c PSA_KEY_TYPE_XXX value). - * \param[in] policy The key policy to save. - * \param[in] data Buffer containing the key data. - * \param data_length The number of bytes that make up the key data. + * \param[in] attributes The attributes of the key to save. + * The key identifier field in the attributes + * determines the key's location. + * \param[in] data Buffer containing the key data. + * \param data_length The number of bytes that make up the key data. * * \retval PSA_SUCCESS * \retval PSA_ERROR_INSUFFICIENT_MEMORY @@ -96,9 +95,7 @@ int psa_is_key_present_in_storage( const psa_key_file_id_t key ); * \retval PSA_ERROR_STORAGE_FAILURE * \retval PSA_ERROR_ALREADY_EXISTS */ -psa_status_t psa_save_persistent_key( const psa_key_file_id_t key, - const psa_key_type_t type, - const psa_key_policy_t *policy, +psa_status_t psa_save_persistent_key( const psa_key_attributes_t *attributes, const uint8_t *data, const size_t data_length ); @@ -114,11 +111,11 @@ psa_status_t psa_save_persistent_key( const psa_key_file_id_t key, * this function to zeroize and free this buffer, regardless of whether this * function succeeds or fails. * - * \param key Persistent identifier of the key to be loaded. This - * should be an occupied storage location. - * \param[out] type On success, the key type (a \c PSA_KEY_TYPE_XXX - * value). - * \param[out] policy On success, the key's policy. + * \param[in,out] attributes + * On input, the key identifier field identifies + * the key to load. Other fields are ignored. + * On success, the attribute structure contains + * the key metadata that was loaded from storage. * \param[out] data Pointer to an allocated key data buffer on return. * \param[out] data_length The number of bytes that make up the key data. * @@ -127,9 +124,7 @@ psa_status_t psa_save_persistent_key( const psa_key_file_id_t key, * \retval PSA_ERROR_STORAGE_FAILURE * \retval PSA_ERROR_DOES_NOT_EXIST */ -psa_status_t psa_load_persistent_key( psa_key_file_id_t key, - psa_key_type_t *type, - psa_key_policy_t *policy, +psa_status_t psa_load_persistent_key( psa_key_attributes_t *attributes, uint8_t **data, size_t *data_length ); @@ -161,17 +156,15 @@ void psa_free_persistent_key_data( uint8_t *key_data, size_t key_data_length ); /** * \brief Formats key data and metadata for persistent storage * - * \param[in] data Buffer for the key data. + * \param[in] data Buffer containing the key data. * \param data_length Length of the key data buffer. - * \param type Key type (a \c PSA_KEY_TYPE_XXX value). - * \param policy The key policy. + * \param[in] attributes The attributes of the key. * \param[out] storage_data Output buffer for the formatted data. * */ void psa_format_key_data_for_storage( const uint8_t *data, const size_t data_length, - const psa_key_type_t type, - const psa_key_policy_t *policy, + const psa_key_attributes_t *attributes, uint8_t *storage_data ); /** @@ -183,8 +176,8 @@ void psa_format_key_data_for_storage( const uint8_t *data, * containing the key data. This must be freed * using psa_free_persistent_key_data() * \param[out] key_data_length Length of the key data buffer - * \param[out] type Key type (a \c PSA_KEY_TYPE_XXX value). - * \param[out] policy The key policy. + * \param[out] attributes On success, the attribute structure is filled + * with the loaded key metadata. * * \retval PSA_SUCCESS * \retval PSA_ERROR_INSUFFICIENT_STORAGE @@ -195,8 +188,7 @@ psa_status_t psa_parse_key_data_from_storage( const uint8_t *storage_data, size_t storage_data_length, uint8_t **key_data, size_t *key_data_length, - psa_key_type_t *type, - psa_key_policy_t *policy ); + psa_key_attributes_t *attributes ); #if defined(MBEDTLS_PSA_CRYPTO_SE_C) /** This symbol is defined if transaction support is required. */ diff --git a/tests/suites/test_suite_psa_crypto_persistent_key.function b/tests/suites/test_suite_psa_crypto_persistent_key.function index fc1924897e..fb9860748f 100644 --- a/tests/suites/test_suite_psa_crypto_persistent_key.function +++ b/tests/suites/test_suite_psa_crypto_persistent_key.function @@ -33,16 +33,17 @@ void format_storage_data_check( data_t *key_data, { uint8_t *file_data; size_t file_data_length; - psa_key_policy_t key_policy; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - key_policy.usage = (psa_key_usage_t) key_usage; - key_policy.alg = (psa_algorithm_t) key_alg; - key_policy.alg2 = (psa_algorithm_t) key_alg2; + psa_set_key_type( &attributes, key_type ); + psa_set_key_usage_flags( &attributes, key_usage ); + psa_set_key_algorithm( &attributes, key_alg ); + psa_set_key_enrollment_algorithm( &attributes, key_alg2 ); file_data_length = key_data->len + sizeof( psa_persistent_key_storage_format ); file_data = mbedtls_calloc( 1, file_data_length ); psa_format_key_data_for_storage( key_data->x, key_data->len, - (psa_key_type_t) key_type, &key_policy, + &attributes, file_data ); ASSERT_COMPARE( expected_file_data->x, expected_file_data->len, @@ -62,22 +63,25 @@ void parse_storage_data_check( data_t *file_data, { uint8_t *key_data = NULL; size_t key_data_length = 0; - psa_key_type_t key_type = 0; - psa_key_policy_t key_policy; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_status_t status; status = psa_parse_key_data_from_storage( file_data->x, file_data->len, &key_data, &key_data_length, - &key_type, &key_policy ); + &attributes ); TEST_EQUAL( status, expected_status ); if( status != PSA_SUCCESS ) goto exit; - TEST_EQUAL( key_type, (psa_key_type_t) expected_key_type ); - TEST_EQUAL( key_policy.usage, (uint32_t) expected_key_usage ); - TEST_EQUAL( key_policy.alg, (uint32_t) expected_key_alg ); - TEST_EQUAL( key_policy.alg2, (uint32_t) expected_key_alg2 ); + TEST_EQUAL( psa_get_key_type( &attributes ), + (psa_key_type_t) expected_key_type ); + TEST_EQUAL( psa_get_key_usage_flags( &attributes ), + (uint32_t) expected_key_usage ); + TEST_EQUAL( psa_get_key_algorithm( &attributes ), + (uint32_t) expected_key_alg ); + TEST_EQUAL( psa_get_key_enrollment_algorithm( &attributes ), + (uint32_t) expected_key_alg2 ); ASSERT_COMPARE( expected_key_data->x, expected_key_data->len, key_data, key_data_length ); From 0e8d495bd9b1f63717ee146070430d32e0f82c27 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Jul 2019 14:46:52 +0200 Subject: [PATCH 22/50] Add the lifetime to the key storage format Stored keys must contain lifetime information. The lifetime used to be implied by the location of the key, back when applications supplied the lifetime value when opening the key. Now that all keys' metadata are stored in a central location, this location needs to store the lifetime explicitly. --- library/psa_crypto_storage.c | 3 +++ .../suites/test_suite_psa_crypto_persistent_key.data | 12 ++++++------ .../test_suite_psa_crypto_persistent_key.function | 7 ++++++- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/library/psa_crypto_storage.c b/library/psa_crypto_storage.c index a35808a61d..b8569beb8a 100644 --- a/library/psa_crypto_storage.c +++ b/library/psa_crypto_storage.c @@ -255,6 +255,7 @@ static psa_status_t psa_crypto_storage_get_data_length( typedef struct { uint8_t magic[PSA_KEY_STORAGE_MAGIC_HEADER_LENGTH]; uint8_t version[4]; + uint8_t lifetime[sizeof( psa_key_lifetime_t )]; uint8_t type[sizeof( psa_key_type_t )]; uint8_t policy[sizeof( psa_key_policy_t )]; uint8_t data_len[4]; @@ -271,6 +272,7 @@ void psa_format_key_data_for_storage( const uint8_t *data, memcpy( storage_format->magic, PSA_KEY_STORAGE_MAGIC_HEADER, PSA_KEY_STORAGE_MAGIC_HEADER_LENGTH ); PUT_UINT32_LE( 0, storage_format->version, 0 ); + PUT_UINT32_LE( psa_get_key_lifetime( attributes ), storage_format->lifetime, 0 ); PUT_UINT32_LE( psa_get_key_type( attributes ), storage_format->type, 0 ); PUT_UINT32_LE( psa_get_key_usage_flags( attributes ), storage_format->policy, 0 ); PUT_UINT32_LE( psa_get_key_algorithm( attributes ), storage_format->policy, sizeof( uint32_t ) ); @@ -326,6 +328,7 @@ psa_status_t psa_parse_key_data_from_storage( const uint8_t *storage_data, memcpy( *key_data, storage_format->key_data, *key_data_length ); } + GET_UINT32_LE( attributes->lifetime, storage_format->lifetime, 0 ); GET_UINT32_LE( attributes->type, storage_format->type, 0 ); GET_UINT32_LE( attributes->policy.usage, storage_format->policy, 0 ); GET_UINT32_LE( attributes->policy.alg, storage_format->policy, sizeof( uint32_t ) ); diff --git a/tests/suites/test_suite_psa_crypto_persistent_key.data b/tests/suites/test_suite_psa_crypto_persistent_key.data index dead13d010..925c0f54a9 100644 --- a/tests/suites/test_suite_psa_crypto_persistent_key.data +++ b/tests/suites/test_suite_psa_crypto_persistent_key.data @@ -1,20 +1,20 @@ PSA Storage format data for storage -format_storage_data_check:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"505341004b4559000000000000000170010000000000001200000010620200003082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN +format_storage_data_check:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"505341004b455900000000000100000000000170010000000000001200000010620200003082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN PSA Storage parse stored data -parse_storage_data_check:"505341004b4559000000000000000170010000000000001200000010620200003082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN:PSA_SUCCESS +parse_storage_data_check:"505341004b455900000000000100000000000170010000000000001200000010620200003082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN:PSA_SUCCESS PSA Storage parse stored data wrong version, should fail -parse_storage_data_check:"505341004b455900ffffffff00000170010000000000001200000010620200003082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN:PSA_ERROR_STORAGE_FAILURE +parse_storage_data_check:"505341004b455900ffffffff0100000000000170010000000000001200000010620200003082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN:PSA_ERROR_STORAGE_FAILURE PSA Storage parse too big data, should fail -parse_storage_data_check:"505341004b4559000000000000000170010000000000001200000010ffffffff3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"":PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN:PSA_ERROR_STORAGE_FAILURE +parse_storage_data_check:"505341004b455900000000000100000000000170010000000000001200000010ffffffff3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"":PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN:PSA_ERROR_STORAGE_FAILURE PSA Storage parse bad magic, should fail -parse_storage_data_check:"645341004b4559000000000000000170010000000000001200000010620200003082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN:PSA_ERROR_STORAGE_FAILURE +parse_storage_data_check:"645341004b455900000000000100000000000170010000000000001200000010620200003082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN:PSA_ERROR_STORAGE_FAILURE PSA Storage parse not enough magic, should fail -parse_storage_data_check:"505341004b4559":"":PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN:PSA_ERROR_STORAGE_FAILURE +parse_storage_data_check:"505341004b4559":"":PSA_KEY_LIFETIME_PERSISTENT:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_CATEGORY_ASYMMETRIC_ENCRYPTION:PSA_ALG_CATEGORY_SIGN:PSA_ERROR_STORAGE_FAILURE # Not specific to files, but only run this test in an environment where the maximum size could be reached. Save maximum size persistent raw key diff --git a/tests/suites/test_suite_psa_crypto_persistent_key.function b/tests/suites/test_suite_psa_crypto_persistent_key.function index fb9860748f..b76c7330ae 100644 --- a/tests/suites/test_suite_psa_crypto_persistent_key.function +++ b/tests/suites/test_suite_psa_crypto_persistent_key.function @@ -12,6 +12,7 @@ typedef struct { uint8_t magic[PSA_KEY_STORAGE_MAGIC_HEADER_LENGTH]; uint8_t version[4]; + uint8_t lifetime[sizeof( psa_key_lifetime_t )]; uint8_t type[sizeof( psa_key_type_t )]; uint8_t policy[sizeof( psa_key_policy_t )]; uint8_t data_len[4]; @@ -28,13 +29,14 @@ typedef struct { /* BEGIN_CASE */ void format_storage_data_check( data_t *key_data, data_t *expected_file_data, - int key_type, + int key_lifetime, int key_type, int key_usage, int key_alg, int key_alg2 ) { uint8_t *file_data; size_t file_data_length; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_set_key_lifetime( &attributes, key_lifetime ); psa_set_key_type( &attributes, key_type ); psa_set_key_usage_flags( &attributes, key_usage ); psa_set_key_algorithm( &attributes, key_alg ); @@ -55,6 +57,7 @@ void format_storage_data_check( data_t *key_data, /* BEGIN_CASE */ void parse_storage_data_check( data_t *file_data, data_t *expected_key_data, + int expected_key_lifetime, int expected_key_type, int expected_key_usage, int expected_key_alg, @@ -74,6 +77,8 @@ void parse_storage_data_check( data_t *file_data, if( status != PSA_SUCCESS ) goto exit; + TEST_EQUAL( psa_get_key_lifetime( &attributes ), + (psa_key_type_t) expected_key_lifetime ); TEST_EQUAL( psa_get_key_type( &attributes ), (psa_key_type_t) expected_key_type ); TEST_EQUAL( psa_get_key_usage_flags( &attributes ), From 1df83d4f5b0e6c701a13acd7b795aad3313f2a0e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Jul 2019 16:13:14 +0200 Subject: [PATCH 23/50] SE keys: implement persistent storage For a key in a secure element, persist the key slot. This is implemented in the nominal case. Failures may not be handled properly. --- library/psa_crypto.c | 41 +++++++++++++------ library/psa_crypto_slot_management.c | 22 +++++++++- .../test_suite_psa_crypto_se_driver_hal.data | 10 ++++- ...st_suite_psa_crypto_se_driver_hal.function | 25 ++++++++++- 4 files changed, 80 insertions(+), 18 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index e048e9f2bc..84b691196d 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1469,20 +1469,30 @@ static psa_status_t psa_finish_key_creation( (void) driver; #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) - if( slot->lifetime == PSA_KEY_LIFETIME_PERSISTENT ) + if( slot->lifetime != PSA_KEY_LIFETIME_VOLATILE ) { uint8_t *buffer = NULL; size_t buffer_size = 0; - size_t length; + size_t length = 0; - buffer_size = PSA_KEY_EXPORT_MAX_SIZE( slot->type, - psa_get_key_slot_bits( slot ) ); - buffer = mbedtls_calloc( 1, buffer_size ); - if( buffer == NULL && buffer_size != 0 ) - return( PSA_ERROR_INSUFFICIENT_MEMORY ); - status = psa_internal_export_key( slot, - buffer, buffer_size, &length, - 0 ); +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( driver != NULL ) + { + buffer = (uint8_t*) &slot->data.se.slot_number; + length = sizeof( slot->data.se.slot_number ); + } + else +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + { + buffer_size = PSA_KEY_EXPORT_MAX_SIZE( slot->type, + psa_get_key_slot_bits( slot ) ); + buffer = mbedtls_calloc( 1, buffer_size ); + if( buffer == NULL && buffer_size != 0 ) + return( PSA_ERROR_INSUFFICIENT_MEMORY ); + status = psa_internal_export_key( slot, + buffer, buffer_size, &length, + 0 ); + } if( status == PSA_SUCCESS ) { @@ -1491,9 +1501,14 @@ static psa_status_t psa_finish_key_creation( status = psa_save_persistent_key( &attributes, buffer, length ); } - if( buffer_size != 0 ) - mbedtls_platform_zeroize( buffer, buffer_size ); - mbedtls_free( buffer ); +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( driver == NULL ) +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + { + if( buffer_size != 0 ) + mbedtls_platform_zeroize( buffer, buffer_size ); + mbedtls_free( buffer ); + } } #endif /* defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */ diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 5326fbd6ad..6b87ea0b01 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -131,10 +131,28 @@ static psa_status_t psa_load_persistent_key_into_slot( psa_key_slot_t *p_slot ) &key_data, &key_data_length ); if( status != PSA_SUCCESS ) goto exit; + p_slot->lifetime = psa_get_key_lifetime( &attributes ); p_slot->type = psa_get_key_type( &attributes ); p_slot->policy = attributes.policy; - status = psa_import_key_into_slot( p_slot, - key_data, key_data_length ); + +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( psa_key_lifetime_is_external( p_slot->lifetime ) ) + { + if( key_data_length != sizeof( p_slot->data.se.slot_number ) ) + { + status = PSA_ERROR_STORAGE_FAILURE; + goto exit; + } + memcpy( &p_slot->data.se.slot_number, key_data, + sizeof( p_slot->data.se.slot_number ) ); + } + else +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + { + status = psa_import_key_into_slot( p_slot, + key_data, key_data_length ); + } + exit: psa_free_persistent_key_data( key_data, key_data_length ); 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 28c7d7583f..cb21ab549f 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -28,7 +28,13 @@ Register SE driver: maximum number of drivers register_max: Key creation smoke test (p_allocate allows all slots) -key_creation_import_export:0 +key_creation_import_export:0:0 Key creation smoke test (p_allocate allows 1 slot) -key_creation_import_export:ARRAY_LENGTH( ram_slots ) - 1 +key_creation_import_export:ARRAY_LENGTH( ram_slots ) - 1:0 + +Key creation smoke test, check after restart (slot 0) +key_creation_import_export:0:1 + +Key creation smoke test, check after restart (slot 3) +key_creation_import_export:3:1 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 2e2a6480f0..5a2ebe71bc 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -3,6 +3,7 @@ #include "psa/crypto_se_driver.h" #include "psa_crypto_se.h" +#include "psa_crypto_storage.h" /** The minimum valid lifetime value for a secure element driver. */ #define MIN_DRIVER_LIFETIME 2 @@ -115,6 +116,18 @@ psa_status_t ram_allocate( psa_drv_se_context_t *context, return( PSA_ERROR_INSUFFICIENT_STORAGE ); } +#define MAX_KEY_ID_FOR_TEST 10 +void psa_purge_storage( void ) +{ + psa_key_id_t i; + /* The tests may have potentially created key ids from 1 to + * MAX_KEY_ID_FOR_TEST. In addition, run the destroy function on key id + * 0, which file-based storage uses as a temporary file. */ + for( i = 0; i <= MAX_KEY_ID_FOR_TEST; i++ ) + psa_destroy_persistent_key( i ); + psa_crypto_stop_transaction( ); +} + /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -188,7 +201,7 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void key_creation_import_export( int min_slot ) +void key_creation_import_export( int min_slot, int restart ) { psa_drv_se_t driver; psa_drv_se_key_management_t key_management; @@ -223,6 +236,15 @@ void key_creation_import_export( int min_slot ) key_material, sizeof( key_material ), &handle ) ); + /* Maybe restart, to check that the information is saved correctly. */ + if( restart ) + { + 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_ASSERT( ram_slots[min_slot].type == PSA_KEY_TYPE_RAW_DATA ); @@ -240,5 +262,6 @@ void key_creation_import_export( int min_slot ) exit: PSA_DONE( ); ram_slots_reset( ); + psa_purge_storage( ); } /* END_CASE */ From 8b96cad20483554ff3d7825d8c94807bdd2ce3ca Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Jul 2019 17:38:08 +0200 Subject: [PATCH 24/50] SE drivers: implement persistent storage Store the persistent data of secure element drivers. This is fully implemented, but not at all tested. --- library/psa_crypto_se.c | 58 ++++++++++++++++--- library/psa_crypto_se.h | 33 +++++++++++ ...st_suite_psa_crypto_se_driver_hal.function | 11 +++- 3 files changed, 92 insertions(+), 10 deletions(-) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index 7287ac0d76..bae44fa04b 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -35,6 +35,13 @@ #include "psa_crypto_se.h" +#if defined(MBEDTLS_PSA_ITS_FILE_C) +#include "psa_crypto_its.h" +#else /* Native ITS implementation */ +#include "psa/error.h" +#include "psa/internal_trusted_storage.h" +#endif + #include "mbedtls/platform.h" #if !defined(MBEDTLS_PLATFORM_C) #define mbedtls_calloc calloc @@ -114,20 +121,52 @@ int psa_get_se_driver( psa_key_lifetime_t lifetime, /* Persistent data management */ /****************************************************************/ +static psa_status_t psa_get_se_driver_its_file_uid( + const psa_se_drv_table_entry_t *driver, + psa_storage_uid_t *uid ) +{ + if( driver->lifetime > PSA_MAX_SE_LIFETIME ) + return( PSA_ERROR_NOT_SUPPORTED ); + *uid = PSA_CRYPTO_SE_DRIVER_ITS_UID_BASE + driver->lifetime; + return( PSA_SUCCESS ); +} + psa_status_t psa_load_se_persistent_data( const psa_se_drv_table_entry_t *driver ) { - /*TODO*/ - (void) driver; - return( PSA_SUCCESS ); + psa_status_t status; + psa_storage_uid_t uid; + + status = psa_get_se_driver_its_file_uid( driver, &uid ); + if( status != PSA_SUCCESS ) + return( status ); + + return( psa_its_get( uid, 0, driver->internal.persistent_data_size, + driver->internal.persistent_data ) ); } psa_status_t psa_save_se_persistent_data( const psa_se_drv_table_entry_t *driver ) { - /*TODO*/ - (void) driver; - return( PSA_SUCCESS ); + psa_status_t status; + psa_storage_uid_t uid; + + status = psa_get_se_driver_its_file_uid( driver, &uid ); + if( status != PSA_SUCCESS ) + return( status ); + + return( psa_its_set( uid, driver->internal.persistent_data_size, + driver->internal.persistent_data, + 0 ) ); +} + +psa_status_t psa_destroy_se_persistent_data( psa_key_lifetime_t lifetime ) +{ + psa_storage_uid_t uid; + if( lifetime > PSA_MAX_SE_LIFETIME ) + return( PSA_ERROR_NOT_SUPPORTED ); + uid = PSA_CRYPTO_SE_DRIVER_ITS_UID_BASE + lifetime; + return( psa_its_remove( uid ) ); } psa_status_t psa_find_se_slot_for_key( @@ -201,6 +240,8 @@ psa_status_t psa_register_se_driver( { return( PSA_ERROR_INVALID_ARGUMENT ); } + if( lifetime > PSA_MAX_SE_LIFETIME ) + return( PSA_ERROR_NOT_SUPPORTED ); for( i = 0; i < PSA_MAX_SE_DRIVERS; i++ ) { @@ -227,8 +268,11 @@ psa_status_t psa_register_se_driver( status = PSA_ERROR_INSUFFICIENT_MEMORY; goto error; } + /* Load the driver's persistent data. On first use, the persistent + * data does not exist in storage, and is initialized to + * all-bits-zero by the calloc call just above. */ status = psa_load_se_persistent_data( &driver_table[i] ); - if( status != PSA_SUCCESS ) + if( status != PSA_SUCCESS && status != PSA_ERROR_DOES_NOT_EXIST ) goto error; } driver_table[i].internal.persistent_data_size = diff --git a/library/psa_crypto_se.h b/library/psa_crypto_se.h index f1d7e7c367..08e658cddc 100644 --- a/library/psa_crypto_se.h +++ b/library/psa_crypto_se.h @@ -31,6 +31,30 @@ #include "psa/crypto.h" #include "psa/crypto_se_driver.h" +/** The maximum lifetime value that this implementation supports + * for a secure element. + * + * This is not a characteristic that each PSA implementation has, but a + * limitation of the current implementation due to the constraints imposed + * by storage. See #PSA_CRYPTO_SE_DRIVER_ITS_UID_BASE. + * + * The minimum lifetime value for a secure element is 2, like on any + * PSA implementation (0=volatile and 1=internal-storage are taken). + */ +#define PSA_MAX_SE_LIFETIME 255 + +/** The base of the range of ITS file identifiers for secure element + * driver persistent data. + * + * We use a slice of the implemenation reserved range 0xffff0000..0xffffffff, + * specifically the range 0xfffffe00..0xfffffeff. The length of this range + * drives the value of #PSA_MAX_SE_LIFETIME. + * The identifiers 0xfffffe00 and 0xfffffe01 are actually not used since + * they correspond to #PSA_KEY_LIFETIME_VOLATILE and + * #PSA_KEY_LIFETIME_PERSISTENT which don't have a driver. + */ +#define PSA_CRYPTO_SE_DRIVER_ITS_UID_BASE ( (psa_key_id_t) 0xfffffe00 ) + /** The maximum number of registered secure element driver lifetimes. */ #define PSA_MAX_SE_DRIVERS 4 @@ -138,4 +162,13 @@ psa_status_t psa_load_se_persistent_data( psa_status_t psa_save_se_persistent_data( const psa_se_drv_table_entry_t *driver ); +/** Destroy the persistent data of a secure element driver. + * + * This is currently only used for testing. + * + * \param[in] lifetime The driver lifetime whose persistent data should + * be erased. + */ +psa_status_t psa_destroy_se_persistent_data( psa_key_lifetime_t lifetime ); + #endif /* PSA_CRYPTO_SE_H */ 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 5a2ebe71bc..010f696845 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -119,13 +119,18 @@ psa_status_t ram_allocate( psa_drv_se_context_t *context, #define MAX_KEY_ID_FOR_TEST 10 void psa_purge_storage( void ) { - psa_key_id_t i; + psa_key_id_t id; + psa_key_lifetime_t lifetime; /* The tests may have potentially created key ids from 1 to * MAX_KEY_ID_FOR_TEST. In addition, run the destroy function on key id * 0, which file-based storage uses as a temporary file. */ - for( i = 0; i <= MAX_KEY_ID_FOR_TEST; i++ ) - psa_destroy_persistent_key( i ); + for( id = 0; id <= MAX_KEY_ID_FOR_TEST; id++ ) + psa_destroy_persistent_key( id ); + /* Purge the transaction file. */ psa_crypto_stop_transaction( ); + /* Purge driver persistent data. */ + for( lifetime = 0; lifetime < PSA_MAX_SE_LIFETIME; lifetime++ ) + psa_destroy_se_persistent_data( lifetime ); } /* END_HEADER */ From 1d04b05fae0fc7f475aa6e17eaeb4b61a16f3125 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Jul 2019 17:38:41 +0200 Subject: [PATCH 25/50] Dear check-names, where you accept struct, also accept union. --- tests/scripts/list-identifiers.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/list-identifiers.sh b/tests/scripts/list-identifiers.sh index eaf270c7ae..4828c80eb4 100755 --- a/tests/scripts/list-identifiers.sh +++ b/tests/scripts/list-identifiers.sh @@ -41,7 +41,7 @@ rm -f identifiers grep '^[^ /#{]' $HEADERS | \ sed -e 's/^[^:]*://' | \ - egrep -v '^(extern "C"|(typedef )?(struct|enum)( {)?$|};?$)' \ + egrep -v '^(extern "C"|(typedef )?(struct|union|enum)( {)?$|};?$)' \ > _decls if true; then From 9dd125d8bb2d84853ac771cacba2cd1e66a8f8e2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Jul 2019 18:26:43 +0200 Subject: [PATCH 26/50] Fix overly complex Doxygen markup --- include/psa/crypto_se_driver.h | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index e7fe00671d..3f3d7ca8dd 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -63,9 +63,8 @@ typedef struct { * like a pointer returned by `malloc` (but the core can use any * method to allocate the buffer, not necessarily `malloc`). * - * The size of this buffer is given by psa_drv_se_t::persistent_data_size - * when the driver is registered, and this value is also recorded in the - * psa_drv_se_context_t::persistent_data_size field of this structure. + * The size of this buffer is in the \c persistent_data_size field of + * this structure. * * Before the driver is initialized for the first time, the content of * the persistent data is all-bits-zero. After a driver upgrade, if the @@ -81,8 +80,8 @@ typedef struct { /** The size of \c persistent_data in bytes. * - * This is always equal to the value of - * psa_drv_se_t::persistent_data_size when the driver is registered. + * This is always equal to the value of the `persistent_data_size` field + * of the ::psa_drv_se_t structure when the driver is registered. */ const size_t persistent_data_size; @@ -902,7 +901,7 @@ typedef psa_status_t (*psa_drv_se_export_key_t)(psa_drv_se_context_t *drv_contex * \brief A function that generates a symmetric or asymmetric key on a secure * element * - * If \p type is asymmetric (`#PSA_KEY_TYPE_IS_ASYMMETRIC(\p type) == 1`), + * If \p type is asymmetric (#PSA_KEY_TYPE_IS_ASYMMETRIC(\p type) = 1), * the public component of the generated key will be placed in `p_pubkey_out`. * The format of the public key information will match the format specified for * the psa_export_key() function for the key type. From 105f67f0fa050315aaaec650b35a9f36c1b5ec93 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Jul 2019 18:16:05 +0200 Subject: [PATCH 27/50] Move the definition of psa_key_attributes_t to crypto_types.h psa_key_attributes_t is used in the SE driver HAL, so it must be defined in a common header, not in the API-only header crypto.h. --- include/psa/crypto.h | 109 +------------------------------ include/psa/crypto_se_driver.h | 4 -- include/psa/crypto_types.h | 113 +++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 112 deletions(-) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 3036d17b47..ea7d18d2bc 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -93,117 +93,10 @@ psa_status_t psa_crypto_init(void); /**@}*/ -/** \defgroup attributes Key attributes +/** \addtogroup attributes * @{ */ -/** The type of a structure containing key attributes. - * - * This is an opaque structure that can represent the metadata of a key - * object. Metadata that can be stored in attributes includes: - * - The location of the key in storage, indicated by its key identifier - * and its lifetime. - * - The key's policy, comprising usage flags and a specification of - * the permitted algorithm(s). - * - Information about the key itself: the key type and its size. - * - Implementations may define additional attributes. - * - * The actual key material is not considered an attribute of a key. - * Key attributes do not contain information that is generally considered - * highly confidential. - * - * An attribute structure can be a simple data structure where each function - * `psa_set_key_xxx` sets a field and the corresponding function - * `psa_get_key_xxx` retrieves the value of the corresponding field. - * However, implementations may report values that are equivalent to the - * original one, but have a different encoding. For example, an - * implementation may use a more compact representation for types where - * many bit-patterns are invalid or not supported, and store all values - * that it does not support as a special marker value. In such an - * implementation, after setting an invalid value, the corresponding - * get function returns an invalid value which may not be the one that - * was originally stored. - * - * An attribute structure may contain references to auxiliary resources, - * for example pointers to allocated memory or indirect references to - * pre-calculated values. In order to free such resources, the application - * must call psa_reset_key_attributes(). As an exception, calling - * psa_reset_key_attributes() on an attribute structure is optional if - * the structure has only been modified by the following functions - * since it was initialized or last reset with psa_reset_key_attributes(): - * - psa_set_key_id() - * - psa_set_key_lifetime() - * - psa_set_key_type() - * - psa_set_key_bits() - * - psa_set_key_usage_flags() - * - psa_set_key_algorithm() - * - * Before calling any function on a key attribute structure, the application - * must initialize it by any of the following means: - * - Set the structure to all-bits-zero, for example: - * \code - * psa_key_attributes_t attributes; - * memset(&attributes, 0, sizeof(attributes)); - * \endcode - * - Initialize the structure to logical zero values, for example: - * \code - * psa_key_attributes_t attributes = {0}; - * \endcode - * - Initialize the structure to the initializer #PSA_KEY_ATTRIBUTES_INIT, - * for example: - * \code - * psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - * \endcode - * - Assign the result of the function psa_key_attributes_init() - * to the structure, for example: - * \code - * psa_key_attributes_t attributes; - * attributes = psa_key_attributes_init(); - * \endcode - * - * A freshly initialized attribute structure contains the following - * values: - * - * - lifetime: #PSA_KEY_LIFETIME_VOLATILE. - * - key identifier: unspecified. - * - type: \c 0. - * - key size: \c 0. - * - usage flags: \c 0. - * - algorithm: \c 0. - * - * A typical sequence to create a key is as follows: - * -# Create and initialize an attribute structure. - * -# If the key is persistent, call psa_set_key_id(). - * Also call psa_set_key_lifetime() to place the key in a non-default - * location. - * -# Set the key policy with psa_set_key_usage_flags() and - * psa_set_key_algorithm(). - * -# Set the key type with psa_set_key_type(). - * Skip this step if copying an existing key with psa_copy_key(). - * -# When generating a random key with psa_generate_key() or deriving a key - * with psa_key_derivation_output_key(), set the desired key size with - * psa_set_key_bits(). - * -# Call a key creation function: psa_import_key(), psa_generate_key(), - * psa_key_derivation_output_key() or psa_copy_key(). This function reads - * the attribute structure, creates a key with these attributes, and - * outputs a handle to the newly created key. - * -# The attribute structure is now no longer necessary. - * You may call psa_reset_key_attributes(), although this is optional - * with the workflow presented here because the attributes currently - * defined in this specification do not require any additional resources - * beyond the structure itself. - * - * A typical sequence to query a key's attributes is as follows: - * -# Call psa_get_key_attributes(). - * -# Call `psa_get_key_xxx` functions to retrieve the attribute(s) that - * you are interested in. - * -# Call psa_reset_key_attributes() to free any resources that may be - * used by the attribute structure. - * - * Once a key has been created, it is impossible to change its attributes. - */ -typedef struct psa_key_attributes_s psa_key_attributes_t; - /** \def PSA_KEY_ATTRIBUTES_INIT * * This macro returns a suitable initializer for a key attribute structure diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 3f3d7ca8dd..57d077c2e8 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -782,10 +782,6 @@ typedef struct { */ /**@{*/ -/* This type is documented in crypto.h. As far as drivers are concerned, - * this is an opaque type. */ -typedef struct psa_key_attributes_s psa_key_attributes_t; - /** \brief A function that allocates a slot for a key. * * \param[in,out] drv_context The driver context structure. diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h index 7f0f38cddf..1944be4b26 100644 --- a/include/psa/crypto_types.h +++ b/include/psa/crypto_types.h @@ -133,6 +133,119 @@ typedef uint32_t psa_key_usage_t; /**@}*/ +/** \defgroup attributes Key attributes + * @{ + */ + +/** The type of a structure containing key attributes. + * + * This is an opaque structure that can represent the metadata of a key + * object. Metadata that can be stored in attributes includes: + * - The location of the key in storage, indicated by its key identifier + * and its lifetime. + * - The key's policy, comprising usage flags and a specification of + * the permitted algorithm(s). + * - Information about the key itself: the key type and its size. + * - Implementations may define additional attributes. + * + * The actual key material is not considered an attribute of a key. + * Key attributes do not contain information that is generally considered + * highly confidential. + * + * An attribute structure can be a simple data structure where each function + * `psa_set_key_xxx` sets a field and the corresponding function + * `psa_get_key_xxx` retrieves the value of the corresponding field. + * However, implementations may report values that are equivalent to the + * original one, but have a different encoding. For example, an + * implementation may use a more compact representation for types where + * many bit-patterns are invalid or not supported, and store all values + * that it does not support as a special marker value. In such an + * implementation, after setting an invalid value, the corresponding + * get function returns an invalid value which may not be the one that + * was originally stored. + * + * An attribute structure may contain references to auxiliary resources, + * for example pointers to allocated memory or indirect references to + * pre-calculated values. In order to free such resources, the application + * must call psa_reset_key_attributes(). As an exception, calling + * psa_reset_key_attributes() on an attribute structure is optional if + * the structure has only been modified by the following functions + * since it was initialized or last reset with psa_reset_key_attributes(): + * - psa_set_key_id() + * - psa_set_key_lifetime() + * - psa_set_key_type() + * - psa_set_key_bits() + * - psa_set_key_usage_flags() + * - psa_set_key_algorithm() + * + * Before calling any function on a key attribute structure, the application + * must initialize it by any of the following means: + * - Set the structure to all-bits-zero, for example: + * \code + * psa_key_attributes_t attributes; + * memset(&attributes, 0, sizeof(attributes)); + * \endcode + * - Initialize the structure to logical zero values, for example: + * \code + * psa_key_attributes_t attributes = {0}; + * \endcode + * - Initialize the structure to the initializer #PSA_KEY_ATTRIBUTES_INIT, + * for example: + * \code + * psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + * \endcode + * - Assign the result of the function psa_key_attributes_init() + * to the structure, for example: + * \code + * psa_key_attributes_t attributes; + * attributes = psa_key_attributes_init(); + * \endcode + * + * A freshly initialized attribute structure contains the following + * values: + * + * - lifetime: #PSA_KEY_LIFETIME_VOLATILE. + * - key identifier: unspecified. + * - type: \c 0. + * - key size: \c 0. + * - usage flags: \c 0. + * - algorithm: \c 0. + * + * A typical sequence to create a key is as follows: + * -# Create and initialize an attribute structure. + * -# If the key is persistent, call psa_set_key_id(). + * Also call psa_set_key_lifetime() to place the key in a non-default + * location. + * -# Set the key policy with psa_set_key_usage_flags() and + * psa_set_key_algorithm(). + * -# Set the key type with psa_set_key_type(). + * Skip this step if copying an existing key with psa_copy_key(). + * -# When generating a random key with psa_generate_key() or deriving a key + * with psa_key_derivation_output_key(), set the desired key size with + * psa_set_key_bits(). + * -# Call a key creation function: psa_import_key(), psa_generate_key(), + * psa_key_derivation_output_key() or psa_copy_key(). This function reads + * the attribute structure, creates a key with these attributes, and + * outputs a handle to the newly created key. + * -# The attribute structure is now no longer necessary. + * You may call psa_reset_key_attributes(), although this is optional + * with the workflow presented here because the attributes currently + * defined in this specification do not require any additional resources + * beyond the structure itself. + * + * A typical sequence to query a key's attributes is as follows: + * -# Call psa_get_key_attributes(). + * -# Call `psa_get_key_xxx` functions to retrieve the attribute(s) that + * you are interested in. + * -# Call psa_reset_key_attributes() to free any resources that may be + * used by the attribute structure. + * + * Once a key has been created, it is impossible to change its attributes. + */ +typedef struct psa_key_attributes_s psa_key_attributes_t; + +/**@}*/ + /** \defgroup derivation Key derivation * @{ */ From 831ac72338d16ce8b9ee0a299db81b12f322cc1d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Jul 2019 19:29:35 +0200 Subject: [PATCH 28/50] Add transaction file and driver storage; new key file format Update the storage architecture with the new features introduced for secure element support: * Lifetime field in key files. * Slot number in key files for keys in a secure element. * Transaction file (name and format). * Persistent storage for secure element drivers (name and format). The version number is not determined yet. --- .../mbed-crypto-storage-specification.md | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/docs/architecture/mbed-crypto-storage-specification.md b/docs/architecture/mbed-crypto-storage-specification.md index f4abd3e706..2c3119fbdc 100644 --- a/docs/architecture/mbed-crypto-storage-specification.md +++ b/docs/architecture/mbed-crypto-storage-specification.md @@ -193,3 +193,92 @@ The layout of a key file is: * key material length (4 bytes) * key material: output of `psa_export_key` * Any trailing data is rejected on load. + +Mbed Crypto TBD +--------------- + +Tags: TBD + +Released in TBD 2019.
+Integrated in Mbed OS TBD. + +### Changes introduced in TBD + +* The layout of a key file now has a lifetime field before the type field. +* Key files can store references to keys in a secure element. In such key files, the key material contains the slot number. + +### File namespace on a PSA platform on TBD + +Assumption: ITS provides a 64-bit file identifier namespace. The Crypto service can use arbitrary file identifiers and no other part of the system accesses the same file identifier namespace. + +Assumption: the owner identifier is a nonzero value of type `int32_t`. + +* Files 0 through 0xfffeffff: unused. +* Files 0xffff0000 through 0xffffffff: reserved for internal use of the crypto library or crypto service. See [non-key files](#non-key-files-on-tbd). +* Files 0x100000000 through 0xffffffffffff: [content](#key-file-format-for-1.0.0) of the [key whose identifier is the file identifier](#key-names-for-1.0.0). The upper 32 bits determine the owner. + +### File namespace on ITS as a library on TBD + +Assumption: ITS provides a 64-bit file identifier namespace. The entity using the crypto library can use arbitrary file identifiers and no other part of the system accesses the same file identifier namespace. + +This is a library integration, so there is no owner. The key file identifier is identical to the key identifier. + +* File 0: unused. +* Files 1 through 0xfffeffff: [content](#key-file-format-for-1.0.0) of the [key whose identifier is the file identifier](#key-names-for-1.0.0). +* Files 0xffff0000 through 0xffffffff: reserved for internal use of the crypto library or crypto service. See [non-key files](#non-key-files-on-tbd). +* Files 0x100000000 through 0xffffffffffffffff: unused. + +### Non-key files on TBD + +File identifiers in the range 0xffff0000 through 0xffffffff are reserved for internal use in Mbed Crypto. + +* Files 0xfffffe02 through 0xfffffeff (`PSA_CRYPTO_SE_DRIVER_ITS_UID_BASE + lifetime`): secure element driver storage. The content of the file is the secure element driver's persistent data. +* File 0xffffff52 (`PSA_CRYPTO_ITS_RANDOM_SEED_UID`): [nonvolatile random seed](#nonvolatile-random-seed-file-format-for-1.0.0). +* File 0xffffff54 (`PSA_CRYPTO_ITS_TRANSACTION_UID`): [transaction file](#transaction-file-format-for-tbd). +* Other files are unused and reserved for future use. + +### Key file format for TBD + +All integers are encoded in little-endian order in 8-bit bytes except where otherwise indicated. + +The layout of a key file is: + +* magic (8 bytes): `"PSA\0KEY\0"`. +* version (4 bytes): 0. +* lifetime (4 bytes): `psa_key_lifetime_t` value. +* type (4 bytes): `psa_key_type_t` value. +* policy usage flags (4 bytes): `psa_key_usage_t` value. +* policy usage algorithm (4 bytes): `psa_algorithm_t` value. +* policy enrollment algorithm (4 bytes): `psa_algorithm_t` value. +* key material length (4 bytes). +* key material: + * For a transparent key: output of `psa_export_key`. + * For an opaque key (key in a secure element): slot number (8 bytes), in platform endianness. +* Any trailing data is rejected on load. + +### Transaction file format for TBD + +The transaction file contains data about an ongoing action that cannot be completed atomically. It exists only if there is an ongoing transaction. + +All integers are encoded in platform endianness. + +All currently existing transactions concern a key in a secure element. + +The layout of a transaction file is: + +* type (2 bytes): the [transaction type](#transaction-types-on-tbd). +* unused (2 bytes) +* lifetime (4 bytes): `psa_key_lifetime_t` value that corresponds to a key in a secure element. +* slot number (8 bytes): `psa_key_slot_number_t` value. This is the unique designation of the key for the secure element driver. +* key identifier (4 bytes in a library integration, 8 bytes on a PSA platform): the internal representation of the key identifier. On a PSA platform, this encodes the key owner in the same way as [in file identifiers for key files](#file-namespace-on-a-psa-platform-on-tbd)). + +#### Transaction types on TBD + +* 0x0001: key creation. The following locations may or may not contain data about the key that is being created: + * The slot in the secure element designated by the slot number. + * The file containing the key metadata designated by the key identifier. + * The driver persistent data. +* 0x0002: key destruction. The following locations may or may not still contain data about the key that is being destroyed: + * The slot in the secure element designated by the slot number. + * The file containing the key metadata designated by the key identifier. + * The driver persistent data. From 573bbc1b4e744080fb9735e5e7c30298cd9b31b4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Jul 2019 19:59:23 +0200 Subject: [PATCH 29/50] Error out if a driver tries to store more than ITS can handle Cast explicitly for the sake of MSVC which otherwise (usefully!) warns about the truncation. --- library/psa_crypto_se.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index bae44fa04b..714a03904b 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -127,6 +127,13 @@ static psa_status_t psa_get_se_driver_its_file_uid( { if( driver->lifetime > PSA_MAX_SE_LIFETIME ) return( PSA_ERROR_NOT_SUPPORTED ); + +#if SIZE_MAX > UINT32_MAX + /* ITS file sizes are limited to 32 bits. */ + if( driver->internal.persistent_data_size > UINT32_MAX ) + return( PSA_ERROR_NOT_SUPPORTED ); +#endif + *uid = PSA_CRYPTO_SE_DRIVER_ITS_UID_BASE + driver->lifetime; return( PSA_SUCCESS ); } @@ -141,7 +148,8 @@ psa_status_t psa_load_se_persistent_data( if( status != PSA_SUCCESS ) return( status ); - return( psa_its_get( uid, 0, driver->internal.persistent_data_size, + return( psa_its_get( uid, 0, + (uint32_t) driver->internal.persistent_data_size, driver->internal.persistent_data ) ); } @@ -155,7 +163,8 @@ psa_status_t psa_save_se_persistent_data( if( status != PSA_SUCCESS ) return( status ); - return( psa_its_set( uid, driver->internal.persistent_data_size, + return( psa_its_set( uid, + (uint32_t) driver->internal.persistent_data_size, driver->internal.persistent_data, 0 ) ); } From 28f8f3068f97bb3c23c4f4aebc47abe094b12f81 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 24 Jul 2019 13:30:31 +0200 Subject: [PATCH 30/50] SE keys: ensure that functions that lack support properly error out Introduce a new function psa_get_transparent_key which returns NOT_SUPPORTED if the key is in a secure element. Use this function in functions that don't support keys in a secure element. After this commit, all functions that access a key slot directly via psa_get_key_slot or psa_get_key_from_slot rather than via psa_get_transparent_key have at least enough support for secure elements not to crash or otherwise cause undefined behavior. Lesser bad behavior such as wrong results or resource leakage is still possible in error cases. --- library/psa_crypto.c | 59 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 84b691196d..5fcf0ac866 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -871,6 +871,39 @@ static psa_status_t psa_get_key_from_slot( psa_key_handle_t handle, return( PSA_SUCCESS ); } +/** Retrieve a slot which must contain a transparent key. + * + * A transparent key is a key for which the key material is directly + * available, as opposed to a key in a secure element. + * + * This is a temporary function until secure element support is + * fully implemented. + */ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +static psa_status_t psa_get_transparent_key( psa_key_handle_t handle, + psa_key_slot_t **p_slot, + psa_key_usage_t usage, + psa_algorithm_t alg ) +{ + psa_status_t status = psa_get_key_from_slot( handle, p_slot, usage, alg ); + if( status != PSA_SUCCESS ) + return( status ); + /* Use a simple, cheap test to check whether the key is transparent. + * This check assumes that there are no persistent lifetimes other than + * PSA_KEY_LIFETIME_PERSISTENT. */ + if( ( *p_slot )->lifetime > PSA_KEY_LIFETIME_PERSISTENT ) + { + *p_slot = NULL; + return( PSA_ERROR_NOT_SUPPORTED ); + } + return( PSA_SUCCESS ); +} +#else /* MBEDTLS_PSA_CRYPTO_SE_C */ +/* With no secure element support, all keys are transparent. */ +#define psa_get_transparent_key( handle, p_slot, usage, alg ) \ + psa_get_key_from_slot( handle, p_slot, usage, alg ) +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + /** Wipe key data from a slot. Preserve metadata such as the policy. */ static psa_status_t psa_remove_key_data_from_memory( psa_key_slot_t *slot ) { @@ -1124,7 +1157,7 @@ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, psa_reset_key_attributes( attributes ); - status = psa_get_key_slot( handle, &slot ); + status = psa_get_transparent_key( handle, &slot, 0, 0 ); if( status != PSA_SUCCESS ) return( status ); @@ -1704,7 +1737,7 @@ psa_status_t psa_copy_key( psa_key_handle_t source_handle, psa_key_attributes_t actual_attributes = *specified_attributes; psa_se_drv_table_entry_t *driver = NULL; - status = psa_get_key_from_slot( source_handle, &source_slot, + status = psa_get_transparent_key( source_handle, &source_slot, PSA_KEY_USAGE_COPY, 0 ); if( status != PSA_SUCCESS ) goto exit; @@ -2485,7 +2518,7 @@ static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, if( is_sign ) operation->is_sign = 1; - status = psa_get_key_from_slot( handle, &slot, usage, alg ); + status = psa_get_transparent_key( handle, &slot, usage, alg ); if( status != PSA_SUCCESS ) goto exit; key_bits = psa_get_key_slot_bits( slot ); @@ -3064,7 +3097,7 @@ psa_status_t psa_asymmetric_sign( psa_key_handle_t handle, *signature_length = signature_size; - status = psa_get_key_from_slot( handle, &slot, PSA_KEY_USAGE_SIGN, alg ); + status = psa_get_transparent_key( handle, &slot, PSA_KEY_USAGE_SIGN, alg ); if( status != PSA_SUCCESS ) goto exit; if( ! PSA_KEY_TYPE_IS_KEY_PAIR( slot->type ) ) @@ -3137,7 +3170,7 @@ psa_status_t psa_asymmetric_verify( psa_key_handle_t handle, psa_key_slot_t *slot; psa_status_t status; - status = psa_get_key_from_slot( handle, &slot, PSA_KEY_USAGE_VERIFY, alg ); + status = psa_get_transparent_key( handle, &slot, PSA_KEY_USAGE_VERIFY, alg ); if( status != PSA_SUCCESS ) return( status ); @@ -3207,7 +3240,7 @@ psa_status_t psa_asymmetric_encrypt( psa_key_handle_t handle, if( ! PSA_ALG_IS_RSA_OAEP( alg ) && salt_length != 0 ) return( PSA_ERROR_INVALID_ARGUMENT ); - status = psa_get_key_from_slot( handle, &slot, PSA_KEY_USAGE_ENCRYPT, alg ); + status = psa_get_transparent_key( handle, &slot, PSA_KEY_USAGE_ENCRYPT, alg ); if( status != PSA_SUCCESS ) return( status ); if( ! ( PSA_KEY_TYPE_IS_PUBLIC_KEY( slot->type ) || @@ -3287,7 +3320,7 @@ psa_status_t psa_asymmetric_decrypt( psa_key_handle_t handle, if( ! PSA_ALG_IS_RSA_OAEP( alg ) && salt_length != 0 ) return( PSA_ERROR_INVALID_ARGUMENT ); - status = psa_get_key_from_slot( handle, &slot, PSA_KEY_USAGE_DECRYPT, alg ); + status = psa_get_transparent_key( handle, &slot, PSA_KEY_USAGE_DECRYPT, alg ); if( status != PSA_SUCCESS ) return( status ); if( ! PSA_KEY_TYPE_IS_KEY_PAIR( slot->type ) ) @@ -3396,7 +3429,7 @@ static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation, if( status != PSA_SUCCESS ) return( status ); - status = psa_get_key_from_slot( handle, &slot, usage, alg); + status = psa_get_transparent_key( handle, &slot, usage, alg); if( status != PSA_SUCCESS ) goto exit; key_bits = psa_get_key_slot_bits( slot ); @@ -3733,7 +3766,7 @@ static psa_status_t psa_aead_setup( aead_operation_t *operation, size_t key_bits; mbedtls_cipher_id_t cipher_id; - status = psa_get_key_from_slot( handle, &operation->slot, usage, alg ); + status = psa_get_transparent_key( handle, &operation->slot, usage, alg ); if( status != PSA_SUCCESS ) return( status ); @@ -4908,7 +4941,7 @@ psa_status_t psa_key_derivation( psa_key_derivation_operation_t *operation, if( ! PSA_ALG_IS_KEY_DERIVATION( alg ) ) return( PSA_ERROR_INVALID_ARGUMENT ); - status = psa_get_key_from_slot( handle, &slot, PSA_KEY_USAGE_DERIVE, alg ); + status = psa_get_transparent_key( handle, &slot, PSA_KEY_USAGE_DERIVE, alg ); if( status != PSA_SUCCESS ) return( status ); @@ -5282,7 +5315,7 @@ psa_status_t psa_key_derivation_input_key( { psa_key_slot_t *slot; psa_status_t status; - status = psa_get_key_from_slot( handle, &slot, + status = psa_get_transparent_key( handle, &slot, PSA_KEY_USAGE_DERIVE, operation->alg ); if( status != PSA_SUCCESS ) @@ -5431,7 +5464,7 @@ psa_status_t psa_key_derivation_key_agreement( psa_key_derivation_operation_t *o psa_status_t status; if( ! PSA_ALG_IS_KEY_AGREEMENT( operation->alg ) ) return( PSA_ERROR_INVALID_ARGUMENT ); - status = psa_get_key_from_slot( private_key, &slot, + status = psa_get_transparent_key( private_key, &slot, PSA_KEY_USAGE_DERIVE, operation->alg ); if( status != PSA_SUCCESS ) return( status ); @@ -5459,7 +5492,7 @@ psa_status_t psa_raw_key_agreement( psa_algorithm_t alg, status = PSA_ERROR_INVALID_ARGUMENT; goto exit; } - status = psa_get_key_from_slot( private_key, &slot, + status = psa_get_transparent_key( private_key, &slot, PSA_KEY_USAGE_DERIVE, alg ); if( status != PSA_SUCCESS ) goto exit; From 89870eb1238159abaaec84bc8a01f06a463a6639 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 24 Jul 2019 13:44:03 +0200 Subject: [PATCH 31/50] Cosmetic improvements in SE driver tests --- .../test_suite_psa_crypto_se_driver_hal.data | 8 ++-- ...st_suite_psa_crypto_se_driver_hal.function | 44 +++++++++++++------ 2 files changed, 35 insertions(+), 17 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 cb21ab549f..e9c0694778 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -27,14 +27,14 @@ register_twice:3 Register SE driver: maximum number of drivers register_max: -Key creation smoke test (p_allocate allows all slots) +SE key import-export (p_allocate allows all slots) key_creation_import_export:0:0 -Key creation smoke test (p_allocate allows 1 slot) +SE key import-export (p_allocate allows 1 slot) key_creation_import_export:ARRAY_LENGTH( ram_slots ) - 1:0 -Key creation smoke test, check after restart (slot 0) +SE key import-export, check after restart (slot 0) key_creation_import_export:0:1 -Key creation smoke test, check after restart (slot 3) +SE key import-export, check after restart (slot 3) key_creation_import_export:3:1 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 010f696845..661fb054f4 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -5,6 +5,12 @@ #include "psa_crypto_se.h" #include "psa_crypto_storage.h" + + +/****************************************************************/ +/* Test driver helpers */ +/****************************************************************/ + /** The minimum valid lifetime value for a secure element driver. */ #define MIN_DRIVER_LIFETIME 2 @@ -25,6 +31,12 @@ } \ } while( 0 ) + + +/****************************************************************/ +/* RAM-based test driver */ +/****************************************************************/ + #define RAM_MAX_KEY_SIZE 64 typedef struct { @@ -69,11 +81,11 @@ static psa_status_t ram_import( psa_drv_se_context_t *context, return( PSA_SUCCESS ); } -psa_status_t ram_export( psa_drv_se_context_t *context, - psa_key_slot_number_t slot_number, - uint8_t *p_data, - size_t data_size, - size_t *p_data_length ) +static psa_status_t ram_export( psa_drv_se_context_t *context, + psa_key_slot_number_t slot_number, + uint8_t *p_data, + size_t data_size, + size_t *p_data_length ) { size_t actual_size; (void) context; @@ -86,9 +98,9 @@ psa_status_t ram_export( psa_drv_se_context_t *context, return( PSA_SUCCESS ); } -psa_status_t ram_destroy( psa_drv_se_context_t *context, - void *persistent_data, - psa_key_slot_number_t slot_number ) +static psa_status_t ram_destroy( psa_drv_se_context_t *context, + void *persistent_data, + psa_key_slot_number_t slot_number ) { ram_slot_usage_t *slot_usage = persistent_data; DRIVER_ASSERT( context->persistent_data_size == sizeof( ram_slot_usage_t ) ); @@ -98,10 +110,10 @@ psa_status_t ram_destroy( psa_drv_se_context_t *context, return( PSA_SUCCESS ); } -psa_status_t ram_allocate( psa_drv_se_context_t *context, - void *persistent_data, - const psa_key_attributes_t *attributes, - psa_key_slot_number_t *slot_number ) +static psa_status_t ram_allocate( psa_drv_se_context_t *context, + void *persistent_data, + const psa_key_attributes_t *attributes, + psa_key_slot_number_t *slot_number ) { ram_slot_usage_t *slot_usage = persistent_data; (void) attributes; @@ -116,8 +128,14 @@ psa_status_t ram_allocate( psa_drv_se_context_t *context, return( PSA_ERROR_INSUFFICIENT_STORAGE ); } + + +/****************************************************************/ +/* Other test helper functions */ +/****************************************************************/ + #define MAX_KEY_ID_FOR_TEST 10 -void psa_purge_storage( void ) +static void psa_purge_storage( void ) { psa_key_id_t id; psa_key_lifetime_t lifetime; From f4ee6628681f7889b2dd82d8feee2c1a8712998b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 24 Jul 2019 13:44:30 +0200 Subject: [PATCH 32/50] SE keys: error out in key creation function that lack support --- library/psa_crypto.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 5fcf0ac866..e508f8f091 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1756,6 +1756,15 @@ psa_status_t psa_copy_key( psa_key_handle_t source_handle, if( status != PSA_SUCCESS ) goto exit; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( driver != NULL ) + { + /* Copying to a secure element is not implemented yet. */ + status = PSA_ERROR_NOT_SUPPORTED; + goto exit; + } +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + status = psa_copy_key_material( source_slot, target_slot ); if( status != PSA_SUCCESS ) goto exit; @@ -4661,6 +4670,13 @@ psa_status_t psa_key_derivation_output_key( const psa_key_attributes_t *attribut psa_key_slot_t *slot = NULL; psa_se_drv_table_entry_t *driver = NULL; status = psa_start_key_creation( attributes, handle, &slot, &driver ); +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( driver != NULL ) + { + /* Deriving a key in a secure element is not implemented yet. */ + status = PSA_ERROR_NOT_SUPPORTED; + } +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ if( status == PSA_SUCCESS ) { status = psa_generate_derived_key_internal( slot, @@ -5692,6 +5708,13 @@ psa_status_t psa_generate_key( const psa_key_attributes_t *attributes, psa_key_slot_t *slot = NULL; psa_se_drv_table_entry_t *driver = NULL; status = psa_start_key_creation( attributes, handle, &slot, &driver ); +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( driver != NULL ) + { + /* Generating a key in a secure element is not implemented yet. */ + status = PSA_ERROR_NOT_SUPPORTED; + } +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ if( status == PSA_SUCCESS ) { status = psa_generate_key_internal( From d1cd766e96e8c4a2d9a0cb04c632f50cae5e04dc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 24 Jul 2019 13:45:02 +0200 Subject: [PATCH 33/50] SE keys: test NOT_SUPPORTED error from generate_key --- .../test_suite_psa_crypto_se_driver_hal.data | 3 ++ ...st_suite_psa_crypto_se_driver_hal.function | 36 +++++++++++++++++++ 2 files changed, 39 insertions(+) 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 e9c0694778..275197f417 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -38,3 +38,6 @@ key_creation_import_export:0:1 SE key import-export, check after restart (slot 3) key_creation_import_export:3:1 + +Generate key: not supported +generate_key_not_supported:PSA_KEY_TYPE_AES:128 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 661fb054f4..38066a34f5 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -288,3 +288,39 @@ exit: psa_purge_storage( ); } /* END_CASE */ + +/* BEGIN_CASE */ +void generate_key_not_supported( int type_arg, int bits_arg ) +{ + psa_key_type_t type = type_arg; + size_t bits = bits_arg; + 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; + + 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( psa_key_slot_number_t ); + key_management.p_allocate = counter_allocate; + + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + PSA_ASSERT( psa_crypto_init( ) ); + + psa_set_key_id( &attributes, id ); + psa_set_key_lifetime( &attributes, lifetime ); + psa_set_key_type( &attributes, type ); + psa_set_key_bits( &attributes, bits ); + TEST_EQUAL( psa_generate_key( &attributes, &handle ), + PSA_ERROR_NOT_SUPPORTED ); + +exit: + PSA_DONE( ); + ram_slots_reset( ); + psa_purge_storage( ); +} +/* END_CASE */ From 105736653ffcfa530fe989b81880acffc2c02441 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 24 Jul 2019 13:45:36 +0200 Subject: [PATCH 34/50] SE keys: test that no function goes crazy Run all functions that take a key handle as input with a key that is in a secure element. All calls are expected to error out one way or another (not permitted by policy, invalid key type, method not implemented in the secure element, ...). The goal of this test is to ensure that nothing bad happens (e.g. invalid pointer dereference). Run with various key types and algorithms to get good coverage. --- .../test_suite_psa_crypto_se_driver_hal.data | 54 ++++ ...st_suite_psa_crypto_se_driver_hal.function | 233 ++++++++++++++++++ 2 files changed, 287 insertions(+) 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 275197f417..6fb65f02a3 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -39,5 +39,59 @@ key_creation_import_export:0:1 SE key import-export, check after restart (slot 3) key_creation_import_export:3:1 +Key creation smoke test: AES-CTR +key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CTR:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: AES-CBC +key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CBC_NO_PADDING:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: AES-CMAC +key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: AES-CCM +key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: AES-GCM +key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_GCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: CAMELLIA-CTR +key_creation_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_CTR:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: CAMELLIA-CBC +key_creation_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_CBC_NO_PADDING:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: CAMELLIA-CMAC +key_creation_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_CMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: CAMELLIA-CCM +key_creation_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_GCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: CAMELLIA-CCM +key_creation_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_GCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: HMAC-SHA-256 +key_creation_smoke:PSA_KEY_TYPE_HMAC:PSA_ALG_HMAC( PSA_ALG_SHA_256 ):"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: HKDF-SHA-256 +key_creation_smoke:PSA_KEY_TYPE_DERIVE:PSA_ALG_HKDF( PSA_ALG_SHA_256 ):"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + +Key creation smoke test: RSA PKCS#1v1.5 signature +key_creation_smoke:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_ALG_RSA_PKCS1V15_SIGN_RAW:"30818902818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc30203010001" + +Key creation smoke test: RSA PKCS#1v1.5 encryption +key_creation_smoke:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_ALG_RSA_PKCS1V15_CRYPT:"30818902818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc30203010001" + +Key creation smoke test: RSA OAEP encryption +key_creation_smoke:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_ALG_RSA_OAEP( PSA_ALG_SHA_256 ):"30818902818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc30203010001" + +Key creation smoke test: ECDSA secp256r1 +key_creation_smoke:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee" + +Key creation smoke test: ECDH secp256r1 +key_creation_smoke:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDH:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee" + +Key creation smoke test: ECDH secp256r1 with HKDF +key_creation_smoke:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_KEY_AGREEMENT( PSA_ALG_ECDH, PSA_ALG_HKDF( PSA_ALG_SHA_256 ) ):"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee" + Generate key: not supported generate_key_not_supported:PSA_KEY_TYPE_AES:128 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 38066a34f5..e0b8d29a57 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -33,6 +33,50 @@ +/****************************************************************/ +/* Miscellaneous driver methods */ +/****************************************************************/ + +/* Allocate slot numbers with a monotonic counter. */ +static psa_status_t counter_allocate( psa_drv_se_context_t *context, + void *persistent_data, + const psa_key_attributes_t *attributes, + psa_key_slot_number_t *slot_number ) +{ + psa_key_slot_number_t *p_counter = persistent_data; + (void) attributes; + if( context->persistent_data_size != sizeof( psa_key_slot_number_t ) ) + return( PSA_ERROR_DETECTED_BY_DRIVER ); + ++*p_counter; + if( *p_counter == 0 ) + return( PSA_ERROR_INSUFFICIENT_STORAGE ); + *slot_number = *p_counter; + return( PSA_SUCCESS ); +} + +/* Null import: do nothing, but pretend it worked. */ +static psa_status_t null_import( psa_drv_se_context_t *context, + psa_key_slot_number_t slot_number, + psa_key_lifetime_t lifetime, + psa_key_type_t type, + psa_algorithm_t algorithm, + psa_key_usage_t usage, + const uint8_t *p_data, + size_t data_length ) +{ + (void) context; + (void) slot_number; + (void) lifetime; + (void) type; + (void) algorithm; + (void) usage; + (void) p_data; + (void) data_length; + return( PSA_SUCCESS ); +} + + + /****************************************************************/ /* RAM-based test driver */ /****************************************************************/ @@ -134,6 +178,136 @@ static psa_status_t ram_allocate( psa_drv_se_context_t *context, /* Other test helper functions */ /****************************************************************/ +/* Check that a function's return status is "smoke-free", i.e. that + * it's an acceptable error code when calling an API function that operates + * on a key with potentially bogus parameters. */ +static int is_status_smoke_free( psa_status_t status ) +{ + switch( status ) + { + case PSA_SUCCESS: + case PSA_ERROR_NOT_SUPPORTED: + case PSA_ERROR_NOT_PERMITTED: + case PSA_ERROR_BUFFER_TOO_SMALL: + case PSA_ERROR_INVALID_ARGUMENT: + case PSA_ERROR_INVALID_SIGNATURE: + case PSA_ERROR_INVALID_PADDING: + return( 1 ); + default: + return( 0 ); + } +} +#define SMOKE_ASSERT( expr ) \ + TEST_ASSERT( is_status_smoke_free( expr ) ) + +/* Smoke test a key. There are mostly no wrong answers here since we pass + * mostly bogus parameters: the goal is to ensure that there is no memory + * corruption or crash. This test function is most useful when run under + * an environment with sanity checks such as ASan or MSan. */ +static int smoke_test_key( psa_key_handle_t handle ) +{ + int ok = 0; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_mac_operation_t mac_operation = PSA_MAC_OPERATION_INIT; + psa_cipher_operation_t cipher_operation = PSA_CIPHER_OPERATION_INIT; + psa_key_derivation_operation_t derivation_operation = + PSA_KEY_DERIVATION_OPERATION_INIT; + uint8_t buffer[80]; /* large enough for a public key for ECDH */ + size_t length; + psa_key_handle_t handle2 = 0; + + SMOKE_ASSERT( psa_get_key_attributes( handle, &attributes ) ); + + SMOKE_ASSERT( psa_export_key( handle, + buffer, sizeof( buffer ), &length ) ); + SMOKE_ASSERT( psa_export_public_key( handle, + buffer, sizeof( buffer ), &length ) ); + + SMOKE_ASSERT( psa_copy_key( handle, &attributes, &handle2 ) ); + if( handle2 != 0 ) + PSA_ASSERT( psa_close_key( handle2 ) ); + + SMOKE_ASSERT( psa_mac_sign_setup( &mac_operation, handle, PSA_ALG_CMAC ) ); + PSA_ASSERT( psa_mac_abort( &mac_operation ) ); + SMOKE_ASSERT( psa_mac_verify_setup( &mac_operation, handle, + PSA_ALG_HMAC( PSA_ALG_SHA_256 ) ) ); + PSA_ASSERT( psa_mac_abort( &mac_operation ) ); + + SMOKE_ASSERT( psa_cipher_encrypt_setup( &cipher_operation, handle, + PSA_ALG_CTR ) ); + PSA_ASSERT( psa_cipher_abort( &cipher_operation ) ); + SMOKE_ASSERT( psa_cipher_decrypt_setup( &cipher_operation, handle, + PSA_ALG_CTR ) ); + PSA_ASSERT( psa_cipher_abort( &cipher_operation ) ); + + SMOKE_ASSERT( psa_aead_encrypt( handle, PSA_ALG_CCM, + buffer, sizeof( buffer ), + NULL, 0, + buffer, sizeof( buffer), + buffer, sizeof( buffer), &length ) ); + SMOKE_ASSERT( psa_aead_decrypt( handle, PSA_ALG_CCM, + buffer, sizeof( buffer ), + NULL, 0, + buffer, sizeof( buffer), + buffer, sizeof( buffer), &length ) ); + + SMOKE_ASSERT( psa_asymmetric_sign( handle, PSA_ALG_ECDSA_ANY, + buffer, 32, + buffer, sizeof( buffer ), &length ) ); + SMOKE_ASSERT( psa_asymmetric_verify( handle, PSA_ALG_ECDSA_ANY, + buffer, 32, + buffer, sizeof( buffer ) ) ); + + SMOKE_ASSERT( psa_asymmetric_encrypt( handle, PSA_ALG_RSA_PKCS1V15_CRYPT, + buffer, 10, NULL, 0, + buffer, sizeof( buffer ), &length ) ); + SMOKE_ASSERT( psa_asymmetric_decrypt( handle, PSA_ALG_RSA_PKCS1V15_CRYPT, + buffer, sizeof( buffer ), NULL, 0, + buffer, sizeof( buffer ), &length ) ); + +#if defined(MBEDTLS_SHA256_C) + /* Try the key in a plain key derivation. */ + PSA_ASSERT( psa_key_derivation_setup( &derivation_operation, + PSA_ALG_HKDF( PSA_ALG_SHA_256 ) ) ); + PSA_ASSERT( psa_key_derivation_input_bytes( &derivation_operation, + PSA_KEY_DERIVATION_INPUT_SALT, + NULL, 0 ) ); + SMOKE_ASSERT( psa_key_derivation_input_key( &derivation_operation, + PSA_KEY_DERIVATION_INPUT_SECRET, + handle ) ); + PSA_ASSERT( psa_key_derivation_abort( &derivation_operation ) ); + + /* If the key is asymmetric, try it in a key agreement, both as + * part of a derivation operation and standalone. */ + if( psa_export_public_key( handle, buffer, sizeof( buffer ), &length ) == + PSA_SUCCESS ) + { + psa_algorithm_t alg = + PSA_ALG_KEY_AGREEMENT( PSA_ALG_ECDH, + PSA_ALG_HKDF( PSA_ALG_SHA_256 ) ); + PSA_ASSERT( psa_key_derivation_setup( &derivation_operation, alg ) ); + PSA_ASSERT( psa_key_derivation_input_bytes( + &derivation_operation, PSA_KEY_DERIVATION_INPUT_SALT, + NULL, 0 ) ); + SMOKE_ASSERT( psa_key_derivation_key_agreement( + &derivation_operation, + PSA_KEY_DERIVATION_INPUT_SECRET, + handle, buffer, length ) ); + PSA_ASSERT( psa_key_derivation_abort( &derivation_operation ) ); + + SMOKE_ASSERT( psa_raw_key_agreement( + alg, handle, buffer, length, + buffer, sizeof( buffer ), &length ) ); + } +#endif /* MBEDTLS_SHA256_C */ + + ok = 1; + +exit: + psa_reset_key_attributes( &attributes ); + return( ok ); +} + #define MAX_KEY_ID_FOR_TEST 10 static void psa_purge_storage( void ) { @@ -289,6 +463,65 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void key_creation_smoke( int type_arg, int alg_arg, + data_t *key_material ) +{ + psa_key_type_t type = type_arg; + psa_algorithm_t alg = alg_arg; + 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; + + 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( psa_key_slot_number_t ); + key_management.p_allocate = counter_allocate; + key_management.p_import = null_import; + + 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_SIGN | PSA_KEY_USAGE_VERIFY | + PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT | + PSA_KEY_USAGE_EXPORT ); + psa_set_key_algorithm( &attributes, alg ); + psa_set_key_type( &attributes, type ); + PSA_ASSERT( psa_import_key( &attributes, + key_material->x, key_material->len, + &handle ) ); + + /* Do stuff with the key. */ + if( ! smoke_test_key( handle ) ) + goto exit; + + /* Restart and try again. */ + mbedtls_psa_crypto_free( ); + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + PSA_ASSERT( psa_crypto_init( ) ); + PSA_ASSERT( psa_open_key( id, &handle ) ); + if( ! smoke_test_key( handle ) ) + goto exit; + + /* We're done. */ + PSA_ASSERT( psa_destroy_key( handle ) ); + +exit: + PSA_DONE( ); + ram_slots_reset( ); + psa_purge_storage( ); +} +/* END_CASE */ + /* BEGIN_CASE */ void generate_key_not_supported( int type_arg, int bits_arg ) { From d0e66b00fbf588b4fee72df7ee3f226b2a431163 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 24 Jul 2019 13:52:51 +0200 Subject: [PATCH 35/50] Turn off secure element support by default Secure element support is not yet usable in the real world. Only part of the feature is implemented and the part that's implemented is not sufficient for real-world uses. A lot of error handling is missing, and there are no tests. This commit should be reverted once the feature has stabilized. --- include/mbedtls/config.h | 5 ++++- scripts/config.pl | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 0e8d7550e1..bd6f7b6a03 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1715,12 +1715,15 @@ * Enable secure element support in the Platform Security Architecture * cryptography API. * + * \warning This feature is not yet suitable for production. It is provided + * for API evaluation and testing purposes only. + * * Module: library/psa_crypto_se.c * * Requires: MBEDTLS_PSA_CRYPTO_C, MBEDTLS_PSA_CRYPTO_STORAGE_C * */ -#define MBEDTLS_PSA_CRYPTO_SE_C +//#define MBEDTLS_PSA_CRYPTO_SE_C /** * \def MBEDTLS_PSA_CRYPTO_STORAGE_C diff --git a/scripts/config.pl b/scripts/config.pl index 05cc52e648..6479c6d53b 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -85,6 +85,7 @@ MBEDTLS_NO_PLATFORM_ENTROPY MBEDTLS_RSA_NO_CRT MBEDTLS_NO_UDBL_DIVISION MBEDTLS_NO_64BIT_MULTIPLICATION +MBEDTLS_PSA_CRYPTO_SE_C MBEDTLS_PSA_CRYPTO_SPM MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER MBEDTLS_PSA_INJECT_ENTROPY From f96aefe3ad2fc3bfb0165a6b49c66cde25ec555e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 24 Jul 2019 14:58:38 +0200 Subject: [PATCH 36/50] Test with secure element support Test with default config + SE with Clang and with full config + SE with GCC, for variety. Full+Clang+Asan has known issues so don't do that. --- tests/scripts/all.sh | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index c1e1ffe24b..28225899fc 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -783,6 +783,24 @@ component_test_aes_fewer_tables_and_rom_tables () { make test } +component_test_se_default () { + msg "build: default config + MBEDTLS_PSA_CRYPTO_SE_C" + scripts/config.pl set MBEDTLS_PSA_CRYPTO_SE_C + make CC=clang CFLAGS='-Werror -Wall -Wextra -Wno-unused-function -Os -fsanitize=address' LDFLAGS='-fsanitize=address' + + msg "test: default config + MBEDTLS_PSA_CRYPTO_SE_C" + make test +} + +component_test_se_full () { + msg "build: full config + MBEDTLS_PSA_CRYPTO_SE_C" + scripts/config.pl set MBEDTLS_PSA_CRYPTO_SE_C + make CC=gcc CFLAGS='-Werror -Wall -Wextra -O2 -fsanitize=address' LDFLAGS='-fsanitize=address' + + msg "test: full config + MBEDTLS_PSA_CRYPTO_SE_C" + make test +} + component_test_make_shared () { msg "build/test: make shared" # ~ 40s make SHARED=1 all check From 75c126b958295be1c45257b4e4bba86b7924c8db Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 24 Jul 2019 15:56:01 +0200 Subject: [PATCH 37/50] Explain some non-obvious parts of the code Comment changes only. --- library/psa_crypto_se.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index 714a03904b..648022aed0 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -81,6 +81,10 @@ psa_se_drv_table_entry_t *psa_get_se_driver_entry( psa_key_lifetime_t lifetime ) { size_t i; + /* In the driver table, lifetime=0 means an entry that isn't used. + * No driver has a lifetime of 0 because it's a reserved value + * (which designates volatile keys). Make sure we never return + * a driver entry for lifetime 0. */ if( lifetime == 0 ) return( NULL ); for( i = 0; i < PSA_MAX_SE_DRIVERS; i++ ) @@ -134,6 +138,7 @@ static psa_status_t psa_get_se_driver_its_file_uid( return( PSA_ERROR_NOT_SUPPORTED ); #endif + /* See the documentation of PSA_CRYPTO_SE_DRIVER_ITS_UID_BASE. */ *uid = PSA_CRYPTO_SE_DRIVER_ITS_UID_BASE + driver->lifetime; return( PSA_SUCCESS ); } @@ -148,6 +153,9 @@ psa_status_t psa_load_se_persistent_data( if( status != PSA_SUCCESS ) return( status ); + /* psa_get_se_driver_its_file_uid ensures that the size_t + * persistent_data_size is in range, but compilers don't know that, + * so cast to reassure them. */ return( psa_its_get( uid, 0, (uint32_t) driver->internal.persistent_data_size, driver->internal.persistent_data ) ); @@ -163,6 +171,9 @@ psa_status_t psa_save_se_persistent_data( if( status != PSA_SUCCESS ) return( status ); + /* psa_get_se_driver_its_file_uid ensures that the size_t + * persistent_data_size is in range, but compilers don't know that, + * so cast to reassure them. */ return( psa_its_set( uid, (uint32_t) driver->internal.persistent_data_size, driver->internal.persistent_data, From 4b734223180a81751cbd189f430803464db37cd9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 24 Jul 2019 15:56:31 +0200 Subject: [PATCH 38/50] Transaction support: be more future-proof If there's ever a non-SE-related transaction, make sure it gets handled during init. --- library/psa_crypto.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index e508f8f091..f175fc2d06 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5800,7 +5800,7 @@ psa_status_t psa_crypto_init( void ) if( status != PSA_SUCCESS ) goto exit; -#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +#if defined(PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS) status = psa_crypto_load_transaction( ); if( status == PSA_SUCCESS ) { @@ -5811,7 +5811,7 @@ psa_status_t psa_crypto_init( void ) /* There's no transaction to complete. It's all good. */ status = PSA_SUCCESS; } -#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ +#endif /* PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS */ /* All done. */ global_data.initialized = 1; From f77a6acf83875b02a489169ca6164c364983e5a5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 10:51:03 +0200 Subject: [PATCH 39/50] Fix indentation --- library/psa_crypto.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index f175fc2d06..8595a0f9a7 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1738,7 +1738,7 @@ psa_status_t psa_copy_key( psa_key_handle_t source_handle, psa_se_drv_table_entry_t *driver = NULL; status = psa_get_transparent_key( source_handle, &source_slot, - PSA_KEY_USAGE_COPY, 0 ); + PSA_KEY_USAGE_COPY, 0 ); if( status != PSA_SUCCESS ) goto exit; @@ -5332,8 +5332,8 @@ psa_status_t psa_key_derivation_input_key( psa_key_slot_t *slot; psa_status_t status; status = psa_get_transparent_key( handle, &slot, - PSA_KEY_USAGE_DERIVE, - operation->alg ); + PSA_KEY_USAGE_DERIVE, + operation->alg ); if( status != PSA_SUCCESS ) return( status ); if( slot->type != PSA_KEY_TYPE_DERIVE ) @@ -5481,7 +5481,7 @@ psa_status_t psa_key_derivation_key_agreement( psa_key_derivation_operation_t *o if( ! PSA_ALG_IS_KEY_AGREEMENT( operation->alg ) ) return( PSA_ERROR_INVALID_ARGUMENT ); status = psa_get_transparent_key( private_key, &slot, - PSA_KEY_USAGE_DERIVE, operation->alg ); + PSA_KEY_USAGE_DERIVE, operation->alg ); if( status != PSA_SUCCESS ) return( status ); status = psa_key_agreement_internal( operation, step, @@ -5509,7 +5509,7 @@ psa_status_t psa_raw_key_agreement( psa_algorithm_t alg, goto exit; } status = psa_get_transparent_key( private_key, &slot, - PSA_KEY_USAGE_DERIVE, alg ); + PSA_KEY_USAGE_DERIVE, alg ); if( status != PSA_SUCCESS ) goto exit; From 6a3dd89a64daaba135ecc9b304c099f7c89d2768 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 10:56:39 +0200 Subject: [PATCH 40/50] Improve alignment in comments --- include/psa/crypto_se_driver.h | 129 +++++++++++++++++---------------- 1 file changed, 65 insertions(+), 64 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 57d077c2e8..60447ce3b1 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -59,7 +59,7 @@ typedef struct { * session to the next. * * The core allocates a memory buffer for the persistent data. - * The pointer is guaranteed to be suitably alignedfor any data type, + * The pointer is guaranteed to be suitably aligned for any data type, * like a pointer returned by `malloc` (but the core can use any * method to allocate the buffer, not necessarily `malloc`). * @@ -164,7 +164,7 @@ typedef psa_status_t (*psa_drv_se_mac_setup_t)(psa_drv_se_context_t *drv_context * updated * \param[in] p_input A buffer containing the message to be appended * to the MAC operation - * \param[in] input_length The size in bytes of the input message buffer + * \param[in] input_length The size in bytes of the input message buffer */ typedef psa_status_t (*psa_drv_se_mac_update_t)(void *op_context, const uint8_t *p_input, @@ -195,10 +195,10 @@ typedef psa_status_t (*psa_drv_se_mac_finish_t)(void *op_context, * operation by comparing the resulting MAC against a provided value * * \param[in,out] op_context A hardware-specific structure for the previously - * started MAC operation to be fiinished - * \param[in] p_mac The MAC value against which the resulting MAC will - * be compared against - * \param[in] mac_length The size in bytes of the value stored in `p_mac` + * started MAC operation to be fiinished + * \param[in] p_mac The MAC value against which the resulting MAC + * will be compared against + * \param[in] mac_length The size in bytes of the value stored in `p_mac` * * \retval PSA_SUCCESS * The operation completed successfully and the MACs matched each @@ -215,14 +215,14 @@ typedef psa_status_t (*psa_drv_se_mac_finish_verify_t)(void *op_context, * operation * * \param[in,out] op_context A hardware-specific structure for the previously - * started MAC operation to be aborted + * started MAC operation to be aborted */ typedef psa_status_t (*psa_drv_se_mac_abort_t)(void *op_context); /** \brief A function that performs a secure element MAC operation in one * command and returns the calculated MAC * - * \param[in,out] drv_context The driver context structure. + * \param[in,out] drv_context The driver context structure. * \param[in] p_input A buffer containing the message to be MACed * \param[in] input_length The size in bytes of `p_input` * \param[in] key_slot The slot of the key to be used @@ -344,7 +344,7 @@ typedef struct { /** \brief A function that provides the cipher setup function for a * secure element driver * - * \param[in,out] drv_context The driver context structure. + * \param[in,out] drv_context The driver context structure. * \param[in,out] op_context A structure that will contain the * hardware-specific cipher context. * \param[in] key_slot The slot of the key to be used for the @@ -440,19 +440,19 @@ typedef psa_status_t (*psa_drv_se_cipher_abort_t)(void *op_context); * Note: this function should only be used with implementations that do not * provide a needed higher-level operation. * - * \param[in,out] drv_context The driver context structure. - * \param[in] key_slot The slot of the key to be used for the operation - * \param[in] algorithm The algorithm to be used in the cipher operation - * \param[in] direction Indicates whether the operation is an encrypt or - * decrypt - * \param[in] p_input A buffer containing the data to be - * encrypted/decrypted - * \param[in] input_size The size in bytes of the buffer pointed to by - * `p_input` - * \param[out] p_output The caller-allocated buffer where the output will - * be placed - * \param[in] output_size The allocated size in bytes of the `p_output` - * buffer + * \param[in,out] drv_context The driver context structure. + * \param[in] key_slot The slot of the key to be used for the operation + * \param[in] algorithm The algorithm to be used in the cipher operation + * \param[in] direction Indicates whether the operation is an encrypt or + * decrypt + * \param[in] p_input A buffer containing the data to be + * encrypted/decrypted + * \param[in] input_size The size in bytes of the buffer pointed to by + * `p_input` + * \param[out] p_output The caller-allocated buffer where the output + * will be placed + * \param[in] output_size The allocated size in bytes of the `p_output` + * buffer * * \retval PSA_SUCCESS * \retval PSA_ERROR_NOT_SUPPORTED @@ -538,7 +538,7 @@ typedef psa_status_t (*psa_drv_se_asymmetric_sign_t)(psa_drv_se_context_t *drv_c * \brief A function that verifies the signature a hash or short message using * an asymmetric public key in a secure element * - * \param[in,out] drv_context The driver context structure. + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Key slot of a public key or an asymmetric key * pair * \param[in] alg A signature algorithm that is compatible with @@ -563,7 +563,7 @@ typedef psa_status_t (*psa_drv_se_asymmetric_verify_t)(psa_drv_se_context_t *drv * \brief A function that encrypts a short message with an asymmetric public * key in a secure element * - * \param[in,out] drv_context The driver context structure. + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Key slot of a public key or an asymmetric key * pair * \param[in] alg An asymmetric encryption algorithm that is @@ -604,7 +604,7 @@ typedef psa_status_t (*psa_drv_se_asymmetric_encrypt_t)(psa_drv_se_context_t *dr * \brief A function that decrypts a short message with an asymmetric private * key in a secure element. * - * \param[in,out] drv_context The driver context structure. + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Key slot of an asymmetric key pair * \param[in] alg An asymmetric encryption algorithm that is * compatible with the type of `key` @@ -674,7 +674,7 @@ typedef struct { /** \brief A function that performs a secure element authenticated encryption * operation * - * \param[in,out] drv_context The driver context structure. + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Slot containing the key to use. * \param[in] algorithm The AEAD algorithm to compute * (\c PSA_ALG_XXX value such that @@ -717,7 +717,7 @@ typedef psa_status_t (*psa_drv_se_aead_encrypt_t)(psa_drv_se_context_t *drv_cont /** A function that peforms a secure element authenticated decryption operation * - * \param[in,out] drv_context The driver context structure. + * \param[in,out] drv_context The driver context structure. * \param[in] key_slot Slot containing the key to use * \param[in] algorithm The AEAD algorithm to compute * (\c PSA_ALG_XXX value such that @@ -787,10 +787,10 @@ typedef struct { * \param[in,out] drv_context The driver context structure. * \param[in,out] persistent_data A pointer to the persistent data * that allows writing. - * \param[in] attributes Attributes of the key. - * \param[out] key_slot Slot where the key will be stored. - * This must be a valid slot for a key of the - * chosen type. It must be unoccupied. + * \param[in] attributes Attributes of the key. + * \param[out] key_slot Slot where the key will be stored. + * This must be a valid slot for a key of the + * chosen type. It must be unoccupied. * * \retval #PSA_SUCCESS * Success. @@ -810,16 +810,16 @@ typedef psa_status_t (*psa_drv_se_allocate_key_t)( * This function can support any output from psa_export_key(). Refer to the * documentation of psa_export_key() for the format for each key type. * - * \param[in,out] drv_context The driver context structure. - * \param[in] key_slot Slot where the key will be stored - * This must be a valid slot for a key of the chosen - * type. It must be unoccupied. - * \param[in] lifetime The required lifetime of the key storage - * \param[in] type Key type (a \c PSA_KEY_TYPE_XXX value) - * \param[in] algorithm Key algorithm (a \c PSA_ALG_XXX value) - * \param[in] usage The allowed uses of the key - * \param[in] p_data Buffer containing the key data - * \param[in] data_length Size of the `data` buffer in bytes + * \param[in,out] drv_context The driver context structure. + * \param[in] key_slot Slot where the key will be stored + * This must be a valid slot for a key of the chosen + * type. It must be unoccupied. + * \param[in] lifetime The required lifetime of the key storage + * \param[in] type Key type (a \c PSA_KEY_TYPE_XXX value) + * \param[in] algorithm Key algorithm (a \c PSA_ALG_XXX value) + * \param[in] usage The allowed uses of the key + * \param[in] p_data Buffer containing the key data + * \param[in] data_length Size of the `data` buffer in bytes * * \retval #PSA_SUCCESS * Success. @@ -846,7 +846,7 @@ typedef psa_status_t (*psa_drv_se_import_key_t)(psa_drv_se_context_t *drv_contex * \param[in,out] drv_context The driver context structure. * \param[in,out] persistent_data A pointer to the persistent data * that allows writing. - * \param key_slot The key slot to erase. + * \param key_slot The key slot to erase. * * \retval #PSA_SUCCESS * The slot's content, if any, has been erased. @@ -871,7 +871,7 @@ typedef psa_status_t (*psa_drv_se_destroy_key_t)( * `psa_export_key()` does. Refer to the * documentation of `psa_export_key()` for the format for each key type. * - * \param[in,out] drv_context The driver context structure. + * \param[in,out] drv_context The driver context structure. * \param[in] key Slot whose content is to be exported. This must * be an occupied key slot. * \param[out] p_data Buffer where the key data is to be written. @@ -902,22 +902,23 @@ typedef psa_status_t (*psa_drv_se_export_key_t)(psa_drv_se_context_t *drv_contex * The format of the public key information will match the format specified for * the psa_export_key() function for the key type. * - * \param[in,out] drv_context The driver context structure. - * \param[in] key_slot Slot where the generated key will be placed - * \param[in] type The type of the key to be generated - * \param[in] usage The prescribed usage of the generated key - * Note: Not all Secure Elements support the same - * restrictions that PSA Crypto does (and vice versa). - * Driver developers should endeavor to match the - * usages as close as possible. - * \param[in] bits The size in bits of the key to be generated. - * \param[in] extra Extra parameters for key generation. The - * interpretation of this parameter should match the - * interpretation in the `extra` parameter is the - * `psa_generate_key` function - * \param[in] extra_size The size in bytes of the \p extra buffer - * \param[out] p_pubkey_out The buffer where the public key information will - * be placed + * \param[in,out] drv_context The driver context structure. + * \param[in] key_slot Slot where the generated key will be placed + * \param[in] type The type of the key to be generated + * \param[in] usage The prescribed usage of the generated key + * Note: Not all Secure Elements support the same + * restrictions that PSA Crypto does (and vice + * versa). + * Driver developers should endeavor to match the + * usages as close as possible. + * \param[in] bits The size in bits of the key to be generated. + * \param[in] extra Extra parameters for key generation. The + * interpretation of this parameter should match + * the interpretation in the `extra` parameter is + * the `psa_generate_key` function + * \param[in] extra_size The size in bytes of the \p extra buffer + * \param[out] p_pubkey_out The buffer where the public key information will + * be placed * \param[in] pubkey_out_size The size in bytes of the `p_pubkey_out` buffer * \param[out] p_pubkey_length Upon successful completion, will contain the * size of the data placed in `p_pubkey_out`. @@ -1011,12 +1012,12 @@ typedef struct { /** \brief A function that Sets up a secure element key derivation operation by * specifying the algorithm and the source key sot * - * \param[in,out] drv_context The driver context structure. + * \param[in,out] drv_context The driver context structure. * \param[in,out] op_context A hardware-specific structure containing any - * context information for the implementation - * \param[in] kdf_alg The algorithm to be used for the key derivation - * \param[in] source_key The key to be used as the source material for the - * key derivation + * context information for the implementation + * \param[in] kdf_alg The algorithm to be used for the key derivation + * \param[in] source_key The key to be used as the source material for + * the key derivation * * \retval PSA_SUCCESS */ From adad813d7b5b780c59c5049b97e4cffdf26a578c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 11:31:23 +0200 Subject: [PATCH 41/50] psa_key_slot_is_external exists. Use it. --- library/psa_crypto.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 8595a0f9a7..92364ca4cd 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -888,10 +888,7 @@ static psa_status_t psa_get_transparent_key( psa_key_handle_t handle, psa_status_t status = psa_get_key_from_slot( handle, p_slot, usage, alg ); if( status != PSA_SUCCESS ) return( status ); - /* Use a simple, cheap test to check whether the key is transparent. - * This check assumes that there are no persistent lifetimes other than - * PSA_KEY_LIFETIME_PERSISTENT. */ - if( ( *p_slot )->lifetime > PSA_KEY_LIFETIME_PERSISTENT ) + if( psa_key_slot_is_external( *p_slot ) ) { *p_slot = NULL; return( PSA_ERROR_NOT_SUPPORTED ); From 725f22a545c12b135bbd68ca42f4cefae40baf88 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 11:31:48 +0200 Subject: [PATCH 42/50] Bug fix: save the driver's persistent data in destroy_key --- library/psa_crypto.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 92364ca4cd..eefb26116c 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1007,7 +1007,11 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle ) #if defined(MBEDTLS_PSA_CRYPTO_SE_C) if( driver != NULL ) { - status = psa_crypto_stop_transaction( ); + psa_status_t status2; + status = psa_save_se_persistent_data( driver ); + status2 = psa_crypto_stop_transaction( ); + if( status == PSA_SUCCESS ) + status = status2; if( status != PSA_SUCCESS ) { /* TOnogrepDO: destroy what can be destroyed anyway */ From 60450a4812d6e72e9140f8de0acf86f299524652 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 11:32:45 +0200 Subject: [PATCH 43/50] Improve comments --- library/psa_crypto.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index eefb26116c..84070c1cc2 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -876,8 +876,8 @@ static psa_status_t psa_get_key_from_slot( psa_key_handle_t handle, * A transparent key is a key for which the key material is directly * available, as opposed to a key in a secure element. * - * This is a temporary function until secure element support is - * fully implemented. + * This is a temporary function to use instead of psa_get_key_from_slot() + * until secure element support is fully implemented. */ #if defined(MBEDTLS_PSA_CRYPTO_SE_C) static psa_status_t psa_get_transparent_key( psa_key_handle_t handle, @@ -981,6 +981,11 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle ) driver = psa_get_se_driver_entry( slot->lifetime ); if( driver != NULL ) { + /* For a key in a secure element, we need to do three things: + * remove the key file in internal storage, destroy the + * key inside the secure element, and update the driver's + * persistent data. Start a transaction that will encompass these + * three actions. */ psa_crypto_prepare_transaction( PSA_CRYPTO_TRANSACTION_DESTROY_KEY ); psa_crypto_transaction.key.lifetime = slot->lifetime; psa_crypto_transaction.key.slot = slot->data.se.slot_number; @@ -1454,9 +1459,18 @@ static psa_status_t psa_start_key_creation( slot->type = attributes->type; #if defined(MBEDTLS_PSA_CRYPTO_SE_C) - /* Find a slot number for the new key. Save the slot number in - * persistent storage, but do not yet save the driver's persistent - * state, so that if the power fails during the key creation process, + /* For a key in a secure element, we need to do three things: + * create the key file in internal storage, create the + * key inside the secure element, and update the driver's + * persistent data. Start a transaction that will encompass these + * three actions. */ + /* The first thing to do is to find a slot number for the new key. + * We save the slot number in persistent storage as part of the + * transaction data. It will be needed to recover if the power + * fails during the key creation process, to clean up on the secure + * element side after restarting. Obtaining a slot number from the + * secure element driver updates its persistent state, but we do not yet + * save the driver's persistent state, so that if the power fails, * we can roll back to a state where the key doesn't exist. */ if( *p_drv != NULL ) { From 2e0f388d2afcaaa171c996ab3301e3cbb52ff85d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 11:34:33 +0200 Subject: [PATCH 44/50] Don't explicitly dereference function pointers Be stylistically consistent. --- library/psa_crypto.c | 6 +++--- library/psa_crypto_se.c | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 84070c1cc2..4bd2d13d26 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1231,9 +1231,9 @@ static psa_status_t psa_internal_export_key( const psa_key_slot_t *slot, drv->key_management->p_export ); if( method == NULL ) return( PSA_ERROR_NOT_SUPPORTED ); - return( ( *method )( drv_context, - slot->data.se.slot_number, - data, data_size, data_length ) ); + return( method( drv_context, + slot->data.se.slot_number, + data, data_size, data_length ) ); } #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index 648022aed0..e6dbe3241b 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -211,10 +211,10 @@ psa_status_t psa_find_se_slot_for_key( if( p_allocate == NULL ) return( PSA_ERROR_NOT_SUPPORTED ); - status = ( *p_allocate )( &driver->context, - driver->internal.persistent_data, - attributes, - slot_number ); + status = p_allocate( &driver->context, + driver->internal.persistent_data, + attributes, + slot_number ); return( status ); } From 0c3ae1f0b4b75f343dfb17af62c615490dce07c3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 14:04:38 +0200 Subject: [PATCH 45/50] Improve documentation of SE driver persistent state Explain what it can be used for and when it is saved to storage. --- include/psa/crypto_se_driver.h | 36 ++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 60447ce3b1..9aebc45c1c 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -55,8 +55,13 @@ extern "C" { typedef struct { /** A read-only pointer to the driver's persistent data. * - * The PSA Cryptography core saves the persistent data from one - * session to the next. + * Drivers typically use this persistent data to keep track of + * which slot numbers are available. This is only a guideline: + * drivers may use the persistent data for any purpose, keeping + * in mind the restrictions on when the persistent data is saved + * to storage: the persistent data is only saved after calling + * certain functions that receive a writable pointer to the + * persistent data. * * The core allocates a memory buffer for the persistent data. * The pointer is guaranteed to be suitably aligned for any data type, @@ -74,7 +79,23 @@ typedef struct { * * This pointer is to read-only data. Only a few driver functions are * allowed to modify the persistent data. These functions receive a - * writable pointer. + * writable pointer. These functions are: + * - psa_drv_se_t::p_init + * - psa_drv_se_key_management_t::p_allocate + * - psa_drv_se_key_management_t::p_destroy + * + * The PSA Cryptography core saves the persistent data from one + * session to the next. It does this before returning from API functions + * that call a driver method that is allowed to modify the persistent + * data, specifically: + * - psa_crypto_init() causes a call to psa_drv_se_t::p_init, and may call + * psa_drv_se_key_management_t::p_destroy to complete an action + * that was interrupted by a power failure. + * - Key creation functions cause a call to + * psa_drv_se_key_management_t::p_allocate, and may cause a call to + * psa_drv_se_key_management_t::p_destroy in case an error occurs. + * - psa_destroy_key() causes a call to + * psa_drv_se_key_management_t::p_destroy. */ const void *const persistent_data; @@ -1118,7 +1139,14 @@ typedef struct { */ uint32_t hal_version; - /** The size of the driver's persistent data in bytes. */ + /** The size of the driver's persistent data in bytes. + * + * This can be 0 if the driver does not need persistent data. + * + * See the documentation of psa_drv_se_context_t::persistent_data + * for more information about why and how a driver can use + * persistent data. + */ size_t persistent_data_size; /** The driver initialization function. From 340b127ed1a697dd97ce9974a3f314820c62af97 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 14:13:24 +0200 Subject: [PATCH 46/50] psa_destroy_se_key: explain why the error is NOT_PERMITTED --- library/psa_crypto_se.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index e6dbe3241b..aece47d01c 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -223,6 +223,14 @@ psa_status_t psa_destroy_se_key( psa_se_drv_table_entry_t *driver, { psa_status_t status; psa_status_t storage_status; + /* Normally a missing method would mean that the action is not + * supported. But psa_destroy_key() is not supposed to return + * PSA_ERROR_NOT_SUPPORTED: if you can create a key, you should + * be able to destroy it. The only use case for a driver that + * does not have a way to destroy keys at all is if the keys are + * locked in a read-only state: we can use the keys but not + * destroy them. Hence, if the driver doesn't support destroying + * keys, it's really a lack of permission. */ if( driver->methods->key_management == NULL || driver->methods->key_management->p_destroy == NULL ) return( PSA_ERROR_NOT_PERMITTED ); From 4aea1036c64ed9b57c30e80f84d9f8301d66435c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 17:38:34 +0200 Subject: [PATCH 47/50] Bug fix: don't start a transaction for non-SE keys --- library/psa_crypto.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 4bd2d13d26..50be99799b 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1478,14 +1478,14 @@ static psa_status_t psa_start_key_creation( &slot->data.se.slot_number ); if( status != PSA_SUCCESS ) return( status ); + psa_crypto_prepare_transaction( PSA_CRYPTO_TRANSACTION_CREATE_KEY ); + psa_crypto_transaction.key.lifetime = slot->lifetime; + psa_crypto_transaction.key.slot = slot->data.se.slot_number; + psa_crypto_transaction.key.id = slot->persistent_storage_id; + status = psa_crypto_save_transaction( ); + if( status != PSA_SUCCESS ) + return( status ); } - psa_crypto_prepare_transaction( PSA_CRYPTO_TRANSACTION_CREATE_KEY ); - psa_crypto_transaction.key.lifetime = slot->lifetime; - psa_crypto_transaction.key.slot = slot->data.se.slot_number; - psa_crypto_transaction.key.id = slot->persistent_storage_id; - status = psa_crypto_save_transaction( ); - if( status != PSA_SUCCESS ) - return( status ); #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ return( status ); From f9bb29ec2628aefc4f5564c20384b59f71871d87 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 17:52:59 +0200 Subject: [PATCH 48/50] Add boilerplate to recover a transaction during init --- library/psa_crypto.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 50be99799b..92c9668d36 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5782,6 +5782,30 @@ void mbedtls_psa_crypto_free( void ) #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ } +#if defined(PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS) +/** Recover a transaction that was interrupted by a power failure. + * + * This function is called during initialization, before psa_crypto_init() + * returns. If this function returns a failure status, the initialization + * fails. + */ +static psa_status_t psa_crypto_recover_transaction( + const psa_crypto_transaction_t *transaction ) +{ + switch( transaction->unknown.type ) + { + case PSA_CRYPTO_TRANSACTION_CREATE_KEY: + case PSA_CRYPTO_TRANSACTION_DESTROY_KEY: + /* TOnogrepDO - fall through to the failure case until this + * is implemented */ + default: + /* We found an unsupported transaction in the storage. + * We don't know what state the storage is in. Give up. */ + return( PSA_ERROR_STORAGE_FAILURE ); + } +} +#endif /* PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS */ + psa_status_t psa_crypto_init( void ) { psa_status_t status; @@ -5819,7 +5843,10 @@ psa_status_t psa_crypto_init( void ) status = psa_crypto_load_transaction( ); if( status == PSA_SUCCESS ) { - /*TOnogrepDO: complete or abort the transaction*/ + status = psa_crypto_recover_transaction( &psa_crypto_transaction ); + if( status != PSA_SUCCESS ) + goto exit; + status = psa_crypto_stop_transaction( ); } else if( status == PSA_ERROR_DOES_NOT_EXIST ) { From 2ea06fd48da7a02dd13aaa25b01c2354f2b8537d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 17:53:16 +0200 Subject: [PATCH 49/50] Improve documentation of transaction storage --- library/psa_crypto_storage.h | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/library/psa_crypto_storage.h b/library/psa_crypto_storage.h index 25049b08de..8fe20ac320 100644 --- a/library/psa_crypto_storage.h +++ b/library/psa_crypto_storage.h @@ -206,6 +206,9 @@ psa_status_t psa_parse_key_data_from_storage( const uint8_t *storage_data, typedef uint16_t psa_crypto_transaction_type_t; /** No transaction is in progress. + * + * This has the value 0, so zero-initialization sets a transaction's type to + * this value. */ #define PSA_CRYPTO_TRANSACTION_NONE ( (psa_crypto_transaction_type_t) 0x0000 ) @@ -244,16 +247,22 @@ typedef uint16_t psa_crypto_transaction_type_t; * -# Fill in the type-specific fields of #psa_crypto_transaction. * -# Call psa_crypto_save_transaction() to start the transaction. This * saves the transaction data to internal storage. + * -# Perform the work of the transaction by modifying files, contacting + * external entities, or whatever needs doing. Note that the transaction + * may be interrupted by a power failure, so you need to have a way + * recover from interruptions either by undoing what has been done + * so far or by resuming where you left off. * -# If there are intermediate stages in the transaction, update * the fields of #psa_crypto_transaction and call * psa_crypto_save_transaction() again when each stage is reached. - * -# When the transaction is over, whether it has been committed or aborted, - * call psa_crypto_stop_transaction() to remove the transaction data in - * storage and in memory. + * -# When the transaction is over, call psa_crypto_stop_transaction() to + * remove the transaction data in storage and in memory. * * If the system crashes while a transaction is in progress, psa_crypto_init() * calls psa_crypto_load_transaction() and takes care of completing or - * rewinding the transaction. + * rewinding the transaction. This is done in psa_crypto_recover_transaction() + * in psa_crypto.c. If you add a new type of transaction, be + * sure to add code for it in psa_crypto_recover_transaction(). */ typedef union { @@ -328,8 +337,10 @@ psa_status_t psa_crypto_load_transaction( void ); /** Indicate that the current transaction is finished. * - * Call this function at the very end of transaction processing, whether - * the transaction has been committed or aborted. + * Call this function at the very end of transaction processing. + * This function does not "commit" or "abort" the transaction: the storage + * subsystem has no concept of "commit" and "abort", just saving and + * removing the transaction information in storage. * * This function erases the transaction data in storage (if any) and * resets the transaction data in memory. From 66be51c35d2e67cdce99fed0bd7636195495afba Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 25 Jul 2019 18:02:52 +0200 Subject: [PATCH 50/50] If starting a transaction fails, wipe the transaction data Nothing has been saved to disk yet, but there is stale data in psa_crypto_transaction. This stale data should not be reused, but do wipe it to reduce the risk of it mattering somehow in the future. --- library/psa_crypto.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 92c9668d36..b2fc26e1be 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -993,6 +993,7 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle ) status = psa_crypto_save_transaction( ); if( status != PSA_SUCCESS ) { + (void) psa_crypto_stop_transaction( ); /* TOnogrepDO: destroy what can be destroyed anyway */ return( status ); } @@ -1484,7 +1485,10 @@ static psa_status_t psa_start_key_creation( psa_crypto_transaction.key.id = slot->persistent_storage_id; status = psa_crypto_save_transaction( ); if( status != PSA_SUCCESS ) + { + (void) psa_crypto_stop_transaction( ); return( status ); + } } #endif /* MBEDTLS_PSA_CRYPTO_SE_C */