mbedtls: configurable PSA ITS provider #86184
+223
−4
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -243,3 +243,27 @@ config APP_LINK_WITH_MBEDTLS | |
| issues for 'app'. | ||
|
|
||
| endif # MBEDTLS | ||
|
|
||
| choice MBEDTLS_PSA_ITS_BACKEND | ||
| prompt "PSA Internal-Trusted-Storage backend implementation" | ||
|
|
||
| config MBEDTLS_PSA_ITS_BACKEND_TFM | ||
| bool "Trusted-Firmware-M" | ||
| depends on BUILD_WITH_TFM | ||
|
|
||
| config MBEDTLS_PSA_ITS_BACKEND_SECURE_STORAGE | ||
| bool "Zephyr Secure Storage" | ||
| depends on !BUILD_WITH_TFM | ||
|
|
||
| config MBEDTLS_PSA_ITS_BACKEND_MBEDTLS_FILE | ||
| bool "MbedTLS builtin file" | ||
| depends on !BUILD_WITH_TFM | ||
|
There was a problem hiding this comment. I think this should also depend on |
||
| depends on ARCH_POSIX | ||
| select MBEDTLS | ||
| select MBEDTLS_PSA_CRYPTO_C | ||
|
Comment on lines
+262
to
+263
There was a problem hiding this comment. In your other PR you were removing There was a problem hiding this comment.
There was a problem hiding this comment. In some places we also have There was a problem hiding this comment. Because your current secure storage backends end up enabling There was a problem hiding this comment. Or there was some link over a long chain of options that ended up back there (probably through There was a problem hiding this comment.
Ah okay. Yeah that's annoying. Something likely needs to be reworked but that's out of scope. |
||
| help | ||
| Native filesystem based implementation of PSA ITS. Note that multiple | ||
| instances of the sample application running at the same time will all | ||
| share the same backed. | ||
|
|
||
| endchoice | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -486,6 +486,12 @@ | |
| #define MBEDTLS_PSA_CRYPTO_STORAGE_C | ||
| #endif | ||
|
|
||
| #if defined(CONFIG_MBEDTLS_PSA_ITS_BACKEND_MBEDTLS_FILE) | ||
| #define MBEDTLS_PSA_CRYPTO_STORAGE_C | ||
| #define MBEDTLS_PSA_ITS_FILE_C | ||
| #define MBEDTLS_FS_IO | ||
|
Comment on lines
+490
to
+492
There was a problem hiding this comment. I think it makes sense to turn those into Kconfig options. Especially |
||
| #endif | ||
|
|
||
| #endif /* CONFIG_MBEDTLS_PSA_CRYPTO_C */ | ||
|
|
||
| #if defined(CONFIG_MBEDTLS_PSA_STATIC_KEY_SLOTS) | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| /* | ||
| * Copyright (c) 2025 Embeint Inc. | ||
| * | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| #include <psa/crypto_values.h> |
|
There was a problem hiding this comment. I am really not a fan of copying header files, especially considering we already have a Mbed TLS has made it private probably for a good reason. And they have no plans whatsoever to make it public. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,153 @@ | ||
| /* | ||
| * Copyright The Mbed TLS Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * Copy of `mbedtls/library/psa_crypto_its.h` until it is available at the same | ||
| * include path as TF-M (`psa/internal_trusted_storage.h`). | ||
| */ | ||
|
|
||
| #ifndef PSA_CRYPTO_ITS_H | ||
| #define PSA_CRYPTO_ITS_H | ||
|
|
||
| #include <stddef.h> | ||
| #include <stdint.h> | ||
|
|
||
| #include <psa/crypto_types.h> | ||
| #include <psa/crypto_values.h> | ||
|
|
||
| #ifdef __cplusplus | ||
| extern "C" { | ||
| #endif | ||
|
|
||
| /** \brief Flags used when creating a data entry | ||
| */ | ||
| typedef uint32_t psa_storage_create_flags_t; | ||
|
|
||
| /** \brief A type for UIDs used for identifying data | ||
| */ | ||
| typedef uint64_t psa_storage_uid_t; | ||
|
|
||
| /** No flags to pass */ | ||
| #define PSA_STORAGE_FLAG_NONE 0 | ||
| /** | ||
| * The data associated with the uid will not be able to be modified or deleted. | ||
| * Intended to be used to set bits in `psa_storage_create_flags_t` | ||
| */ | ||
| #define PSA_STORAGE_FLAG_WRITE_ONCE (1 << 0) | ||
|
|
||
| /** | ||
| * \brief A container for metadata associated with a specific uid | ||
| */ | ||
| struct psa_storage_info_t { | ||
| uint32_t size; /**< The size of the data associated with a uid **/ | ||
| psa_storage_create_flags_t flags; /**< The flags set when the uid was created **/ | ||
| }; | ||
|
|
||
| /** Flag indicating that \ref psa_storage_create and \ref psa_storage_set_extended are supported */ | ||
| #define PSA_STORAGE_SUPPORT_SET_EXTENDED (1 << 0) | ||
|
|
||
| #define PSA_ITS_API_VERSION_MAJOR 1 | ||
| #define PSA_ITS_API_VERSION_MINOR 1 | ||
|
|
||
| /** | ||
| * \brief create a new or modify an existing uid/value pair | ||
| * | ||
| * \param[in] uid the identifier for the data | ||
| * \param[in] data_length The size in bytes of the data in `p_data` | ||
| * \param[in] p_data A buffer containing the data | ||
| * \param[in] create_flags The flags that the data will be stored with | ||
| * | ||
| * \return A status indicating the success/failure of the operation | ||
| * | ||
| * \retval #PSA_SUCCESS The operation completed successfully | ||
| * \retval #PSA_ERROR_NOT_PERMITTED The operation failed because the provided `uid` value | ||
| * was already created with PSA_STORAGE_FLAG_WRITE_ONCE | ||
| * \retval #PSA_ERROR_NOT_SUPPORTED The operation failed because one or more of the flags | ||
| * provided in `create_flags` is not supported or is not | ||
| * valid | ||
| * \retval #PSA_ERROR_INSUFFICIENT_STORAGE The operation failed because there was insufficient | ||
| * space on the storage medium | ||
| * \retval #PSA_ERROR_STORAGE_FAILURE The operation failed because the physical storage has | ||
| * failed (Fatal error) | ||
| * \retval #PSA_ERROR_INVALID_ARGUMENT The operation failed because one of the provided | ||
| * pointers(`p_data`) is invalid, for example is `NULL` | ||
| * or references memory the caller cannot access | ||
| */ | ||
| psa_status_t psa_its_set(psa_storage_uid_t uid, | ||
| uint32_t data_length, | ||
| const void *p_data, | ||
| psa_storage_create_flags_t create_flags); | ||
|
|
||
| /** | ||
| * \brief Retrieve the value associated with a provided uid | ||
| * | ||
| * \param[in] uid The uid value | ||
| * \param[in] data_offset The starting offset of the data requested | ||
| * \param[in] data_length the amount of data requested (and the minimum allocated size of the | ||
| * `p_data` buffer) | ||
| * \param[out] p_data The buffer where the data will be placed upon successful completion | ||
| * \param[out] p_data_length The amount of data returned in the p_data buffer | ||
| * | ||
| * | ||
| * \return A status indicating the success/failure of the operation | ||
| * | ||
| * \retval #PSA_SUCCESS The operation completed successfully | ||
| * \retval #PSA_ERROR_DOES_NOT_EXIST The operation failed because the provided `uid` value was | ||
| * not found in the storage | ||
| * \retval #PSA_ERROR_STORAGE_FAILURE The operation failed because the physical storage has | ||
| * failed (Fatal error) | ||
| * \retval #PSA_ERROR_DATA_CORRUPT The operation failed because stored data has been corrupted | ||
| * \retval #PSA_ERROR_INVALID_ARGUMENT The operation failed because one of the provided | ||
| * pointers(`p_data`, `p_data_length`) is invalid. For example | ||
| * is `NULL` or references memory the caller cannot access. In | ||
| * addition, this can also happen if an invalid offset was | ||
| * provided. | ||
| */ | ||
| psa_status_t psa_its_get(psa_storage_uid_t uid, | ||
| uint32_t data_offset, | ||
| uint32_t data_length, | ||
| void *p_data, | ||
| size_t *p_data_length); | ||
|
|
||
| /** | ||
| * \brief Retrieve the metadata about the provided uid | ||
| * | ||
| * \param[in] uid The uid value | ||
| * \param[out] p_info A pointer to the `psa_storage_info_t` struct that will be populated | ||
| * with the metadata | ||
| * | ||
| * \return A status indicating the success/failure of the operation | ||
| * | ||
| * \retval #PSA_SUCCESS The operation completed successfully | ||
| * \retval #PSA_ERROR_DOES_NOT_EXIST The operation failed because the provided uid value was | ||
| * not found in the storage | ||
| * \retval #PSA_ERROR_DATA_CORRUPT The operation failed because stored data has been corrupted | ||
| * \retval #PSA_ERROR_INVALID_ARGUMENT The operation failed because one of the provided | ||
| * pointers(`p_info`) is invalid, for example is `NULL` or | ||
| * references memory the caller cannot access | ||
| */ | ||
| psa_status_t psa_its_get_info(psa_storage_uid_t uid, | ||
| struct psa_storage_info_t *p_info); | ||
|
|
||
| /** | ||
| * \brief Remove the provided key and its associated data from the storage | ||
| * | ||
| * \param[in] uid The uid value | ||
| * | ||
| * \return A status indicating the success/failure of the operation | ||
| * | ||
| * \retval #PSA_SUCCESS The operation completed successfully | ||
| * \retval #PSA_ERROR_DOES_NOT_EXIST The operation failed because the provided key value was | ||
| * not found in the storage | ||
| * \retval #PSA_ERROR_NOT_PERMITTED The operation failed because the provided key value was | ||
| * created with PSA_STORAGE_FLAG_WRITE_ONCE | ||
| * \retval #PSA_ERROR_STORAGE_FAILURE The operation failed because the physical storage has | ||
| * failed (Fatal error) | ||
| */ | ||
| psa_status_t psa_its_remove(psa_storage_uid_t uid); | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
|
|
||
| #endif /* PSA_CRYPTO_ITS_H */ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems those choice options don't really bring value. I think we could just have the option to enable the persistent keys in the Mbed TLS PSA Crypto core (by enabling its own internal ITS implementation).
Though that would mean introducing a (helper) Kconfig option that wouldn't have a corresponding build symbol in Mbed TLS.
The alternative would be to just expose the 3 Mbed TLS defines that are needed as Kconfig options and let whoever wants to use that enable them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing the application cares about is support for the PSA key operations (
psa_generate_key, etc).To support
PSA_KEY_LIFETIME_PERSISTENT, there needs to be a single implementation of the PSA ITS API.That seems like the perfect use of a
choicesymbol to me, even if some options are only used implicitly (MBEDTLS_PSA_ITS_BACKEND_TFM), since Kconfig is now doing the work of ensuring the mutual exclusion.Otherwise when implementation N of the ITS comes along, every other implementation needs to have a
depends on !ITS_Nadded.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the logic, but I don't think we'll see any other ITS implementation come along, and all we really need here is a single Kconfig option to handle the Mbed TLS ITS implementation you want to make available.
Also for example for the TF-M case it's more complex than that. TF-M could be enabled but without the ITS partition, which means there wouldn't be ITS even though there is TF-M.
As of now I don't see benefits in having a
choiceas the TF-M and secure storage options are already handled by preexisting configuration.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not upstream, but there are enough problems currently with the Secure Storage API (see #86177) that in the short term at least my downstream is more likely to implement the ITS API than the Secure Storage one.