PSA unified driver specification #3493

gilles-peskine-arm · 2020-07-13T11:33:55Z

This pull request introduces the work in progress on the specification of PSA crypto drivers. Eventually the PSA crypto driver specification will be a PSA document issued by Arm. But for the time being, this working draft is being maintained in Mbed TLS.

This pull request also introduces documents that are specific to the Mbed TLS implementation: a driver integration guide and a driver developer guide. These documents are still very incomplete.

This pull request starts where #3313 stopped. The initial state of the PSA draft specification is the final state in #3313 with the mbedtls-specific.

Status:

This will be merged as proposed specifications, without any implementation. Implementations will come in subsequent pull requests, and the specifications may be revised based on experience gained and external feedback.
The sections on multipart operations (including key derivation), psa_copy_key and volatile secure element keys are out of scope.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Working draft of the PSA cryptography unified interface specification. Eventually this document will be under Arm PSA architecture ownership, but for the time being this draft is maintained in Mbed TLS. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Driver developer's guide: introduction on how to write a driver. Driver integration guide: how to build Mbed TLS with drivers. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

PSA_KEY_LOCATION_acme, PSA_KEY_LIFETIME_acme Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Call the functions listed in the driver description "entry points". It's more precise than "functions", which could also mean any C function defined in the driver code. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Opaque drivers only have a destroy function if the key is stored in the secure element. Expand on how key creation works. Provide more explanations of allocate_key in drivers for secure elements with storage. Discuss key destruction as well. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Rather than have some functions take the in-memory copy of the persistent data as argument, allow all of them to access the persistent data, including modifying it. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

It's out of scope. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

stevew817 · 2020-08-04T11:33:14Z

docs/proposed/psa-driver-interface.md

-It's fairly common for secure elements to also store only the private value. When a key is generated randomly or derived, the hardware reports the public key, and it is up to the software to store it. The current design makes this the job of the driver. Should it be the work of the core instead?
-
-Note that a solution also has to work for transparent keys, and when importing a private key into a secure element. If the core already has code to calculate the public key, it would make sense for the driver to be able to use it, especially in these cases.
+The specification doesn't mention when the public key might be calculated. The core may calculate it on creation, on demand, or anything in between. Opaque drivers have a choice of storing the public key in the key context or calculating it on demand and can convey whether the core should store the public key with the `"store_public_key"` property. Is this good enough or should the specification include non-functional requirements?


Why would the core care about whether or not the opaque driver stores the public part in its key context? An opaque driver is free to declare context sizes, so as long as it declares the correct size, I don't see why the core should know about this behaviour through a property.

The intent of the property is to allow the core to choose the size of the context based on the size of the key.

Isn't that already supported through key_pair_size?

key_pair_size is independent of the key size. See “Size of a statically allocated key context” below.

key_size_function then? The reason that got added is to catch-all support all drivers that might err from the predefined path. I have the same comment about the symmetric key size multiplier, too.

You don't want to start adding flags for each and every custom usecase, that's why the key size function got added. Then why does store_public_key (and symmetric multiplier) deserve a flag?

Indeed, if a driver uses key_size_function, the core won't know about its propensity to store the public key. The point of store_public_key, key_pair_size etc is to support drivers without using key_size_function, for systems that don't allocate arbitrarily-sized memory dynamically. I tried to have enough granularity to allow a reasonable compromise between complexity and wasted memory. For example it's possible to have a fixed array of slots, but with slots of different sizes to accommodate keys of different sizes.

Then I suggest adding a size multiplier instead. Even if a driver indicates it wants to store the public key, you wouldn't know whether it wants to store in compressed or uncompressed form. Plus, the multiplier would create symmetry with symmetric key handling.

stevew817 · 2020-08-04T11:41:43Z

docs/proposed/psa-driver-interface.md

+
+An example use case for updating the persistent state at arbitrary times is to renew a key that is used to encrypt communications between the application processor and the secure element.
+
+`psa_crypto_driver_get_persistent_state` does not identify the calling driver, so the driver needs to remember which driver it's calling. This may require a thread-local variable in a multithreaded core. Is this ok?


I think this'll cause a lot of boilerplate in the code that is calling driver entry points. If we can contain the boilerplate to the autogen'ed wrapper file that could be fine, but if this particularity is going to leak into psa_crypto.c it's another story.

