From b6cadea6a5db02c4faedb40f08fa57f58b1ed31a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 24 Jun 2019 13:46:37 +0200 Subject: [PATCH 01/12] Secure element driver structure Define a structure type containing all the methods of a secure element driver. --- include/psa/crypto_se_driver.h | 42 ++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 9f54947ca..95947dbdd 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -8,11 +8,11 @@ * space in which the PSA Crypto implementation runs, typically secure * elements (SEs). * - * This file is part of the PSA Crypto Driver Model, containing functions for - * driver developers to implement to enable hardware to be called in a - * standardized way by a PSA Cryptographic API implementation. The functions - * comprising the driver model, which driver authors implement, are not - * intended to be called by application developers. + * This file is part of the PSA Crypto Driver HAL (hardware abstraction layer), + * containing functions for driver developers to implement to enable hardware + * to be called in a standardized way by a PSA Cryptography API + * implementation. The functions comprising the driver HAL, which driver + * authors implement, are not intended to be called by application developers. */ /* @@ -961,6 +961,38 @@ typedef struct { /**@}*/ +/** \defgroup se_registration Secure element driver registration + */ +/**@{*/ + +/** A structure containing pointers to all the entry points of a + * secure element driver. + * + * Future versions of this specification may add extra substructures at + * the end of this structure. + */ +typedef struct { + /** The version of the driver model that this driver implements. + * This is a protection against linking driver binaries built against + * a different version of this specification. + * Use #PSA_DRV_SE_HAL_VERSION. + */ + uint32_t hal_version; + psa_drv_se_key_management_t key_management; + psa_drv_se_mac_t mac; + psa_drv_se_cipher_t cipher; + psa_drv_se_aead_t aead; + psa_drv_se_asymmetric_t asymmetric; + psa_drv_se_key_derivation_t derivation; +} psa_drv_se_t; + +/** The current version of the opaque driver model. + */ +/* 0.0.0 patchlevel 5 */ +#define PSA_DRV_SE_HAL_VERSION 0x00000005 + +/**@}*/ + #ifdef __cplusplus } #endif From d910e928e889faeb4692bff06823610497b5ffaa Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 24 Jun 2019 13:47:07 +0200 Subject: [PATCH 02/12] Declare a function to register a secure element driver --- include/psa/crypto_se_driver.h | 45 ++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 95947dbdd..85dc05a54 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -991,6 +991,51 @@ typedef struct { /* 0.0.0 patchlevel 5 */ #define PSA_DRV_SE_HAL_VERSION 0x00000005 +/** Register an external cryptoprocessor driver. + * + * This function is only intended to be used by driver code, not by + * application code. In implementations with separation between the + * PSA cryptography module and applications, this function should + * only be available to callers that run in the same memory space as + * the cryptography module, and should not be exposed to applications + * running in a different memory space. + * + * This function may be called before psa_crypto_init(). It is + * implementation-defined whether this function may be called + * after psa_crypto_init(). + * + * \param lifetime The lifetime value through which this driver will + * be exposed to applications. + * The values #PSA_KEY_LIFETIME_VOLATILE and + * #PSA_KEY_LIFETIME_PERSISTENT are reserved and + * may not be used for opaque drivers. Implementations + * may reserve other values. + * \param[in] methods The method table of the driver. This structure must + * remain valid for as long as the cryptography + * module keeps running. It is typically a global + * constant. + * + * \return PSA_SUCCESS + * The driver was successfully registered. Applications can now + * use \p lifetime to access keys through the methods passed to + * this function. + * \return PSA_ERROR_BAD_STATE + * This function was called after the initialization of the + * cryptography module, and this implementation does not support + * driver registration at this stage. + * \return PSA_ERROR_ALREADY_EXISTS + * There is already a registered driver for this value of \p lifetime. + * \return PSA_ERROR_INVALID_ARGUMENT + * \p lifetime is a reserved value + * \return PSA_ERROR_NOT_SUPPORTED + * `methods->interface_version` is not supported by this implementation. + * \return PSA_ERROR_INSUFFICIENT_MEMORY + * \return PSA_ERROR_NOT_PERMITTED + */ +psa_status_t psa_register_se_driver( + psa_key_lifetime_t lifetime, + const psa_drv_se_t *methods); + /**@}*/ #ifdef __cplusplus From 2c2243dc0b750743c4a1eaf5dca5891b5b07908f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 24 Jun 2019 13:47:24 +0200 Subject: [PATCH 03/12] Smoke test for secure element driver registration --- tests/CMakeLists.txt | 1 + .../test_suite_psa_crypto_se_driver_hal.data | 2 ++ ...st_suite_psa_crypto_se_driver_hal.function | 29 +++++++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 tests/suites/test_suite_psa_crypto_se_driver_hal.data create mode 100644 tests/suites/test_suite_psa_crypto_se_driver_hal.function diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 42d99d623..323ad2dc6 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -135,6 +135,7 @@ add_test_suite(psa_crypto_hash) add_test_suite(psa_crypto_init) add_test_suite(psa_crypto_metadata) add_test_suite(psa_crypto_persistent_key) +add_test_suite(psa_crypto_se_driver_hal) add_test_suite(psa_crypto_slot_management) add_test_suite(psa_its) add_test_suite(shax) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data new file mode 100644 index 000000000..1d27fb9b6 --- /dev/null +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -0,0 +1,2 @@ +Register SE driver: good +register_one:2:PSA_DRV_SE_HAL_VERSION: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 new file mode 100644 index 000000000..efbbd4e36 --- /dev/null +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -0,0 +1,29 @@ +/* BEGIN_HEADER */ +#include "psa_crypto_helpers.h" +#include "psa/crypto_se_driver.h" + +/* END_HEADER */ + +/* BEGIN_DEPENDENCIES + * depends_on:MBEDTLS_PSA_CRYPTO_C + * END_DEPENDENCIES + */ + +/* BEGIN_CASE */ +void register_one( int lifetime, int version, int expected_status_arg ) +{ + psa_status_t expected_status = expected_status_arg; + psa_drv_se_t driver; + + memset( &driver, 0, sizeof( driver ) ); + driver.hal_version = version; + + TEST_EQUAL( psa_register_se_driver( lifetime, &driver ), + expected_status ); + + PSA_ASSERT( psa_crypto_init( ) ); + +exit: + PSA_DONE( ); +} +/* END_CASE */ From a899a72fd0a854199048e3ca69ccc55f14a4678a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 24 Jun 2019 14:06:43 +0200 Subject: [PATCH 04/12] Implement the secure element driver registration function --- library/CMakeLists.txt | 1 + library/Makefile | 2 +- library/psa_crypto_se.c | 72 ++++++++++++++++++++++++++++++++++ library/psa_crypto_se.h | 37 +++++++++++++++++ visualc/VS2010/mbedTLS.vcxproj | 2 + 5 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 library/psa_crypto_se.c create mode 100644 library/psa_crypto_se.h diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 61bc13d32..78c233a08 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -53,6 +53,7 @@ set(src_crypto platform_util.c poly1305.c psa_crypto.c + psa_crypto_se.c psa_crypto_slot_management.c psa_crypto_storage.c psa_its_file.c diff --git a/library/Makefile b/library/Makefile index 921b68ec7..2b979b487 100644 --- a/library/Makefile +++ b/library/Makefile @@ -80,7 +80,7 @@ OBJS_CRYPTO= aes.o aesni.o arc4.o \ pk.o pk_wrap.o pkcs12.o \ pkcs5.o pkparse.o pkwrite.o \ platform.o platform_util.o poly1305.o \ - psa_crypto.o \ + psa_crypto.o psa_crypto_se.o \ psa_crypto_slot_management.o \ psa_crypto_storage.o \ psa_its_file.o \ diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c new file mode 100644 index 000000000..33d0da894 --- /dev/null +++ b/library/psa_crypto_se.c @@ -0,0 +1,72 @@ +/* + * PSA crypto support for secure element drivers + */ +/* Copyright (C) 2019, ARM Limited, All Rights Reserved + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * This file is part of Mbed TLS (https://tls.mbed.org) + */ + +#if !defined(MBEDTLS_CONFIG_FILE) +#include "mbedtls/config.h" +#else +#include MBEDTLS_CONFIG_FILE +#endif + +#if defined(MBEDTLS_PSA_CRYPTO_C) + +#include "psa_crypto_se.h" + +typedef struct +{ + psa_key_lifetime_t lifetime; + const psa_drv_se_t *methods; +} method_table_entry_t; + +static method_table_entry_t driver_table[PSA_MAX_SE_DRIVERS]; + +psa_status_t psa_register_se_driver( + psa_key_lifetime_t lifetime, + const psa_drv_se_t *methods) +{ + size_t i; + + if( methods->hal_version != PSA_DRV_SE_HAL_VERSION ) + return( PSA_ERROR_NOT_SUPPORTED ); + if( lifetime == PSA_KEY_LIFETIME_VOLATILE || + lifetime == PSA_KEY_LIFETIME_PERSISTENT ) + { + return( PSA_ERROR_INVALID_ARGUMENT ); + } + + for( i = 0; i < PSA_MAX_SE_DRIVERS; i++ ) + { + if( driver_table[i].lifetime == 0 ) + break; + /* Check that lifetime isn't already in use up to the first free + * entry. Since entries are created in order and never deleted, + * there can't be a used entry after the first free entry. */ + if( driver_table[i].lifetime == lifetime ) + return( PSA_ERROR_ALREADY_EXISTS ); + } + if( i == PSA_MAX_SE_DRIVERS ) + return( PSA_ERROR_INSUFFICIENT_MEMORY ); + + driver_table[i].lifetime = lifetime; + driver_table[i].methods = methods; + return( PSA_SUCCESS ); +} + +#endif /* MBEDTLS_PSA_CRYPTO_C */ diff --git a/library/psa_crypto_se.h b/library/psa_crypto_se.h new file mode 100644 index 000000000..1085f488d --- /dev/null +++ b/library/psa_crypto_se.h @@ -0,0 +1,37 @@ +/* + * PSA crypto support for secure element drivers + */ +/* Copyright (C) 2019, ARM Limited, All Rights Reserved + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * This file is part of Mbed TLS (https://tls.mbed.org) + */ + +#ifndef PSA_CRYPTO_SE_H +#define PSA_CRYPTO_SE_H + +#if !defined(MBEDTLS_CONFIG_FILE) +#include "mbedtls/config.h" +#else +#include MBEDTLS_CONFIG_FILE +#endif + +#include "psa/crypto.h" +#include "psa/crypto_se_driver.h" + +/** The maximum number of registered secure element driver lifetimes. */ +#define PSA_MAX_SE_DRIVERS 4 + +#endif /* PSA_CRYPTO_SE_H */ diff --git a/visualc/VS2010/mbedTLS.vcxproj b/visualc/VS2010/mbedTLS.vcxproj index 07c80e84f..2034a8411 100644 --- a/visualc/VS2010/mbedTLS.vcxproj +++ b/visualc/VS2010/mbedTLS.vcxproj @@ -225,6 +225,7 @@ + @@ -281,6 +282,7 @@ + From d0890211287d80b06f8851bfc3c80498fb4ed2f5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 24 Jun 2019 14:34:43 +0200 Subject: [PATCH 05/12] Unregister drivers on library deinitialization --- library/psa_crypto.c | 4 ++++ library/psa_crypto_se.c | 7 +++++++ library/psa_crypto_se.h | 8 ++++++++ 3 files changed, 19 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index b3be2617b..7e2007129 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -32,6 +32,7 @@ #include "psa_crypto_core.h" #include "psa_crypto_invasive.h" +#include "psa_crypto_se.h" #include "psa_crypto_slot_management.h" /* Include internal declarations that are useful for implementing persistently * stored keys. */ @@ -5211,6 +5212,9 @@ void mbedtls_psa_crypto_free( void ) * In particular, this sets all state indicator to the value * indicating "uninitialized". */ mbedtls_platform_zeroize( &global_data, sizeof( global_data ) ); + /* Unregister all secure element drivers, so that we restart from + * a pristine state. */ + psa_unregister_all_se_drivers( ); } psa_status_t psa_crypto_init( void ) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index 33d0da894..32142eb9a 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -27,6 +27,8 @@ #if defined(MBEDTLS_PSA_CRYPTO_C) +#include + #include "psa_crypto_se.h" typedef struct @@ -69,4 +71,9 @@ psa_status_t psa_register_se_driver( return( PSA_SUCCESS ); } +void psa_unregister_all_se_drivers( void ) +{ + memset( driver_table, 0, sizeof( driver_table ) ); +} + #endif /* MBEDTLS_PSA_CRYPTO_C */ diff --git a/library/psa_crypto_se.h b/library/psa_crypto_se.h index 1085f488d..e99bd2576 100644 --- a/library/psa_crypto_se.h +++ b/library/psa_crypto_se.h @@ -34,4 +34,12 @@ /** The maximum number of registered secure element driver lifetimes. */ #define PSA_MAX_SE_DRIVERS 4 +/** Unregister all secure element drivers. + * + * \warning Do not call this function while the library is in the initialized + * state. This function is only intended to be called at the end + * of mbedtls_psa_crypto_free(). + */ +void psa_unregister_all_se_drivers( void ); + #endif /* PSA_CRYPTO_SE_H */ From 55a6acfe4d596db313706e1c83d369f15e7af251 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 24 Jun 2019 14:34:59 +0200 Subject: [PATCH 06/12] Add negative tests for driver registration --- .../test_suite_psa_crypto_se_driver_hal.data | 20 ++++++++ ...st_suite_psa_crypto_se_driver_hal.function | 51 +++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data index 1d27fb9b6..b55ab3264 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -1,2 +1,22 @@ Register SE driver: good register_one:2:PSA_DRV_SE_HAL_VERSION:PSA_SUCCESS + +# Run this test case a second time to verify that the library deinit +# unregistered the first driver. +Register SE driver: good, again +register_one:2:PSA_DRV_SE_HAL_VERSION:PSA_SUCCESS + +Register SE driver: invalid lifetime (VOLATILE) +register_one:PSA_KEY_LIFETIME_VOLATILE:PSA_DRV_SE_HAL_VERSION:PSA_ERROR_INVALID_ARGUMENT + +Register SE driver: invalid lifetime (PERSISTENT) +register_one:PSA_KEY_LIFETIME_VOLATILE:PSA_DRV_SE_HAL_VERSION:PSA_ERROR_INVALID_ARGUMENT + +Register SE driver: invalid version +register_one:2:PSA_DRV_SE_HAL_VERSION - 1:PSA_ERROR_NOT_SUPPORTED + +Register SE driver: already registered +register_twice:3 + +Register SE driver: maximum number of drivers +register_max: 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 efbbd4e36..522065a90 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -2,6 +2,11 @@ #include "psa_crypto_helpers.h" #include "psa/crypto_se_driver.h" +#include "psa_crypto_se.h" + +/* The minimum valid lifetime value for a secure element driver. */ +#define MIN_DRIVER_LIFETIME 2 + /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -27,3 +32,49 @@ exit: PSA_DONE( ); } /* END_CASE */ + +/* BEGIN_CASE */ +void register_twice( int count ) +{ + psa_drv_se_t driver; + psa_key_lifetime_t lifetime; + psa_key_lifetime_t max = MIN_DRIVER_LIFETIME + count; + + memset( &driver, 0, sizeof( driver ) ); + driver.hal_version = PSA_DRV_SE_HAL_VERSION; + + for( lifetime = MIN_DRIVER_LIFETIME; lifetime < max; lifetime++ ) + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + for( lifetime = MIN_DRIVER_LIFETIME; lifetime < max; lifetime++ ) + TEST_EQUAL( psa_register_se_driver( lifetime, &driver ), + PSA_ERROR_ALREADY_EXISTS ); + + PSA_ASSERT( psa_crypto_init( ) ); + +exit: + PSA_DONE( ); +} +/* END_CASE */ + +/* BEGIN_CASE */ +void register_max( ) +{ + psa_drv_se_t driver; + psa_key_lifetime_t lifetime; + psa_key_lifetime_t max = MIN_DRIVER_LIFETIME + PSA_MAX_SE_DRIVERS; + + memset( &driver, 0, sizeof( driver ) ); + driver.hal_version = PSA_DRV_SE_HAL_VERSION; + + for( lifetime = MIN_DRIVER_LIFETIME; lifetime < max; lifetime++ ) + PSA_ASSERT( psa_register_se_driver( lifetime, &driver ) ); + + TEST_EQUAL( psa_register_se_driver( lifetime, &driver ), + PSA_ERROR_INSUFFICIENT_MEMORY ); + + PSA_ASSERT( psa_crypto_init( ) ); + +exit: + PSA_DONE( ); +} +/* END_CASE */ From 45a8ca373c34e314191155e689ec1a5adfdf012b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 24 Jun 2019 15:08:56 +0200 Subject: [PATCH 07/12] Fix typos in function argument names --- include/psa/crypto_se_driver.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index 85dc05a54..b7a37133b 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -726,7 +726,7 @@ typedef psa_status_t (*psa_drv_se_import_key_t)(psa_key_slot_number_t key_slot, * \retval #PSA_SUCCESS * The slot's content, if any, has been erased. */ -typedef psa_status_t (*psa_drv_se_destroy_key_t)(psa_key_slot_number_t key); +typedef psa_status_t (*psa_drv_se_destroy_key_t)(psa_key_slot_number_t key_slot); /** * \brief A function that exports a secure element key in binary format @@ -878,7 +878,7 @@ typedef struct { * \param[in,out] p_context A hardware-specific structure containing any * context information for the implementation * \param[in] kdf_alg The algorithm to be used for the key derivation - * \param[in] souce_key The key to be used as the source material for the + * \param[in] source_key The key to be used as the source material for the * key derivation * * \retval PSA_SUCCESS From 7a52464fbae9a550798ab291b88b158a6aa0e501 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 24 Jun 2019 19:58:12 +0200 Subject: [PATCH 08/12] Driver registration: more future-proof bad-version test --- tests/suites/test_suite_psa_crypto_se_driver_hal.data | 7 +++++-- 1 file changed, 5 insertions(+), 2 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 b55ab3264..20a06e843 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -12,8 +12,11 @@ register_one:PSA_KEY_LIFETIME_VOLATILE:PSA_DRV_SE_HAL_VERSION:PSA_ERROR_INVALID_ Register SE driver: invalid lifetime (PERSISTENT) register_one:PSA_KEY_LIFETIME_VOLATILE:PSA_DRV_SE_HAL_VERSION:PSA_ERROR_INVALID_ARGUMENT -Register SE driver: invalid version -register_one:2:PSA_DRV_SE_HAL_VERSION - 1:PSA_ERROR_NOT_SUPPORTED +Register SE driver: invalid version (ancient) +register_one:2:0x00000003:PSA_ERROR_NOT_SUPPORTED + +Register SE driver: invalid version (future) +register_one:2:PSA_DRV_SE_HAL_VERSION + 1:PSA_ERROR_NOT_SUPPORTED Register SE driver: already registered register_twice:3 From c93a43bed677d21cde1bc6072db77d97c8e091b4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Jun 2019 11:21:41 +0200 Subject: [PATCH 09/12] Improve documentation --- include/psa/crypto_se_driver.h | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/include/psa/crypto_se_driver.h b/include/psa/crypto_se_driver.h index b7a37133b..85247051e 100644 --- a/include/psa/crypto_se_driver.h +++ b/include/psa/crypto_se_driver.h @@ -972,8 +972,8 @@ typedef struct { * the end of this structure. */ typedef struct { - /** The version of the driver model that this driver implements. - * This is a protection against linking driver binaries built against + /** The version of the driver HAL that this driver implements. + * This is a protection against loading driver binaries built against * a different version of this specification. * Use #PSA_DRV_SE_HAL_VERSION. */ @@ -986,12 +986,12 @@ typedef struct { psa_drv_se_key_derivation_t derivation; } psa_drv_se_t; -/** The current version of the opaque driver model. +/** The current version of the secure element driver HAL. */ /* 0.0.0 patchlevel 5 */ #define PSA_DRV_SE_HAL_VERSION 0x00000005 -/** Register an external cryptoprocessor driver. +/** Register an external cryptoprocessor (secure element) driver. * * This function is only intended to be used by driver code, not by * application code. In implementations with separation between the @@ -1004,11 +1004,18 @@ typedef struct { * implementation-defined whether this function may be called * after psa_crypto_init(). * + * \note Implementations store metadata about keys including the lifetime + * value. Therefore, from one instantiation of the PSA Cryptography + * library to the next one, if there is a key in storage with a certain + * lifetime value, you must always register the same driver (or an + * updated version that communicates with the same secure element) + * with the same lifetime value. + * * \param lifetime The lifetime value through which this driver will * be exposed to applications. * The values #PSA_KEY_LIFETIME_VOLATILE and * #PSA_KEY_LIFETIME_PERSISTENT are reserved and - * may not be used for opaque drivers. Implementations + * may not be used for drivers. Implementations * may reserve other values. * \param[in] methods The method table of the driver. This structure must * remain valid for as long as the cryptography @@ -1026,9 +1033,9 @@ typedef struct { * \return PSA_ERROR_ALREADY_EXISTS * There is already a registered driver for this value of \p lifetime. * \return PSA_ERROR_INVALID_ARGUMENT - * \p lifetime is a reserved value + * \p lifetime is a reserved value. * \return PSA_ERROR_NOT_SUPPORTED - * `methods->interface_version` is not supported by this implementation. + * `methods->hal_version` is not supported by this implementation. * \return PSA_ERROR_INSUFFICIENT_MEMORY * \return PSA_ERROR_NOT_PERMITTED */ From a8ade16ffd4e8466be75a03fbec4e98cb4db479f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Jun 2019 11:24:49 +0200 Subject: [PATCH 10/12] Gate secure element support by a separate config option Secure element support has its own source file, and in addition requires many hooks in other files. This is a nontrivial amount of code, so make it optional (but default on). --- include/mbedtls/check_config.h | 6 ++++++ include/mbedtls/config.h | 13 +++++++++++++ library/psa_crypto.c | 4 ++++ library/psa_crypto_se.c | 4 ++-- library/version_features.c | 3 +++ programs/test/query_config.c | 8 ++++++++ scripts/config.pl | 1 + tests/scripts/all.sh | 5 +++++ .../test_suite_psa_crypto_se_driver_hal.function | 2 +- 9 files changed, 43 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 78bf131e0..13210746d 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -458,6 +458,12 @@ #error "MBEDTLS_PSA_CRYPTO_SPM defined, but not all prerequisites" #endif +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) && \ + ! ( defined(MBEDTLS_PSA_CRYPTO_C) && \ + defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) ) +#error "MBEDTLS_PSA_CRYPTO_SE_C defined, but not all prerequisites" +#endif + #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) && \ ! defined(MBEDTLS_PSA_CRYPTO_C) #error "MBEDTLS_PSA_CRYPTO_STORAGE_C defined, but not all prerequisites" diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 56ad01c40..0e8d7550e 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1709,6 +1709,19 @@ */ #define MBEDTLS_PSA_CRYPTO_C +/** + * \def MBEDTLS_PSA_CRYPTO_SE_C + * + * Enable secure element support in the Platform Security Architecture + * cryptography API. + * + * Module: library/psa_crypto_se.c + * + * Requires: MBEDTLS_PSA_CRYPTO_C, MBEDTLS_PSA_CRYPTO_STORAGE_C + * + */ +#define MBEDTLS_PSA_CRYPTO_SE_C + /** * \def MBEDTLS_PSA_CRYPTO_STORAGE_C * diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 7e2007129..8789084d1 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -32,7 +32,9 @@ #include "psa_crypto_core.h" #include "psa_crypto_invasive.h" +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) #include "psa_crypto_se.h" +#endif #include "psa_crypto_slot_management.h" /* Include internal declarations that are useful for implementing persistently * stored keys. */ @@ -5212,9 +5214,11 @@ void mbedtls_psa_crypto_free( void ) * In particular, this sets all state indicator to the value * indicating "uninitialized". */ mbedtls_platform_zeroize( &global_data, sizeof( global_data ) ); +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) /* Unregister all secure element drivers, so that we restart from * a pristine state. */ psa_unregister_all_se_drivers( ); +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ } psa_status_t psa_crypto_init( void ) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index 32142eb9a..814c6a003 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -25,7 +25,7 @@ #include MBEDTLS_CONFIG_FILE #endif -#if defined(MBEDTLS_PSA_CRYPTO_C) +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) #include @@ -76,4 +76,4 @@ void psa_unregister_all_se_drivers( void ) memset( driver_table, 0, sizeof( driver_table ) ); } -#endif /* MBEDTLS_PSA_CRYPTO_C */ +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ diff --git a/library/version_features.c b/library/version_features.c index 4f1da6aea..57015986c 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -555,6 +555,9 @@ static const char *features[] = { #if defined(MBEDTLS_PSA_CRYPTO_C) "MBEDTLS_PSA_CRYPTO_C", #endif /* MBEDTLS_PSA_CRYPTO_C */ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + "MBEDTLS_PSA_CRYPTO_SE_C", +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) "MBEDTLS_PSA_CRYPTO_STORAGE_C", #endif /* MBEDTLS_PSA_CRYPTO_STORAGE_C */ diff --git a/programs/test/query_config.c b/programs/test/query_config.c index fc25353fa..ee754d9b0 100644 --- a/programs/test/query_config.c +++ b/programs/test/query_config.c @@ -1509,6 +1509,14 @@ int query_config( const char *config ) } #endif /* MBEDTLS_PSA_CRYPTO_C */ +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + if( strcmp( "MBEDTLS_PSA_CRYPTO_SE_C", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_PSA_CRYPTO_SE_C ); + return( 0 ); + } +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) if( strcmp( "MBEDTLS_PSA_CRYPTO_STORAGE_C", config ) == 0 ) { diff --git a/scripts/config.pl b/scripts/config.pl index b66790514..05cc52e64 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -107,6 +107,7 @@ MBEDTLS_MEMORY_BACKTRACE MBEDTLS_MEMORY_BUFFER_ALLOC_C MBEDTLS_PLATFORM_TIME_ALT MBEDTLS_PLATFORM_FPRINTF_ALT +MBEDTLS_PSA_CRYPTO_SE_C MBEDTLS_PSA_CRYPTO_STORAGE_C MBEDTLS_PSA_CRYPTO_STORAGE_ITS_C MBEDTLS_PSA_ITS_FILE_C diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index b38c7d457..c1e1ffe24 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -709,6 +709,7 @@ component_test_no_platform () { scripts/config.pl unset MBEDTLS_ENTROPY_NV_SEED scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C scripts/config.pl unset MBEDTLS_FS_IO + scripts/config.pl unset MBEDTLS_PSA_CRYPTO_SE_C scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_C scripts/config.pl unset MBEDTLS_PSA_ITS_FILE_C scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_ITS_C @@ -894,6 +895,7 @@ component_build_arm_none_eabi_gcc () { scripts/config.pl unset MBEDTLS_TIMING_C scripts/config.pl unset MBEDTLS_FS_IO scripts/config.pl unset MBEDTLS_PSA_ITS_FILE_C + scripts/config.pl unset MBEDTLS_PSA_CRYPTO_SE_C scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_ITS_C scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_C scripts/config.pl unset MBEDTLS_ENTROPY_NV_SEED @@ -913,6 +915,7 @@ component_build_arm_none_eabi_gcc_no_udbl_division () { scripts/config.pl unset MBEDTLS_TIMING_C scripts/config.pl unset MBEDTLS_FS_IO scripts/config.pl unset MBEDTLS_PSA_ITS_FILE_C + scripts/config.pl unset MBEDTLS_PSA_CRYPTO_SE_C scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_ITS_C scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_C scripts/config.pl unset MBEDTLS_ENTROPY_NV_SEED @@ -935,6 +938,7 @@ component_build_arm_none_eabi_gcc_no_64bit_multiplication () { scripts/config.pl unset MBEDTLS_TIMING_C scripts/config.pl unset MBEDTLS_FS_IO scripts/config.pl unset MBEDTLS_PSA_ITS_FILE_C + scripts/config.pl unset MBEDTLS_PSA_CRYPTO_SE_C scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_ITS_C scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_C scripts/config.pl unset MBEDTLS_ENTROPY_NV_SEED @@ -957,6 +961,7 @@ component_build_armcc () { scripts/config.pl unset MBEDTLS_TIMING_C scripts/config.pl unset MBEDTLS_FS_IO scripts/config.pl unset MBEDTLS_PSA_ITS_FILE_C + scripts/config.pl unset MBEDTLS_PSA_CRYPTO_SE_C scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_ITS_C scripts/config.pl unset MBEDTLS_PSA_CRYPTO_STORAGE_C scripts/config.pl unset MBEDTLS_ENTROPY_NV_SEED 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 522065a90..b9d0a1f0a 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.function +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.function @@ -10,7 +10,7 @@ /* END_HEADER */ /* BEGIN_DEPENDENCIES - * depends_on:MBEDTLS_PSA_CRYPTO_C + * depends_on:MBEDTLS_PSA_CRYPTO_SE_C * END_DEPENDENCIES */ From 9717d107ca5bcc8d9a32b0cae64686297b942f36 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Jun 2019 11:50:04 +0200 Subject: [PATCH 11/12] Explain that lifetime=0 from static initialization means VOLATILE --- library/psa_crypto_se.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/library/psa_crypto_se.c b/library/psa_crypto_se.c index 814c6a003..688d4e7c8 100644 --- a/library/psa_crypto_se.c +++ b/library/psa_crypto_se.c @@ -27,6 +27,7 @@ #if defined(MBEDTLS_PSA_CRYPTO_SE_C) +#include #include #include "psa_crypto_se.h" @@ -47,6 +48,12 @@ psa_status_t psa_register_se_driver( if( methods->hal_version != PSA_DRV_SE_HAL_VERSION ) return( PSA_ERROR_NOT_SUPPORTED ); + /* Driver table entries are 0-initialized. 0 is not a valid driver + * lifetime because it means a volatile key. */ +#if defined(static_assert) + static_assert( PSA_KEY_LIFETIME_VOLATILE == 0, + "Secure element support requires 0 to mean a volatile key" ); +#endif if( lifetime == PSA_KEY_LIFETIME_VOLATILE || lifetime == PSA_KEY_LIFETIME_PERSISTENT ) { From 651447905c56c1eebf656658a898dbe4590c6174 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Jun 2019 11:50:30 +0200 Subject: [PATCH 12/12] Fix typo in invalid-lifetime test and add explicit test for 0 --- tests/suites/test_suite_psa_crypto_se_driver_hal.data | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data index 20a06e843..c04b70d96 100644 --- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -6,11 +6,14 @@ register_one:2:PSA_DRV_SE_HAL_VERSION:PSA_SUCCESS Register SE driver: good, again register_one:2:PSA_DRV_SE_HAL_VERSION:PSA_SUCCESS +Register SE driver: invalid lifetime (0) +register_one:0:PSA_DRV_SE_HAL_VERSION:PSA_ERROR_INVALID_ARGUMENT + Register SE driver: invalid lifetime (VOLATILE) register_one:PSA_KEY_LIFETIME_VOLATILE:PSA_DRV_SE_HAL_VERSION:PSA_ERROR_INVALID_ARGUMENT Register SE driver: invalid lifetime (PERSISTENT) -register_one:PSA_KEY_LIFETIME_VOLATILE:PSA_DRV_SE_HAL_VERSION:PSA_ERROR_INVALID_ARGUMENT +register_one:PSA_KEY_LIFETIME_PERSISTENT:PSA_DRV_SE_HAL_VERSION:PSA_ERROR_INVALID_ARGUMENT Register SE driver: invalid version (ancient) register_one:2:0x00000003:PSA_ERROR_NOT_SUPPORTED