[nrf fromtree] secure_storage: improve expandability and other minor improvements

subsys/secure_storage/CMakeLists.txt

@@ -31,21 +31,33 @@ if(CONFIG_SECURE_STORAGE_PS_IMPLEMENTATION_CUSTOM)
   make_available(ps.h)
 endif()
 
-if(CONFIG_SECURE_STORAGE_ITS_TRANSFORM_IMPLEMENTATION_CUSTOM
-    OR (CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_CUSTOM
-        AND CONFIG_SECURE_STORAGE_ITS_TRANSFORM_MODULE))
-  make_available(its/transform.h)
-endif()
+if(CONFIG_SECURE_STORAGE_ITS_TRANSFORM_MODULE)
+
+  if(NOT CONFIG_SECURE_STORAGE_ITS_TRANSFORM_IMPLEMENTATION_AEAD)
+    make_available(its/transform.h)
+  endif()
+
+endif() # CONFIG_SECURE_STORAGE_ITS_TRANSFORM_MODULE
 
 if(CONFIG_SECURE_STORAGE_ITS_STORE_IMPLEMENTATION_CUSTOM)
   make_available(its/store.h)
 endif()
 
-if(CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_SCHEME_CUSTOM
- OR CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_CUSTOM
- OR CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_NONCE_PROVIDER_CUSTOM)
-  make_available(its/transform/aead_get.h)
-endif()
+if(CONFIG_SECURE_STORAGE_ITS_TRANSFORM_IMPLEMENTATION_AEAD)
+
+  # Make the aead_get.h header available whenever none of the Zephyr-provided
+  # implementations are in use. In that case either the custom or an additional
+  # option added downstream is used, and it needs that header file.
+
+  if((NOT CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_SCHEME_AES_GCM
+      AND NOT CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_SCHEME_CHACHA20_POLY1305)
+  OR (NOT CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_DEVICE_ID_HASH
+      AND NOT CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_ENTRY_UID_HASH)
+  OR (NOT CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_NONCE_PROVIDER_DEFAULT))
+    make_available(its/transform/aead_get.h)
+  endif()
+
+endif() # CONFIG_SECURE_STORAGE_ITS_TRANSFORM_IMPLEMENTATION_AEAD
 
 if(CONFIG_SECURE_STORAGE_ITS_STORE_SETTINGS_NAME_CUSTOM)
   make_available(its/store/settings_get.h)

subsys/secure_storage/Kconfig.its_transform

@@ -97,7 +97,8 @@ config SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_SIZE
 	int "AEAD ITS transform module encryption key size in bytes"
 	default 32
 
-if !SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_CUSTOM
+if SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_DEVICE_ID_HASH \
+|| SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_ENTRY_UID_HASH
 
 config SECURE_STORAGE_ITS_TRANSFORM_AEAD_NO_INSECURE_KEY_WARNING
 	bool "Silence the insecure ITS encryption key warnings"

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

@@ -28,4 +28,19 @@
 	secure_storage_its_caller_id_t caller_id;
 } __packed secure_storage_its_uid_t;
 
+#ifdef CONFIG_SECURE_STORAGE_ITS_TRANSFORM_MODULE
+
+/** The maximum size, in bytes, of an entry's data after it has been transformed for storage. */
+enum { SECURE_STORAGE_ITS_TRANSFORM_MAX_STORED_DATA_SIZE
+	= CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE
+	  + sizeof(secure_storage_packed_create_flags_t)
+	  + CONFIG_SECURE_STORAGE_ITS_TRANSFORM_OUTPUT_OVERHEAD };
+
+/** The size, in bytes, of an entry's data given its size once transformed for storage. */
+#define SECURE_STORAGE_ITS_TRANSFORM_DATA_SIZE(transformed_data_size) \
+	(transformed_data_size - (SECURE_STORAGE_ITS_TRANSFORM_MAX_STORED_DATA_SIZE \
+				  - CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE))
+
+#endif /* CONFIG_SECURE_STORAGE_ITS_TRANSFORM_MODULE */
+
 #endif

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

