[nrf fromlist] secure_storage: make UIDs 32-bit

Make the storage UID type 32-bit long.
This makes it more convenient to use those UIDs as storage entry IDs
when storing the entries to NVM.
The previous 64+ bits UIDs made it incovenient to use them as such.
As Zephyr defines UID ranges to be used (see
e.g. `zephyr/psa/key_ids.h`), this guarantees that all the UIDs will
fit within the 30 bits reserved for them.

The secure storage ITS implementation API is changed to take
`psa_storage_uid_t` separately so the implementation can check
that no forbidden bits are set before they are packed into
`secure_storage_its_uid_t`.

This change breaks backward compatibility because
`secure_storage_its_uid_t`, which is used both as part of the additional
data for authentication and for generating encryption keys, changes
size from 12 to 4 bytes.
For users wanting to preserve backward compatibility (for example when
upgrading an existing installation to a newer Zephyr release) or that
for some reason want to use a 64-bit `psa_storage_uid_t`, the Kconfig
option CONFIG_SECURE_STORAGE_64_BIT_UID is added. When enabled, it
makes the implementation behave the same as previously and compatibility
with existing entries is preserved. This was tested manually.

Fixes zephyrproject-rtos/zephyr#86177.

Signed-off-by: Tomi Fontanilles <[email protected]>

Upstream PR #: 94171

(cherry picked from commit c784ed1)

Author: tomi-font

subsys/secure_storage/Kconfig

@@ -25,6 +25,17 @@ module = SECURE_STORAGE
 module-str = secure_storage
 source "subsys/logging/Kconfig.template.log_config"
 
+config SECURE_STORAGE_64_BIT_UID
+	bool "Make psa_storage_uid_t 64-bit"
+	help
+	  Zephyr, by default, uses a 30-bit psa_storage_uid_t, which allows using only 32 bits to
+	  identify entries.
+	  UID ranges are defined for the different users of the API which guarantees that all the
+	  UIDs fit within 30 bits. See for example the zephyr/psa/key_ids.h header file.
+	  Enable this for backward compatibility if you are updating an existing installation with
+	  stored entries that was using Zephyr prior to 4.3 or if for some reason you need the full
+	  64-bit UID range.
+
 choice SECURE_STORAGE_ITS_IMPLEMENTATION
 	prompt "Internal Trusted Storage (ITS) API implementation"
 

subsys/secure_storage/Kconfig.its_store

@@ -17,7 +17,7 @@ config SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_ZMS
 	depends on 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
+	  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.
@@ -78,7 +78,8 @@ config SECURE_STORAGE_ITS_STORE_SETTINGS_PREFIX
 config SECURE_STORAGE_ITS_STORE_SETTINGS_NAME_MAX_LEN
 	int "Maximum setting name length"
 	range 2 64
-	default 22 if !SECURE_STORAGE_ITS_STORE_SETTINGS_NAME_CUSTOM
-	default 0
+	default 0 if SECURE_STORAGE_ITS_STORE_SETTINGS_NAME_CUSTOM
+	default 22 if SECURE_STORAGE_64_BIT_UID
+	default 14
 
 endif # SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_SETTINGS

subsys/secure_storage/include/internal/zephyr/secure_storage/its.h

@@ -14,18 +14,21 @@
 #include "its/common.h"
 
 /** @brief See `psa_its_set()`, to which this function is analogous. */
-psa_status_t secure_storage_its_set(secure_storage_its_uid_t uid, size_t data_length,
-				    const void *p_data, psa_storage_create_flags_t create_flags);
+psa_status_t secure_storage_its_set(secure_storage_its_caller_id_t caller_id, psa_storage_uid_t uid,
+				    size_t data_length, const void *p_data,
+				    psa_storage_create_flags_t create_flags);
 
 /** @brief See `psa_its_get()`, to which this function is analogous. */
-psa_status_t secure_storage_its_get(secure_storage_its_uid_t uid, size_t data_offset,
-				    size_t data_size, void *p_data, size_t *p_data_length);
+psa_status_t secure_storage_its_get(secure_storage_its_caller_id_t caller_id, psa_storage_uid_t uid,
+				    size_t data_offset, size_t data_size,
+				    void *p_data, size_t *p_data_length);
 
 /** @brief See `psa_its_get_info()`, to which this function is analogous. */
