diff --git a/subsys/fs/zms/zms.c b/subsys/fs/zms/zms.c index 57ac0ad52e01b..e00506dec4b4f 100644 --- a/subsys/fs/zms/zms.c +++ b/subsys/fs/zms/zms.c @@ -1362,7 +1362,7 @@ int zms_mount(struct zms_fs *fs) * 1 close ATE, 1 empty ATE, 1 GC done ATE, 1 Delete ATE, 1 ID/Value ATE */ if (fs->sector_size < ZMS_MIN_ATE_NUM * fs->ate_size) { - LOG_ERR("Invalid sector size, should be at least %u", + LOG_ERR("Invalid sector size, should be at least %zu", ZMS_MIN_ATE_NUM * fs->ate_size); } diff --git a/subsys/secure_storage/Kconfig.its_store b/subsys/secure_storage/Kconfig.its_store index 9e4d9b6502059..05ebf72ca8f64 100644 --- a/subsys/secure_storage/Kconfig.its_store +++ b/subsys/secure_storage/Kconfig.its_store @@ -4,12 +4,33 @@ choice SECURE_STORAGE_ITS_STORE_IMPLEMENTATION prompt "ITS store module implementation" +DT_ITS_PARTITION := $(dt_chosen_path,secure_storage_its_partition) +DT_CHOSEN_Z_SETTINGS_PARTITION := zephyr,settings-partition +DT_SETTINGS_PARTITIION := $(dt_chosen_path,$(DT_CHOSEN_Z_SETTINGS_PARTITION)) +DT_STORAGE_PARTITION := $(dt_nodelabel_path,storage_partition) + +config SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS + bool "ITS store module implementation using ZMS for storage" + depends on FLASH_HAS_DRIVER_ENABLED \ + && $(dt_path_enabled,$(DT_ITS_PARTITION)) \ + && $(dt_node_has_compat,$(dt_node_parent,$(DT_ITS_PARTITION)),fixed-partitions) + select ZMS + help + This implementation of the ITS store module makes direct use of ZMS for storage. + It needs a `secure_storage_its_partition` devicetree chosen property that points + to a fixed storage partition that will be dedicated to the ITS. It has lower + overhead compared to the settings-based implementation, both in terms of runtime + execution and storage space, and also ROM footprint if the settings subsystem is disabled. + As this implementations directly maps the PSA storage UIDs to ZMS entry IDs, it limits + their values to the first 30 bits. + config SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS bool "ITS store module implementation using the settings subsystem for storage" - DT_STORAGE_PARTITION := $(dt_nodelabel_path,storage_partition) depends on FLASH_HAS_DRIVER_ENABLED \ - && $(dt_path_enabled,$(DT_STORAGE_PARTITION)) \ - && $(dt_node_has_compat,$(dt_node_parent,$(DT_STORAGE_PARTITION)),fixed-partitions) + && (($(dt_path_enabled,$(DT_SETTINGS_PARTITIION)) \ + && $(dt_node_has_compat,$(dt_node_parent,$(DT_SETTINGS_PARTITIION)),fixed-partitions))\ + || ($(dt_path_enabled,$(DT_STORAGE_PARTITION)) \ + && $(dt_node_has_compat,$(dt_node_parent,$(DT_STORAGE_PARTITION)),fixed-partitions))) imply FLASH_MAP imply NVS select SETTINGS @@ -25,6 +46,20 @@ config SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_CUSTOM endchoice # SECURE_STORAGE_ITS_STORE_IMPLEMENTATION +if SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS + +config SECURE_STORAGE_ITS_STORE_ZMS_SECTOR_SIZE + int "Sector size of the ZMS partition" + default 4096 + help + The sector size impacts the runtime behavior of ZMS and restricts the maximum + ITS entry data size (which is the sector size minus ZMS and ITS overhead). + Changing it will result in loss of existing data stored on a partition. + It must be a multiple of the flash page size on devices that require an erase. + See the ZMS documentation for more information. + +endif # SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS + if SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS config SECURE_STORAGE_ITS_STORE_SETTINGS_PREFIX diff --git a/subsys/secure_storage/include/internal/zephyr/secure_storage/its/common.h b/subsys/secure_storage/include/internal/zephyr/secure_storage/its/common.h index 4b73eafcf3d88..347d96f435e8e 100644 --- a/subsys/secure_storage/include/internal/zephyr/secure_storage/its/common.h +++ b/subsys/secure_storage/include/internal/zephyr/secure_storage/its/common.h @@ -19,6 +19,7 @@ typedef enum { SECURE_STORAGE_ITS_CALLER_PSA_ITS, SECURE_STORAGE_ITS_CALLER_PSA_PS, SECURE_STORAGE_ITS_CALLER_MBEDTLS, + SECURE_STORAGE_ITS_CALLER_COUNT } secure_storage_its_caller_id_t; /** The UID (caller + entry IDs) of an ITS entry. */ diff --git a/subsys/secure_storage/include/internal/zephyr/secure_storage/its/store.h b/subsys/secure_storage/include/internal/zephyr/secure_storage/its/store.h index fec3448b21589..6dbb7a8a3be07 100644 --- a/subsys/secure_storage/include/internal/zephyr/secure_storage/its/store.h +++ b/subsys/secure_storage/include/internal/zephyr/secure_storage/its/store.h @@ -19,9 +19,7 @@ * @param data_length The number of bytes in `data`. * @param data The data to store. * - * @retval `PSA_SUCCESS` on success. - * @retval `PSA_ERROR_INSUFFICIENT_STORAGE` if there is insufficient storage space. - * @retval `PSA_ERROR_STORAGE_FAILURE` on any other failure. + * @return One of the return values of `psa_its_set()`. */ psa_status_t secure_storage_its_store_set(secure_storage_its_uid_t uid, size_t data_length, const void *data); @@ -34,9 +32,7 @@ psa_status_t secure_storage_its_store_set(secure_storage_its_uid_t uid, * @param[out] data_length On success, the number of bytes written to `data`. * May be less than `data_size`. * - * @retval `PSA_SUCCESS` on success. - * @retval `PSA_ERROR_DOES_NOT_EXIST` if no entry with the given UID exists. - * @retval `PSA_ERROR_STORAGE_FAILURE` on any other failure. + * @return One of the return values of `psa_its_get()`. */ psa_status_t secure_storage_its_store_get(secure_storage_its_uid_t uid, size_t data_size, void *data, size_t *data_length); diff --git a/subsys/secure_storage/src/its/CMakeLists.txt b/subsys/secure_storage/src/its/CMakeLists.txt index b9e2bb877d9c1..ddb0a0265cef4 100644 --- a/subsys/secure_storage/src/its/CMakeLists.txt +++ b/subsys/secure_storage/src/its/CMakeLists.txt @@ -24,6 +24,15 @@ if (NOT CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_NO_INSECURE_KEY_WARNING) endif() endif() +zephyr_library_sources_ifdef(CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS + store/zms.c +) zephyr_library_sources_ifdef(CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS - store_settings.c + store/settings.c ) +if (CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_NONE) + message(ERROR " + The secure storage ITS module is enabled but has no implementation. + (CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_NONE) + ") +endif() diff --git a/subsys/secure_storage/src/its/store_settings.c b/subsys/secure_storage/src/its/store/settings.c similarity index 91% rename from subsys/secure_storage/src/its/store_settings.c rename to subsys/secure_storage/src/its/store/settings.c index d6942fbae6984..e9725761201a1 100644 --- a/subsys/secure_storage/src/its/store_settings.c +++ b/subsys/secure_storage/src/its/store/settings.c @@ -11,7 +11,6 @@ #ifdef CONFIG_SECURE_STORAGE_ITS_IMPLEMENTATION_ZEPHYR #include -BUILD_ASSERT(SECURE_STORAGE_ITS_TRANSFORM_MAX_STORED_DATA_SIZE <= SETTINGS_MAX_VAL_LEN); #endif LOG_MODULE_DECLARE(secure_storage, CONFIG_SECURE_STORAGE_LOG_LEVEL); @@ -21,7 +20,7 @@ static int init_settings_subsys(void) const int ret = settings_subsys_init(); if (ret) { - LOG_DBG("Failed to initialize the settings subsystem. (%d)", ret); + LOG_DBG("Failed. (%d)", ret); } return ret; } @@ -108,9 +107,7 @@ psa_status_t secure_storage_its_store_remove(secure_storage_its_uid_t uid) make_name(uid, name); ret = settings_delete(name); - if (ret) { - LOG_DBG("Failed to delete %s. (%d)", name, ret); - return PSA_ERROR_STORAGE_FAILURE; - } - return PSA_SUCCESS; + + LOG_DBG("%s %s. (%d)", ret ? "Failed to delete" : "Deleted", name, ret); + return ret ? PSA_ERROR_STORAGE_FAILURE : PSA_SUCCESS; } diff --git a/subsys/secure_storage/src/its/store/zms.c b/subsys/secure_storage/src/its/store/zms.c new file mode 100644 index 0000000000000..a0703e81e72fc --- /dev/null +++ b/subsys/secure_storage/src/its/store/zms.c @@ -0,0 +1,122 @@ +/* Copyright (c) 2024 Nordic Semiconductor + * SPDX-License-Identifier: Apache-2.0 + */ +#include +#include +#include +#include +#ifdef CONFIG_SECURE_STORAGE_ITS_IMPLEMENTATION_ZEPHYR +#include +#endif + +LOG_MODULE_DECLARE(secure_storage, CONFIG_SECURE_STORAGE_LOG_LEVEL); + +BUILD_ASSERT(CONFIG_SECURE_STORAGE_ITS_STORE_ZMS_SECTOR_SIZE + > 2 * CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE); + +#define PARTITION_DT_NODE DT_CHOSEN(secure_storage_its_partition) + +static struct zms_fs s_zms = { + .flash_device = FIXED_PARTITION_NODE_DEVICE(PARTITION_DT_NODE), + .offset = FIXED_PARTITION_NODE_OFFSET(PARTITION_DT_NODE), + .sector_size = CONFIG_SECURE_STORAGE_ITS_STORE_ZMS_SECTOR_SIZE, +}; + +static int init_zms(void) +{ + int ret; + + s_zms.sector_count = FIXED_PARTITION_NODE_SIZE(PARTITION_DT_NODE) / s_zms.sector_size; + + ret = zms_mount(&s_zms); + if (ret) { + LOG_DBG("Failed. (%d)", ret); + } + return ret; +} +SYS_INIT(init_zms, APPLICATION, CONFIG_APPLICATION_INIT_PRIORITY); + +/* Bit position of the ITS caller ID in the ZMS entry ID. */ +#define ITS_CALLER_ID_POS 30 +/* Make sure that every ITS caller ID fits in ZMS entry IDs at the defined position. */ +BUILD_ASSERT(1 << (32 - ITS_CALLER_ID_POS) >= SECURE_STORAGE_ITS_CALLER_COUNT); + +static bool has_forbidden_bits_set(secure_storage_its_uid_t uid) +{ + if (uid.uid & GENMASK64(63, ITS_CALLER_ID_POS)) { + LOG_DBG("UID %u/0x%llx cannot be used as it has bits set past " + "the first " STRINGIFY(ITS_CALLER_ID_POS) " ones.", + uid.caller_id, (unsigned long long)uid.uid); + return true; + } + return false; +} + +static uint32_t zms_id_from(secure_storage_its_uid_t uid) +{ + return (uint32_t)uid.uid | (uid.caller_id << ITS_CALLER_ID_POS); +} + +psa_status_t secure_storage_its_store_set(secure_storage_its_uid_t uid, + size_t data_length, const void *data) +{ + psa_status_t psa_ret; + ssize_t zms_ret; + const uint32_t zms_id = zms_id_from(uid); + + if (has_forbidden_bits_set(uid)) { + return PSA_ERROR_INVALID_ARGUMENT; + } + + zms_ret = zms_write(&s_zms, zms_id, data, data_length); + if (zms_ret == data_length) { + psa_ret = PSA_SUCCESS; + } else if (zms_ret == -ENOSPC) { + psa_ret = PSA_ERROR_INSUFFICIENT_STORAGE; + } else { + psa_ret = PSA_ERROR_STORAGE_FAILURE; + } + LOG_DBG("%s 0x%x with %zu bytes. (%zd)", (psa_ret == PSA_SUCCESS) ? + "Wrote" : "Failed to write", zms_id, data_length, zms_ret); + return psa_ret; +} + +psa_status_t secure_storage_its_store_get(secure_storage_its_uid_t uid, size_t data_size, + void *data, size_t *data_length) +{ + psa_status_t psa_ret; + ssize_t zms_ret; + const uint32_t zms_id = zms_id_from(uid); + + if (has_forbidden_bits_set(uid)) { + return PSA_ERROR_INVALID_ARGUMENT; + } + + zms_ret = zms_read(&s_zms, zms_id, data, data_size); + if (zms_ret > 0) { + *data_length = zms_ret; + psa_ret = PSA_SUCCESS; + } else if (zms_ret == -ENOENT) { + psa_ret = PSA_ERROR_DOES_NOT_EXIST; + } else { + psa_ret = PSA_ERROR_STORAGE_FAILURE; + } + LOG_DBG("%s 0x%x for up to %zu bytes. (%zd)", (psa_ret != PSA_ERROR_STORAGE_FAILURE) ? + "Read" : "Failed to read", zms_id, data_size, zms_ret); + return psa_ret; +} + +psa_status_t secure_storage_its_store_remove(secure_storage_its_uid_t uid) +{ + int zms_ret; + const uint32_t zms_id = zms_id_from(uid); + + if (has_forbidden_bits_set(uid)) { + return PSA_ERROR_INVALID_ARGUMENT; + } + + zms_ret = zms_delete(&s_zms, zms_id); + LOG_DBG("%s 0x%x. (%d)", zms_ret ? "Failed to delete" : "Deleted", zms_id, zms_ret); + BUILD_ASSERT(PSA_SUCCESS == 0); + return zms_ret; +} diff --git a/tests/subsys/secure_storage/psa/its/overlay-default_store.conf b/tests/subsys/secure_storage/psa/its/overlay-default_store.conf deleted file mode 100644 index 584a2d08febb5..0000000000000 --- a/tests/subsys/secure_storage/psa/its/overlay-default_store.conf +++ /dev/null @@ -1,2 +0,0 @@ -# Limit the space available for the maximum entry test to not take too long. -CONFIG_SETTINGS_NVS_SECTOR_COUNT=2 diff --git a/tests/subsys/secure_storage/psa/its/overlay-custom_store.conf b/tests/subsys/secure_storage/psa/its/overlay-store_custom.conf similarity index 100% rename from tests/subsys/secure_storage/psa/its/overlay-custom_store.conf rename to tests/subsys/secure_storage/psa/its/overlay-store_custom.conf diff --git a/tests/subsys/secure_storage/psa/its/overlay-store_settings.conf b/tests/subsys/secure_storage/psa/its/overlay-store_settings.conf new file mode 100644 index 0000000000000..e6604bc94b011 --- /dev/null +++ b/tests/subsys/secure_storage/psa/its/overlay-store_settings.conf @@ -0,0 +1,7 @@ +CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS=y + +# 256 - flags (1) - CONFIG_SECURE_STORAGE_ITS_TRANSFORM_OUTPUT_OVERHEAD (28) +CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE=227 + +# Limit the space available for the maximum entry test to not take too long with NVS. +CONFIG_SETTINGS_NVS_SECTOR_COUNT=2 diff --git a/tests/subsys/secure_storage/psa/its/overlay-custom_transform.conf b/tests/subsys/secure_storage/psa/its/overlay-transform_custom.conf similarity index 57% rename from tests/subsys/secure_storage/psa/its/overlay-custom_transform.conf rename to tests/subsys/secure_storage/psa/its/overlay-transform_custom.conf index 8a95663d98a7f..2d845b7321b07 100644 --- a/tests/subsys/secure_storage/psa/its/overlay-custom_transform.conf +++ b/tests/subsys/secure_storage/psa/its/overlay-transform_custom.conf @@ -1,5 +1,4 @@ CONFIG_SECURE_STORAGE_ITS_TRANSFORM_IMPLEMENTATION_CUSTOM=y CONFIG_SECURE_STORAGE_ITS_TRANSFORM_OUTPUT_OVERHEAD=0 -# SETTINGS_MAX_VAL_LEN (256) - flags (1) -CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE=255 +CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE=500 diff --git a/tests/subsys/secure_storage/psa/its/overlay-default_transform.conf b/tests/subsys/secure_storage/psa/its/overlay-transform_default.conf similarity index 60% rename from tests/subsys/secure_storage/psa/its/overlay-default_transform.conf rename to tests/subsys/secure_storage/psa/its/overlay-transform_default.conf index dd30410730963..c743a735e4d56 100644 --- a/tests/subsys/secure_storage/psa/its/overlay-default_transform.conf +++ b/tests/subsys/secure_storage/psa/its/overlay-transform_default.conf @@ -5,5 +5,4 @@ CONFIG_TIMER_RANDOM_GENERATOR=y CONFIG_COMMON_LIBC_MALLOC_ARENA_SIZE=2048 CONFIG_MBEDTLS_PSA_CRYPTO_C=y -# SETTINGS_MAX_VAL_LEN (256) - flags (1) - CONFIG_SECURE_STORAGE_ITS_TRANSFORM_OUTPUT_OVERHEAD (28) -CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE=227 +CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE=300 diff --git a/tests/subsys/secure_storage/psa/its/src/main.c b/tests/subsys/secure_storage/psa/its/src/main.c index d9bab46ba090d..1fb1b522133eb 100644 --- a/tests/subsys/secure_storage/psa/its/src/main.c +++ b/tests/subsys/secure_storage/psa/its/src/main.c @@ -114,7 +114,7 @@ ZTEST(secure_storage_psa_its, test_write_once_flag) { psa_status_t ret; /* Use a UID that isn't used in the other tests for the write-once entry. */ - const psa_storage_uid_t uid = ~UID; + const psa_storage_uid_t uid = 1 << 16; const uint8_t data[MAX_DATA_SIZE] = {}; struct psa_storage_info_t info; diff --git a/tests/subsys/secure_storage/psa/its/testcase.yaml b/tests/subsys/secure_storage/psa/its/testcase.yaml index c59c58532b493..b3dce34dd1842 100644 --- a/tests/subsys/secure_storage/psa/its/testcase.yaml +++ b/tests/subsys/secure_storage/psa/its/testcase.yaml @@ -1,42 +1,45 @@ common: integration_platforms: - native_sim + - nrf54l15dk/nrf54l15/cpuapp platform_exclude: - qemu_cortex_m0 # settings subsystem initialization fails timeout: 600 tags: - psa.secure_storage tests: - secure_storage.psa.its.secure_storage: - filter: CONFIG_SECURE_STORAGE and not CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_NONE + secure_storage.psa.its.secure_storage.zms: + # DT-based filtering is not possible for this test scenario. + # Platforms with a storage_partition must be manually added here. + platform_allow: native_sim mps2/an385 qemu_x86/atom qemu_x86_64/atom + nrf54l15dk/nrf54l15/cpuapp nrf5340dk/nrf5340/cpuapp nrf52840dk/nrf52840 + nrf9151dk/nrf9151 nrf9160dk/nrf9160 nrf9161dk/nrf9161 + extra_args: + - EXTRA_DTC_OVERLAY_FILE=zms.overlay + - EXTRA_CONF_FILE=overlay-secure_storage.conf;overlay-transform_default.conf + - CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS=y + + secure_storage.psa.its.secure_storage.settings.nvs: + filter: CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS extra_args: "EXTRA_CONF_FILE=\ - overlay-secure_storage.conf;overlay-default_transform.conf;overlay-default_store.conf" - integration_platforms: - - native_sim - - nrf54l15dk/nrf54l15/cpuapp + overlay-secure_storage.conf;overlay-transform_default.conf;overlay-store_settings.conf" + secure_storage.psa.its.secure_storage.custom.transform: filter: CONFIG_SECURE_STORAGE and not CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_NONE - extra_args: "EXTRA_CONF_FILE=\ - overlay-secure_storage.conf;overlay-custom_transform.conf;overlay-default_store.conf" - integration_platforms: - - native_sim - - nrf54l15dk/nrf54l15/cpuapp + extra_args: EXTRA_CONF_FILE=overlay-secure_storage.conf;overlay-transform_custom.conf + secure_storage.psa.its.secure_storage.custom.store: filter: CONFIG_SECURE_STORAGE extra_args: "EXTRA_CONF_FILE=\ - overlay-secure_storage.conf;overlay-default_transform.conf;overlay-custom_store.conf" - integration_platforms: - - native_sim - - nrf54l15dk/nrf54l15/cpuapp + overlay-secure_storage.conf;overlay-transform_default.conf;overlay-store_custom.conf" + secure_storage.psa.its.secure_storage.custom.both: filter: CONFIG_SECURE_STORAGE extra_args: "EXTRA_CONF_FILE=\ - overlay-secure_storage.conf;overlay-custom_transform.conf;overlay-custom_store.conf" - integration_platforms: - - native_sim - - nrf54l15dk/nrf54l15/cpuapp + overlay-secure_storage.conf;overlay-transform_custom.conf;overlay-store_custom.conf" + secure_storage.psa.its.tfm: filter: CONFIG_BUILD_WITH_TFM - extra_args: EXTRA_CONF_FILE=overlay-tfm.conf integration_platforms: - nrf9151dk/nrf9151/ns + extra_args: EXTRA_CONF_FILE=overlay-tfm.conf diff --git a/tests/subsys/secure_storage/psa/its/zms.overlay b/tests/subsys/secure_storage/psa/its/zms.overlay new file mode 100644 index 0000000000000..6a739db514ce5 --- /dev/null +++ b/tests/subsys/secure_storage/psa/its/zms.overlay @@ -0,0 +1,5 @@ +/ { + chosen { + secure_storage_its_partition = &storage_partition; + }; +};