Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduction of a subsystem that implements the PSA Secure Storage API #75275

Closed
tomi-font opened this issue Jul 1, 2024 · 24 comments · Fixed by #76222
Closed

Introduction of a subsystem that implements the PSA Secure Storage API #75275

tomi-font opened this issue Jul 1, 2024 · 24 comments · Fixed by #76222
Assignees
Labels
Architecture Review Discussion in the Architecture WG required area: mbedTLS / PSA Crypto area: Secure Storage Secure Storage area: Security Security area: TF-M ARM Trusted Firmware-M (TF-M) RFC Request For Comments: want input from the community

Comments

@tomi-font
Copy link
Collaborator

tomi-font commented Jul 1, 2024

Introduction

The aim is to introduce a subsystem that implements the Platform Security Architecture (PSA) Secure Storage API. The subsystem will be available only on platforms without Trusted Firmware-M (TF-M) or similar secure processing environment that already implements the API.

The subsystem will allow storing data of any kind to different kinds of non-volatile memories (NVM).
Optionally, depending on device-specific features and on a best-effort basis, it may provide security to the assets stored through the PSA Secure Storage API. This is highly dependent on the hardware capabilities that are available and used on every device.
In any case, regardless of the storage medium or the means employed to secure the data at rest, the subsystem will not provide the same the level of security than that of a secure processing environment like TF-M, which provides isolation and protection guarantees.

The subsystem is completely separate from TF-M, and aims at providing an implementation of the PSA Secure Storage API on platforms that don't have one.
As a consequence of that, the subsystem will also enable the use of persistent keys in the PSA Crypto API.

Problem description

PSA Crypto