-psa_status_t secure_storage_its_get_info(secure_storage_its_uid_t uid,
-					 struct psa_storage_info_t *p_info);
+psa_status_t secure_storage_its_get_info(secure_storage_its_caller_id_t caller_id,
+					 psa_storage_uid_t uid, struct psa_storage_info_t *p_info);
 
 /** @brief See `psa_its_remove()`, to which this function is analogous. */
-psa_status_t secure_storage_its_remove(secure_storage_its_uid_t uid);
+psa_status_t secure_storage_its_remove(secure_storage_its_caller_id_t caller_id,
+				       psa_storage_uid_t uid);
 
 #endif

subsys/secure_storage/include/internal/zephyr/secure_storage/its/common.h

@@ -12,8 +12,7 @@
 #include <psa/storage_common.h>
 
 /** @brief The ID of the caller from which the ITS API call originates.
- * This is used to prevent ID collisions between different callers that are not aware
- * of each other and so might use the same numerical IDs, e.g. PSA Crypto and PSA ITS.
+ * This is used to namespace the different callers and possibly treat them differently.
  */
 typedef enum {
 	SECURE_STORAGE_ITS_CALLER_PSA_ITS,
@@ -22,12 +21,32 @@ typedef enum {
 	SECURE_STORAGE_ITS_CALLER_COUNT
 } secure_storage_its_caller_id_t;
 
+#ifdef CONFIG_SECURE_STORAGE_64_BIT_UID
+
 /** The UID (caller + entry IDs) of an ITS entry. */
 typedef struct {
 	psa_storage_uid_t uid;
 	secure_storage_its_caller_id_t caller_id;
 } __packed secure_storage_its_uid_t;
 
+#else
+
+#define SECURE_STORAGE_ITS_UID_BIT_SIZE 30
+#define SECURE_STORAGE_ITS_CALLER_ID_BIT_SIZE 2
+
+/** @brief The UID (caller + entry IDs) of an ITS entry.
+ * This is a packed, 32-bit version of `psa_storage_uid_t` which allows storing
+ * smaller IDs compared to the 64-bit ones that PSA Secure Storage specifies.
+ * Zephyr defines ranges of IDs to be used by different users of the API (subsystems, application)
+ * which guarantees 1. no collisions and 2. that the IDs used fit within `uid`.
+ */
+typedef struct {
+	psa_storage_uid_t uid : SECURE_STORAGE_ITS_UID_BIT_SIZE;
+	secure_storage_its_caller_id_t caller_id : SECURE_STORAGE_ITS_CALLER_ID_BIT_SIZE;
+} secure_storage_its_uid_t;
+
+#endif /* CONFIG_SECURE_STORAGE_64_BIT_UID */
+
 #ifdef CONFIG_SECURE_STORAGE_ITS_TRANSFORM_MODULE
 
 /** The maximum size, in bytes, of an entry's data after it has been transformed for storage. */

subsys/secure_storage/include/psa/internal_trusted_storage.h

@@ -16,7 +16,6 @@
 #else
 #define ITS_CALLER_ID SECURE_STORAGE_ITS_CALLER_PSA_ITS
 #endif
-#define ITS_UID (secure_storage_its_uid_t){.uid = uid, .caller_id = ITS_CALLER_ID}
 /** @endcond */
 
 #include <psa/storage_common.h>
@@ -50,7 +49,7 @@ static ALWAYS_INLINE
 psa_status_t psa_its_set(psa_storage_uid_t uid, size_t data_length,
 			 const void *p_data, psa_storage_create_flags_t create_flags)
 {
-	return secure_storage_its_set(ITS_UID, data_length, p_data, create_flags);
+	return secure_storage_its_set(ITS_CALLER_ID, uid, data_length, p_data, create_flags);
 }
 
 /**
@@ -76,7 +75,8 @@ static ALWAYS_INLINE
 psa_status_t psa_its_get(psa_storage_uid_t uid, size_t data_offset,
 			 size_t data_size, void *p_data, size_t *p_data_length)
 {
-	return secure_storage_its_get(ITS_UID, data_offset, data_size, p_data, p_data_length);
+	return secure_storage_its_get(ITS_CALLER_ID, uid, data_offset,
+				      data_size, p_data, p_data_length);
 }
 
 /**
@@ -96,7 +96,7 @@ static ALWAYS_INLINE
 /** @endcond  */
 psa_status_t psa_its_get_info(psa_storage_uid_t uid, struct psa_storage_info_t *p_info)
 {
-	return secure_storage_its_get_info(ITS_UID, p_info);
+	return secure_storage_its_get_info(ITS_CALLER_ID, uid, p_info);
 }
 
 /**
@@ -117,7 +117,7 @@ static ALWAYS_INLINE
 /** @endcond  */
 psa_status_t psa_its_remove(psa_storage_uid_t uid)
 {
-	return secure_storage_its_remove(ITS_UID);
+	return secure_storage_its_remove(ITS_CALLER_ID, uid);
 }
 
 #undef ITS_UID

subsys/secure_storage/include/psa/protected_storage.h

@@ -12,8 +12,7 @@
 /** @cond INTERNAL_HIDDEN */
 #ifdef CONFIG_SECURE_STORAGE_PS_IMPLEMENTATION_ITS
 #include "../internal/zephyr/secure_storage/its.h"
-#define ITS_UID (secure_storage_its_uid_t){.uid = uid, \
-					   .caller_id = SECURE_STORAGE_ITS_CALLER_PSA_PS}
+#define ITS_CALLER_ID SECURE_STORAGE_ITS_CALLER_PSA_PS
 #else
 #include "../internal/zephyr/secure_storage/ps.h"
 #endif
