From 91e8c33f48a6e36a97e28513f3bdb8007ac7ad5d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 2 Aug 2019 19:19:39 +0200 Subject: [PATCH 1/8] Add infrastructure for key attribute flags Add infrastructure for internal, external and dual-use flags, with a compile-time check (if static_assert is available) to ensure that the same numerical value doesn't get declared for two different purposes in crypto_struct.h (external or dual-use) and psa_crypto_core.h (internal). --- include/psa/crypto_struct.h | 23 ++++++++++++++++++++++- library/psa_crypto.c | 18 ++++++++++++++++++ library/psa_crypto_core.h | 5 +++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index 9e38e53ce..3bace6088 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -322,6 +322,27 @@ typedef uint16_t psa_key_bits_t; * conditionals. */ #define PSA_MAX_KEY_BITS 0xfff8 +/** A mask of flags that can be stored in key attributes. + * + * This type is also used internally to store flags in slots. Internal + * flags are defined in library/psa_crypto_core.h. Internal flags may have + * the same value as external flags if they are properly handled during + * key creation and in psa_get_key_attributes. + */ +typedef uint16_t psa_key_attributes_flag_t; + +#define MBEDLTS_PSA_KA_FLAG_SLOT_NUMBER ( (psa_key_attributes_flag_t) 0x0001 ) + +/* A mask of key attribute flags used externally only. + * Only meant for internal checks inside the library. */ +#define MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ( \ + 0 ) + +/* A mask of key attribute flags used both internally and externally. + * Currently there aren't any. */ +#define MBEDTLS_PSA_KA_MASK_DUAL_USE ( \ + 0 ) + typedef struct { psa_key_type_t type; @@ -329,7 +350,7 @@ typedef struct psa_key_id_t id; psa_key_policy_t policy; psa_key_bits_t bits; - uint16_t flags; + psa_key_attributes_flag_t flags; } psa_core_key_attributes_t; #define PSA_CORE_KEY_ATTRIBUTES_INIT {0, 0, 0, {0, 0, 0}, 0, 0} diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 41289c607..e043d7000 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1408,6 +1408,15 @@ psa_status_t psa_export_public_key( psa_key_handle_t handle, data_length, 1 ) ); } +#if defined(static_assert) +static_assert( ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, + "One or more key attribute flag is listed as both external-only and dual-use" ); +static_assert( ( MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, + "One or more key attribute flag is listed as both external-only and dual-use" ); +static_assert( ( MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ) == 0, + "One or more key attribute flag is listed as both internal-only and external-only" ); +#endif + /** Validate that a key policy is internally well-formed. * * This function only rejects invalid policies. It does not validate the @@ -1467,6 +1476,11 @@ static psa_status_t psa_validate_key_attributes( if( psa_get_key_bits( attributes ) > PSA_MAX_KEY_BITS ) return( PSA_ERROR_NOT_SUPPORTED ); + /* Reject invalid flags. These should not be reachable through the API. */ + if( attributes->core.flags & ~ ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY | + MBEDTLS_PSA_KA_MASK_DUAL_USE ) ) + return( PSA_ERROR_INVALID_ARGUMENT ); + return( PSA_SUCCESS ); } @@ -1523,6 +1537,10 @@ static psa_status_t psa_start_key_creation( slot->attr = attributes->core; + /* Erase external-only flags from the internal copy. To access + * external-only flags, query `attributes`. */ + slot->attr.flags |= ~MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY; + #if defined(MBEDTLS_PSA_CRYPTO_SE_C) /* For a key in a secure element, we need to do three things: * create the key file in internal storage, create the diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index fbfb6daef..e289dbeef 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -64,6 +64,11 @@ typedef struct } data; } psa_key_slot_t; +/* A mask of key attribute flags used only internally. + * Currently there aren't any. */ +#define MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY ( \ + 0 ) + /** Test whether a key slot is occupied. * * A key slot is occupied iff the key type is nonzero. This works because From 74f3352b05a6d1b119fa6325ac1882eedf3860f9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 2 Aug 2019 19:21:49 +0200 Subject: [PATCH 2/8] Add missing guard around a union field --- library/psa_crypto_core.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index e289dbeef..b67c0c576 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -56,11 +56,13 @@ typedef struct /* EC public key or key pair */ mbedtls_ecp_keypair *ecp; #endif /* MBEDTLS_ECP_C */ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) /* Any key type in a secure element */ struct se { psa_key_slot_number_t slot_number; } se; +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ } data; } psa_key_slot_t; From c8000c005aa16da442e716ed89009631e3770c8a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 2 Aug 2019 20:15:51 +0200 Subject: [PATCH 3/8] Add slot_number attribute Add a slot_number field to psa_key_attributes_t and getter/setter functions. Since slot numbers can have the value 0, indicate the presence of the field via a separate flag. In psa_get_key_attributes(), report the slot number if the key is in a secure element. When creating a key, for now, applications cannot choose a slot number. A subsequent commit will add this capability in the secure element HAL. --- include/psa/crypto_extra.h | 61 ++++++++++++++++++++++++++++++++++ include/psa/crypto_se_driver.h | 7 ++++ include/psa/crypto_struct.h | 12 ++++++- include/psa/crypto_types.h | 11 ++++++ library/psa_crypto.c | 28 +++++++++++++++- 5 files changed, 117 insertions(+), 2 deletions(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 6dfaa1300..5359b580a 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -104,6 +104,67 @@ static inline psa_algorithm_t psa_get_key_enrollment_algorithm( return( attributes->core.policy.alg2 ); } +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + +/** Retrieve the slot number where a key is stored. + * + * A slot number is only defined for keys that are stored in a secure + * element. + * + * This information is only useful if the secure element is not entirely + * managed through the PSA Cryptography API. It is up to the secure + * element driver to decide how PSA slot numbers map to any other interface + * that the secure element may have. + * + * \param[in] attributes The key attribute structure to query. + * \param[out] slot_number On success, the slot number containing the key. + * + * \retval #PSA_SUCCESS + * The key is located in a secure element, and \p *slot_number + * indicates the slot number that contains it. + * \retval #PSA_ERROR_NOT_PERMITTED + * The caller is not permitted to query the slot number. + * Mbed Crypto currently does not return this error. + * \retval #PSA_ERROR_INVALID_ARGUMENT + * The key is not located in a secure element. + */ +psa_status_t psa_get_key_slot_number( + const psa_key_attributes_t *attributes, + psa_key_slot_number_t *slot_number ); + +/** Choose the slot number where a key is stored. + * + * This function declares a slot number in the specified attribute + * structure. + * + * A slot number is only meaningful for keys that are stored in a secure + * element. It is up to the secure element driver to decide how PSA slot + * numbers map to any other interface that the secure element may have. + * + * \note Setting a slot number in key attributes for a key creation can + * cause the following errors when creating the key: + * - #PSA_ERROR_NOT_SUPPORTED if the selected secure element does + * not support choosing a specific slot number. + * - #PSA_ERROR_NOT_PERMITTED if the caller is not permitted to + * choose slot numbers in general or to choose this specific slot. + * - #PSA_ERROR_INVALID_ARGUMENT if the chosen slot number is not + * valid in general or not valid for this specific key. + * - #PSA_ERROR_ALREADY_EXISTS if there is already a key in the + * selected slot. + * + * \param[out] attributes The attribute structure to write to. + * \param slot_number The slot number to set. + */ +static inline void psa_set_key_slot_number( + psa_key_attributes_t *attributes, + psa_key_slot_number_t slot_number ) +{ + attributes->core.flags |= MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER; + attributes->slot_number = slot_number; +} + +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + /**@}*/ /** diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index f95eaeb33..69cdababa 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -134,10 +134,17 @@ typedef psa_status_t (*psa_drv_se_init_t)(psa_drv_se_context_t *drv_context, void *persistent_data, psa_key_lifetime_t lifetime); +#if defined(__DOXYGEN_ONLY__) || !defined(MBEDTLS_PSA_CRYPTO_SE_C) +/* Mbed Crypto with secure element support enabled defines this type in + * crypto_types.h because it is also visible to applications through an + * implementation-specific extension. + * For the PSA Cryptography specification, this type is only visible + * via crypto_se_driver.h. */ /** 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; +#endif /* __DOXYGEN_ONLY__ || !MBEDTLS_PSA_CRYPTO_SE_C */ /**@}*/ diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index 3bace6088..fbfe77e62 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -331,11 +331,13 @@ typedef uint16_t psa_key_bits_t; */ typedef uint16_t psa_key_attributes_flag_t; -#define MBEDLTS_PSA_KA_FLAG_SLOT_NUMBER ( (psa_key_attributes_flag_t) 0x0001 ) +#define MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER \ + ( (psa_key_attributes_flag_t) 0x0001 ) /* A mask of key attribute flags used externally only. * Only meant for internal checks inside the library. */ #define MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ( \ + MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER | \ 0 ) /* A mask of key attribute flags used both internally and externally. @@ -358,11 +360,19 @@ typedef struct struct psa_key_attributes_s { psa_core_key_attributes_t core; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + psa_key_slot_number_t slot_number; +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ void *domain_parameters; size_t domain_parameters_size; }; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +#define PSA_KEY_ATTRIBUTES_INIT {PSA_CORE_KEY_ATTRIBUTES_INIT, 0, NULL, 0} +#else #define PSA_KEY_ATTRIBUTES_INIT {PSA_CORE_KEY_ATTRIBUTES_INIT, NULL, 0} +#endif + static inline struct psa_key_attributes_s psa_key_attributes_init( void ) { const struct psa_key_attributes_s v = PSA_KEY_ATTRIBUTES_INIT; diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h index 1944be4b2..9af4957df 100644 --- a/include/psa/crypto_types.h +++ b/include/psa/crypto_types.h @@ -244,6 +244,17 @@ typedef uint32_t psa_key_usage_t; */ typedef struct psa_key_attributes_s psa_key_attributes_t; + +#ifndef __DOXYGEN_ONLY__ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +/* Mbed Crypto defines this type in crypto_types.h because it is also + * visible to applications through an implementation-specific extension. + * For the PSA Cryptography specification, this type is only visible + * via crypto_se_driver.h. */ +typedef uint64_t psa_key_slot_number_t; +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ +#endif /* !__DOXYGEN_ONLY__ */ + /**@}*/ /** \defgroup derivation Key derivation diff --git a/library/psa_crypto.c b/library/psa_crypto.c index e043d7000..a54cd73bb 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1187,6 +1187,13 @@ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, return( status ); attributes->core = slot->attr; + attributes->core.flags &= ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY | + MBEDTLS_PSA_KA_MASK_DUAL_USE ); + +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( psa_key_slot_is_external( slot ) ) + psa_set_key_slot_number( attributes, slot->data.se.slot_number ); +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ switch( slot->attr.type ) { @@ -1196,7 +1203,7 @@ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, #if defined(MBEDTLS_PSA_CRYPTO_SE_C) /* TOnogrepDO: reporting the public exponent for opaque keys * is not yet implemented. */ - if( psa_get_se_driver( slot->attr.lifetime, NULL, NULL ) ) + if( psa_key_slot_is_external( slot ) ) break; #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ status = psa_get_rsa_public_exponent( slot->data.rsa, attributes ); @@ -1212,6 +1219,21 @@ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, return( status ); } +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +psa_status_t psa_get_key_slot_number( + const psa_key_attributes_t *attributes, + psa_key_slot_number_t *slot_number ) +{ + if( attributes->core.flags & MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER ) + { + *slot_number = attributes->slot_number; + return( PSA_SUCCESS ); + } + else + return( PSA_ERROR_INVALID_ARGUMENT ); +} +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + #if defined(MBEDTLS_RSA_C) || defined(MBEDTLS_ECP_C) static int pk_write_pubkey_simple( mbedtls_pk_context *key, unsigned char *buf, size_t size ) @@ -1557,6 +1579,10 @@ static psa_status_t psa_start_key_creation( * we can roll back to a state where the key doesn't exist. */ if( *p_drv != NULL ) { + /* Choosing a slot number is not supported yet. */ + if( attributes->core.flags & MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER ) + return( PSA_ERROR_NOT_SUPPORTED ); + status = psa_find_se_slot_for_key( attributes, *p_drv, &slot->data.se.slot_number ); if( status != PSA_SUCCESS ) From 5fe5e2759160a040ae47561d7d26db12e392f07b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 2 Aug 2019 20:30:01 +0200 Subject: [PATCH 4/8] Test slot_number attribute Test the behavior of the getter/setter functions. Test that psa_get_key_slot_number() reports a slot number for a key in a secure element, and doesn't report a slot number for a key that is not in a secure element. Test that psa_get_key_slot_number() reports the correct slot number for a key in a secure element. --- include/psa/crypto_extra.h | 12 ++++ tests/suites/test_suite_psa_crypto.data | 3 + tests/suites/test_suite_psa_crypto.function | 61 +++++++++++++++++++ ...st_suite_psa_crypto_se_driver_hal.function | 30 ++++++++- 4 files changed, 105 insertions(+), 1 deletion(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 5359b580a..130ce7544 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -163,6 +163,18 @@ static inline void psa_set_key_slot_number( attributes->slot_number = slot_number; } +/** Remove the slot number attribute from a key attribute structure. + * + * This function undoes the action of psa_set_key_slot_number(). + * + * \param[out] attributes The attribute structure to write to. + */ +static inline void psa_clear_key_slot_number( + psa_key_attributes_t *attributes ) +{ + attributes->core.flags &= ~MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER; +} + #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ /**@}*/ diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index b04984024..4118d2f3e 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -19,6 +19,9 @@ persistence_attributes:0x1234:3:-1:0x1234:3 PSA key attributes: lifetime then id persistence_attributes:0x1234:3:0x1235:0x1235:3 +PSA key attributes: slot number +slot_number_attribute: + PSA import/export raw: 0 bytes import_export:"":PSA_KEY_TYPE_RAW_DATA:PSA_KEY_USAGE_EXPORT:0:0:0:PSA_SUCCESS:1 diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 0eb6172a4..3225bef34 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -1113,6 +1113,23 @@ exit: return( ok ); } +/* Assert that a key isn't reported as having a slot number. */ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +#define ASSERT_NO_SLOT_NUMBER( attributes ) \ + do \ + { \ + psa_key_slot_number_t ASSERT_NO_SLOT_NUMBER_slot_number; \ + TEST_EQUAL( psa_get_key_slot_number( \ + attributes, \ + &ASSERT_NO_SLOT_NUMBER_slot_number ), \ + PSA_ERROR_INVALID_ARGUMENT ); \ + } \ + while( 0 ) +#else /* MBEDTLS_PSA_CRYPTO_SE_C */ +#define ASSERT_NO_SLOT_NUMBER( attributes ) \ + ( (void) 0 ) +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + /* An overapproximation of the amount of storage needed for a key of the * given type and with the given content. The API doesn't make it easy * to find a good value for the size. The current implementation doesn't @@ -1214,6 +1231,46 @@ void persistence_attributes( int id1_arg, int lifetime_arg, int id2_arg, } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_PSA_CRYPTO_SE_C */ +void slot_number_attribute( ) +{ + psa_key_slot_number_t slot_number = 0xdeadbeef; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + + /* Initially, there is no slot number. */ + TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ), + PSA_ERROR_INVALID_ARGUMENT ); + + /* Test setting a slot number. */ + psa_set_key_slot_number( &attributes, 0 ); + PSA_ASSERT( psa_get_key_slot_number( &attributes, &slot_number ) ); + TEST_EQUAL( slot_number, 0 ); + + /* Test changing the slot number. */ + psa_set_key_slot_number( &attributes, 42 ); + PSA_ASSERT( psa_get_key_slot_number( &attributes, &slot_number ) ); + TEST_EQUAL( slot_number, 42 ); + + /* Test clearing the slot number. */ + psa_clear_key_slot_number( &attributes ); + TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ), + PSA_ERROR_INVALID_ARGUMENT ); + + /* Clearing again should have no effect. */ + psa_clear_key_slot_number( &attributes ); + TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ), + PSA_ERROR_INVALID_ARGUMENT ); + + /* Test that reset clears the slot number. */ + psa_set_key_slot_number( &attributes, 42 ); + PSA_ASSERT( psa_get_key_slot_number( &attributes, &slot_number ) ); + TEST_EQUAL( slot_number, 42 ); + psa_reset_key_attributes( &attributes ); + TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ), + PSA_ERROR_INVALID_ARGUMENT ); +} +/* END_CASE */ + /* BEGIN_CASE */ void import_with_policy( int type_arg, int usage_arg, int alg_arg, @@ -1246,6 +1303,7 @@ void import_with_policy( int type_arg, TEST_EQUAL( psa_get_key_type( &got_attributes ), type ); TEST_EQUAL( psa_get_key_usage_flags( &got_attributes ), usage ); TEST_EQUAL( psa_get_key_algorithm( &got_attributes ), alg ); + ASSERT_NO_SLOT_NUMBER( &got_attributes ); PSA_ASSERT( psa_destroy_key( handle ) ); test_operations_on_invalid_handle( handle ); @@ -1284,6 +1342,7 @@ void import_with_data( data_t *data, int type_arg, TEST_EQUAL( psa_get_key_type( &got_attributes ), type ); if( attr_bits != 0 ) TEST_EQUAL( attr_bits, psa_get_key_bits( &got_attributes ) ); + ASSERT_NO_SLOT_NUMBER( &got_attributes ); PSA_ASSERT( psa_destroy_key( handle ) ); test_operations_on_invalid_handle( handle ); @@ -1328,6 +1387,7 @@ void import_large_key( int type_arg, int byte_size_arg, TEST_EQUAL( psa_get_key_type( &attributes ), type ); TEST_EQUAL( psa_get_key_bits( &attributes ), PSA_BYTES_TO_BITS( byte_size ) ); + ASSERT_NO_SLOT_NUMBER( &attributes ); memset( buffer, 0, byte_size + 1 ); PSA_ASSERT( psa_export_key( handle, buffer, byte_size, &n ) ); for( n = 0; n < byte_size; n++ ) @@ -1420,6 +1480,7 @@ void import_export( data_t *data, PSA_ASSERT( psa_get_key_attributes( handle, &got_attributes ) ); TEST_EQUAL( psa_get_key_type( &got_attributes ), type ); TEST_EQUAL( psa_get_key_bits( &got_attributes ), (size_t) expected_bits ); + ASSERT_NO_SLOT_NUMBER( &got_attributes ); /* Export the key */ status = psa_export_key( handle, 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 6ac19a60e..9a5746476 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -212,6 +212,31 @@ static int check_key_attributes( psa_get_key_bits( reference_attributes ) ); } + { + psa_key_slot_number_t actual_slot_number = 0xdeadbeef; + psa_key_slot_number_t desired_slot_number = 0xb90cc011; + psa_key_lifetime_t lifetime = + psa_get_key_lifetime( &actual_attributes ); + psa_status_t status = psa_get_key_slot_number( &actual_attributes, + &actual_slot_number ); + if( lifetime < MIN_DRIVER_LIFETIME ) + { + /* The key is not in a secure element. */ + TEST_EQUAL( status, PSA_ERROR_INVALID_ARGUMENT ); + } + else + { + /* The key is in a secure element. If it had been created + * in a specific slot, check that it is reported there. */ + PSA_ASSERT( status ); + status = psa_get_key_slot_number( reference_attributes, + &desired_slot_number ); + if( status == PSA_SUCCESS ) + { + TEST_EQUAL( desired_slot_number, actual_slot_number ); + } + } + } ok = 1; exit: @@ -485,11 +510,14 @@ void key_creation_import_export( int min_slot, int restart ) /* Test that the key was created in the expected slot. */ TEST_ASSERT( ram_slots[min_slot].type == PSA_KEY_TYPE_RAW_DATA ); - /* Test the key attributes and the key data. */ + /* Test the key attributes, including the reported slot number. */ psa_set_key_bits( &attributes, PSA_BYTES_TO_BITS( sizeof( key_material ) ) ); + psa_set_key_slot_number( &attributes, min_slot ); if( ! check_key_attributes( handle, &attributes ) ) goto exit; + + /* Test the key data. */ PSA_ASSERT( psa_export_key( handle, exported, sizeof( exported ), &exported_length ) ); From 5a68056755f6e75f281b4d2884cff72ca85ecd78 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 17:32:13 +0200 Subject: [PATCH 5/8] Rename internal macro to pass check-names.sh check-names.sh rejects MBEDTLS_XXX identifiers that are not defined in a public header. --- library/psa_crypto.c | 4 ++-- library/psa_crypto_core.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index a54cd73bb..9f7b5cbc0 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1433,9 +1433,9 @@ psa_status_t psa_export_public_key( psa_key_handle_t handle, #if defined(static_assert) static_assert( ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, "One or more key attribute flag is listed as both external-only and dual-use" ); -static_assert( ( MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, +static_assert( ( PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, "One or more key attribute flag is listed as both external-only and dual-use" ); -static_assert( ( MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ) == 0, +static_assert( ( PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ) == 0, "One or more key attribute flag is listed as both internal-only and external-only" ); #endif diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index b67c0c576..edf3ab603 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -68,7 +68,7 @@ typedef struct /* A mask of key attribute flags used only internally. * Currently there aren't any. */ -#define MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY ( \ +#define PSA_KA_MASK_INTERNAL_ONLY ( \ 0 ) /** Test whether a key slot is occupied. From 013f5474cfc639412d787246b5af45a971e0493e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 7 Aug 2019 15:42:14 +0200 Subject: [PATCH 6/8] Fix erasure of external flags This didn't break anything now, but would have broken things once we start to add internal flags. --- library/psa_crypto.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 9f7b5cbc0..5742f627d 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1560,8 +1560,11 @@ static psa_status_t psa_start_key_creation( slot->attr = attributes->core; /* Erase external-only flags from the internal copy. To access - * external-only flags, query `attributes`. */ - slot->attr.flags |= ~MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY; + * external-only flags, query `attributes`. Thanks to the check + * in psa_validate_key_attributes(), this leaves the dual-use + * flags and any internal flag that psa_internal_allocate_key_slot() + * may have set. */ + slot->attr.flags &= ~MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY; #if defined(MBEDTLS_PSA_CRYPTO_SE_C) /* For a key in a secure element, we need to do three things: From 094dac1d12e16c3c0a933cf9c548b2a73cee478a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 7 Aug 2019 18:19:46 +0200 Subject: [PATCH 7/8] Fix copypasta --- library/psa_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 5742f627d..254ab2a71 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1434,7 +1434,7 @@ psa_status_t psa_export_public_key( psa_key_handle_t handle, static_assert( ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, "One or more key attribute flag is listed as both external-only and dual-use" ); static_assert( ( PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, - "One or more key attribute flag is listed as both external-only and dual-use" ); + "One or more key attribute flag is listed as both internal-only and dual-use" ); static_assert( ( PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ) == 0, "One or more key attribute flag is listed as both internal-only and external-only" ); #endif From edbed5670afe1caa6be6cb9ec8dbca4fab3b82c8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 7 Aug 2019 18:19:59 +0200 Subject: [PATCH 8/8] Rename psa_internal_allocate_key_slot to psa_get_empty_key_slot This function no longer modifies anything, so it doesn't actually allocate the slot. Now, it just returns the empty key slot, and it's up to the caller to cause the slot to be in use (or not). --- library/psa_crypto.c | 4 ++-- library/psa_crypto_slot_management.c | 4 ++-- library/psa_crypto_slot_management.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 254ab2a71..5cb88de7e 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1546,7 +1546,7 @@ static psa_status_t psa_start_key_creation( if( status != PSA_SUCCESS ) return( status ); - status = psa_internal_allocate_key_slot( handle, p_slot ); + status = psa_get_empty_key_slot( handle, p_slot ); if( status != PSA_SUCCESS ) return( status ); slot = *p_slot; @@ -1562,7 +1562,7 @@ static psa_status_t psa_start_key_creation( /* Erase external-only flags from the internal copy. To access * external-only flags, query `attributes`. Thanks to the check * in psa_validate_key_attributes(), this leaves the dual-use - * flags and any internal flag that psa_internal_allocate_key_slot() + * flags and any internal flag that psa_get_empty_key_slot() * may have set. */ slot->attr.flags &= ~MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY; diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 073400988..fe9214831 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -102,7 +102,7 @@ void psa_wipe_all_key_slots( void ) global_data.key_slots_initialized = 0; } -psa_status_t psa_internal_allocate_key_slot( psa_key_handle_t *handle, +psa_status_t psa_get_empty_key_slot( psa_key_handle_t *handle, psa_key_slot_t **p_slot ) { if( ! global_data.key_slots_initialized ) @@ -228,7 +228,7 @@ psa_status_t psa_open_key( psa_key_file_id_t id, psa_key_handle_t *handle ) if( status != PSA_SUCCESS ) return( status ); - status = psa_internal_allocate_key_slot( handle, &slot ); + status = psa_get_empty_key_slot( handle, &slot ); if( status != PSA_SUCCESS ) return( status ); diff --git a/library/psa_crypto_slot_management.h b/library/psa_crypto_slot_management.h index cde590fc5..472253dd9 100644 --- a/library/psa_crypto_slot_management.h +++ b/library/psa_crypto_slot_management.h @@ -71,8 +71,8 @@ void psa_wipe_all_key_slots( void ); * \retval #PSA_ERROR_INSUFFICIENT_MEMORY * \retval #PSA_ERROR_BAD_STATE */ -psa_status_t psa_internal_allocate_key_slot( psa_key_handle_t *handle, - psa_key_slot_t **p_slot ); +psa_status_t psa_get_empty_key_slot( psa_key_handle_t *handle, + psa_key_slot_t **p_slot ); /** Test whether a lifetime designates a key in an external cryptoprocessor. *