From 28648236710155d30538638428b935dd1cf4d20f Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Wed, 6 Sep 2023 11:50:45 +0800 Subject: [PATCH 01/22] configs: move TFM config to a subdirectory Signed-off-by: Yanray Wang --- configs/{ => ext}/crypto_config_profile_medium.h | 0 configs/{ => ext}/tfm_mbedcrypto_config_profile_medium.h | 0 scripts/code_size_compare.py | 4 ++-- 3 files changed, 2 insertions(+), 2 deletions(-) rename configs/{ => ext}/crypto_config_profile_medium.h (100%) rename configs/{ => ext}/tfm_mbedcrypto_config_profile_medium.h (100%) diff --git a/configs/crypto_config_profile_medium.h b/configs/ext/crypto_config_profile_medium.h similarity index 100% rename from configs/crypto_config_profile_medium.h rename to configs/ext/crypto_config_profile_medium.h diff --git a/configs/tfm_mbedcrypto_config_profile_medium.h b/configs/ext/tfm_mbedcrypto_config_profile_medium.h similarity index 100% rename from configs/tfm_mbedcrypto_config_profile_medium.h rename to configs/ext/tfm_mbedcrypto_config_profile_medium.h diff --git a/scripts/code_size_compare.py b/scripts/code_size_compare.py index 53d859edfa..b45f45599f 100755 --- a/scripts/code_size_compare.py +++ b/scripts/code_size_compare.py @@ -156,8 +156,8 @@ def detect_arch() -> str: print("Unknown host architecture, cannot auto-detect arch.") sys.exit(1) -TFM_MEDIUM_CONFIG_H = 'configs/tfm_mbedcrypto_config_profile_medium.h' -TFM_MEDIUM_CRYPTO_CONFIG_H = 'configs/crypto_config_profile_medium.h' +TFM_MEDIUM_CONFIG_H = 'configs/ext/tfm_mbedcrypto_config_profile_medium.h' +TFM_MEDIUM_CRYPTO_CONFIG_H = 'configs/ext/crypto_config_profile_medium.h' CONFIG_H = 'include/mbedtls/mbedtls_config.h' CRYPTO_CONFIG_H = 'include/psa/crypto_config.h' From b153aaed9e16672f16312fd6dd31f034b778d42d Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Wed, 6 Sep 2023 12:32:10 +0800 Subject: [PATCH 02/22] configs: add config_tfm.h which includes TFM configs Signed-off-by: Yanray Wang --- configs/config-tfm.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 configs/config-tfm.h diff --git a/configs/config-tfm.h b/configs/config-tfm.h new file mode 100644 index 0000000000..ec9dc98df7 --- /dev/null +++ b/configs/config-tfm.h @@ -0,0 +1,27 @@ +/** + * \file config-tfm.h + * + * \brief TF-M configuration with tweaks for a successful build and test. + */ +/* + * 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. + */ + +/* TF-M Configuration Options */ +#include "../configs/ext/tfm_mbedcrypto_config_profile_medium.h" + +/* TF-M PSA Crypto Configuration */ +#define MBEDTLS_PSA_CRYPTO_CONFIG_FILE "../configs/ext/crypto_config_profile_medium.h" From 0c98f9f8423c74fcd2a04a0f1c17e3f9fb37448b Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Wed, 6 Sep 2023 15:47:49 +0800 Subject: [PATCH 03/22] test-ref-configs: test config-tfm.h Tweak some configurations based on TF-M config in order to get a successful build and test. Signed-off-by: Yanray Wang --- configs/config-tfm.h | 20 ++++++++++++++++++++ tests/scripts/test-ref-configs.pl | 3 +++ 2 files changed, 23 insertions(+) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index ec9dc98df7..772e6e5e34 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -25,3 +25,23 @@ /* TF-M PSA Crypto Configuration */ #define MBEDTLS_PSA_CRYPTO_CONFIG_FILE "../configs/ext/crypto_config_profile_medium.h" + +/*****************************************************************************/ +/* Tweak configuration based on TF-M config for a successful build and test. */ +/*****************************************************************************/ + +/* MBEDTLS_PSA_CRYPTO_SPM needs third party files, so disable it. */ +#undef MBEDTLS_PSA_CRYPTO_SPM +/* TF-M provides its own (dummy) implemenations which Mbed TLS doesn't need. */ +#undef MBEDTLS_AES_SETKEY_DEC_ALT +#undef MBEDTLS_AES_DECRYPT_ALT +/* pkparse.c fails to link without this. */ +#define MBEDTLS_OID_C + +/* Since MBEDTLS_PSA_CRYPTO_STORAGE_C is disabled, we need to disable this to + pass test_suite_psa_crypto_slot_management. */ +#undef MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER +/* Use built-in platform entropy functions. */ +#undef MBEDTLS_NO_PLATFORM_ENTROPY +/* Disable buffer-based memory allocator */ +#undef MBEDTLS_MEMORY_BUFFER_ALLOC_C diff --git a/tests/scripts/test-ref-configs.pl b/tests/scripts/test-ref-configs.pl index 15209b4a0d..9fd98be8d0 100755 --- a/tests/scripts/test-ref-configs.pl +++ b/tests/scripts/test-ref-configs.pl @@ -53,6 +53,9 @@ my %configs = ( 'opt' => '-f ECJPAKE.*nolog', 'test_again_with_use_psa' => 1, }, + 'config-tfm.h' => { + 'test_again_with_use_psa' => 0, # Uses PSA by default, no need to test it twice + }, ); # If no config-name is provided, use all known configs. From 5f573f8301267a8d8b443f61530ab279358107b9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 6 Sep 2023 16:17:55 +0200 Subject: [PATCH 04/22] Fix broken test with MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER When testing the lifecycle of a transient key, it doesn't make much sense to try psa_open_key: that expects a persistent key and the lookup takes a different path. The error from psa_open_key is also different depending on whether MBEDTLS_PSA_CRYPTO_STORAGE_C is enabled. To check that the key ownership is taken into account, try to access the same key id with a different owner without expecting that this is a persistent key. Just call psa_get_key_attributes, which works fine for a transient key. This fixes a test failure when MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER is enabled and MBEDTLS_PSA_CRYPTO_STORAGE_C is disabled. Signed-off-by: Gilles Peskine --- configs/config-tfm.h | 3 --- tests/suites/test_suite_psa_crypto_slot_management.function | 5 ++--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index 772e6e5e34..500cb243f4 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -38,9 +38,6 @@ /* pkparse.c fails to link without this. */ #define MBEDTLS_OID_C -/* Since MBEDTLS_PSA_CRYPTO_STORAGE_C is disabled, we need to disable this to - pass test_suite_psa_crypto_slot_management. */ -#undef MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER /* Use built-in platform entropy functions. */ #undef MBEDTLS_NO_PLATFORM_ENTROPY /* Disable buffer-based memory allocator */ diff --git a/tests/suites/test_suite_psa_crypto_slot_management.function b/tests/suites/test_suite_psa_crypto_slot_management.function index 5bd12eb09e..b4f2d234ea 100644 --- a/tests/suites/test_suite_psa_crypto_slot_management.function +++ b/tests/suites/test_suite_psa_crypto_slot_management.function @@ -142,7 +142,6 @@ void transient_slot_lifecycle(int owner_id_arg, #if defined(MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER) { - psa_key_handle_t handle; mbedtls_svc_key_id_t key_with_invalid_owner = mbedtls_svc_key_id_make(owner_id + 1, MBEDTLS_SVC_KEY_ID_GET_KEY_ID(key)); @@ -150,8 +149,8 @@ void transient_slot_lifecycle(int owner_id_arg, TEST_ASSERT(mbedtls_key_owner_id_equal( owner_id, MBEDTLS_SVC_KEY_ID_GET_OWNER_ID(key))); - TEST_EQUAL(psa_open_key(key_with_invalid_owner, &handle), - PSA_ERROR_DOES_NOT_EXIST); + TEST_EQUAL(psa_get_key_attributes(key_with_invalid_owner, &attributes), + PSA_ERROR_INVALID_HANDLE); } #endif From eaa1c5619aa1a32900323c89a5f60ff92ba16d1a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 6 Sep 2023 16:23:13 +0200 Subject: [PATCH 05/22] Update location of TFM config files Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index c85d4865ed..7d91430ae5 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -3851,8 +3851,8 @@ support_build_tfm_armcc () { component_build_tfm_armcc() { # test the TF-M configuration can build cleanly with various warning flags enabled - cp configs/tfm_mbedcrypto_config_profile_medium.h "$CONFIG_H" - cp configs/crypto_config_profile_medium.h "$CRYPTO_CONFIG_H" + cp configs/ext/tfm_mbedcrypto_config_profile_medium.h "$CONFIG_H" + cp configs/ext/crypto_config_profile_medium.h "$CRYPTO_CONFIG_H" msg "build: TF-M config, armclang armv7-m thumb2" make clean @@ -3860,9 +3860,13 @@ component_build_tfm_armcc() { } component_build_tfm() { - # test the TF-M configuration can build cleanly with various warning flags enabled - cp configs/tfm_mbedcrypto_config_profile_medium.h "$CONFIG_H" - cp configs/crypto_config_profile_medium.h "$CRYPTO_CONFIG_H" + # Check that the TF-M configuration can build cleanly with various + # warning flags enabled. We don't build or run tests, since the + # TF-M configuration needs a TF-M platform. A tweaked version of + # the configuration that works on mainstream platforms is in + # configs/config-tfm.h, tested via test-ref-configs.pl. + cp configs/ext/tfm_mbedcrypto_config_profile_medium.h "$CONFIG_H" + cp configs/ext/crypto_config_profile_medium.h "$CRYPTO_CONFIG_H" msg "build: TF-M config, clang, armv7-m thumb2" make lib CC="clang" CFLAGS="--target=arm-linux-gnueabihf -march=armv7-m -mthumb -Os -std=c99 -Werror -Wall -Wextra -Wwrite-strings -Wpointer-arith -Wimplicit-fallthrough -Wshadow -Wvla -Wformat=2 -Wno-format-nonliteral -Wshadow -Wasm-operand-widths -Wunused" From da26a5172c005f5b06b6469b534c9ab4f99c875e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 6 Sep 2023 17:15:49 +0200 Subject: [PATCH 06/22] Disable PK_PARSE and PK_WRITE This is what TF-M intended and they have done so since we copied the file. It's either disable these options, or enable MBEDTLS_OID_C. Signed-off-by: Gilles Peskine --- configs/config-tfm.h | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index 500cb243f4..64dce48740 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -35,8 +35,17 @@ /* TF-M provides its own (dummy) implemenations which Mbed TLS doesn't need. */ #undef MBEDTLS_AES_SETKEY_DEC_ALT #undef MBEDTLS_AES_DECRYPT_ALT -/* pkparse.c fails to link without this. */ -#define MBEDTLS_OID_C +/* The configuration we have enables MBEDTLS_PK_PARSE_C and MBEDTLS_PK_WRITE_C + * but not MBEDTLS_OID_C. This is inconsistent, and leads to a link error + * when using one of the mbedtls_pk_parse_xxx or mbedtls_pk_write_xxx + * functions that depend on an mbedtls_oid_xxx function. + * Mbed TLS needs PK parse/write for RSA with PSA, but the medium + * profile doesn't have RSA. Later versions of TF-M no longer enable + * PK parse/write: it wasn't a wanted feature. So disable it here + * (otherwise we'd have to enable MBEDTLS_OID_C). + */ +#undef MBEDTLS_PK_PARSE_C +#undef MBEDTLS_PK_WRITE_C /* Use built-in platform entropy functions. */ #undef MBEDTLS_NO_PLATFORM_ENTROPY From e23fa41f1090807121414c9826c2602fb470aba7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 6 Sep 2023 17:16:36 +0200 Subject: [PATCH 07/22] Documentation improvements Signed-off-by: Gilles Peskine --- configs/config-tfm.h | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index 64dce48740..7ca83b6c94 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -1,7 +1,7 @@ /** * \file config-tfm.h * - * \brief TF-M configuration with tweaks for a successful build and test. + * \brief TF-M medium profile, adapted to work on other platforms. */ /* * Copyright The Mbed TLS Contributors @@ -20,19 +20,26 @@ * limitations under the License. */ -/* TF-M Configuration Options */ -#include "../configs/ext/tfm_mbedcrypto_config_profile_medium.h" +/* TF-M medium profile: mbedtls legacy configuration */ +#include "ext/tfm_mbedcrypto_config_profile_medium.h" -/* TF-M PSA Crypto Configuration */ +/* TF-M medium profile: PSA crypto configuration */ #define MBEDTLS_PSA_CRYPTO_CONFIG_FILE "../configs/ext/crypto_config_profile_medium.h" -/*****************************************************************************/ -/* Tweak configuration based on TF-M config for a successful build and test. */ -/*****************************************************************************/ +/***********************************************************/ +/* Tweak the configuration to remove dependencies on TF-M. */ +/***********************************************************/ -/* MBEDTLS_PSA_CRYPTO_SPM needs third party files, so disable it. */ +/* MBEDTLS_PSA_CRYPTO_SPM needs third-party files, so disable it. */ #undef MBEDTLS_PSA_CRYPTO_SPM -/* TF-M provides its own (dummy) implemenations which Mbed TLS doesn't need. */ + +/* TF-M provides its own dummy implementations to save code size. + * We don't have any way to disable the tests that need these feature, + * so we just keep AES decryption enabled. We will resolve this though + * an official way to disable AES decryption, then this deviation + * will no longer be needed: + * https://github.com/Mbed-TLS/mbedtls/issues/7368 + */ #undef MBEDTLS_AES_SETKEY_DEC_ALT #undef MBEDTLS_AES_DECRYPT_ALT /* The configuration we have enables MBEDTLS_PK_PARSE_C and MBEDTLS_PK_WRITE_C @@ -47,7 +54,10 @@ #undef MBEDTLS_PK_PARSE_C #undef MBEDTLS_PK_WRITE_C -/* Use built-in platform entropy functions. */ +/* Use built-in platform entropy functions (TF-M provides its own). */ #undef MBEDTLS_NO_PLATFORM_ENTROPY -/* Disable buffer-based memory allocator */ + +/* Disable buffer-based memory allocator. This isn't strictly required, + * but using the native allocator is faster and works better with + * memory management analysis frameworks such as ASan. */ #undef MBEDTLS_MEMORY_BUFFER_ALLOC_C From 5baf66755c5dd80f5266dc0452a799b683bdc218 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 6 Sep 2023 17:17:10 +0200 Subject: [PATCH 08/22] Keep the list in alphabetical order Signed-off-by: Gilles Peskine --- tests/scripts/test-ref-configs.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/scripts/test-ref-configs.pl b/tests/scripts/test-ref-configs.pl index 9fd98be8d0..80aa87a99b 100755 --- a/tests/scripts/test-ref-configs.pl +++ b/tests/scripts/test-ref-configs.pl @@ -49,13 +49,13 @@ my %configs = ( 'config-symmetric-only.h' => { 'test_again_with_use_psa' => 0, # Uses PSA by default, no need to test it twice }, + 'config-tfm.h' => { + 'test_again_with_use_psa' => 0, # Uses PSA by default, no need to test it twice + }, 'config-thread.h' => { 'opt' => '-f ECJPAKE.*nolog', 'test_again_with_use_psa' => 1, }, - 'config-tfm.h' => { - 'test_again_with_use_psa' => 0, # Uses PSA by default, no need to test it twice - }, ); # If no config-name is provided, use all known configs. From 4419d38a1559fdea57f70e3cf8658701d3e89062 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Thu, 7 Sep 2023 11:28:27 +0800 Subject: [PATCH 09/22] config-tfm.h: include TF-M medium profile properly config-tfm.h is copied into mbedtls_config.h in test-ref-config.pl. The relative path is include/ not configs/. Signed-off-by: Yanray Wang --- configs/config-tfm.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index 7ca83b6c94..dab13814e9 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -21,7 +21,7 @@ */ /* TF-M medium profile: mbedtls legacy configuration */ -#include "ext/tfm_mbedcrypto_config_profile_medium.h" +#include "../configs/ext/tfm_mbedcrypto_config_profile_medium.h" /* TF-M medium profile: PSA crypto configuration */ #define MBEDTLS_PSA_CRYPTO_CONFIG_FILE "../configs/ext/crypto_config_profile_medium.h" @@ -35,7 +35,7 @@ /* TF-M provides its own dummy implementations to save code size. * We don't have any way to disable the tests that need these feature, - * so we just keep AES decryption enabled. We will resolve this though + * so we just keep AES decryption enabled. We will resolve this through * an official way to disable AES decryption, then this deviation * will no longer be needed: * https://github.com/Mbed-TLS/mbedtls/issues/7368 From 7050504bdcd2cfe24fb560df4c9f346e27a4a0ee Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Thu, 21 Sep 2023 17:11:40 +0800 Subject: [PATCH 10/22] all.sh: simplify common_tfm_config Signed-off-by: Yanray Wang --- tests/scripts/all.sh | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 7d91430ae5..1e92d6ff0b 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2893,25 +2893,18 @@ component_test_psa_crypto_config_reference_ecc_ffdh_no_bignum () { # - component_test_tfm_config() common_tfm_config () { # Enable TF-M config - cp configs/tfm_mbedcrypto_config_profile_medium.h "$CONFIG_H" - cp configs/crypto_config_profile_medium.h "$CRYPTO_CONFIG_H" - - # Adjust for the fact that we're building outside the TF-M environment. - # - # TF-M has separation, our build doesn't - scripts/config.py unset MBEDTLS_PSA_CRYPTO_SPM - scripts/config.py unset MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER - # TF-M provdes its own (dummy) implemenation, from their tree - scripts/config.py unset MBEDTLS_AES_DECRYPT_ALT - scripts/config.py unset MBEDTLS_AES_SETKEY_DEC_ALT - # We have an OS that provides entropy, use it - scripts/config.py unset MBEDTLS_NO_PLATFORM_ENTROPY + cp configs/config-tfm.h "$CONFIG_H" + echo "#undef MBEDTLS_PSA_CRYPTO_CONFIG_FILE" >> "$CONFIG_H" + cp configs/ext/crypto_config_profile_medium.h "$CRYPTO_CONFIG_H" # Other config adjustments to make the tests pass. # Those should probably be adopted upstream. # # - USE_PSA_CRYPTO for PK_HAVE_ECC_KEYS echo "#define MBEDTLS_USE_PSA_CRYPTO" >> "$CONFIG_H" + # PK_[PARSE/WRITE]_C used to avoid build and link errors in test_suite_pk.c + echo "#define MBEDTLS_PK_PARSE_C" >> "$CONFIG_H" + echo "#define MBEDTLS_PK_WRITE_C" >> "$CONFIG_H" # pkparse.c and pkwrite.c fail to link without this echo "#define MBEDTLS_OID_C" >> "$CONFIG_H" # - ASN1_[PARSE/WRITE]_C found by check_config.h for pkparse/pkwrite @@ -2925,8 +2918,6 @@ common_tfm_config () { # # Enable filesystem I/O for the benefit of PK parse/write tests. echo "#define MBEDTLS_FS_IO" >> "$CONFIG_H" - # Disable this for maximal ASan efficiency - scripts/config.py unset MBEDTLS_MEMORY_BUFFER_ALLOC_C # Config adjustments for features that are not supported # when using only drivers / by p256-m From 382966d1a75021a8ef337e669468847ffb3617d1 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Thu, 21 Sep 2023 17:18:49 +0800 Subject: [PATCH 11/22] all.sh: fix a comment in common_tfm_config Signed-off-by: Yanray Wang --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 1e92d6ff0b..85fadf4f8d 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2922,7 +2922,7 @@ common_tfm_config () { # Config adjustments for features that are not supported # when using only drivers / by p256-m # - # Disable all the features that auto-enable ECP_LIGHT (see build_info.h) + # Disable all the features that auto-enable ECP_LIGHT (see config_adjust_legacy_crypto.h) scripts/config.py -f "$CRYPTO_CONFIG_H" unset PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_DERIVE # Disable deterministic ECDSA as p256-m only does randomized scripts/config.py -f "$CRYPTO_CONFIG_H" unset PSA_WANT_ALG_DETERMINISTIC_ECDSA From 09f9300c013ed754653a315500ac4342a562ec0b Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Mon, 25 Sep 2023 10:54:24 +0800 Subject: [PATCH 12/22] config-tfm.h: remove PK_[PARSE/WRITE]_C As we have removed PK_[PARSE_WRITE]_C in TF-M config, we do not have to undef them in config-tfm.h Signed-off-by: Yanray Wang --- configs/config-tfm.h | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index dab13814e9..b8233e9005 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -42,17 +42,6 @@ */ #undef MBEDTLS_AES_SETKEY_DEC_ALT #undef MBEDTLS_AES_DECRYPT_ALT -/* The configuration we have enables MBEDTLS_PK_PARSE_C and MBEDTLS_PK_WRITE_C - * but not MBEDTLS_OID_C. This is inconsistent, and leads to a link error - * when using one of the mbedtls_pk_parse_xxx or mbedtls_pk_write_xxx - * functions that depend on an mbedtls_oid_xxx function. - * Mbed TLS needs PK parse/write for RSA with PSA, but the medium - * profile doesn't have RSA. Later versions of TF-M no longer enable - * PK parse/write: it wasn't a wanted feature. So disable it here - * (otherwise we'd have to enable MBEDTLS_OID_C). - */ -#undef MBEDTLS_PK_PARSE_C -#undef MBEDTLS_PK_WRITE_C /* Use built-in platform entropy functions (TF-M provides its own). */ #undef MBEDTLS_NO_PLATFORM_ENTROPY From 4eaf5adda995e4296eefccce6e4f93254f300c37 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Mon, 25 Sep 2023 10:57:16 +0800 Subject: [PATCH 13/22] all.sh: remove define MD_C in common_tfm_config We have set MBEDTLS_MD_C in tfm_mbedcrypto_config_profile_medium.h so there is no need to enable it again. Signed-off-by: Yanray Wang --- tests/scripts/all.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 85fadf4f8d..88d7414544 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2910,8 +2910,6 @@ common_tfm_config () { # - ASN1_[PARSE/WRITE]_C found by check_config.h for pkparse/pkwrite echo "#define MBEDTLS_ASN1_PARSE_C" >> "$CONFIG_H" echo "#define MBEDTLS_ASN1_WRITE_C" >> "$CONFIG_H" - # - MD_C for HKDF_C - echo "#define MBEDTLS_MD_C" >> "$CONFIG_H" # Config adjustments for better test coverage in our environment. # These are not needed just to build and pass tests. From 145bb2946e0d18448caab07300fd34be7614fa0d Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Mon, 25 Sep 2023 11:10:25 +0800 Subject: [PATCH 14/22] check_config: add check of ASN1_[WRITE/PARSE]_C This commit adds dependency check when PK_CAN_ECDSA_SIGN or PK_CAN_ECDSA_VERIFY is enabled but no corresponding ASN1_WRITE_C or ASN1_PARSE_C is enabled under PSA. Signed-off-by: Yanray Wang --- include/mbedtls/check_config.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 17eb0340cf..5c96223d73 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -106,6 +106,15 @@ #error "MBEDTLS_ECDSA_C defined, but not all prerequisites" #endif +#if defined(MBEDTLS_PK_C) && defined(MBEDTLS_USE_PSA_CRYPTO) +#if defined(MBEDTLS_PK_CAN_ECDSA_SIGN) && !defined(MBEDTLS_ASN1_WRITE_C) +#error "MBEDTLS_PK_C with MBEDTLS_USE_PSA_CRYPTO needs MBEDTLS_ASN1_WRITE_C for ECDSA signature" +#endif +#if defined(MBEDTLS_PK_CAN_ECDSA_VERIFY) && !defined(MBEDTLS_ASN1_PARSE_C) +#error "MBEDTLS_PK_C with MBEDTLS_USE_PSA_CRYPTO needs MBEDTLS_ASN1_PARSE_C for ECDSA verification" +#endif +#endif /* MBEDTLS_PK_C && MBEDTLS_USE_PSA_CRYPTO */ + #if defined(MBEDTLS_ECJPAKE_C) && \ ( !defined(MBEDTLS_ECP_C) || \ !( defined(MBEDTLS_MD_C) || defined(MBEDTLS_PSA_CRYPTO_C) ) ) From 73bb2318785b997ad0ba729c512d5d97b5f3205c Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Mon, 25 Sep 2023 11:21:15 +0800 Subject: [PATCH 15/22] all.sh: remove not needed #define in common_tfm_config Since we have removed PK_C, PK_[WRITE/PARSE]_C, there is no need to define PK related configurations again. Therefore we removed them in common_tfm_config to make a simpler. Signed-off-by: Yanray Wang --- tests/scripts/all.sh | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 88d7414544..0557ee3594 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2897,22 +2897,14 @@ common_tfm_config () { echo "#undef MBEDTLS_PSA_CRYPTO_CONFIG_FILE" >> "$CONFIG_H" cp configs/ext/crypto_config_profile_medium.h "$CRYPTO_CONFIG_H" - # Other config adjustments to make the tests pass. - # Those should probably be adopted upstream. + # Other config adjustment to make the tests pass. + # This should probably be adopted upstream. # # - USE_PSA_CRYPTO for PK_HAVE_ECC_KEYS echo "#define MBEDTLS_USE_PSA_CRYPTO" >> "$CONFIG_H" - # PK_[PARSE/WRITE]_C used to avoid build and link errors in test_suite_pk.c - echo "#define MBEDTLS_PK_PARSE_C" >> "$CONFIG_H" - echo "#define MBEDTLS_PK_WRITE_C" >> "$CONFIG_H" - # pkparse.c and pkwrite.c fail to link without this - echo "#define MBEDTLS_OID_C" >> "$CONFIG_H" - # - ASN1_[PARSE/WRITE]_C found by check_config.h for pkparse/pkwrite - echo "#define MBEDTLS_ASN1_PARSE_C" >> "$CONFIG_H" - echo "#define MBEDTLS_ASN1_WRITE_C" >> "$CONFIG_H" - # Config adjustments for better test coverage in our environment. - # These are not needed just to build and pass tests. + # Config adjustment for better test coverage in our environment. + # This is not needed just to build and pass tests. # # Enable filesystem I/O for the benefit of PK parse/write tests. echo "#define MBEDTLS_FS_IO" >> "$CONFIG_H" @@ -2924,7 +2916,6 @@ common_tfm_config () { scripts/config.py -f "$CRYPTO_CONFIG_H" unset PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_DERIVE # Disable deterministic ECDSA as p256-m only does randomized scripts/config.py -f "$CRYPTO_CONFIG_H" unset PSA_WANT_ALG_DETERMINISTIC_ECDSA - } # Keep this in sync with component_test_tfm_config() as they are both meant From 61f96608ccf71b8314b9d85b5ae7953f8ba00b5a Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Mon, 25 Sep 2023 14:13:22 +0800 Subject: [PATCH 16/22] test_suite_pk: add extra dependency for pk_psa_sign pk_psa_sign is guarded by MBEDTLS_TEST_PK_PSA_SIGN which is set under: - The build has PK_[PARSE/WRITE]_C for RSA or ECDSA signature. - The build has built-in ECC and ECDSA signature. Signed-off-by: Yanray Wang --- tests/suites/test_suite_pk.function | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 881429c2d1..fa0b03b343 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -24,6 +24,17 @@ #define RSA_KEY_SIZE MBEDTLS_RSA_GEN_KEY_MIN_BITS #define RSA_KEY_LEN (MBEDTLS_RSA_GEN_KEY_MIN_BITS/8) +/* MBEDTLS_TEST_PK_PSA_SIGN is enabled when: + * - The build has PK_[PARSE/WRITE]_C for RSA or ECDSA signature. + * - The build has built-in ECC and ECDSA signature. + */ +#if (defined(MBEDTLS_PK_PARSE_C) && defined(MBEDTLS_PK_WRITE_C) && \ + ((defined(MBEDTLS_RSA_C) && defined(MBEDTLS_GENPRIME)) || \ + defined(MBEDTLS_PK_CAN_ECDSA_SIGN))) || \ + (defined(MBEDTLS_ECP_C) && defined(MBEDTLS_PK_CAN_ECDSA_SIGN)) +#define MBEDTLS_TEST_PK_PSA_SIGN +#endif + #if defined(MBEDTLS_PK_USE_PSA_EC_DATA) static int pk_genkey_ec(mbedtls_pk_context *pk, mbedtls_ecp_group_id grp_id) { @@ -1274,7 +1285,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_USE_PSA_CRYPTO */ +/* BEGIN_CASE depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_USE_PSA_CRYPTO:MBEDTLS_TEST_PK_PSA_SIGN */ void pk_psa_sign(int parameter_arg, int psa_type_arg, int expected_bits_arg) { From 079b3bb97b4c1a769fdc517340e73d1eff45bcbb Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Tue, 26 Sep 2023 12:19:15 +0800 Subject: [PATCH 17/22] test_suite_asn1parse.data: remove {} in test data description In analyze_outcomes.py, if a test case passes in reference_test but not in driver_test, we log the key by key.format in python. However, this causes error because of the grammar {} in python string format. So removing {} to avoid KeyError for sys.stderr.write((fmt + '\n').format(*args, **kwargs)) Signed-off-by: Yanray Wang --- tests/suites/test_suite_asn1parse.data | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_asn1parse.data b/tests/suites/test_suite_asn1parse.data index c129e3c8f7..4b13729623 100644 --- a/tests/suites/test_suite_asn1parse.data +++ b/tests/suites/test_suite_asn1parse.data @@ -514,13 +514,13 @@ traverse_sequence_of:"300705000203123456":0:0:0xff:0x02:"6,0x02,3":0 Traverse SEQUENCE of INTEGER, skip everything traverse_sequence_of:"30080203123456020178":0xff:0x02:0:1:"":0 -Traverse SEQUENCE of {NULL, OCTET STRING}, skip NULL: OS, NULL +Traverse SEQUENCE of NULL, OCTET STRING, skip NULL: OS, NULL traverse_sequence_of:"300704031234560500":0xfe:0x04:0xff:0x04:"4,0x04,3":0 -Traverse SEQUENCE of {NULL, OCTET STRING}, skip NULL: NULL, OS +Traverse SEQUENCE of NULL, OCTET STRING, skip NULL: NULL, OS traverse_sequence_of:"300705000403123456":0xfe:0x04:0xff:0x04:"6,0x04,3":0 -Traverse SEQUENCE of {NULL, OCTET STRING}, skip everything +Traverse SEQUENCE of NULL, OCTET STRING, skip everything traverse_sequence_of:"300705000403123456":0xfe:0x04:0:1:"":0 Traverse SEQUENCE of INTEGER, stop at 0: NULL From ffbdd33f043534016fe562d2552c29df71de15bb Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Tue, 26 Sep 2023 15:16:30 +0800 Subject: [PATCH 18/22] Revert "test_suite_asn1parse.data: remove {} in test data description" This reverts commit 929311e9a7c092b54a05d84bc74daa8efdb07422. Signed-off-by: Yanray Wang --- tests/suites/test_suite_asn1parse.data | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_asn1parse.data b/tests/suites/test_suite_asn1parse.data index 4b13729623..c129e3c8f7 100644 --- a/tests/suites/test_suite_asn1parse.data +++ b/tests/suites/test_suite_asn1parse.data @@ -514,13 +514,13 @@ traverse_sequence_of:"300705000203123456":0:0:0xff:0x02:"6,0x02,3":0 Traverse SEQUENCE of INTEGER, skip everything traverse_sequence_of:"30080203123456020178":0xff:0x02:0:1:"":0 -Traverse SEQUENCE of NULL, OCTET STRING, skip NULL: OS, NULL +Traverse SEQUENCE of {NULL, OCTET STRING}, skip NULL: OS, NULL traverse_sequence_of:"300704031234560500":0xfe:0x04:0xff:0x04:"4,0x04,3":0 -Traverse SEQUENCE of NULL, OCTET STRING, skip NULL: NULL, OS +Traverse SEQUENCE of {NULL, OCTET STRING}, skip NULL: NULL, OS traverse_sequence_of:"300705000403123456":0xfe:0x04:0xff:0x04:"6,0x04,3":0 -Traverse SEQUENCE of NULL, OCTET STRING, skip everything +Traverse SEQUENCE of {NULL, OCTET STRING}, skip everything traverse_sequence_of:"300705000403123456":0xfe:0x04:0:1:"":0 Traverse SEQUENCE of INTEGER, stop at 0: NULL From 0e319ae5776f1c03617a1f78cbc2a6337f81b4f6 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Tue, 26 Sep 2023 16:19:18 +0800 Subject: [PATCH 19/22] analyze_outcomes: escape {} in string format for test description {} are valid characters in test description, but they're not escaped properly in python string format(). To resolve the bug of KeyError when it tries to log test description which contains {}, we replace {XXX} format with {{XXX}} in order to escape {} in python string format() properly. In addition, the calls to Results.log() are also handled to avoid similar potential problems. Signed-off-by: Yanray Wang --- tests/scripts/analyze_outcomes.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 119dbb57a5..085bff2f26 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -60,13 +60,13 @@ def execute_reference_driver_tests(ref_component, driver_component, outcome_file # If the outcome file already exists, we assume that the user wants to # perform the comparison analysis again without repeating the tests. if os.path.exists(outcome_file): - Results.log("Outcome file (" + outcome_file + ") already exists. " + \ - "Tests will be skipped.") + Results.log("Outcome file {} already exists. Tests will be skipped.", + outcome_file) return shell_command = "tests/scripts/all.sh --outcome-file " + outcome_file + \ " " + ref_component + " " + driver_component - Results.log("Running: " + shell_command) + Results.log("Running: {}", shell_command) ret_val = subprocess.run(shell_command.split(), check=False).returncode if ret_val != 0: @@ -101,6 +101,7 @@ def analyze_driver_vs_reference(outcomes, component_ref, component_driver, """ available = check_test_cases.collect_available_test_cases() result = True + escape_curly_brace = lambda x: x.replace('{', '{{').replace('}', '}}') for key in available: # Continue if test was not executed by any component @@ -125,7 +126,7 @@ def analyze_driver_vs_reference(outcomes, component_ref, component_driver, if component_ref in entry: reference_test_passed = True if(reference_test_passed and not driver_test_passed): - Results.log(key) + Results.log(escape_curly_brace(key)) result = False return result @@ -172,8 +173,8 @@ def do_analyze_driver_vs_reference(outcome_file, args): ignored_suites = ['test_suite_' + x for x in args['ignored_suites']] outcomes = read_outcome_file(outcome_file) - Results.log("\n*** Analyze driver {} vs reference {} ***\n".format( - args['component_driver'], args['component_ref'])) + Results.log("\n*** Analyze driver {} vs reference {} ***\n", + args['component_driver'], args['component_ref']) return analyze_driver_vs_reference(outcomes, args['component_ref'], args['component_driver'], ignored_suites, args['ignored_tests']) @@ -652,7 +653,7 @@ def main(): for task in tasks: if task not in TASKS: - Results.log('Error: invalid task: {}'.format(task)) + Results.log('Error: invalid task: {}', task) sys.exit(1) TASKS['analyze_coverage']['args']['full_coverage'] = \ From 5c0c858026189b106864b2763fb3f1884286b580 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Tue, 26 Sep 2023 16:52:33 +0800 Subject: [PATCH 20/22] analyze_outcomes: ignore asn1parse and asn1write in result analysis By default, we disable ASN1_[PARSE/WRITE]_C in common_tfm_config. In fact, this is what happens for accelerated p256m driver, which means all asn1[parse/write] tests are skipped in driver_accel test. However, those two macros are automatically enabled for built-in ECDSA via PSA, which means all asn1[parse/write] tests are passed in tfm_config test. This commit simply ignores the whole asn1[parse/write] test suite when analyzing between driver and reference. Signed-off-by: Yanray Wang --- tests/scripts/analyze_outcomes.py | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 085bff2f26..0c63eff4f0 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -530,6 +530,8 @@ TASKS = { 'ignored_suites': [ # Ignore test suites for the modules that are disabled in the # accelerated test case. + 'asn1parse', + 'asn1write', 'ecp', 'ecdsa', 'ecdh', @@ -593,28 +595,6 @@ TASKS = { 'test_suite_psa_crypto_pake': [ 'PSA PAKE: ecjpake size macros', ], - 'test_suite_asn1parse': [ - # This test depends on BIGNUM_C - 'INTEGER too large for mpi', - ], - 'test_suite_asn1write': [ - # Following tests depends on BIGNUM_C - 'ASN.1 Write mpi 0 (1 limb)', - 'ASN.1 Write mpi 0 (null)', - 'ASN.1 Write mpi 0x100', - 'ASN.1 Write mpi 0x7f', - 'ASN.1 Write mpi 0x7f with leading 0 limb', - 'ASN.1 Write mpi 0x80', - 'ASN.1 Write mpi 0x80 with leading 0 limb', - 'ASN.1 Write mpi 0xff', - 'ASN.1 Write mpi 1', - 'ASN.1 Write mpi, 127*8 bits', - 'ASN.1 Write mpi, 127*8+1 bits', - 'ASN.1 Write mpi, 127*8-1 bits', - 'ASN.1 Write mpi, 255*8 bits', - 'ASN.1 Write mpi, 255*8-1 bits', - 'ASN.1 Write mpi, 256*8-1 bits', - ], } } } From 89c88bb44bf2b4e35ecce38bd38e502ed6cebbf5 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Wed, 27 Sep 2023 10:31:44 +0800 Subject: [PATCH 21/22] analyze_outcomes: fix incorrect use of Results.log() Signed-off-by: Yanray Wang --- tests/scripts/analyze_outcomes.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 0c63eff4f0..48457e67a2 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -101,7 +101,6 @@ def analyze_driver_vs_reference(outcomes, component_ref, component_driver, """ available = check_test_cases.collect_available_test_cases() result = True - escape_curly_brace = lambda x: x.replace('{', '{{').replace('}', '}}') for key in available: # Continue if test was not executed by any component @@ -126,7 +125,7 @@ def analyze_driver_vs_reference(outcomes, component_ref, component_driver, if component_ref in entry: reference_test_passed = True if(reference_test_passed and not driver_test_passed): - Results.log(escape_curly_brace(key)) + Results.log('{}', key) result = False return result @@ -621,7 +620,7 @@ def main(): if options.list: for task in TASKS: - Results.log(task) + Results.log('{}', task) sys.exit(0) result = True From 8636d471b3db1294518ab2ab5016914cb5fe59e8 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Wed, 8 Nov 2023 10:07:01 +0800 Subject: [PATCH 22/22] config-tfm.h: License Change Signed-off-by: Yanray Wang --- configs/config-tfm.h | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/configs/config-tfm.h b/configs/config-tfm.h index b8233e9005..191e4c4f41 100644 --- a/configs/config-tfm.h +++ b/configs/config-tfm.h @@ -5,19 +5,7 @@ */ /* * 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. + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later */ /* TF-M medium profile: mbedtls legacy configuration */