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

Study: Make use of PSA non-optional #5156

Closed
mpg opened this issue Nov 12, 2021 · 8 comments
Closed

Study: Make use of PSA non-optional #5156

mpg opened this issue Nov 12, 2021 · 8 comments
Labels
api-break This issue/PR breaks the API and must wait for a new major version enhancement size-l Estimated task size: large (2w+)

Comments

@mpg
Copy link
Contributor

mpg commented Nov 12, 2021

In order to avoid having to maintain two version of the code for each crypto operation in TLS, X.509 and PK, we're making MBEDTLS_USE_PSA_CRYPTO active by default.

Expected result:

  • If any part of MBEDTLS_PK_C, MBEDTLS_X509_USE_C, MBEDTLS_X509_WRITE_C or MBEDTLS_SSL_TLS_C is enabled in the build-time configuration, then ``MBEDTLS_USE_PSA_CRYPTOis automatically enabled, as well as enough ofMBEDTLS_PSA_CRYPTO_C` to support it (see comments below).
  • MBEDTLS_USE_PSA_CRYPTO remains as an option in mbedtls_config.h, in order to avoid breaking compatibility; documentation is updated to reflect that the user value is now ignored and the value is forced as described above.

This task is to study how this could be done while preserving backwards compatibility (see challenges and ideas in the next comment), and make a decision if that's something we want to do - if so, agree on a plan and create a task break down.

@mpg mpg added this to Incoming Items in OBSOLETE - SEE https://github.com/orgs/Mbed-TLS/projects/3 via automation Nov 12, 2021
@mpg mpg mentioned this issue Nov 12, 2021
6 tasks
@mpg mpg added the size-s Estimated task size: small (~2d) label Nov 12, 2021
@mpg mpg added size-m Estimated task size: medium (~1w) and removed size-s Estimated task size: small (~2d) labels Nov 25, 2021
@laumor01 laumor01 moved this from Use PSA Crypto More - Part 1 to Use PSA Crypto more - part 2+ in OBSOLETE - PLEASE SEE https://github.com/orgs/Mbed-TLS/projects/1 Dec 1, 2021
@mpg mpg added size-l Estimated task size: large (2w+) and removed size-m Estimated task size: medium (~1w) labels Dec 2, 2021
@mpg mpg changed the title Make use of PSA non-optional Study: Make use of PSA non-optional Dec 2, 2021
@mpg
Copy link
Contributor Author

mpg commented Dec 2, 2021

This is more complex than initially thought, for two reasons, both related to backwards compatibility:

  1. Existing applications may not be calling psa_crypto_init() before using TLS, X.509 or PK. We can try to work around that by calling (the relevant part of) it ourselves under the hood as needed, but that would likely require splitting init between the parts that can fail and the parts that can't (see https://github.com/ARM-software/psa-crypto-api/pull/536 for that). See Finish the PSA subsystem initialization draft #6006 and Define the PSA subsystem initialization interface in Mbed TLS #6007
  2. It's currently not possible to enable MBEDTLS_PSA_CRYPTO_C in configurations that don't have MBEDTLS_ENTROPY_C, and we can't just auto-enable the latter, as it won't build or work out of the box on all platforms. There are two kinds of things we'd need to do if we want to work around that:
    1. Make it possible to enable the parts of PSA Crypto that don't require an RNG (typically, public key operations, symmetric crypto, some key management functions (destroy etc)) in configurations that don't have ENTROPY_C. This requires going through the PSA code base to adjust dependencies. Risk: there may be annoying dependencies, some of which may be surprising.
    2. For operations that require an RNG, provide an alternative function accepting an explicit f_rng parameter (see PSA: per-operation RNG proof-of-concept #5238), that would be available in entropy-less builds. (Then code using those functions still needs to have one version using it, for entropy-less builds, and one version using the standard function, for driver support in build with entropy.)

@mpg mpg moved this from Use PSA Crypto more - part 2 to Use PSA Crypto more - unplanned in OBSOLETE - PLEASE SEE https://github.com/orgs/Mbed-TLS/projects/1 Mar 11, 2022
@daverodgman daverodgman added this to Use PSA Crypto more - unplanned in EPICs for Mbed TLS Mar 30, 2022
@daverodgman daverodgman added this to Use PSA Crypto more - unplanned in Backlog for Mbed TLS Feb 22, 2023
@daverodgman daverodgman removed this from Use PSA Crypto more - unplanned in EPICs for Mbed TLS Feb 22, 2023
@daverodgman daverodgman moved this from Use PSA Crypto more - unplanned to Mbed TLS 4.0 candidates in Backlog for Mbed TLS Aug 15, 2023
@daverodgman daverodgman moved this from Mbed TLS 4.0 candidates to Mbed TLS 4.0 in Backlog for Mbed TLS Aug 15, 2023
@daverodgman daverodgman moved this from Mbed TLS 4.0 to Mbed TLS 4.0 MUST in Backlog for Mbed TLS Aug 30, 2023
@daverodgman daverodgman added the api-break This issue/PR breaks the API and must wait for a new major version label Sep 1, 2023
@daverodgman
Copy link
Contributor

Does MBEDTLS_USE_PSA_CRYPTO need to remain a user-visible option? It should be possible to simply remove it both from mbedtls_config.h and from the codebase itself (where we would assume it is always set).

In 4.0, at least one of MBEDTLS_PSA_CRYPTO_C / MBEDTLS_PSA_CRYPTO_CLIENT must be set.

@gilles-peskine-arm
Copy link
Contributor

This issue was written a while ago when support for MBEDTLS_USE_PSA_CRYPTO was incomplete. Now it's almost complete and we have a consensus that 4.0 will effectively have MBEDTLS_USE_PSA_CRYPTO always on, with no way to deactivate it.

@daverodgman
Copy link
Contributor

Indeed. I think the first point (split init) is covered by #6006 and #6007. Are you aware of any issues covering what's needed for entropy?

@daverodgman
Copy link
Contributor

Closing in favour of other issues in 4.0 MUST epic

@gilles-peskine-arm
Copy link
Contributor

Entropy is an epic of its own. We have a draft specification (PSA randomness drivers) which we need to implement. If we do it in a minor release then we also have to provide a gradual transition path, if we do it in a major release then we don't and it's a big win.

@daverodgman
Copy link
Contributor

Agreed, I was under the impression that entropy was one of our must-haves for 4.0. I think we will want to do some scoping / initial investigation and define that epic soon, possibly in Q4.

@gilles-peskine-arm
Copy link
Contributor

I was under the impression that entropy was one of our must-haves for 4.0

Having configurable entropy sources is a must-have. Switching to the PSA driver system is not strictly a must-have: we can do it independently of accelerator and secure element drivers. But if we don't do that in 4.0 then we have to carry the current entropy interface over to 4.x, and it's not PSA (marketing won't be happy) and it's cumbersome (engineering won't be happy). So I'd strongly prefer if we decide to make it a must-have for 4.0.

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 enhancement size-l Estimated task size: large (2w+)
Projects
Backlog for Mbed TLS
Mbed TLS 4.0 MUST
Development

No branches or pull requests

4 participants