Add crypto configuration components #769

2026-04-20T03:54:50-05:00

ronald-cron-arm commented

2026-04-20 03:54:50 -05:00

(Migrated from github.com)

Description

Add the equivalent of all Mbed TLS components-configuration-crypto.sh but:

the components based on and related to libtestdriver1
the depends.py components (see https://github.com/Mbed-TLS/mbedtls-docs/pull/201)
a few other components, see the commit "Remove various config crypto components"

Only 14 commits to review starting from https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/769/commits/ab99f98dc9c0d8cf08ccc5cb687809d4cfb2270c (merge commit to get the Mbed TLS components-configuration-crypto.sh components and their history into TF-PSA-Crypto).

Fixes #65, #63, #67

PR checklist

changelog not required because: expending testing
framework PR not required
TF-PSA-Crypto development PR provided here
TF-PSA-Crypto 1.1 PR provided https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/854
mbedtls development PR not required because: will remove some components in Mbed TLS components-configuration-crypto.sh independently
mbedtls 4.1 PR not required because: will remove some components in Mbed TLS components-configuration-crypto.sh independently
mbedtls 3.6 PR not required because: n/a
tests provided

## Description Add the equivalent of all Mbed TLS components-configuration-crypto.sh but: - the components based on and related to libtestdriver1 - the depends.py components (see https://github.com/Mbed-TLS/mbedtls-docs/pull/201) - a few other components, see the commit "Remove various config crypto components" Only 14 commits to review starting from https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/769/commits/ab99f98dc9c0d8cf08ccc5cb687809d4cfb2270c (merge commit to get the Mbed TLS components-configuration-crypto.sh components and their history into TF-PSA-Crypto). Fixes #65, #63, #67 ## PR checklist - [x] **changelog** not required because: expending testing - [x] **framework PR** not required - [x] **TF-PSA-Crypto development PR** provided here - [x] **TF-PSA-Crypto 1.1 PR** provided https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/854 - [x] **mbedtls development PR** not required because: will remove some components in Mbed TLS components-configuration-crypto.sh independently - [x] **mbedtls 4.1 PR** not required because: will remove some components in Mbed TLS components-configuration-crypto.sh independently - [x] **mbedtls 3.6 PR** not required because: n/a - **tests** provided

valeriosetti (Migrated from github.com) requested changes

2026-04-29 09:18:43 -05:00

valeriosetti (Migrated from github.com) left a comment

There are also a couple of remaining references to library/libmbedcrypto.a

There are also a couple of remaining references to `library/libmbedcrypto.a`

tests/scripts/components-configuration-crypto.sh

					
				@ -470,0 +512,4 @@

				}

				# check_renamed_symbols HEADER LIB

				# Check that symbols renamed by HEADER are not present in LIB.

This comment was related to the function are_empty_libraries() which has been removed. Should we remove it as well?

This comment was related to the function `are_empty_libraries()` which has been removed. Should we remove it as well?

minosgalanakis (Migrated from github.com) requested changes

2026-05-13 10:43:31 -05:00

minosgalanakis (Migrated from github.com) left a comment

Reviewed the following:

Git history for merging up to TF-PSA-Crypto: 8426c9bc51 -> Looks good and verified locally
The commits range within the range
The changes in analyze_outcomes.py with regards to configuration pruning. I would happier if there was some more context for commit c0ea3da206 because of cases like unsupported-MD5 and ctr_drbg that is it not clear how they are affected by the new components. This PR is undoing some of the changes introduced in ba96751781 by #729
There is an actual bugs in the check_renamed_symbols that will need to be addressed even if we remove the references to the libmbedcrypto.a

Since the CI is happy I am not going to complain about increased coverage

Reviewed the following: - Git history for merging up to TF-PSA-Crypto: 8426c9bc519303ff3cc8598ac2cf31b737e7e1c1 -> Looks good and verified locally - The commits range within the [range](https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/769/changes/ab99f98dc9c0d8cf08ccc5cb687809d4cfb2270c..c0ea3da20676344c1926ef5697b1412d268cae65) - The changes in analyze_outcomes.py with regards to configuration pruning. I would happier if there was some more context for commit c0ea3da20676344c1926ef5697b1412d268cae65 because of cases like `unsupported-MD5` and `ctr_drbg` that is it not clear how they are affected by the new components. This PR is undoing some of the changes introduced in ba96751781e5923b09e2333f16229415199519aa by #729 - There is an actual bugs in the `check_renamed_symbols` that will need to be addressed even if we remove the references to the `libmbedcrypto.a` Since the CI is happy I am not going to complain about increased coverage

tests/scripts/analyze_outcomes.py

Comment: This is not very intuitive as to why those now work with the added components.

tests/scripts/components-configuration-crypto.sh

					
				@ -470,0 +512,4 @@

				}

				# check_renamed_symbols HEADER LIB

				# Check that symbols renamed by HEADER are not present in LIB.

