Key wrapping API proposal #364

gilles-peskine-arm · 2020-02-10T14:37:15Z

This is a first draft of the key wrapping API for PSA Cryptography 1.x.

This pull request only provides function prototypes and associated macro definitions. Tests and implementations will come later.

I implemented the proposal discussed in private. I'm not attached to function/macro names if you have better suggestions. The API comprises:

Functions to back up and restore a key, wrapped with another key, using a vendor-chosen algorithm.
A reserved identifier value for a wrapping key. There is currently no way to create a wrapping key that can wrap keys with metadata.
Functions to wrap key material with another key, using standard algorithms.

This was implicitly conveyed by the general requirement of initializing attribute structure and by declaring \p attributes to be an in-out parameter, but it is better to make this explicit.

MarcusJGStreets · 2020-02-10T16:02:38Z

include/psa/crypto.h

+ *                          They are used as follows:
+ *                          - The key type and size may be 0. If either is
+ *                            nonzero, it must match the corresponding
+ *                            attribute of the source key.


You have used both source key and \p data which I believe both refer to the key to unwrap.
I would prefer consistency as it is quite hard to understand.

MarcusJGStreets · 2020-02-10T16:06:38Z

include/psa/crypto.h

+ *                            must match the location embedded in \p data.
+ *                            If the wrapped key has the usage
+ *                            flag #PSA_KEY_USAGE_COPY, then the location
+ *                            embedded in \p data is ignored.


I believe that from this description the caller must set attributes -> location.
So the caller needs to call psa_get_wrapped_key_attributes

Is there a reason why they should not be able to leave it unset and use the one in the wrapped key?
And potentiall not offer psa_get_wrapped_key_attributes?

I'd prefer not to have psa_get_wrapped_key_attributes, but I added it because I couldn't find a better way.

“Leave unset” works for the key type and the key size because for those 0 is not a valid value, so it's interpreted as unset. But for lifetimes, 0 is a valid value, indicating a volatile key: keys are volatile by default, you only need to specify a lifetime explicitly if you want the key to live longer. If a wrapped key is non-copyable and persistent, attempting to unwrap it as volatile should fail.

Likewise, for the policy, 0 means “you can't do anything”. That's not the same thing as “use whatever is in the wrapped data”.

Ok - good answer.

Good use cases. How about one or moe of these options as alternatives:

Permit attributes == NULL - this indicates that the key should be unwrapped exactly as it was wrapped. No override attributes.

Define a special lifetime value to mean ANY - i.e. "as wrapped"

Permit usage flags of MAX_UINT32 - and rely on the 'constrain to wrapped key policy' behaviour to replicate the wrapped key usage flags in the unwrapped key.

Permit attributes == NULL

Doesn't help if you want to use the wrapped policy but specify a different lifetime or id, which I think is a very common use case.

Permit usage flags of MAX_UINT32

That does in fact work for the usage flags, but not for the algorithm. It could be added as a feature for the algorithm.

The second bullet implies that we define (within the new structured framework) a special lifetime value, e.g.

#define PSA_KEY_LIFETIME_UNSPECIFIED ((psa_key_lifetime_t) 0x0000007f)

This value is only valid in attributes passed to psa_unwrap_key_with_policy() to indicate that the key should be unwrapped into the same lifetime that it was wrapped from.

You could also support all of the above mechanisms:

If you want to restore a backed up key, just pass attributes = NULL. (Volatile keys might have a different key handle/id after unwrapping)

If you want to move the key then initialise the attributes, set the wildcard usage flags (PSA_KEY_USAGE_UNSPECIFIED - all bits 1, not all known bits one: only valid for unwrapping/copying), and set the lifetime/id for the desired location

If you you want to limit the policy, but maintain the lifetime, then initialise the attributes, set the usage flags, and set the lifetime to PSA_KEY_LIFETIME_UNSPECIFIED (only valid for unwrap).

For (1), it would be possible to just use UNSPECIFIED for both the usage flags and the lifetime, but I suspect that the compact attributes = NULL form might be a common usage?

Instead of *_UNSPECIFIED, other terms could be *_AS_SOURCE or *_ORIGINAL.

MarcusJGStreets · 2020-02-10T16:21:30Z