That's a good point. I liked this being automatic to reduce the risk of mistakes. But maybe psa_crypto_driver_get_persistent_state should take a lifetime argument, and optionally the core could refuse if the lifetime is wrong?

Then you could end up with rogue drivers trying to grab each other's state... Not sure whether that's a concern or not.

There's no memory isolation between drivers. At least not on typical implementations. A high-security implementation could isolate drivers (and make sure they have sufficiently restrictive DMA permission so that they can't access arbitrary memory via the crypto hardware!), and then it had better check the lifetime.

On the other hand, having to pass the location will create similar boilerplate in drivers. So I prefer to leave it this way for now and wait to see how it goes in practice.

stevew817 · 2020-08-04T11:44:11Z

docs/proposed/psa-driver-interface.md

@@ -514,10 +514,6 @@ In a multithreaded environment, the driver may only call these two functions fro

 ## How to use drivers from an application

-### Declaring which cryptographic mechanism an application needs


I think "Declaring which cryptographic mechanism an application needs" is still a very relevant question that should be part of this spec work. Even if the spec itself is not changing anything and pushing out changing it to a later time, it's a good opportunity to define how exactly the current system is supposed to work, since different people seem to have different understandings (ref discussions on 3313).

It's very relevant and needs to be done, but I don't want to hold up the PSA driver spec on this, and I'm not sure if we'll end up codifying this at the PSA level rather than doing something in Mbed TLS. Drivers are mostly a new area whereas selecting which parts of the library to include involves changing what already exists in Mbed TLS.

danh-arm

I don't think any of my comments gate merging but I think I need some education on how fallback and persistent storage works to be sure. @gilles-peskine-arm I'll contact you separately about that.

docs/proposed/psa-driver-interface.md

danh-arm · 2020-08-05T11:47:15Z

docs/proposed/psa-driver-interface.md

+
+### Fallback
+
+If a transparent driver entry point is part of a capability which has a true `"fallback"` property and returns `PSA_ERROR_NOT_SUPPORTED`, the built-in software implementation will be called instead. Any other value (`PSA_SUCCESS` or a different error code) is returned to the application.


In line 91 you say the core should call another driver or the built-in software whereas here you say just the latter. Need to be consistent.

Ok, ok, the wording isn't formally precise. But I think it's clearer this way, because having a single applicable driver is the most common case.

docs/proposed/psa-driver-interface.md

danh-arm · 2020-08-05T17:32:07Z

docs/proposed/psa-driver-developer-guide.md

+
+The interface is not fully implemented in Mbed TLS yet and is disabled by default. You can enable the experimental work in progress by setting `MBEDTLS_PSA_CRYPTO_DRIVERS` in the compile-time configuration. Please note that the interface may still change: until further notice, we do not guarantee backward compatibility with existing driver code when `MBEDTLS_PSA_CRYPTO_DRIVERS` is enabled.
+
+## Introduction


There's some duplication here with the driver interface spec but that can be addressed later.

Surely some duplication in the material that presents the context is expected, for documents that have similar contexts.

Some yes, though I'd say there's a bit too much right now. Anyway, this is fine for now

Locations aren't in the official PSA API specification yet (they've only be made public in Mbed TLS). Until version 1.0.1 of the API specification is out, this document needs to explain locations. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Rename psa_crypto_driver_update_persistent_state to psa_crypto_driver_commit_persistent_state. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

gilles-peskine-arm

Thanks Steve and Dan for your feedback. I made some minor updates, mostly clarifications based on Dan's feedback.

gilles-peskine-arm · 2020-08-05T19:34:18Z

docs/proposed/psa-driver-developer-guide.md

+
+The interface is not fully implemented in Mbed TLS yet and is disabled by default. You can enable the experimental work in progress by setting `MBEDTLS_PSA_CRYPTO_DRIVERS` in the compile-time configuration. Please note that the interface may still change: until further notice, we do not guarantee backward compatibility with existing driver code when `MBEDTLS_PSA_CRYPTO_DRIVERS` is enabled.
+
+## Introduction


Surely some duplication in the material that presents the context is expected, for documents that have similar contexts.

docs/proposed/psa-driver-interface.md

gilles-peskine-arm · 2020-08-05T20:26:49Z

docs/proposed/psa-driver-interface.md

