Clean up build_info.h after 1.0 by gilles-peskine-arm · Pull Request #571 · Mbed-TLS/TF-PSA-Crypto

gilles-peskine-arm · 2025-11-10T19:28:15Z

Clean up some things in build_info.h and the header that it includes:

Resolves Cleanup after mechanism for marking a compilation option as private mbedtls#10305.
Move config adjustment headers to .../private — this had only been partially done in 1.0.
Sort out which config adjustments belong under /include and which ones belong under /drivers/builtin/include.
Rename config adjustment headers to follow a more systematic naming scheme, and to better reflect their role in a world where the user configuration is only driven by PSA.

Needs preceding PR: framework Mbed-TLS/mbedtls-framework#236

I used (and even wrote) Mbed-TLS/mbedtls-docs#193 to check for mistakes when reordering parts of build_info.h.

PR checklist

changelog provided | not required because: only internal changes
framework PR provided Clean up after 1.0: generate_config_checks.py, tf-psa-crypto/build_info.h mbedtls-framework#236
mbedtls development PR not required because: only internal changes in crypto
mbedtls 3.6 PR not required because: new stuff
tests provided

ronald-cron-arm

Thanks for this nice clean-up. Thanks for the commit message that helped a lot reviewing the changes. I have some minor comments/questions.

ronald-cron-arm · 2025-11-20T13:29:44Z

One minor comment remaining. Otherwise it looks good to me.

ronald-cron-arm

LGTM, thanks.

minosgalanakis

LGTM

…nyms.h Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Individual headers or C files are supposed to include what they need. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

This is changing PSA configurations, based on conditions that are not driver-specific, so keep it in `/include`. The file has guards based on whether some key derivation algorithms are accelerated, but these guards are currently irrelevant since key derivation drivers are not supported yet. This is not meant to be included directly by applications, so move it under `.../private`. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

This is at the PSA level, not driver-specific, so keep it in `/include`. This is not meant to be included directly by applications, so move it under `.../private`. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

This commit changes the location of the file in the inclusion order, to group it with other headers that tweak the PSA configuration. The former order also worked because the macros that it references aren't used in other configuration-tweaking headers at this time (and have never been, I believe), so this is not a bug fix, just a readability and robustness improvement. This is at the PSA level, not driver-specific, so keep it in `/include`. This is not meant to be included directly by applications, so move it under `.../private`. Rename the file to match the pattern followed by similar files (I had forgotten a word when I created that file initially). Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

This commit changes the location of the file in the inclusion order, to group it with other headers that tweak the PSA configuration. Specificaly, this header is now included before `config_adjust_legacy_from_psa.h` and `config_adjust_test_accelerators.h`. Both of these set driver-specific macros based on `PSA_WANT` macros, whereas `crypto_adjust_config_derived.h` does not query driver-specific macros, so the new order is no worse. In fact the new order more correct because it would make sense for `config_adjust_legacy_from_psa.h` to query macros set by `crypto_adjust_config_derived.h`. However, it currently doesn't, so this is not a bug fix, just a robustness improvement. This is at the PSA level, not driver-specific, so keep it in `/include`. This is not meant to be included directly by applications, so move it under `.../private`. Rename the file to match the pattern followed by similar files (I had forgotten a word when I created that file initially). Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Historically, we did config ajustments in a separate header `config_psa.h`. Then we split `config_psa.h` into pieces. Today, having `config_psa.h` as a layer of indirection in header inclusions is no longer useful. So remove it. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Rename the file to match the pattern followed by similar files, and to better reflect the role of the file in the context of TF-PSA-Crypto. It's no longer about enabling legacy interfaces, since low-level crypto is mostly no longer part of a public interface, but about enabling internal interfaces that implement actual cryptography. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

This is specific to modules that are under `/drivers`, so keep it under `/drivers`. Some of these modules are also public interfaces that are used in builds with only third-party drivers, so the corresponding macros should be outside of `/drivers`; this will be handled in a subsequent commit. This is not meant to be included directly by applications, so move it under `.../private`. Rename the file to match the pattern followed by similar files, and to better reflect its current role: it's tweaking modules that implement the built-in cryptography, which were formerly legacy public interfaces but are now internal. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

This one comes after both the PSA feature set and the feature set of built-in crypto have been determined. This commit creates an empty file. It will be populated in a subsequent commit. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Split out the parts of `crypto_adjust_config_tweak_builtins.h` that are actually about generic support modules (platform, ASN.1, MD, ...), and move them outside `/drivers`, into `crypto_adjust_config_support.h`. One macro only depends on PSA options, so move it to `crypto_adjust_config_derived.h` instead. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Install all headers without hard-coding which sub(sub)directories exist. Don't use file(GLOB), following CMake best practices (the list might not be updated if a source file is added or removed when the build tree already exists). It isn't actually useful anyway. To keep things simple, don't permissions: we just want the default permissions. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

This was done too late, so it could have led to mysterious build errors if PKCS5 was the only thing that requires MD. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

We added `#error` directives to prevent some headers from being included directly because those headers were not clearly indicated as public (and, in the case of `check_config.h`, were formerly public). Headers under `.../private` have no history of being public and are obviously not intended for direct consumption, so remove the enforcement. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

gilles-peskine-arm · 2025-11-20T16:43:52Z