include/psa/crypto.h

+ *                            &handle)
+ * psa_reset_key_attributes(&attributes);
+ * \endcode
+ *


Is it worth adding a line checking if the copy permission is set.

Why would the calling code check the copy permission? I'd expect typical application code to either expect a copyable key and specify a location here, or not expect a copyable key and not specify the location. Neither case calls for checking the copy permission in the application code.

Yes you are probably right for IOT.

MarcusJGStreets

Apporved follwing discussion with giles

athoelke · 2020-02-10T23:56:36Z

include/psa/crypto_values.h

+ * is used. If this key exists, it must be private to the device, i.e. it
+ * must not be shared with any other device or entity.
+ */
+#define PSA_KEY_ID_WRAP_BOUND                   ((psa_app_key_id_t)0x80000062)


Is there any particular significance to the value chosen? - why MIN_UINT32 + 0x62?

Not enamoured with the name, as we haven't used "bound" anywhere else in the API or documentation. Is PSA_KEY_ID_WRAP_BUILT_IN any better, or worse?

0x80000000 is the start of the range reserved for ids defined by PSA.

0x80000000 + 'b' (for “bound”) because why not.

I'm very open to naming suggestions. Once a few people have made theirs I'll do a pass of renaming stuff with an eye to consistency and the other eye to comprehensibility.

athoelke · 2020-02-11T00:20:51Z

Do we also need a usage flag for WRAP and UNWRAP, as well as the defined flags for a key being exporting in wrapped form? Keys used as wrapping keys in psa_wrap_key_*() or psa_unwrap_key_*() would then require the applicable usage flag.

athoelke

A few corrections and comments

athoelke · 2020-02-11T00:23:37Z

include/psa/crypto.h

+ *         One or both of \p handle and \p wrapping_key is not a valid
+ *         handle to a key.
+ * \retval #PSA_ERROR_NOT_PERMITTED
+ *         The key \p handle does not have the #PSA_KEY_USAGE_BACKUP flag.


copy-pasta: presume PSA_KEY_USAGE_EXPORT_WRAPPED

This issue is still present.

athoelke · 2020-02-11T00:26:32Z

include/psa/crypto.h

+ * \retval #PSA_ERROR_INVALID_SIGNATURE
+ *         \p data is not a valid wrapped key for \p wrapping_key.
+ * \retval #PSA_ERROR_NOT_SUPPORTED
+ *         Some of the metadata in either \p attributes or \p data is


There is no metadata in data for this API.

athoelke · 2020-02-11T00:29:54Z

include/psa/crypto_values.h

+ * described in SP 800-38F, but replacing uses of AES with the 128-bit
+ * block cipher that the key is for.
+ */
+#define PSA_ALG_NIST_KW                         ((psa_algorithm_t)0x0a401003)


This does not satisfy PSA_ALG_IS_KEY_WRAP() - the category should be 0x0e000000?

athoelke · 2020-02-11T00:30:25Z

include/psa/crypto_values.h

+ * described in SP 800-38F, but replacing uses of AES with the 128-bit
+ * block cipher that the key is for.
+ */
+#define PSA_ALG_NIST_KWP                        ((psa_algorithm_t)0x0a401005)


This does not satisfy PSA_ALG_IS_KEY_WRAP() - the category should be 0x0e000000?

athoelke · 2020-02-11T00:31:59Z

include/psa/crypto_values.h

+ * is used. If this key exists, it must be private to the device, i.e. it
+ * must not be shared with any other device or entity.
+ */
+#define PSA_KEY_ID_WRAP_BOUND                   ((psa_app_key_id_t)0x80000062)


Not enamoured with the name, as we haven't used "bound" anywhere else in the API or documentation. Is PSA_KEY_ID_WRAP_BUILT_IN any better, or worse?

athoelke · 2020-02-11T00:36:29Z

include/psa/crypto_values.h

+ * policy. In practical terms, the key material is encrypted, and
+ * the key data and metadata are authenticated together.
+ */
+#define PSA_KEY_USAGE_BACKUP                    ((psa_key_usage_t)0x00000020)


If we used PSA_KEY_ID_WRAP_BOUND then I would be inclined to make this PSA_KEY_USAGE_EXPORT_BOUND or perhaps PSA_KEY_USAGE_WRAP_BOUND.

