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

Evolution of md.h in 4.0 #8450

Open
mpg opened this issue Oct 31, 2023 · 7 comments
Open

Evolution of md.h in 4.0 #8450

mpg opened this issue Oct 31, 2023 · 7 comments
Labels
api-break This issue/PR breaks the API and must wait for a new major version component-crypto Crypto primitives and low-level interfaces needs-design-approval

Comments

@mpg
Copy link
Contributor

mpg commented Oct 31, 2023

This issue is meant as a place to discuss what we want to do with MD in 4.0.

There are two main options:

  1. Keep it as part of the public API until 5.0, but aim to make it a thin wrapper around PSA Crypto (in 4.0 or 4.x).
  2. Remove it from the public API in 4.0, and eventually remove it entirely (in 4.0 or 4.x).

Note: MD currently provides functionality similar to two PSA Crypto families: psa_hash_xxx() and psa_mac_xxx() (HMAC). It provides both streaming and one-shot API for each family.

Note: for low-level hash modules (sha256.h etc) I think the plan is to remove them from the public API, regardless of what we do with MD.

Rationale for option 1 (keep)

This gives users more time to move to PSA. People who were using low-level hash modules need to migrate immediately, but those who were using the generic MD API for hashes and HMAC can migrate at a convenient time between now and 5.0.

Work needed for option 1 (keep)

In 4.0:

  • We may still want to remove some parts:
  • We might want to remove mbedtls_md_info_t from the API in order to make it slicker but that would require users to change their code, so it runs against the stated goal. (Alternatively, we can keep the structure but later make it trivial as it has no public field.)
  • We need some investigation (probably including a prototype) to make sure the current HMAC API can be implemented efficiently as a thin wrapper around PSA. (Note: for the hashing API, that was already done as part of driver-only work.)

In 4.0 or 4.x:

  • Actually make it a thin wrapper around PSA crypto
  • Migrate all internal users (library and tests) to use PSA Crypto directly (note: common with option 2).

Rationale for option 2 (remove)

This leaves us with only one hashing/HMAC API to maintain, document and test. For new users, this also gives more clarity.

Work needed for option 2 (remove)

In 4.0:

  • Move md.h to an internal location and adapt all files that #include it.
  • Change or remove all example programs that used it.
  • Provide a migration guide. (Note: we already have a pair of example programs (psa/hmac_demo and hash/md_hmac_demo.c going in that direction, and an example program psa_hash.c.)

In 4.0 or 4.x:

  • Migrate all internal users (library and tests) to use PSA Crypto directly (note: common with option 2).
  • Then remove MD from the code base entirely.

Other options

  • We could go for a mix: keep one part (hash or HMAC) but remove the other. I'm not really seeing reasons to do this, but mentioning it anyway for the sake of completeness.
  • Other?
@gilles-peskine-arm
Copy link
Contributor

The general wisdom for API transitions is to have one full major version cycle where the new API is stable and the old API is present but deprecated. In Mbed TLS 3, we're at an intermediate stage where the new API is stable but old API is not deprecated.

Because hashes are so ubiquitous, and because we already have code in md.c that wraps around PSA, I'm in favor of keeping at least MD light as the md.h of Mbed TLS 4.x. This helps our users, and saves us work for 4.0 (even if we removed md.h from the public API, we'd very likely keep it internally until we have time to update all the modules that use it).

mbedtls_md_info_t can be reduced to a single byte. It's already only 8 bytes on typical archictectures.

I'm not sure what to do about HMAC. I don't think it's hard to implement it on top of PSA. (Note that mbedtls_md_clone doesn't support HMAC, otherwise that would be difficult to implement.) Migrating all the modules that currently call MD and don't have a PSA alternative (hkdf.c, hmac_drbg.c, pkcs5.c) is likely to be more work. So I'd lean towards keeping the MD HMAC interface in 4.x as well.

@mpg
Copy link
Contributor Author

mpg commented Nov 2, 2023

