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.05k stars 2.51k forks source link

Add API for setting the owner of a key ID for volatile keys when MBEDTLS_SVC_KEY_ID_ENCODES_OWNER is set #7437

Open adeaarm opened 1 year ago

adeaarm commented 1 year ago

Suggested enhancement

Introduce an Mbed TLS specific setter API that allows setting the owner field for volatile keys, in those cases when MBEDTLS_SVC_KEY_ID_ENCODES_OWNER is set. Current behaviour is that permanent keys allow to set the key ID completely through psa_set_key_id(), but this call will make the key persistent. For volatile keys, the owner is implicitly initialized by inspecting the key attributes during the key import operation. This means that the private owner field must be accessed through MBEDTLS_PRIVATE and manually set. This is relevant only for those implementations that use Mbed TLS PSA Crypto APIs as a part of a service implementation, i.e. TF-M.

Justification

Allowing an API for this aligns when MBEDTLS_SVC_KEY_ID_ENCODES_OWNER not set case and enforces good programming paradigms.

gilles-peskine-arm commented 1 year ago

This function already exists. It's called mbedtls_set_key_owner_id.

Do we need to give it a psa_xxx name? (I admit we have been inconsistent about the naming of functions that are not intended for PSA standardization, but are specific to the PSA interface in Mbed TLS.)

What we definitely should do is write a guide that explains, among other things, how to use this function.

adeaarm commented 1 year ago

Thanks @gilles-peskine-arm , we missed that in our offline conversation. I think keeping the name mbedtls_<something> makes more sense than using psa_<something>. In general, I believe prefixing everything that is not coming from the standard spec with mbedtls_ (or even mbedtls_psa) helps with modularization, as it keeps clear what is part of the standard and what is not. In this case, being the owner ID not part of the standard, I believe we shouldn't have it under the psa_ prefix. Obviously, the guide proposal is very useful, especially when vehiculating these concepts to partners.

adeaarm commented 1 year ago

I had a go at using this API. While it's true that this sets the owner ID only as required in the original request, it still not fulfills completely TF-M's use case, which is essentially translating key IDs from a client view to a server view by adding the owner ID with an existing psa_key_id_t. For example:

A psa_key_id_t key_id gets passed from the client to server, which then retrieves the owner and needs to create a new key ID structure made of the pair (owner, key_id). The present API could be used to set the owner, but there is no API available to set the key_id only, without changing the volatility of the key (i.e. psa_set_key_id). Please let me know your thoughts on this.

gilles-peskine-arm commented 1 year ago

adding the owner ID with an existing psa_key_id_t

That's mbedtls_svc_key_id_make. Which again looks like on this topic the problem is our lack of user documentation (the functions are documented but not so discoverable) rather than a lack of functionality.

adeaarm commented 1 year ago

Thanks - I was actually aware of this, I have completely removed it... 😄