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

Move to an additive configuration mechanism #8107

Open
gilles-peskine-arm opened this issue Aug 22, 2023 · 0 comments
Open

Move to an additive configuration mechanism #8107

gilles-peskine-arm opened this issue Aug 22, 2023 · 0 comments
Labels
api-break This issue/PR breaks the API and must wait for a new major version component-platform Portability layer and build scripts enhancement size-l Estimated task size: large (2w+)

Comments

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented Aug 22, 2023

The configuration mechanism inherited from PolarSSL to specify which modules are included is exact: the user has to list the modules that they want, as well as the dependencies of those modules. This is a problem for users because they have to traverse the dependency tree manually. This is a problem for maintainers because we can't change dependencies since that would break user code. This is also a problem for maintainers because it's harder to construct test configurations, especially when enumerating them automatically (as in depends.py).

Over time, we've deviated from this principle in order to preserve backward compatibility. For example, MBEDTLS_SSL_CLI_C requires MBEDTLS_SSL_TLS_C because ssl_cli.c calls functions in ssl_tls.c. When we split some code from ssl_tls.c into ssl_msg.c, we did not add a configuration option MBEDTLS_SSL_MSG_C because that would have broken every application that has its own config.h: we continued to gate the code in ssl_msg.c with MBEDTLS_SSL_TLS_C. But really, MBEDTLS_SSL_TLS_C should just be an internal symbol defined when defined(MBEDTLS_SSL_CLI_C) || defined(MBEDTLS_SSL_SRV_C), it isn't something the library users should be concerned with.

PSA crypto uses an additive configuration mechanism: the user specifies the features they want (#define PSA_WANT_xxx 1), and it's the library's job to include all the code that's necessary to provide those features. Users don't have to care how the features are implemented.

The goal of this issue is to move all configuration to a new mechanism that's similar to PSA crypto. If A requires B then activating A should activate B under the hood.

This can be done in a minor release, but the new mechanism would have to coexist with the old one, since we can't get rid of the old one. So it's easier to do in a major release.

There will still be a few invalid configurations: when A requires (B1 or B2 or B3), it may be an error to enable A but none of the B's (e.g. enabling TLS but not enough cryptographic mechanisms for any cipher suite). But this should be rare.

Note that feature configuration must rely only on the C preprocessor, because there are prominent users who use tools that edit lists of #define statements in a file in a BSP. We can provide other mechanisms, but list-of-define must work.

@gilles-peskine-arm gilles-peskine-arm added enhancement component-platform Portability layer and build scripts api-break This issue/PR breaks the API and must wait for a new major version size-l Estimated task size: large (2w+) labels Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-break This issue/PR breaks the API and must wait for a new major version component-platform Portability layer and build scripts enhancement size-l Estimated task size: large (2w+)
Projects
Status: No status
Status: No status
Status: Requirements needed
Development

No branches or pull requests

1 participant