+* `"key_types"` (optional, list of strings). Each element is a [key type specification](#key-type-specifications). If specified, the core will invoke this capability of the driver only for operations involving a key with one of the specified key types. If omitted, the core will invoke this capability of the driver for all applicable key types.
+* `"key_sizes"` (optional, list of integers). If specified, the core will invoke this capability of the driver only for operations involving a key with one of the specified key sizes. If omitted, the core will invoke this capability of the driver for all applicable key sizes. Key sizes are expressed in bits.
+* `"names"` (optional, object). A mapping from function names described by the `"functions"` property, to the name of the C function in the driver that implements the corresponding function. If a function is not listed here, name of the driver function that implements it is the driver's prefix followed by an underscore (`_`) followed by the function name. If this property is omitted, it is equivalent to an empty object (so each function *suffix* is implemented by a function with called *prefix*`_`*suffix*).
+* `"fallback"` (optional for transparent drivers, not permitted for opaque drivers, boolean). If present and true, the driver may return `PSA_ERROR_NOT_SUPPORTED`, in which case the core should call another driver or use built-in code to perform this operation. If absent or false, the core should not include built-in code to perform this particular cryptographic mechanism.


This is just the one-short-paragraph overview. I added a link to the section with all the details.

gilles-peskine-arm · 2020-08-05T20:31:02Z

docs/proposed/psa-driver-interface.md

+
+### Fallback
+
+If a transparent driver entry point is part of a capability which has a true `"fallback"` property and returns `PSA_ERROR_NOT_SUPPORTED`, the built-in software implementation will be called instead. Any other value (`PSA_SUCCESS` or a different error code) is returned to the application.


Ok, ok, the wording isn't formally precise. But I think it's clearer this way, because having a single applicable driver is the most common case.

docs/proposed/psa-driver-interface.md

gilles-peskine-arm · 2020-08-05T20:47:16Z

docs/proposed/psa-driver-interface.md

+
+An example use case for updating the persistent state at arbitrary times is to renew a key that is used to encrypt communications between the application processor and the secure element.
+
+`psa_crypto_driver_get_persistent_state` does not identify the calling driver, so the driver needs to remember which driver it's calling. This may require a thread-local variable in a multithreaded core. Is this ok?


On the other hand, having to pass the location will create similar boilerplate in drivers. So I prefer to leave it this way for now and wait to see how it goes in practice.

danh-arm

OK, let's go with this version for now

danh-arm · 2020-08-06T13:26:43Z

docs/proposed/psa-driver-developer-guide.md

+
+The interface is not fully implemented in Mbed TLS yet and is disabled by default. You can enable the experimental work in progress by setting `MBEDTLS_PSA_CRYPTO_DRIVERS` in the compile-time configuration. Please note that the interface may still change: until further notice, we do not guarantee backward compatibility with existing driver code when `MBEDTLS_PSA_CRYPTO_DRIVERS` is enabled.
+
+## Introduction


Some yes, though I'd say there's a bit too much right now. Anyway, this is fine for now

danh-arm · 2020-08-06T15:11:36Z

I merged this with 1 approval because I think it's had enough review for now and we acknowledge this is a work in progress.

ronald-cron-arm · 2020-08-06T15:14:43Z

I am currently reviewing it but I guess I will be able to push my comments anyway.

ronald-cron-arm

This is what I have for now. I haven't look at the changes you pushed yesterday following Dan's review thus there are probably duplicates, sorry about that.

docs/proposed/psa-driver-interface.md

ronald-cron-arm · 2020-07-21T16:29:45Z

docs/proposed/psa-driver-interface.md

+
+[Req.location] Applications can tell which location values correspond to which secure element drivers.
+
+[Req.fallback] Accelerator drivers can specify that they do not fully support a cryptographic mechanism and that a fallback to core code may be necessary. Conversely, if an accelerator fully supports cryptographic mechanism, the core does not need to include code for this mechanism.


"cryptographic operation" as before in the document rather than "cryptographic mechanism"? If the change of wording is deliberate it would be good to explain the difference between the two. Otherwise I'd suggest to stick to the same wording throughout the document.

A mechanism is a behavioral specification (an algorithm) which code can implement. An operation is the act of performing an instance of this algorithm. It's a subtle distinction that is not particularly useful there, so I think it having formal definitions would make the document harder to understand. But I find it jarring to use the same word throughout because neither “mechanism” nor “operation” works in all contexts. For example, here, “operation” doesn't work.

Ok make sense, maybe it is just the case of reviewing where "mechanism" is used and avoid it if "operation" is a good substitute. I am writing this because I have the feeling (but I may be wrong) that there is a portion of the documentation (not here) where "mechanism" is used quite systematically.

docs/proposed/psa-driver-interface.md

gilles-peskine-arm · 2020-08-06T21:01:29Z

@ronald-cron-arm Thanks for the review. I've submitted the changes where I understood and agreed with your feedback in #3542

…rial Add partial implementation of accelerator API defined in #3493

gilles-peskine-arm added 5 commits July 13, 2020 13:27

Add a directory for proposed specifications

abeb58e

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

PSA unified driver interface

2e66aca

Working draft of the PSA cryptography unified interface specification. Eventually this document will be under Arm PSA architecture ownership, but for the time being this draft is maintained in Mbed TLS. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Start Mbed TLS guides to PSA drivers

278e5eb

Driver developer's guide: introduction on how to write a driver. Driver integration guide: how to build Mbed TLS with drivers. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Automatically define location/lifetime constants

71db60b

PSA_KEY_LOCATION_acme, PSA_KEY_LIFETIME_acme Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Transparent drivers may have init functions too

bcce2ef

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

gilles-peskine-arm added component-crypto Crypto primitives and low-level interfaces PSA compliance labels Jul 13, 2020

gilles-peskine-arm added this to the July 2020 Sprint milestone Jul 13, 2020

gilles-peskine-arm self-assigned this Jul 13, 2020

gilles-peskine-arm mentioned this pull request Jul 13, 2020

PSA unified driver prototype #3313

Closed

stevew817 mentioned this pull request Jul 13, 2020

Rework PSA Crypto core to store keys in export representation #3492

Merged

2 tasks

mpg added the mbed TLS team label Jul 17, 2020

gilles-peskine-arm mentioned this pull request Jul 20, 2020

Add possibility to use pk_sign without providing a dummy private key #3496

Open

stevew817 mentioned this pull request Jul 20, 2020

Add partial implementation of accelerator API defined in #3493 #3501

Merged

3 tasks

gilles-peskine-arm added 6 commits August 3, 2020 11:21

Call driver entry point functions "entry point"

b6c43f6

Call the functions listed in the driver description "entry points". It's more precise than "functions", which could also mean any C function defined in the driver code. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Fix typos and copypasta

9214926

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Change driver persistent data to a callback interface

c1d388a

Rather than have some functions take the in-memory copy of the persistent data as argument, allow all of them to access the persistent data, including modifying it. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Remove the paragraph about declaring application needs

c93c4ed

It's out of scope. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Update open question section about public key storage

ab808e7

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

gilles-peskine-arm added the needs-review Every commit must be reviewed by at least two team members, label Aug 3, 2020

danh-arm modified the milestones: July 2020 Sprint, August 2020 Sprint Aug 3, 2020

stevew817 reviewed Aug 4, 2020

View reviewed changes

danh-arm reviewed Aug 5, 2020

View reviewed changes

gilles-peskine-arm added 7 commits August 5, 2020 22:18

Explain locations vs lifetimes

929ab8a

Locations aren't in the official PSA API specification yet (they've only be made public in Mbed TLS). Until version 1.0.1 of the API specification is out, this document needs to explain locations. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Add a link to the PSA API specification