Zephyr has decided to adopt the PSA Crypto API as the single API to be used for cryptographic operations (#43712), and recent efforts have been made towards that goal.

The PSA Crypto API contains key management functions that must be called to be able to perform cryptographic operations that make use of keys. Under the hood, those key management functions call into the PSA Internal Trusted Storage (ITS) API (which is part of the PSA Secure Storage API) to store the key material to persistent storage.

In Zephyr, Mbed TLS is the PSA Crypto provider. However it doesn't provide an implementation of the PSA ITS API. So the current state is that, on platforms without PSA ITS provider (such as TF-M), keys imported into PSA Crypto for use are kept in a volatile key store, which means that they must be imported into PSA after every power cycle.
In practice it's worse than that: pretty much every use of PSA Crypto keys in Zephyr imports (and destroys) keys for every cryptographic operation.

The PSA Crypto API is designed to use cryptographic keys only by reference (psa_key_id_t) to avoid having the calling code handle the key material for every cryptographic operation, because this is detrimental to the overall security.
Also, when writing code that relies on keys to perform cryptographic operations, a developer cannot currently just do the needed operations. The burden of key management is present everywhere those operations are called, and there is no standard way to provide any kind of security to the key material at rest. It's up to every developer to implement that, over and over again.

This is something that will change once the PSA ITS API is made available on all platforms.

Uniform storage solution

Additionally, Zephyr currently doesn't have a standardized API to store data to NVM with the possibility of providing device-specific protection guarantees (encryption, isolation, etc.).

Platforms with a PSA Secure Storage provider are already able to do that, but in a non-portable way that cannot be extended unconditionally to the whole codebase, which hurts its adoption for them as well.

Implementing and standardizing on the PSA Secure Storage API makes it possible to provide support for a generic solution for storing critical assets that code relies on for operation.
It will ensure there is a portable way to take advantage of different devices' security features for storage, including TF-M-enabled platforms.

Because the security that can be provided is inherently tied to the security features every device provides, the level of security of the storage solution will vary.
It is however in any case better than that current state of storing data in clear; the level of security increases along with the hardware features made available on a given device.

Also note that, because the goal is to standardize on an API and make it available on all the platforms, the implementation provided by Zephyr will not be fully compliant with the specification. PSA has a specific model in mind, and specifies certain requirements that are not compatible with all devices.

Proposed change

The subsystem will provide an implementation of the PSA Secure Storage API on all platforms that don't already have one such that the API will be usable on every platform, without hard requirement on security features.

The PSA Secure Storage API specification assumes security features are used to protect the stored assets. The level of security that can be claimed on a specific device is dependent on the security features that are available and enabled. If the device doesn't make its security features available to the API, then the implementation can't claim that it provides security. In this case it will only be providing functional support for the API.
Certifiability is also not in the scope of this proposal. It is the end users' responsibility to make sure that the implementation they use is fully compliant and certifiable, if it is so desired.

The initial proposed implementation is based on something that already exists and lives in the nRF Connect SDK (NCS) as the trusted storage library. A look can be taken to see its current architecture there, however it will be changed and NCS specifics such as the HUK part will be removed.

Detailed RFC

The intent is to have a customizable PSA Secure Storage provider so that different devices and use cases can be supported.

To this end, the subsystem will be modular and configurable with different implementations, including custom ones.

Proposed change (detailed)

Below is a proposal of the initial architecture of the subsystem.

Exposed interface

The interface exposed to Zephyr and application code by the subsystem is the PSA Secure Storage API, which consists of the Internal Trusted Storage (ITS) and the Protected Storage (PS) APIs.

From there the subsystem takes on the task of transforming and storing the data.

PSA ITS API implementation

The implementation calls into the (ITS-specific) modules described below. It handles the logic pertaining to the PSA ITS API and orchestrates the transformation and storage of the data.
It can be replaced with a custom implementation of the whole PSA ITS API if wanted.

Transformation of the data

This module is responsible for the transformation of the data before it's written to and after it's read from NVM. Any implementation may be used by simply implementing the relevant API, allowing to cater for device-specific capabilities. For instance, if a device chooses to store the ITS data to a hardware-protected secure element, it may not need to protect the data in any way before storing it.

Zephyr will provide an implementation of this module that uses authenticated encryption with associated data (AEAD), which can ensure the confidentiality, integrity and authenticity of the data, but only when a secure encryption key and a random entropy source are provided. How the encryption key is provided, what AEAD scheme is used and how nonces are provided are all separately configurable and custom implementations can replace the default ones by implementing the relevant functions.

Storage to NVM

This module is responsible for storing the data to and retrieving it from the storage medium.

It can be implemented with any type of backend for persistent storage, allowing for example to store data off-chip on a secure element.

Zephyr will provide some implementation(s) of this module using NVM API(s) like the settings subsystem.

Again, custom implementations may be used instead of the existing one(s) by implementing the relevant API (set/get/remove functions).

PSA PS API implementation

As of the initial implementation, the PS API won't have an implementation of its own and will by default call into that of the ITS API (a well-expected use case - see 2.4. The Internal Trusted Storage API: [...] it is expected that many platforms will have the Protected Storage API call directly into the Internal Trusted Storage API).

External flash support (that the PSA PS covers) will thus not be supported right away. It is one of the things that will be addressed by follow-up work to give the PS API its own implementation.

Anyhow, this is something that can already be circumvented by providing one's own implementation of the PSA PS API.

Dependencies

The PSA Crypto API is dependent on the PSA ITS API for a proper functioning.
Having a PSA ITS provider on all platforms will help Zephyr in its adoption of the PSA Crypto API.

Concerns and Unresolved Questions

Giving a false sense of security

The subsystem shall not give a false sense of security to the end users if no hardware security features are used to securely protect the data it stores.
Documentation should clearly explain this and warnings could be output at build and/or boot time if the implementation in use may not be truly secure.

ID collisions

When Mbed TLS's implementation of the PSA Crypto API will have access to the PSA ITS API, it will start storing all persistent keys using the provided psa_key_id_t as the psa_storage_uid_t when calling into the PSA ITS API.

This means that collisions will happen if the same numerical ID values are used for both PSA Crypto keys and PSA ITS entries. Different approaches to this are possible:

  • Not do anything about it. Document this as a limitation, possibly stating that to avoid collisions PSA ITS entry IDs should be > UINT32_MAX (because PSA Crypto's psa_key_id_t is 32 bits while PSA ITS's psa_storage_uid_t is 64 bits).

  • Try to enforce collisions not happening in a way that is transparent for the calling code, e.g. by forcefully setting one of the 32 upper bits of the PSA ITS entry UIDs when calls don't come from Mbed TLS. For that, some mechanism to differentiate non-Mbed TLS callers would be nedded. Could be done by:

    • Using GCC's __builtin_return_address() and guessing whether the call comes from Mbed TLS? GCC-specific and fragile.
    • Defining the PSA ITS API functions as macros for Mbed TLS. That would allow the implementation to differentiate when the call comes from there.
@tomi-font tomi-font added the RFC Request For Comments: want input from the community label Jul 1, 2024
Copy link

github-actions bot commented Jul 1, 2024

Hi @tomi-font! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

@tomi-font
Copy link
Collaborator Author

Updated the proposal, specifically:

  • Updated Problem description.
  • Added PSA PS implementation.
  • Expanded on the ID collisons concern.

@ceolin
Copy link
Member

ceolin commented Jul 17, 2024

Some points from my side:

  1. It is not clear / defined the HW dependencies to guarantee confidentiality and authenticity. Where the AEAD key is stored / generated ?
  2. For the ID collision problem, the macro idea seems good to me. We can use upper bits to define Zephyr's namespace.
    The only possible drawback (but maybe it is not a problem) is that we are limited to one namespace for Zephyr's code and application.

The general proposal / idea looks great to me :)