Migrating all the modules that currently call MD and don't have a PSA alternative (hkdf.c, hmac_drbg.c, pkcs5.c) is likely to be more work.

Minor: I don't think HKDF belongs on that list: it does have a PSA alternative, whose built-in implementation (currently in psa_crypto.c) does not depend on hkdf.c. (So, I think as soon as hkdf.h is no longer public, we can remove hkdf.c as well.)

Also, I think migrating internal users of HMAC to PSA is work indeed, but relatively easy work, though.

@mpg
Copy link
Contributor Author

mpg commented Nov 2, 2023

Note (just for reference, I don't think it influences the decision to keep MD or not): currently (with USE_PSA), internal users of MD (hashes or HMAC) are:

  • ecdsa.c
  • ecjpake.c
  • entropy.c
  • hkdf.c (will disappear though, see above)
  • hmac_drbg.c
  • pem.c
  • pk.c, pk_wrap.c
  • pkcs12.c
  • pkcs5.c
  • pkcs7.c
  • psa_crypto.c
  • psa_crypto_ecp.c
  • psa_crypto_rsa.c
  • rsa.c

@mpg
Copy link
Contributor Author

mpg commented Nov 2, 2023

Aw, I think I forgot a work item for "option 2 (remove)": what about uses of types from md.h in other APIs?

  • In TLS: mbedtls_ssl_async_sign_t takes a parameter of type mbedtls_md_type_t
  • In X.509: mbedtls_x509write_crt_set_md_alg() and mbedtls_x509write_csr_set_md_alg() both take a parameter of type mbedtls_md_type_t (there are other such functions in x509.h but they're internal (should be moved: tracked here).

Actually, I say that's a work item for option 2, but that's something we probably still want to do with option 1 anyway.

@gilles-peskine-arm
Copy link
Contributor

what about uses of types from md.h in other APIs?

We definitely need to remove those or provide alternatives. md.h in 4.0 should be a transition path, not something that new applications would have to use.

@daverodgman
Copy link
Contributor

"This gives users more time to move to PSA. People who were using low-level hash modules need to migrate immediately, but those who were using the generic MD API for hashes and HMAC can migrate at a convenient time between now and 5.0."

Users who want to use md.h will of course have support in 3.6 until March 2027. If we provide it in 4.0, we have to offer support until probably 2028.

AFAICT it is very low impact to break these cases. In the X.509 case, it's trivial for the user (pass PSA_ALG_xxx instead of MBEDTLS_MD_xxx). For the TLS callback, it's a bit more work, but not much more.

So I am in favour of removal.

@gilles-peskine-arm gilles-peskine-arm moved this to Requirements needed in Mbed TLS 4.0 planning Jun 5, 2024
@gilles-peskine-arm gilles-peskine-arm added component-crypto Crypto primitives and low-level interfaces api-break This issue/PR breaks the API and must wait for a new major version labels Jun 19, 2024
@gilles-peskine-arm
Copy link
Contributor

Keep it as part of the public API until 5.0, but aim to make it a thin wrapper around PSA Crypto (in 4.0 or 4.x).

That's the plan. The cost of keeping it is small and the cost of tearing out of our code base is large.

@gilles-peskine-arm gilles-peskine-arm changed the title [discussion] The fate of MD in 4.0 Evolution of md in 4.0 Aug 14, 2024
@gilles-peskine-arm gilles-peskine-arm changed the title Evolution of md in 4.0 Evolution of md.h in 4.0 Aug 14, 2024
@yanesca yanesca moved this to 4.0 - Prepare High Level Crypto in Mbed TLS Backlog Aug 27, 2024
@github-project-automation github-project-automation bot moved this to Mbed TLS 4.0 candidates in Backlog for Mbed TLS Aug 30, 2024
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-crypto Crypto primitives and low-level interfaces needs-design-approval
Projects
Status: Mbed TLS 4.0 candidates
Status: Design needed
Status: No status
Development

No branches or pull requests

3 participants