Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.07k stars 2.52k forks source link

[discussion] The fate of PK in 4.0 #8452

Open mpg opened 8 months ago

mpg commented 8 months ago

This issue is meant as a place to discuss what we want to do with PK 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).

Scope of PK

Note: for low-level modules related to asymmetric crypto (rsa.h, ecdsa.h covered by PK, but also ecdh.h and ecjpake.h, and underneath them ecp.h and bignum.h) I think the plan is to remove them from the public API, regardless of what we do with PK.

The problem of X.509 vs the PSA key store

With option 1 and pk_context structures being thin wrappers around psa_key_id_t, or with option 2 and blindly replacing all uses of pk_context with psa_key_id_t and no other code change, we'd have the following problem:

There are a number of ways to avoid that problem; we can act at any step in the chain leading to that potential issue:

Rationale for option 1 (keep)

This gives users more time to move to PSA. People who were using low-level modules (including those not covered by PK) need to migrate immediately, but those who were only using the generic PK API can migrate at a convenient time between now and 5.0.

Work needed for option 1 (keep)

In 4.0:

In 4.0 or 4.x:

Rationale for option 2 (remove)

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

Note the current pk.h API has a few shortcomings, the most prominent being it doesn't manage key permissions like PSA does. So, pushing users to the PSA API is also pushing them to a better API.

Work needed for option 2 (remove)

In 4.0:

In 4.0 or 4.x:

Other options

gilles-peskine-arm commented 8 months ago

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.6, we won't have a new API that covers the major features of the legacy pk API. In particular, PSA is missing the equivalent of pkparse and pkwrite.

Thus I don't see any choice. We need to keep pk (without its rough edges such as mbedtls_pk_ec and mbedtls_pk_rsa) in 4.x.

mpg commented 8 months ago

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.

I we want to follow that rule (which I think is generally speaking a good guideline), then even if we're keeping PK in 4.0, we should still provide PSA-based replacements for the X.509 and TLS APIs that use a key. So, for example, change mbedtls_ssl_conf_own_cert() to accept a PSA key slot instead of a pk_context and add a new, and immediately deprecated, function mbedtls_ssl_conf_own_cert_pk() - and similarly for the other APIs.

(Considering TLS and X.509 both use PK internally for now, the easiest way to implement this initially is for the new, PSA-based API to just internally wrap the PSA key in a PK context, and store the result.)

mpg commented 8 months ago

I forgot to note some optional features that PK parse has, for which it's unclear whether the upcoming PSA import extension will have support:

(Note: PK parse only supports these options when at least some of the built-in implementation (ECP_LIGHT) is enabled.)

gilles-peskine-arm commented 8 months ago

parsing ECC keys where the public key is in compressed format (Weierstrass curves);
parsing ECC keys where the curve is encoded as SpecifiedECDomain rather than NamedCurve.

Yes, we will use the PSA formatted import API to support those. But I fear that it won't be in 4.0, only in 4.1 or so.

mpg commented 8 months ago

If we don't have a complete replacement in 4.0 but only in 4.1, I think that's a rather strong argument for keeping PK public in 4.0.

mpg commented 8 months ago

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

mpg commented 8 months ago

I think there's a pretty strong argument for keeping PK in 4.0 so assuming we do, I think we need to be careful about its implementation and whether TLS & X.509 depend on it, in order to not make things harder for us in the future.

I think we want to avoid a situation where using TLS & X.509 require using PK and PK relies on psa_set_key_enrollment_algorithm() (I think it's OK if only one of this conditions is true), see part of the discussion at #8449.

More generally, I think currently our story about policies when PK internally stores key in PSA key slots (that is, for EC keys in builds where MBEDTLS_PK_USE_PSA_EC_DATA is defined) is not clean and we should clarify it. Might also be related with part of the discussion that happened here: https://github.com/Mbed-TLS/mbedtls/pull/7930#issuecomment-1652315212

If we keep PK in 4.0, we probably need some hard thinking about its API and semantics.

mpg commented 7 months ago

We could go for a mix: keep some parts but remove others. For example, we could keep PK parse or pk_context but remove the part about doing operations?

The more I think about it, the more I like this option: keep PK-parse/write but remove the sign/verify, encrypt/decrypt APIs (hereunder PK-ops). Instead, the only thing you can to with a pk_context is make a PSA key out of it - and then you can use PSA APIs for crypto operations.

Rationale: