From b84b6a68c73f1b5e67d7f2addb159f13674d8987 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 30 Jul 2019 11:38:12 +0200 Subject: [PATCH 01/34] Add some negative tests for policy checks Add a few test cases to ensure that alg=0 in policy does not allow using the key for an operation. Add a test case to ensure that ANY_HASH does not have a wildcard meaning for HMAC. --- tests/suites/test_suite_psa_crypto.data | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 53f842201..5c12caafd 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -336,6 +336,14 @@ PSA key policy: MAC, wrong algorithm depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C mac_key_policy:PSA_KEY_USAGE_SIGN | PSA_KEY_USAGE_VERIFY:PSA_ALG_HMAC(PSA_ALG_SHA_256):PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_224) +PSA key policy: MAC, alg=0 in policy +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_key_policy:PSA_KEY_USAGE_SIGN | PSA_KEY_USAGE_VERIFY:0:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_256) + +PSA key policy: MAC, ANY_HASH in policy is not meaningful +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_key_policy:PSA_KEY_USAGE_SIGN | PSA_KEY_USAGE_VERIFY:PSA_ALG_HMAC(PSA_ALG_ANY_HASH):PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_256) + PSA key policy: MAC, sign but not verify depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C mac_key_policy:PSA_KEY_USAGE_SIGN:PSA_ALG_HMAC(PSA_ALG_SHA_256):PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_256) @@ -368,6 +376,10 @@ PSA key policy: cipher, neither encrypt nor decrypt depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CTR cipher_key_policy:0:PSA_ALG_CTR:PSA_KEY_TYPE_AES:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_CTR +PSA key policy: cipher, alg=0 in policy +depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CTR +cipher_key_policy:PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:0:PSA_KEY_TYPE_AES:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_CTR + PSA key policy: AEAD, encrypt | decrypt depends_on:MBEDTLS_AES_C:MBEDTLS_CCM_C aead_key_policy:PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:PSA_ALG_CCM:PSA_KEY_TYPE_AES:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":13:16:PSA_ALG_CCM @@ -376,6 +388,10 @@ PSA key policy: AEAD, wrong algorithm depends_on:MBEDTLS_AES_C:MBEDTLS_CCM_C:MBEDTLS_GCM_C aead_key_policy:PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:PSA_ALG_CCM:PSA_KEY_TYPE_AES:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":16:16:PSA_ALG_GCM +PSA key policy: AEAD, alg=0 in policy +depends_on:MBEDTLS_AES_C:MBEDTLS_CCM_C +aead_key_policy:PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:0:PSA_KEY_TYPE_AES:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":16:16:PSA_ALG_CCM + PSA key policy: AEAD, encrypt but not decrypt depends_on:MBEDTLS_AES_C:MBEDTLS_CCM_C aead_key_policy:PSA_KEY_USAGE_ENCRYPT:PSA_ALG_CCM:PSA_KEY_TYPE_AES:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":13:16:PSA_ALG_CCM @@ -400,6 +416,10 @@ PSA key policy: asymmetric encryption, wrong algorithm (OAEP with different hash depends_on:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V21:MBEDTLS_SHA256_C asymmetric_encryption_key_policy:PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:PSA_ALG_RSA_OAEP(PSA_ALG_SHA_224):PSA_KEY_TYPE_RSA_KEY_PAIR:"3082013b020100024100ee2b131d6b1818a94ca8e91c42387eb15a7c271f57b89e7336b144d4535b16c83097ecdefbbb92d1b5313b5a37214d0e8f25922dca778b424b25295fc8a1a7070203010001024100978ac8eadb0dc6035347d6aba8671215ff21283385396f7897c04baf5e2a835f3b53ef80a82ed36ae687a925380b55a0c73eb85656e989dcf0ed7fb4887024e1022100fdad8e1c6853563f8b921d2d112462ae7d6b176082d2ba43e87e1a37fc1a8b33022100f0592cf4c55ba44307b18981bcdbda376c51e590ffa5345ba866f6962dca94dd02201995f1a967d44ff4a4cd1de837bc65bf97a2bf7eda730a9a62cea53254591105022027f96cf4b8ee68ff8d04062ec1ce7f18c0b74e4b3379b29f9bfea3fc8e592731022100cefa6d220496b43feb83194255d8fb930afcf46f36606e3aa0eb7a93ad88c10c":PSA_ALG_RSA_OAEP(PSA_ALG_SHA_256) +PSA key policy: asymmetric encryption, alg=0 in policy +depends_on:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15 +asymmetric_encryption_key_policy:PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:0:PSA_KEY_TYPE_RSA_KEY_PAIR:"3082013b020100024100ee2b131d6b1818a94ca8e91c42387eb15a7c271f57b89e7336b144d4535b16c83097ecdefbbb92d1b5313b5a37214d0e8f25922dca778b424b25295fc8a1a7070203010001024100978ac8eadb0dc6035347d6aba8671215ff21283385396f7897c04baf5e2a835f3b53ef80a82ed36ae687a925380b55a0c73eb85656e989dcf0ed7fb4887024e1022100fdad8e1c6853563f8b921d2d112462ae7d6b176082d2ba43e87e1a37fc1a8b33022100f0592cf4c55ba44307b18981bcdbda376c51e590ffa5345ba866f6962dca94dd02201995f1a967d44ff4a4cd1de837bc65bf97a2bf7eda730a9a62cea53254591105022027f96cf4b8ee68ff8d04062ec1ce7f18c0b74e4b3379b29f9bfea3fc8e592731022100cefa6d220496b43feb83194255d8fb930afcf46f36606e3aa0eb7a93ad88c10c":PSA_ALG_RSA_PKCS1V15_CRYPT + PSA key policy: asymmetric encryption, ANY_HASH in policy is not meaningful depends_on:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V21:MBEDTLS_SHA256_C asymmetric_encryption_key_policy:PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:PSA_ALG_RSA_OAEP(PSA_ALG_ANY_HASH):PSA_KEY_TYPE_RSA_KEY_PAIR:"3082013b020100024100ee2b131d6b1818a94ca8e91c42387eb15a7c271f57b89e7336b144d4535b16c83097ecdefbbb92d1b5313b5a37214d0e8f25922dca778b424b25295fc8a1a7070203010001024100978ac8eadb0dc6035347d6aba8671215ff21283385396f7897c04baf5e2a835f3b53ef80a82ed36ae687a925380b55a0c73eb85656e989dcf0ed7fb4887024e1022100fdad8e1c6853563f8b921d2d112462ae7d6b176082d2ba43e87e1a37fc1a8b33022100f0592cf4c55ba44307b18981bcdbda376c51e590ffa5345ba866f6962dca94dd02201995f1a967d44ff4a4cd1de837bc65bf97a2bf7eda730a9a62cea53254591105022027f96cf4b8ee68ff8d04062ec1ce7f18c0b74e4b3379b29f9bfea3fc8e592731022100cefa6d220496b43feb83194255d8fb930afcf46f36606e3aa0eb7a93ad88c10c":PSA_ALG_RSA_OAEP(PSA_ALG_SHA_256) @@ -444,6 +464,10 @@ PSA key policy: asymmetric signature, wrong hash algorithm depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C asymmetric_signature_key_policy:PSA_KEY_USAGE_SIGN | PSA_KEY_USAGE_VERIFY:PSA_ALG_RSA_PKCS1V15_SIGN(PSA_ALG_SHA_256):PSA_KEY_TYPE_RSA_KEY_PAIR:"3082013b020100024100ee2b131d6b1818a94ca8e91c42387eb15a7c271f57b89e7336b144d4535b16c83097ecdefbbb92d1b5313b5a37214d0e8f25922dca778b424b25295fc8a1a7070203010001024100978ac8eadb0dc6035347d6aba8671215ff21283385396f7897c04baf5e2a835f3b53ef80a82ed36ae687a925380b55a0c73eb85656e989dcf0ed7fb4887024e1022100fdad8e1c6853563f8b921d2d112462ae7d6b176082d2ba43e87e1a37fc1a8b33022100f0592cf4c55ba44307b18981bcdbda376c51e590ffa5345ba866f6962dca94dd02201995f1a967d44ff4a4cd1de837bc65bf97a2bf7eda730a9a62cea53254591105022027f96cf4b8ee68ff8d04062ec1ce7f18c0b74e4b3379b29f9bfea3fc8e592731022100cefa6d220496b43feb83194255d8fb930afcf46f36606e3aa0eb7a93ad88c10c":PSA_ALG_RSA_PKCS1V15_SIGN_RAW:0 +PSA key policy: asymmetric signature, alg=0 in policy +depends_on:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15 +asymmetric_signature_key_policy:PSA_KEY_USAGE_SIGN | PSA_KEY_USAGE_VERIFY:0:PSA_KEY_TYPE_RSA_KEY_PAIR:"3082013b020100024100ee2b131d6b1818a94ca8e91c42387eb15a7c271f57b89e7336b144d4535b16c83097ecdefbbb92d1b5313b5a37214d0e8f25922dca778b424b25295fc8a1a7070203010001024100978ac8eadb0dc6035347d6aba8671215ff21283385396f7897c04baf5e2a835f3b53ef80a82ed36ae687a925380b55a0c73eb85656e989dcf0ed7fb4887024e1022100fdad8e1c6853563f8b921d2d112462ae7d6b176082d2ba43e87e1a37fc1a8b33022100f0592cf4c55ba44307b18981bcdbda376c51e590ffa5345ba866f6962dca94dd02201995f1a967d44ff4a4cd1de837bc65bf97a2bf7eda730a9a62cea53254591105022027f96cf4b8ee68ff8d04062ec1ce7f18c0b74e4b3379b29f9bfea3fc8e592731022100cefa6d220496b43feb83194255d8fb930afcf46f36606e3aa0eb7a93ad88c10c":PSA_ALG_RSA_PKCS1V15_SIGN_RAW:0 + PSA key policy: asymmetric signature, sign but not verify depends_on:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15 asymmetric_signature_key_policy:PSA_KEY_USAGE_SIGN:PSA_ALG_RSA_PKCS1V15_SIGN_RAW:PSA_KEY_TYPE_RSA_KEY_PAIR:"3082013b020100024100ee2b131d6b1818a94ca8e91c42387eb15a7c271f57b89e7336b144d4535b16c83097ecdefbbb92d1b5313b5a37214d0e8f25922dca778b424b25295fc8a1a7070203010001024100978ac8eadb0dc6035347d6aba8671215ff21283385396f7897c04baf5e2a835f3b53ef80a82ed36ae687a925380b55a0c73eb85656e989dcf0ed7fb4887024e1022100fdad8e1c6853563f8b921d2d112462ae7d6b176082d2ba43e87e1a37fc1a8b33022100f0592cf4c55ba44307b18981bcdbda376c51e590ffa5345ba866f6962dca94dd02201995f1a967d44ff4a4cd1de837bc65bf97a2bf7eda730a9a62cea53254591105022027f96cf4b8ee68ff8d04062ec1ce7f18c0b74e4b3379b29f9bfea3fc8e592731022100cefa6d220496b43feb83194255d8fb930afcf46f36606e3aa0eb7a93ad88c10c":PSA_ALG_RSA_PKCS1V15_SIGN_RAW:1 From 91e8c33f48a6e36a97e28513f3bdb8007ac7ad5d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 2 Aug 2019 19:19:39 +0200 Subject: [PATCH 02/34] Add infrastructure for key attribute flags Add infrastructure for internal, external and dual-use flags, with a compile-time check (if static_assert is available) to ensure that the same numerical value doesn't get declared for two different purposes in crypto_struct.h (external or dual-use) and psa_crypto_core.h (internal). --- include/psa/crypto_struct.h | 23 ++++++++++++++++++++++- library/psa_crypto.c | 18 ++++++++++++++++++ library/psa_crypto_core.h | 5 +++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index 9e38e53ce..3bace6088 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -322,6 +322,27 @@ typedef uint16_t psa_key_bits_t; * conditionals. */ #define PSA_MAX_KEY_BITS 0xfff8 +/** A mask of flags that can be stored in key attributes. + * + * This type is also used internally to store flags in slots. Internal + * flags are defined in library/psa_crypto_core.h. Internal flags may have + * the same value as external flags if they are properly handled during + * key creation and in psa_get_key_attributes. + */ +typedef uint16_t psa_key_attributes_flag_t; + +#define MBEDLTS_PSA_KA_FLAG_SLOT_NUMBER ( (psa_key_attributes_flag_t) 0x0001 ) + +/* A mask of key attribute flags used externally only. + * Only meant for internal checks inside the library. */ +#define MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ( \ + 0 ) + +/* A mask of key attribute flags used both internally and externally. + * Currently there aren't any. */ +#define MBEDTLS_PSA_KA_MASK_DUAL_USE ( \ + 0 ) + typedef struct { psa_key_type_t type; @@ -329,7 +350,7 @@ typedef struct psa_key_id_t id; psa_key_policy_t policy; psa_key_bits_t bits; - uint16_t flags; + psa_key_attributes_flag_t flags; } psa_core_key_attributes_t; #define PSA_CORE_KEY_ATTRIBUTES_INIT {0, 0, 0, {0, 0, 0}, 0, 0} diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 41289c607..e043d7000 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1408,6 +1408,15 @@ psa_status_t psa_export_public_key( psa_key_handle_t handle, data_length, 1 ) ); } +#if defined(static_assert) +static_assert( ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, + "One or more key attribute flag is listed as both external-only and dual-use" ); +static_assert( ( MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, + "One or more key attribute flag is listed as both external-only and dual-use" ); +static_assert( ( MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ) == 0, + "One or more key attribute flag is listed as both internal-only and external-only" ); +#endif + /** Validate that a key policy is internally well-formed. * * This function only rejects invalid policies. It does not validate the @@ -1467,6 +1476,11 @@ static psa_status_t psa_validate_key_attributes( if( psa_get_key_bits( attributes ) > PSA_MAX_KEY_BITS ) return( PSA_ERROR_NOT_SUPPORTED ); + /* Reject invalid flags. These should not be reachable through the API. */ + if( attributes->core.flags & ~ ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY | + MBEDTLS_PSA_KA_MASK_DUAL_USE ) ) + return( PSA_ERROR_INVALID_ARGUMENT ); + return( PSA_SUCCESS ); } @@ -1523,6 +1537,10 @@ static psa_status_t psa_start_key_creation( slot->attr = attributes->core; + /* Erase external-only flags from the internal copy. To access + * external-only flags, query `attributes`. */ + slot->attr.flags |= ~MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY; + #if defined(MBEDTLS_PSA_CRYPTO_SE_C) /* For a key in a secure element, we need to do three things: * create the key file in internal storage, create the diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index fbfb6daef..e289dbeef 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -64,6 +64,11 @@ typedef struct } data; } psa_key_slot_t; +/* A mask of key attribute flags used only internally. + * Currently there aren't any. */ +#define MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY ( \ + 0 ) + /** Test whether a key slot is occupied. * * A key slot is occupied iff the key type is nonzero. This works because From 74f3352b05a6d1b119fa6325ac1882eedf3860f9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 2 Aug 2019 19:21:49 +0200 Subject: [PATCH 03/34] Add missing guard around a union field --- library/psa_crypto_core.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index e289dbeef..b67c0c576 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -56,11 +56,13 @@ typedef struct /* EC public key or key pair */ mbedtls_ecp_keypair *ecp; #endif /* MBEDTLS_ECP_C */ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) /* Any key type in a secure element */ struct se { psa_key_slot_number_t slot_number; } se; +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ } data; } psa_key_slot_t; From c8000c005aa16da442e716ed89009631e3770c8a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 2 Aug 2019 20:15:51 +0200 Subject: [PATCH 04/34] Add slot_number attribute Add a slot_number field to psa_key_attributes_t and getter/setter functions. Since slot numbers can have the value 0, indicate the presence of the field via a separate flag. In psa_get_key_attributes(), report the slot number if the key is in a secure element. When creating a key, for now, applications cannot choose a slot number. A subsequent commit will add this capability in the secure element HAL. --- include/psa/crypto_extra.h | 61 ++++++++++++++++++++++++++++++++++ include/psa/crypto_se_driver.h | 7 ++++ include/psa/crypto_struct.h | 12 ++++++- include/psa/crypto_types.h | 11 ++++++ library/psa_crypto.c | 28 +++++++++++++++- 5 files changed, 117 insertions(+), 2 deletions(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 6dfaa1300..5359b580a 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -104,6 +104,67 @@ static inline psa_algorithm_t psa_get_key_enrollment_algorithm( return( attributes->core.policy.alg2 ); } +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + +/** Retrieve the slot number where a key is stored. + * + * A slot number is only defined for keys that are stored in a secure + * element. + * + * This information is only useful if the secure element is not entirely + * managed through the PSA Cryptography API. It is up to the secure + * element driver to decide how PSA slot numbers map to any other interface + * that the secure element may have. + * + * \param[in] attributes The key attribute structure to query. + * \param[out] slot_number On success, the slot number containing the key. + * + * \retval #PSA_SUCCESS + * The key is located in a secure element, and \p *slot_number + * indicates the slot number that contains it. + * \retval #PSA_ERROR_NOT_PERMITTED + * The caller is not permitted to query the slot number. + * Mbed Crypto currently does not return this error. + * \retval #PSA_ERROR_INVALID_ARGUMENT + * The key is not located in a secure element. + */ +psa_status_t psa_get_key_slot_number( + const psa_key_attributes_t *attributes, + psa_key_slot_number_t *slot_number ); + +/** Choose the slot number where a key is stored. + * + * This function declares a slot number in the specified attribute + * structure. + * + * A slot number is only meaningful for keys that are stored in a secure + * element. It is up to the secure element driver to decide how PSA slot + * numbers map to any other interface that the secure element may have. + * + * \note Setting a slot number in key attributes for a key creation can + * cause the following errors when creating the key: + * - #PSA_ERROR_NOT_SUPPORTED if the selected secure element does + * not support choosing a specific slot number. + * - #PSA_ERROR_NOT_PERMITTED if the caller is not permitted to + * choose slot numbers in general or to choose this specific slot. + * - #PSA_ERROR_INVALID_ARGUMENT if the chosen slot number is not + * valid in general or not valid for this specific key. + * - #PSA_ERROR_ALREADY_EXISTS if there is already a key in the + * selected slot. + * + * \param[out] attributes The attribute structure to write to. + * \param slot_number The slot number to set. + */ +static inline void psa_set_key_slot_number( + psa_key_attributes_t *attributes, + psa_key_slot_number_t slot_number ) +{ + attributes->core.flags |= MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER; + attributes->slot_number = slot_number; +} + +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + /**@}*/ /** diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index f95eaeb33..69cdababa 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -134,10 +134,17 @@ typedef psa_status_t (*psa_drv_se_init_t)(psa_drv_se_context_t *drv_context, void *persistent_data, psa_key_lifetime_t lifetime); +#if defined(__DOXYGEN_ONLY__) || !defined(MBEDTLS_PSA_CRYPTO_SE_C) +/* Mbed Crypto with secure element support enabled defines this type in + * crypto_types.h because it is also visible to applications through an + * implementation-specific extension. + * For the PSA Cryptography specification, this type is only visible + * via crypto_se_driver.h. */ /** An internal designation of a key slot between the core part of the * PSA Crypto implementation and the driver. The meaning of this value * is driver-dependent. */ typedef uint64_t psa_key_slot_number_t; +#endif /* __DOXYGEN_ONLY__ || !MBEDTLS_PSA_CRYPTO_SE_C */ /**@}*/ diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index 3bace6088..fbfe77e62 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -331,11 +331,13 @@ typedef uint16_t psa_key_bits_t; */ typedef uint16_t psa_key_attributes_flag_t; -#define MBEDLTS_PSA_KA_FLAG_SLOT_NUMBER ( (psa_key_attributes_flag_t) 0x0001 ) +#define MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER \ + ( (psa_key_attributes_flag_t) 0x0001 ) /* A mask of key attribute flags used externally only. * Only meant for internal checks inside the library. */ #define MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ( \ + MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER | \ 0 ) /* A mask of key attribute flags used both internally and externally. @@ -358,11 +360,19 @@ typedef struct struct psa_key_attributes_s { psa_core_key_attributes_t core; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + psa_key_slot_number_t slot_number; +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ void *domain_parameters; size_t domain_parameters_size; }; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +#define PSA_KEY_ATTRIBUTES_INIT {PSA_CORE_KEY_ATTRIBUTES_INIT, 0, NULL, 0} +#else #define PSA_KEY_ATTRIBUTES_INIT {PSA_CORE_KEY_ATTRIBUTES_INIT, NULL, 0} +#endif + static inline struct psa_key_attributes_s psa_key_attributes_init( void ) { const struct psa_key_attributes_s v = PSA_KEY_ATTRIBUTES_INIT; diff --git a/include/psa/crypto_types.h b/include/psa/crypto_types.h index 1944be4b2..9af4957df 100644 --- a/include/psa/crypto_types.h +++ b/include/psa/crypto_types.h @@ -244,6 +244,17 @@ typedef uint32_t psa_key_usage_t; */ typedef struct psa_key_attributes_s psa_key_attributes_t; + +#ifndef __DOXYGEN_ONLY__ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +/* Mbed Crypto defines this type in crypto_types.h because it is also + * visible to applications through an implementation-specific extension. + * For the PSA Cryptography specification, this type is only visible + * via crypto_se_driver.h. */ +typedef uint64_t psa_key_slot_number_t; +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ +#endif /* !__DOXYGEN_ONLY__ */ + /**@}*/ /** \defgroup derivation Key derivation diff --git a/library/psa_crypto.c b/library/psa_crypto.c index e043d7000..a54cd73bb 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1187,6 +1187,13 @@ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, return( status ); attributes->core = slot->attr; + attributes->core.flags &= ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY | + MBEDTLS_PSA_KA_MASK_DUAL_USE ); + +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( psa_key_slot_is_external( slot ) ) + psa_set_key_slot_number( attributes, slot->data.se.slot_number ); +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ switch( slot->attr.type ) { @@ -1196,7 +1203,7 @@ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, #if defined(MBEDTLS_PSA_CRYPTO_SE_C) /* TOnogrepDO: reporting the public exponent for opaque keys * is not yet implemented. */ - if( psa_get_se_driver( slot->attr.lifetime, NULL, NULL ) ) + if( psa_key_slot_is_external( slot ) ) break; #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ status = psa_get_rsa_public_exponent( slot->data.rsa, attributes ); @@ -1212,6 +1219,21 @@ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, return( status ); } +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +psa_status_t psa_get_key_slot_number( + const psa_key_attributes_t *attributes, + psa_key_slot_number_t *slot_number ) +{ + if( attributes->core.flags & MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER ) + { + *slot_number = attributes->slot_number; + return( PSA_SUCCESS ); + } + else + return( PSA_ERROR_INVALID_ARGUMENT ); +} +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + #if defined(MBEDTLS_RSA_C) || defined(MBEDTLS_ECP_C) static int pk_write_pubkey_simple( mbedtls_pk_context *key, unsigned char *buf, size_t size ) @@ -1557,6 +1579,10 @@ static psa_status_t psa_start_key_creation( * we can roll back to a state where the key doesn't exist. */ if( *p_drv != NULL ) { + /* Choosing a slot number is not supported yet. */ + if( attributes->core.flags & MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER ) + return( PSA_ERROR_NOT_SUPPORTED ); + status = psa_find_se_slot_for_key( attributes, *p_drv, &slot->data.se.slot_number ); if( status != PSA_SUCCESS ) From 5fe5e2759160a040ae47561d7d26db12e392f07b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 2 Aug 2019 20:30:01 +0200 Subject: [PATCH 05/34] Test slot_number attribute Test the behavior of the getter/setter functions. Test that psa_get_key_slot_number() reports a slot number for a key in a secure element, and doesn't report a slot number for a key that is not in a secure element. Test that psa_get_key_slot_number() reports the correct slot number for a key in a secure element. --- include/psa/crypto_extra.h | 12 ++++ tests/suites/test_suite_psa_crypto.data | 3 + tests/suites/test_suite_psa_crypto.function | 61 +++++++++++++++++++ ...st_suite_psa_crypto_se_driver_hal.function | 30 ++++++++- 4 files changed, 105 insertions(+), 1 deletion(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 5359b580a..130ce7544 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -163,6 +163,18 @@ static inline void psa_set_key_slot_number( attributes->slot_number = slot_number; } +/** Remove the slot number attribute from a key attribute structure. + * + * This function undoes the action of psa_set_key_slot_number(). + * + * \param[out] attributes The attribute structure to write to. + */ +static inline void psa_clear_key_slot_number( + psa_key_attributes_t *attributes ) +{ + attributes->core.flags &= ~MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER; +} + #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ /**@}*/ diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index b04984024..4118d2f3e 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -19,6 +19,9 @@ persistence_attributes:0x1234:3:-1:0x1234:3 PSA key attributes: lifetime then id persistence_attributes:0x1234:3:0x1235:0x1235:3 +PSA key attributes: slot number +slot_number_attribute: + PSA import/export raw: 0 bytes import_export:"":PSA_KEY_TYPE_RAW_DATA:PSA_KEY_USAGE_EXPORT:0:0:0:PSA_SUCCESS:1 diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 0eb6172a4..3225bef34 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -1113,6 +1113,23 @@ exit: return( ok ); } +/* Assert that a key isn't reported as having a slot number. */ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +#define ASSERT_NO_SLOT_NUMBER( attributes ) \ + do \ + { \ + psa_key_slot_number_t ASSERT_NO_SLOT_NUMBER_slot_number; \ + TEST_EQUAL( psa_get_key_slot_number( \ + attributes, \ + &ASSERT_NO_SLOT_NUMBER_slot_number ), \ + PSA_ERROR_INVALID_ARGUMENT ); \ + } \ + while( 0 ) +#else /* MBEDTLS_PSA_CRYPTO_SE_C */ +#define ASSERT_NO_SLOT_NUMBER( attributes ) \ + ( (void) 0 ) +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + /* An overapproximation of the amount of storage needed for a key of the * given type and with the given content. The API doesn't make it easy * to find a good value for the size. The current implementation doesn't @@ -1214,6 +1231,46 @@ void persistence_attributes( int id1_arg, int lifetime_arg, int id2_arg, } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_PSA_CRYPTO_SE_C */ +void slot_number_attribute( ) +{ + psa_key_slot_number_t slot_number = 0xdeadbeef; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + + /* Initially, there is no slot number. */ + TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ), + PSA_ERROR_INVALID_ARGUMENT ); + + /* Test setting a slot number. */ + psa_set_key_slot_number( &attributes, 0 ); + PSA_ASSERT( psa_get_key_slot_number( &attributes, &slot_number ) ); + TEST_EQUAL( slot_number, 0 ); + + /* Test changing the slot number. */ + psa_set_key_slot_number( &attributes, 42 ); + PSA_ASSERT( psa_get_key_slot_number( &attributes, &slot_number ) ); + TEST_EQUAL( slot_number, 42 ); + + /* Test clearing the slot number. */ + psa_clear_key_slot_number( &attributes ); + TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ), + PSA_ERROR_INVALID_ARGUMENT ); + + /* Clearing again should have no effect. */ + psa_clear_key_slot_number( &attributes ); + TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ), + PSA_ERROR_INVALID_ARGUMENT ); + + /* Test that reset clears the slot number. */ + psa_set_key_slot_number( &attributes, 42 ); + PSA_ASSERT( psa_get_key_slot_number( &attributes, &slot_number ) ); + TEST_EQUAL( slot_number, 42 ); + psa_reset_key_attributes( &attributes ); + TEST_EQUAL( psa_get_key_slot_number( &attributes, &slot_number ), + PSA_ERROR_INVALID_ARGUMENT ); +} +/* END_CASE */ + /* BEGIN_CASE */ void import_with_policy( int type_arg, int usage_arg, int alg_arg, @@ -1246,6 +1303,7 @@ void import_with_policy( int type_arg, TEST_EQUAL( psa_get_key_type( &got_attributes ), type ); TEST_EQUAL( psa_get_key_usage_flags( &got_attributes ), usage ); TEST_EQUAL( psa_get_key_algorithm( &got_attributes ), alg ); + ASSERT_NO_SLOT_NUMBER( &got_attributes ); PSA_ASSERT( psa_destroy_key( handle ) ); test_operations_on_invalid_handle( handle ); @@ -1284,6 +1342,7 @@ void import_with_data( data_t *data, int type_arg, TEST_EQUAL( psa_get_key_type( &got_attributes ), type ); if( attr_bits != 0 ) TEST_EQUAL( attr_bits, psa_get_key_bits( &got_attributes ) ); + ASSERT_NO_SLOT_NUMBER( &got_attributes ); PSA_ASSERT( psa_destroy_key( handle ) ); test_operations_on_invalid_handle( handle ); @@ -1328,6 +1387,7 @@ void import_large_key( int type_arg, int byte_size_arg, TEST_EQUAL( psa_get_key_type( &attributes ), type ); TEST_EQUAL( psa_get_key_bits( &attributes ), PSA_BYTES_TO_BITS( byte_size ) ); + ASSERT_NO_SLOT_NUMBER( &attributes ); memset( buffer, 0, byte_size + 1 ); PSA_ASSERT( psa_export_key( handle, buffer, byte_size, &n ) ); for( n = 0; n < byte_size; n++ ) @@ -1420,6 +1480,7 @@ void import_export( data_t *data, PSA_ASSERT( psa_get_key_attributes( handle, &got_attributes ) ); TEST_EQUAL( psa_get_key_type( &got_attributes ), type ); TEST_EQUAL( psa_get_key_bits( &got_attributes ), (size_t) expected_bits ); + ASSERT_NO_SLOT_NUMBER( &got_attributes ); /* Export the key */ status = psa_export_key( handle, diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 6ac19a60e..9a5746476 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -212,6 +212,31 @@ static int check_key_attributes( psa_get_key_bits( reference_attributes ) ); } + { + psa_key_slot_number_t actual_slot_number = 0xdeadbeef; + psa_key_slot_number_t desired_slot_number = 0xb90cc011; + psa_key_lifetime_t lifetime = + psa_get_key_lifetime( &actual_attributes ); + psa_status_t status = psa_get_key_slot_number( &actual_attributes, + &actual_slot_number ); + if( lifetime < MIN_DRIVER_LIFETIME ) + { + /* The key is not in a secure element. */ + TEST_EQUAL( status, PSA_ERROR_INVALID_ARGUMENT ); + } + else + { + /* The key is in a secure element. If it had been created + * in a specific slot, check that it is reported there. */ + PSA_ASSERT( status ); + status = psa_get_key_slot_number( reference_attributes, + &desired_slot_number ); + if( status == PSA_SUCCESS ) + { + TEST_EQUAL( desired_slot_number, actual_slot_number ); + } + } + } ok = 1; exit: @@ -485,11 +510,14 @@ void key_creation_import_export( int min_slot, int restart ) /* Test that the key was created in the expected slot. */ TEST_ASSERT( ram_slots[min_slot].type == PSA_KEY_TYPE_RAW_DATA ); - /* Test the key attributes and the key data. */ + /* Test the key attributes, including the reported slot number. */ psa_set_key_bits( &attributes, PSA_BYTES_TO_BITS( sizeof( key_material ) ) ); + psa_set_key_slot_number( &attributes, min_slot ); if( ! check_key_attributes( handle, &attributes ) ) goto exit; + + /* Test the key data. */ PSA_ASSERT( psa_export_key( handle, exported, sizeof( exported ), &exported_length ) ); From 5a68056755f6e75f281b4d2884cff72ca85ecd78 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 17:32:13 +0200 Subject: [PATCH 06/34] Rename internal macro to pass check-names.sh check-names.sh rejects MBEDTLS_XXX identifiers that are not defined in a public header. --- library/psa_crypto.c | 4 ++-- library/psa_crypto_core.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index a54cd73bb..9f7b5cbc0 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1433,9 +1433,9 @@ psa_status_t psa_export_public_key( psa_key_handle_t handle, #if defined(static_assert) static_assert( ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, "One or more key attribute flag is listed as both external-only and dual-use" ); -static_assert( ( MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, +static_assert( ( PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, "One or more key attribute flag is listed as both external-only and dual-use" ); -static_assert( ( MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ) == 0, +static_assert( ( PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ) == 0, "One or more key attribute flag is listed as both internal-only and external-only" ); #endif diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index b67c0c576..edf3ab603 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -68,7 +68,7 @@ typedef struct /* A mask of key attribute flags used only internally. * Currently there aren't any. */ -#define MBEDTLS_PSA_KA_MASK_INTERNAL_ONLY ( \ +#define PSA_KA_MASK_INTERNAL_ONLY ( \ 0 ) /** Test whether a key slot is occupied. From 013f5474cfc639412d787246b5af45a971e0493e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 7 Aug 2019 15:42:14 +0200 Subject: [PATCH 07/34] Fix erasure of external flags This didn't break anything now, but would have broken things once we start to add internal flags. --- library/psa_crypto.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 9f7b5cbc0..5742f627d 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1560,8 +1560,11 @@ static psa_status_t psa_start_key_creation( slot->attr = attributes->core; /* Erase external-only flags from the internal copy. To access - * external-only flags, query `attributes`. */ - slot->attr.flags |= ~MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY; + * external-only flags, query `attributes`. Thanks to the check + * in psa_validate_key_attributes(), this leaves the dual-use + * flags and any internal flag that psa_internal_allocate_key_slot() + * may have set. */ + slot->attr.flags &= ~MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY; #if defined(MBEDTLS_PSA_CRYPTO_SE_C) /* For a key in a secure element, we need to do three things: From 094dac1d12e16c3c0a933cf9c548b2a73cee478a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 7 Aug 2019 18:19:46 +0200 Subject: [PATCH 08/34] Fix copypasta --- library/psa_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 5742f627d..254ab2a71 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1434,7 +1434,7 @@ psa_status_t psa_export_public_key( psa_key_handle_t handle, static_assert( ( MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, "One or more key attribute flag is listed as both external-only and dual-use" ); static_assert( ( PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE ) == 0, - "One or more key attribute flag is listed as both external-only and dual-use" ); + "One or more key attribute flag is listed as both internal-only and dual-use" ); static_assert( ( PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY ) == 0, "One or more key attribute flag is listed as both internal-only and external-only" ); #endif From edbed5670afe1caa6be6cb9ec8dbca4fab3b82c8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 7 Aug 2019 18:19:59 +0200 Subject: [PATCH 09/34] Rename psa_internal_allocate_key_slot to psa_get_empty_key_slot This function no longer modifies anything, so it doesn't actually allocate the slot. Now, it just returns the empty key slot, and it's up to the caller to cause the slot to be in use (or not). --- library/psa_crypto.c | 4 ++-- library/psa_crypto_slot_management.c | 4 ++-- library/psa_crypto_slot_management.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 254ab2a71..5cb88de7e 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1546,7 +1546,7 @@ static psa_status_t psa_start_key_creation( if( status != PSA_SUCCESS ) return( status ); - status = psa_internal_allocate_key_slot( handle, p_slot ); + status = psa_get_empty_key_slot( handle, p_slot ); if( status != PSA_SUCCESS ) return( status ); slot = *p_slot; @@ -1562,7 +1562,7 @@ static psa_status_t psa_start_key_creation( /* Erase external-only flags from the internal copy. To access * external-only flags, query `attributes`. Thanks to the check * in psa_validate_key_attributes(), this leaves the dual-use - * flags and any internal flag that psa_internal_allocate_key_slot() + * flags and any internal flag that psa_get_empty_key_slot() * may have set. */ slot->attr.flags &= ~MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY; diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 073400988..fe9214831 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -102,7 +102,7 @@ void psa_wipe_all_key_slots( void ) global_data.key_slots_initialized = 0; } -psa_status_t psa_internal_allocate_key_slot( psa_key_handle_t *handle, +psa_status_t psa_get_empty_key_slot( psa_key_handle_t *handle, psa_key_slot_t **p_slot ) { if( ! global_data.key_slots_initialized ) @@ -228,7 +228,7 @@ psa_status_t psa_open_key( psa_key_file_id_t id, psa_key_handle_t *handle ) if( status != PSA_SUCCESS ) return( status ); - status = psa_internal_allocate_key_slot( handle, &slot ); + status = psa_get_empty_key_slot( handle, &slot ); if( status != PSA_SUCCESS ) return( status ); diff --git a/library/psa_crypto_slot_management.h b/library/psa_crypto_slot_management.h index cde590fc5..472253dd9 100644 --- a/library/psa_crypto_slot_management.h +++ b/library/psa_crypto_slot_management.h @@ -71,8 +71,8 @@ void psa_wipe_all_key_slots( void ); * \retval #PSA_ERROR_INSUFFICIENT_MEMORY * \retval #PSA_ERROR_BAD_STATE */ -psa_status_t psa_internal_allocate_key_slot( psa_key_handle_t *handle, - psa_key_slot_t **p_slot ); +psa_status_t psa_get_empty_key_slot( psa_key_handle_t *handle, + psa_key_slot_t **p_slot ); /** Test whether a lifetime designates a key in an external cryptoprocessor. * From 0a233224316f85bcd30c40106579c3d35addf4c4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 14:50:28 +0200 Subject: [PATCH 10/34] Improve documentation of the allocate method --- include/psa/crypto_se_driver.h | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 69cdababa..cd57b065d 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -977,7 +977,21 @@ typedef psa_status_t (*psa_drv_se_generate_key_t)(psa_drv_se_context_t *drv_cont * If one of the functions is not implemented, it should be set to NULL. */ typedef struct { - /** Function that allocates a slot. */ + /** Function that allocates a slot for a key. + * + * The core calls this function to determine a slot number, then + * calls the actual creation function (such as + * psa_drv_se_key_management_t::p_import or + * psa_drv_se_key_management_t::p_generate). + * + * If this function succeeds, the next call that the core makes to the + * driver is either the creation function or + * psa_drv_se_key_management_t::p_destroy. Note that + * if the platform is reset after this function returns, the core + * may either subsequently call + * psa_drv_se_key_management_t::p_destroy or may behave as if the + * last call to this function had not taken place. + */ psa_drv_se_allocate_key_t p_allocate; /** Function that performs a key import operation */ psa_drv_se_import_key_t p_import; From ae9964d3ef2bc9f76a05eb43fcc216bdf5252c72 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 14:55:14 +0200 Subject: [PATCH 11/34] Add validate_slot_number method to SE drivers Pave the way for allowing the application to choose the slot number in a secure element, rather than always letting the driver choose. --- include/psa/crypto_se_driver.h | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index cd57b065d..127f17b5c 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -833,6 +833,30 @@ typedef psa_status_t (*psa_drv_se_allocate_key_t)( const psa_key_attributes_t *attributes, psa_key_slot_number_t *key_slot); +/** \brief A function that determines whether a slot number is valid + * for a key. + * + * \param[in,out] drv_context The driver context structure. + * \param[in] attributes Attributes of the key. + * \param[in] key_slot Slot where the key is to be stored. + * + * \retval #PSA_SUCCESS + * The given slot number is valid for a key with the given + * attributes. + * \retval #PSA_ERROR_INVALID_ARGUMENT + * The given slot number is not valid for a key with the + * given attributes. This includes the case where the slot + * number is not valid at all. + * \retval #PSA_ERROR_ALREADY_EXISTS + * There is already a key with the specified slot number. + * Drivers may choose to return this error from the key + * creation function instead. + */ +typedef psa_status_t (*psa_drv_se_validate_slot_number_t)( + psa_drv_se_context_t *drv_context, + const psa_key_attributes_t *attributes, + psa_key_slot_number_t key_slot); + /** \brief A function that imports a key into a secure element in binary format * * This function can support any output from psa_export_key(). Refer to the @@ -993,6 +1017,16 @@ typedef struct { * last call to this function had not taken place. */ psa_drv_se_allocate_key_t p_allocate; + /** Function that checks the validity of a slot for a key. + * + * The core calls this function instead of + * psa_drv_se_key_management_t::p_allocate to create + * a key in a specific slot. It then calls the actual creation function + * (such as psa_drv_se_key_management_t::p_import or + * psa_drv_se_key_management_t::p_generate) or + * psa_drv_se_key_management_t::p_destroy. + */ + psa_drv_se_validate_slot_number_t p_validate_slot_number; /** Function that performs a key import operation */ psa_drv_se_import_key_t p_import; /** Function that performs a generation */ From 46d9439a5ec82d09b76d007f785cef74924f969c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 14:55:50 +0200 Subject: [PATCH 12/34] Support slot_number attribute when creating a key Allow the application to choose the slot number in a secure element, rather than always letting the driver choose. With this commit, any application may request any slot. In an implementation with isolation, it's up to the service to filter key creation requests and apply policies to limit which applications can request which slot. --- library/psa_crypto.c | 4 -- library/psa_crypto_se.c | 37 +++++++--- .../test_suite_psa_crypto_se_driver_hal.data | 9 +++ ...st_suite_psa_crypto_se_driver_hal.function | 70 +++++++++++++++++++ 4 files changed, 105 insertions(+), 15 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 5cb88de7e..856d8622d 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1582,10 +1582,6 @@ static psa_status_t psa_start_key_creation( * we can roll back to a state where the key doesn't exist. */ if( *p_drv != NULL ) { - /* Choosing a slot number is not supported yet. */ - if( attributes->core.flags & MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER ) - return( PSA_ERROR_NOT_SUPPORTED ); - status = psa_find_se_slot_for_key( attributes, *p_drv, &slot->data.se.slot_number ); if( status != PSA_SUCCESS ) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index bc7325180..ca38e2065 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -201,7 +201,6 @@ psa_status_t psa_find_se_slot_for_key( psa_key_slot_number_t *slot_number ) { psa_status_t status; - psa_drv_se_allocate_key_t p_allocate = NULL; /* If the lifetime is wrong, it's a bug in the library. */ if( driver->lifetime != psa_get_key_lifetime( attributes ) ) @@ -210,17 +209,33 @@ psa_status_t psa_find_se_slot_for_key( /* If the driver doesn't support key creation in any way, give up now. */ if( driver->methods->key_management == NULL ) return( PSA_ERROR_NOT_SUPPORTED ); - p_allocate = driver->methods->key_management->p_allocate; - /* If the driver doesn't tell us how to allocate a slot, that's - * not supported for the time being. */ - if( p_allocate == NULL ) - return( PSA_ERROR_NOT_SUPPORTED ); - - status = p_allocate( &driver->context, - driver->internal.persistent_data, - attributes, - slot_number ); + if( psa_get_key_slot_number( attributes, slot_number ) == PSA_SUCCESS ) + { + /* The application wants to use a specific slot. Allow it if + * the driver supports it. On a system with isolation, + * the crypto service must check that the application is + * permitted to request this slot. */ + psa_drv_se_validate_slot_number_t p_validate_slot_number = + driver->methods->key_management->p_validate_slot_number; + if( p_validate_slot_number == NULL ) + return( PSA_ERROR_NOT_SUPPORTED ); + status = p_validate_slot_number( &driver->context, attributes, + *slot_number ); + } + else + { + /* The application didn't tell us which slot to use. Let the driver + * choose. This is the normal case. */ + psa_drv_se_allocate_key_t p_allocate = + driver->methods->key_management->p_allocate; + if( p_allocate == NULL ) + return( PSA_ERROR_NOT_SUPPORTED ); + status = p_allocate( &driver->context, + driver->internal.persistent_data, + attributes, + slot_number ); + } return( status ); } diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data index 6fb65f02a..57aa47f76 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -39,6 +39,15 @@ key_creation_import_export:0:1 SE key import-export, check after restart (slot 3) key_creation_import_export:3:1 +Key creation in a specific slot (0) +key_creation_in_chosen_slot:0:PSA_SUCCESS + +Key creation in a specific slot (max) +key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ) - 1:PSA_SUCCESS + +Key creation in a specific slot (too large) +key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ):PSA_ERROR_INVALID_ARGUMENT + Key creation smoke test: AES-CTR key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CTR:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 9a5746476..8924ae1e7 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -177,6 +177,18 @@ static psa_status_t ram_allocate( psa_drv_se_context_t *context, return( PSA_ERROR_INSUFFICIENT_STORAGE ); } +static psa_status_t ram_validate_slot_number( + psa_drv_se_context_t *context, + const psa_key_attributes_t *attributes, + psa_key_slot_number_t slot_number ) +{ + (void) context; + (void) attributes; + if( slot_number >= ARRAY_LENGTH( ram_slots ) ) + return( PSA_ERROR_INVALID_ARGUMENT ); + return( PSA_SUCCESS ); +} + /****************************************************************/ @@ -536,6 +548,64 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void key_creation_in_chosen_slot( int slot_arg, + int expected_status_arg ) +{ + psa_key_slot_number_t wanted_slot = slot_arg; + psa_status_t expected_status = expected_status_arg; + psa_status_t status; + psa_drv_se_t driver; + psa_drv_se_key_management_t key_management; + psa_key_lifetime_t lifetime = 2; + psa_key_id_t id = 1; + psa_key_handle_t handle = 0; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + const uint8_t key_material[3] = {0xfa, 0xca, 0xde}; + + memset( &driver, 0, sizeof( driver ) ); + memset( &key_management, 0, sizeof( key_management ) ); + driver.hal_version = PSA_DRV_SE_HAL_VERSION; + driver.key_management = &key_management; + driver.persistent_data_size = sizeof( ram_slot_usage_t ); + key_management.p_validate_slot_number = ram_validate_slot_number; + key_management.p_import = ram_import; + key_management.p_destroy = ram_destroy; + key_management.p_export = ram_export; + + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + PSA_ASSERT( psa_crypto_init( ) ); + + /* Create a key. */ + psa_set_key_id( &attributes, id ); + psa_set_key_lifetime( &attributes, lifetime ); + psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_EXPORT ); + psa_set_key_type( &attributes, PSA_KEY_TYPE_RAW_DATA ); + psa_set_key_slot_number( &attributes, wanted_slot ); + status = psa_import_key( &attributes, + key_material, sizeof( key_material ), + &handle ); + TEST_EQUAL( status, expected_status ); + + if( status == PSA_SUCCESS ) + { + /* Test that the key was created in the expected slot. */ + TEST_EQUAL( ram_slots[wanted_slot].type, PSA_KEY_TYPE_RAW_DATA ); + + /* Test that the key is reported with the correct attributes, + * including the expected slot. */ + PSA_ASSERT( psa_get_key_attributes( handle, &attributes ) ); + + PSA_ASSERT( psa_destroy_key( handle ) ); + } + +exit: + PSA_DONE( ); + ram_slots_reset( ); + psa_purge_storage( ); +} +/* END_CASE */ + /* BEGIN_CASE */ void key_creation_smoke( int type_arg, int alg_arg, data_t *key_material ) From 0a1104474b5a2ff2405d7d1807506c20a727333d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 14:59:15 +0200 Subject: [PATCH 13/34] Test restarting after creating a key in a specific slot --- .../test_suite_psa_crypto_se_driver_hal.data | 12 ++++++-- ...st_suite_psa_crypto_se_driver_hal.function | 28 +++++++++++++------ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data index 57aa47f76..e6482ddbc 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -40,13 +40,19 @@ SE key import-export, check after restart (slot 3) key_creation_import_export:3:1 Key creation in a specific slot (0) -key_creation_in_chosen_slot:0:PSA_SUCCESS +key_creation_in_chosen_slot:0:0:PSA_SUCCESS Key creation in a specific slot (max) -key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ) - 1:PSA_SUCCESS +key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ) - 1:0:PSA_SUCCESS + +Key creation in a specific slot (0, restart) +key_creation_in_chosen_slot:0:1:PSA_SUCCESS + +Key creation in a specific slot (max, restart) +key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ) - 1:1:PSA_SUCCESS Key creation in a specific slot (too large) -key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ):PSA_ERROR_INVALID_ARGUMENT +key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ):0:PSA_ERROR_INVALID_ARGUMENT Key creation smoke test: AES-CTR key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CTR:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 8924ae1e7..0fab0433f 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -550,6 +550,7 @@ exit: /* BEGIN_CASE */ void key_creation_in_chosen_slot( int slot_arg, + int restart, int expected_status_arg ) { psa_key_slot_number_t wanted_slot = slot_arg; @@ -587,18 +588,27 @@ void key_creation_in_chosen_slot( int slot_arg, &handle ); TEST_EQUAL( status, expected_status ); - if( status == PSA_SUCCESS ) + if( status != PSA_SUCCESS ) + goto exit; + + /* Maybe restart, to check that the information is saved correctly. */ + if( restart ) { - /* Test that the key was created in the expected slot. */ - TEST_EQUAL( ram_slots[wanted_slot].type, PSA_KEY_TYPE_RAW_DATA ); - - /* Test that the key is reported with the correct attributes, - * including the expected slot. */ - PSA_ASSERT( psa_get_key_attributes( handle, &attributes ) ); - - PSA_ASSERT( psa_destroy_key( handle ) ); + mbedtls_psa_crypto_free( ); + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + PSA_ASSERT( psa_crypto_init( ) ); + PSA_ASSERT( psa_open_key( id, &handle ) ); } + /* Test that the key was created in the expected slot. */ + TEST_EQUAL( ram_slots[wanted_slot].type, PSA_KEY_TYPE_RAW_DATA ); + + /* Test that the key is reported with the correct attributes, + * including the expected slot. */ + PSA_ASSERT( psa_get_key_attributes( handle, &attributes ) ); + + PSA_ASSERT( psa_destroy_key( handle ) ); + exit: PSA_DONE( ); ram_slots_reset( ); From 9d75202efb8267e34e871a023539b2d9050aedf9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 9 Aug 2019 11:33:48 +0200 Subject: [PATCH 14/34] Clarify and expand the documentation of the allocate/create sequence --- include/psa/crypto_se_driver.h | 74 +++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 127f17b5c..9a5d97da7 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -811,6 +811,42 @@ typedef struct { /**@{*/ /** \brief A function that allocates a slot for a key. + * + * To create a key in a specific slot in a secure element, the core + * first calls this function to determine a valid slot number, + * then calls a function to create the key material in that slot. + * For example, in nominal conditions (that is, if no error occurs), + * the effect of a call to psa_import_key() with a lifetime that places + * the key in a secure element is the following: + * -# The core calls psa_drv_se_key_management_t::p_allocate + * (or in some implementations + * psa_drv_se_key_management_t::p_validate_slot_number). The driver + * selects (or validates) a suitable slot number given the key attributes + * and the state of the secure element. + * -# The core calls psa_drv_se_key_management_t::p_import to import + * the key material in the selected slot. + * + * Other key creation methods lead to similar sequences. For example, the + * sequence for psa_generate_key() is the same except that the second step + * is a call to psa_drv_se_key_management_t::p_generate. + * + * In case of errors, other behaviors are possible. + * - If the PSA Cryptography subsystem dies after the first step, + * for example because the device has lost power abruptly, + * the second step may never happen, or may happen after a reset + * and re-initialization. Alternatively, after a reset and + * re-initialization, the core may call + * psa_drv_se_key_management_t::p_destroy on the slot number that + * was allocated (or validated) instead of calling a key creation function. + * - If an error occurs, the core may call + * psa_drv_se_key_management_t::p_destroy on the slot number that + * was allocated (or validated) instead of calling a key creation function. + * + * Errors and system resets also have an impact on the driver's persistent + * data. If a reset happens before the overall key creation process is + * completed (before or after the second step above), it is unspecified + * whether the persistent data after the reset is identical to what it + * was before or after the call to `p_allocate` (or `p_validate_slot_number`). * * \param[in,out] drv_context The driver context structure. * \param[in,out] persistent_data A pointer to the persistent data @@ -836,6 +872,18 @@ typedef psa_status_t (*psa_drv_se_allocate_key_t)( /** \brief A function that determines whether a slot number is valid * for a key. * + * To create a key in a specific slot in a secure element, the core + * first calls this function to validate the choice of slot number, + * then calls a function to create the key material in that slot. + * See the documentation of #psa_drv_se_allocate_key_t for more details. + * + * As of the PSA Cryptography API specification version 1.0, there is no way + * for applications to trigger a call to this function. However some + * implementations offer the capability to create or declare a key in + * a specific slot via implementation-specific means, generally for the + * sake of initial device provisioning or onboarding. Such a mechanism may + * be added to a future version of the PSA Cryptography API specification. + * * \param[in,out] drv_context The driver context structure. * \param[in] attributes Attributes of the key. * \param[in] key_slot Slot where the key is to be stored. @@ -1001,31 +1049,9 @@ typedef psa_status_t (*psa_drv_se_generate_key_t)(psa_drv_se_context_t *drv_cont * If one of the functions is not implemented, it should be set to NULL. */ typedef struct { - /** Function that allocates a slot for a key. - * - * The core calls this function to determine a slot number, then - * calls the actual creation function (such as - * psa_drv_se_key_management_t::p_import or - * psa_drv_se_key_management_t::p_generate). - * - * If this function succeeds, the next call that the core makes to the - * driver is either the creation function or - * psa_drv_se_key_management_t::p_destroy. Note that - * if the platform is reset after this function returns, the core - * may either subsequently call - * psa_drv_se_key_management_t::p_destroy or may behave as if the - * last call to this function had not taken place. - */ + /** Function that allocates a slot for a key. */ psa_drv_se_allocate_key_t p_allocate; - /** Function that checks the validity of a slot for a key. - * - * The core calls this function instead of - * psa_drv_se_key_management_t::p_allocate to create - * a key in a specific slot. It then calls the actual creation function - * (such as psa_drv_se_key_management_t::p_import or - * psa_drv_se_key_management_t::p_generate) or - * psa_drv_se_key_management_t::p_destroy. - */ + /** Function that checks the validity of a slot for a key. */ psa_drv_se_validate_slot_number_t p_validate_slot_number; /** Function that performs a key import operation */ psa_drv_se_import_key_t p_import; From df17914e01f923dd1c32c9e6067ca46cd4d8e1cd Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 15 Jul 2019 22:02:14 +0200 Subject: [PATCH 15/34] psa_start_key_creation: take the method as a parameter Let psa_start_key_creation know what type of key creation this is. This will be used at least for key registration in a secure element, which is a peculiar kind of creation since it uses existing key material. --- library/psa_crypto.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 856d8622d..0c8b99b37 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1506,6 +1506,16 @@ static psa_status_t psa_validate_key_attributes( return( PSA_SUCCESS ); } +/** An enumeration indicating how a key is created. + */ +typedef enum +{ + PSA_KEY_CREATION_IMPORT, + PSA_KEY_CREATION_GENERATE, + PSA_KEY_CREATION_DERIVE, + PSA_KEY_CREATION_COPY, +} psa_key_creation_method_t; + /** Prepare a key slot to receive key material. * * This function allocates a key slot and sets its metadata. @@ -1520,6 +1530,7 @@ static psa_status_t psa_validate_key_attributes( * In case of failure at any step, stop the sequence and call * psa_fail_key_creation(). * + * \param method An identification of the calling function. * \param[in] attributes Key attributes for the new key. * \param[out] handle On success, a handle for the allocated slot. * \param[out] p_slot On success, a pointer to the prepared slot. @@ -1532,6 +1543,7 @@ static psa_status_t psa_validate_key_attributes( * You must call psa_fail_key_creation() to wipe and free the slot. */ static psa_status_t psa_start_key_creation( + psa_key_creation_method_t method, const psa_key_attributes_t *attributes, psa_key_handle_t *handle, psa_key_slot_t **p_slot, @@ -1540,6 +1552,7 @@ static psa_status_t psa_start_key_creation( psa_status_t status; psa_key_slot_t *slot; + (void) method; *p_drv = NULL; status = psa_validate_key_attributes( attributes, p_drv ); @@ -1796,7 +1809,8 @@ psa_status_t psa_import_key( const psa_key_attributes_t *attributes, psa_key_slot_t *slot = NULL; psa_se_drv_table_entry_t *driver = NULL; - status = psa_start_key_creation( attributes, handle, &slot, &driver ); + status = psa_start_key_creation( PSA_KEY_CREATION_IMPORT, attributes, + handle, &slot, &driver ); if( status != PSA_SUCCESS ) goto exit; @@ -1899,7 +1913,8 @@ psa_status_t psa_copy_key( psa_key_handle_t source_handle, if( status != PSA_SUCCESS ) goto exit; - status = psa_start_key_creation( &actual_attributes, + status = psa_start_key_creation( PSA_KEY_CREATION_COPY, + &actual_attributes, target_handle, &target_slot, &driver ); if( status != PSA_SUCCESS ) goto exit; @@ -4817,7 +4832,8 @@ psa_status_t psa_key_derivation_output_key( const psa_key_attributes_t *attribut psa_status_t status; psa_key_slot_t *slot = NULL; psa_se_drv_table_entry_t *driver = NULL; - status = psa_start_key_creation( attributes, handle, &slot, &driver ); + status = psa_start_key_creation( PSA_KEY_CREATION_DERIVE, + attributes, handle, &slot, &driver ); #if defined(MBEDTLS_PSA_CRYPTO_SE_C) if( driver != NULL ) { @@ -5863,7 +5879,8 @@ psa_status_t psa_generate_key( const psa_key_attributes_t *attributes, psa_status_t status; psa_key_slot_t *slot = NULL; psa_se_drv_table_entry_t *driver = NULL; - status = psa_start_key_creation( attributes, handle, &slot, &driver ); + status = psa_start_key_creation( PSA_KEY_CREATION_GENERATE, + attributes, handle, &slot, &driver ); #if defined(MBEDTLS_PSA_CRYPTO_SE_C) if( driver != NULL ) { From e88c2c1338e57cfae2c793eab111860e01c77edc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 16:44:14 +0200 Subject: [PATCH 16/34] Pass the key creation method to drivers Pass the key creation method (import/generate/derive/copy) to the driver methods to allocate or validate a slot number. This allows drivers to enforce policies such as "this key slot can only be used for keys generated inside the secure element". --- include/psa/crypto_se_driver.h | 46 ++++++++++++++----- library/psa_crypto.c | 12 +---- library/psa_crypto_se.c | 6 ++- library/psa_crypto_se.h | 1 + ...st_suite_psa_crypto_se_driver_hal.function | 6 +++ 5 files changed, 47 insertions(+), 24 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 9a5d97da7..cdf0de116 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -810,25 +810,45 @@ typedef struct { */ /**@{*/ +/** An enumeration indicating how a key is created. + */ +typedef enum +{ + PSA_KEY_CREATION_IMPORT, /**< During psa_import_key() */ + PSA_KEY_CREATION_GENERATE, /**< During psa_generate_key() */ + PSA_KEY_CREATION_DERIVE, /**< During psa_key_derivation_output_key() */ + PSA_KEY_CREATION_COPY, /**< During psa_copy_key() */ +} psa_key_creation_method_t; + /** \brief A function that allocates a slot for a key. * * To create a key in a specific slot in a secure element, the core * first calls this function to determine a valid slot number, * then calls a function to create the key material in that slot. - * For example, in nominal conditions (that is, if no error occurs), - * the effect of a call to psa_import_key() with a lifetime that places - * the key in a secure element is the following: + * In nominal conditions (that is, if no error occurs), + * the effect of a call to a key creation function in the PSA Cryptography + * API with a lifetime that places the key in a secure element is the + * following: * -# The core calls psa_drv_se_key_management_t::p_allocate * (or in some implementations * psa_drv_se_key_management_t::p_validate_slot_number). The driver * selects (or validates) a suitable slot number given the key attributes * and the state of the secure element. - * -# The core calls psa_drv_se_key_management_t::p_import to import - * the key material in the selected slot. + * -# The core calls a key creation function in the driver. * - * Other key creation methods lead to similar sequences. For example, the - * sequence for psa_generate_key() is the same except that the second step - * is a call to psa_drv_se_key_management_t::p_generate. + * The key creation functions in the PSA Cryptography API are: + * - psa_import_key(), which causes + * a call to `p_allocate` with \p method = #PSA_KEY_CREATION_IMPORT + * then a call to psa_drv_se_key_management_t::p_import. + * - psa_generate_key(), which causes + * a call to `p_allocate` with \p method = #PSA_KEY_CREATION_GENERATE + * then a call to psa_drv_se_key_management_t::p_import. + * - psa_key_derivation_output_key(), which causes + * a call to `p_allocate` with \p method = #PSA_KEY_CREATION_DERIVE + * then a call to psa_drv_se_key_derivation_t::p_derive. + * - psa_copy_key(), which causes + * a call to `p_allocate` with \p method = #PSA_KEY_CREATION_COPY + * then a call to psa_drv_se_key_management_t::p_export. * * In case of errors, other behaviors are possible. * - If the PSA Cryptography subsystem dies after the first step, @@ -852,6 +872,7 @@ typedef struct { * \param[in,out] persistent_data A pointer to the persistent data * that allows writing. * \param[in] attributes Attributes of the key. + * \param method The way in which the key is being created. * \param[out] key_slot Slot where the key will be stored. * This must be a valid slot for a key of the * chosen type. It must be unoccupied. @@ -867,6 +888,7 @@ typedef psa_status_t (*psa_drv_se_allocate_key_t)( psa_drv_se_context_t *drv_context, void *persistent_data, const psa_key_attributes_t *attributes, + psa_key_creation_method_t method, psa_key_slot_number_t *key_slot); /** \brief A function that determines whether a slot number is valid @@ -884,9 +906,10 @@ typedef psa_status_t (*psa_drv_se_allocate_key_t)( * sake of initial device provisioning or onboarding. Such a mechanism may * be added to a future version of the PSA Cryptography API specification. * - * \param[in,out] drv_context The driver context structure. - * \param[in] attributes Attributes of the key. - * \param[in] key_slot Slot where the key is to be stored. + * \param[in,out] drv_context The driver context structure. + * \param[in] attributes Attributes of the key. + * \param method The way in which the key is being created. + * \param[in] key_slot Slot where the key is to be stored. * * \retval #PSA_SUCCESS * The given slot number is valid for a key with the given @@ -903,6 +926,7 @@ typedef psa_status_t (*psa_drv_se_allocate_key_t)( typedef psa_status_t (*psa_drv_se_validate_slot_number_t)( psa_drv_se_context_t *drv_context, const psa_key_attributes_t *attributes, + psa_key_creation_method_t method, psa_key_slot_number_t key_slot); /** \brief A function that imports a key into a secure element in binary format diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 0c8b99b37..08f9476f9 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1506,16 +1506,6 @@ static psa_status_t psa_validate_key_attributes( return( PSA_SUCCESS ); } -/** An enumeration indicating how a key is created. - */ -typedef enum -{ - PSA_KEY_CREATION_IMPORT, - PSA_KEY_CREATION_GENERATE, - PSA_KEY_CREATION_DERIVE, - PSA_KEY_CREATION_COPY, -} psa_key_creation_method_t; - /** Prepare a key slot to receive key material. * * This function allocates a key slot and sets its metadata. @@ -1595,7 +1585,7 @@ static psa_status_t psa_start_key_creation( * we can roll back to a state where the key doesn't exist. */ if( *p_drv != NULL ) { - status = psa_find_se_slot_for_key( attributes, *p_drv, + status = psa_find_se_slot_for_key( attributes, method, *p_drv, &slot->data.se.slot_number ); if( status != PSA_SUCCESS ) return( status ); diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index ca38e2065..523c62105 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -197,6 +197,7 @@ psa_status_t psa_destroy_se_persistent_data( psa_key_lifetime_t lifetime ) psa_status_t psa_find_se_slot_for_key( const psa_key_attributes_t *attributes, + psa_key_creation_method_t method, psa_se_drv_table_entry_t *driver, psa_key_slot_number_t *slot_number ) { @@ -220,7 +221,8 @@ psa_status_t psa_find_se_slot_for_key( driver->methods->key_management->p_validate_slot_number; if( p_validate_slot_number == NULL ) return( PSA_ERROR_NOT_SUPPORTED ); - status = p_validate_slot_number( &driver->context, attributes, + status = p_validate_slot_number( &driver->context, + attributes, method, *slot_number ); } else @@ -233,7 +235,7 @@ psa_status_t psa_find_se_slot_for_key( return( PSA_ERROR_NOT_SUPPORTED ); status = p_allocate( &driver->context, driver->internal.persistent_data, - attributes, + attributes, method, slot_number ); } return( status ); diff --git a/library/psa_crypto_se.h b/library/psa_crypto_se.h index 378c78ffe..900a72bd3 100644 --- a/library/psa_crypto_se.h +++ b/library/psa_crypto_se.h @@ -135,6 +135,7 @@ psa_drv_se_context_t *psa_get_se_driver_context( */ psa_status_t psa_find_se_slot_for_key( const psa_key_attributes_t *attributes, + psa_key_creation_method_t method, psa_se_drv_table_entry_t *driver, psa_key_slot_number_t *slot_number ); diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 0fab0433f..19b421dd3 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -41,10 +41,12 @@ static psa_status_t counter_allocate( psa_drv_se_context_t *context, void *persistent_data, const psa_key_attributes_t *attributes, + psa_key_creation_method_t method, psa_key_slot_number_t *slot_number ) { psa_key_slot_number_t *p_counter = persistent_data; (void) attributes; + (void) method; if( context->persistent_data_size != sizeof( psa_key_slot_number_t ) ) return( PSA_ERROR_DETECTED_BY_DRIVER ); ++*p_counter; @@ -162,10 +164,12 @@ static psa_status_t ram_destroy( psa_drv_se_context_t *context, static psa_status_t ram_allocate( psa_drv_se_context_t *context, void *persistent_data, const psa_key_attributes_t *attributes, + psa_key_creation_method_t method, psa_key_slot_number_t *slot_number ) { ram_slot_usage_t *slot_usage = persistent_data; (void) attributes; + (void) method; DRIVER_ASSERT( context->persistent_data_size == sizeof( ram_slot_usage_t ) ); for( *slot_number = ram_min_slot; *slot_number < ARRAY_LENGTH( ram_slots ); @@ -180,10 +184,12 @@ static psa_status_t ram_allocate( psa_drv_se_context_t *context, static psa_status_t ram_validate_slot_number( psa_drv_se_context_t *context, const psa_key_attributes_t *attributes, + psa_key_creation_method_t method, psa_key_slot_number_t slot_number ) { (void) context; (void) attributes; + (void) method; if( slot_number >= ARRAY_LENGTH( ram_slots ) ) return( PSA_ERROR_INVALID_ARGUMENT ); return( PSA_SUCCESS ); From d772958ffc8c500af436ca700d83815244789e43 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 15:55:54 +0200 Subject: [PATCH 17/34] New function mbedtls_psa_register_se_key Register an existing key in a secure element. Minimal implementation that doesn't call any driver method and just lets the application declare whatever it wants. --- include/psa/crypto_extra.h | 38 ++++++++++ include/psa/crypto_se_driver.h | 1 + library/psa_crypto.c | 75 +++++++++++++++++-- .../test_suite_psa_crypto_se_driver_hal.data | 12 +++ ...st_suite_psa_crypto_se_driver_hal.function | 56 ++++++++++++++ 5 files changed, 176 insertions(+), 6 deletions(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 130ce7544..355012236 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -175,6 +175,44 @@ static inline void psa_clear_key_slot_number( attributes->core.flags &= ~MBEDTLS_PSA_KA_FLAG_HAS_SLOT_NUMBER; } +/** Register a key that is already present in a secure element. + * + * The key must be located in a secure element designated by the + * lifetime field in \p attributes, in the slot set with + * psa_set_key_slot_number() in the attribute structure. + * This function makes the key available through the key identifier + * specified in \p attributes. + * + * \param[in] attributes The attributes of the existing key. + * + * \retval #PSA_SUCCESS + * The key was successfully registered. + * Note that depending on the design of the driver, this may or may + * not guarantee that a key actually exists in the designated slot + * and is compatible with the specified attributes. + * \retval #PSA_ERROR_ALREADY_EXISTS + * There is already a key with the identifier specified in + * \p attributes. + * \retval #PSA_ERROR_INVALID_ARGUMENT + * \p attributes specifies a lifetime which is not located + * in a secure element. + * \retval #PSA_ERROR_INVALID_ARGUMENT + * No slot number is specified in \p attributes, + * or the specified slot number is not valid. + * \retval #PSA_ERROR_NOT_PERMITTED + * The caller is not authorized to register the specified key slot. + * \retval #PSA_ERROR_INSUFFICIENT_MEMORY + * \retval #PSA_ERROR_COMMUNICATION_FAILURE + * \retval #PSA_ERROR_HARDWARE_FAILURE + * \retval #PSA_ERROR_CORRUPTION_DETECTED + * \retval #PSA_ERROR_BAD_STATE + * The library has not been previously initialized by psa_crypto_init(). + * It is implementation-dependent whether a failure to initialize + * results in this error code. + */ +psa_status_t mbedtls_psa_register_se_key( + const psa_key_attributes_t *attributes); + #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ /**@}*/ diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index cdf0de116..1b0b3b2cc 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -818,6 +818,7 @@ typedef enum PSA_KEY_CREATION_GENERATE, /**< During psa_generate_key() */ PSA_KEY_CREATION_DERIVE, /**< During psa_key_derivation_output_key() */ PSA_KEY_CREATION_COPY, /**< During psa_copy_key() */ + PSA_KEY_CREATION_REGISTER, /*TEMPORARY*/ } psa_key_creation_method_t; /** \brief A function that allocates a slot for a key. diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 08f9476f9..086ba82d3 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1570,7 +1570,8 @@ static psa_status_t psa_start_key_creation( slot->attr.flags &= ~MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY; #if defined(MBEDTLS_PSA_CRYPTO_SE_C) - /* For a key in a secure element, we need to do three things: + /* For a key in a secure element, we need to do three things + * when creating a key (but not when registering an existing key): * create the key file in internal storage, create the * key inside the secure element, and update the driver's * persistent data. Start a transaction that will encompass these @@ -1583,7 +1584,7 @@ static psa_status_t psa_start_key_creation( * secure element driver updates its persistent state, but we do not yet * save the driver's persistent state, so that if the power fails, * we can roll back to a state where the key doesn't exist. */ - if( *p_drv != NULL ) + if( *p_drv != NULL && method != PSA_KEY_CREATION_REGISTER ) { status = psa_find_se_slot_for_key( attributes, method, *p_drv, &slot->data.se.slot_number ); @@ -1677,7 +1678,13 @@ static psa_status_t psa_finish_key_creation( #endif /* defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */ #if defined(MBEDTLS_PSA_CRYPTO_SE_C) - if( driver != NULL ) + /* Finish the transaction for a key creation. This does not + * happen when registering an existing key. Detect this case + * by checking whether a transaction is in progress (actual + * creation of a key in a secure element requires a transaction, + * but registration doesn't use one). */ + if( driver != NULL && + psa_crypto_transaction.unknown.type == PSA_CRYPTO_TRANSACTION_CREATE_KEY ) { status = psa_save_se_persistent_data( driver ); if( status != PSA_SUCCESS ) @@ -1720,9 +1727,12 @@ static void psa_fail_key_creation( psa_key_slot_t *slot, * to internal storage), we need to destroy the key in the secure * element. */ - /* Abort the ongoing transaction if any. We already did what it - * takes to undo any partial creation. All that's left is to update - * the transaction data itself. */ + /* Abort the ongoing transaction if any (there may not be one if + * the creation process failed before starting one, or if the + * key creation is a registration of a key in a secure element). + * Earlier functions must already have done what it takes to undo any + * partial creation. All that's left is to update the transaction data + * itself. */ (void) psa_crypto_stop_transaction( ); #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ @@ -1852,6 +1862,59 @@ exit: return( status ); } +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +psa_status_t mbedtls_psa_register_se_key( + const psa_key_attributes_t *attributes ) +{ + psa_status_t status; + psa_key_slot_t *slot = NULL; + psa_se_drv_table_entry_t *driver = NULL; + const psa_drv_se_t *drv; + psa_key_handle_t handle = 0; + + /* Leaving attributes unspecified is not currently supported. + * It could make sense to query the key type and size from the + * secure element, but not all secure elements support this + * and the driver HAL doesn't currently support it. */ + if( psa_get_key_type( attributes ) == PSA_KEY_TYPE_NONE ) + return( PSA_ERROR_NOT_SUPPORTED ); + if( psa_get_key_bits( attributes ) == 0 ) + return( PSA_ERROR_NOT_SUPPORTED ); + + status = psa_start_key_creation( PSA_KEY_CREATION_REGISTER, attributes, + &handle, &slot, &driver ); + if( status != PSA_SUCCESS ) + goto exit; + + if( driver == NULL ) + { + status = PSA_ERROR_INVALID_ARGUMENT; + goto exit; + } + drv = psa_get_se_driver_methods( driver ); + + if ( psa_get_key_slot_number( attributes, + &slot->data.se.slot_number ) != PSA_SUCCESS ) + { + /* The application didn't specify a slot number. This doesn't + * make sense when registering a slot. */ + status = PSA_ERROR_INVALID_ARGUMENT; + goto exit; + } + + status = psa_finish_key_creation( slot, driver ); + +exit: + if( status != PSA_SUCCESS ) + { + psa_fail_key_creation( slot, driver ); + } + /* Registration doesn't keep the key in RAM. */ + psa_close_key( handle ); + return( status ); +} +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + static psa_status_t psa_copy_key_material( const psa_key_slot_t *source, psa_key_slot_t *target ) { diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data index e6482ddbc..a8dd0c71d 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -110,3 +110,15 @@ key_creation_smoke:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ Generate key: not supported generate_key_not_supported:PSA_KEY_TYPE_AES:128 + +Key registration: smoke test +register_key_smoke_test:MIN_DRIVER_LIFETIME:PSA_SUCCESS + +Key registration: invalid lifetime (volatile) +register_key_smoke_test:PSA_KEY_LIFETIME_VOLATILE:PSA_ERROR_INVALID_ARGUMENT + +Key registration: invalid lifetime (internal storage) +register_key_smoke_test:PSA_KEY_LIFETIME_PERSISTENT:PSA_ERROR_INVALID_ARGUMENT + +Key registration: invalid lifetime (no registered driver) +register_key_smoke_test:MIN_DRIVER_LIFETIME + 1:PSA_ERROR_INVALID_ARGUMENT diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 19b421dd3..2edf94f55 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -716,3 +716,59 @@ exit: psa_purge_storage( ); } /* END_CASE */ + +/* BEGIN_CASE */ +void register_key_smoke_test( int lifetime_arg, int expected_status_arg ) +{ + psa_key_lifetime_t lifetime = lifetime_arg; + psa_status_t expected_status = expected_status_arg; + psa_drv_se_t driver; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_key_id_t id = 1; + size_t bit_size = 48; + psa_key_slot_number_t wanted_slot = 0x123456789; + psa_key_handle_t handle = 0; + psa_status_t status; + + memset( &driver, 0, sizeof( driver ) ); + driver.hal_version = PSA_DRV_SE_HAL_VERSION; + + PSA_ASSERT( psa_register_se_driver( MIN_DRIVER_LIFETIME, &driver ) ); + PSA_ASSERT( psa_crypto_init( ) ); + + psa_set_key_id( &attributes, id ); + psa_set_key_lifetime( &attributes, lifetime ); + psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_EXPORT ); + psa_set_key_type( &attributes, PSA_KEY_TYPE_RAW_DATA ); + psa_set_key_bits( &attributes, bit_size ); + psa_set_key_slot_number( &attributes, wanted_slot ); + + status = mbedtls_psa_register_se_key( &attributes ); + TEST_EQUAL( status, expected_status ); + + if( status != PSA_SUCCESS ) + goto exit; + + /* Test that the key exists and has the expected attributes. */ + PSA_ASSERT( psa_open_key( id, &handle ) ); + if( ! check_key_attributes( handle, &attributes ) ) + goto exit; + PSA_ASSERT( psa_close_key( handle ) ); + + /* Restart and try again. */ + PSA_DONE( ); + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + PSA_ASSERT( psa_crypto_init( ) ); + PSA_ASSERT( psa_open_key( id, &handle ) ); + if( ! check_key_attributes( handle, &attributes ) ) + goto exit; + /* This time, destroy the key. */ + PSA_ASSERT( psa_destroy_key( handle ) ); + +exit: + psa_reset_key_attributes( &attributes ); + psa_destroy_key( handle ); + PSA_DONE( ); + psa_purge_storage( ); +} +/* END_CASE */ From a5f8749812db2ccde29b3e1ecb4bd4709eb7bf28 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 16:46:18 +0200 Subject: [PATCH 18/34] SE key registration: call p_validate_slot_number When registering a key in a secure element, if the driver has a p_validate_slot_number method, call it. --- include/psa/crypto_se_driver.h | 22 +++++++++++++++++++++- library/psa_crypto.c | 15 +++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 1b0b3b2cc..f04aa3468 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -818,7 +818,27 @@ typedef enum PSA_KEY_CREATION_GENERATE, /**< During psa_generate_key() */ PSA_KEY_CREATION_DERIVE, /**< During psa_key_derivation_output_key() */ PSA_KEY_CREATION_COPY, /**< During psa_copy_key() */ - PSA_KEY_CREATION_REGISTER, /*TEMPORARY*/ + +#ifndef __DOXYGEN_ONLY__ + /** A key is being registered with mbedtls_psa_register_se_key(). + * + * The core only passes this value to + * psa_drv_se_key_management_t::p_validate_slot_number, not to + * psa_drv_se_key_management_t::p_allocate. The call to + * `p_validate_slot_number` is not followed by any other call to the + * driver: the key is considered successfully registered if the call to + * `p_validate_slot_number` succeeds, or if `p_validate_slot_number` is + * null. + * + * With this creation method, the driver must return #PSA_SUCCESS if + * the given attributes are compatible with the existing key in the slot, + * and #PSA_ERROR_DOES_NOT_EXIST if the driver can determine that there + * is no key with the specified slot number. + * + * This is an Mbed Crypto extension. + */ + PSA_KEY_CREATION_REGISTER, +#endif } psa_key_creation_method_t; /** \brief A function that allocates a slot for a key. diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 086ba82d3..87ac037b6 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1902,6 +1902,21 @@ psa_status_t mbedtls_psa_register_se_key( goto exit; } + /* If the driver has a slot number validation method, call it. + * If it doesn't, it means the secure element is unable to validate + * anything and so we have to trust the application. */ + if( drv->key_management != NULL && + drv->key_management->p_validate_slot_number != NULL ) + { + status = drv->key_management->p_validate_slot_number( + psa_get_se_driver_context( driver ), + attributes, + PSA_KEY_CREATION_REGISTER, + slot->data.se.slot_number ); + if( status != PSA_SUCCESS ) + goto exit; + } + status = psa_finish_key_creation( slot, driver ); exit: From 49bd58274eec426468bab9fd5753bccc9393bfc0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 5 Aug 2019 17:17:52 +0200 Subject: [PATCH 19/34] Test the call to p_validate_slot_number when registering a key --- .../test_suite_psa_crypto_se_driver_hal.data | 14 +++++-- ...st_suite_psa_crypto_se_driver_hal.function | 39 ++++++++++++++++++- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data index a8dd0c71d..267c7b88b 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -112,13 +112,19 @@ Generate key: not supported generate_key_not_supported:PSA_KEY_TYPE_AES:128 Key registration: smoke test -register_key_smoke_test:MIN_DRIVER_LIFETIME:PSA_SUCCESS +register_key_smoke_test:MIN_DRIVER_LIFETIME:-1:PSA_SUCCESS Key registration: invalid lifetime (volatile) -register_key_smoke_test:PSA_KEY_LIFETIME_VOLATILE:PSA_ERROR_INVALID_ARGUMENT +register_key_smoke_test:PSA_KEY_LIFETIME_VOLATILE:-1:PSA_ERROR_INVALID_ARGUMENT Key registration: invalid lifetime (internal storage) -register_key_smoke_test:PSA_KEY_LIFETIME_PERSISTENT:PSA_ERROR_INVALID_ARGUMENT +register_key_smoke_test:PSA_KEY_LIFETIME_PERSISTENT:-1:PSA_ERROR_INVALID_ARGUMENT Key registration: invalid lifetime (no registered driver) -register_key_smoke_test:MIN_DRIVER_LIFETIME + 1:PSA_ERROR_INVALID_ARGUMENT +register_key_smoke_test:MIN_DRIVER_LIFETIME + 1:-1:PSA_ERROR_INVALID_ARGUMENT + +Key registration: with driver validation (accepted) +register_key_smoke_test:MIN_DRIVER_LIFETIME:1:PSA_SUCCESS + +Key registration: with driver validation (rejected) +register_key_smoke_test:MIN_DRIVER_LIFETIME:0:PSA_ERROR_NOT_PERMITTED diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 2edf94f55..4673835d5 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -37,6 +37,28 @@ /* Miscellaneous driver methods */ /****************************************************************/ +typedef struct +{ + psa_key_slot_number_t slot_number; + psa_key_creation_method_t method; + psa_status_t status; +} validate_slot_number_directions_t; +static validate_slot_number_directions_t validate_slot_number_directions; + +/* Validate a choice of slot number as directed. */ +static psa_status_t validate_slot_number_as_directed( + psa_drv_se_context_t *context, + const psa_key_attributes_t *attributes, + psa_key_creation_method_t method, + psa_key_slot_number_t slot_number ) +{ + (void) context; + (void) attributes; + DRIVER_ASSERT( slot_number == validate_slot_number_directions.slot_number ); + DRIVER_ASSERT( method == validate_slot_number_directions.method ); + return( validate_slot_number_directions.status ); +} + /* Allocate slot numbers with a monotonic counter. */ static psa_status_t counter_allocate( psa_drv_se_context_t *context, void *persistent_data, @@ -718,11 +740,14 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void register_key_smoke_test( int lifetime_arg, int expected_status_arg ) +void register_key_smoke_test( int lifetime_arg, + int validate, + int expected_status_arg ) { psa_key_lifetime_t lifetime = lifetime_arg; psa_status_t expected_status = expected_status_arg; psa_drv_se_t driver; + psa_drv_se_key_management_t key_management; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_key_id_t id = 1; size_t bit_size = 48; @@ -732,6 +757,16 @@ void register_key_smoke_test( int lifetime_arg, int expected_status_arg ) memset( &driver, 0, sizeof( driver ) ); driver.hal_version = PSA_DRV_SE_HAL_VERSION; + if( validate >= 0 ) + { + memset( &key_management, 0, sizeof( key_management ) ); + driver.key_management = &key_management; + key_management.p_validate_slot_number = validate_slot_number_as_directed; + validate_slot_number_directions.slot_number = wanted_slot; + validate_slot_number_directions.method = PSA_KEY_CREATION_REGISTER; + validate_slot_number_directions.status = + ( validate > 0 ? PSA_SUCCESS : PSA_ERROR_NOT_PERMITTED ); + } PSA_ASSERT( psa_register_se_driver( MIN_DRIVER_LIFETIME, &driver ) ); PSA_ASSERT( psa_crypto_init( ) ); @@ -770,5 +805,7 @@ exit: psa_destroy_key( handle ); PSA_DONE( ); psa_purge_storage( ); + memset( &validate_slot_number_directions, 0, + sizeof( validate_slot_number_directions ) ); } /* END_CASE */ From f3801fff773bcf2641bf5a3aec341c2af923b892 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 6 Aug 2019 17:32:04 +0200 Subject: [PATCH 20/34] Update import_key and generate_key SE methods to the current API The methods to import and generate a key in a secure element drivers were written for an earlier version of the application-side interface. Now that there is a psa_key_attributes_t structure that combines all key metadata including its lifetime (location), type, size, policy and extra type-specific data (domain parameters), pass that to drivers instead of separate arguments for each piece of metadata. This makes the interface less cluttered. Update parameter names and descriptions to follow general conventions. Document the public-key output on key generation more precisely. Explain that it is optional in a driver, and when a driver would implement it. Declare that it is optional in the core, too (which means that a crypto core might not support drivers for secure elements that do need this feature). Update the implementation and the tests accordingly. --- include/psa/crypto_se_driver.h | 110 ++++++++++-------- library/psa_crypto.c | 5 +- ...st_suite_psa_crypto_se_driver_hal.function | 29 ++--- 3 files changed, 72 insertions(+), 72 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index f04aa3468..a43e0db48 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -956,15 +956,21 @@ typedef psa_status_t (*psa_drv_se_validate_slot_number_t)( * documentation of psa_export_key() for the format for each key type. * * \param[in,out] drv_context The driver context structure. - * \param[in] key_slot Slot where the key will be stored + * \param key_slot Slot where the key will be stored. * This must be a valid slot for a key of the * chosen type. It must be unoccupied. - * \param[in] lifetime The required lifetime of the key storage - * \param[in] type Key type (a \c PSA_KEY_TYPE_XXX value) - * \param[in] algorithm Key algorithm (a \c PSA_ALG_XXX value) - * \param[in] usage The allowed uses of the key - * \param[in] p_data Buffer containing the key data - * \param[in] data_length Size of the `data` buffer in bytes + * \param[in] attributes The key attributes, including the lifetime, + * the key type and the usage policy. + * Drivers should not access the key size stored + * in the attributes: it may not match the + * data passed in \p data. + * Drivers can call psa_get_key_lifetime(), + * psa_get_key_type(), + * psa_get_key_usage_flags() and + * psa_get_key_algorithm() to access this + * information. + * \param[in] data Buffer containing the key data. + * \param[in] data_length Size of the \p data buffer in bytes. * \param[out] bits On success, the key size in bits. The driver * must determine this value after parsing the * key according to the key type. @@ -973,15 +979,13 @@ typedef psa_status_t (*psa_drv_se_validate_slot_number_t)( * \retval #PSA_SUCCESS * Success. */ -typedef psa_status_t (*psa_drv_se_import_key_t)(psa_drv_se_context_t *drv_context, - psa_key_slot_number_t key_slot, - psa_key_lifetime_t lifetime, - psa_key_type_t type, - psa_algorithm_t algorithm, - psa_key_usage_t usage, - const uint8_t *p_data, - size_t data_length, - size_t *bits); +typedef psa_status_t (*psa_drv_se_import_key_t)( + psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot, + const psa_key_attributes_t *attributes, + const uint8_t *data, + size_t data_length, + size_t *bits); /** * \brief A function that destroys a secure element key and restore the slot to @@ -1048,41 +1052,51 @@ typedef psa_status_t (*psa_drv_se_export_key_t)(psa_drv_se_context_t *drv_contex * element * * If \p type is asymmetric (#PSA_KEY_TYPE_IS_ASYMMETRIC(\p type) = 1), - * the public component of the generated key will be placed in `p_pubkey_out`. - * The format of the public key information will match the format specified for - * the psa_export_key() function for the key type. + * the driver may export the public key at the time of generation, + * in the format documented for psa_export_public_key() by writing it + * to the \p pubkey buffer. + * This is optional, intended for secure elements that output the + * public key at generation time and that cannot export the public key + * later. Drivers that do not need this feature should leave + * \p *pubkey_length set to 0 and should + * implement the psa_drv_key_management_t::p_export_public function. + * Some implementations do not support this feature, in which case + * \p pubkey is \c NULL and \p pubkey_size is 0. * * \param[in,out] drv_context The driver context structure. - * \param[in] key_slot Slot where the generated key will be placed - * \param[in] type The type of the key to be generated - * \param[in] usage The prescribed usage of the generated key - * Note: Not all Secure Elements support the same - * restrictions that PSA Crypto does (and vice - * versa). - * Driver developers should endeavor to match the - * usages as close as possible. - * \param[in] bits The size in bits of the key to be generated. - * \param[in] extra Extra parameters for key generation. The - * interpretation of this parameter should match - * the interpretation in the `extra` parameter is - * the `psa_generate_key` function - * \param[in] extra_size The size in bytes of the \p extra buffer - * \param[out] p_pubkey_out The buffer where the public key information will - * be placed - * \param[in] pubkey_out_size The size in bytes of the `p_pubkey_out` buffer - * \param[out] p_pubkey_length Upon successful completion, will contain the - * size of the data placed in `p_pubkey_out`. + * \param key_slot Slot where the key will be stored. + * This must be a valid slot for a key of the + * chosen type. It must be unoccupied. + * \param[in] attributes The key attributes, including the lifetime, + * the key type and size, and the usage policy. + * Drivers can call psa_get_key_lifetime(), + * psa_get_key_type(), psa_get_key_bits(), + * psa_get_key_usage_flags() and + * psa_get_key_algorithm() to access this + * information. + * \param[out] pubkey A buffer where the driver can write the + * public key, when generating an asymmetric + * key pair. + * This is \c NULL when generating a symmetric + * key or if the core does not support + * exporting the public key at generation time. + * \param pubkey_size The size of the `pubkey` buffer in bytes. + * This is 0 when generating a symmetric + * key or if the core does not support + * exporting the public key at generation time. + * \param[out] pubkey_length On entry, this is always 0. + * On success, the number of bytes written to + * \p pubkey. If this is 0 or unchanged on return, + * the core will not read the \p pubkey buffer, + * and will instead call the driver's + * psa_drv_key_management_t::p_export_public + * function to export the public key when needed. */ -typedef psa_status_t (*psa_drv_se_generate_key_t)(psa_drv_se_context_t *drv_context, - psa_key_slot_number_t key_slot, - psa_key_type_t type, - psa_key_usage_t usage, - size_t bits, - const void *extra, - size_t extra_size, - uint8_t *p_pubkey_out, - size_t pubkey_out_size, - size_t *p_pubkey_length); +typedef psa_status_t (*psa_drv_se_generate_key_t)( + psa_drv_se_context_t *drv_context, + psa_key_slot_number_t key_slot, + const psa_key_attributes_t *attributes, + uint8_t *pubkey, size_t pubkey_size, size_t *pubkey_length); /** * \brief A struct containing all of the function pointers needed to for secure diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 87ac037b6..f64487b85 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1827,10 +1827,7 @@ psa_status_t psa_import_key( const psa_key_attributes_t *attributes, } status = drv->key_management->p_import( psa_get_se_driver_context( driver ), - slot->data.se.slot_number, - slot->attr.lifetime, slot->attr.type, - slot->attr.policy.alg, slot->attr.policy.usage, - data, data_length, + slot->data.se.slot_number, attributes, data, data_length, &bits ); if( status != PSA_SUCCESS ) goto exit; diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 4673835d5..6c308512c 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -81,21 +81,15 @@ static psa_status_t counter_allocate( psa_drv_se_context_t *context, /* Null import: do nothing, but pretend it worked. */ static psa_status_t null_import( psa_drv_se_context_t *context, psa_key_slot_number_t slot_number, - psa_key_lifetime_t lifetime, - psa_key_type_t type, - psa_algorithm_t algorithm, - psa_key_usage_t usage, - const uint8_t *p_data, + const psa_key_attributes_t *attributes, + const uint8_t *data, size_t data_length, size_t *bits ) { (void) context; (void) slot_number; - (void) lifetime; - (void) type; - (void) algorithm; - (void) usage; - (void) p_data; + (void) attributes; + (void) data; /* We're supposed to return a key size. Return one that's correct for * plain data keys. */ *bits = PSA_BYTES_TO_BITS( data_length ); @@ -132,11 +126,8 @@ static void ram_slots_reset( void ) static psa_status_t ram_import( psa_drv_se_context_t *context, psa_key_slot_number_t slot_number, - psa_key_lifetime_t lifetime, - psa_key_type_t type, - psa_algorithm_t algorithm, - psa_key_usage_t usage, - const uint8_t *p_data, + const psa_key_attributes_t *attributes, + const uint8_t *data, size_t data_length, size_t *bits ) { @@ -144,13 +135,11 @@ static psa_status_t ram_import( psa_drv_se_context_t *context, DRIVER_ASSERT( slot_number < ARRAY_LENGTH( ram_slots ) ); if( data_length > sizeof( ram_slots[slot_number].content ) ) return( PSA_ERROR_INSUFFICIENT_STORAGE ); - ram_slots[slot_number].lifetime = lifetime; - ram_slots[slot_number].type = type; + ram_slots[slot_number].lifetime = psa_get_key_lifetime( attributes ); + ram_slots[slot_number].type = psa_get_key_type( attributes ); ram_slots[slot_number].bits = PSA_BYTES_TO_BITS( data_length ); *bits = PSA_BYTES_TO_BITS( data_length ); - (void) algorithm; - (void) usage; - memcpy( ram_slots[slot_number].content, p_data, data_length ); + memcpy( ram_slots[slot_number].content, data, data_length ); return( PSA_SUCCESS ); } From 11792086cc475a6a362752de82447f38f936b638 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 6 Aug 2019 18:36:36 +0200 Subject: [PATCH 21/34] SE keys: implement and smoke-test p_generate --- library/psa_crypto.c | 22 ++++- .../test_suite_psa_crypto_se_driver_hal.data | 81 ++++++++++-------- ...st_suite_psa_crypto_se_driver_hal.function | 85 ++++++++++++++++++- 3 files changed, 147 insertions(+), 41 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index f64487b85..35c03dde3 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5944,21 +5944,37 @@ psa_status_t psa_generate_key( const psa_key_attributes_t *attributes, psa_status_t status; psa_key_slot_t *slot = NULL; psa_se_drv_table_entry_t *driver = NULL; + status = psa_start_key_creation( PSA_KEY_CREATION_GENERATE, attributes, handle, &slot, &driver ); + if( status != PSA_SUCCESS ) + goto exit; + #if defined(MBEDTLS_PSA_CRYPTO_SE_C) if( driver != NULL ) { - /* Generating a key in a secure element is not implemented yet. */ - status = PSA_ERROR_NOT_SUPPORTED; + const psa_drv_se_t *drv = psa_get_se_driver_methods( driver ); + size_t pubkey_length = 0; /* We don't support this feature yet */ + if( drv->key_management == NULL || + drv->key_management->p_generate == NULL ) + { + status = PSA_ERROR_NOT_SUPPORTED; + goto exit; + } + status = drv->key_management->p_generate( + psa_get_se_driver_context( driver ), + slot->data.se.slot_number, attributes, + NULL, 0, &pubkey_length ); } + else #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ - if( status == PSA_SUCCESS ) { status = psa_generate_key_internal( slot, attributes->core.bits, attributes->domain_parameters, attributes->domain_parameters_size ); } + +exit: if( status == PSA_SUCCESS ) status = psa_finish_key_creation( slot, driver ); if( status != PSA_SUCCESS ) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data index 267c7b88b..0bec8419c 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -54,63 +54,72 @@ key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ) - 1:1:PSA_SUCCESS Key creation in a specific slot (too large) key_creation_in_chosen_slot:ARRAY_LENGTH( ram_slots ):0:PSA_ERROR_INVALID_ARGUMENT -Key creation smoke test: AES-CTR -key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CTR:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: AES-CTR +import_key_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CTR:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: AES-CBC -key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CBC_NO_PADDING:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: AES-CBC +import_key_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CBC_NO_PADDING:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: AES-CMAC -key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: AES-CMAC +import_key_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: AES-CCM -key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: AES-CCM +import_key_smoke:PSA_KEY_TYPE_AES:PSA_ALG_CCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: AES-GCM -key_creation_smoke:PSA_KEY_TYPE_AES:PSA_ALG_GCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: AES-GCM +import_key_smoke:PSA_KEY_TYPE_AES:PSA_ALG_GCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: CAMELLIA-CTR -key_creation_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_CTR:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: CAMELLIA-CTR +import_key_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_CTR:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: CAMELLIA-CBC -key_creation_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_CBC_NO_PADDING:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: CAMELLIA-CBC +import_key_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_CBC_NO_PADDING:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: CAMELLIA-CMAC -key_creation_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_CMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: CAMELLIA-CMAC +import_key_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_CMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: CAMELLIA-CCM -key_creation_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_GCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: CAMELLIA-CCM +import_key_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_GCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: CAMELLIA-CCM -key_creation_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_GCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: CAMELLIA-CCM +import_key_smoke:PSA_KEY_TYPE_CAMELLIA:PSA_ALG_GCM:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: HMAC-SHA-256 -key_creation_smoke:PSA_KEY_TYPE_HMAC:PSA_ALG_HMAC( PSA_ALG_SHA_256 ):"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: HMAC-SHA-256 +import_key_smoke:PSA_KEY_TYPE_HMAC:PSA_ALG_HMAC( PSA_ALG_SHA_256 ):"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: HKDF-SHA-256 -key_creation_smoke:PSA_KEY_TYPE_DERIVE:PSA_ALG_HKDF( PSA_ALG_SHA_256 ):"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +Key import smoke test: HKDF-SHA-256 +import_key_smoke:PSA_KEY_TYPE_DERIVE:PSA_ALG_HKDF( PSA_ALG_SHA_256 ):"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" -Key creation smoke test: RSA PKCS#1v1.5 signature -key_creation_smoke:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_ALG_RSA_PKCS1V15_SIGN_RAW:"30818902818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc30203010001" +Key import smoke test: RSA PKCS#1v1.5 signature +import_key_smoke:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_ALG_RSA_PKCS1V15_SIGN_RAW:"30818902818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc30203010001" -Key creation smoke test: RSA PKCS#1v1.5 encryption -key_creation_smoke:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_ALG_RSA_PKCS1V15_CRYPT:"30818902818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc30203010001" +Key import smoke test: RSA PKCS#1v1.5 encryption +import_key_smoke:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_ALG_RSA_PKCS1V15_CRYPT:"30818902818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc30203010001" -Key creation smoke test: RSA OAEP encryption -key_creation_smoke:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_ALG_RSA_OAEP( PSA_ALG_SHA_256 ):"30818902818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc30203010001" +Key import smoke test: RSA OAEP encryption +import_key_smoke:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_ALG_RSA_OAEP( PSA_ALG_SHA_256 ):"30818902818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc30203010001" -Key creation smoke test: ECDSA secp256r1 -key_creation_smoke:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee" +Key import smoke test: ECDSA secp256r1 +import_key_smoke:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee" -Key creation smoke test: ECDH secp256r1 -key_creation_smoke:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDH:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee" +Key import smoke test: ECDH secp256r1 +import_key_smoke:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDH:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee" -Key creation smoke test: ECDH secp256r1 with HKDF -key_creation_smoke:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_KEY_AGREEMENT( PSA_ALG_ECDH, PSA_ALG_HKDF( PSA_ALG_SHA_256 ) ):"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee" +Key import smoke test: ECDH secp256r1 with HKDF +import_key_smoke:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_KEY_AGREEMENT( PSA_ALG_ECDH, PSA_ALG_HKDF( PSA_ALG_SHA_256 ) ):"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee" Generate key: not supported generate_key_not_supported:PSA_KEY_TYPE_AES:128 +Key generation smoke test: AES-128-CTR +generate_key_smoke:PSA_KEY_TYPE_AES:128:PSA_ALG_CTR + +Key generation smoke test: AES-256-CTR +generate_key_smoke:PSA_KEY_TYPE_AES:128:PSA_ALG_CTR + +Key generation smoke test: HMAC-SHA-256 +generate_key_smoke:PSA_KEY_TYPE_HMAC:256:PSA_ALG_HMAC( PSA_ALG_SHA_256 ) + Key registration: smoke test register_key_smoke_test:MIN_DRIVER_LIFETIME:-1:PSA_SUCCESS diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 6c308512c..d13e2f248 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -96,6 +96,28 @@ static psa_status_t null_import( psa_drv_se_context_t *context, return( PSA_SUCCESS ); } +/* Null generate: do nothing, but pretend it worked. */ +static psa_status_t null_generate( psa_drv_se_context_t *context, + psa_key_slot_number_t slot_number, + const psa_key_attributes_t *attributes, + uint8_t *pubkey, + size_t pubkey_size, + size_t *pubkey_length ) +{ + (void) context; + (void) slot_number; + (void) attributes; + + DRIVER_ASSERT( *pubkey_length == 0 ); + if( ! PSA_KEY_TYPE_IS_KEY_PAIR( psa_get_key_type( attributes ) ) ) + { + DRIVER_ASSERT( pubkey == NULL ); + DRIVER_ASSERT( pubkey_size == 0 ); + } + + return( PSA_SUCCESS ); +} + /****************************************************************/ @@ -634,8 +656,8 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void key_creation_smoke( int type_arg, int alg_arg, - data_t *key_material ) +void import_key_smoke( int type_arg, int alg_arg, + data_t *key_material ) { psa_key_type_t type = type_arg; psa_algorithm_t alg = alg_arg; @@ -710,6 +732,7 @@ void generate_key_not_supported( int type_arg, int bits_arg ) driver.key_management = &key_management; driver.persistent_data_size = sizeof( psa_key_slot_number_t ); key_management.p_allocate = counter_allocate; + /* No p_generate method */ PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); PSA_ASSERT( psa_crypto_init( ) ); @@ -728,6 +751,64 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void generate_key_smoke( int type_arg, int bits_arg, int alg_arg ) +{ + psa_key_type_t type = type_arg; + psa_key_bits_t bits = bits_arg; + psa_algorithm_t alg = alg_arg; + psa_drv_se_t driver; + psa_drv_se_key_management_t key_management; + psa_key_lifetime_t lifetime = 2; + psa_key_id_t id = 1; + psa_key_handle_t handle = 0; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + + memset( &driver, 0, sizeof( driver ) ); + memset( &key_management, 0, sizeof( key_management ) ); + driver.hal_version = PSA_DRV_SE_HAL_VERSION; + driver.key_management = &key_management; + driver.persistent_data_size = sizeof( psa_key_slot_number_t ); + key_management.p_allocate = counter_allocate; + key_management.p_generate = null_generate; + + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + PSA_ASSERT( psa_crypto_init( ) ); + + /* Create a key. */ + psa_set_key_id( &attributes, id ); + psa_set_key_lifetime( &attributes, lifetime ); + psa_set_key_usage_flags( &attributes, + PSA_KEY_USAGE_SIGN | PSA_KEY_USAGE_VERIFY | + PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT | + PSA_KEY_USAGE_EXPORT ); + psa_set_key_algorithm( &attributes, alg ); + psa_set_key_type( &attributes, type ); + psa_set_key_bits( &attributes, bits ); + PSA_ASSERT( psa_generate_key( &attributes, &handle ) ); + + /* Do stuff with the key. */ + if( ! smoke_test_key( handle ) ) + goto exit; + + /* Restart and try again. */ + mbedtls_psa_crypto_free( ); + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + PSA_ASSERT( psa_crypto_init( ) ); + PSA_ASSERT( psa_open_key( id, &handle ) ); + if( ! smoke_test_key( handle ) ) + goto exit; + + /* We're done. */ + PSA_ASSERT( psa_destroy_key( handle ) ); + +exit: + PSA_DONE( ); + ram_slots_reset( ); + psa_purge_storage( ); +} +/* END_CASE */ + /* BEGIN_CASE */ void register_key_smoke_test( int lifetime_arg, int validate, From edc6424d7760f30879638eb4ecc9f8c798c1ee01 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 7 Aug 2019 21:05:07 +0200 Subject: [PATCH 22/34] SE driver support: Implement sign and verify hooks --- library/psa_crypto.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 35c03dde3..3a78f5653 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3331,10 +3331,14 @@ psa_status_t psa_asymmetric_sign( psa_key_handle_t handle, { psa_key_slot_t *slot; psa_status_t status; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + const psa_drv_se_t *drv; + psa_drv_se_context_t *drv_context; +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ *signature_length = signature_size; - status = psa_get_transparent_key( handle, &slot, PSA_KEY_USAGE_SIGN, alg ); + status = psa_get_key_from_slot( handle, &slot, PSA_KEY_USAGE_SIGN, alg ); if( status != PSA_SUCCESS ) goto exit; if( ! PSA_KEY_TYPE_IS_KEY_PAIR( slot->attr.type ) ) @@ -3343,6 +3347,24 @@ psa_status_t psa_asymmetric_sign( psa_key_handle_t handle, goto exit; } +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( psa_get_se_driver( slot->attr.lifetime, &drv, &drv_context ) ) + { + if( drv->asymmetric == NULL || + drv->asymmetric->p_sign == NULL ) + { + status = PSA_ERROR_NOT_SUPPORTED; + goto exit; + } + status = drv->asymmetric->p_sign( drv_context, + slot->data.se.slot_number, + alg, + hash, hash_length, + signature, signature_size, + signature_length ); + } + else +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ #if defined(MBEDTLS_RSA_C) if( slot->attr.type == PSA_KEY_TYPE_RSA_KEY_PAIR ) { @@ -3406,11 +3428,29 @@ psa_status_t psa_asymmetric_verify( psa_key_handle_t handle, { psa_key_slot_t *slot; psa_status_t status; +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + const psa_drv_se_t *drv; + psa_drv_se_context_t *drv_context; +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ - status = psa_get_transparent_key( handle, &slot, PSA_KEY_USAGE_VERIFY, alg ); + status = psa_get_key_from_slot( handle, &slot, PSA_KEY_USAGE_VERIFY, alg ); if( status != PSA_SUCCESS ) return( status ); +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( psa_get_se_driver( slot->attr.lifetime, &drv, &drv_context ) ) + { + if( drv->asymmetric == NULL || + drv->asymmetric->p_verify == NULL ) + return( PSA_ERROR_NOT_SUPPORTED ); + return( drv->asymmetric->p_verify( drv_context, + slot->data.se.slot_number, + alg, + hash, hash_length, + signature, signature_length ) ); + } + else +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ #if defined(MBEDTLS_RSA_C) if( PSA_KEY_TYPE_IS_RSA( slot->attr.type ) ) { From eecadde6adb6b1d1c5662a449081823ab85a1047 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 7 Aug 2019 21:05:24 +0200 Subject: [PATCH 23/34] SE support: Test sign and verify hooks with a passthrough driver --- .../test_suite_psa_crypto_se_driver_hal.data | 16 + ...st_suite_psa_crypto_se_driver_hal.function | 296 +++++++++++++++++- 2 files changed, 298 insertions(+), 14 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data index 0bec8419c..bdd5703b0 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -137,3 +137,19 @@ register_key_smoke_test:MIN_DRIVER_LIFETIME:1:PSA_SUCCESS Key registration: with driver validation (rejected) register_key_smoke_test:MIN_DRIVER_LIFETIME:0:PSA_ERROR_NOT_PERMITTED + +Import-sign-verify: sign in driver, ECDSA +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +sign_verify:1:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:0:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" + +Import-sign-verify: sign in software, ECDSA +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +sign_verify:0:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:0:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" + +Generate-sign-verify: sign in driver, ECDSA +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +sign_verify:1:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:256:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" + +Generate-sign-verify: sign in software, ECDSA +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +sign_verify:0:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:256:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index d13e2f248..e14fa5838 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -18,7 +18,25 @@ * This is probably a bug in the library. */ #define PSA_ERROR_DETECTED_BY_DRIVER ((psa_status_t)( -500 )) -/** Like #TEST_ASSERT for use in a driver method. +/** Like #TEST_ASSERT for use in a driver method, with no cleanup. + * + * If an error happens, this macro returns from the calling function. + * + * Use this macro to assert on guarantees provided by the core. + */ +#define DRIVER_ASSERT_RETURN( TEST ) \ + do { \ + if( ! (TEST) ) \ + { \ + test_fail( #TEST, __LINE__, __FILE__ ); \ + return( PSA_ERROR_DETECTED_BY_DRIVER ); \ + } \ + } while( 0 ) + +/** Like #TEST_ASSERT for use in a driver method, with cleanup. + * + * In case of error, this macro sets `status` and jumps to the + * label `exit`. * * Use this macro to assert on guarantees provided by the core. */ @@ -27,10 +45,34 @@ if( ! (TEST) ) \ { \ test_fail( #TEST, __LINE__, __FILE__ ); \ - return( PSA_ERROR_DETECTED_BY_DRIVER ); \ + status = PSA_ERROR_DETECTED_BY_DRIVER; \ + goto exit; \ } \ } while( 0 ) +/** Like #PSA_ASSERT for a PSA API call that calls a driver underneath. + * + * Run the code \p expr. If this returns \p expected_status, + * do nothing. If this returns #PSA_ERROR_DETECTED_BY_DRIVER, + * jump directly to the `exit` label. If this returns any other + * status, call test_fail() then jump to `exit`. + * + * The special case for #PSA_ERROR_DETECTED_BY_DRIVER is because in this + * case, the test driver code is expected to have called test_fail() + * already, so we make sure not to overwrite the failure information. + */ +#define PSA_ASSERT_VIA_DRIVER( expr, expected_status ) \ + do { \ + psa_status_t PSA_ASSERT_VIA_DRIVER_status = ( expr ); \ + if( PSA_ASSERT_VIA_DRIVER_status == PSA_ERROR_DETECTED_BY_DRIVER ) \ + goto exit; \ + if( PSA_ASSERT_VIA_DRIVER_status != ( expected_status ) ) \ + { \ + test_fail( #expr, __LINE__, __FILE__ ); \ + goto exit; \ + } \ + } while( 0 ) + /****************************************************************/ @@ -54,8 +96,10 @@ static psa_status_t validate_slot_number_as_directed( { (void) context; (void) attributes; - DRIVER_ASSERT( slot_number == validate_slot_number_directions.slot_number ); - DRIVER_ASSERT( method == validate_slot_number_directions.method ); + DRIVER_ASSERT_RETURN( slot_number == + validate_slot_number_directions.slot_number ); + DRIVER_ASSERT_RETURN( method == + validate_slot_number_directions.method ); return( validate_slot_number_directions.status ); } @@ -108,11 +152,11 @@ static psa_status_t null_generate( psa_drv_se_context_t *context, (void) slot_number; (void) attributes; - DRIVER_ASSERT( *pubkey_length == 0 ); + DRIVER_ASSERT_RETURN( *pubkey_length == 0 ); if( ! PSA_KEY_TYPE_IS_KEY_PAIR( psa_get_key_type( attributes ) ) ) { - DRIVER_ASSERT( pubkey == NULL ); - DRIVER_ASSERT( pubkey_size == 0 ); + DRIVER_ASSERT_RETURN( pubkey == NULL ); + DRIVER_ASSERT_RETURN( pubkey_size == 0 ); } return( PSA_SUCCESS ); @@ -146,6 +190,42 @@ static void ram_slots_reset( void ) ram_min_slot = 0; } +/* This function does everything except actually generating key material. + * After calling it, you must copy the desired key material to + * ram_slots[slot_number].content. */ +static psa_status_t ram_fake_generate( psa_drv_se_context_t *context, + psa_key_slot_number_t slot_number, + const psa_key_attributes_t *attributes, + uint8_t *pubkey, + size_t pubkey_size, + size_t *pubkey_length ) +{ + (void) context; + + DRIVER_ASSERT_RETURN( *pubkey_length == 0 ); + if( ! PSA_KEY_TYPE_IS_KEY_PAIR( psa_get_key_type( attributes ) ) ) + { + DRIVER_ASSERT_RETURN( pubkey == NULL ); + DRIVER_ASSERT_RETURN( pubkey_size == 0 ); + } + + { + /* Check that the key can be stored in the memory slot. + * This check only works for key in a "raw" representation: + * symmetric keys or ECC are ok, but not RSA or FFDH. */ + size_t required_storage = + PSA_BITS_TO_BYTES( psa_get_key_bits( attributes ) ); + size_t available_storage = sizeof( ram_slots[slot_number].content ); + if( required_storage > available_storage ) + return( PSA_ERROR_INSUFFICIENT_STORAGE ); + } + + ram_slots[slot_number].lifetime = psa_get_key_lifetime( attributes ); + ram_slots[slot_number].type = psa_get_key_type( attributes ); + ram_slots[slot_number].bits = psa_get_key_bits( attributes ); + return( PSA_SUCCESS ); +} + static psa_status_t ram_import( psa_drv_se_context_t *context, psa_key_slot_number_t slot_number, const psa_key_attributes_t *attributes, @@ -154,7 +234,7 @@ static psa_status_t ram_import( psa_drv_se_context_t *context, size_t *bits ) { (void) context; - DRIVER_ASSERT( slot_number < ARRAY_LENGTH( ram_slots ) ); + DRIVER_ASSERT_RETURN( slot_number < ARRAY_LENGTH( ram_slots ) ); if( data_length > sizeof( ram_slots[slot_number].content ) ) return( PSA_ERROR_INSUFFICIENT_STORAGE ); ram_slots[slot_number].lifetime = psa_get_key_lifetime( attributes ); @@ -173,7 +253,7 @@ static psa_status_t ram_export( psa_drv_se_context_t *context, { size_t actual_size; (void) context; - DRIVER_ASSERT( slot_number < ARRAY_LENGTH( ram_slots ) ); + DRIVER_ASSERT_RETURN( slot_number < ARRAY_LENGTH( ram_slots ) ); actual_size = PSA_BITS_TO_BYTES( ram_slots[slot_number].bits ); if( actual_size > data_size ) return( PSA_ERROR_BUFFER_TOO_SMALL ); @@ -187,8 +267,8 @@ static psa_status_t ram_destroy( psa_drv_se_context_t *context, psa_key_slot_number_t slot_number ) { ram_slot_usage_t *slot_usage = persistent_data; - DRIVER_ASSERT( context->persistent_data_size == sizeof( ram_slot_usage_t ) ); - DRIVER_ASSERT( slot_number < ARRAY_LENGTH( ram_slots ) ); + DRIVER_ASSERT_RETURN( context->persistent_data_size == sizeof( ram_slot_usage_t ) ); + DRIVER_ASSERT_RETURN( slot_number < ARRAY_LENGTH( ram_slots ) ); memset( &ram_slots[slot_number], 0, sizeof( ram_slots[slot_number] ) ); *slot_usage &= ~(ram_slot_usage_t)( 1 << slot_number ); return( PSA_SUCCESS ); @@ -203,7 +283,7 @@ static psa_status_t ram_allocate( psa_drv_se_context_t *context, ram_slot_usage_t *slot_usage = persistent_data; (void) attributes; (void) method; - DRIVER_ASSERT( context->persistent_data_size == sizeof( ram_slot_usage_t ) ); + DRIVER_ASSERT_RETURN( context->persistent_data_size == sizeof( ram_slot_usage_t ) ); for( *slot_number = ram_min_slot; *slot_number < ARRAY_LENGTH( ram_slots ); ++( *slot_number ) ) @@ -228,6 +308,76 @@ static psa_status_t ram_validate_slot_number( return( PSA_SUCCESS ); } +static psa_status_t ram_sign( psa_drv_se_context_t *context, + psa_key_slot_number_t slot_number, + psa_algorithm_t alg, + const uint8_t *hash, + size_t hash_length, + uint8_t *signature, + size_t signature_size, + size_t *signature_length ) +{ + ram_slot_t *slot; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_key_handle_t handle = 0; + psa_status_t status = PSA_ERROR_GENERIC_ERROR; + + (void) context; + DRIVER_ASSERT_RETURN( slot_number < ARRAY_LENGTH( ram_slots ) ); + slot = &ram_slots[slot_number]; + + psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_SIGN ); + psa_set_key_algorithm( &attributes, alg ); + psa_set_key_type( &attributes, slot->type ); + DRIVER_ASSERT( psa_import_key( &attributes, + slot->content, + PSA_BITS_TO_BYTES( slot->bits ), + &handle ) == PSA_SUCCESS ); + status = psa_asymmetric_sign( handle, alg, + hash, hash_length, + signature, signature_size, + signature_length ); + +exit: + psa_destroy_key( handle ); + return( status ); +} + +static psa_status_t ram_verify( psa_drv_se_context_t *context, + psa_key_slot_number_t slot_number, + psa_algorithm_t alg, + const uint8_t *hash, + size_t hash_length, + const uint8_t *signature, + size_t signature_length ) +{ + ram_slot_t *slot; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_key_handle_t handle = 0; + psa_status_t status = PSA_ERROR_GENERIC_ERROR; + + (void) context; + DRIVER_ASSERT_RETURN( slot_number < ARRAY_LENGTH( ram_slots ) ); + slot = &ram_slots[slot_number]; + + psa_set_key_usage_flags( &attributes, PSA_KEY_USAGE_VERIFY ); + psa_set_key_algorithm( &attributes, alg ); + psa_set_key_type( &attributes, slot->type ); + DRIVER_ASSERT( psa_import_key( &attributes, + slot->content, + PSA_BITS_TO_BYTES( slot->bits ), + &handle ) == + PSA_SUCCESS ); + status = psa_asymmetric_verify( handle, alg, + hash, hash_length, + signature, signature_length ); + +exit: + psa_destroy_key( handle ); + return( status ); +} + + /****************************************************************/ @@ -709,7 +859,6 @@ void import_key_smoke( int type_arg, int alg_arg, exit: PSA_DONE( ); - ram_slots_reset( ); psa_purge_storage( ); } /* END_CASE */ @@ -746,7 +895,6 @@ void generate_key_not_supported( int type_arg, int bits_arg ) exit: PSA_DONE( ); - ram_slots_reset( ); psa_purge_storage( ); } /* END_CASE */ @@ -803,6 +951,126 @@ void generate_key_smoke( int type_arg, int bits_arg, int alg_arg ) PSA_ASSERT( psa_destroy_key( handle ) ); exit: + PSA_DONE( ); + psa_purge_storage( ); +} +/* END_CASE */ + +/* BEGIN_CASE */ +void sign_verify( int sign_in_driver, + int type_arg, int alg_arg, + int bits_arg, data_t *key_material, + data_t *input ) +{ + psa_key_type_t type = type_arg; + psa_algorithm_t alg = alg_arg; + size_t bits = bits_arg; + /* Pass bits=0 to import, bits>0 to fake-generate */ + int generating = ( bits != 0 ); + + psa_drv_se_t driver; + psa_drv_se_key_management_t key_management; + psa_drv_se_asymmetric_t asymmetric; + + psa_key_lifetime_t lifetime = 2; + psa_key_id_t id = 1; + psa_key_handle_t drv_handle = 0; /* key managed by the driver */ + psa_key_handle_t sw_handle = 0; /* transparent key */ + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + uint8_t signature[PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE]; + size_t signature_length; + + memset( &driver, 0, sizeof( driver ) ); + memset( &key_management, 0, sizeof( key_management ) ); + driver.hal_version = PSA_DRV_SE_HAL_VERSION; + driver.key_management = &key_management; + driver.asymmetric = &asymmetric; + driver.persistent_data_size = sizeof( psa_key_slot_number_t ); + driver.persistent_data_size = sizeof( ram_slot_usage_t ); + key_management.p_allocate = ram_allocate; + key_management.p_destroy = ram_destroy; + if( generating ) + key_management.p_generate = ram_fake_generate; + else + key_management.p_import = ram_import; + if( sign_in_driver ) + asymmetric.p_sign = ram_sign; + asymmetric.p_verify = ram_verify; + + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + PSA_ASSERT( psa_crypto_init( ) ); + + /* Create two keys with the same key material: a transparent key, + * and one that goes through the driver. */ + psa_set_key_usage_flags( &attributes, + PSA_KEY_USAGE_SIGN | PSA_KEY_USAGE_VERIFY ); + psa_set_key_algorithm( &attributes, alg ); + psa_set_key_type( &attributes, type ); + PSA_ASSERT( psa_import_key( &attributes, + key_material->x, key_material->len, + &sw_handle ) ); + psa_set_key_id( &attributes, id ); + psa_set_key_lifetime( &attributes, lifetime ); + if( generating ) + { + psa_set_key_bits( &attributes, bits ); + PSA_ASSERT( psa_generate_key( &attributes, &drv_handle ) ); + /* Since we called a generate method that does not actually + * generate material, store the desired result of generation in + * the mock secure element storage. */ + PSA_ASSERT( psa_get_key_attributes( drv_handle, &attributes ) ); + TEST_ASSERT( key_material->len == PSA_BITS_TO_BYTES( bits ) ); + memcpy( ram_slots[ram_min_slot].content, key_material->x, + key_material->len ); + } + else + { + PSA_ASSERT( psa_import_key( &attributes, + key_material->x, key_material->len, + &drv_handle ) ); + } + + /* Sign with the chosen key. */ + if( sign_in_driver ) + PSA_ASSERT_VIA_DRIVER( + psa_asymmetric_sign( drv_handle, + alg, + input->x, input->len, + signature, sizeof( signature ), + &signature_length ), + PSA_SUCCESS ); + else + PSA_ASSERT( psa_asymmetric_sign( sw_handle, + alg, + input->x, input->len, + signature, sizeof( signature ), + &signature_length ) ); + + /* Verify with both keys. */ + PSA_ASSERT( psa_asymmetric_verify( sw_handle, alg, + input->x, input->len, + signature, signature_length ) ); + PSA_ASSERT_VIA_DRIVER( + psa_asymmetric_verify( drv_handle, alg, + input->x, input->len, + signature, signature_length ), + PSA_SUCCESS ); + + /* Change the signature and verify again. */ + signature[0] ^= 1; + TEST_EQUAL( psa_asymmetric_verify( sw_handle, alg, + input->x, input->len, + signature, signature_length ), + PSA_ERROR_INVALID_SIGNATURE ); + PSA_ASSERT_VIA_DRIVER( + psa_asymmetric_verify( drv_handle, alg, + input->x, input->len, + signature, signature_length ), + PSA_ERROR_INVALID_SIGNATURE ); + +exit: + psa_destroy_key( drv_handle ); + psa_destroy_key( sw_handle ); PSA_DONE( ); ram_slots_reset( ); psa_purge_storage( ); From c068ded0155e76ed951c5eb57c07a69aa13d7470 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 9 Aug 2019 12:49:06 +0200 Subject: [PATCH 24/34] RAM test driver: improve key creation Factor common code of ram_import and ram_fake_generate into a common auxiliary function. Reject key types that aren't supported by this test code. Report the bit size correctly for EC key pairs. --- ...st_suite_psa_crypto_se_driver_hal.function | 85 +++++++++++++------ 1 file changed, 59 insertions(+), 26 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index e14fa5838..4cba693c2 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -190,6 +190,35 @@ static void ram_slots_reset( void ) ram_min_slot = 0; } +/* Common parts of key creation. + * + * In case of error, zero out ram_slots[slot_number]. But don't + * do that if the error is PSA_ERROR_DETECTED_BY_DRIVER: in this case + * you don't need to clean up (ram_slot_reset() will take care of it + * in the test case function's cleanup code) and it might be wrong + * (if slot_number is invalid). + */ +static psa_status_t ram_create_common( psa_drv_se_context_t *context, + psa_key_slot_number_t slot_number, + const psa_key_attributes_t *attributes, + size_t required_storage ) +{ + (void) context; + DRIVER_ASSERT_RETURN( slot_number < ARRAY_LENGTH( ram_slots ) ); + + ram_slots[slot_number].lifetime = psa_get_key_lifetime( attributes ); + ram_slots[slot_number].type = psa_get_key_type( attributes ); + ram_slots[slot_number].bits = psa_get_key_bits( attributes ); + + if( required_storage > sizeof( ram_slots[slot_number].content ) ) + { + memset( &ram_slots[slot_number], 0, sizeof( ram_slots[slot_number] ) ); + return( PSA_ERROR_INSUFFICIENT_STORAGE ); + } + + return( PSA_SUCCESS ); +} + /* This function does everything except actually generating key material. * After calling it, you must copy the desired key material to * ram_slots[slot_number].content. */ @@ -200,7 +229,10 @@ static psa_status_t ram_fake_generate( psa_drv_se_context_t *context, size_t pubkey_size, size_t *pubkey_length ) { - (void) context; + psa_status_t status; + size_t required_storage = + PSA_KEY_EXPORT_MAX_SIZE( psa_get_key_type( attributes ), + psa_get_key_bits( attributes ) ); DRIVER_ASSERT_RETURN( *pubkey_length == 0 ); if( ! PSA_KEY_TYPE_IS_KEY_PAIR( psa_get_key_type( attributes ) ) ) @@ -209,21 +241,9 @@ static psa_status_t ram_fake_generate( psa_drv_se_context_t *context, DRIVER_ASSERT_RETURN( pubkey_size == 0 ); } - { - /* Check that the key can be stored in the memory slot. - * This check only works for key in a "raw" representation: - * symmetric keys or ECC are ok, but not RSA or FFDH. */ - size_t required_storage = - PSA_BITS_TO_BYTES( psa_get_key_bits( attributes ) ); - size_t available_storage = sizeof( ram_slots[slot_number].content ); - if( required_storage > available_storage ) - return( PSA_ERROR_INSUFFICIENT_STORAGE ); - } - - ram_slots[slot_number].lifetime = psa_get_key_lifetime( attributes ); - ram_slots[slot_number].type = psa_get_key_type( attributes ); - ram_slots[slot_number].bits = psa_get_key_bits( attributes ); - return( PSA_SUCCESS ); + status = ram_create_common( context, slot_number, attributes, + required_storage ); + return( status ); } static psa_status_t ram_import( psa_drv_se_context_t *context, @@ -233,23 +253,36 @@ static psa_status_t ram_import( psa_drv_se_context_t *context, size_t data_length, size_t *bits ) { - (void) context; - DRIVER_ASSERT_RETURN( slot_number < ARRAY_LENGTH( ram_slots ) ); - if( data_length > sizeof( ram_slots[slot_number].content ) ) - return( PSA_ERROR_INSUFFICIENT_STORAGE ); - ram_slots[slot_number].lifetime = psa_get_key_lifetime( attributes ); - ram_slots[slot_number].type = psa_get_key_type( attributes ); - ram_slots[slot_number].bits = PSA_BYTES_TO_BITS( data_length ); - *bits = PSA_BYTES_TO_BITS( data_length ); + psa_key_type_t type = psa_get_key_type( attributes ); + psa_status_t status = ram_create_common( context, slot_number, attributes, + data_length ); + if( status != PSA_SUCCESS ) + return( status ); + + /* The RAM driver only works for certain key types: raw keys, + * and ECC key pairs. This is true in particular of the bit-size + * calculation here. */ + if( PSA_KEY_TYPE_IS_UNSTRUCTURED( type ) ) + *bits = PSA_BYTES_TO_BITS( data_length ); + else if ( PSA_KEY_TYPE_IS_ECC_KEY_PAIR( type ) ) + *bits = PSA_ECC_CURVE_BITS( PSA_KEY_TYPE_GET_CURVE( type ) ); + else + { + memset( &ram_slots[slot_number], 0, sizeof( ram_slots[slot_number] ) ); + return( PSA_ERROR_NOT_SUPPORTED ); + } + + ram_slots[slot_number].bits = *bits; memcpy( ram_slots[slot_number].content, data, data_length ); + return( PSA_SUCCESS ); } static psa_status_t ram_export( psa_drv_se_context_t *context, psa_key_slot_number_t slot_number, - uint8_t *p_data, + uint8_t *data, size_t data_size, - size_t *p_data_length ) + size_t *data_length ) { size_t actual_size; (void) context; From af906f852cf20f24cd49a90ae303bee71a098536 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 9 Aug 2019 12:50:18 +0200 Subject: [PATCH 25/34] RAM test driver: implement export_public --- ...st_suite_psa_crypto_se_driver_hal.function | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 4cba693c2..4757f6c0c 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -290,8 +290,35 @@ static psa_status_t ram_export( psa_drv_se_context_t *context, actual_size = PSA_BITS_TO_BYTES( ram_slots[slot_number].bits ); if( actual_size > data_size ) return( PSA_ERROR_BUFFER_TOO_SMALL ); - *p_data_length = actual_size; - memcpy( p_data, ram_slots[slot_number].content, actual_size ); + *data_length = actual_size; + memcpy( data, ram_slots[slot_number].content, actual_size ); + return( PSA_SUCCESS ); +} + +static psa_status_t ram_export_public( psa_drv_se_context_t *context, + psa_key_slot_number_t slot_number, + uint8_t *data, + size_t data_size, + size_t *data_length ) +{ + psa_status_t status; + psa_key_handle_t handle; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + + (void) context; + DRIVER_ASSERT_RETURN( slot_number < ARRAY_LENGTH( ram_slots ) ); + DRIVER_ASSERT_RETURN( + PSA_KEY_TYPE_IS_KEY_PAIR( ram_slots[slot_number].type ) ); + + psa_set_key_type( &attributes, ram_slots[slot_number].type ); + status = psa_import_key( &attributes, + ram_slots[slot_number].content, + PSA_BITS_TO_BYTES( ram_slots[slot_number].bits ), + &handle ); + if( status != PSA_SUCCESS ) + return( status ); + status = psa_export_public_key( handle, data, data_size, data_length ); + psa_destroy_key( handle ); return( PSA_SUCCESS ); } From 8df72f271f593116810795cad52b02693fd52ae5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 9 Aug 2019 12:50:39 +0200 Subject: [PATCH 26/34] SE generate/sign/verify tests: also test export_public Add a flow where the key is imported or fake-generated in the secure element, then call psa_export_public_key and do the software verification with the public key. --- .../test_suite_psa_crypto_se_driver_hal.data | 16 ++- ...st_suite_psa_crypto_se_driver_hal.function | 119 +++++++++++++----- 2 files changed, 98 insertions(+), 37 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data index bdd5703b0..5819f785b 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -140,16 +140,24 @@ register_key_smoke_test:MIN_DRIVER_LIFETIME:0:PSA_ERROR_NOT_PERMITTED Import-sign-verify: sign in driver, ECDSA depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED -sign_verify:1:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:0:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" +sign_verify:SIGN_IN_DRIVER_AND_PARALLEL_CREATION:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:0:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" + +Import-sign-verify: sign in driver then export_public, ECDSA +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +sign_verify:SIGN_IN_DRIVER_THEN_EXPORT_PUBLIC:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:0:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" Import-sign-verify: sign in software, ECDSA depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED -sign_verify:0:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:0:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" +sign_verify:SIGN_IN_SOFTWARE_AND_PARALLEL_CREATION:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:0:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" Generate-sign-verify: sign in driver, ECDSA depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED -sign_verify:1:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:256:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" +sign_verify:SIGN_IN_DRIVER_AND_PARALLEL_CREATION:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:256:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" + +Generate-sign-verify: sign in driver then export_public, ECDSA +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +sign_verify:SIGN_IN_DRIVER_THEN_EXPORT_PUBLIC:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:256:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" Generate-sign-verify: sign in software, ECDSA depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED -sign_verify:0:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:256:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" +sign_verify:SIGN_IN_SOFTWARE_AND_PARALLEL_CREATION:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_CURVE_SECP256R1 ):PSA_ALG_ECDSA_ANY:256:"49c9a8c18c4b885638c431cf1df1c994131609b580d4fd43a0cab17db2f13eee":"54686973206973206e6f74206120686173682e" diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 4757f6c0c..202f18c6d 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -444,6 +444,13 @@ exit: /* Other test helper functions */ /****************************************************************/ +typedef enum +{ + SIGN_IN_SOFTWARE_AND_PARALLEL_CREATION, + SIGN_IN_DRIVER_AND_PARALLEL_CREATION, + SIGN_IN_DRIVER_THEN_EXPORT_PUBLIC, +} sign_verify_method_t; + /* Check that the attributes of a key reported by psa_get_key_attributes() * are consistent with the attributes used when creating the key. */ static int check_key_attributes( @@ -1017,7 +1024,7 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void sign_verify( int sign_in_driver, +void sign_verify( int flow, int type_arg, int alg_arg, int bits_arg, data_t *key_material, data_t *input ) @@ -1036,16 +1043,17 @@ void sign_verify( int sign_in_driver, psa_key_id_t id = 1; psa_key_handle_t drv_handle = 0; /* key managed by the driver */ psa_key_handle_t sw_handle = 0; /* transparent key */ - psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_key_attributes_t sw_attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_key_attributes_t drv_attributes; uint8_t signature[PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE]; size_t signature_length; memset( &driver, 0, sizeof( driver ) ); memset( &key_management, 0, sizeof( key_management ) ); + memset( &asymmetric, 0, sizeof( asymmetric ) ); driver.hal_version = PSA_DRV_SE_HAL_VERSION; driver.key_management = &key_management; driver.asymmetric = &asymmetric; - driver.persistent_data_size = sizeof( psa_key_slot_number_t ); driver.persistent_data_size = sizeof( ram_slot_usage_t ); key_management.p_allocate = ram_allocate; key_management.p_destroy = ram_destroy; @@ -1053,58 +1061,103 @@ void sign_verify( int sign_in_driver, key_management.p_generate = ram_fake_generate; else key_management.p_import = ram_import; - if( sign_in_driver ) - asymmetric.p_sign = ram_sign; + switch( flow ) + { + case SIGN_IN_SOFTWARE_AND_PARALLEL_CREATION: + break; + case SIGN_IN_DRIVER_AND_PARALLEL_CREATION: + asymmetric.p_sign = ram_sign; + break; + case SIGN_IN_DRIVER_THEN_EXPORT_PUBLIC: + asymmetric.p_sign = ram_sign; + key_management.p_export_public = ram_export_public; + break; + default: + TEST_ASSERT( ! "unsupported flow (should be SIGN_IN_xxx)" ); + break; + } asymmetric.p_verify = ram_verify; PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); PSA_ASSERT( psa_crypto_init( ) ); - /* Create two keys with the same key material: a transparent key, - * and one that goes through the driver. */ - psa_set_key_usage_flags( &attributes, + /* Prepare to create two keys with the same key material: a transparent + * key, and one that goes through the driver. */ + psa_set_key_usage_flags( &sw_attributes, PSA_KEY_USAGE_SIGN | PSA_KEY_USAGE_VERIFY ); - psa_set_key_algorithm( &attributes, alg ); - psa_set_key_type( &attributes, type ); - PSA_ASSERT( psa_import_key( &attributes, - key_material->x, key_material->len, - &sw_handle ) ); - psa_set_key_id( &attributes, id ); - psa_set_key_lifetime( &attributes, lifetime ); + psa_set_key_algorithm( &sw_attributes, alg ); + psa_set_key_type( &sw_attributes, type ); + drv_attributes = sw_attributes; + psa_set_key_id( &drv_attributes, id ); + psa_set_key_lifetime( &drv_attributes, lifetime ); + + /* Create the key in the driver. */ if( generating ) { - psa_set_key_bits( &attributes, bits ); - PSA_ASSERT( psa_generate_key( &attributes, &drv_handle ) ); + psa_set_key_bits( &drv_attributes, bits ); + PSA_ASSERT( psa_generate_key( &drv_attributes, &drv_handle ) ); /* Since we called a generate method that does not actually * generate material, store the desired result of generation in * the mock secure element storage. */ - PSA_ASSERT( psa_get_key_attributes( drv_handle, &attributes ) ); + PSA_ASSERT( psa_get_key_attributes( drv_handle, &drv_attributes ) ); TEST_ASSERT( key_material->len == PSA_BITS_TO_BYTES( bits ) ); memcpy( ram_slots[ram_min_slot].content, key_material->x, key_material->len ); } else { - PSA_ASSERT( psa_import_key( &attributes, + PSA_ASSERT( psa_import_key( &drv_attributes, key_material->x, key_material->len, &drv_handle ) ); } + /* Either import the same key in software, or export the driver's + * public key and import that. */ + switch( flow ) + { + case SIGN_IN_SOFTWARE_AND_PARALLEL_CREATION: + case SIGN_IN_DRIVER_AND_PARALLEL_CREATION: + PSA_ASSERT( psa_import_key( &sw_attributes, + key_material->x, key_material->len, + &sw_handle ) ); + break; + case SIGN_IN_DRIVER_THEN_EXPORT_PUBLIC: + { + uint8_t public_key[PSA_KEY_EXPORT_ECC_PUBLIC_KEY_MAX_SIZE( PSA_VENDOR_ECC_MAX_CURVE_BITS )]; + size_t public_key_length; + PSA_ASSERT( psa_export_public_key( drv_handle, + public_key, sizeof( public_key ), + &public_key_length ) ); + psa_set_key_type( &sw_attributes, + PSA_KEY_TYPE_PUBLIC_KEY_OF_KEY_PAIR( type ) ); + PSA_ASSERT( psa_import_key( &sw_attributes, + public_key, public_key_length, + &sw_handle ) ); + break; + } + } + /* Sign with the chosen key. */ - if( sign_in_driver ) - PSA_ASSERT_VIA_DRIVER( - psa_asymmetric_sign( drv_handle, - alg, - input->x, input->len, - signature, sizeof( signature ), - &signature_length ), - PSA_SUCCESS ); - else - PSA_ASSERT( psa_asymmetric_sign( sw_handle, - alg, - input->x, input->len, - signature, sizeof( signature ), - &signature_length ) ); + switch( flow ) + { + case SIGN_IN_DRIVER_AND_PARALLEL_CREATION: + case SIGN_IN_DRIVER_THEN_EXPORT_PUBLIC: + PSA_ASSERT_VIA_DRIVER( + psa_asymmetric_sign( drv_handle, + alg, + input->x, input->len, + signature, sizeof( signature ), + &signature_length ), + PSA_SUCCESS ); + break; + case SIGN_IN_SOFTWARE_AND_PARALLEL_CREATION: + PSA_ASSERT( psa_asymmetric_sign( sw_handle, + alg, + input->x, input->len, + signature, sizeof( signature ), + &signature_length ) ); + break; + } /* Verify with both keys. */ PSA_ASSERT( psa_asymmetric_verify( sw_handle, alg, From b4e73e9747ecc5ffa749100f91b18c2b79bcf5b4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Aug 2019 15:00:57 +0200 Subject: [PATCH 27/34] Add some design notes about multipart operation structures --- include/psa/crypto_struct.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index fbfe77e62..28bbc6ac8 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -12,6 +12,26 @@ * In implementations with isolation between the application and the * cryptography module, it is expected that the front-end and the back-end * would have different versions of this file. + * + *

Design notes about multipart operation structures

+ * + * Each multipart operation structure contains a `psa_algorithm_t alg` + * field which indicates which specific algorithm the structure is for. + * When the structure is not in use, `alg` is 0. Most of the structure + * consists of a union which is discriminated by `alg`. + * + * Note that when `alg` is 0, the content of other fields is undefined. + * In particular, it is not guaranteed that a freshly-initialized structure + * is all-zero: we initialize structures to something like `{0, 0}`, which + * is only guaranteed to initializes the first member of the union; + * GCC and Clang initialize the whole structure to 0 (at the time of writing), + * but MSVC and CompCert don't. + * + * In Mbed Crypto, multipart operation structures live independently from + * the key. This allows Mbed Crypto to free the key objects when destroying + * a key slot. If a multipart operation needs to remember the key after + * the setup function returns, the operation structure needs to contain a + * copy of the key. */ /* * Copyright (C) 2018, ARM Limited, All Rights Reserved From 3f7cd62ff5cfd8e6e23800bca93f7f56c0592d84 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Aug 2019 15:01:08 +0200 Subject: [PATCH 28/34] Document better what wiping a key slot does not do When a key slot is wiped, a copy of the key material may remain in operations. This is undesirable, but does not violate the safety of the code. Tracked in https://github.com/ARMmbed/mbed-crypto/issues/86 --- library/psa_crypto.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 3a78f5653..6041732fd 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -994,18 +994,16 @@ static psa_status_t psa_remove_key_data_from_memory( psa_key_slot_t *slot ) return( PSA_SUCCESS ); } -static void psa_abort_operations_using_key( psa_key_slot_t *slot ) -{ - /*FIXME how to implement this?*/ - (void) slot; -} - /** Completely wipe a slot in memory, including its policy. * Persistent storage is not affected. */ psa_status_t psa_wipe_key_slot( psa_key_slot_t *slot ) { psa_status_t status = psa_remove_key_data_from_memory( slot ); - psa_abort_operations_using_key( slot ); + /* Multipart operations may still be using the key. This is safe + * because all multipart operation objects are independent from + * the key slot: if they need to access the key after the setup + * phase, they have a copy of the key. Note that this means that + * key material can linger until all operations are completed. */ /* At this point, key material and other type-specific content has * been wiped. Clear remaining metadata. We can call memset and not * zeroize because the metadata is not particularly sensitive. */ From 8fe253ae4abe8e5f3fb7436cedee09e1ec67cd8d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Aug 2019 15:11:25 +0200 Subject: [PATCH 29/34] SE keys: test that psa_destroy_key removes the key from storage --- .../test_suite_psa_crypto_se_driver_hal.function | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 202f18c6d..867016b3e 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -793,6 +793,9 @@ void key_creation_import_export( int min_slot, int restart ) exported, exported_length ); PSA_ASSERT( psa_destroy_key( handle ) ); + handle = 0; + TEST_EQUAL( psa_open_key( id, &handle ), + PSA_ERROR_DOES_NOT_EXIST ); /* Test that the key has been erased from the designated slot. */ TEST_ASSERT( ram_slots[min_slot].type == 0 ); @@ -864,6 +867,9 @@ void key_creation_in_chosen_slot( int slot_arg, PSA_ASSERT( psa_get_key_attributes( handle, &attributes ) ); PSA_ASSERT( psa_destroy_key( handle ) ); + handle = 0; + TEST_EQUAL( psa_open_key( id, &handle ), + PSA_ERROR_DOES_NOT_EXIST ); exit: PSA_DONE( ); @@ -923,6 +929,9 @@ void import_key_smoke( int type_arg, int alg_arg, /* We're done. */ PSA_ASSERT( psa_destroy_key( handle ) ); + handle = 0; + TEST_EQUAL( psa_open_key( id, &handle ), + PSA_ERROR_DOES_NOT_EXIST ); exit: PSA_DONE( ); @@ -1016,6 +1025,9 @@ void generate_key_smoke( int type_arg, int bits_arg, int alg_arg ) /* We're done. */ PSA_ASSERT( psa_destroy_key( handle ) ); + handle = 0; + TEST_EQUAL( psa_open_key( id, &handle ), + PSA_ERROR_DOES_NOT_EXIST ); exit: PSA_DONE( ); @@ -1250,6 +1262,9 @@ void register_key_smoke_test( int lifetime_arg, goto exit; /* This time, destroy the key. */ PSA_ASSERT( psa_destroy_key( handle ) ); + handle = 0; + TEST_EQUAL( psa_open_key( id, &handle ), + PSA_ERROR_DOES_NOT_EXIST ); exit: psa_reset_key_attributes( &attributes ); From caec27821fab3db60bc52bea08602d7b1528d724 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Aug 2019 15:11:49 +0200 Subject: [PATCH 30/34] SE keys: make psa_destroy_key remove the key from storage --- library/psa_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 6041732fd..4fee3cd02 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1050,7 +1050,7 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle ) #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) - if( slot->attr.lifetime == PSA_KEY_LIFETIME_PERSISTENT ) + if( slot->attr.lifetime != PSA_KEY_LIFETIME_VOLATILE ) { storage_status = psa_destroy_persistent_key( slot->attr.id ); From 9ce31c466d181848f31ba932428db3d4da398a6d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Aug 2019 15:14:20 +0200 Subject: [PATCH 31/34] Note about destroying a key with other open handles https://github.com/ARMmbed/mbed-crypto/issues/214 --- library/psa_crypto.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 4fee3cd02..66c515108 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1052,8 +1052,11 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle ) #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) if( slot->attr.lifetime != PSA_KEY_LIFETIME_VOLATILE ) { - storage_status = - psa_destroy_persistent_key( slot->attr.id ); + storage_status = psa_destroy_persistent_key( slot->attr.id ); + /* TODO: other slots may have a copy of the same key. We should + * invalidate them. + * https://github.com/ARMmbed/mbed-crypto/issues/214 + */ } #endif /* defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) */ From 4b7f340fbfb8b3e85e1fa44a9230ae933d0e58e3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Aug 2019 15:58:36 +0200 Subject: [PATCH 32/34] Clean up status code handling inside psa_destroy_key Adopt a simple method for tracking whether there was a failure: each fallible operation sets overall_status, unless overall_status is already non-successful. Thus in case of multiple failures, the function always reports whatever failed first. This may not always be the right thing, but it's simple. This revealed a bug whereby if the only failure was the call to psa_destroy_se_key(), i.e. if the driver reported a failure or if the driver lacked support for destroying keys, psa_destroy_key() would ignore that failure. For a key in a secure element, if creating a transaction file fails, don't touch storage, but close the key in memory. This may not be right, but it's no wronger than it was before. Tracked in https://github.com/ARMmbed/mbed-crypto/issues/215 --- library/psa_crypto.c | 46 ++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 66c515108..bce777b15 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1014,8 +1014,8 @@ psa_status_t psa_wipe_key_slot( psa_key_slot_t *slot ) psa_status_t psa_destroy_key( psa_key_handle_t handle ) { psa_key_slot_t *slot; - psa_status_t status = PSA_SUCCESS; - psa_status_t storage_status = PSA_SUCCESS; + psa_status_t status; /* status of the last operation */ + psa_status_t overall_status = PSA_SUCCESS; #if defined(MBEDTLS_PSA_CRYPTO_SE_C) psa_se_drv_table_entry_t *driver; #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ @@ -1041,18 +1041,30 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle ) if( status != PSA_SUCCESS ) { (void) psa_crypto_stop_transaction( ); - /* TOnogrepDO: destroy what can be destroyed anyway */ - return( status ); + /* We should still try to destroy the key in the secure + * element and the key metadata in storage. This is especially + * important if the error is that the storage is full. + * But how to do it exactly without risking an inconsistent + * state after a reset? + * https://github.com/ARMmbed/mbed-crypto/issues/215 + */ + overall_status = status; + goto exit; } status = psa_destroy_se_key( driver, slot->data.se.slot_number ); + if( overall_status == PSA_SUCCESS ) + overall_status = status; } #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) if( slot->attr.lifetime != PSA_KEY_LIFETIME_VOLATILE ) { - storage_status = psa_destroy_persistent_key( slot->attr.id ); + status = psa_destroy_persistent_key( slot->attr.id ); + if( overall_status == PSA_SUCCESS ) + overall_status = status; + /* TODO: other slots may have a copy of the same key. We should * invalidate them. * https://github.com/ARMmbed/mbed-crypto/issues/214 @@ -1063,23 +1075,23 @@ psa_status_t psa_destroy_key( psa_key_handle_t handle ) #if defined(MBEDTLS_PSA_CRYPTO_SE_C) if( driver != NULL ) { - psa_status_t status2; status = psa_save_se_persistent_data( driver ); - status2 = psa_crypto_stop_transaction( ); - if( status == PSA_SUCCESS ) - status = status2; - if( status != PSA_SUCCESS ) - { - /* TOnogrepDO: destroy what can be destroyed anyway */ - return( status ); - } + if( overall_status == PSA_SUCCESS ) + overall_status = status; + status = psa_crypto_stop_transaction( ); + if( overall_status == PSA_SUCCESS ) + overall_status = status; } #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) +exit: +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ status = psa_wipe_key_slot( slot ); - if( status != PSA_SUCCESS ) - return( status ); - return( storage_status ); + /* Prioritize CORRUPTION_DETECTED from wiping over a storage error */ + if( overall_status == PSA_SUCCESS ) + overall_status = status; + return( overall_status ); } void psa_reset_key_attributes( psa_key_attributes_t *attributes ) From 5da7b3e55cf01c85da5a70bdb48c6ec80451a135 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Aug 2019 16:03:28 +0200 Subject: [PATCH 33/34] Drivers must have a psa_destroy_key method Drivers that allow destroying a key must have a destroy method. This test bug was previously not caught because of an implementation bug that lost the error triggered by the missing destroy method. --- ...est_suite_psa_crypto_se_driver_hal.function | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.function b/tests/suites/test_suite_psa_crypto_se_driver_hal.function index 867016b3e..fc6f66816 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -162,6 +162,17 @@ static psa_status_t null_generate( psa_drv_se_context_t *context, return( PSA_SUCCESS ); } +/* Null destroy: do nothing, but pretend it worked. */ +static psa_status_t null_destroy( psa_drv_se_context_t *context, + void *persistent_data, + psa_key_slot_number_t slot_number ) +{ + (void) context; + (void) persistent_data; + (void) slot_number; + return( PSA_SUCCESS ); +} + /****************************************************************/ @@ -898,6 +909,7 @@ void import_key_smoke( int type_arg, int alg_arg, driver.persistent_data_size = sizeof( psa_key_slot_number_t ); key_management.p_allocate = counter_allocate; key_management.p_import = null_import; + key_management.p_destroy = null_destroy; PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); PSA_ASSERT( psa_crypto_init( ) ); @@ -995,6 +1007,7 @@ void generate_key_smoke( int type_arg, int bits_arg, int alg_arg ) driver.persistent_data_size = sizeof( psa_key_slot_number_t ); key_management.p_allocate = counter_allocate; key_management.p_generate = null_generate; + key_management.p_destroy = null_destroy; PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); PSA_ASSERT( psa_crypto_init( ) ); @@ -1220,10 +1233,11 @@ void register_key_smoke_test( int lifetime_arg, memset( &driver, 0, sizeof( driver ) ); driver.hal_version = PSA_DRV_SE_HAL_VERSION; + memset( &key_management, 0, sizeof( key_management ) ); + driver.key_management = &key_management; + key_management.p_destroy = null_destroy; if( validate >= 0 ) { - memset( &key_management, 0, sizeof( key_management ) ); - driver.key_management = &key_management; key_management.p_validate_slot_number = validate_slot_number_as_directed; validate_slot_number_directions.slot_number = wanted_slot; validate_slot_number_directions.method = PSA_KEY_CREATION_REGISTER; From c9d7f94a654815fe8dbf1fef1ca074e865423f46 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Aug 2019 16:17:16 +0200 Subject: [PATCH 34/34] Add issue numbers for some missing parts of secure element support --- library/psa_crypto.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index bce777b15..34a023190 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1214,8 +1214,10 @@ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, case PSA_KEY_TYPE_RSA_KEY_PAIR: case PSA_KEY_TYPE_RSA_PUBLIC_KEY: #if defined(MBEDTLS_PSA_CRYPTO_SE_C) - /* TOnogrepDO: reporting the public exponent for opaque keys - * is not yet implemented. */ + /* TODO: reporting the public exponent for opaque keys + * is not yet implemented. + * https://github.com/ARMmbed/mbed-crypto/issues/216 + */ if( psa_key_slot_is_external( slot ) ) break; #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ @@ -1735,10 +1737,12 @@ static void psa_fail_key_creation( psa_key_slot_t *slot, return; #if defined(MBEDTLS_PSA_CRYPTO_SE_C) - /* TOnogrepDO: If the key has already been created in the secure + /* TODO: If the key has already been created in the secure * element, and the failure happened later (when saving metadata * to internal storage), we need to destroy the key in the secure - * element. */ + * element. + * https://github.com/ARMmbed/mbed-crypto/issues/217 + */ /* Abort the ongoing transaction if any (there may not be one if * the creation process failed before starting one, or if the @@ -6088,8 +6092,10 @@ static psa_status_t psa_crypto_recover_transaction( { case PSA_CRYPTO_TRANSACTION_CREATE_KEY: case PSA_CRYPTO_TRANSACTION_DESTROY_KEY: - /* TOnogrepDO - fall through to the failure case until this - * is implemented */ + /* TODO - fall through to the failure case until this + * is implemented. + * https://github.com/ARMmbed/mbed-crypto/issues/218 + */ default: /* We found an unsupported transaction in the storage. * We don't know what state the storage is in. Give up. */