suit: Validate algorithm used for signing

Nordic elements can only use EdDSA, not Hashed EdDSA Signed-off-by: Artur Hadasz <[email protected]>
nrfconnect · Feb 6, 2025 · 0075971 · 0075971
1 parent 265c581
commit 0075971
Show file tree

Hide file tree

Showing 10 changed files with 38 additions and 23 deletions.
diff --git a/subsys/suit/mci/CMakeLists.txt b/subsys/suit/mci/CMakeLists.txt
@@ -22,3 +22,4 @@ zephyr_library_link_libraries(suit_mci)
 zephyr_library_link_libraries(suit_utils)
 zephyr_library_link_libraries_ifdef(CONFIG_SUIT_STORAGE suit_storage_interface)
 zephyr_library_link_libraries(suit_execution_mode)
+zephyr_library_link_libraries(suit)
diff --git a/subsys/suit/mci/include/suit_mci.h b/subsys/suit/mci/include/suit_mci.h
@@ -143,22 +143,23 @@ mci_err_t suit_mci_independent_update_policy_get(const suit_manifest_class_id_t
 mci_err_t suit_mci_manifest_class_id_validate(const suit_manifest_class_id_t *class_id);
 
 /**
- * @brief Verifying whether specific key_id is valid for signing/checking signature of specific
- *manifest class
+ * @brief Verifying whether specific key_id and algorithm is valid for signing/checking
+ * signature of specific manifest class
  *
  * @param[in]   class_id	Manifest class id
  * @param[in]   key_id		Identifier of key utilized for manifest signing. key_id may be equal
  *				to 0. In that case function returns success if manifest class id
  *				does not require signing.
+ * @param[in]   cose_alg	COSE algorithm identifier
  *
  * @retval SUIT_PLAT_SUCCESS        on success
  * @retval SUIT_PLAT_ERR_INVAL      invalid parameter, i.e. null pointer
  * @retval MCI_ERR_MANIFESTCLASSID  manifest class id unsupported
- * @retval MCI_ERR_WRONGKEYID       provided key ID is invalid for signing
+ * @retval MCI_ERR_WRONGKEYID       provided key ID or algorithm is invalid for signing
  *                                  for provided manifest class
  */
-mci_err_t suit_mci_signing_key_id_validate(const suit_manifest_class_id_t *class_id,
-					   uint32_t key_id);
+mci_err_t suit_mci_signing_key_id_and_alg_validate(const suit_manifest_class_id_t *class_id,
+						   uint32_t key_id, int32_t cose_alg);
 
 #ifdef CONFIG_ZTEST
 /**

diff --git a/subsys/suit/mci/src/suit_mci_nrf54h20.c b/subsys/suit/mci/src/suit_mci_nrf54h20.c
@@ -13,6 +13,7 @@
 #endif /* CONFIG_SDFW_LCS */
 #include <zephyr/logging/log.h>
 #include <sdfw/arbiter.h>
+#include <suit_types.h>
 
 #define MANIFEST_PUBKEY_NRF_TOP_GEN0	 0x4000BB00
 #define MANIFEST_PUBKEY_SYSCTRL_GEN0	 0x40082100
@@ -229,8 +230,8 @@ static bool skip_validation(suit_manifest_role_t role)
 	return false;
 }
 