BACKUP is quite cute, but then we should use 'backup' in the wrap-with-policy API name and the built-in wrap-with-policy binding key to strengthen the association of these API elements.

Declare functions to wrap and unwrap a key together with its metadata, in an unspecified vendor-specific format but with an explicitly specified key. Declare a corresponding usage flag.

Declare a key binding key that implementations are encouraged to provide.

Declare functions to wrap and unwrap key material in standard formats. Declare an algorithm category for key wrapping. Declare a corresponding usage flag.

Using attributes with psa_key_unwrap_with_policy() is inconvenient. When unwrapping a key to its original location (a straightforward restore from backup), there's no reason to specify any attributes. When unwrapping a key in a different location and possibly restricting its policy, the new attributes are generally a refinement of the old ones. Remove the attributes argument from psa_key_unwrap_with_policy() and provide another function to unwrap to a volatile location. The application can then use existing functions to manipulate the key, including psa_get_key_attributes() and psa_copy_key().

gilles-peskine-arm · 2020-02-12T18:29:56Z

@MarcusJGStreets @athoelke I pushed an update with a rewritten history that should resolve all your comments and cover what we discussed orally today, other than the naming. The original version is in https://github.com/gilles-peskine-arm/mbed-crypto/tree/psa-wrap-api-1

I fixed copypasta by amending the corresponding commits.
I removed the commit that introduced psa_get_wrapped_key_attributes, and instead made a new commit introducing psa_unwrap_key_to_alternate_lifetime and removing the attributes parameter to psa_unwrap_key_with_policy.
I added usage flags for wrapping keys.

