From d28f293bb00c5f61072633f5d909e7fd4d6f4830 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Jun 2021 18:18:07 +0200 Subject: [PATCH 1/5] Import crypto_spe.h from TF-M https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/plain/secure_fw/partitions/crypto/crypto_spe.h?h=refs/heads/master Signed-off-by: Gilles Peskine --- tests/include/spe/crypto_spe.h | 132 +++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 tests/include/spe/crypto_spe.h diff --git a/tests/include/spe/crypto_spe.h b/tests/include/spe/crypto_spe.h new file mode 100644 index 000000000..f80fd86bd --- /dev/null +++ b/tests/include/spe/crypto_spe.h @@ -0,0 +1,132 @@ +/* + * Copyright (c) 2019-2021, Arm Limited. All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + * + */ + +/** + * \file crypto_spe.h + * + * \brief When Mbed Crypto is built with the MBEDTLS_PSA_CRYPTO_SPM option + * enabled, this header is included by all .c files in Mbed Crypto that + * use PSA Crypto function names. This avoids duplication of symbols + * between TF-M and Mbed Crypto. + * + * \note This file should be included before including any PSA Crypto headers + * from Mbed Crypto. + */ + +#ifndef CRYPTO_SPE_H +#define CRYPTO_SPE_H + +#define PSA_FUNCTION_NAME(x) mbedcrypto__ ## x + +#define psa_crypto_init \ + PSA_FUNCTION_NAME(psa_crypto_init) +#define psa_key_derivation_get_capacity \ + PSA_FUNCTION_NAME(psa_key_derivation_get_capacity) +#define psa_key_derivation_set_capacity \ + PSA_FUNCTION_NAME(psa_key_derivation_set_capacity) +#define psa_key_derivation_input_bytes \ + PSA_FUNCTION_NAME(psa_key_derivation_input_bytes) +#define psa_key_derivation_output_bytes \ + PSA_FUNCTION_NAME(psa_key_derivation_output_bytes) +#define psa_key_derivation_input_key \ + PSA_FUNCTION_NAME(psa_key_derivation_input_key) +#define psa_key_derivation_output_key \ + PSA_FUNCTION_NAME(psa_key_derivation_output_key) +#define psa_key_derivation_setup \ + PSA_FUNCTION_NAME(psa_key_derivation_setup) +#define psa_key_derivation_abort \ + PSA_FUNCTION_NAME(psa_key_derivation_abort) +#define psa_key_derivation_key_agreement \ + PSA_FUNCTION_NAME(psa_key_derivation_key_agreement) +#define psa_raw_key_agreement \ + PSA_FUNCTION_NAME(psa_raw_key_agreement) +#define psa_generate_random \ + PSA_FUNCTION_NAME(psa_generate_random) +#define psa_aead_encrypt \ + PSA_FUNCTION_NAME(psa_aead_encrypt) +#define psa_aead_decrypt \ + PSA_FUNCTION_NAME(psa_aead_decrypt) +#define psa_open_key \ + PSA_FUNCTION_NAME(psa_open_key) +#define psa_close_key \ + PSA_FUNCTION_NAME(psa_close_key) +#define psa_import_key \ + PSA_FUNCTION_NAME(psa_import_key) +#define psa_destroy_key \ + PSA_FUNCTION_NAME(psa_destroy_key) +#define psa_get_key_attributes \ + PSA_FUNCTION_NAME(psa_get_key_attributes) +#define psa_reset_key_attributes \ + PSA_FUNCTION_NAME(psa_reset_key_attributes) +#define psa_export_key \ + PSA_FUNCTION_NAME(psa_export_key) +#define psa_export_public_key \ + PSA_FUNCTION_NAME(psa_export_public_key) +#define psa_purge_key \ + PSA_FUNCTION_NAME(psa_purge_key) +#define psa_copy_key \ + PSA_FUNCTION_NAME(psa_copy_key) +#define psa_cipher_operation_init \ + PSA_FUNCTION_NAME(psa_cipher_operation_init) +#define psa_cipher_generate_iv \ + PSA_FUNCTION_NAME(psa_cipher_generate_iv) +#define psa_cipher_set_iv \ + PSA_FUNCTION_NAME(psa_cipher_set_iv) +#define psa_cipher_encrypt_setup \ + PSA_FUNCTION_NAME(psa_cipher_encrypt_setup) +#define psa_cipher_decrypt_setup \ + PSA_FUNCTION_NAME(psa_cipher_decrypt_setup) +#define psa_cipher_update \ + PSA_FUNCTION_NAME(psa_cipher_update) +#define psa_cipher_finish \ + PSA_FUNCTION_NAME(psa_cipher_finish) +#define psa_cipher_abort \ + PSA_FUNCTION_NAME(psa_cipher_abort) +#define psa_hash_operation_init \ + PSA_FUNCTION_NAME(psa_hash_operation_init) +#define psa_hash_setup \ + PSA_FUNCTION_NAME(psa_hash_setup) +#define psa_hash_update \ + PSA_FUNCTION_NAME(psa_hash_update) +#define psa_hash_finish \ + PSA_FUNCTION_NAME(psa_hash_finish) +#define psa_hash_verify \ + PSA_FUNCTION_NAME(psa_hash_verify) +#define psa_hash_abort \ + PSA_FUNCTION_NAME(psa_hash_abort) +#define psa_hash_clone \ + PSA_FUNCTION_NAME(psa_hash_clone) +#define psa_hash_compute \ + PSA_FUNCTION_NAME(psa_hash_compute) +#define psa_hash_compare \ + PSA_FUNCTION_NAME(psa_hash_compare) +#define psa_mac_operation_init \ + PSA_FUNCTION_NAME(psa_mac_operation_init) +#define psa_mac_sign_setup \ + PSA_FUNCTION_NAME(psa_mac_sign_setup) +#define psa_mac_verify_setup \ + PSA_FUNCTION_NAME(psa_mac_verify_setup) +#define psa_mac_update \ + PSA_FUNCTION_NAME(psa_mac_update) +#define psa_mac_sign_finish \ + PSA_FUNCTION_NAME(psa_mac_sign_finish) +#define psa_mac_verify_finish \ + PSA_FUNCTION_NAME(psa_mac_verify_finish) +#define psa_mac_abort \ + PSA_FUNCTION_NAME(psa_mac_abort) +#define psa_sign_hash \ + PSA_FUNCTION_NAME(psa_sign_hash) +#define psa_verify_hash \ + PSA_FUNCTION_NAME(psa_verify_hash) +#define psa_asymmetric_encrypt \ + PSA_FUNCTION_NAME(psa_asymmetric_encrypt) +#define psa_asymmetric_decrypt \ + PSA_FUNCTION_NAME(psa_asymmetric_decrypt) +#define psa_generate_key \ + PSA_FUNCTION_NAME(psa_generate_key) + +#endif /* CRYPTO_SPE_H */ From 984c19f553e9281077c5c40480e01a84602f08fd Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Jun 2021 18:37:38 +0200 Subject: [PATCH 2/5] Do a test build with MBEDTLS_PSA_CRYPTO_SPM Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index dd3dbb739..561b362d9 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -798,6 +798,19 @@ component_test_psa_crypto_key_id_encodes_owner () { make test } +component_build_psa_crypto_spm () { + msg "build: full config - USE_PSA_CRYPTO + PSA_CRYPTO_KEY_ID_ENCODES_OWNER + PSA_CRYPTO_SPM, make, gcc" + scripts/config.py full + scripts/config.py unset MBEDTLS_USE_PSA_CRYPTO + scripts/config.py unset MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS + scripts/config.py set MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER + scripts/config.py set MBEDTLS_PSA_CRYPTO_SPM + # We can only compile, not link, since our test and sample programs + # aren't equipped for the modified names used when MBEDTLS_PSA_CRYPTO_SPM + # is active. + make CC=gcc CFLAGS='-Werror -Wall -Wextra -I../tests/include/spe' lib +} + component_test_psa_crypto_client () { msg "build: default config - PSA_CRYPTO_C + PSA_CRYPTO_CLIENT, make" scripts/config.py unset MBEDTLS_PSA_CRYPTO_C From 99a346278548387afaf16bdc222fc609152353f5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 17 Jun 2021 11:37:52 +0200 Subject: [PATCH 3/5] In the SPM test build, fail if a symbol wasn't renamed Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 561b362d9..388bdf8fe 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -798,6 +798,14 @@ component_test_psa_crypto_key_id_encodes_owner () { make test } +# check_renamed_symbols HEADER LIB +# Check that if HEADER contains '#define MACRO ...' then MACRO is not a symbol +# name is LIB. +check_renamed_symbols () { + ! nm "$2" | sed 's/.* //' | + grep -x -F "$(sed -n 's/^ *# *define *\([A-Z_a-z][0-9A-Z_a-z]*\)..*/\1/p' "$1")" +} + component_build_psa_crypto_spm () { msg "build: full config - USE_PSA_CRYPTO + PSA_CRYPTO_KEY_ID_ENCODES_OWNER + PSA_CRYPTO_SPM, make, gcc" scripts/config.py full @@ -809,6 +817,11 @@ component_build_psa_crypto_spm () { # aren't equipped for the modified names used when MBEDTLS_PSA_CRYPTO_SPM # is active. make CC=gcc CFLAGS='-Werror -Wall -Wextra -I../tests/include/spe' lib + + # Check that if a symbol is renamed by crypto_spe.h, the non-renamed + # version is not present. + echo "Checking for renamed symbols in the library" + if_build_succeeded check_renamed_symbols tests/include/spe/crypto_spe.h library/libmbedcrypto.a } component_test_psa_crypto_client () { From 76dec15d54f6ba69fb4e6b8e92b58e8ab3a25685 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 15 Jun 2021 18:36:05 +0200 Subject: [PATCH 4/5] Move the inclusion of crypto_spe.h to psa/crypto_platform.h This makes it easier to ensure that crypto_spe.h is included everywhere it needs to be, and that it's included early enough to do its job (it must be included before any mention of psa_xxx() functions with external linkage, because it defines macros to rename these functions). Signed-off-by: Gilles Peskine --- ChangeLog.d/spm_build.txt | 4 +++ include/psa/crypto_platform.h | 12 ++++++++ library/psa_crypto.c | 1 - library/psa_crypto_client.c | 1 - library/psa_crypto_service_integration.h | 39 ------------------------ library/psa_crypto_slot_management.c | 1 - library/psa_crypto_storage.c | 1 - 7 files changed, 16 insertions(+), 43 deletions(-) create mode 100644 ChangeLog.d/spm_build.txt delete mode 100644 library/psa_crypto_service_integration.h diff --git a/ChangeLog.d/spm_build.txt b/ChangeLog.d/spm_build.txt new file mode 100644 index 000000000..6016d84e0 --- /dev/null +++ b/ChangeLog.d/spm_build.txt @@ -0,0 +1,4 @@ +Bugfix + * When MBEDTLS_PSA_CRYPTO_SPM is enabled, crypto_spe.h was not included + in all the right places. Include it from crypto_platform.h, which is + the natural place. Fixes #4649. diff --git a/include/psa/crypto_platform.h b/include/psa/crypto_platform.h index 8acf22c7f..66f468793 100644 --- a/include/psa/crypto_platform.h +++ b/include/psa/crypto_platform.h @@ -81,6 +81,18 @@ static inline int mbedtls_key_owner_id_equal( mbedtls_key_owner_id_t id1, #endif /* MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER */ +/* + * When MBEDTLS_PSA_CRYPTO_SPM is defined, the code is being built for SPM + * (Secure Partition Manager) integration which separates the code into two + * parts: NSPE (Non-Secure Processing Environment) and SPE (Secure Processing + * Environment). When building for the SPE, an additional header file should be + * included. + */ +#if defined(MBEDTLS_PSA_CRYPTO_SPM) +#define PSA_CRYPTO_SECURE 1 +#include "crypto_spe.h" +#endif // MBEDTLS_PSA_CRYPTO_SPM + #if defined(MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG) /** The type of the context passed to mbedtls_psa_external_get_random(). * diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 775923af5..845fc74bb 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -26,7 +26,6 @@ #include "check_crypto_config.h" #endif -#include "psa_crypto_service_integration.h" #include "psa/crypto.h" #include "psa_crypto_cipher.h" diff --git a/library/psa_crypto_client.c b/library/psa_crypto_client.c index e84cf3015..629feb7df 100644 --- a/library/psa_crypto_client.c +++ b/library/psa_crypto_client.c @@ -19,7 +19,6 @@ */ #include "common.h" -#include "psa_crypto_service_integration.h" #include "psa/crypto.h" #if defined(MBEDTLS_PSA_CRYPTO_CLIENT) diff --git a/library/psa_crypto_service_integration.h b/library/psa_crypto_service_integration.h deleted file mode 100644 index 87889af49..000000000 --- a/library/psa_crypto_service_integration.h +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright The Mbed TLS Contributors - * SPDX-License-Identifier: Apache-2.0 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef PSA_CRYPTO_SERVICE_INTEGRATION_H -#define PSA_CRYPTO_SERVICE_INTEGRATION_H - -/* - * When MBEDTLS_PSA_CRYPTO_SPM is defined, the code is being built for SPM - * (Secure Partition Manager) integration which separates the code into two - * parts: NSPE (Non-Secure Processing Environment) and SPE (Secure Processing - * Environment). When building for the SPE, an additional header file should be - * included. - */ -#if defined(MBEDTLS_PSA_CRYPTO_SPM) -/* - * PSA_CRYPTO_SECURE means that the file which included this file is being - * compiled for SPE. The files crypto_structs.h and crypto_types.h have - * different implementations for NSPE and SPE and are compiled according to this - * flag. - */ -#define PSA_CRYPTO_SECURE 1 -#include "crypto_spe.h" -#endif // MBEDTLS_PSA_CRYPTO_SPM - -#endif // PSA_CRYPTO_SERVICE_INTEGRATION_H diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 0b1a3c166..672388531 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -22,7 +22,6 @@ #if defined(MBEDTLS_PSA_CRYPTO_C) -#include "psa_crypto_service_integration.h" #include "psa/crypto.h" #include "psa_crypto_core.h" diff --git a/library/psa_crypto_storage.c b/library/psa_crypto_storage.c index 773d3aaaf..2ebfc26a8 100644 --- a/library/psa_crypto_storage.c +++ b/library/psa_crypto_storage.c @@ -29,7 +29,6 @@ #include #include -#include "psa_crypto_service_integration.h" #include "psa/crypto.h" #include "psa_crypto_storage.h" #include "mbedtls/platform_util.h" From ee334d1b75f441d1f31c5bd0653668c5857717d2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 24 Jun 2021 20:05:20 +0200 Subject: [PATCH 5/5] Update Visual Studio project Signed-off-by: Gilles Peskine --- visualc/VS2010/mbedTLS.vcxproj | 1 - 1 file changed, 1 deletion(-) diff --git a/visualc/VS2010/mbedTLS.vcxproj b/visualc/VS2010/mbedTLS.vcxproj index 9cec716e3..cd68bb026 100644 --- a/visualc/VS2010/mbedTLS.vcxproj +++ b/visualc/VS2010/mbedTLS.vcxproj @@ -272,7 +272,6 @@ -