-mci_err_t suit_mci_signing_key_id_validate(const suit_manifest_class_id_t *class_id,
-					   uint32_t key_id)
+mci_err_t suit_mci_signing_key_id_and_alg_validate(const suit_manifest_class_id_t *class_id,
+						   uint32_t key_id, int32_t cose_alg)
 {
 	suit_manifest_role_t role = SUIT_MANIFEST_UNKNOWN;
 
@@ -271,14 +272,16 @@ mci_err_t suit_mci_signing_key_id_validate(const suit_manifest_class_id_t *class
 	case SUIT_MANIFEST_SEC_TOP:
 	case SUIT_MANIFEST_SEC_SDFW:
 		if (key_id >= MANIFEST_PUBKEY_NRF_TOP_GEN0 &&
-		    key_id <= MANIFEST_PUBKEY_NRF_TOP_GEN0 + MANIFEST_PUBKEY_GEN_RANGE) {
+		    key_id <= MANIFEST_PUBKEY_NRF_TOP_GEN0 + MANIFEST_PUBKEY_GEN_RANGE &&
+		    (cose_alg == suit_cose_EdDSA)) {
 			return SUIT_PLAT_SUCCESS;
 		}
 		break;
 
 	case SUIT_MANIFEST_SEC_SYSCTRL:
 		if (key_id >= MANIFEST_PUBKEY_SYSCTRL_GEN0 &&
-		    key_id <= MANIFEST_PUBKEY_SYSCTRL_GEN0 + MANIFEST_PUBKEY_GEN_RANGE) {
+		    key_id <= MANIFEST_PUBKEY_SYSCTRL_GEN0 + MANIFEST_PUBKEY_GEN_RANGE &&
+		    (cose_alg == suit_cose_EdDSA)) {
 			return SUIT_PLAT_SUCCESS;
 		}
 		break;

diff --git a/subsys/suit/mci/src/suit_mci_nrf9280.c b/subsys/suit/mci/src/suit_mci_nrf9280.c
@@ -229,8 +229,8 @@ static bool skip_validation(suit_manifest_role_t role)
 	return false;
 }
 
-mci_err_t suit_mci_signing_key_id_validate(const suit_manifest_class_id_t *class_id,
-					   uint32_t key_id)
+mci_err_t suit_mci_signing_key_id_and_alg_validate(const suit_manifest_class_id_t *class_id,
+						   uint32_t key_id, int32_t cose_alg)
 {
 	suit_manifest_role_t role = SUIT_MANIFEST_UNKNOWN;
 
@@ -271,14 +271,16 @@ mci_err_t suit_mci_signing_key_id_validate(const suit_manifest_class_id_t *class
 	case SUIT_MANIFEST_SEC_TOP:
 	case SUIT_MANIFEST_SEC_SDFW:
 		if (key_id >= MANIFEST_PUBKEY_NRF_TOP_GEN0 &&
-		    key_id <= MANIFEST_PUBKEY_NRF_TOP_GEN0 + MANIFEST_PUBKEY_GEN_RANGE) {
+		    key_id <= MANIFEST_PUBKEY_NRF_TOP_GEN0 + MANIFEST_PUBKEY_GEN_RANGE &&
+		    (cose_alg == suit_cose_EdDSA)) {
 			return SUIT_PLAT_SUCCESS;
 		}
 		break;
 
 	case SUIT_MANIFEST_SEC_SYSCTRL:
 		if (key_id >= MANIFEST_PUBKEY_SYSCTRL_GEN0 &&
-		    key_id <= MANIFEST_PUBKEY_SYSCTRL_GEN0 + MANIFEST_PUBKEY_GEN_RANGE) {
+		    key_id <= MANIFEST_PUBKEY_SYSCTRL_GEN0 + MANIFEST_PUBKEY_GEN_RANGE &&
+		    (cose_alg == suit_cose_EdDSA)) {
 			return SUIT_PLAT_SUCCESS;
 		}
 		break;

diff --git a/subsys/suit/platform/sdfw/src/suit_plat_authenticate.c b/subsys/suit/platform/sdfw/src/suit_plat_authenticate.c
@@ -76,7 +76,7 @@ int suit_plat_authenticate_manifest(struct zcbor_string *manifest_component_id,
 	}
 
 	/* Validate KEY ID */
-	ret = suit_mci_signing_key_id_validate(class_id, public_key_id);
+	ret = suit_mci_signing_key_id_and_alg_validate(class_id, public_key_id, alg_id);
 	if (ret != SUIT_PLAT_SUCCESS) {
 		LOG_ERR("Signing key validation failed: MCI err %i", ret);
 		return SUIT_ERR_AUTHENTICATION;
@@ -135,7 +135,7 @@ int suit_plat_authorize_unsigned_manifest(struct zcbor_string *manifest_componen
 	}
 
 	/* Check if unsigned manifest is allowed - pass key_id == 0*/
-	ret = suit_mci_signing_key_id_validate(class_id, 0);
+	ret = suit_mci_signing_key_id_and_alg_validate(class_id, 0, 0);
 
 	if (ret == SUIT_PLAT_SUCCESS) {
 		return SUIT_SUCCESS;

diff --git a/tests/subsys/suit/common/mci_test/mci_test.c b/tests/subsys/suit/common/mci_test/mci_test.c
@@ -231,8 +231,11 @@ int suit_mci_manifest_class_id_validate(const suit_manifest_class_id_t *class_id
 	return SUIT_PLAT_SUCCESS;
 }
 
-int suit_mci_signing_key_id_validate(const suit_manifest_class_id_t *class_id, uint32_t key_id)
+int suit_mci_signing_key_id_and_alg_validate(const suit_manifest_class_id_t *class_id,
+					     uint32_t key_id, int32_t cose_alg)
 {
+	(void) cose_alg;
+
 	if (NULL == class_id) {
 		return SUIT_PLAT_ERR_INVAL;
 	}

diff --git a/tests/subsys/suit/mci/prj.conf b/tests/subsys/suit/mci/prj.conf
@@ -11,6 +11,7 @@ CONFIG_SUIT_UTILS=y
 CONFIG_SUIT_MCI=y
 CONFIG_SUIT_EXECUTION_MODE=y
 CONFIG_SUIT_METADATA=y
+CONFIG_SUIT_PROCESSOR=y
 
 CONFIG_ZCBOR=y
 CONFIG_ZCBOR_CANONICAL=y
diff --git a/tests/subsys/suit/mci/src/api_positive_scenarios.c b/tests/subsys/suit/mci/src/api_positive_scenarios.c
@@ -121,11 +121,13 @@ ZTEST(mci_api_positive_scenarios_tests, test_signing_key_id_validate)
 
 	for (int i = 0; i < output_size; ++i) {
 		uint32_t key_id = 0;
+		int32_t alg_id = 0;
 
-		rc = suit_mci_signing_key_id_validate(result_class_info[i].class_id, key_id);
+		rc = suit_mci_signing_key_id_and_alg_validate(result_class_info[i].class_id,
+							      key_id, alg_id);
 		zassert_true((rc == MCI_ERR_NOACCESS || rc == SUIT_PLAT_SUCCESS ||
 			      rc == MCI_ERR_WRONGKEYID),
-			     "suit_mci_signing_key_id_validate returned (%d)", rc);
+			     "suit_mci_signing_key_id_and_alg_validate returned (%d)", rc);
 	}
 }
 

diff --git a/tests/subsys/suit/mci/src/sanity.c b/tests/subsys/suit/mci/src/sanity.c
@@ -37,6 +37,7 @@ ZTEST(mci_snity_tests, test_null_pointers)
 		{'u', 'n', 's', 'u', 'p', 'p', 'o', 'r', 't', 'e', 'd', '!', '!', '!', ' ', ' '}};
 	size_t output_size = OUTPUT_MAX_SIZE;
 	uint32_t key_id = 0;
+	int32_t alg_id = 0;
 	int processor_id = 0;
 	void *mem_address = &mem_address;
 	size_t mem_size = sizeof(mem_address);
@@ -80,9 +81,9 @@ ZTEST(mci_snity_tests, test_null_pointers)
 	zassert_equal(rc, SUIT_PLAT_ERR_INVAL,
 		      "suit_mci_independent_update_policy_get returned (%d)", rc);
 
-	rc = suit_mci_signing_key_id_validate(NULL, key_id);
-	zassert_equal(rc, SUIT_PLAT_ERR_INVAL, "suit_mci_signing_key_id_validate returned (%d)",
-		      rc);
+	rc = suit_mci_signing_key_id_and_alg_validate(NULL, key_id, alg_id);
+	zassert_equal(rc, SUIT_PLAT_ERR_INVAL,
+		      "suit_mci_signing_key_id_and_alg_validate returned (%d)", rc);
 
 	rc = suit_mci_fw_encryption_key_id_validate(NULL, key_id);
 	zassert_equal(rc, SUIT_PLAT_ERR_INVAL,

diff --git a/tests/subsys/suit/unit/mocks/include/mock_suit_mci.h b/tests/subsys/suit/unit/mocks/include/mock_suit_mci.h
@@ -33,7 +33,8 @@ FAKE_VALUE_FUNC(int, suit_mci_downgrade_prevention_policy_get, const suit_manife
 FAKE_VALUE_FUNC(int, suit_mci_independent_update_policy_get, const suit_manifest_class_id_t *,
 		suit_independent_updateability_policy_t *);
 FAKE_VALUE_FUNC(int, suit_mci_manifest_class_id_validate, const suit_manifest_class_id_t *);
-FAKE_VALUE_FUNC(int, suit_mci_signing_key_id_validate, const suit_manifest_class_id_t *, uint32_t);
+FAKE_VALUE_FUNC(int, suit_mci_signing_key_id_and_alg_validate, const suit_manifest_class_id_t *,
+		uint32_t, int32_t);
 FAKE_VALUE_FUNC(int, suit_mci_signing_key_id_get, const suit_manifest_class_id_t *, uint32_t *);
 FAKE_VALUE_FUNC(int, suit_mci_processor_start_rights_validate, const suit_manifest_class_id_t *,
 		int);
@@ -67,7 +68,7 @@ static inline void mock_suit_mci_reset(void)
 	RESET_FAKE(suit_mci_downgrade_prevention_policy_get);
 	RESET_FAKE(suit_mci_independent_update_policy_get);
 	RESET_FAKE(suit_mci_manifest_class_id_validate);
-	RESET_FAKE(suit_mci_signing_key_id_validate);
+	RESET_FAKE(suit_mci_signing_key_id_and_alg_validate);
 	RESET_FAKE(suit_mci_signing_key_id_get);
 	RESET_FAKE(suit_mci_processor_start_rights_validate);
 	RESET_FAKE(suit_mci_memory_access_rights_validate);