@JordanYates
Copy link
Collaborator

It is not clear / defined the HW dependencies to guarantee confidentiality and authenticity. Where the AEAD key is stored / generated ?

From my understanding this is entirely dependent on the implementation and the hardware capabilities. A truly secure solution is not possible unless the HW provides a persistent storage location for encryption keys that cannot be accessed by software but can be used for crypto operations (i.e. nRF key storage/cryptocell).

But even if that is not available you can still do better than storing the data in plaintext. For example, my fallback solution for the key is to calculate a SHA-256 hash over the unique hardware ID, which at least means that data is not encrypted with the same key on any two devices.

@nashif nashif added the Architecture Review Discussion in the Architecture WG required label Jul 18, 2024
@tomi-font
Copy link
Collaborator Author

It is not clear / defined the HW dependencies to guarantee confidentiality and authenticity. Where the AEAD key is stored / generated ?

@JordanYates mostly already addressed this. The key isn't stored, it's generated when needed (when the API is called). The default behavior is to use the SHA-256 hash of a HW identifier if available via the HW info API, or of the UID of the entry being processed, which is admittedly not very secure as UIDs could be guessed, but still better than nothing. Some implementation must be available on all platforms for the API to be usable unconditionally.

For the ID collision problem, the macro idea seems good to me. We can use upper bits to define Zephyr's namespace.
The only possible drawback (but maybe it is not a problem) is that we are limited to one namespace for Zephyr's code and application.

Yeah. This limitation comes from the PSA Secure Storage API which defines entry UIDs as a single 64-bit field without defined ranges.
But if/when Zephyr code starts using key IDs/entry UIDs for persistent data, we could define ranges of values to be used by different callers that are not really aware of each other (different subsystems, application code...).

@tomi-font tomi-font moved this to In Progress in RFC Backlog Jul 21, 2024
@Laczen
Copy link
Collaborator

Laczen commented Jul 23, 2024

