Framework to generate config checks: specification #184

gilles-peskine-arm · 2025-07-14T20:03:19Z

Specify a framework for generated config checks. This will let us easily have a large set of complaints about bad configuration options. The reason to do this now is that the upcoming release of TF-PSA-Crypto 1.0 and Mbed TLS 4.0 will create two scenarios where users upgrading from 3.6 are likely to make mistakes that would result in a non-working library:

Keep using an option that is now internal, typically a legacy crypto option.
Keep a crypto option in the mbedtls config.

Specification for Mbed-TLS/mbedtls#10147.

Discuss why some configurations are bad.
Overview of the implementation structure.
Overview of the tests.

Implementation: #164, Mbed-TLS/TF-PSA-Crypto#269, TODO

PR checklist

TF-PSA-Crypto PR not required because: doc only
development PR not required because: doc only
3.6 PR not required because: doc only

Discuss why some configurations are undesirable. Document the current `check_config.h`. Document new generated checks: why and how, but not the details of what (RTFS). Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Now that they aren't public headers any longer, there's no reason to have a different mechanism. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

mpg

I just completed my first pass. This was a very fragmented read, so I'll need a 2nd pass to get a better overall picture, but I'm releasing the comments I have so far in the meantime.

docs/architecture/config-check-framework.md

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

There was a section about this, but the idea was missing in the introductory section. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

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

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

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

mpg

LGTM, thanks!

…bout config symbols Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

mpg

LGTM, thanks for the addition!

minosgalanakis · 2025-09-16T15:46:27Z

docs/architecture/config-check-framework.md

+
+* Setting a superproject option identically in the subproject and in the superproject is perfectly fine.
+* Setting a superproject option only in the subproject is maybe a little weird, but harmless.
+* Setting a superproject option to different values in the superproject and in a subproject has a well-defined effect, but is probably a mistake. (Note that for boolean options, setting to off is usually indistinguishable from not setting, but can technically be done with `#undef`.)


I feel that this is something that needs to be more clearly spelled out in its own section.

What do we define as configuration enabled, from 4.0.0 onwards. And also the difference between boolean options and value based ones, as well as the value based acting like boolean e.g PSA_WANT_ etc.

We need to make it very clear for the users since the configuration options are their primary means to consume the library.

I feel that this is something that needs to be more clearly spelled out in its own section.

I'm sorry, I don't understand. What does “this” refer to?

We need to make it very clear for the users

I'm not sure what “it” is, but in any case, the document here is an internal architectural document, not something users are expected to read. So whatever users need to know needs to go into a different document.

This refers to the type of config options we have. boolean, with value and withh boolean value. If that is meant for internal consumption as a design review document my comment can be disregarded. thanks

minosgalanakis · 2025-09-16T15:47:59Z

docs/architecture/config-check-framework.md

+* Setting a superproject option identically in the subproject and in the superproject is perfectly fine.
+* Setting a superproject option only in the subproject is maybe a little weird, but harmless.
+* Setting a superproject option to different values in the superproject and in a subproject has a well-defined effect, but is probably a mistake. (Note that for boolean options, setting to off is usually indistinguishable from not setting, but can technically be done with `#undef`.)
+* Setting a subproject option in the superproject, except when it was already set (to the same value if applicable), is dangerous. It won't enable the feature and may lead to undefined behavior in the superproject.


Should be illegal, and we cannot make it so now, we can through a warning until the offending code has been removed as we polish the post-split codebases.

Mbed-TLS/mbedtls#10306 triggers errors if you try to set a crypto (subproject) option in the mbedtls (superproject) config file.

I haven't reached a clean CI yet, but I don't expect problems for this particular case: the config.py split makes it happen naturally. We've also split our few hard-coded config files.

minosgalanakis · 2025-09-16T15:50:55Z

docs/architecture/config-check-framework.md

+* Setting a subproject option in the superproject, except when it was already set (to the same value if applicable), is dangerous. It won't enable the feature and may lead to undefined behavior in the superproject.
+* It is also dangerous to set or unset a macro in the superproject's configuration if that macro is a derived one in the subproject.
+
+In Mbed TLS 4.x, there is a high risk that users migrating from Mbed TLS 3.6 will accidentally leave a crypto option in `mbedtls_config.h` instead of moving it to `crypto_config.h`. So we should particularly try to handle this case: macros that were configuration options in Mbed TLS 3.6 and that are now a configuration option or a derived macro in TF-PSA-Crypto.


So we should particularly try to handle

Maybe:

So we should particularly try totry to detect and account for this case?

minosgalanakis · 2025-09-16T15:53:20Z

docs/architecture/config-check-framework.md

+
+In Mbed TLS 2.x, there were manually written config checks in `<mbedtls/check_config.h>`. It was the job of `mbedtls/config.h` (which could be overridden by the user) to include that file. The configuration checks were thus performed whenever building a library source file or an application source file.
+
+In Mbed TLS 3.x, there were manually written config checks in `<mbedtls/check_config.h>`. This header was systematically included by `<mbedtls/build_info.h>`. The configuration checks were thus performed whenever building a library source file or an application source file.


We are now in Mbed TLS 4.x category. Should we add a line as to what we aim to do moving forward with the manually written config_checks?

I don't have any particular post-4.0 plans related to manually written config checks.

minosgalanakis · 2025-09-16T15:59:42Z

docs/architecture/config-check-framework.md