As @valeriosetti I am a bit concerned as to why this was passing the CI to begin with. Looking at the check_renamed_symbols is not working as intended

As @valeriosetti I am a bit concerned as to why this was passing the CI to begin with. Looking at the [check_renamed_symbols](https://github.com/ronald-cron-arm/TF-PSA-Crypto/blob/c0ea3da20676344c1926ef5697b1412d268cae65/tests/scripts/components-configuration-crypto.sh#L566) is not working as intended

This logic has a bug. It was intended to invert the return of grep, and return 1 when it finds no match, but currently nm is failing it's retcode is inverted. We need to split that logic

We should be doing

nm "$2" | sed 's/.* //' | not grep -x -F "$(sed -n 's/^ *# *define *$[A-Z_a-z][0-9A-Z_a-z]*$..*/\1/p' "$1")"

This logic has a bug. It was intended to invert the return of grep, and return 1 when it finds no match, but currently `nm` is failing it's `retcode` is inverted. We need to split that logic We should be doing `nm "$2" | sed 's/.* //' | not grep -x -F "$(sed -n 's/^ *# *define *$[A-Z_a-z][0-9A-Z_a-z]*$..*/\1/p' "$1")"`

ronald-cron-arm (Migrated from github.com) reviewed

2026-07-09 11:39:57 -05:00

tests/scripts/analyze_outcomes.py

If these tests were still not executed, analyze_outcomes.py would complain.

ronald-cron-arm (Migrated from github.com) reviewed

2026-07-10 05:03:25 -05:00

tests/scripts/analyze_outcomes.py

Looking at the CI's outcomes.csv, these test cases are now covered by component_test_ccm_aes_sha256, component_test_tfm_config_as_is ...

ronald-cron-arm commented

2026-07-10 08:14:37 -05:00

(Migrated from github.com)

@valeriosetti @minosgalanakis I believe I have addressed all your comments in the following commits:

Remove leftover comment
Improve check_renamed_symbols() robustness
Fix crypto library path

To resolve a conflict with the current head of development, I then merged tf-psa-crypto/development into this PR. This avoids having to redo the merge of mbedtls/development needed to get the Mbed TLS configuration-crypto components. Note that the merge was done with commit 8426c9bc from mbedtls/development, and tests/scripts/components-configuration-crypto.sh is unchanged in the current head of mbedtls/development.

Finally, after merging tf-psa-crypto/development, some additional adjustments were needed to fully resolve the conflict and get CI passing again (3 commits).

I hope this helps with your re-review. It would be good if you could give the re-review some priority, as this PR may bit-rot quite quickly, essentially as soon as we change something in components-configuration-crypto.sh.

@valeriosetti @minosgalanakis I believe I have addressed all your comments in the following commits: * Remove leftover comment * Improve `check_renamed_symbols()` robustness * Fix crypto library path To resolve a conflict with the current head of `development`, I then merged `tf-psa-crypto/development` into this PR. This avoids having to redo the merge of `mbedtls/development` needed to get the Mbed TLS configuration-crypto components. Note that the merge was done with commit `8426c9bc` from `mbedtls/development`, and `tests/scripts/components-configuration-crypto.sh` is unchanged in the current head of `mbedtls/development`. Finally, after merging `tf-psa-crypto/development`, some additional adjustments were needed to fully resolve the conflict and get CI passing again (3 commits). I hope this helps with your re-review. It would be good if you could give the re-review some priority, as this PR may bit-rot quite quickly, essentially as soon as we change something in `components-configuration-crypto.sh`.

minosgalanakis (Migrated from github.com) reviewed

2026-07-20 05:57:57 -05:00

minosgalanakis (Migrated from github.com) left a comment

No description provided.

LTGM thanks. Confirmed the history, addressing the previous review comments, and the introduced code changes.

valeriosetti (Migrated from github.com) reviewed

2026-07-27 08:47:18 -05:00

gilles-peskine-arm (Migrated from github.com) reviewed

2026-07-29 09:48:56 -05:00

gilles-peskine-arm (Migrated from github.com) left a comment

I noticed a mistake when making the 1.1 backport. Since Ronald is away, I'll push a commit here (cherry-picked from the 1.1 PR).

tests/scripts/components-configuration-crypto.sh

					
				@ -467,6 +468,916 @@ component_test_accel_hash () {

				    ctest

component_full_no_pkparse_pkwrite and component_full_no_pkwrite already exist in components-configuration.sh under the names component_tf_psa_crypto_full_no_pkparse_pkwrite and component_tf_psa_crypto_full_no_pkwrite.

`component_full_no_pkparse_pkwrite` and `component_full_no_pkwrite` already exist in `components-configuration.sh` under the names `component_tf_psa_crypto_full_no_pkparse_pkwrite` and `component_tf_psa_crypto_full_no_pkwrite`.