@tomi-font from the documentation and your last remark it seems that the api to use secure storage is limited to using a identifier as a key for data storage. Although nvs could have been used like this for settings (limited to a 16bit identifier) it was never used in this way because it is not flexible enough to support multiple subsystems and varying keys (e.g. bt id's).

It would be good to include this limitation to validate whether this method should be considered as a method to securely store any kind of data.

@tomi-font
Copy link
Collaborator Author

@tomi-font from the documentation and your last remark it seems that the api to use secure storage is limited to using a identifier as a key for data storage. Although nvs could have been used like this for settings (limited to a 16bit identifier) it was never used in this way because it is not flexible enough to support multiple subsystems and varying keys (e.g. bt id's).

Though nothing forbids using NVS directly if wanted; an implementation of the storing module could do that by properly packing the UIDs into 16-bit identifiers, obviously introducing the constraint of less possible UIDs.

It would be good to include this limitation to validate whether this method should be considered as a method to securely store any kind of data.

I'm not following you here. Include what limitation and where? Validate what method?

@Laczen
Copy link
Collaborator

Laczen commented Jul 23, 2024

It would be good to include this limitation to validate whether this method should be considered as a method to securely store any kind of data.

I'm not following you here. Include what limitation and where?

The limitation of only being able to store data by a "integer" identifier. Suppose you would like to store data for some node that has an identifier e.g. a MAC address, this can very easily be mapped to "/net/AA:BB:CC:DD/prop1"=..., "/net/AA:BB:CC:DD/prop2"=..., ... while it is a lot more involving to map this to "integers" especially when it is needed to delete data from storage (keeping track of unused identifiers, ...).

Validate what method?

Validation of:
"The subsystem will allow securely storing data of any kind, with the actual security provided depending on what security features are available on a given platform.",
is the secure storage api (with its limitations) a good api to provide this.

@tomi-font
Copy link
Collaborator Author

The limitation of only being able to store data by a "integer" identifier. Suppose you would like to store data for some node that has an identifier e.g. a MAC address, this can very easily be mapped to "/net/AA:BB:CC:DD/prop1"=..., "/net/AA:BB:CC:DD/prop2"=..., ... while it is a lot more involving to map this to "integers" especially when it is needed to delete data from storage (keeping track of unused identifiers, ...).

This limitation is part of the PSA Secure Storage API: https://arm-software.github.io/psa-api/storage/1.0/api/api.html#c.psa_storage_uid_t

Validation of:
"The subsystem will allow securely storing data of any kind, with the actual security provided depending on what security features are available on a given platform.",
is the secure storage api (with its limitations) a good api to provide this.

The API is not a free choice, it's purely and simply the PSA Secure Storage API. And I think that we are past the point of wondering whether it's the right choice because

  • Zephyr is moving towards having the PSA Crypto API as the only API, and so is Mbed TLS;
  • Mbed TLS calls the PSA ITS API for storage of persistent keys;
  • TF-M implements and exposes the PSA Secure Storage API;
  • we don't want to create more fragmentation of APIs.

The limitation you are talking about can be solved by implementing a translation from e.g. string identifiers to the numerical psa_storage_uid_t ones. 64 bits is a lot.

Then, if there are serious limitations in the PSA API specification, I imagine we could participate in the discussion to shape its future.

@omkar3141
Copy link
Collaborator

One question:
#1 The secure storage subsystem could be incredibly useful not just for on chip storage, but also off-chip storage in external flash. Is external flash support considered here?

One suggestion
#1 IMO, settings subsystem as it is envisioned and implemented by various modules in zephyr is quite anti-embedded and inefficient. It has enormous amount of overhead in the form of (a current pattern established in zephyr) long file names (e.g. "bt/mesh/..." vs "bm/..."). Often to store couple of bytes of frequently changing state data, the overhead is 15+ bytes. For many practical secure embedded applications frequently changing state data needs to be stored (such as last known state of the light bulb with its various internal values, or replay protection sequence number for an incoming message from given address). Though not directly impacting the secure storage, but designing secure storage assuming existing settings subsystem is used as a storage mechanism could lead to some inefficiencies. Almost all of the data for commercial/industrial products can be classified as 'secure' and will end up requiring usage of this library. Bluetooth Mesh literally requires hundreds of 'settings' to be stored, some of them are frequently changing. Potentially many of them will be classified as 'security related'. Therefore, please consider - minimizing flash wear, duplicate detection, non-existing entry deletion, and reporting corrupted entry to top level application code (through some callbacks) - aspects in this design.

@str4t0m
Copy link
Collaborator

str4t0m commented Jul 23, 2024

I do have a question regarding the generation of the AEAD key. In Proposed change you write:

A look can be taken to see its current architecture there, however it is expected to change and NCS specifics such as the HUK part will be removed.

Do you really mean to remove the HUK part or do have the intention to define an API which different platforms can implement to derive such a key?

@henrikbrixandersen
Copy link
Member

Architecture WG 2024-07-23:

@tomi-font
Copy link
Collaborator Author

One question:
#1 The secure storage subsystem could be incredibly useful not just for on chip storage, but also off-chip storage in external flash. Is external flash support considered here?

It is. The PSA PS API is meant for external storage. Though, as stated in the RFC, the proposed initial implementation won't support external storage. The PSA PS API will still be available, but it will call directly into the ITS API.
This is to get the most important things in first and not have an overwhelming amount of changes at once. (The PSA PS depends on the PSA ITS for storage of replay protection values.)
But, seeing that there is strong interest for external storage, this would definitely be a follow-up item.

One suggestion
#1 IMO, settings subsystem as it is envisioned and implemented by various modules in zephyr is quite anti-embedded and inefficient. It has enormous amount of overhead in the form of (a current pattern established in zephyr) long file names (e.g. "bt/mesh/..." vs "bm/..."). Often to store couple of bytes of frequently changing state data, the overhead is 15+ bytes. For many practical secure embedded applications frequently changing state data needs to be stored (such as last known state of the light bulb with its various internal values, or replay protection sequence number for an incoming message from given address). Though not directly impacting the secure storage, but designing secure storage assuming existing settings subsystem is used as a storage mechanism could lead to some inefficiencies. Almost all of the data for commercial/industrial products can be classified as 'secure' and will end up requiring usage of this library. Bluetooth Mesh literally requires hundreds of 'settings' to be stored, some of them are frequently changing. Potentially many of them will be classified as 'security related'. Therefore, please consider - minimizing flash wear, duplicate detection, non-existing entry deletion, and reporting corrupted entry to top level application code (through some callbacks) - aspects in this design.

Thanks for the feedback. I get the concerns about the use of the settings subsystem. I didn't do much thinking about the storage solution and simply went with the option that had been chosen in NCS: the settings subsystem. The API it exposes is convenient (e.g. the setting names are not limited to a numerical value smaller than psa_storage_uid_t).
However one thing I can assure you is that its use is in no way baked into the secure storage subsystem. It's just one of the possible (storing module) implementations. It's just a matter of implementing set/get/remove functions.
One idea that has come up already by different parties is to use NVS directly. This (or any other storage solution) could be provided as an option in Zephyr. It's just that its implementation would be more complex than the settings subsystem one (need to shrink the entry UIDs or implement a way around it, consider the partitioning...).
So again, this can (and will) definitely be improved. I think it's more a matter of first getting something in. There has already been remarks that there are lots of changes in the PR.

@tomi-font
Copy link
Collaborator Author

Do you really mean to remove the HUK part or do have the intention to define an API which different platform can implement to derive such a key?

The initial implementation indeed doesn't have any mention of HUK. The HUK library in NCS is highly Nordic-specific.
Though I think it could be worth adding an API to Zephyr that would provide similar functionality, with drivers for different platforms (similarly to the HW info API).
However this would be a separate thing, e.g. follow-up work. cc @ceolin

@tomi-font
Copy link
Collaborator Author

I have reworked the RFC to clarify the intent, the scope and the limitations. I invite everyone to give a read to the updated version.

Architecture WG 2024-07-23:

I realized that I should have focused more on explaining the different components that are at play and consider more that the audience was likely not familiar with those topics.
Please let me know what you think of the revised RFC.

tomi-font added a commit to tomi-font/zephyr that referenced this issue Jul 29, 2024
tomi-font added a commit to tomi-font/zephyr that referenced this issue Jul 29, 2024
tomi-font added a commit to tomi-font/zephyr that referenced this issue Aug 2, 2024
@tomi-font tomi-font removed the area: Storage Storage subsystem label Aug 2, 2024
@ceolin ceolin moved this to RFC / Discussion required in Security Aug 5, 2024
@carlescufi carlescufi moved this from In Progress to Done in Architecture Review Aug 29, 2024
@ceolin ceolin moved this from RFC / Discussion required to In Progress in Security Aug 29, 2024
@tomi-font tomi-font linked a pull request Sep 4, 2024 that will close this issue
tomi-font added a commit to tomi-font/zephyr that referenced this issue Sep 10, 2024
Implements RFC zephyrproject-rtos#75275.
See the added documentation for more information.

Signed-off-by: Tomi Fontanilles <[email protected]>
tomi-font added a commit to tomi-font/zephyr that referenced this issue Sep 11, 2024
Implements RFC zephyrproject-rtos#75275.

See also the added documentation and the PR
(zephyrproject-rtos#76222)
for more information.

Signed-off-by: Tomi Fontanilles <[email protected]>
tomi-font added a commit to tomi-font/zephyr that referenced this issue Sep 13, 2024
tomi-font added a commit to tomi-font/zephyr that referenced this issue Sep 13, 2024
tomi-font added a commit to tomi-font/zephyr that referenced this issue Sep 13, 2024
tomi-font added a commit to tomi-font/zephyr that referenced this issue Sep 13, 2024
Implements RFC zephyrproject-rtos#75275.

See also the PR (zephyrproject-rtos#76222)
for more information.

Signed-off-by: Tomi Fontanilles <[email protected]>
tomi-font added a commit to tomi-font/zephyr that referenced this issue Sep 25, 2024
Implements RFC zephyrproject-rtos#75275.

See also the PR (zephyrproject-rtos#76222)
for more information.

Signed-off-by: Tomi Fontanilles <[email protected]>
@tomi-font tomi-font added the area: Secure Storage Secure Storage label Oct 7, 2024
carlescufi pushed a commit that referenced this issue Oct 7, 2024
Implements RFC #75275.

See also the PR (#76222)
for more information.

Signed-off-by: Tomi Fontanilles <[email protected]>
@github-project-automation github-project-automation bot moved this from In Progress to Done in RFC Backlog Oct 7, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Security Oct 7, 2024
HubertYGuan pushed a commit to HubertYGuan/zephyr that referenced this issue Oct 10, 2024
Implements RFC zephyrproject-rtos#75275.

See also the PR (zephyrproject-rtos#76222)
for more information.

Signed-off-by: Tomi Fontanilles <[email protected]>
coreboot-org-bot pushed a commit to coreboot/zephyr-cros that referenced this issue Oct 11, 2024
Implements RFC zephyrproject-rtos/zephyr#75275.

See also the PR (zephyrproject-rtos/zephyr#76222)
for more information.

(cherry picked from commit bf0e6d7)

Original-Signed-off-by: Tomi Fontanilles <[email protected]>
GitOrigin-RevId: bf0e6d7
Cr-Build-Id: 8734380568354140561
Cr-Build-Url: https://cr-buildbucket.appspot.com/build/8734380568354140561
Copybot-Job-Name: zephyr-main-copybot-downstream
Change-Id: I1babaa0d25b3a12261f2bbc68ddbe7ca16124b14
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5914244
Tested-by: Jeremy Bettis <[email protected]>
Tested-by: ChromeOS Prod (Robot) <[email protected]>
Reviewed-by: Jeremy Bettis <[email protected]>
ycsin pushed a commit to ycsin/zephyr that referenced this issue Oct 12, 2024
Implements RFC zephyrproject-rtos#75275.

See also the PR (zephyrproject-rtos#76222)
for more information.

Signed-off-by: Tomi Fontanilles <[email protected]>
mpenate-ellenbytech pushed a commit to mpenate-ellenbytech/zephyr that referenced this issue Nov 14, 2024
Implements RFC zephyrproject-rtos#75275.

See also the PR (zephyrproject-rtos#76222)
for more information.

Signed-off-by: Tomi Fontanilles <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture Review Discussion in the Architecture WG required area: mbedTLS / PSA Crypto area: Secure Storage Secure Storage area: Security Security area: TF-M ARM Trusted Firmware-M (TF-M) RFC Request For Comments: want input from the community
Projects
Status: Done
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

10 participants