+
+Users who are upgrading from a previous major version of Mbed TLS (or TF-PSA-Crypto, once TF-PSA-Crypto 2.0 is released) may still attempt to use removed options, if they haven't fully upgraded their configuration file. This will likely not have the intended effect. Worse, if a former option has become an [internal macro that is now derived from other options](#derived-macros), this may result in an inconsistent configuration. This is notably the case for legacy crypto options from Mbed TLS 3.6, which for the most part are internal macros in TF-PSA-Crypto 1.0.
+
+A configuration file may use removed options harmlessly if the configuration was intended to work with both the old and the new version of the library. Users who want to do that can make the definitions conditional on the library version (e.g. `#if defined(TF_PSA_CRYPTO_VERSION_MAJOR) && TF_PSA_CRYPTO_VERSION_MAJOR > 1`), so if we have a mechanism to forbid removed options, it is not particularly useful to allow users to bypass it.


Rather than having a mechanism to forbid users, should we consider adding a mechanism that users can express intent to use a removed option? Either a macro or an alias? That way we would have a uniform and controlled way to check and plan around it?

There's a mechanism to bypass the checks completely (I'll add a sentence about it). But I intend that only in case we want to do something weird in a test script.

We already do have a mechanism to allow users to keep using some removed options: keep the option, but document that it is deprecated and has no effect. I don't particularly see a need to use this mechanism, though, since there's a simple workaround.

One exception is options that are now effectively always on. (As in: to get a certain effect, you did #define FOO in 3.x, and you do nothing in 1.x/4.x.) I can think of two: MBEDTLS_USE_PSA_CRYPTO and MBEDTLS_PSA_CRYPTO_CONFIG. I was thinking of keeping them allowed (but I haven't written the corresponding code yet).

gilles-peskine-arm added 2 commits July 14, 2025 21:54

Initial specification document

fe5b50a

Discuss why some configurations are undesirable. Document the current `check_config.h`. Document new generated checks: why and how, but not the details of what (RTFS). Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Do include the generated checks in make generated_files

bb70210

Now that they aren't public headers any longer, there's no reason to have a different mechanism. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

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 labels Jul 14, 2025

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

gilles-peskine-arm added size-s Estimated task size: small (~2d) priority-high High priority - will be reviewed soon labels Jul 14, 2025

github-project-automation bot moved this to In Development in Roadmap pull requests (new board) Jul 14, 2025

This was referenced Jul 14, 2025

Mechanism to error out on removed configuration options #164

Merged

Mechanism to error out on removed configuration options Mbed-TLS/TF-PSA-Crypto#269

Merged

Mechanism to error out on removed configuration options Mbed-TLS/mbedtls#10306

Merged

mpg reviewed Jul 29, 2025

View reviewed changes

gilles-peskine-arm added 4 commits July 29, 2025 11:47

Typos

fa16c76

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

Note that changing a subproject option is bad

97b6288

There was a section about this, but the idea was missing in the introductory section. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Fix formatting inside <code>

04d5012

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

Recall that config checks are an internal matter

7a93b38

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

gilles-peskine-arm requested a review from mpg July 29, 2025 09:51

gilles-peskine-arm added 2 commits July 29, 2025 13:22

Clarify that including *_after.h is not the normal behavior of build_…

5c1da4b

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

We can now have generated checks on the finalized config as well

425ea96

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

mpg previously approved these changes Sep 1, 2025

View reviewed changes

Mention how config checks are generated from historical information a…

b8bebd8

…bout config symbols Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

gilles-peskine-arm dismissed mpg’s stale review via b8bebd8 September 15, 2025 16:07

gilles-peskine-arm requested a review from minosgalanakis September 15, 2025 16:45

mpg approved these changes Sep 16, 2025

View reviewed changes

mpg removed the needs-reviewer This PR needs someone to pick it up for review label Sep 16, 2025

minosgalanakis reviewed Sep 16, 2025

View reviewed changes


		In Mbed TLS 2.x, there were manually written config checks in `<mbedtls/check_config.h>`. It was the job of `mbedtls/config.h` (which could be overridden by the user) to include that file. The configuration checks were thus performed whenever building a library source file or an application source file.

		In Mbed TLS 3.x, there were manually written config checks in `<mbedtls/check_config.h>`. This header was systematically included by `<mbedtls/build_info.h>`. The configuration checks were thus performed whenever building a library source file or an application source file.


		Users who are upgrading from a previous major version of Mbed TLS (or TF-PSA-Crypto, once TF-PSA-Crypto 2.0 is released) may still attempt to use removed options, if they haven't fully upgraded their configuration file. This will likely not have the intended effect. Worse, if a former option has become an [internal macro that is now derived from other options](#derived-macros), this may result in an inconsistent configuration. This is notably the case for legacy crypto options from Mbed TLS 3.6, which for the most part are internal macros in TF-PSA-Crypto 1.0.

		A configuration file may use removed options harmlessly if the configuration was intended to work with both the old and the new version of the library. Users who want to do that can make the definitions conditional on the library version (e.g. `#if defined(TF_PSA_CRYPTO_VERSION_MAJOR) && TF_PSA_CRYPTO_VERSION_MAJOR > 1`), so if we have a mechanism to forbid removed options, it is not particularly useful to allow users to bypass it.

Framework to generate config checks: specification #184

Are you sure you want to change the base?

Framework to generate config checks: specification #184

Uh oh!

Conversation

gilles-peskine-arm commented Jul 14, 2025

PR checklist

Uh oh!

mpg 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!

mpg left a comment

Choose a reason for hiding this comment

Uh oh!

mpg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!