@@ -50,7 +49,7 @@ psa_status_t psa_ps_set(psa_storage_uid_t uid, size_t data_length,
 			const void *p_data, psa_storage_create_flags_t create_flags)
 {
 #ifdef CONFIG_SECURE_STORAGE_PS_IMPLEMENTATION_ITS
-	return secure_storage_its_set(ITS_UID, data_length, p_data, create_flags);
+	return secure_storage_its_set(ITS_CALLER_ID, uid, data_length, p_data, create_flags);
 #else
 	return secure_storage_ps_set(uid, data_length, p_data, create_flags);
 #endif
@@ -83,7 +82,8 @@ psa_status_t psa_ps_get(psa_storage_uid_t uid, size_t data_offset,
 			size_t data_size, void *p_data, size_t *p_data_length)
 {
 #ifdef CONFIG_SECURE_STORAGE_PS_IMPLEMENTATION_ITS
-	return secure_storage_its_get(ITS_UID, data_offset, data_size, p_data, p_data_length);
+	return secure_storage_its_get(ITS_CALLER_ID, uid, data_offset,
+				      data_size, p_data, p_data_length);
 #else
 	return secure_storage_ps_get(uid, data_offset, data_size, p_data, p_data_length);
 #endif
@@ -110,7 +110,7 @@ static ALWAYS_INLINE
 psa_status_t psa_ps_get_info(psa_storage_uid_t uid, struct psa_storage_info_t *p_info)
 {
 #ifdef CONFIG_SECURE_STORAGE_PS_IMPLEMENTATION_ITS
-	return secure_storage_its_get_info(ITS_UID, p_info);
+	return secure_storage_its_get_info(ITS_CALLER_ID, uid, p_info);
 #else
 	return secure_storage_ps_get_info(uid, p_info);
 #endif
@@ -138,7 +138,7 @@ static ALWAYS_INLINE
 psa_status_t psa_ps_remove(psa_storage_uid_t uid)
 {
 #ifdef CONFIG_SECURE_STORAGE_PS_IMPLEMENTATION_ITS
-	return secure_storage_its_remove(ITS_UID);
+	return secure_storage_its_remove(ITS_CALLER_ID, uid);
 #else
 	return secure_storage_ps_remove(uid);
 #endif

subsys/secure_storage/include/psa/storage_common.h

@@ -20,7 +20,11 @@
 #include <stddef.h>
 
 /** UID type for identifying entries. */
+#ifdef CONFIG_SECURE_STORAGE_64_BIT_UID
 typedef uint64_t psa_storage_uid_t;
+#else
+typedef uint32_t psa_storage_uid_t;
+#endif
 
 /** Flags used when creating an entry. */
 typedef uint32_t psa_storage_create_flags_t;

subsys/secure_storage/src/its/implementation.c

@@ -11,6 +11,33 @@
 
 LOG_MODULE_DECLARE(secure_storage, CONFIG_SECURE_STORAGE_LOG_LEVEL);
 
+#ifndef CONFIG_SECURE_STORAGE_64_BIT_UID
+BUILD_ASSERT(sizeof(secure_storage_its_uid_t) == 4); /* ITS UIDs are 32-bit */
+BUILD_ASSERT(1 << SECURE_STORAGE_ITS_CALLER_ID_BIT_SIZE >= SECURE_STORAGE_ITS_CALLER_COUNT);
+BUILD_ASSERT(SECURE_STORAGE_ITS_CALLER_ID_BIT_SIZE + SECURE_STORAGE_ITS_UID_BIT_SIZE == 32);
+#endif
+
+static psa_status_t make_its_uid(secure_storage_its_caller_id_t caller_id, psa_storage_uid_t uid,
+				 secure_storage_its_uid_t *its_uid)
+{
+	if (uid == 0) {
+		return PSA_ERROR_INVALID_ARGUMENT;
+	}
+
+#ifndef CONFIG_SECURE_STORAGE_64_BIT_UID
+	/* Check that the UID is not bigger than the maximum defined size. */
+	if (uid & GENMASK64(63, SECURE_STORAGE_ITS_UID_BIT_SIZE)) {
+		LOG_DBG("UID %u/0x%llx cannot be used as it has bits set past "
+			"the first " STRINGIFY(SECURE_STORAGE_ITS_UID_BIT_SIZE) " ones.",
+			caller_id, (unsigned long long)uid);
+		return PSA_ERROR_INVALID_ARGUMENT;
+	}
+#endif /* !CONFIG_SECURE_STORAGE_64_BIT_UID */
+
+	*its_uid = (secure_storage_its_uid_t){.caller_id = caller_id, .uid = uid};
+	return PSA_SUCCESS;
+}
+
 static void log_failed_operation(const char *operation, const char *preposition, psa_status_t ret)
 {
 	LOG_ERR("Failed to %s data %s storage. (%d)", operation, preposition, ret);
@@ -117,12 +144,17 @@ static psa_status_t store_entry(secure_storage_its_uid_t uid, size_t data_length
 	return ret;
 }
 
-psa_status_t secure_storage_its_set(secure_storage_its_uid_t uid, size_t data_length,
-				    const void *p_data, psa_storage_create_flags_t create_flags)
+psa_status_t secure_storage_its_set(secure_storage_its_caller_id_t caller_id, psa_storage_uid_t uid,
+				    size_t data_length, const void *p_data,
+				    psa_storage_create_flags_t create_flags)
 {
 	psa_status_t ret;
+	secure_storage_its_uid_t its_uid;
 
-	if (uid.uid == 0 || (p_data == NULL && data_length != 0)) {
+	if (make_its_uid(caller_id, uid, &its_uid) != PSA_SUCCESS) {
+		return PSA_ERROR_INVALID_ARGUMENT;
+	}
+	if (p_data == NULL && data_length != 0) {
 		return PSA_ERROR_INVALID_ARGUMENT;
 	}
 	if (create_flags & ~SECURE_STORAGE_ALL_CREATE_FLAGS) {
@@ -134,43 +166,49 @@ psa_status_t secure_storage_its_set(secure_storage_its_uid_t uid, size_t data_le
 		return PSA_ERROR_INSUFFICIENT_STORAGE;
 	}
 
-	if (keep_stored_entry(uid, data_length, p_data, create_flags, &ret)) {
+	if (keep_stored_entry(its_uid, data_length, p_data, create_flags, &ret)) {
 		return ret;
 	}
 
-	ret = store_entry(uid, data_length, p_data, create_flags);
+	ret = store_entry(its_uid, data_length, p_data, create_flags);
 	return ret;
 }
-
-psa_status_t secure_storage_its_get(secure_storage_its_uid_t uid, size_t data_offset,
-				    size_t data_size, void *p_data, size_t *p_data_length)
+psa_status_t secure_storage_its_get(secure_storage_its_caller_id_t caller_id, psa_storage_uid_t uid,
+				    size_t data_offset, size_t data_size,
+				    void *p_data, size_t *p_data_length)
 {
-	if (uid.uid == 0 || (p_data == NULL && data_size != 0) || p_data_length == NULL) {
+	psa_status_t ret;
+	secure_storage_its_uid_t its_uid;
+	uint8_t stored_data[SECURE_STORAGE_ITS_TRANSFORM_MAX_STORED_DATA_SIZE];
+	size_t stored_data_len;
+	psa_storage_create_flags_t create_flags;
+
+	if (make_its_uid(caller_id, uid, &its_uid) != PSA_SUCCESS) {
+		return PSA_ERROR_INVALID_ARGUMENT;
+	}
+	if ((p_data == NULL && data_size != 0) || p_data_length == NULL) {
 		return PSA_ERROR_INVALID_ARGUMENT;
 	}
 	if (data_size == 0) {
 		*p_data_length = 0;
 		return PSA_SUCCESS;
 	}
-	psa_status_t ret;
-	uint8_t stored_data[SECURE_STORAGE_ITS_TRANSFORM_MAX_STORED_DATA_SIZE];
-	size_t stored_data_len;
-	psa_storage_create_flags_t create_flags;
 
-	ret = get_stored_data(uid, stored_data, &stored_data_len);
+
+	ret = get_stored_data(its_uid, stored_data, &stored_data_len);
 	if (ret != PSA_SUCCESS) {
 		return ret;
 	}
 	if (data_offset == 0
 	 && data_size >= SECURE_STORAGE_ITS_TRANSFORM_DATA_SIZE(stored_data_len)) {
 		/* All the data fits directly in the provided buffer. */
-		return transform_stored_data(uid, stored_data_len, stored_data, data_size, p_data,
-					     p_data_length, &create_flags);
+		return transform_stored_data(its_uid, stored_data_len, stored_data, data_size,
+					     p_data, p_data_length, &create_flags);
 	}
 	uint8_t data[CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE];
 	size_t data_len;
 
-	ret = transform_stored_data(uid, stored_data_len, stored_data, sizeof(data), data,
+	ret = transform_stored_data(its_uid, stored_data_len, stored_data, sizeof(data), data,
 				    &data_len, &create_flags);
 	if (ret == PSA_SUCCESS) {
 		if (data_offset > data_len) {
@@ -184,41 +222,47 @@ psa_status_t secure_storage_its_get(secure_storage_its_uid_t uid, size_t data_of
 	return ret;
 }
 
-psa_status_t secure_storage_its_get_info(secure_storage_its_uid_t uid,
-					 struct psa_storage_info_t *p_info)
+psa_status_t secure_storage_its_get_info(secure_storage_its_caller_id_t caller_id,
+					 psa_storage_uid_t uid, struct psa_storage_info_t *p_info)
 {
 	psa_status_t ret;
+	secure_storage_its_uid_t its_uid;
 	uint8_t data[CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE];
 
-	if (uid.uid == 0 || p_info == NULL) {
+	if (make_its_uid(caller_id, uid, &its_uid) != PSA_SUCCESS) {
+		return PSA_ERROR_INVALID_ARGUMENT;
+	}
+	if (p_info == NULL) {
 		return PSA_ERROR_INVALID_ARGUMENT;
 	}
 
-	ret = get_entry(uid, sizeof(data), data, &p_info->size, &p_info->flags);
+	ret = get_entry(its_uid, sizeof(data), data, &p_info->size, &p_info->flags);
 	if (ret == PSA_SUCCESS) {
 		p_info->capacity = p_info->size;
 	}
 	return ret;
 }
 
-psa_status_t secure_storage_its_remove(secure_storage_its_uid_t uid)
+psa_status_t secure_storage_its_remove(secure_storage_its_caller_id_t caller_id,
+				       psa_storage_uid_t uid)
 {
 	psa_status_t ret;
+	secure_storage_its_uid_t its_uid;
 	psa_storage_create_flags_t create_flags;
 	uint8_t data[CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE];
 	size_t data_len;
 
-	if (uid.uid == 0) {
+	if (make_its_uid(caller_id, uid, &its_uid) != PSA_SUCCESS) {
 		return PSA_ERROR_INVALID_ARGUMENT;
 	}
 
-	ret = get_entry(uid, sizeof(data), data, &data_len, &create_flags);
+	ret = get_entry(its_uid, sizeof(data), data, &data_len, &create_flags);
 	if (ret == PSA_SUCCESS && (create_flags & PSA_STORAGE_FLAG_WRITE_ONCE)) {
 		return PSA_ERROR_NOT_PERMITTED;
 	}
 	/* Allow overwriting corrupted entries as well to not be stuck with them forever. */
 	if (ret == PSA_SUCCESS || ret == PSA_ERROR_STORAGE_FAILURE) {
-		ret = secure_storage_its_store_remove(uid);
+		ret = secure_storage_its_store_remove(its_uid);
 		if (ret != PSA_SUCCESS) {
 			log_failed_operation("remove", "from", ret);
 			return PSA_ERROR_STORAGE_FAILURE;