@@ -13,16 +13,6 @@
  */
 #include <zephyr/secure_storage/its/common.h>
 
-/** The maximum size, in bytes, of an entry's data after it has been transformed for storage. */
-enum { SECURE_STORAGE_ITS_TRANSFORM_MAX_STORED_DATA_SIZE
-	= CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE
-	  + sizeof(secure_storage_packed_create_flags_t)
-	  + CONFIG_SECURE_STORAGE_ITS_TRANSFORM_OUTPUT_OVERHEAD };
-
-#define SECURE_STORAGE_ITS_TRANSFORM_DATA_SIZE(stored_data_len) \
-	(stored_data_len - (SECURE_STORAGE_ITS_TRANSFORM_MAX_STORED_DATA_SIZE \
-			    - CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE))
-
 /** @brief Transforms the data of an ITS entry for storage.
  *
  * @param[in]  uid             The entry's UID.

subsys/secure_storage/src/its/store/settings.c

@@ -10,10 +10,6 @@
 #include <errno.h>
 #include <stdio.h>
 
-#ifdef CONFIG_SECURE_STORAGE_ITS_IMPLEMENTATION_ZEPHYR
-#include <zephyr/secure_storage/its/transform.h>
-#endif
-
 LOG_MODULE_DECLARE(secure_storage, CONFIG_SECURE_STORAGE_LOG_LEVEL);
 
 static int init_settings_subsys(void)
@@ -120,7 +116,7 @@ psa_status_t secure_storage_its_store_remove(secure_storage_its_uid_t uid)
 	secure_storage_its_store_settings_get_name(uid, name);
 
 	ret = settings_delete(name);
-
 	LOG_DBG("%s %s. (%d)", ret ? "Failed to delete" : "Deleted", name, ret);
+
 	return ret ? PSA_ERROR_STORAGE_FAILURE : PSA_SUCCESS;
 }

subsys/secure_storage/src/its/store/zms.c

@@ -5,9 +5,6 @@
 #include <zephyr/logging/log.h>
 #include <zephyr/fs/zms.h>
 #include <zephyr/storage/flash_map.h>
-#ifdef CONFIG_SECURE_STORAGE_ITS_IMPLEMENTATION_ZEPHYR
-#include <zephyr/secure_storage/its/transform.h>
-#endif
 
 LOG_MODULE_DECLARE(secure_storage, CONFIG_SECURE_STORAGE_LOG_LEVEL);
 
@@ -108,15 +105,15 @@ psa_status_t secure_storage_its_store_get(secure_storage_its_uid_t uid, size_t d
 
 psa_status_t secure_storage_its_store_remove(secure_storage_its_uid_t uid)
 {
-	int zms_ret;
+	int 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;
+	ret = zms_delete(&s_zms, zms_id);
+	LOG_DBG("%s 0x%x. (%d)", ret ? "Failed to delete" : "Deleted", zms_id, ret);
+
+	return ret ? PSA_ERROR_STORAGE_FAILURE : PSA_SUCCESS;
 }

subsys/secure_storage/src/its/transform/aead.c

@@ -31,7 +31,7 @@ static psa_status_t psa_aead_crypt(psa_key_usage_t operation, secure_storage_its
 	psa_set_key_lifetime(&key_attributes, PSA_KEY_LIFETIME_VOLATILE);
 	psa_set_key_type(&key_attributes, key_type);
 	psa_set_key_algorithm(&key_attributes, alg);
-	psa_set_key_bits(&key_attributes, sizeof(key) * 8);
+	psa_set_key_bits(&key_attributes, PSA_BYTES_TO_BITS(sizeof(key)));
 
 	/* Avoid calling psa_aead_*crypt() because that would require importing keys into
 	 * PSA Crypto. This gets called from PSA Crypto for storing persistent keys so,
@@ -57,10 +57,6 @@ static psa_status_t psa_aead_crypt(psa_key_usage_t operation, secure_storage_its
 enum { CIPHERTEXT_MAX_SIZE
 	= PSA_AEAD_ENCRYPT_OUTPUT_MAX_SIZE(CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE) };
 
-BUILD_ASSERT(CONFIG_SECURE_STORAGE_ITS_TRANSFORM_OUTPUT_OVERHEAD
-	     == CIPHERTEXT_MAX_SIZE - CONFIG_SECURE_STORAGE_ITS_MAX_DATA_SIZE
-		+ CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_NONCE_SIZE);
-
 BUILD_ASSERT(SECURE_STORAGE_ALL_CREATE_FLAGS
 	     <= (1 << (8 * sizeof(secure_storage_packed_create_flags_t))) - 1);
 
@@ -113,7 +109,7 @@ psa_status_t secure_storage_its_transform_from_store(
 		psa_storage_create_flags_t *create_flags)
 {
 	if (stored_data_len < STORED_ENTRY_LEN(0)) {
-		return PSA_ERROR_STORAGE_FAILURE;
+		return PSA_ERROR_DATA_CORRUPT;
 	}
 
 	psa_status_t ret;

subsys/secure_storage/src/its/transform/aead_get.c

@@ -12,22 +12,23 @@
 
 LOG_MODULE_DECLARE(secure_storage, CONFIG_SECURE_STORAGE_LOG_LEVEL);
 
-#ifdef CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_SCHEME_AES_GCM
+#if defined(CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_SCHEME_AES_GCM)
 #define PSA_KEY_TYPE PSA_KEY_TYPE_AES
 #define PSA_ALG PSA_ALG_GCM
 #elif defined(CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_SCHEME_CHACHA20_POLY1305)
 #define PSA_KEY_TYPE PSA_KEY_TYPE_CHACHA20
 #define PSA_ALG PSA_ALG_CHACHA20_POLY1305
 #endif
-#ifndef CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_SCHEME_CUSTOM
+#ifdef PSA_KEY_TYPE
 void secure_storage_its_transform_aead_get_scheme(psa_key_type_t *key_type, psa_algorithm_t *alg)
 {
 	*key_type = PSA_KEY_TYPE;
 	*alg = PSA_ALG;
 }
-#endif /* !CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_SCHEME_CUSTOM */
+#endif /* PSA_KEY_TYPE */
 
