Define some structure for lifetime values #358

gilles-peskine-arm · 2020-02-05T18:19:51Z

After some internal discussion (private link), here's a proposal for how to assign lifetimes.

This proposal is backward-compatible with the current values for volatile keys (0) and persistent keys (1). Platforms with a secure element (i.e. a cryptoprocessor that can process keys which are not accessible to the PSA crypto core) should use the lifetime 0x1ff (whether the key material is stored inside the secure element, or in generic PSA storage in a form wrapped by the secure element).

This pull request defines the lifetime structure, but does not yet implement any new feature. A future pull request will adapt the (currently experimental) secure element support code accordingly.

We intend to include the types and values defined in this pull request with the same semantics in an upcoming release of the PSA Cryptography API specification.

@athoelke @MarcusJGStreets

* Lower 8 bits: persistence level * 0: volatile * 1: persistent (default) * 2-127: persistent (reserved for future PSA specifications) * 128-254: persistent (reserved for vendors) * 255: read-only * Upper 24 bits: location indicator * 0: built-in * 1: primary secure element * 2-0x7fffff: reserved for future PSA specifications * 0x800000-0xffffff: vendor-specific

yanesca

I have a couple of suggestions for improvement and a question.

include/psa/crypto_types.h

yanesca · 2020-02-06T13:00:04Z

include/psa/crypto_values.h

 #define PSA_KEY_LIFETIME_PERSISTENT             ((psa_key_lifetime_t)0x00000001)

+#define PSA_KEY_LIFETIME_PERSISTENCE_VOLATILE ((psa_key_lifetime_persistence_t)0x00)
+#define PSA_KEY_LIFETIME_PERSISTENCE_PRIMARY ((psa_key_lifetime_persistence_t)0x01)


We have a location with the same value that we describe as the "primary secure element". This made me confused at first about the purpose of this constant.

Suggested change

#define PSA_KEY_LIFETIME_PERSISTENCE_PRIMARY ((psa_key_lifetime_persistence_t)0x01)

#define PSA_KEY_LIFETIME_PERSISTENCE_PERSISTENT ((psa_key_lifetime_persistence_t)0x01)

Yeah, I don't like PERSISTENCE_PRIMARY either. But PERSISTENCE_PERSISTENT is weird and misleading: other PERSISTENT_xxx values are also persistent. And I don't like PERSISTENCE_DEFAULT either (for example, when you create a key, the persistence defaults to VOLATILE, not to DEFAULT). Does anyone have a better suggestion?

Well it might be a little bit overkill, but how about PSA_KEY_LIFETIME_PERSISTENCE_PERSISTENT_DEFAULT?

Another candidate perhaps: PSA_KEY_LIFETIME_PERSISTENCE_STANDARD?

Standard works for me

I don't like “standard”: which standard? I went with “default”.

include/psa/crypto_types.h

include/psa/crypto_values.h

yanesca · 2020-02-06T13:09:05Z

include/psa/crypto_values.h

+ * the application instance terminates. In particular, a volatile key
+ * is automatically on a power reset of the device.
+ *
+ * A key that is not volatile is persistent. Persistent keys are


This means that there can't be a lifetime for keys that survives the termination of the application but not a power reset. I can hardly imagine a use case where that would be needed, I just would like to confirm that this is what we want?

Seem sensible changes.
I am still thinking about "volatile" keys that last beyond the life of the calling partition.

A lifetime that survives the instance of the calling application but not a power reset could exist, but:

This is an imprecise definition. If the application is running in a virtual machine, is the scope of the power reset the VM or its physical host?

How does another instance of the application refer to the same key? It would need to use a key identifier, so this would be more like a persistent key from the application's perspective.

Yes, it's possible. I've worked with a system that basically had this third intermediate scope (volatile-to-application-instance, volatile-to-device, persistent). But AFAIK this intermediate scope was never used outside of the test suite. So I'm with “can hardly imagine a use case”.

If an implementation really wants this feature, it can provide it through the vendor range for persistence levels.

Yes that covers what I meant by "thinking".
In general PSA has been writtien for M-class with a fixed set of secure partitions all of which active - so there really is no concept of application life time except reboot.

However, when we extend to Cortex A we will need to deal with other life times - but I believe that can be left to the next release.

athoelke · 2020-02-06T22:54:36Z

include/psa/crypto_types.h

+ *       interfaces operate on lifetimes (type ::psa_key_lifetime_t) which
+ *       encode the persistence as the lower 8 bits of a 32-bit value.
+ */
+typedef uint8_t psa_key_lifetime_persistence_t;


Is the lifetime scoping in this identifier name required?

At present psa_key_persistence_t (and likewise for psa_key_location_t) would be clear and unambiguous - the only downside is that the name does not provide any hints that this is part of a psa_key_lifetime_t value.

I prefer to make the connection with lifetimes explicit.

On second thoughts I shortened the names.

athoelke · 2020-02-06T23:01:28Z

include/psa/crypto_types.h