I'll subsequently make another update to fix the terminology issues (which includes finding a decent name for psa_unwrap_key_to_alternate_lifetime; I couldn't resolve myself to psa_unwrap_key_with_policy_to_alternate_lifetime). I'm more and more warming up to backup/restore for wrap-with-policy. It isn't the only use case for this function (there's also sharing, when another entity has the unwrapping key), but it's a common one. I think anyone would understand backup/restore, even if it isn't what crypto architects would use, unlike “binding”, which even crypto architects don't fully agree on.

athoelke

Still some oopses in the usage flag association with the APIs.

athoelke · 2020-02-12T22:20:20Z

include/psa/crypto.h

+ * The format of the wrapped data is implementation-dependent. It may depend
+ * both on the choice of wrapping key and on the type of key to wrap.
+ *
+ * The policy on the key must have the usage flag


Need to specify which key needs this policy?

athoelke · 2020-02-12T22:28:47Z

include/psa/crypto.h

+ * both on the choice of wrapping key and on the type of key to wrap.
+ *
+ * The policy on the key must have the usage flag
+ * #PSA_KEY_USAGE_EXPORT_WRAPPED set.


#PSA_KEY_USAGE_BACKUP for this API

athoelke · 2020-02-12T22:29:53Z

include/psa/crypto.h

+ *         One or both of \p handle and \p wrapping_key is not a valid
+ *         handle to a key.
+ * \retval #PSA_ERROR_NOT_PERMITTED
+ *         The key \p handle does not have the #PSA_KEY_USAGE_BACKUP flag.


This issue is still present.

athoelke · 2020-02-12T22:48:13Z

Hi @gilles-peskine-arm - thanks for the update.

I'm quite happy with the idea of backup/restore terminology. Would you drop the _material from the other wrapping functions as a result?
Some ideas for the tricky-to-name API:
- psa_restore_key_as_volatile()
- psa_restore_key_elsewhere() :-)
- `psa_restore_key_ex()

The usage flag naming gets interesting now that we have "can be wrapped" and "can use to wrap" policy flags. It is clear there are two categories of flags:

Management policies - things the implementation/application can do to a key object: EXPORT, COPY, CACHE, WRAP, BACKUP
Usage policies - things the implementation/application can do with the key material: ENCRYPT, DECRYPT, SIGN, VERIFY, DERIVE, WRAP.

I wonder now if we should have named these categories of flag separately to make it easier to distinguish can-wrap-with from can-be-wrapped.

gilles-peskine-arm added 2 commits February 10, 2020 15:20

Fix copypasta in the description of PSA_KEY_EXPORT_MAX_SIZE

6669753

psa_get_key_attributes: state that *attributes must be valid on input

da79e92

This was implicitly conveyed by the general requirement of initializing attribute structure and by declaring \p attributes to be an in-out parameter, but it is better to make this explicit.

gilles-peskine-arm added enhancement New feature or request needs: review The pull request is ready for review. This generally means that it has no known issues. needs: design review api-spec Issue or PR about the PSA specifications labels Feb 10, 2020

gilles-peskine-arm requested a review from yanesca February 10, 2020 14:37

MarcusJGStreets suggested changes Feb 10, 2020

View reviewed changes

MarcusJGStreets previously approved these changes Feb 10, 2020

View reviewed changes

athoelke reviewed Feb 10, 2020

View reviewed changes

athoelke reviewed Feb 11, 2020

View reviewed changes

gilles-peskine-arm added 6 commits February 12, 2020 18:45

Declare key binding functions

3cb715b

Declare functions to wrap and unwrap a key together with its metadata, in an unspecified vendor-specific format but with an explicitly specified key. Declare a corresponding usage flag.

Declare PSA_KEY_ID_WRAP_BOUND

ec1b1ba

Declare a key binding key that implementations are encouraged to provide.

Declare key material wrapping functions

341ab2c

Declare functions to wrap and unwrap key material in standard formats. Declare an algorithm category for key wrapping. Declare a corresponding usage flag.

Declare NIST KW and KWP as examples of key wrapping algorithms

d29bc2d

Add usage flags for wrapping keys

c3177d4

gilles-peskine-arm dismissed MarcusJGStreets’s stale review via c3177d4 February 12, 2020 18:23

gilles-peskine-arm force-pushed the psa-wrap-api branch from 8d63d5d to c3177d4 Compare February 12, 2020 18:23

athoelke suggested changes Feb 12, 2020

View reviewed changes

gilles-peskine-arm mentioned this pull request Apr 23, 2020

Finish and review PSA key wrapping and lifetime API extensions Mbed-TLS/mbedtls#3233

Open

danh-arm assigned gilles-peskine-arm May 20, 2020

bensze01 added the Product Backlog label Mar 30, 2021

gilles-peskine-arm mentioned this pull request Jan 31, 2023

Add APIs to support wrapped keys ARM-software/psa-api#50

Open

giacomo3000 approved these changes May 6, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Key wrapping API proposal #364

Key wrapping API proposal #364

gilles-peskine-arm commented Feb 10, 2020

MarcusJGStreets Feb 10, 2020

gilles-peskine-arm Feb 10, 2020

MarcusJGStreets Feb 10, 2020

MarcusJGStreets Feb 10, 2020

gilles-peskine-arm Feb 10, 2020

MarcusJGStreets Feb 10, 2020

athoelke Feb 10, 2020

gilles-peskine-arm Feb 11, 2020

athoelke Feb 11, 2020

athoelke Feb 11, 2020

MarcusJGStreets Feb 10, 2020

gilles-peskine-arm Feb 10, 2020

MarcusJGStreets Feb 10, 2020

MarcusJGStreets left a comment

athoelke Feb 10, 2020

athoelke Feb 11, 2020

gilles-peskine-arm Feb 11, 2020

athoelke commented Feb 11, 2020

athoelke left a comment

athoelke Feb 11, 2020

athoelke Feb 12, 2020

athoelke Feb 11, 2020

athoelke Feb 11, 2020

athoelke Feb 11, 2020

athoelke Feb 11, 2020

athoelke Feb 11, 2020

gilles-peskine-arm commented Feb 12, 2020

athoelke left a comment

athoelke Feb 12, 2020

athoelke Feb 12, 2020

athoelke Feb 12, 2020

athoelke commented Feb 12, 2020

Key wrapping API proposal #364

Are you sure you want to change the base?

Key wrapping API proposal #364

Conversation

gilles-peskine-arm commented Feb 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarcusJGStreets left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

athoelke commented Feb 11, 2020

athoelke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gilles-peskine-arm commented Feb 12, 2020

athoelke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

athoelke commented Feb 12, 2020