3d1bcc6

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Fix typos

e265b9d

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Give some examples of purpsoses of pure-software transparent driver

ea739f0

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Minor clarifications

b320d08

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

With multiple applicable transparent drivers, the order is unspecified

5001da4

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

Clarify updates to the persistent state in storage

404e1db

Rename psa_crypto_driver_update_persistent_state to psa_crypto_driver_commit_persistent_state. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

gilles-peskine-arm commented Aug 5, 2020

View reviewed changes

danh-arm approved these changes Aug 6, 2020

View reviewed changes

danh-arm merged commit 0ca6d38 into Mbed-TLS:development Aug 6, 2020

ronald-cron-arm reviewed Aug 6, 2020

View reviewed changes

gilles-peskine-arm mentioned this pull request Aug 6, 2020

PSA unified driver specification: minor updates #3542

Merged

gilles-peskine-arm mentioned this pull request Aug 24, 2020

PSA: support arbitrary data storage from secure element drivers #3289

Closed

stevew817 mentioned this pull request Sep 3, 2020

Add multipart cipher accelerator support and test driver #3644

Merged

3 tasks

gilles-peskine-arm added a commit that referenced this pull request Sep 7, 2020

Merge pull request #3501 from stevew817/feature/transparent_drivers_t…

1ffec8f

