From 6801f089733e3e9ab46c1b6be23597d4f135a97d Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Fri, 19 Feb 2021 17:21:22 +0100 Subject: [PATCH 01/26] Implement support for MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS According to the design in psa-driver-interface.md. Compiles without issue in test_psa_crypto_drivers. Signed-off-by: Steven Cooreman --- include/mbedtls/config.h | 16 +++++ include/psa/crypto_extra.h | 93 ++++++++++++++++++++++++++++ library/psa_crypto_driver_wrappers.c | 17 +++++ library/psa_crypto_driver_wrappers.h | 5 ++ library/psa_crypto_slot_management.c | 78 +++++++++++++++++++++-- library/version_features.c | 3 + programs/test/query_config.c | 8 +++ tests/scripts/all.sh | 1 + tests/src/helpers.c | 35 +++++++++++ 9 files changed, 251 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index d370dbff5..62d89c977 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1338,6 +1338,22 @@ */ #define MBEDTLS_PKCS1_V21 +/** \def MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS + * + * Enable support for platform built-in keys. If you enable this feature, + * you must implement the function mbedtls_psa_platform_get_builtin_key(). + * See the documentation of that function for more information. + * + * Built-in keys are typically derived from a hardware unique key or + * stored in a secure element. + * + * Requires: MBEDTLS_PSA_CRYPTO_C. + * + * \warning This interface is experimental and may change or be removed + * without notice. + */ +//#define MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS + /** \def MBEDTLS_PSA_CRYPTO_CLIENT * * Enable support for PSA crypto client. diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index e01d827e8..f9a9aeeaf 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -713,6 +713,99 @@ psa_status_t mbedtls_psa_external_get_random( /**@}*/ +/** \defgroup psa_builtin_keys Built-in keys + * @{ + */ + +/** The minimum value for a key identifier that is built into the + * implementation. + * + * The range of key identifiers from #MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + * to #MBEDTLS_PSA_KEY_ID_BUILTIN_MAX within the range from + * #PSA_KEY_ID_VENDOR_MIN and #PSA_KEY_ID_VENDOR_MAX and must not intersect + * with any other set of implementation-chosen key identifiers. + * + * This value is part of the library's ABI since changing it would invalidate + * the values of built-in key identifiers in applications. + */ +#define MBEDTLS_PSA_KEY_ID_BUILTIN_MIN ((psa_key_id_t)0x7fff0000) + +/** The maximum value for a key identifier that is built into the + * implementation. + * + * See #MBEDTLS_PSA_KEY_ID_BUILTIN_MIN for more information. + */ +#define MBEDTLS_PSA_KEY_ID_BUILTIN_MAX ((psa_key_id_t)0x7fffefff) + +/** A slot number identifying a key in a driver. + * + * Values of this type are used to identify built-in keys. + */ +typedef uint64_t psa_drv_slot_number_t; + +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) +/** Test whether a key identifier belongs to the builtin key range. + * + * \param key_id Key identifier to test. + * + * \retval 1 + * The key identifier is a builtin key identifier. + * \retval 0 + * The key identifier is not a builtin key identifier. + */ +static inline int psa_key_id_is_builtin( psa_key_id_t key_id ) +{ + return( ( key_id >= MBEDTLS_PSA_KEY_ID_BUILTIN_MIN ) && + ( key_id <= MBEDTLS_PSA_KEY_ID_BUILTIN_MAX ) ); +} + +/** Platform function to obtain the data of a built-in key. + * + * An application-specific implementation of this function must be provided if + * #MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS is enabled. This would typically provided + * as part of a platform's system image. + * + * Call psa_get_key_id(\p attributes) to obtain the key identifier \c key_id. + * #MBEDTLS_SVC_KEY_ID_GET_KEY_ID(\p key_id) is in the range from + * #MBEDTLS_PSA_KEY_ID_BUILTIN_MIN to #MBEDTLS_PSA_KEY_ID_BUILTIN_MAX. + * + * In a multi-application configuration + * (\c MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER is defined), + * this function should check that #MBEDTLS_SVC_KEY_ID_GET_OWNER_ID(\p key_id) + * is allowed to use the given key. + * + * \param[in,out] attributes On entry, this is #PSA_KEY_ATTRIBUTES_INIT or + * an equivalent value, except that the key + * identifier field is set. + * On successful return, this function must set + * the attributes of the key: lifetime, type, + * bit-size, usage policy. + * \param[out] slot_number On successful return, this function must + * this to the slot number known to the driver for + * the lifetime location reported through + * \p attributes which corresponds to the + * requested built-in key. + * + * \retval #PSA_SUCCESS + * The requested key identifier designates a built-in key. + * In a multi-application configuration, the requested owner + * is allowed to access it. + * \retval #PSA_ERROR_DOES_NOT_EXIST + * The requested key identifier is not a built-in key which is known + * to this function. If a key exists in the key storage with this + * identifier, the data from the storage will be used. + * \retval (any other error) + * Any other error is propagated to the function that requested the key. + * Common errors include: + * - #PSA_ERROR_NOT_PERMITTED: the key exists but the requested owner + * is not allowed to access it. + */ +psa_status_t mbedtls_psa_platform_get_builtin_key( + psa_key_attributes_t *attributes, psa_drv_slot_number_t *slot_number ); +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ + +/** @} */ + #ifdef __cplusplus } #endif diff --git a/library/psa_crypto_driver_wrappers.c b/library/psa_crypto_driver_wrappers.c index 536505ef4..70c3026ce 100644 --- a/library/psa_crypto_driver_wrappers.c +++ b/library/psa_crypto_driver_wrappers.c @@ -574,6 +574,23 @@ psa_status_t psa_driver_wrapper_export_public_key( } } +psa_status_t psa_driver_wrapper_get_builtin_key( + psa_drv_slot_number_t slot_number, + psa_key_attributes_t *attributes, + uint8_t *key_buffer, size_t key_buffer_size, size_t *key_buffer_length ) +{ + psa_key_location_t location = PSA_KEY_LIFETIME_GET_LOCATION( attributes->core.lifetime ); + switch( location ) + { + default: + (void) slot_number; + (void) key_buffer; + (void) key_buffer_size; + (void) key_buffer_length; + return( PSA_ERROR_DOES_NOT_EXIST ); + } +} + /* * Cipher functions */ diff --git a/library/psa_crypto_driver_wrappers.h b/library/psa_crypto_driver_wrappers.h index e49941138..e82d0931b 100644 --- a/library/psa_crypto_driver_wrappers.h +++ b/library/psa_crypto_driver_wrappers.h @@ -68,6 +68,11 @@ psa_status_t psa_driver_wrapper_generate_key( const psa_key_attributes_t *attributes, uint8_t *key_buffer, size_t key_buffer_size, size_t *key_buffer_length ); +psa_status_t psa_driver_wrapper_get_builtin_key( + psa_drv_slot_number_t slot_number, + psa_key_attributes_t *attributes, + uint8_t *key_buffer, size_t key_buffer_size, size_t *key_buffer_length ); + /* * Cipher functions */ diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index cf07a3693..c90ebee00 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -274,6 +274,67 @@ exit: } #endif /* MBEDTLS_PSA_CRYPTO_STORAGE_C */ +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) +#include "psa_crypto_driver_wrappers.h" + +static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) +{ + /* Load keys in the 'builtin' range through their own interface */ + if( psa_key_id_is_builtin( MBEDTLS_SVC_KEY_ID_GET_KEY_ID( slot->attr.id ) ) ) + { + /* Check the platform function to see whether this key actually exists */ + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_drv_slot_number_t slot_number; + + psa_set_key_id(&attributes, slot->attr.id); + psa_status_t status = mbedtls_psa_platform_get_builtin_key( + &attributes, &slot_number ); + if( status != PSA_SUCCESS ) + return( status ); + + /* If the key should exist according to the platform, load it through + * the driver interface. */ + uint8_t *key_buffer = NULL; + size_t key_buffer_length = 0; + + status = psa_driver_wrapper_get_key_buffer_size( &attributes, &key_buffer_length ); + if( status != PSA_SUCCESS ) + return( status ); + + key_buffer = mbedtls_calloc( 1, key_buffer_length ); + if( key_buffer == NULL ) + return( PSA_ERROR_INSUFFICIENT_MEMORY ); + + status = psa_driver_wrapper_get_builtin_key( + slot_number, &attributes, + key_buffer, key_buffer_length, &key_buffer_length ); + if( status != PSA_SUCCESS ) + goto exit; + + status = psa_copy_key_material_into_slot( slot, key_buffer, key_buffer_length ); + if( status != PSA_SUCCESS ) + goto exit; + + /* Copy core attributes into the slot on success. + * Use static allocations to make the compiler yell at us should one + * of the two structures change type. */ + psa_core_key_attributes_t* builtin_key_core_attributes = + &attributes.core; + psa_core_key_attributes_t* slot_core_attributes = + &slot->attr; + memcpy( slot_core_attributes, + builtin_key_core_attributes, + sizeof(psa_core_key_attributes_t) ); + +exit: + mbedtls_free( key_buffer ); + return( status ); + } else { + return( PSA_ERROR_DOES_NOT_EXIST ); + } +} +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ + psa_status_t psa_get_and_lock_key_slot( mbedtls_svc_key_id_t key, psa_key_slot_t **p_slot ) { @@ -291,17 +352,27 @@ psa_status_t psa_get_and_lock_key_slot( mbedtls_svc_key_id_t key, if( status != PSA_ERROR_DOES_NOT_EXIST ) return( status ); -#if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) psa_key_id_t volatile_key_id; status = psa_get_empty_key_slot( &volatile_key_id, p_slot ); if( status != PSA_SUCCESS ) return( status ); - (*p_slot)->attr.lifetime = PSA_KEY_LIFETIME_PERSISTENT; (*p_slot)->attr.id = key; + (*p_slot)->attr.lifetime = PSA_KEY_LIFETIME_PERSISTENT; + status = PSA_ERROR_DOES_NOT_EXIST; +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) + status = psa_load_builtin_key_into_slot( *p_slot ); + if( status == PSA_SUCCESS ) + goto exit; +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ + +#if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) status = psa_load_persistent_key_into_slot( *p_slot ); +#endif /* defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */ + +exit: if( status != PSA_SUCCESS ) { psa_wipe_key_slot( *p_slot ); @@ -309,9 +380,6 @@ psa_status_t psa_get_and_lock_key_slot( mbedtls_svc_key_id_t key, status = PSA_ERROR_INVALID_HANDLE; } return( status ); -#else - return( PSA_ERROR_INVALID_HANDLE ); -#endif /* defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */ } psa_status_t psa_unlock_key_slot( psa_key_slot_t *slot ) diff --git a/library/version_features.c b/library/version_features.c index 93329879a..f665a2375 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -438,6 +438,9 @@ static const char * const features[] = { #if defined(MBEDTLS_PKCS1_V21) "MBEDTLS_PKCS1_V21", #endif /* MBEDTLS_PKCS1_V21 */ +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) + "MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS", +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ #if defined(MBEDTLS_PSA_CRYPTO_CLIENT) "MBEDTLS_PSA_CRYPTO_CLIENT", #endif /* MBEDTLS_PSA_CRYPTO_CLIENT */ diff --git a/programs/test/query_config.c b/programs/test/query_config.c index b9105f812..9760f626c 100644 --- a/programs/test/query_config.c +++ b/programs/test/query_config.c @@ -1226,6 +1226,14 @@ int query_config( const char *config ) } #endif /* MBEDTLS_PKCS1_V21 */ +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) + if( strcmp( "MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS ); + return( 0 ); + } +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ + #if defined(MBEDTLS_PSA_CRYPTO_CLIENT) if( strcmp( "MBEDTLS_PSA_CRYPTO_CLIENT", config ) == 0 ) { diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index f768e1e5e..a85c7ce00 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2267,6 +2267,7 @@ component_test_psa_crypto_drivers () { msg "build: MBEDTLS_PSA_CRYPTO_DRIVERS w/ driver hooks" scripts/config.py full scripts/config.py set MBEDTLS_PSA_CRYPTO_DRIVERS + scripts/config.py set MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS # Need to define the correct symbol and include the test driver header path in order to build with the test driver loc_cflags="$ASAN_CFLAGS -DPSA_CRYPTO_DRIVER_TEST" loc_cflags="${loc_cflags} -DMBEDTLS_PSA_ACCEL_KEY_TYPE_AES" diff --git a/tests/src/helpers.c b/tests/src/helpers.c index e323275e5..c282edc84 100644 --- a/tests/src/helpers.c +++ b/tests/src/helpers.c @@ -282,3 +282,38 @@ void mbedtls_param_failed( const char *failure_condition, } } #endif /* MBEDTLS_CHECK_PARAMS */ + +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) +#include +typedef struct +{ + psa_key_id_t builtin_key_id; + psa_key_location_t location; + psa_drv_slot_number_t slot_number; +} mbedtls_psa_builtin_key_description_t; +static const mbedtls_psa_builtin_key_description_t builtin_keys[] = { + // TODO: declare some keys + {0, 0, 0}, +}; +psa_status_t mbedtls_psa_platform_get_builtin_key( + psa_key_attributes_t *attributes, psa_drv_slot_number_t *slot_number ) +{ + mbedtls_svc_key_id_t svc_key_id = psa_get_key_id( attributes ); + psa_key_id_t app_key_id = MBEDTLS_SVC_KEY_ID_GET_KEY_ID( svc_key_id ); + + for( size_t i = 0; i < ( sizeof( builtin_keys ) / sizeof( builtin_keys[0] ) ); i++ ) + { + if( builtin_keys[i].builtin_key_id == app_key_id ) + { + psa_set_key_lifetime( attributes, + PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( + PSA_KEY_PERSISTENCE_READ_ONLY, + builtin_keys[i].location ) ); + *slot_number = builtin_keys[i].slot_number; + return( PSA_SUCCESS ); + } + } + + return( PSA_ERROR_DOES_NOT_EXIST ); +} +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ From f9a55ffa2ce2b6fa9489abd961d5de0d383f20f6 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Fri, 19 Feb 2021 18:04:59 +0100 Subject: [PATCH 02/26] Add test driver implementation for MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS As part of test_psa_crypto_drivers, define a builtin symmetric plus an ECC key on the test driver lifetime. Signed-off-by: Steven Cooreman --- library/psa_crypto_driver_wrappers.c | 21 ++++++++ tests/include/test/drivers/key_management.h | 10 ++++ tests/src/drivers/key_management.c | 59 +++++++++++++++++++++ tests/src/helpers.c | 21 +++++++- 4 files changed, 109 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto_driver_wrappers.c b/library/psa_crypto_driver_wrappers.c index 70c3026ce..28087de07 100644 --- a/library/psa_crypto_driver_wrappers.c +++ b/library/psa_crypto_driver_wrappers.c @@ -257,6 +257,16 @@ psa_status_t psa_driver_wrapper_get_key_buffer_size( { #if defined(PSA_CRYPTO_DRIVER_TEST) case PSA_CRYPTO_TEST_DRIVER_LIFETIME: +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) + /* Emulate property 'builtin_key_size' */ + if( psa_key_id_is_builtin( + MBEDTLS_SVC_KEY_ID_GET_KEY_ID( + psa_get_key_id( attributes ) ) ) ) + { + *key_buffer_size = sizeof(psa_drv_slot_number_t); + return( PSA_SUCCESS ); + } +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ #ifdef TEST_DRIVER_KEY_CONTEXT_SIZE_FUNCTION *key_buffer_size = test_size_function( key_type, key_bits ); return( PSA_SUCCESS ); @@ -582,6 +592,17 @@ psa_status_t psa_driver_wrapper_get_builtin_key( psa_key_location_t location = PSA_KEY_LIFETIME_GET_LOCATION( attributes->core.lifetime ); switch( location ) { +#if defined(PSA_CRYPTO_DRIVER_TEST) + case PSA_CRYPTO_TEST_DRIVER_LIFETIME: +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) + return( test_opaque_get_builtin_key( + slot_number, + attributes, + key_buffer, key_buffer_size, key_buffer_length ) ); +#else + return( PSA_ERROR_DOES_NOT_EXIST ); +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ +#endif /* PSA_CRYPTO_DRIVER_TEST */ default: (void) slot_number; (void) key_buffer; diff --git a/tests/include/test/drivers/key_management.h b/tests/include/test/drivers/key_management.h index b30baa205..ee96024df 100644 --- a/tests/include/test/drivers/key_management.h +++ b/tests/include/test/drivers/key_management.h @@ -29,6 +29,11 @@ #if defined(PSA_CRYPTO_DRIVER_TEST) #include +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) +#define PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT 0 +#define PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT 1 +#endif + typedef struct { /* If non-null, on success, copy this to the output. */ void *forced_output; @@ -82,5 +87,10 @@ psa_status_t test_transparent_import_key( size_t *key_buffer_length, size_t *bits); +psa_status_t test_opaque_get_builtin_key( + psa_drv_slot_number_t slot_number, + psa_key_attributes_t *attributes, + uint8_t *key_buffer, size_t key_buffer_size, size_t *key_buffer_length ); + #endif /* PSA_CRYPTO_DRIVER_TEST */ #endif /* PSA_CRYPTO_TEST_DRIVERS_KEY_MANAGEMENT_H */ diff --git a/tests/src/drivers/key_management.c b/tests/src/drivers/key_management.c index 10a40c37d..d8410be4e 100644 --- a/tests/src/drivers/key_management.c +++ b/tests/src/drivers/key_management.c @@ -232,4 +232,63 @@ psa_status_t test_opaque_export_public_key( return( PSA_ERROR_NOT_SUPPORTED ); } +/* The opaque test driver exposes two built-in keys when builtin key support is + * compiled in. + * The key in slot #PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT is an AES-128 key which allows CTR mode + * The key in slot #PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT is a secp256r1 private key which allows ECDSA sign & verify + * The key buffer format for these is the raw format of psa_drv_slot_number_t + * (i.e. for an actual driver this would mean 'builtin_key_size' = sizeof(psa_drv_slot_number_t)) + */ +psa_status_t test_opaque_get_builtin_key( + psa_drv_slot_number_t slot_number, + psa_key_attributes_t *attributes, + uint8_t *key_buffer, size_t key_buffer_size, size_t *key_buffer_length ) +{ +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) + switch( slot_number ) + { + case PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT: + if( key_buffer_size < sizeof( psa_drv_slot_number_t ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + + psa_set_key_type( attributes, PSA_KEY_TYPE_AES ); + psa_set_key_bits( attributes, 128 ); + psa_set_key_usage_flags( attributes, PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT ); + psa_set_key_algorithm( attributes, PSA_ALG_CTR ); + + *( (psa_drv_slot_number_t*) key_buffer ) = + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT; + *key_buffer_length = sizeof( psa_drv_slot_number_t ); + return( PSA_SUCCESS ); + case PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT: + if( key_buffer_size < sizeof( psa_drv_slot_number_t ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + + psa_set_key_type( attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1) ); + psa_set_key_bits( attributes, 256 ); + psa_set_key_usage_flags( attributes, PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_VERIFY_HASH ); + psa_set_key_algorithm( attributes, PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ); + + *( (psa_drv_slot_number_t*) key_buffer) = + PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT; + *key_buffer_length = sizeof( psa_drv_slot_number_t ); + return( PSA_SUCCESS ); + default: + (void) slot_number; + (void) attributes; + (void) key_buffer; + (void) key_buffer_size; + (void) key_buffer_length; + return( PSA_ERROR_INVALID_ARGUMENT ); + } +#else + (void) slot_number; + (void) attributes; + (void) key_buffer; + (void) key_buffer_size; + (void) key_buffer_length; + return( PSA_ERROR_DOES_NOT_EXIST ); +#endif +} + #endif /* MBEDTLS_PSA_CRYPTO_DRIVERS && PSA_CRYPTO_DRIVER_TEST */ diff --git a/tests/src/helpers.c b/tests/src/helpers.c index c282edc84..ee7fa209c 100644 --- a/tests/src/helpers.c +++ b/tests/src/helpers.c @@ -285,16 +285,33 @@ void mbedtls_param_failed( const char *failure_condition, #if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) #include + +#if defined(PSA_CRYPTO_DRIVER_TEST) +#include "test/drivers/test_driver.h" +#endif + typedef struct { psa_key_id_t builtin_key_id; psa_key_location_t location; psa_drv_slot_number_t slot_number; } mbedtls_psa_builtin_key_description_t; + static const mbedtls_psa_builtin_key_description_t builtin_keys[] = { - // TODO: declare some keys - {0, 0, 0}, +#if defined(PSA_CRYPTO_DRIVER_TEST) + /* For testing, assign the AES builtin key slot to the boundary values. + * ECDSA can be exercised on key ID MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1. */ + {MBEDTLS_PSA_KEY_ID_BUILTIN_MIN - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + {MBEDTLS_PSA_KEY_ID_BUILTIN_MIN, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + {MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT}, + {MBEDTLS_PSA_KEY_ID_BUILTIN_MAX - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + {MBEDTLS_PSA_KEY_ID_BUILTIN_MAX, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + {MBEDTLS_PSA_KEY_ID_BUILTIN_MAX + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, +#else + {0, 0, 0} +#endif }; + psa_status_t mbedtls_psa_platform_get_builtin_key( psa_key_attributes_t *attributes, psa_drv_slot_number_t *slot_number ) { From 437fcfc32ed1249d7ff07ad381bae627295402cc Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Mon, 22 Feb 2021 12:44:15 +0100 Subject: [PATCH 03/26] Add simple test coverage for builtin keys (PSA opaque driver export) Signed-off-by: Steven Cooreman --- tests/include/test/drivers/key_management.h | 3 + tests/src/drivers/key_management.c | 114 +++++++++++++++++- ...test_suite_psa_crypto_driver_wrappers.data | 24 ++++ ..._suite_psa_crypto_driver_wrappers.function | 104 ++++++++++++++++ 4 files changed, 240 insertions(+), 5 deletions(-) diff --git a/tests/include/test/drivers/key_management.h b/tests/include/test/drivers/key_management.h index ee96024df..cf6fbb0b0 100644 --- a/tests/include/test/drivers/key_management.h +++ b/tests/include/test/drivers/key_management.h @@ -32,6 +32,9 @@ #if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) #define PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT 0 #define PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT 1 + +extern const uint8_t test_driver_aes_key[16]; +extern const uint8_t test_driver_ecdsa_key[32]; #endif typedef struct { diff --git a/tests/src/drivers/key_management.c b/tests/src/drivers/key_management.c index d8410be4e..77a217f06 100644 --- a/tests/src/drivers/key_management.c +++ b/tests/src/drivers/key_management.c @@ -41,6 +41,30 @@ test_driver_key_management_hooks_t test_driver_key_management_hooks = TEST_DRIVER_KEY_MANAGEMENT_INIT; +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) +const uint8_t test_driver_aes_key[16] = + { 0x36, 0x77, 0x39, 0x7A, 0x24, 0x43, 0x26, 0x46, + 0x29, 0x4A, 0x40, 0x4E, 0x63, 0x52, 0x66, 0x55 }; +const uint8_t test_driver_ecdsa_key[32] = + { 0xdc, 0x7d, 0x9d, 0x26, 0xd6, 0x7a, 0x4f, 0x63, + 0x2c, 0x34, 0xc2, 0xdc, 0x0b, 0x69, 0x86, 0x18, + 0x38, 0x82, 0xc2, 0x06, 0xdf, 0x04, 0xcd, 0xb7, + 0xd6, 0x9a, 0xab, 0xe2, 0x8b, 0xe4, 0xf8, 0x1a }; +const uint8_t test_driver_ecdsa_pubkey[65] = + { 0x04, + 0x85, 0xf6, 0x4d, 0x89, 0xf0, 0x0b, 0xe6, 0x6c, + 0x88, 0xdd, 0x93, 0x7e, 0xfd, 0x6d, 0x7c, 0x44, + 0x56, 0x48, 0xdc, 0xb7, 0x01, 0x15, 0x0b, 0x8a, + 0x95, 0x09, 0x29, 0x58, 0x50, 0xf4, 0x1c, 0x19, + 0x31, 0xe5, 0x71, 0xfb, 0x8f, 0x8c, 0x78, 0x31, + 0x7a, 0x20, 0xb3, 0x80, 0xe8, 0x66, 0x58, 0x4b, + 0xbc, 0x25, 0x16, 0xc3, 0xd2, 0x70, 0x2d, 0x79, + 0x2f, 0x13, 0x1a, 0x92, 0x20, 0x95, 0xfd, 0x6c }; + +static const psa_drv_slot_number_t aes_slot = PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT; +static const psa_drv_slot_number_t ecdsa_slot = PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT; +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ + psa_status_t test_transparent_generate_key( const psa_key_attributes_t *attributes, uint8_t *key, size_t key_size, size_t *key_length ) @@ -154,6 +178,57 @@ psa_status_t test_opaque_export_key( const uint8_t *key, size_t key_length, uint8_t *data, size_t data_size, size_t *data_length ) { +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) + if( psa_key_id_is_builtin( MBEDTLS_SVC_KEY_ID_GET_KEY_ID( psa_get_key_id( attributes ) ) ) ) + { + if( key_length != sizeof( psa_drv_slot_number_t ) ) + return( PSA_ERROR_INVALID_ARGUMENT ); + + if( memcmp( key, &ecdsa_slot, sizeof( psa_drv_slot_number_t ) ) == 0 ) + { + /* This is the ECDSA slot. Verify key attributes before returning pubkey. */ + if( psa_get_key_type( attributes ) != PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_bits( attributes ) != 256 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_algorithm( attributes ) != PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( (psa_get_key_usage_flags( attributes ) & PSA_KEY_USAGE_EXPORT) == 0 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + + if( data_size < sizeof( test_driver_ecdsa_key ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + + memcpy( data, test_driver_ecdsa_key, sizeof( test_driver_ecdsa_key ) ); + *data_length = sizeof( test_driver_ecdsa_key ); + return( PSA_SUCCESS ); + } + + if( memcmp( key, &aes_slot, sizeof( psa_drv_slot_number_t ) ) == 0 ) + { + /* This is the ECDSA slot. Verify key attributes before returning pubkey. */ + if( psa_get_key_type( attributes ) != PSA_KEY_TYPE_AES ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_bits( attributes ) != 128 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_algorithm( attributes ) != PSA_ALG_CTR ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( (psa_get_key_usage_flags( attributes ) & PSA_KEY_USAGE_EXPORT) == 0 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + + if( data_size < sizeof( test_driver_aes_key ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + + memcpy( data, test_driver_aes_key, sizeof( test_driver_aes_key ) ); + *data_length = sizeof( test_driver_aes_key ); + return( PSA_SUCCESS ); + } + + /* Potentially add more slots here */ + + return( PSA_ERROR_DOES_NOT_EXIST ); + } +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ (void) attributes; (void) key; (void) key_length; @@ -223,6 +298,35 @@ psa_status_t test_opaque_export_public_key( const uint8_t *key, size_t key_length, uint8_t *data, size_t data_size, size_t *data_length ) { +#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) + if( psa_key_id_is_builtin( MBEDTLS_SVC_KEY_ID_GET_KEY_ID( psa_get_key_id( attributes ) ) ) ) + { + if( key_length != sizeof( psa_drv_slot_number_t ) ) + return( PSA_ERROR_INVALID_ARGUMENT ); + + if( memcmp( key, &ecdsa_slot, sizeof( psa_drv_slot_number_t ) ) == 0 ) + { + /* This is the ECDSA slot. Verify key attributes before returning pubkey. */ + if( psa_get_key_type( attributes ) != PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_bits( attributes ) != 256 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_algorithm( attributes ) != PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + + if( data_size < sizeof( test_driver_ecdsa_pubkey ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + + memcpy(data, test_driver_ecdsa_pubkey, sizeof( test_driver_ecdsa_pubkey ) ); + *data_length = sizeof( test_driver_ecdsa_pubkey ); + return( PSA_SUCCESS ); + } + + /* Potentially add more slots here */ + + return( PSA_ERROR_DOES_NOT_EXIST ); + } +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ (void) attributes; (void) key; (void) key_length; @@ -253,7 +357,7 @@ psa_status_t test_opaque_get_builtin_key( psa_set_key_type( attributes, PSA_KEY_TYPE_AES ); psa_set_key_bits( attributes, 128 ); - psa_set_key_usage_flags( attributes, PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT ); + psa_set_key_usage_flags( attributes, PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT | PSA_KEY_USAGE_EXPORT ); psa_set_key_algorithm( attributes, PSA_ALG_CTR ); *( (psa_drv_slot_number_t*) key_buffer ) = @@ -264,9 +368,9 @@ psa_status_t test_opaque_get_builtin_key( if( key_buffer_size < sizeof( psa_drv_slot_number_t ) ) return( PSA_ERROR_BUFFER_TOO_SMALL ); - psa_set_key_type( attributes, PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1) ); + psa_set_key_type( attributes, PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ); psa_set_key_bits( attributes, 256 ); - psa_set_key_usage_flags( attributes, PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_VERIFY_HASH ); + psa_set_key_usage_flags( attributes, PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_VERIFY_HASH | PSA_KEY_USAGE_EXPORT ); psa_set_key_algorithm( attributes, PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ); *( (psa_drv_slot_number_t*) key_buffer) = @@ -281,14 +385,14 @@ psa_status_t test_opaque_get_builtin_key( (void) key_buffer_length; return( PSA_ERROR_INVALID_ARGUMENT ); } -#else +#else /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ (void) slot_number; (void) attributes; (void) key_buffer; (void) key_buffer_size; (void) key_buffer_length; return( PSA_ERROR_DOES_NOT_EXIST ); -#endif +#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ } #endif /* MBEDTLS_PSA_CRYPTO_DRIVERS && PSA_CRYPTO_DRIVER_TEST */ diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.data b/tests/suites/test_suite_psa_crypto_driver_wrappers.data index 241d715b3..251388378 100644 --- a/tests/suites/test_suite_psa_crypto_driver_wrappers.data +++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.data @@ -243,3 +243,27 @@ aead_decrypt:PSA_KEY_TYPE_AES:"a0ec7b0052541d9e9c091fb7fc481409":PSA_ALG_GCM:"00 PSA AEAD decrypt, AES-GCM, 144 bytes #1, INSUFFICIENT_MEMORY depends_on:MBEDTLS_AES_C:MBEDTLS_GCM_C aead_decrypt:PSA_KEY_TYPE_AES:"a0ec7b0052541d9e9c091fb7fc481409":PSA_ALG_GCM:"00e440846db73a490573deaf3728c94f":"a3cfcb832e935eb5bc3812583b3a1b2e82920c07fda3668a35d939d8f11379bb606d39e6416b2ef336fffb15aec3f47a71e191f4ff6c56ff15913562619765b26ae094713d60bab6ab82bfc36edaaf8c7ce2cf5906554dcc5933acdb9cb42c1d24718efdc4a09256020b024b224cfe602772bd688c6c8f1041a46f7ec7d51208":"3b6de52f6e582d317f904ee768895bd4d0790912efcf27b58651d0eb7eb0b2f07222c6ffe9f7e127d98ccb132025b098a67dc0ec0083235e9f83af1ae1297df4319547cbcb745cebed36abc1f32a059a05ede6c00e0da097521ead901ad6a73be20018bda4c323faa135169e21581e5106ac20853642e9d6b17f1dd925c872814365847fe0b7b7fbed325953df344a96":"5431d93278c35cfcd7ffa9ce2de5c6b922edffd5055a9eaa5b54cae088db007cf2d28efaf9edd1569341889073e87c0a88462d77016744be62132fd14a243ed6e30e12cd2f7d08a8daeec161691f3b27d4996df8745d74402ee208e4055615a8cb069d495cf5146226490ac615d7b17ab39fb4fdd098e4e7ee294d34c1312826":PSA_ERROR_INSUFFICIENT_MEMORY + +PSA opaque driver builtin key export: AES +builtin_key_export:MBEDTLS_PSA_KEY_ID_BUILTIN_MIN:PSA_KEY_TYPE_AES:128:PSA_ALG_CTR:"3677397A24432646294A404E63526655":PSA_SUCCESS + +PSA opaque driver builtin key export: AES (registered to ID_MAX-1) +builtin_key_export:MBEDTLS_PSA_KEY_ID_BUILTIN_MAX - 1:PSA_KEY_TYPE_AES:128:PSA_ALG_CTR:"3677397A24432646294A404E63526655":PSA_SUCCESS + +PSA opaque driver builtin key export: AES (registered to ID_MAX) +builtin_key_export:MBEDTLS_PSA_KEY_ID_BUILTIN_MAX:PSA_KEY_TYPE_AES:128:PSA_ALG_CTR:"3677397A24432646294A404E63526655":PSA_SUCCESS + +PSA opaque driver builtin key export: key ID out of range (ID_MIN - 1) +builtin_key_export:MBEDTLS_PSA_KEY_ID_BUILTIN_MIN - 1:PSA_KEY_TYPE_AES:128:PSA_ALG_CTR:"3677397A24432646294A404E63526655":PSA_ERROR_INVALID_HANDLE + +PSA opaque driver builtin key export: key ID out of range (ID_MAX + 1) +builtin_key_export:MBEDTLS_PSA_KEY_ID_BUILTIN_MAX + 1:PSA_KEY_TYPE_AES:128:PSA_ALG_CTR:"3677397A24432646294A404E63526655":PSA_ERROR_INVALID_HANDLE + +PSA opaque driver builtin key export: secp256r1 +builtin_key_export:MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):256:PSA_ALG_ECDSA(PSA_ALG_ANY_HASH):"dc7d9d26d67a4f632c34c2dc0b6986183882c206df04cdb7d69aabe28be4f81a":PSA_SUCCESS + +PSA opaque driver builtin pubkey export: secp256r1 +builtin_pubkey_export:MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):256:PSA_ALG_ECDSA(PSA_ALG_ANY_HASH):"0485f64d89f00be66c88dd937efd6d7c445648dcb701150b8a9509295850f41c1931e571fb8f8c78317a20b380e866584bbc2516c3d2702d792f131a922095fd6c":PSA_SUCCESS + +PSA opaque driver builtin pubkey export: not a public key +builtin_pubkey_export:MBEDTLS_PSA_KEY_ID_BUILTIN_MIN:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):256:PSA_ALG_ECDSA(PSA_ALG_ANY_HASH):"0485f64d89f00be66c88dd937efd6d7c445648dcb701150b8a9509295850f41c1931e571fb8f8c78317a20b380e866584bbc2516c3d2702d792f131a922095fd6c":PSA_ERROR_INVALID_ARGUMENT diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function index 20452b70c..449b52871 100644 --- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function +++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function @@ -936,3 +936,107 @@ exit: test_driver_aead_hooks = test_driver_aead_hooks_init(); } /* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ +void builtin_key_export( int builtin_key_id_arg, + int builtin_key_type_arg, + int builtin_key_bits_arg, + int builtin_key_algorithm_arg, + data_t *expected_output, + int expected_status_arg ) +{ + psa_key_id_t builtin_key_id = (psa_key_id_t) builtin_key_id_arg; + psa_key_type_t builtin_key_type = (psa_key_type_t) builtin_key_type_arg; + psa_algorithm_t builtin_key_alg = (psa_algorithm_t) builtin_key_algorithm_arg; + size_t builtin_key_bits = (size_t) builtin_key_bits_arg; + psa_status_t expected_status = expected_status_arg; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + + mbedtls_svc_key_id_t key = mbedtls_svc_key_id_make(0, builtin_key_id); + uint8_t* output_buffer = NULL; + size_t output_size = 0; + psa_status_t actual_status; + + PSA_ASSERT( psa_crypto_init( ) ); + ASSERT_ALLOC( output_buffer, expected_output->len ); + + actual_status = psa_export_key( key, output_buffer, expected_output->len, &output_size ); + + if( expected_status == PSA_SUCCESS ) + { + PSA_ASSERT( actual_status ); + TEST_EQUAL( output_size, expected_output->len ); + ASSERT_COMPARE( output_buffer, output_size, + expected_output->x, expected_output->len ); + + PSA_ASSERT( psa_get_key_attributes( key, &attributes ) ); + TEST_EQUAL( psa_get_key_bits( &attributes ), builtin_key_bits ); + TEST_EQUAL( psa_get_key_type( &attributes ), builtin_key_type ); + TEST_EQUAL( psa_get_key_algorithm( &attributes ), builtin_key_alg ); + } + else + { + if( actual_status != expected_status ) + fprintf(stderr, "Expected %d but got %d\n", expected_status, actual_status); + TEST_EQUAL( actual_status, expected_status ); + TEST_EQUAL( output_size, 0 ); + } + +exit: + mbedtls_free( output_buffer ); + psa_reset_key_attributes( &attributes ); + psa_destroy_key( key ); + PSA_DONE( ); +} +/* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ +void builtin_pubkey_export( int builtin_key_id_arg, + int builtin_key_type_arg, + int builtin_key_bits_arg, + int builtin_key_algorithm_arg, + data_t *expected_output, + int expected_status_arg ) +{ + psa_key_id_t builtin_key_id = (psa_key_id_t) builtin_key_id_arg; + psa_key_type_t builtin_key_type = (psa_key_type_t) builtin_key_type_arg; + psa_algorithm_t builtin_key_alg = (psa_algorithm_t) builtin_key_algorithm_arg; + size_t builtin_key_bits = (size_t) builtin_key_bits_arg; + psa_status_t expected_status = expected_status_arg; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + + mbedtls_svc_key_id_t key = mbedtls_svc_key_id_make(0, builtin_key_id); + uint8_t* output_buffer = NULL; + size_t output_size = 0; + psa_status_t actual_status; + + PSA_ASSERT( psa_crypto_init( ) ); + ASSERT_ALLOC( output_buffer, expected_output->len ); + + actual_status = psa_export_public_key( key, output_buffer, expected_output->len, &output_size ); + + if( expected_status == PSA_SUCCESS ) + { + PSA_ASSERT( actual_status ); + TEST_EQUAL( output_size, expected_output->len ); + ASSERT_COMPARE( output_buffer, output_size, + expected_output->x, expected_output->len ); + + PSA_ASSERT( psa_get_key_attributes( key, &attributes ) ); + TEST_EQUAL( psa_get_key_bits( &attributes ), builtin_key_bits ); + TEST_EQUAL( psa_get_key_type( &attributes ), builtin_key_type ); + TEST_EQUAL( psa_get_key_algorithm( &attributes ), builtin_key_alg ); + } + else + { + TEST_EQUAL( actual_status, expected_status ); + TEST_EQUAL( output_size, 0 ); + } + +exit: + mbedtls_free( output_buffer ); + psa_reset_key_attributes( &attributes ); + psa_destroy_key( key ); + PSA_DONE( ); +} +/* END_CASE */ From 5be864f6451445af364b45182f02907f2fb425df Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Mon, 22 Feb 2021 12:48:51 +0100 Subject: [PATCH 04/26] Add changelog for MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS Signed-off-by: Steven Cooreman --- ChangeLog.d/psa-builtin-keys-implementation.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/psa-builtin-keys-implementation.txt diff --git a/ChangeLog.d/psa-builtin-keys-implementation.txt b/ChangeLog.d/psa-builtin-keys-implementation.txt new file mode 100644 index 000000000..66ba77d07 --- /dev/null +++ b/ChangeLog.d/psa-builtin-keys-implementation.txt @@ -0,0 +1,4 @@ +Features + * Added support for built-in driver keys through the PSA opaque crypto + driver interface. Refer to the documentation of + MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS for more information. From e5e30859b7bffef72e993f91326b37ed89172714 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Mon, 22 Feb 2021 14:40:04 +0100 Subject: [PATCH 05/26] Remove potentially unused exit label Signed-off-by: Steven Cooreman --- library/psa_crypto_slot_management.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index c90ebee00..dfc03fd5a 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -364,15 +364,13 @@ psa_status_t psa_get_and_lock_key_slot( mbedtls_svc_key_id_t key, status = PSA_ERROR_DOES_NOT_EXIST; #if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) status = psa_load_builtin_key_into_slot( *p_slot ); - if( status == PSA_SUCCESS ) - goto exit; #endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) - status = psa_load_persistent_key_into_slot( *p_slot ); + if( status == PSA_ERROR_DOES_NOT_EXIST ) + status = psa_load_persistent_key_into_slot( *p_slot ); #endif /* defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */ -exit: if( status != PSA_SUCCESS ) { psa_wipe_key_slot( *p_slot ); From 203bcbbc47d824b12c95e7d8e3aa9cde3d0b410f Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 18 Mar 2021 17:17:40 +0100 Subject: [PATCH 06/26] Style fixes (typos, whitespace, 80 column limit) Signed-off-by: Steven Cooreman --- include/psa/crypto_extra.h | 6 +- library/psa_crypto_driver_wrappers.c | 2 +- library/psa_crypto_slot_management.c | 99 ++++++++++--------- tests/src/drivers/key_management.c | 75 +++++++++----- tests/src/helpers.c | 21 ++-- ..._suite_psa_crypto_driver_wrappers.function | 6 +- 6 files changed, 123 insertions(+), 86 deletions(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index f9a9aeeaf..34436e4d4 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -762,7 +762,7 @@ static inline int psa_key_id_is_builtin( psa_key_id_t key_id ) /** Platform function to obtain the data of a built-in key. * * An application-specific implementation of this function must be provided if - * #MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS is enabled. This would typically provided + * #MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS is enabled. This would typically be provided * as part of a platform's system image. * * Call psa_get_key_id(\p attributes) to obtain the key identifier \c key_id. @@ -780,7 +780,7 @@ static inline int psa_key_id_is_builtin( psa_key_id_t key_id ) * On successful return, this function must set * the attributes of the key: lifetime, type, * bit-size, usage policy. - * \param[out] slot_number On successful return, this function must + * \param[out] slot_number On successful return, this function must set * this to the slot number known to the driver for * the lifetime location reported through * \p attributes which corresponds to the @@ -794,7 +794,7 @@ static inline int psa_key_id_is_builtin( psa_key_id_t key_id ) * The requested key identifier is not a built-in key which is known * to this function. If a key exists in the key storage with this * identifier, the data from the storage will be used. - * \retval (any other error) + * \return (any other error) * Any other error is propagated to the function that requested the key. * Common errors include: * - #PSA_ERROR_NOT_PERMITTED: the key exists but the requested owner diff --git a/library/psa_crypto_driver_wrappers.c b/library/psa_crypto_driver_wrappers.c index 28087de07..160076e15 100644 --- a/library/psa_crypto_driver_wrappers.c +++ b/library/psa_crypto_driver_wrappers.c @@ -263,7 +263,7 @@ psa_status_t psa_driver_wrapper_get_key_buffer_size( MBEDTLS_SVC_KEY_ID_GET_KEY_ID( psa_get_key_id( attributes ) ) ) ) { - *key_buffer_size = sizeof(psa_drv_slot_number_t); + *key_buffer_size = sizeof( psa_drv_slot_number_t ); return( PSA_SUCCESS ); } #endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index dfc03fd5a..7a01f80fd 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -280,58 +280,59 @@ exit: static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) { /* Load keys in the 'builtin' range through their own interface */ - if( psa_key_id_is_builtin( MBEDTLS_SVC_KEY_ID_GET_KEY_ID( slot->attr.id ) ) ) + if( ! psa_key_id_is_builtin( + MBEDTLS_SVC_KEY_ID_GET_KEY_ID( slot->attr.id ) ) ) { - /* Check the platform function to see whether this key actually exists */ - psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - psa_drv_slot_number_t slot_number; - - psa_set_key_id(&attributes, slot->attr.id); - psa_status_t status = mbedtls_psa_platform_get_builtin_key( - &attributes, &slot_number ); - if( status != PSA_SUCCESS ) - return( status ); - - /* If the key should exist according to the platform, load it through - * the driver interface. */ - uint8_t *key_buffer = NULL; - size_t key_buffer_length = 0; - - status = psa_driver_wrapper_get_key_buffer_size( &attributes, &key_buffer_length ); - if( status != PSA_SUCCESS ) - return( status ); - - key_buffer = mbedtls_calloc( 1, key_buffer_length ); - if( key_buffer == NULL ) - return( PSA_ERROR_INSUFFICIENT_MEMORY ); - - status = psa_driver_wrapper_get_builtin_key( - slot_number, &attributes, - key_buffer, key_buffer_length, &key_buffer_length ); - if( status != PSA_SUCCESS ) - goto exit; - - status = psa_copy_key_material_into_slot( slot, key_buffer, key_buffer_length ); - if( status != PSA_SUCCESS ) - goto exit; - - /* Copy core attributes into the slot on success. - * Use static allocations to make the compiler yell at us should one - * of the two structures change type. */ - psa_core_key_attributes_t* builtin_key_core_attributes = - &attributes.core; - psa_core_key_attributes_t* slot_core_attributes = - &slot->attr; - memcpy( slot_core_attributes, - builtin_key_core_attributes, - sizeof(psa_core_key_attributes_t) ); - -exit: - mbedtls_free( key_buffer ); - return( status ); - } else { return( PSA_ERROR_DOES_NOT_EXIST ); } + + /* Check the platform function to see whether this key actually exists */ + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_drv_slot_number_t slot_number; + + psa_set_key_id( &attributes, slot->attr.id ); + psa_status_t status = mbedtls_psa_platform_get_builtin_key( + &attributes, &slot_number ); + if( status != PSA_SUCCESS ) + return( status ); + + /* If the key should exist according to the platform, load it through the + * driver interface. */ + uint8_t *key_buffer = NULL; + size_t key_buffer_length = 0; + + status = psa_driver_wrapper_get_key_buffer_size( &attributes, + &key_buffer_length ); + if( status != PSA_SUCCESS ) + return( status ); + + key_buffer = mbedtls_calloc( 1, key_buffer_length ); + if( key_buffer == NULL ) + return( PSA_ERROR_INSUFFICIENT_MEMORY ); + + status = psa_driver_wrapper_get_builtin_key( + slot_number, &attributes, + key_buffer, key_buffer_length, &key_buffer_length ); + if( status != PSA_SUCCESS ) + goto exit; + + status = psa_copy_key_material_into_slot( + slot, key_buffer, key_buffer_length ); + if( status != PSA_SUCCESS ) + goto exit; + + /* Copy core attributes into the slot on success. + * Use static allocations to make the compiler yell at us should one + * of the two structures change type. */ + psa_core_key_attributes_t* builtin_key_core_attributes = &attributes.core; + psa_core_key_attributes_t* slot_core_attributes = &slot->attr; + memcpy( slot_core_attributes, + builtin_key_core_attributes, + sizeof( psa_core_key_attributes_t ) ); + +exit: + mbedtls_free( key_buffer ); + return( status ); } #endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ diff --git a/tests/src/drivers/key_management.c b/tests/src/drivers/key_management.c index 77a217f06..ca00fe0e8 100644 --- a/tests/src/drivers/key_management.c +++ b/tests/src/drivers/key_management.c @@ -61,8 +61,10 @@ const uint8_t test_driver_ecdsa_pubkey[65] = 0xbc, 0x25, 0x16, 0xc3, 0xd2, 0x70, 0x2d, 0x79, 0x2f, 0x13, 0x1a, 0x92, 0x20, 0x95, 0xfd, 0x6c }; -static const psa_drv_slot_number_t aes_slot = PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT; -static const psa_drv_slot_number_t ecdsa_slot = PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT; +static const psa_drv_slot_number_t aes_slot = + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT; +static const psa_drv_slot_number_t ecdsa_slot = + PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT; #endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ psa_status_t test_transparent_generate_key( @@ -179,41 +181,49 @@ psa_status_t test_opaque_export_key( uint8_t *data, size_t data_size, size_t *data_length ) { #if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) - if( psa_key_id_is_builtin( MBEDTLS_SVC_KEY_ID_GET_KEY_ID( psa_get_key_id( attributes ) ) ) ) + if( psa_key_id_is_builtin( + MBEDTLS_SVC_KEY_ID_GET_KEY_ID( psa_get_key_id( attributes ) ) ) ) { if( key_length != sizeof( psa_drv_slot_number_t ) ) return( PSA_ERROR_INVALID_ARGUMENT ); if( memcmp( key, &ecdsa_slot, sizeof( psa_drv_slot_number_t ) ) == 0 ) { - /* This is the ECDSA slot. Verify key attributes before returning pubkey. */ - if( psa_get_key_type( attributes ) != PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) + /* This is the ECDSA slot. Verify key attributes before returning + * the private key. */ + if( psa_get_key_type( attributes ) != + PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) return( PSA_ERROR_CORRUPTION_DETECTED ); if( psa_get_key_bits( attributes ) != 256 ) return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_algorithm( attributes ) != PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) + if( psa_get_key_algorithm( attributes ) != + PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) return( PSA_ERROR_CORRUPTION_DETECTED ); - if( (psa_get_key_usage_flags( attributes ) & PSA_KEY_USAGE_EXPORT) == 0 ) + if( ( psa_get_key_usage_flags( attributes ) & + PSA_KEY_USAGE_EXPORT ) == 0 ) return( PSA_ERROR_CORRUPTION_DETECTED ); if( data_size < sizeof( test_driver_ecdsa_key ) ) return( PSA_ERROR_BUFFER_TOO_SMALL ); - memcpy( data, test_driver_ecdsa_key, sizeof( test_driver_ecdsa_key ) ); + memcpy( data, test_driver_ecdsa_key, + sizeof( test_driver_ecdsa_key ) ); *data_length = sizeof( test_driver_ecdsa_key ); return( PSA_SUCCESS ); } if( memcmp( key, &aes_slot, sizeof( psa_drv_slot_number_t ) ) == 0 ) { - /* This is the ECDSA slot. Verify key attributes before returning pubkey. */ + /* This is the AES slot. Verify key attributes before returning + * the key. */ if( psa_get_key_type( attributes ) != PSA_KEY_TYPE_AES ) return( PSA_ERROR_CORRUPTION_DETECTED ); if( psa_get_key_bits( attributes ) != 128 ) return( PSA_ERROR_CORRUPTION_DETECTED ); if( psa_get_key_algorithm( attributes ) != PSA_ALG_CTR ) return( PSA_ERROR_CORRUPTION_DETECTED ); - if( (psa_get_key_usage_flags( attributes ) & PSA_KEY_USAGE_EXPORT) == 0 ) + if( ( psa_get_key_usage_flags( attributes ) & + PSA_KEY_USAGE_EXPORT ) == 0 ) return( PSA_ERROR_CORRUPTION_DETECTED ); if( data_size < sizeof( test_driver_aes_key ) ) @@ -299,25 +309,30 @@ psa_status_t test_opaque_export_public_key( uint8_t *data, size_t data_size, size_t *data_length ) { #if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) - if( psa_key_id_is_builtin( MBEDTLS_SVC_KEY_ID_GET_KEY_ID( psa_get_key_id( attributes ) ) ) ) + if( psa_key_id_is_builtin( + MBEDTLS_SVC_KEY_ID_GET_KEY_ID( psa_get_key_id( attributes ) ) ) ) { if( key_length != sizeof( psa_drv_slot_number_t ) ) return( PSA_ERROR_INVALID_ARGUMENT ); if( memcmp( key, &ecdsa_slot, sizeof( psa_drv_slot_number_t ) ) == 0 ) { - /* This is the ECDSA slot. Verify key attributes before returning pubkey. */ - if( psa_get_key_type( attributes ) != PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) + /* This is the ECDSA slot. Verify key attributes before returning + * the public key. */ + if( psa_get_key_type( attributes ) != + PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) return( PSA_ERROR_CORRUPTION_DETECTED ); if( psa_get_key_bits( attributes ) != 256 ) return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_algorithm( attributes ) != PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) + if( psa_get_key_algorithm( attributes ) != + PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) return( PSA_ERROR_CORRUPTION_DETECTED ); if( data_size < sizeof( test_driver_ecdsa_pubkey ) ) return( PSA_ERROR_BUFFER_TOO_SMALL ); - memcpy(data, test_driver_ecdsa_pubkey, sizeof( test_driver_ecdsa_pubkey ) ); + memcpy( data, test_driver_ecdsa_pubkey, + sizeof( test_driver_ecdsa_pubkey ) ); *data_length = sizeof( test_driver_ecdsa_pubkey ); return( PSA_SUCCESS ); } @@ -338,10 +353,13 @@ psa_status_t test_opaque_export_public_key( /* The opaque test driver exposes two built-in keys when builtin key support is * compiled in. - * The key in slot #PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT is an AES-128 key which allows CTR mode - * The key in slot #PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT is a secp256r1 private key which allows ECDSA sign & verify + * The key in slot #PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT is an AES-128 + * key which allows CTR mode. + * The key in slot #PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT is a secp256r1 + * private key which allows ECDSA sign & verify. * The key buffer format for these is the raw format of psa_drv_slot_number_t - * (i.e. for an actual driver this would mean 'builtin_key_size' = sizeof(psa_drv_slot_number_t)) + * (i.e. for an actual driver this would mean 'builtin_key_size' = + * sizeof(psa_drv_slot_number_t)). */ psa_status_t test_opaque_get_builtin_key( psa_drv_slot_number_t slot_number, @@ -357,7 +375,11 @@ psa_status_t test_opaque_get_builtin_key( psa_set_key_type( attributes, PSA_KEY_TYPE_AES ); psa_set_key_bits( attributes, 128 ); - psa_set_key_usage_flags( attributes, PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT | PSA_KEY_USAGE_EXPORT ); + psa_set_key_usage_flags( + attributes, + PSA_KEY_USAGE_ENCRYPT | + PSA_KEY_USAGE_DECRYPT | + PSA_KEY_USAGE_EXPORT ); psa_set_key_algorithm( attributes, PSA_ALG_CTR ); *( (psa_drv_slot_number_t*) key_buffer ) = @@ -368,12 +390,19 @@ psa_status_t test_opaque_get_builtin_key( if( key_buffer_size < sizeof( psa_drv_slot_number_t ) ) return( PSA_ERROR_BUFFER_TOO_SMALL ); - psa_set_key_type( attributes, PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ); + psa_set_key_type( + attributes, + PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ); psa_set_key_bits( attributes, 256 ); - psa_set_key_usage_flags( attributes, PSA_KEY_USAGE_SIGN_HASH | PSA_KEY_USAGE_VERIFY_HASH | PSA_KEY_USAGE_EXPORT ); - psa_set_key_algorithm( attributes, PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ); + psa_set_key_usage_flags( + attributes, + PSA_KEY_USAGE_SIGN_HASH | + PSA_KEY_USAGE_VERIFY_HASH | + PSA_KEY_USAGE_EXPORT ); + psa_set_key_algorithm( + attributes, PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ); - *( (psa_drv_slot_number_t*) key_buffer) = + *( (psa_drv_slot_number_t*) key_buffer ) = PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT; *key_buffer_length = sizeof( psa_drv_slot_number_t ); return( PSA_SUCCESS ); diff --git a/tests/src/helpers.c b/tests/src/helpers.c index ee7fa209c..75f55e371 100644 --- a/tests/src/helpers.c +++ b/tests/src/helpers.c @@ -301,12 +301,18 @@ static const mbedtls_psa_builtin_key_description_t builtin_keys[] = { #if defined(PSA_CRYPTO_DRIVER_TEST) /* For testing, assign the AES builtin key slot to the boundary values. * ECDSA can be exercised on key ID MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1. */ - {MBEDTLS_PSA_KEY_ID_BUILTIN_MIN - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, - {MBEDTLS_PSA_KEY_ID_BUILTIN_MIN, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, - {MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT}, - {MBEDTLS_PSA_KEY_ID_BUILTIN_MAX - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, - {MBEDTLS_PSA_KEY_ID_BUILTIN_MAX, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, - {MBEDTLS_PSA_KEY_ID_BUILTIN_MAX + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT}, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, #else {0, 0, 0} #endif @@ -318,7 +324,8 @@ psa_status_t mbedtls_psa_platform_get_builtin_key( mbedtls_svc_key_id_t svc_key_id = psa_get_key_id( attributes ); psa_key_id_t app_key_id = MBEDTLS_SVC_KEY_ID_GET_KEY_ID( svc_key_id ); - for( size_t i = 0; i < ( sizeof( builtin_keys ) / sizeof( builtin_keys[0] ) ); i++ ) + for( size_t i = 0; + i < ( sizeof( builtin_keys ) / sizeof( builtin_keys[0] ) ); i++ ) { if( builtin_keys[i].builtin_key_id == app_key_id ) { diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function index 449b52871..eb6dce941 100644 --- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function +++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function @@ -952,7 +952,7 @@ void builtin_key_export( int builtin_key_id_arg, psa_status_t expected_status = expected_status_arg; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - mbedtls_svc_key_id_t key = mbedtls_svc_key_id_make(0, builtin_key_id); + mbedtls_svc_key_id_t key = mbedtls_svc_key_id_make( 0, builtin_key_id ); uint8_t* output_buffer = NULL; size_t output_size = 0; psa_status_t actual_status; @@ -977,7 +977,7 @@ void builtin_key_export( int builtin_key_id_arg, else { if( actual_status != expected_status ) - fprintf(stderr, "Expected %d but got %d\n", expected_status, actual_status); + fprintf( stderr, "Expected %d but got %d\n", expected_status, actual_status ); TEST_EQUAL( actual_status, expected_status ); TEST_EQUAL( output_size, 0 ); } @@ -1005,7 +1005,7 @@ void builtin_pubkey_export( int builtin_key_id_arg, psa_status_t expected_status = expected_status_arg; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - mbedtls_svc_key_id_t key = mbedtls_svc_key_id_make(0, builtin_key_id); + mbedtls_svc_key_id_t key = mbedtls_svc_key_id_make( 0, builtin_key_id ); uint8_t* output_buffer = NULL; size_t output_size = 0; psa_status_t actual_status; From 85d554a99b155adb60688d0ea34834cc4947615f Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 18 Mar 2021 17:19:30 +0100 Subject: [PATCH 07/26] Use different variables for buffer size and data length Signed-off-by: Steven Cooreman --- library/psa_crypto_slot_management.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 7a01f80fd..93de27331 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -299,20 +299,21 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) /* If the key should exist according to the platform, load it through the * driver interface. */ uint8_t *key_buffer = NULL; + size_t key_buffer_size = 0; size_t key_buffer_length = 0; status = psa_driver_wrapper_get_key_buffer_size( &attributes, - &key_buffer_length ); + &key_buffer_size ); if( status != PSA_SUCCESS ) return( status ); - key_buffer = mbedtls_calloc( 1, key_buffer_length ); + key_buffer = mbedtls_calloc( 1, key_buffer_size ); if( key_buffer == NULL ) return( PSA_ERROR_INSUFFICIENT_MEMORY ); status = psa_driver_wrapper_get_builtin_key( slot_number, &attributes, - key_buffer, key_buffer_length, &key_buffer_length ); + key_buffer, key_buffer_size, &key_buffer_length ); if( status != PSA_SUCCESS ) goto exit; From ffc7fc9b7152554f0ff88c78d670afe857d871f7 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 18 Mar 2021 17:33:46 +0100 Subject: [PATCH 08/26] Move variable declarations to top of function Signed-off-by: Steven Cooreman --- library/psa_crypto_slot_management.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 93de27331..ea3a5fc6b 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -279,6 +279,13 @@ exit: static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) { + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_drv_slot_number_t slot_number = 0; + uint8_t *key_buffer = NULL; + size_t key_buffer_size = 0; + size_t key_buffer_length = 0; + /* Load keys in the 'builtin' range through their own interface */ if( ! psa_key_id_is_builtin( MBEDTLS_SVC_KEY_ID_GET_KEY_ID( slot->attr.id ) ) ) @@ -287,21 +294,13 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) } /* Check the platform function to see whether this key actually exists */ - psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - psa_drv_slot_number_t slot_number; - psa_set_key_id( &attributes, slot->attr.id ); - psa_status_t status = mbedtls_psa_platform_get_builtin_key( - &attributes, &slot_number ); + status = mbedtls_psa_platform_get_builtin_key( &attributes, &slot_number ); if( status != PSA_SUCCESS ) return( status ); /* If the key should exist according to the platform, load it through the * driver interface. */ - uint8_t *key_buffer = NULL; - size_t key_buffer_size = 0; - size_t key_buffer_length = 0; - status = psa_driver_wrapper_get_key_buffer_size( &attributes, &key_buffer_size ); if( status != PSA_SUCCESS ) From 649a8f43017004b0809c200be767a575c6b0a476 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 18 Mar 2021 17:34:55 +0100 Subject: [PATCH 09/26] replace memcpy of structure with regular assignment Signed-off-by: Steven Cooreman --- library/psa_crypto_slot_management.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index ea3a5fc6b..5428b43a3 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -321,14 +321,8 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) if( status != PSA_SUCCESS ) goto exit; - /* Copy core attributes into the slot on success. - * Use static allocations to make the compiler yell at us should one - * of the two structures change type. */ - psa_core_key_attributes_t* builtin_key_core_attributes = &attributes.core; - psa_core_key_attributes_t* slot_core_attributes = &slot->attr; - memcpy( slot_core_attributes, - builtin_key_core_attributes, - sizeof( psa_core_key_attributes_t ) ); + /* Copy core attributes into the slot on success */ + slot->attr = attributes.core; exit: mbedtls_free( key_buffer ); From 33a32f024f73b5a1e55f1df2682e05e97a891891 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 18 Mar 2021 18:43:15 +0100 Subject: [PATCH 10/26] Move test driver implementation of platform_get_builtin_key Move to its own file in the test tree, to simplify platform vendors providing their own implementation. Signed-off-by: Steven Cooreman --- tests/src/drivers/platform_builtin_keys.c | 81 +++++++++++++++++++++++ tests/src/helpers.c | 59 ----------------- 2 files changed, 81 insertions(+), 59 deletions(-) create mode 100644 tests/src/drivers/platform_builtin_keys.c diff --git a/tests/src/drivers/platform_builtin_keys.c b/tests/src/drivers/platform_builtin_keys.c new file mode 100644 index 000000000..131343a90 --- /dev/null +++ b/tests/src/drivers/platform_builtin_keys.c @@ -0,0 +1,81 @@ +/** \file platform_builtin_keys.c + * + * \brief Test driver implementation of the builtin key support + */ + +/* + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#if defined(PSA_CRYPTO_DRIVER_TEST) +#include +#endif + +typedef struct +{ + psa_key_id_t builtin_key_id; + psa_key_location_t location; + psa_drv_slot_number_t slot_number; +} mbedtls_psa_builtin_key_description_t; + +static const mbedtls_psa_builtin_key_description_t builtin_keys[] = { +#if defined(PSA_CRYPTO_DRIVER_TEST) + /* For testing, assign the AES builtin key slot to the boundary values. + * ECDSA can be exercised on key ID MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1. */ + { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT}, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, +#else + {0, 0, 0} +#endif +}; + +psa_status_t mbedtls_psa_platform_get_builtin_key( + psa_key_attributes_t *attributes, psa_drv_slot_number_t *slot_number ) +{ + mbedtls_svc_key_id_t svc_key_id = psa_get_key_id( attributes ); + psa_key_id_t app_key_id = MBEDTLS_SVC_KEY_ID_GET_KEY_ID( svc_key_id ); + const mbedtls_psa_builtin_key_description_t *builtin_key; + + for( size_t i = 0; + i < ( sizeof( builtin_keys ) / sizeof( builtin_keys[0] ) ); i++ ) + { + builtin_key = &builtin_keys[i]; + if( builtin_key->builtin_key_id == app_key_id ) + { + psa_set_key_lifetime( attributes, + PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( + PSA_KEY_PERSISTENCE_READ_ONLY, + builtin_key->location ) ); + *slot_number = builtin_key->slot_number; + return( PSA_SUCCESS ); + } + } + + return( PSA_ERROR_DOES_NOT_EXIST ); +} diff --git a/tests/src/helpers.c b/tests/src/helpers.c index 75f55e371..e323275e5 100644 --- a/tests/src/helpers.c +++ b/tests/src/helpers.c @@ -282,62 +282,3 @@ void mbedtls_param_failed( const char *failure_condition, } } #endif /* MBEDTLS_CHECK_PARAMS */ - -#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) -#include - -#if defined(PSA_CRYPTO_DRIVER_TEST) -#include "test/drivers/test_driver.h" -#endif - -typedef struct -{ - psa_key_id_t builtin_key_id; - psa_key_location_t location; - psa_drv_slot_number_t slot_number; -} mbedtls_psa_builtin_key_description_t; - -static const mbedtls_psa_builtin_key_description_t builtin_keys[] = { -#if defined(PSA_CRYPTO_DRIVER_TEST) - /* For testing, assign the AES builtin key slot to the boundary values. - * ECDSA can be exercised on key ID MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1. */ - { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, - { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, - { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT}, - { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, - { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, - { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, -#else - {0, 0, 0} -#endif -}; - -psa_status_t mbedtls_psa_platform_get_builtin_key( - psa_key_attributes_t *attributes, psa_drv_slot_number_t *slot_number ) -{ - mbedtls_svc_key_id_t svc_key_id = psa_get_key_id( attributes ); - psa_key_id_t app_key_id = MBEDTLS_SVC_KEY_ID_GET_KEY_ID( svc_key_id ); - - for( size_t i = 0; - i < ( sizeof( builtin_keys ) / sizeof( builtin_keys[0] ) ); i++ ) - { - if( builtin_keys[i].builtin_key_id == app_key_id ) - { - psa_set_key_lifetime( attributes, - PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( - PSA_KEY_PERSISTENCE_READ_ONLY, - builtin_keys[i].location ) ); - *slot_number = builtin_keys[i].slot_number; - return( PSA_SUCCESS ); - } - } - - return( PSA_ERROR_DOES_NOT_EXIST ); -} -#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ From e384252cb7b823e7274926b62c41ebad0da0fd00 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 18 Mar 2021 18:52:44 +0100 Subject: [PATCH 11/26] Move include to top of file Signed-off-by: Steven Cooreman --- library/psa_crypto_slot_management.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 5428b43a3..68943c178 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -26,6 +26,7 @@ #include "psa/crypto.h" #include "psa_crypto_core.h" +#include "psa_crypto_driver_wrappers.h" #include "psa_crypto_slot_management.h" #include "psa_crypto_storage.h" #if defined(MBEDTLS_PSA_CRYPTO_SE_C) @@ -275,7 +276,6 @@ exit: #endif /* MBEDTLS_PSA_CRYPTO_STORAGE_C */ #if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) -#include "psa_crypto_driver_wrappers.h" static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) { From 1a0fbacde1f2fbc85eded52eed917dbb5131b804 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 18 Mar 2021 19:19:53 +0100 Subject: [PATCH 12/26] Refactor opaque key handling in the test driver Builtin key support for the test driver is always compiled in, and no longer guarded by MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS. Parsing the key slot from the buffer by cast and assign instead of memcmp. For exporting keys, the test driver no longer reaches into the key identifier in order to check whether a key is builtin, but rather assumes so based on the key buffer length. It's the driver's responsibility to be able to detect the key material it returned as part of the get_builtin_key operation. Signed-off-by: Steven Cooreman --- library/psa_crypto_driver_wrappers.c | 4 - tests/include/test/drivers/key_management.h | 5 - tests/src/drivers/key_management.c | 208 ++++++++------------ 3 files changed, 86 insertions(+), 131 deletions(-) diff --git a/library/psa_crypto_driver_wrappers.c b/library/psa_crypto_driver_wrappers.c index 160076e15..1910894ac 100644 --- a/library/psa_crypto_driver_wrappers.c +++ b/library/psa_crypto_driver_wrappers.c @@ -594,14 +594,10 @@ psa_status_t psa_driver_wrapper_get_builtin_key( { #if defined(PSA_CRYPTO_DRIVER_TEST) case PSA_CRYPTO_TEST_DRIVER_LIFETIME: -#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) return( test_opaque_get_builtin_key( slot_number, attributes, key_buffer, key_buffer_size, key_buffer_length ) ); -#else - return( PSA_ERROR_DOES_NOT_EXIST ); -#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ #endif /* PSA_CRYPTO_DRIVER_TEST */ default: (void) slot_number; diff --git a/tests/include/test/drivers/key_management.h b/tests/include/test/drivers/key_management.h index cf6fbb0b0..100fc18d3 100644 --- a/tests/include/test/drivers/key_management.h +++ b/tests/include/test/drivers/key_management.h @@ -29,14 +29,9 @@ #if defined(PSA_CRYPTO_DRIVER_TEST) #include -#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) #define PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT 0 #define PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT 1 -extern const uint8_t test_driver_aes_key[16]; -extern const uint8_t test_driver_ecdsa_key[32]; -#endif - typedef struct { /* If non-null, on success, copy this to the output. */ void *forced_output; diff --git a/tests/src/drivers/key_management.c b/tests/src/drivers/key_management.c index ca00fe0e8..e908daf46 100644 --- a/tests/src/drivers/key_management.c +++ b/tests/src/drivers/key_management.c @@ -41,7 +41,6 @@ test_driver_key_management_hooks_t test_driver_key_management_hooks = TEST_DRIVER_KEY_MANAGEMENT_INIT; -#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) const uint8_t test_driver_aes_key[16] = { 0x36, 0x77, 0x39, 0x7A, 0x24, 0x43, 0x26, 0x46, 0x29, 0x4A, 0x40, 0x4E, 0x63, 0x52, 0x66, 0x55 }; @@ -61,12 +60,6 @@ const uint8_t test_driver_ecdsa_pubkey[65] = 0xbc, 0x25, 0x16, 0xc3, 0xd2, 0x70, 0x2d, 0x79, 0x2f, 0x13, 0x1a, 0x92, 0x20, 0x95, 0xfd, 0x6c }; -static const psa_drv_slot_number_t aes_slot = - PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT; -static const psa_drv_slot_number_t ecdsa_slot = - PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT; -#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ - psa_status_t test_transparent_generate_key( const psa_key_attributes_t *attributes, uint8_t *key, size_t key_size, size_t *key_length ) @@ -180,72 +173,66 @@ psa_status_t test_opaque_export_key( const uint8_t *key, size_t key_length, uint8_t *data, size_t data_size, size_t *data_length ) { -#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) - if( psa_key_id_is_builtin( - MBEDTLS_SVC_KEY_ID_GET_KEY_ID( psa_get_key_id( attributes ) ) ) ) + if( key_length == sizeof( psa_drv_slot_number_t ) ) { - if( key_length != sizeof( psa_drv_slot_number_t ) ) - return( PSA_ERROR_INVALID_ARGUMENT ); + /* Assume this is a builtin key based on the key material length. */ + psa_drv_slot_number_t slot_number = *( ( psa_drv_slot_number_t* ) key ); - if( memcmp( key, &ecdsa_slot, sizeof( psa_drv_slot_number_t ) ) == 0 ) + switch( slot_number ) { - /* This is the ECDSA slot. Verify key attributes before returning - * the private key. */ - if( psa_get_key_type( attributes ) != - PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_bits( attributes ) != 256 ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_algorithm( attributes ) != - PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( ( psa_get_key_usage_flags( attributes ) & - PSA_KEY_USAGE_EXPORT ) == 0 ) - return( PSA_ERROR_CORRUPTION_DETECTED ); + case PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT: + /* This is the ECDSA slot. Verify the key's attributes before + * returning the private key. */ + if( psa_get_key_type( attributes ) != + PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_bits( attributes ) != 256 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_algorithm( attributes ) != + PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( ( psa_get_key_usage_flags( attributes ) & + PSA_KEY_USAGE_EXPORT ) == 0 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); - if( data_size < sizeof( test_driver_ecdsa_key ) ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); + if( data_size < sizeof( test_driver_ecdsa_key ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); - memcpy( data, test_driver_ecdsa_key, - sizeof( test_driver_ecdsa_key ) ); - *data_length = sizeof( test_driver_ecdsa_key ); - return( PSA_SUCCESS ); + memcpy( data, test_driver_ecdsa_key, + sizeof( test_driver_ecdsa_key ) ); + *data_length = sizeof( test_driver_ecdsa_key ); + return( PSA_SUCCESS ); + + case PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT: + /* This is the AES slot. Verify the key's attributes before + * returning the key. */ + if( psa_get_key_type( attributes ) != PSA_KEY_TYPE_AES ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_bits( attributes ) != 128 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_algorithm( attributes ) != PSA_ALG_CTR ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( ( psa_get_key_usage_flags( attributes ) & + PSA_KEY_USAGE_EXPORT ) == 0 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + + if( data_size < sizeof( test_driver_aes_key ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + + memcpy( data, test_driver_aes_key, + sizeof( test_driver_aes_key ) ); + *data_length = sizeof( test_driver_aes_key ); + return( PSA_SUCCESS ); + + default: + return( PSA_ERROR_DOES_NOT_EXIST ); } - - if( memcmp( key, &aes_slot, sizeof( psa_drv_slot_number_t ) ) == 0 ) - { - /* This is the AES slot. Verify key attributes before returning - * the key. */ - if( psa_get_key_type( attributes ) != PSA_KEY_TYPE_AES ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_bits( attributes ) != 128 ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_algorithm( attributes ) != PSA_ALG_CTR ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( ( psa_get_key_usage_flags( attributes ) & - PSA_KEY_USAGE_EXPORT ) == 0 ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - - if( data_size < sizeof( test_driver_aes_key ) ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); - - memcpy( data, test_driver_aes_key, sizeof( test_driver_aes_key ) ); - *data_length = sizeof( test_driver_aes_key ); - return( PSA_SUCCESS ); - } - - /* Potentially add more slots here */ - - return( PSA_ERROR_DOES_NOT_EXIST ); } -#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ - (void) attributes; - (void) key; - (void) key_length; - (void) data; - (void) data_size; - (void) data_length; - return( PSA_ERROR_NOT_SUPPORTED ); + else + { + /* Test driver does not support generic opaque key handling yet. */ + return( PSA_ERROR_NOT_SUPPORTED ); + } } psa_status_t test_transparent_export_public_key( @@ -308,47 +295,41 @@ psa_status_t test_opaque_export_public_key( const uint8_t *key, size_t key_length, uint8_t *data, size_t data_size, size_t *data_length ) { -#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) - if( psa_key_id_is_builtin( - MBEDTLS_SVC_KEY_ID_GET_KEY_ID( psa_get_key_id( attributes ) ) ) ) + if( key_length == sizeof( psa_drv_slot_number_t ) ) { - if( key_length != sizeof( psa_drv_slot_number_t ) ) - return( PSA_ERROR_INVALID_ARGUMENT ); - - if( memcmp( key, &ecdsa_slot, sizeof( psa_drv_slot_number_t ) ) == 0 ) + /* Assume this is a builtin key based on the key material length. */ + psa_drv_slot_number_t slot_number = *( ( psa_drv_slot_number_t* ) key ); + switch( slot_number ) { - /* This is the ECDSA slot. Verify key attributes before returning - * the public key. */ - if( psa_get_key_type( attributes ) != - PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_bits( attributes ) != 256 ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_algorithm( attributes ) != - PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) - return( PSA_ERROR_CORRUPTION_DETECTED ); + case PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT: + /* This is the ECDSA slot. Verify the key's attributes before + * returning the public key. */ + if( psa_get_key_type( attributes ) != + PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_bits( attributes ) != 256 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_algorithm( attributes ) != + PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); - if( data_size < sizeof( test_driver_ecdsa_pubkey ) ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); + if( data_size < sizeof( test_driver_ecdsa_pubkey ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); - memcpy( data, test_driver_ecdsa_pubkey, - sizeof( test_driver_ecdsa_pubkey ) ); - *data_length = sizeof( test_driver_ecdsa_pubkey ); - return( PSA_SUCCESS ); + memcpy( data, test_driver_ecdsa_pubkey, + sizeof( test_driver_ecdsa_pubkey ) ); + *data_length = sizeof( test_driver_ecdsa_pubkey ); + return( PSA_SUCCESS ); + + default: + return( PSA_ERROR_DOES_NOT_EXIST ); } - - /* Potentially add more slots here */ - - return( PSA_ERROR_DOES_NOT_EXIST ); } -#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ - (void) attributes; - (void) key; - (void) key_length; - (void) data; - (void) data_size; - (void) data_length; - return( PSA_ERROR_NOT_SUPPORTED ); + else + { + /* Test driver does not support generic opaque key handling yet. */ + return( PSA_ERROR_NOT_SUPPORTED ); + } } /* The opaque test driver exposes two built-in keys when builtin key support is @@ -366,13 +347,12 @@ psa_status_t test_opaque_get_builtin_key( psa_key_attributes_t *attributes, uint8_t *key_buffer, size_t key_buffer_size, size_t *key_buffer_length ) { -#if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) + if( key_buffer_size < sizeof( psa_drv_slot_number_t ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + switch( slot_number ) { case PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT: - if( key_buffer_size < sizeof( psa_drv_slot_number_t ) ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); - psa_set_key_type( attributes, PSA_KEY_TYPE_AES ); psa_set_key_bits( attributes, 128 ); psa_set_key_usage_flags( @@ -387,9 +367,6 @@ psa_status_t test_opaque_get_builtin_key( *key_buffer_length = sizeof( psa_drv_slot_number_t ); return( PSA_SUCCESS ); case PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT: - if( key_buffer_size < sizeof( psa_drv_slot_number_t ) ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); - psa_set_key_type( attributes, PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ); @@ -407,21 +384,8 @@ psa_status_t test_opaque_get_builtin_key( *key_buffer_length = sizeof( psa_drv_slot_number_t ); return( PSA_SUCCESS ); default: - (void) slot_number; - (void) attributes; - (void) key_buffer; - (void) key_buffer_size; - (void) key_buffer_length; - return( PSA_ERROR_INVALID_ARGUMENT ); + return( PSA_ERROR_DOES_NOT_EXIST ); } -#else /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ - (void) slot_number; - (void) attributes; - (void) key_buffer; - (void) key_buffer_size; - (void) key_buffer_length; - return( PSA_ERROR_DOES_NOT_EXIST ); -#endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ } #endif /* MBEDTLS_PSA_CRYPTO_DRIVERS && PSA_CRYPTO_DRIVER_TEST */ From 4b51925ede4c19524db3dd266d902b225da0997e Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 18 Mar 2021 20:25:53 +0100 Subject: [PATCH 13/26] Stricter test dependencies on builtin key test It requires the driver under test to be the actual software test driver. Signed-off-by: Steven Cooreman --- tests/suites/test_suite_psa_crypto_driver_wrappers.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function index eb6dce941..1eb087349 100644 --- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function +++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function @@ -990,7 +990,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ +/* BEGIN_CASE depends_on:PSA_CRYPTO_DRIVER_TEST:MBEDTLS_PSA_CRYPTO_DRIVERS:MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ void builtin_pubkey_export( int builtin_key_id_arg, int builtin_key_type_arg, int builtin_key_bits_arg, From c8b95343785b0bab44e88db71ff8bcae1db35299 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 18 Mar 2021 20:48:06 +0100 Subject: [PATCH 14/26] Change signature of mbedtls_psa_platform_get_builtin_key Instead of the full attributes struct, it now only takes/returns what it actually needs to. Signed-off-by: Steven Cooreman --- include/psa/crypto_extra.h | 28 ++++++------- library/psa_crypto_slot_management.c | 7 +++- tests/src/drivers/platform_builtin_keys.c | 50 ++++++++++++++--------- 3 files changed, 50 insertions(+), 35 deletions(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 34436e4d4..38d6c2029 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -759,14 +759,13 @@ static inline int psa_key_id_is_builtin( psa_key_id_t key_id ) ( key_id <= MBEDTLS_PSA_KEY_ID_BUILTIN_MAX ) ); } -/** Platform function to obtain the data of a built-in key. +/** Platform function to obtain the location and slot of a built-in key. * * An application-specific implementation of this function must be provided if * #MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS is enabled. This would typically be provided * as part of a platform's system image. * - * Call psa_get_key_id(\p attributes) to obtain the key identifier \c key_id. - * #MBEDTLS_SVC_KEY_ID_GET_KEY_ID(\p key_id) is in the range from + * #MBEDTLS_SVC_KEY_ID_GET_KEY_ID(\p key_id) needs to be in the range from * #MBEDTLS_PSA_KEY_ID_BUILTIN_MIN to #MBEDTLS_PSA_KEY_ID_BUILTIN_MAX. * * In a multi-application configuration @@ -774,16 +773,15 @@ static inline int psa_key_id_is_builtin( psa_key_id_t key_id ) * this function should check that #MBEDTLS_SVC_KEY_ID_GET_OWNER_ID(\p key_id) * is allowed to use the given key. * - * \param[in,out] attributes On entry, this is #PSA_KEY_ATTRIBUTES_INIT or - * an equivalent value, except that the key - * identifier field is set. - * On successful return, this function must set - * the attributes of the key: lifetime, type, - * bit-size, usage policy. - * \param[out] slot_number On successful return, this function must set - * this to the slot number known to the driver for - * the lifetime location reported through - * \p attributes which corresponds to the + * \param key_id The key ID for which to retrieve the + * location and slot attributes. + * \param[out] lifetime On success, the lifetime associated with the key + * corresponding to \p key_id. Lifetime is a + * combination of which driver contains the key, + * and with what lifecycle the key can be used. + * \param[out] slot_number On success, the slot number known to the driver + * registered at the lifetime location reported + * through \p location which corresponds to the * requested built-in key. * * \retval #PSA_SUCCESS @@ -801,7 +799,9 @@ static inline int psa_key_id_is_builtin( psa_key_id_t key_id ) * is not allowed to access it. */ psa_status_t mbedtls_psa_platform_get_builtin_key( - psa_key_attributes_t *attributes, psa_drv_slot_number_t *slot_number ); + mbedtls_svc_key_id_t key_id, + psa_key_lifetime_t *lifetime, + psa_drv_slot_number_t *slot_number ); #endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ /** @} */ diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 68943c178..232e54401 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -281,6 +281,7 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_key_lifetime_t lifetime = PSA_KEY_LIFETIME_VOLATILE; psa_drv_slot_number_t slot_number = 0; uint8_t *key_buffer = NULL; size_t key_buffer_size = 0; @@ -295,10 +296,14 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) /* Check the platform function to see whether this key actually exists */ psa_set_key_id( &attributes, slot->attr.id ); - status = mbedtls_psa_platform_get_builtin_key( &attributes, &slot_number ); + status = mbedtls_psa_platform_get_builtin_key( + slot->attr.id, &lifetime, &slot_number ); if( status != PSA_SUCCESS ) return( status ); + /* Set mapped lifetime on the attributes */ + psa_set_key_lifetime( &attributes, lifetime ); + /* If the key should exist according to the platform, load it through the * driver interface. */ status = psa_driver_wrapper_get_key_buffer_size( &attributes, diff --git a/tests/src/drivers/platform_builtin_keys.c b/tests/src/drivers/platform_builtin_keys.c index 131343a90..feccbfd0f 100644 --- a/tests/src/drivers/platform_builtin_keys.c +++ b/tests/src/drivers/platform_builtin_keys.c @@ -30,7 +30,7 @@ typedef struct { psa_key_id_t builtin_key_id; - psa_key_location_t location; + psa_key_lifetime_t lifetime; psa_drv_slot_number_t slot_number; } mbedtls_psa_builtin_key_description_t; @@ -38,28 +38,41 @@ static const mbedtls_psa_builtin_key_description_t builtin_keys[] = { #if defined(PSA_CRYPTO_DRIVER_TEST) /* For testing, assign the AES builtin key slot to the boundary values. * ECDSA can be exercised on key ID MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1. */ - { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, - { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, - { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT}, - { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX - 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, - { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, - { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX + 1, PSA_CRYPTO_TEST_DRIVER_LIFETIME, - PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN - 1, + PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN, + PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1, + PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT}, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX - 1, + PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX, + PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, + { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX + 1, + PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, #else {0, 0, 0} #endif }; psa_status_t mbedtls_psa_platform_get_builtin_key( - psa_key_attributes_t *attributes, psa_drv_slot_number_t *slot_number ) + mbedtls_svc_key_id_t key_id, + psa_key_lifetime_t *lifetime, + psa_drv_slot_number_t *slot_number ) { - mbedtls_svc_key_id_t svc_key_id = psa_get_key_id( attributes ); - psa_key_id_t app_key_id = MBEDTLS_SVC_KEY_ID_GET_KEY_ID( svc_key_id ); + psa_key_id_t app_key_id = MBEDTLS_SVC_KEY_ID_GET_KEY_ID( key_id ); const mbedtls_psa_builtin_key_description_t *builtin_key; for( size_t i = 0; @@ -68,10 +81,7 @@ psa_status_t mbedtls_psa_platform_get_builtin_key( builtin_key = &builtin_keys[i]; if( builtin_key->builtin_key_id == app_key_id ) { - psa_set_key_lifetime( attributes, - PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( - PSA_KEY_PERSISTENCE_READ_ONLY, - builtin_key->location ) ); + *lifetime = builtin_key->lifetime; *slot_number = builtin_key->slot_number; return( PSA_SUCCESS ); } From a1ce2f26759754e14e85c0f96c0029d9999fcc72 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 18 Mar 2021 20:49:29 +0100 Subject: [PATCH 15/26] Rename test driver lifetime to location The macro always meant 'location', but was mistakenly named 'lifetime'. Naming it location instead makes much more sense, and drives home the conceptual differences between location and lifetime values. Signed-off-by: Steven Cooreman --- library/psa_crypto_driver_wrappers.c | 22 +++++++++++----------- tests/include/test/drivers/test_driver.h | 2 +- tests/src/drivers/platform_builtin_keys.c | 12 ++++++------ 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/library/psa_crypto_driver_wrappers.c b/library/psa_crypto_driver_wrappers.c index 1910894ac..2a3075b95 100644 --- a/library/psa_crypto_driver_wrappers.c +++ b/library/psa_crypto_driver_wrappers.c @@ -129,7 +129,7 @@ psa_status_t psa_driver_wrapper_sign_hash( /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TEST_DRIVER_LIFETIME: + case PSA_CRYPTO_TEST_DRIVER_LOCATION: return( test_opaque_signature_sign_hash( attributes, key_buffer, key_buffer_size, @@ -211,7 +211,7 @@ psa_status_t psa_driver_wrapper_verify_hash( /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TEST_DRIVER_LIFETIME: + case PSA_CRYPTO_TEST_DRIVER_LOCATION: return( test_opaque_signature_verify_hash( attributes, key_buffer, key_buffer_size, @@ -256,7 +256,7 @@ psa_status_t psa_driver_wrapper_get_key_buffer_size( switch( location ) { #if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TEST_DRIVER_LIFETIME: + case PSA_CRYPTO_TEST_DRIVER_LOCATION: #if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) /* Emulate property 'builtin_key_size' */ if( psa_key_id_is_builtin( @@ -363,7 +363,7 @@ psa_status_t psa_driver_wrapper_generate_key( /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TEST_DRIVER_LIFETIME: + case PSA_CRYPTO_TEST_DRIVER_LOCATION: status = test_opaque_generate_key( attributes, key_buffer, key_buffer_size, key_buffer_length ); break; @@ -495,7 +495,7 @@ psa_status_t psa_driver_wrapper_export_key( /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TEST_DRIVER_LIFETIME: + case PSA_CRYPTO_TEST_DRIVER_LOCATION: return( test_opaque_export_key( attributes, key_buffer, key_buffer_size, @@ -569,7 +569,7 @@ psa_status_t psa_driver_wrapper_export_public_key( /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TEST_DRIVER_LIFETIME: + case PSA_CRYPTO_TEST_DRIVER_LOCATION: return( test_opaque_export_public_key( attributes, key_buffer, key_buffer_size, @@ -593,7 +593,7 @@ psa_status_t psa_driver_wrapper_get_builtin_key( switch( location ) { #if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TEST_DRIVER_LIFETIME: + case PSA_CRYPTO_TEST_DRIVER_LOCATION: return( test_opaque_get_builtin_key( slot_number, attributes, @@ -650,7 +650,7 @@ psa_status_t psa_driver_wrapper_cipher_encrypt( return( PSA_ERROR_NOT_SUPPORTED ); /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TEST_DRIVER_LIFETIME: + case PSA_CRYPTO_TEST_DRIVER_LOCATION: return( test_opaque_cipher_encrypt( &attributes, slot->key.data, slot->key.bytes, @@ -717,7 +717,7 @@ psa_status_t psa_driver_wrapper_cipher_decrypt( return( PSA_ERROR_NOT_SUPPORTED ); /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TEST_DRIVER_LIFETIME: + case PSA_CRYPTO_TEST_DRIVER_LOCATION: return( test_opaque_cipher_decrypt( &attributes, slot->key.data, slot->key.bytes, @@ -794,7 +794,7 @@ psa_status_t psa_driver_wrapper_cipher_encrypt_setup( /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TEST_DRIVER_LIFETIME: + case PSA_CRYPTO_TEST_DRIVER_LOCATION: status = test_opaque_cipher_encrypt_setup( &operation->ctx.opaque_test_driver_ctx, attributes, @@ -865,7 +865,7 @@ psa_status_t psa_driver_wrapper_cipher_decrypt_setup( /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TEST_DRIVER_LIFETIME: + case PSA_CRYPTO_TEST_DRIVER_LOCATION: status = test_opaque_cipher_decrypt_setup( &operation->ctx.opaque_test_driver_ctx, attributes, diff --git a/tests/include/test/drivers/test_driver.h b/tests/include/test/drivers/test_driver.h index 2fdce5c79..84d0caa5e 100644 --- a/tests/include/test/drivers/test_driver.h +++ b/tests/include/test/drivers/test_driver.h @@ -20,7 +20,7 @@ #ifndef PSA_CRYPTO_TEST_DRIVER_H #define PSA_CRYPTO_TEST_DRIVER_H -#define PSA_CRYPTO_TEST_DRIVER_LIFETIME 0x7fffff +#define PSA_CRYPTO_TEST_DRIVER_LOCATION 0x7fffff #include "test/drivers/aead.h" #include "test/drivers/signature.h" diff --git a/tests/src/drivers/platform_builtin_keys.c b/tests/src/drivers/platform_builtin_keys.c index feccbfd0f..759fa7830 100644 --- a/tests/src/drivers/platform_builtin_keys.c +++ b/tests/src/drivers/platform_builtin_keys.c @@ -40,27 +40,27 @@ static const mbedtls_psa_builtin_key_description_t builtin_keys[] = { * ECDSA can be exercised on key ID MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1. */ { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN - 1, PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( - PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LOCATION ), PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN, PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( - PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LOCATION ), PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT }, { MBEDTLS_PSA_KEY_ID_BUILTIN_MIN + 1, PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( - PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LOCATION ), PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT}, { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX - 1, PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( - PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LOCATION ), PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX, PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( - PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LOCATION ), PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, { MBEDTLS_PSA_KEY_ID_BUILTIN_MAX + 1, PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( - PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LIFETIME ), + PSA_KEY_PERSISTENCE_READ_ONLY, PSA_CRYPTO_TEST_DRIVER_LOCATION ), PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT}, #else {0, 0, 0} From b938b0bb03ef5c87339cc57eab5bce8be1999157 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Tue, 6 Apr 2021 13:08:42 +0200 Subject: [PATCH 16/26] Documentation clarification after review Signed-off-by: Steven Cooreman --- include/psa/crypto_extra.h | 4 ++-- library/psa_crypto_driver_wrappers.c | 4 ++-- library/psa_crypto_slot_management.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 38d6c2029..2c0e33ba7 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -759,7 +759,7 @@ static inline int psa_key_id_is_builtin( psa_key_id_t key_id ) ( key_id <= MBEDTLS_PSA_KEY_ID_BUILTIN_MAX ) ); } -/** Platform function to obtain the location and slot of a built-in key. +/** Platform function to obtain the location and slot number of a built-in key. * * An application-specific implementation of this function must be provided if * #MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS is enabled. This would typically be provided @@ -781,7 +781,7 @@ static inline int psa_key_id_is_builtin( psa_key_id_t key_id ) * and with what lifecycle the key can be used. * \param[out] slot_number On success, the slot number known to the driver * registered at the lifetime location reported - * through \p location which corresponds to the + * through \p lifetime which corresponds to the * requested built-in key. * * \retval #PSA_SUCCESS diff --git a/library/psa_crypto_driver_wrappers.c b/library/psa_crypto_driver_wrappers.c index 2a3075b95..11160d82d 100644 --- a/library/psa_crypto_driver_wrappers.c +++ b/library/psa_crypto_driver_wrappers.c @@ -229,8 +229,8 @@ psa_status_t psa_driver_wrapper_verify_hash( } } -/** Get the key buffer size for the key material of a generated key in the - * case of an opaque driver without storage. +/** Get the key buffer size required to store the key material of a key + * associated with an opaque driver without storage. * * \param[in] attributes The key attributes. * \param[out] key_buffer_size Minimum buffer size to contain the key material diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 232e54401..336bc3716 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -287,7 +287,6 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) size_t key_buffer_size = 0; size_t key_buffer_length = 0; - /* Load keys in the 'builtin' range through their own interface */ if( ! psa_key_id_is_builtin( MBEDTLS_SVC_KEY_ID_GET_KEY_ID( slot->attr.id ) ) ) { @@ -363,6 +362,7 @@ psa_status_t psa_get_and_lock_key_slot( mbedtls_svc_key_id_t key, status = PSA_ERROR_DOES_NOT_EXIST; #if defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) + /* Load keys in the 'builtin' range through their own interface */ status = psa_load_builtin_key_into_slot( *p_slot ); #endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ From 43e4a406d9f9e26fe3bea803abd9841e35d01a2d Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Tue, 6 Apr 2021 13:17:36 +0200 Subject: [PATCH 17/26] Give builtin key export test functions the same dependencies Signed-off-by: Steven Cooreman --- tests/suites/test_suite_psa_crypto_driver_wrappers.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function index 1eb087349..ad5b6c5fa 100644 --- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function +++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function @@ -937,7 +937,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ +/* BEGIN_CASE depends_on:PSA_CRYPTO_DRIVER_TEST:MBEDTLS_PSA_CRYPTO_DRIVERS:MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ void builtin_key_export( int builtin_key_id_arg, int builtin_key_type_arg, int builtin_key_bits_arg, From 054bf7f2a00bf7d14b4ba7350a1cc9039f23566f Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Tue, 6 Apr 2021 15:09:19 +0200 Subject: [PATCH 18/26] Reduce indentation need by checking negative case first Signed-off-by: Steven Cooreman --- tests/src/drivers/key_management.c | 170 ++++++++++++++--------------- 1 file changed, 83 insertions(+), 87 deletions(-) diff --git a/tests/src/drivers/key_management.c b/tests/src/drivers/key_management.c index e908daf46..5daec6bd5 100644 --- a/tests/src/drivers/key_management.c +++ b/tests/src/drivers/key_management.c @@ -173,66 +173,64 @@ psa_status_t test_opaque_export_key( const uint8_t *key, size_t key_length, uint8_t *data, size_t data_size, size_t *data_length ) { - if( key_length == sizeof( psa_drv_slot_number_t ) ) - { - /* Assume this is a builtin key based on the key material length. */ - psa_drv_slot_number_t slot_number = *( ( psa_drv_slot_number_t* ) key ); - - switch( slot_number ) - { - case PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT: - /* This is the ECDSA slot. Verify the key's attributes before - * returning the private key. */ - if( psa_get_key_type( attributes ) != - PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_bits( attributes ) != 256 ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_algorithm( attributes ) != - PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( ( psa_get_key_usage_flags( attributes ) & - PSA_KEY_USAGE_EXPORT ) == 0 ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - - if( data_size < sizeof( test_driver_ecdsa_key ) ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); - - memcpy( data, test_driver_ecdsa_key, - sizeof( test_driver_ecdsa_key ) ); - *data_length = sizeof( test_driver_ecdsa_key ); - return( PSA_SUCCESS ); - - case PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT: - /* This is the AES slot. Verify the key's attributes before - * returning the key. */ - if( psa_get_key_type( attributes ) != PSA_KEY_TYPE_AES ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_bits( attributes ) != 128 ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_algorithm( attributes ) != PSA_ALG_CTR ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( ( psa_get_key_usage_flags( attributes ) & - PSA_KEY_USAGE_EXPORT ) == 0 ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - - if( data_size < sizeof( test_driver_aes_key ) ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); - - memcpy( data, test_driver_aes_key, - sizeof( test_driver_aes_key ) ); - *data_length = sizeof( test_driver_aes_key ); - return( PSA_SUCCESS ); - - default: - return( PSA_ERROR_DOES_NOT_EXIST ); - } - } - else + if( key_length != sizeof( psa_drv_slot_number_t ) ) { /* Test driver does not support generic opaque key handling yet. */ return( PSA_ERROR_NOT_SUPPORTED ); } + + /* Assume this is a builtin key based on the key material length. */ + psa_drv_slot_number_t slot_number = *( ( psa_drv_slot_number_t* ) key ); + + switch( slot_number ) + { + case PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT: + /* This is the ECDSA slot. Verify the key's attributes before + * returning the private key. */ + if( psa_get_key_type( attributes ) != + PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_bits( attributes ) != 256 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_algorithm( attributes ) != + PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( ( psa_get_key_usage_flags( attributes ) & + PSA_KEY_USAGE_EXPORT ) == 0 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + + if( data_size < sizeof( test_driver_ecdsa_key ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + + memcpy( data, test_driver_ecdsa_key, + sizeof( test_driver_ecdsa_key ) ); + *data_length = sizeof( test_driver_ecdsa_key ); + return( PSA_SUCCESS ); + + case PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT: + /* This is the AES slot. Verify the key's attributes before + * returning the key. */ + if( psa_get_key_type( attributes ) != PSA_KEY_TYPE_AES ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_bits( attributes ) != 128 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_algorithm( attributes ) != PSA_ALG_CTR ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( ( psa_get_key_usage_flags( attributes ) & + PSA_KEY_USAGE_EXPORT ) == 0 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + + if( data_size < sizeof( test_driver_aes_key ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + + memcpy( data, test_driver_aes_key, + sizeof( test_driver_aes_key ) ); + *data_length = sizeof( test_driver_aes_key ); + return( PSA_SUCCESS ); + + default: + return( PSA_ERROR_DOES_NOT_EXIST ); + } } psa_status_t test_transparent_export_public_key( @@ -295,41 +293,39 @@ psa_status_t test_opaque_export_public_key( const uint8_t *key, size_t key_length, uint8_t *data, size_t data_size, size_t *data_length ) { - if( key_length == sizeof( psa_drv_slot_number_t ) ) - { - /* Assume this is a builtin key based on the key material length. */ - psa_drv_slot_number_t slot_number = *( ( psa_drv_slot_number_t* ) key ); - switch( slot_number ) - { - case PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT: - /* This is the ECDSA slot. Verify the key's attributes before - * returning the public key. */ - if( psa_get_key_type( attributes ) != - PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_bits( attributes ) != 256 ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - if( psa_get_key_algorithm( attributes ) != - PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) - return( PSA_ERROR_CORRUPTION_DETECTED ); - - if( data_size < sizeof( test_driver_ecdsa_pubkey ) ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); - - memcpy( data, test_driver_ecdsa_pubkey, - sizeof( test_driver_ecdsa_pubkey ) ); - *data_length = sizeof( test_driver_ecdsa_pubkey ); - return( PSA_SUCCESS ); - - default: - return( PSA_ERROR_DOES_NOT_EXIST ); - } - } - else + if( key_length != sizeof( psa_drv_slot_number_t ) ) { /* Test driver does not support generic opaque key handling yet. */ return( PSA_ERROR_NOT_SUPPORTED ); } + + /* Assume this is a builtin key based on the key material length. */ + psa_drv_slot_number_t slot_number = *( ( psa_drv_slot_number_t* ) key ); + switch( slot_number ) + { + case PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT: + /* This is the ECDSA slot. Verify the key's attributes before + * returning the public key. */ + if( psa_get_key_type( attributes ) != + PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_bits( attributes ) != 256 ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + if( psa_get_key_algorithm( attributes ) != + PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ) + return( PSA_ERROR_CORRUPTION_DETECTED ); + + if( data_size < sizeof( test_driver_ecdsa_pubkey ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + + memcpy( data, test_driver_ecdsa_pubkey, + sizeof( test_driver_ecdsa_pubkey ) ); + *data_length = sizeof( test_driver_ecdsa_pubkey ); + return( PSA_SUCCESS ); + + default: + return( PSA_ERROR_DOES_NOT_EXIST ); + } } /* The opaque test driver exposes two built-in keys when builtin key support is From 0bb653600f89eae09cbbe5c1925a1cb7da9bbf17 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Tue, 6 Apr 2021 15:09:57 +0200 Subject: [PATCH 19/26] If no storage backend is available, don't even attempt key loading Signed-off-by: Steven Cooreman --- library/psa_crypto_slot_management.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 336bc3716..de20fa137 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -351,6 +351,9 @@ psa_status_t psa_get_and_lock_key_slot( mbedtls_svc_key_id_t key, if( status != PSA_ERROR_DOES_NOT_EXIST ) return( status ); + /* Loading keys from storage requires support for such a mechanism */ +#if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) || \ + defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) psa_key_id_t volatile_key_id; status = psa_get_empty_key_slot( &volatile_key_id, p_slot ); @@ -378,6 +381,9 @@ psa_status_t psa_get_and_lock_key_slot( mbedtls_svc_key_id_t key, status = PSA_ERROR_INVALID_HANDLE; } return( status ); +#else /* MBEDTLS_PSA_CRYPTO_STORAGE_C || MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ + return( PSA_ERROR_INVALID_HANDLE ); +#endif /* MBEDTLS_PSA_CRYPTO_STORAGE_C || MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ } psa_status_t psa_unlock_key_slot( psa_key_slot_t *slot ) From 7609b1ff6cea2d25347a395f4579f4ab03f48a3b Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Tue, 6 Apr 2021 16:45:06 +0200 Subject: [PATCH 20/26] leverage psa_allocate_buffer_to_slot from slot management It makes the implementation of psa_load_builtin_key_into_slot a lot cleaner. Signed-off-by: Steven Cooreman --- library/psa_crypto.c | 16 ++-------------- library/psa_crypto_core.h | 15 +++++++++++++++ library/psa_crypto_slot_management.c | 26 ++++++++++++-------------- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 32568b322..068990a7a 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -604,20 +604,8 @@ MBEDTLS_STATIC_TESTABLE psa_status_t psa_mac_key_can_do( return( PSA_ERROR_INVALID_ARGUMENT ); } -/** Try to allocate a buffer to an empty key slot. - * - * \param[in,out] slot Key slot to attach buffer to. - * \param[in] buffer_length Requested size of the buffer. - * - * \retval #PSA_SUCCESS - * The buffer has been successfully allocated. - * \retval #PSA_ERROR_INSUFFICIENT_MEMORY - * Not enough memory was available for allocation. - * \retval #PSA_ERROR_ALREADY_EXISTS - * Trying to allocate a buffer to a non-empty key slot. - */ -static psa_status_t psa_allocate_buffer_to_slot( psa_key_slot_t *slot, - size_t buffer_length ) +psa_status_t psa_allocate_buffer_to_slot( psa_key_slot_t *slot, + size_t buffer_length ) { if( slot->key.data != NULL ) return( PSA_ERROR_ALREADY_EXISTS ); diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index f949c7188..eeb0105e3 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -180,6 +180,21 @@ static inline psa_key_slot_number_t psa_key_slot_get_slot_number( */ psa_status_t psa_wipe_key_slot( psa_key_slot_t *slot ); +/** Try to allocate a buffer to an empty key slot. + * + * \param[in,out] slot Key slot to attach buffer to. + * \param[in] buffer_length Requested size of the buffer. + * + * \retval #PSA_SUCCESS + * The buffer has been successfully allocated. + * \retval #PSA_ERROR_INSUFFICIENT_MEMORY + * Not enough memory was available for allocation. + * \retval #PSA_ERROR_ALREADY_EXISTS + * Trying to allocate a buffer to a non-empty key slot. + */ +psa_status_t psa_allocate_buffer_to_slot( psa_key_slot_t *slot, + size_t buffer_length ); + /** Copy key data (in export format) into an empty key slot. * * This function assumes that the slot does not contain diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index de20fa137..bdb45eed8 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -283,7 +283,6 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_key_lifetime_t lifetime = PSA_KEY_LIFETIME_VOLATILE; psa_drv_slot_number_t slot_number = 0; - uint8_t *key_buffer = NULL; size_t key_buffer_size = 0; size_t key_buffer_length = 0; @@ -303,33 +302,32 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) /* Set mapped lifetime on the attributes */ psa_set_key_lifetime( &attributes, lifetime ); - /* If the key should exist according to the platform, load it through the - * driver interface. */ + /* If the key should exist according to the platform, then ask the driver + * what its expected size is. */ status = psa_driver_wrapper_get_key_buffer_size( &attributes, &key_buffer_size ); if( status != PSA_SUCCESS ) return( status ); - key_buffer = mbedtls_calloc( 1, key_buffer_size ); - if( key_buffer == NULL ) - return( PSA_ERROR_INSUFFICIENT_MEMORY ); + /* Allocate a buffer of the required size and load the builtin key directly + * into the slot buffer. */ + status = psa_allocate_buffer_to_slot( slot, key_buffer_size ); + if( status != PSA_SUCCESS ) + return( status ); status = psa_driver_wrapper_get_builtin_key( slot_number, &attributes, - key_buffer, key_buffer_size, &key_buffer_length ); + slot->key.data, slot->key.bytes, &key_buffer_length ); if( status != PSA_SUCCESS ) goto exit; - status = psa_copy_key_material_into_slot( - slot, key_buffer, key_buffer_length ); - if( status != PSA_SUCCESS ) - goto exit; - - /* Copy core attributes into the slot on success */ + /* Copy actual key length and core attributes into the slot on success */ + slot->key.bytes = key_buffer_length; slot->attr = attributes.core; exit: - mbedtls_free( key_buffer ); + if( status != PSA_SUCCESS ) + psa_wipe_key_slot( slot ); return( status ); } #endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ From 7ddee7f7c58024ffbd3b129629985bd648a2169b Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Wed, 7 Apr 2021 18:08:30 +0200 Subject: [PATCH 21/26] Use remove_key_data_from_memory instead of wipe_key_slot Since the loading attempt of a builtin key might be followed by trying to load a persistent key, we can only wipe the allocated key data, not the associated metadata. Signed-off-by: Steven Cooreman --- library/psa_crypto.c | 3 +-- library/psa_crypto_core.h | 3 +++ library/psa_crypto_slot_management.c | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 068990a7a..f58df4aef 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1063,8 +1063,7 @@ static psa_status_t psa_get_and_lock_transparent_key_slot_with_policy( psa_get_and_lock_key_slot_with_policy( key, 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 ) +psa_status_t psa_remove_key_data_from_memory( psa_key_slot_t *slot ) { /* Data pointer will always be either a valid pointer or NULL in an * initialized slot, so we can just free it. */ diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index eeb0105e3..90f9d1863 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -195,6 +195,9 @@ psa_status_t psa_wipe_key_slot( psa_key_slot_t *slot ); psa_status_t psa_allocate_buffer_to_slot( psa_key_slot_t *slot, size_t buffer_length ); +/** Wipe key data from a slot. Preserves metadata such as the policy. */ +psa_status_t psa_remove_key_data_from_memory( psa_key_slot_t *slot ); + /** Copy key data (in export format) into an empty key slot. * * This function assumes that the slot does not contain diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index bdb45eed8..f9ea369e8 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -327,7 +327,7 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) exit: if( status != PSA_SUCCESS ) - psa_wipe_key_slot( slot ); + psa_remove_key_data_from_memory( slot ); return( status ); } #endif /* MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ From ce48702448e11bcd3ca702d7d784d84ecd0cdfd7 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Wed, 7 Apr 2021 18:09:53 +0200 Subject: [PATCH 22/26] Get a builtin key's attributes in order to correctly get its size Leverage the fact that the get_builtin_key entrypoint returns a key's attributes, such that a proper size for the builtin key's buffer can be calculated through the driver's get_key_buffer_size hook. Signed-off-by: Steven Cooreman --- library/psa_crypto_slot_management.c | 15 ++++++++++++++- tests/src/drivers/key_management.c | 9 ++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index f9ea369e8..7809c0cff 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -302,6 +302,19 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) /* Set mapped lifetime on the attributes */ psa_set_key_lifetime( &attributes, lifetime ); + /* Get the full key attributes from the driver in order to be able to + * calculate the required buffer size. */ + status = psa_driver_wrapper_get_builtin_key( + slot_number, &attributes, + NULL, 0, NULL ); + if( status != PSA_ERROR_BUFFER_TOO_SMALL ) + { + /* Builtin keys cannot be defined by the attributes alone */ + if( status == PSA_SUCCESS ) + status = PSA_ERROR_CORRUPTION_DETECTED; + goto exit; + } + /* If the key should exist according to the platform, then ask the driver * what its expected size is. */ status = psa_driver_wrapper_get_key_buffer_size( &attributes, @@ -310,7 +323,7 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) return( status ); /* Allocate a buffer of the required size and load the builtin key directly - * into the slot buffer. */ + * into the (now properly sized) slot buffer. */ status = psa_allocate_buffer_to_slot( slot, key_buffer_size ); if( status != PSA_SUCCESS ) return( status ); diff --git a/tests/src/drivers/key_management.c b/tests/src/drivers/key_management.c index 5daec6bd5..a0626fbf4 100644 --- a/tests/src/drivers/key_management.c +++ b/tests/src/drivers/key_management.c @@ -343,9 +343,6 @@ psa_status_t test_opaque_get_builtin_key( psa_key_attributes_t *attributes, uint8_t *key_buffer, size_t key_buffer_size, size_t *key_buffer_length ) { - if( key_buffer_size < sizeof( psa_drv_slot_number_t ) ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); - switch( slot_number ) { case PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT: @@ -358,6 +355,9 @@ psa_status_t test_opaque_get_builtin_key( PSA_KEY_USAGE_EXPORT ); psa_set_key_algorithm( attributes, PSA_ALG_CTR ); + if( key_buffer_size < sizeof( psa_drv_slot_number_t ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + *( (psa_drv_slot_number_t*) key_buffer ) = PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT; *key_buffer_length = sizeof( psa_drv_slot_number_t ); @@ -375,6 +375,9 @@ psa_status_t test_opaque_get_builtin_key( psa_set_key_algorithm( attributes, PSA_ALG_ECDSA( PSA_ALG_ANY_HASH ) ); + if( key_buffer_size < sizeof( psa_drv_slot_number_t ) ) + return( PSA_ERROR_BUFFER_TOO_SMALL ); + *( (psa_drv_slot_number_t*) key_buffer ) = PSA_CRYPTO_TEST_DRIVER_BUILTIN_ECDSA_KEY_SLOT; *key_buffer_length = sizeof( psa_drv_slot_number_t ); From 16141ed2fb38476fa2566b02834cf150131c3b05 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 8 Apr 2021 10:58:33 +0200 Subject: [PATCH 23/26] Add test driver sources to VC build Signed-off-by: Steven Cooreman --- scripts/generate_visualc_files.pl | 3 +++ visualc/VS2010/mbedTLS.vcxproj | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/scripts/generate_visualc_files.pl b/scripts/generate_visualc_files.pl index df5d66e81..d11041c31 100755 --- a/scripts/generate_visualc_files.pl +++ b/scripts/generate_visualc_files.pl @@ -40,6 +40,7 @@ my $source_dir = 'library'; my $test_source_dir = 'tests/src'; my $test_header_dir = 'tests/include/test'; my $test_drivers_header_dir = 'tests/include/test/drivers'; +my $test_drivers_source_dir = 'tests/src/drivers'; my @thirdparty_header_dirs = qw( 3rdparty/everest/include/everest @@ -116,6 +117,7 @@ sub check_dirs { && -d $psa_header_dir && -d $source_dir && -d $test_source_dir + && -d $test_drivers_source_dir && -d $test_header_dir && -d $test_drivers_header_dir && -d $programs_dir; @@ -275,6 +277,7 @@ sub main { my @source_dirs = ( $source_dir, $test_source_dir, + $test_drivers_source_dir, @thirdparty_source_dirs, ); my @sources = (map { <$_/*.c> } @source_dirs); diff --git a/visualc/VS2010/mbedTLS.vcxproj b/visualc/VS2010/mbedTLS.vcxproj index 832d42862..70ea6c5bc 100644 --- a/visualc/VS2010/mbedTLS.vcxproj +++ b/visualc/VS2010/mbedTLS.vcxproj @@ -379,6 +379,12 @@ + + + + + + From 2cca9b8f13c092013068001ce2dba2d88683fbb5 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Thu, 8 Apr 2021 12:34:02 +0200 Subject: [PATCH 24/26] Rename test driver source files to avoid file name conflicts MSVC doesn't like multiple compilation units with the same name. (conflict between cipher.c in the library and in the test driver folder) Signed-off-by: Steven Cooreman --- tests/src/drivers/{aead.c => test_driver_aead.c} | 0 tests/src/drivers/{cipher.c => test_driver_cipher.c} | 0 .../{key_management.c => test_driver_key_management.c} | 0 .../drivers/{signature.c => test_driver_signature.c} | 0 tests/src/drivers/{size.c => test_driver_size.c} | 0 visualc/VS2010/mbedTLS.vcxproj | 10 +++++----- 6 files changed, 5 insertions(+), 5 deletions(-) rename tests/src/drivers/{aead.c => test_driver_aead.c} (100%) rename tests/src/drivers/{cipher.c => test_driver_cipher.c} (100%) rename tests/src/drivers/{key_management.c => test_driver_key_management.c} (100%) rename tests/src/drivers/{signature.c => test_driver_signature.c} (100%) rename tests/src/drivers/{size.c => test_driver_size.c} (100%) diff --git a/tests/src/drivers/aead.c b/tests/src/drivers/test_driver_aead.c similarity index 100% rename from tests/src/drivers/aead.c rename to tests/src/drivers/test_driver_aead.c diff --git a/tests/src/drivers/cipher.c b/tests/src/drivers/test_driver_cipher.c similarity index 100% rename from tests/src/drivers/cipher.c rename to tests/src/drivers/test_driver_cipher.c diff --git a/tests/src/drivers/key_management.c b/tests/src/drivers/test_driver_key_management.c similarity index 100% rename from tests/src/drivers/key_management.c rename to tests/src/drivers/test_driver_key_management.c diff --git a/tests/src/drivers/signature.c b/tests/src/drivers/test_driver_signature.c similarity index 100% rename from tests/src/drivers/signature.c rename to tests/src/drivers/test_driver_signature.c diff --git a/tests/src/drivers/size.c b/tests/src/drivers/test_driver_size.c similarity index 100% rename from tests/src/drivers/size.c rename to tests/src/drivers/test_driver_size.c diff --git a/visualc/VS2010/mbedTLS.vcxproj b/visualc/VS2010/mbedTLS.vcxproj index 70ea6c5bc..5277193f9 100644 --- a/visualc/VS2010/mbedTLS.vcxproj +++ b/visualc/VS2010/mbedTLS.vcxproj @@ -379,12 +379,12 @@ - - - - - + + + + + From 966db2677993048095d108ed508b9d5d0164f5bc Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Tue, 13 Apr 2021 13:45:45 +0200 Subject: [PATCH 25/26] Minor code flow improvements * group setting of attributes before calling get_builtin_key * return early instead of going to exit when no resources are allocated yet Signed-off-by: Steven Cooreman --- library/psa_crypto_slot_management.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 7809c0cff..0b1a3c166 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -293,13 +293,14 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) } /* Check the platform function to see whether this key actually exists */ - psa_set_key_id( &attributes, slot->attr.id ); status = mbedtls_psa_platform_get_builtin_key( slot->attr.id, &lifetime, &slot_number ); if( status != PSA_SUCCESS ) return( status ); - /* Set mapped lifetime on the attributes */ + /* Set required key attributes to ensure get_builtin_key can retrieve the + * full attributes. */ + psa_set_key_id( &attributes, slot->attr.id ); psa_set_key_lifetime( &attributes, lifetime ); /* Get the full key attributes from the driver in order to be able to @@ -312,7 +313,7 @@ static psa_status_t psa_load_builtin_key_into_slot( psa_key_slot_t *slot ) /* Builtin keys cannot be defined by the attributes alone */ if( status == PSA_SUCCESS ) status = PSA_ERROR_CORRUPTION_DETECTED; - goto exit; + return( status ); } /* If the key should exist according to the platform, then ask the driver From 31e27af0cc7c4fdfa5d528baf71d535b1c6acb35 Mon Sep 17 00:00:00 2001 From: Steven Cooreman Date: Wed, 14 Apr 2021 10:32:05 +0200 Subject: [PATCH 26/26] Reword the builtin key language on persistency declaration Specifically allow the driver to override the persistency level of a builtin key in cases where the driver is persistency-aware. Signed-off-by: Steven Cooreman --- docs/proposed/psa-driver-interface.md | 2 +- include/psa/crypto_extra.h | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/proposed/psa-driver-interface.md b/docs/proposed/psa-driver-interface.md index 47d7271e6..2bdbff4e1 100644 --- a/docs/proposed/psa-driver-interface.md +++ b/docs/proposed/psa-driver-interface.md @@ -810,7 +810,7 @@ psa_status_t acme_get_builtin_key(psa_drv_slot_number_t slot_number, If this function returns `PSA_SUCCESS` or `PSA_ERROR_BUFFER_TOO_SMALL`, it must fill `attributes` with the attributes of the key (except for the key identifier). On success, this function must also fill `key_buffer` with the key context. -On entry, `psa_get_key_lifetime(attributes)` is the location at which the driver was declared and the persistence level `#PSA_KEY_LIFETIME_PERSISTENT`. The driver entry point may change the lifetime to one with the same location but a different persistence level. The standard attributes other than the key identifier and lifetime have the value conveyed by `PSA_KEY_ATTRIBUTES_INIT`. +On entry, `psa_get_key_lifetime(attributes)` is the location at which the driver was declared and a persistence level with which the platform is attempting to register the key. The driver entry point may choose to change the lifetime (`psa_set_key_lifetime(attributes, lifetime)`) of the reported key attributes to one with the same location but a different persistence level, in case the driver has more specific knowledge about the actual persistence level of the key which is being retrieved. For example, if a driver knows it cannot delete a key, it may override the persistence level in the lifetime to `PSA_KEY_PERSISTENCE_READ_ONLY`. The standard attributes other than the key identifier and lifetime have the value conveyed by `PSA_KEY_ATTRIBUTES_INIT`. The output parameter `key_buffer` points to a writable buffer of `key_buffer_size` bytes. If the driver has a [`"builtin_key_size"` property](#key-format-for-opaque-drivers) property, `key_buffer_size` has this value, otherwise `key_buffer_size` has the value determined from the key type and size. diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 2c0e33ba7..1310bb576 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -778,7 +778,12 @@ static inline int psa_key_id_is_builtin( psa_key_id_t key_id ) * \param[out] lifetime On success, the lifetime associated with the key * corresponding to \p key_id. Lifetime is a * combination of which driver contains the key, - * and with what lifecycle the key can be used. + * and with what persistence level the key is + * intended to be used. If the platform + * implementation does not contain specific + * information about the intended key persistence + * level, the persistence level may be reported as + * #PSA_KEY_PERSISTENCE_DEFAULT. * \param[out] slot_number On success, the slot number known to the driver * registered at the lifetime location reported * through \p lifetime which corresponds to the