Specify PK for TF-PSA-Crypto 1.0

[gilles-peskine-arm]

Design for pk.h in TF-PSA-Crypto 1.0.

The design attempts to minimize the amount of work we do before 1.0. A lot of things are left under the carpet. In particular, Mbed TLS library code and tests, as well as sample programs, are allowed to call functions that are now declared as private.

Resolves Mbed-TLS/mbedtls#8452

PR checklist

• changelog provided | not required because: doc only
• framework PR not required
• mbedtls development PR not required because: new stuff, doc only
• mbedtls 3.6 PR not required because: new stuff
• mbedtls 2.28 PR not required because: new stuff
• tests not required because: doc only

[mpg]

Looks pretty good, thanks! I only have a couple of question. The rest is very minor (and some comments are just notes that don't require any kind of action).

[mpg]

Consider the following scenario: a TLS 1.2/1.3 server has an RSA key that it wants to use for both protocols, which means making either v1.5 signatures (for 1.2) or PSS signatures (for 1.3). I think the way we support it now is with sign_ext() - nor for the extra fancy PSS settings, just for the ability to say we want PSS.

How is this kind of use case handled in the new PK? Do people need to create two contexts, one for v1.5 one for PSS?

(I don't think mbedtls_pk_set_algorithm() helps here, because mutating the context every time you want to do a signature would cause all kinds of threading issues.)

[gilles-peskine-arm]

You need two contexts. The idea is that under the hood, this can be a PSA key, and PSA only allows one algorithm family. (Our PSA implementation can support two algorithms, but in the medium term we don't want to assume that this extension is present — even TF-M doesn't expose it.)

[mpg]

Ok, but then we need to document this as a breaking change between 3.x and 4.x/1.x. In 3.x I can parse an RSA key with pk_parse(), pass it to ssl_conf_own_cert() and things will just work with TLS 1.2 and 1.3. In 4.x I'll need to create a copy of the context with different permissions and then call ssl_conf_own_cert() twice. I think that's perfectly OK but ought to be mentioned in the ChangeLog and documented in the migration guide.

[gilles-peskine-arm]

Good point, I hadn't really thought this through. I'm at least going to add this to a documentation task.

But now I wonder if we should go further and actually stop using mbedtls_pk_sign_ext in TLS. (Verify is fine, you can always export public keys). Because otherwise existing users are going to have applications that keep working just fine with 4.0, either because they didn't notice this entry in the migration guide or they didn't think it applied to them since their application kept working. And then it'll break in some 1.x PK refactoring,

[mpg]

LGTM, could not find anything worse than a harmless typo.

[mpg]

Typo: nEw

[davidhorstmann-arm]

A few questions / clarifications and a smattering of typos, but no big problems with the design as a whole.

[davidhorstmann-arm]

LGTM, thanks!

[mpg]

Looks generally good to me. However, I'm feeling somewhat uncomfortable about this whole set_algorithm() thing. I'll keep thinking about it.

[mpg]

I think for keys that were just parsed (or more generally, are exportable - or even just copyable) we can make it do an export-import under the hood, so it keeps working when all (private) keys are backed by PSA.

For keys that can't be copied it would fail though. And things that will fail in 1.x should already fail in 1.0. We can achieve that by adding an extra check, or already using the future-proof implementation in 1.0.

[gilles-peskine-arm]

I'm feeling somewhat uncomfortable about this whole set_algorithm() thing.

Same, the more I sleep on it the more wrong it feels.

[gilles-peskine-arm]

Having thought about it further, this is definitely wrong. It can't be implemented in a reasonable way on top of a PSA key, since you can't change the PSA key's policy, and having a PK algorithm that doesn't match the policy of the underlying PSA key is pointless.

We need a different solution for the following scenario:

1. I parse a key.
2. Based on some of the key's metadata, I decide which signature algorithm to use it with.
3. I sign with pk.

There is already such a workflow. It's somewhat inefficient, but I think we can live with it.

1. Parse the key.
2. mbedtls_pk_get_psa_attributes() and mbedtls_pk_import_into_psa(). This workflow was created precisely to solve this problem.
3. mbedtls_pk_copy_from_psa() or, to save some resources, mbedtls_pk_wrap_psa().

[mpg]

Trying to summarize my thoughts. This is mostly redundant with what you've written already, but (re)writing it myself helps me organize my thoughts.

I think the public key case is the easiest, so I'll focus on the private key case for now.

It seems to me there's two ways a user might want a private RSA key to be used:

• with a fixed algorithm decided in advance (for example, only PSS);
• with either algorithm depending on what's negotiated in TLS.

Also, there are two ways a user might get their RSA key:

• by parsing a file / string of bytes;
• as an existing PSA key.

So that's a matrix of 4 cases to consider:

1. Parse -> fixed alg.
2. Parse -> any alg.
3. PSA key -> fixed alg.
4. PSA key -> any alg.

The 4th case is not guaranteed to always work (ie if the key is not exportable), and that's a feature not a bug.

Finally, users might either have access to the key_enrolment_alg extension and want to take advantage of it, or not. It's not clear to me if allowing users to take advantage of it when it's available should be a design goal here.

[gilles-peskine-arm]

Finally, users might either have access to the key_enrolment_alg extension and want to take advantage of it, or not. It's not clear to me if allowing users to take advantage of it when it's available should be a design goal here.

I consider it an optional goal: nice to have, but not a blocker.

[mpg]

So far the general idea is:

1. Parse -> fixed alg
   • if the desired alg is the one that parsing sets by default (v1.5 currently), nothing to do;
   • if it's the other one, want to change the alg on an existing PK context.
2. Parse -> any alg: want to create a copy with the other alg.
3. PSA -> fixed alg: nothing to do, PSA already has a set alg.
4. PSA -> any alg: want to create a copy with the other alg (might fail though).

So, it seems to me we have two basic wants:
a. change the alg on an existing PK context
b. create a copy of an existing PK context with another alg

It's easy to express (a) in terms of (b): just copy then destroy the original (though maybe not optimal on resources). It's also easy to express (b) in terms of (a) plus a generic "copy as is" function, but that "copy as is" function is unlikely to be useful for anything else.

[mpg]

Consider the following scenario: I have a PSA key that allows both v1.5 (primary alg) and PSS (enrolment alg). I create a PK context from it using pk_wrap_psa(). I then call set_algorithm(PSS) on it.

Should I expect to still be able to do v1.5 signatures with the resulting PK context? It's not clear to me what would be the least surprising here.

More generally, I think the fundamental discomfort I have with set_algorithm is that it becomes unclear what part of the code is responsible for expressing and enforcing policies. I think I'd really like a model where policies are clearly expressed and enforced by PSA only (and PK really is just a wrapper).

The only problem with that is that it kind of exposes implementation details: right now, PK is not just a wrapper around PSA, and the plan is that making it a wrapper will be a 4.x thing. I'm almost tempted to suggest making it a wrapper right now (ie, make RSA key storage work the same as ECC key storage currently works (with MBEDTLS_PK_USE_PSA_EC_DATA): private keys are stored as PSA keys, and public keys as the data needed by psa_import(). But that doesn't really change the issue that it makes implementation details almost part of the API.

[mpg]

Typo: certiciate

[gilles-peskine-arm]

In all the variants that I've considered, PK is explicitly not responsible for enforcing policies. However, since PK is the interface, it unavoidably has some control over expressing policies. For example, given that we don't want to change the parsing interface (because we don't have a good alternative), parsing needs to pick some default policy. And that, in turn, means that there has to be some way of using a different policy. There is already a way through import_into_psa. I do wonder if we should just get rid of pk_sign and pk_verify... But it would mean significantly more pre-4.0 work in the code that's using it, so I don't think it would really be a win.

[mpg]

What we want to do now (4.0) depends on lot of what we plan to do later (4.x). We don't want to paint ourselves into a corner where we no longer can make the changes we want to do in 4.x without breaking users code. (And as was mentioned in a few other contexts, I think we shouldn't rely just on documentation: things that will break in 4.x should break in 4.0 already.)

So, I'd like to discuss what we want to do next, in 4.x, which is move RSA keys from being stored as rsa_contexts to being held as PSA keys for all PK contexts (for private keys at least), even those created by PKparse. This will associate a policy with each key, which is a significant change from 3.6 where PK essentially didn't have policies.

If at this point, if relying on the "enrolment algorithm" extension is an option, we could make PKparse create RSA keys that are usable with both v1.5 and PSS. This would be the least disruptive compared to current (3.6) behaviour. (PK contexts created by wrapping a PSA key would inherit its policy, as they already to do in 3.6 with setup_opaque().

Technically I think we could do this, as making TLS/X.509 work with other implementations of PSA crypto is not an official goal until 5.0 (where we're likely to remove PK) - but unofficially perhaps there are people who still want to use PK in a context where the "enrolment alg" extension can't be relied on?

If we can't rely on this extension, that means in 4.x (when all PK (private-key) contexts are backed by PSA) then RSA keys suddenly become single-alg. Which means this change should happen in 4.0. Which means either we move that change forward to 4.0, or in the meantime PK has to enforce policies for RSA keys so that code that will break in 4.x breaks in 4.0 already.

Wdyt?

[gilles-peskine-arm]

unofficially perhaps there are people who still want to use PK in a context where the "enrolment alg" extension can't be relied on?

TF-M, last I heard.

things that will break in 4.x should break in 4.0 already

That's a good point. So far in the design I've tried to ensure that the new way of doing things will be ok for 4.x, but I haven't added enforcement that “cheating” ways no longer work. I'll think about it.

RSA keys suddenly become single-alg. Which means this change should happen in 4.0. Which means either we move that change forward to 4.0, or in the meantime PK has to enforce policies for RSA keys so that code that will break in 4.x breaks in 4.0 already.

That is mostly the case with mbedtls_pk_set_algorithm: although PK allows you to cheat, you have to do it explicitly by calling a function on the context. You can no longer ignore the policy by calling mbedtls_pk_sign_ext().

[mpg]

You can no longer ignore the policy by calling mbedtls_pk_sign_ext().

Because we're removing this function anyway, right? So there's just pk_sign() and when people call it, that will use the algorithm associated with the key.

So we'll need some changes in library/ssl_tls13_generic.c as it's currently calling sign_ext().

You said earlier prototyping wasn't really relevant here because the issue is not to have things work in 4.0 but that they keep working in 4.x. If we add the requirement that things that will fail in 4.x should already fail in 4.0, does this point still stand in your opinion?

At this point, I'm under the impression that we're going in the following direction:

• For public keys, we're keeping verify_ext() (presumably with the type argument changed from mbedtls_pk_type_t to psa_algorithm_t) which supports the use cases in X.509 and TLS and works regardless of the underlying implementation (since public keys are always exportable).
• For private keys, we're not keeping sign_ext() as it's not compatible PSA-based key storage (and quite questionable from a "crypto best practices" perspective).
• For TLS, if a user wants their RSA key to be usable with both v1.5 and an PSS, they will have to explicitly opt in by somehow (see below) creating two PK contexts, one for each alg, and call set_own_cert() twice, one with each key.
• We need to adapt the TLS code always call sign() not sign_ext(), and select the correct key for the desired algorithm (or error out if we can't find a key that matches one of the negotiated algs).
• For private keys, can_do_ext() should probably only return true for the algorithm that would be used if pk_sign was called on that key. (Currently can_do_ext() returns true if the argument matches the key's enrolment alg, but presumably pk_sign() would always use the key's "primary" alg.)

I feel like the undecided point so far was mostly the "somehow" above for creating two PK contexts. It seems to me the main contenders are:

1. Don't add new functions (but perhaps documentation and examples) and just let people use mbedtls_pk_import_into_psa() + copy_from_psa()/wrap_psa() (currently called setup_opaque()).
2. Add pk_set_algorithm() (will fail if the key is PSA-based and not exportable, but will always work on a key that was just parsed). That's cool if the user just wants to use the other alg, but if they want to use both, they need to manage creating a 2nd context with the same key material on their own. (Well, if the key has just been parsed, they could just parse it again to another context and call set_alg on the 2nd context.)
3. Add pk_copy() that allows changing the alg on the copy. (Again, will fail if the key is PSA-based and not exportable, but will always work on freshly parsed keys.) That's cool if the users wants two context, but slightly less cool if they just want one context with the other alg.
4. Add both pk_set_alg() and pk_copy().

It seems to me that this part is really just about user convenience. So I think there are two relatively independent parts to this:

A. Whether we want to keep verify_ext() and/or sign_ext(), and if we remove sign_ext(), clearly specify sign() and can_do_ext() behaviour, and: (1) how we'll implement this when RSA keys are stored as rsa_context and when they're held in PSA, and (2) how to adapt our TLS code.
B. What convenience function(s) (if any) we want to provide for users to create a PK context with the desired alg, or a two contexts with different algs from the same key material.

[mpg]

Fun (not) game: dive into 3.6 documentation and see if anything says that when you pass the result of pk_parse_key() (on an RSA) key to mbedtls_ssl_conf_own_cert() then you'll be able to do handshake signatures with any of v1.5 or PSS as a result. I'm honestly not sure the documentation says anything to that effect (or the opposite).

I'm still strongly inclined to consider that something that shouldn't be broken in 3.6.x, and that should be advertised as a breaking change in 4.0 if we decide to change it, of course. It's just that seeing the wording here made me think, part of our problems every time we touch PK is it's underspecified (and lacks a clear design).

[gilles-peskine-arm]

We don't say it explicitly, but since PK doesn't claim to set a policy on RSA keys, passing a PK object containing a freshly parsed RSA key to a consumer (here SSL) allows that consumer to use both v1.5 and PSS.