…rial Add partial implementation of accelerator API defined in #3493

bensze01 modified the milestones: August 2020 Sprint, Driver interface: API design and prototype Jul 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PSA unified driver specification #3493

PSA unified driver specification #3493

gilles-peskine-arm commented Jul 13, 2020 •

edited

Loading

stevew817 Aug 4, 2020

gilles-peskine-arm Aug 4, 2020

stevew817 Aug 4, 2020

gilles-peskine-arm Aug 4, 2020

stevew817 Aug 4, 2020

gilles-peskine-arm Aug 4, 2020

stevew817 Aug 4, 2020

stevew817 Aug 4, 2020

gilles-peskine-arm Aug 4, 2020

stevew817 Aug 4, 2020

gilles-peskine-arm Aug 4, 2020

gilles-peskine-arm Aug 5, 2020

stevew817 Aug 4, 2020

gilles-peskine-arm Aug 4, 2020

danh-arm left a comment

danh-arm Aug 5, 2020

gilles-peskine-arm Aug 5, 2020

danh-arm Aug 5, 2020

gilles-peskine-arm Aug 5, 2020

danh-arm Aug 6, 2020

gilles-peskine-arm left a comment

gilles-peskine-arm Aug 5, 2020

gilles-peskine-arm Aug 5, 2020

gilles-peskine-arm Aug 5, 2020

gilles-peskine-arm Aug 5, 2020

danh-arm left a comment

danh-arm Aug 6, 2020

danh-arm commented Aug 6, 2020

ronald-cron-arm commented Aug 6, 2020

ronald-cron-arm left a comment

ronald-cron-arm Jul 21, 2020

gilles-peskine-arm Aug 6, 2020

ronald-cron-arm Aug 7, 2020

gilles-peskine-arm commented Aug 6, 2020


		An example use case for updating the persistent state at arbitrary times is to renew a key that is used to encrypt communications between the application processor and the secure element.

		`psa_crypto_driver_get_persistent_state` does not identify the calling driver, so the driver needs to remember which driver it's calling. This may require a thread-local variable in a multithreaded core. Is this ok?

		@@ -514,10 +514,6 @@ In a multithreaded environment, the driver may only call these two functions fro

		## How to use drivers from an application

		### Declaring which cryptographic mechanism an application needs


		### Fallback

		If a transparent driver entry point is part of a capability which has a true `"fallback"` property and returns `PSA_ERROR_NOT_SUPPORTED`, the built-in software implementation will be called instead. Any other value (`PSA_SUCCESS` or a different error code) is returned to the application.


		The interface is not fully implemented in Mbed TLS yet and is disabled by default. You can enable the experimental work in progress by setting `MBEDTLS_PSA_CRYPTO_DRIVERS` in the compile-time configuration. Please note that the interface may still change: until further notice, we do not guarantee backward compatibility with existing driver code when `MBEDTLS_PSA_CRYPTO_DRIVERS` is enabled.

		## Introduction


		[Req.location] Applications can tell which location values correspond to which secure element drivers.

		[Req.fallback] Accelerator drivers can specify that they do not fully support a cryptographic mechanism and that a fallback to core code may be necessary. Conversely, if an accelerator fully supports cryptographic mechanism, the core does not need to include code for this mechanism.

PSA unified driver specification #3493

PSA unified driver specification #3493

Conversation

gilles-peskine-arm commented Jul 13, 2020 • edited Loading

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

Choose a reason for hiding this comment

danh-arm 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

gilles-peskine-arm 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

danh-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danh-arm commented Aug 6, 2020

ronald-cron-arm commented Aug 6, 2020

ronald-cron-arm 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

gilles-peskine-arm commented Aug 6, 2020

gilles-peskine-arm commented Jul 13, 2020 •

edited

Loading