I rebased to include the merge of the framework prerequisite and to resolve the framework conflict. The previous version is in https://github.com/gilles-peskine-arm/TF-PSA-Crypto/tree/build_info-cleanup-1.0-4

ronald-cron-arm

Rebase and framework update look good to me.

minosgalanakis

LGTM

gilles-peskine-arm added this to Roadmap pull requests (new board) Nov 10, 2025

gilles-peskine-arm added size-s Estimated task size: small (~2d) priority-high High priority - will be reviewed soon needs-preceding-pr Requires another PR to be merged first needs-ci Needs to pass CI tests labels Nov 10, 2025

github-project-automation Bot moved this to In Development in Roadmap pull requests (new board) Nov 10, 2025

gilles-peskine-arm mentioned this pull request Nov 10, 2025

Clean up after 1.0: generate_config_checks.py, tf-psa-crypto/build_info.h Mbed-TLS/mbedtls-framework#236

Merged

3 tasks

gilles-peskine-arm requested a review from ronald-cron-arm November 10, 2025 19:28

gilles-peskine-arm force-pushed the build_info-cleanup-1.0 branch 4 times, most recently from d353f00 to 1f55061 Compare November 17, 2025 23:01

gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members needs-reviewer This PR needs someone to pick it up for review and removed needs-ci Needs to pass CI tests labels Nov 18, 2025

ronald-cron-arm requested a review from minosgalanakis November 18, 2025 10:58

ronald-cron-arm removed the needs-reviewer This PR needs someone to pick it up for review label Nov 18, 2025

This comment was marked as off-topic.

Sign in to view

ronald-cron-arm self-requested a review November 19, 2025 10:23

ronald-cron-arm requested changes Nov 19, 2025

View reviewed changes

gilles-peskine-arm requested a review from ronald-cron-arm November 19, 2025 18:46

ronald-cron-arm previously approved these changes Nov 20, 2025

View reviewed changes

minosgalanakis previously approved these changes Nov 20, 2025

View reviewed changes

github-project-automation Bot moved this from In Development to Has Approval in Roadmap pull requests (new board) Nov 20, 2025

ronald-cron-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members needs-preceding-pr Requires another PR to be merged first labels Nov 20, 2025

gilles-peskine-arm added 2 commits November 20, 2025 17:42

Update framework supporting new location of crypto_adjust_config_syno…

c8b7ac2

…nyms.h Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Remove obsolete inclusion of limits.h

1023eaf

Individual headers or C files are supposed to include what they need. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

gilles-peskine-arm added 16 commits November 20, 2025 17:43

Move crypto_adjust_config_key_pair_types where it belongs

bcaae20

This is at the PSA level, not driver-specific, so keep it in `/include`. This is not meant to be included directly by applications, so move it under `.../private`. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Define MBEDTLS_MD_LIGHT before checking it

5815c88

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Update references to headers that have been renamed

1cc213c

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Minor comment fixes

365d702

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Fix auto-enabling of MD for PKCS5

5abb0bd

This was done too late, so it could have led to mysterious build errors if PKCS5 was the only thing that requires MD. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Update comments about MBEDTLS_PLATFORM_SNPRINTF_ALT on Windows

d33a85e

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

gilles-peskine-arm dismissed stale reviews from minosgalanakis and ronald-cron-arm via d33a85e November 20, 2025 16:43

gilles-peskine-arm force-pushed the build_info-cleanup-1.0 branch from b2bd8e8 to d33a85e Compare November 20, 2025 16:43

gilles-peskine-arm requested review from minosgalanakis and ronald-cron-arm November 20, 2025 16:43

gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members and removed approved Design and code approved - may be waiting for CI or backports labels Nov 20, 2025

ronald-cron-arm approved these changes Nov 20, 2025

View reviewed changes

minosgalanakis approved these changes Nov 24, 2025

View reviewed changes

ronald-cron-arm added this pull request to the merge queue Nov 24, 2025

Merged via the queue into Mbed-TLS:development with commit 5df23a9 Nov 24, 2025
7 checks passed

github-project-automation Bot moved this from Has Approval to Done in Roadmap pull requests (new board) Nov 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up build_info.h after 1.0#571

Clean up build_info.h after 1.0#571
ronald-cron-arm merged 20 commits into
Mbed-TLS:developmentfrom
gilles-peskine-arm:build_info-cleanup-1.0

gilles-peskine-arm commented Nov 10, 2025 •

edited

Loading

Uh oh!

This comment was marked as off-topic.

Uh oh!

ronald-cron-arm left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ronald-cron-arm commented Nov 20, 2025 •

edited

Loading

Uh oh!

ronald-cron-arm left a comment

Uh oh!

minosgalanakis left a comment

Uh oh!

gilles-peskine-arm commented Nov 20, 2025

Uh oh!

ronald-cron-arm left a comment

Uh oh!

minosgalanakis left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

gilles-peskine-arm commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

This comment was marked as off-topic.

Uh oh!

ronald-cron-arm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ronald-cron-arm commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ronald-cron-arm left a comment

Choose a reason for hiding this comment

Uh oh!

minosgalanakis left a comment

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm commented Nov 20, 2025

Uh oh!

ronald-cron-arm left a comment

Choose a reason for hiding this comment

Uh oh!

minosgalanakis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gilles-peskine-arm commented Nov 10, 2025 •

edited

Loading

ronald-cron-arm commented Nov 20, 2025 •

edited

Loading