+ *   implementation-specific means such as a factory ROM engraving process.
+ *   Note that keys that are read-only due to policy restrictions
+ *   rather than due to physical limitations should not have this
+ *   persistence levels.


Perhap just for internal clarity, if not for the public documentation: is this lifetime appropriate for keys that have the same lifetime as the device identifier? - Some devices might be designed to support a factory process that erases the device reprograms it as a new device identity with new "read-only" keys.

If the device identifier is set in stone, that's the corresponding lifetime. If the device identifier is rewritten on a factory reset, this corresponds to a different lifetime (which may be 1 or something else depending on what levels of “factory reset” exist).

athoelke · 2020-02-06T23:03:39Z

include/psa/crypto_values.h

+    (PSA_KEY_LIFETIME_GET_PERSISTENCE(lifetime) == \
+     PSA_KEY_LIFETIME_PERSISTENCE_VOLATILE)
+
+#define PSA_KEY_LOCATION_BUILT_IN    ((psa_key_lifetime_location_t)0x000000)


Do we also want a PSA defined symbol for the specification allocated location 0x000001? Maybe PSA_KEY_LOCATION_SECURE_ELEMENT?

So far I consider this an implementation-specific definition, but I don't have a strong opinion about it.

So, unlike other imp-specific parts of the API, this one has a specified value, but not a specified symbol? A bit unusual in the documentation, but we don't provide handy spec-defined symbols for all of the [vendor] ranges in other value types either.

It's a bit like persistence == 1, which we can't find an ideal name for either, but really have to name because it is effectively used by PSA_KEY_LIFETIME_PERSISTENT.

athoelke · 2020-02-06T23:05:19Z

include/psa/crypto_values.h

+    (PSA_KEY_LIFETIME_GET_PERSISTENCE(lifetime) == \
+     PSA_KEY_LIFETIME_PERSISTENCE_VOLATILE)
+
+#define PSA_KEY_LOCATION_BUILT_IN    ((psa_key_lifetime_location_t)0x000000)


These definitions do not have LIFETIME scoping term in the identifier. This should be added for consistency, or the lifetime/LIFETIME scope term should be removed from all of them (as described above).

athoelke · 2020-02-06T23:07:19Z

include/psa/crypto_values.h

+     PSA_KEY_LIFETIME_PERSISTENCE_VOLATILE)
+
+#define PSA_KEY_LOCATION_BUILT_IN    ((psa_key_lifetime_location_t)0x000000)
+#define PSA_KEY_LOCATION_VENDOR_FLAG ((psa_key_lifetime_location_t)0x800000)


The other ranged values no longer have VENDOR_FLAG defintions - this will not appear in the PSA specification.

Right. Like the other helper macros, these macros have no Doxygen, so they aren't intended to be included in the PSA specification.

gilles-peskine-arm · 2020-05-04T17:16:37Z

Since Mbed Crypto is no longer updated, I've rebased this pull request on top of Mbed TLS. To be continued at Mbed-TLS/mbedtls#3302

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. api-spec Issue or PR about the PSA specifications labels Feb 5, 2020

gilles-peskine-arm requested a review from yanesca February 5, 2020 18:19

gilles-peskine-arm mentioned this pull request Feb 5, 2020

Add vendor key support #353

Merged

yanesca requested changes Feb 6, 2020

View reviewed changes

yanesca added needs: work The pull request needs rework before it can be merged. and removed needs: review The pull request is ready for review. This generally means that it has no known issues. labels Feb 6, 2020

athoelke suggested changes Feb 6, 2020

View reviewed changes

gilles-peskine-arm added 3 commits February 10, 2020 16:11

Fix typos

a9960de

Reference explicitly-named persistence levels by name

4967759

Consistently prefix location with lifetime_

1c05442

gilles-peskine-arm requested a review from yanesca February 10, 2020 15:12

gilles-peskine-arm added needs: review The pull request is ready for review. This generally means that it has no known issues. and removed needs: work The pull request needs rework before it can be merged. labels Feb 10, 2020

This was referenced Apr 23, 2020

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

Open

Redesign the secure element interface to support wrapped key storage Mbed-TLS/mbedtls#3287

Closed

PSA: Support volatile secure element keys Mbed-TLS/mbedtls#3288

Closed

gilles-peskine-arm mentioned this pull request May 4, 2020

Define some structure for lifetime values Mbed-TLS/mbedtls#3302

Merged

gilles-peskine-arm closed this May 4, 2020

	#define PSA_KEY_LIFETIME_PERSISTENCE_PRIMARY ((psa_key_lifetime_persistence_t)0x01)
	#define PSA_KEY_LIFETIME_PERSISTENCE_PERSISTENT ((psa_key_lifetime_persistence_t)0x01)

Define some structure for lifetime values #358

Define some structure for lifetime values #358

Uh oh!

Conversation

gilles-peskine-arm commented Feb 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yanesca left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gilles-peskine-arm commented May 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gilles-peskine-arm commented Feb 5, 2020 •

edited

Loading