Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate *_config.h from default_*_config.h #9664

Open
gilles-peskine-arm opened this issue Oct 2, 2024 · 0 comments
Open

Generate *_config.h from default_*_config.h #9664

gilles-peskine-arm opened this issue Oct 2, 2024 · 0 comments
Labels
enhancement needs-design-approval size-s Estimated task size: small (~2d)

Comments

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented Oct 2, 2024

Preamble

We sometimes struggle with include/mbedtls/mbedtls_config.h due to its triple status:

  • In its checked-in state, it's the default configuration.
  • We modify it to obtain the current configuration. Usually we just comment/uncomment lines, but occasionally we overwrite it completely (e.g. we overwrite it with a file from the configs directory.
  • In its checked-in state, usually in its edited state, but not always when it's overwritten, it's a description of all the options that exist (with options that are off by default indicated by lines that start with //).

In particular, we have various scripts used to generate configuration-independent files that need access to the full list of configuration options. These scripts usually work in a different configuration (with different output that only take into account the options that are present in mbedtls_config.h), but that's not always the case. For example, when we test a configuration from configs, we're careful to run make generated_files first and arrange not to re-generate the files during the library build. Some examples of things that can go wrong:

  • generate_psa_tests.py runs a C compiler. This has to be a host compiler. If mbedtls_config.h is incompatible with the host compiler (e.g. it enables something that only works on Arm, or that requires a specific sanitizer), running it will fail.
  • generate_config_tests.py is incomplete with respect to disabled options if its input doesn't list all the disabled options.
  • make generated_files avoids listing dependencies of generated files on mbedtls_config.h. As a result, during development, when we modify mbedtls_config.h, we sometimes end up with out-of-date generated files because we relied on make generated_files.
  • We may end up with generated files having a dependency on the config files. This can cause them to be re-generated after running config.py, which may cause problems. At this point, we suspect this is the root cause of CI failures on PSA Crypto Configuration Split #9567 in test_memsan_constant_flow_psa and test_memsan_constant_flow.

The same considerations apply to include/psa/crypto_config.h. In the past, it's come up less often because we don't often configure it to do less. But that's coming in post-3.6 development, now that we're enabling MBEDTLS_PSA_CRYPTO_CONFIG everywhere (so the complicated cases no longer ignore crypto_config.h).

Proposal

  • Instead of having mbedtls_config.h and crypto_config.h checked in, have them generated (with a simple file copy) from default_mbedtls_config.h and default_crypto_config.h. That way, we always have a copy of the default configuration in the build tree. But the current config file names remain present and editable, so we don't have to change anything in all.sh, and this doesn't change anything for users (both in releases and from a development branch).
  • Change generate_* scripts to read the default_xxx files instead of the usual files. That way, they behave identically after editing the real config files.

Since this is transparent for users, we should do this in 3.6 as well. (If we don't, we'll have to make some framework scripts more complex.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs-design-approval size-s Estimated task size: small (~2d)
Projects
Status: No status
Development

No branches or pull requests

1 participant