-#ifndef CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_CUSTOM
+#if	defined(CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_DEVICE_ID_HASH) || \
+	defined(CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_ENTRY_UID_HASH)
 
 #define SHA256_OUTPUT_SIZE 32
 BUILD_ASSERT(SHA256_OUTPUT_SIZE == PSA_HASH_LENGTH(PSA_ALG_SHA_256));
@@ -75,6 +76,7 @@
 	if (hwinfo_ret != 0) {
 		hwinfo_ret = hwinfo_get_device_id(data.device_id, sizeof(data.device_id));
 		if (hwinfo_ret <= 0) {
+			LOG_DBG("Failed to retrieve the device ID. (%zd)", hwinfo_ret);
 			return PSA_ERROR_HARDWARE_FAILURE;
 		}
 		if (hwinfo_ret < sizeof(data.device_id)) {
@@ -113,7 +115,8 @@
 
 #endif /* !CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_NO_INSECURE_KEY_WARNING */
 
-#endif /* !CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_CUSTOM */
+#endif /* CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_DEVICE_ID_HASH || */
+       /* CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_KEY_PROVIDER_ENTRY_UID_HASH    */
 
 #ifdef CONFIG_SECURE_STORAGE_ITS_TRANSFORM_AEAD_NONCE_PROVIDER_DEFAULT
 

tests/subsys/secure_storage/psa/its/src/custom_store.c

@@ -2,7 +2,6 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 #include <zephyr/secure_storage/its/store.h>
-#include <zephyr/secure_storage/its/transform.h>
 #include <zephyr/sys/util.h>
 #include <string.h>