From 85478a1ee24eaeca588b3c772450e7eccd0efae3 Mon Sep 17 00:00:00 2001 From: Artur Hadasz Date: Thu, 16 Jan 2025 15:31:30 +0100 Subject: [PATCH 1/2] suit: Add support for Ed25519PH Enabled building SUIT envelopes with Ed25519PH and their verification by the SUIT bootloader. Signed-off-by: Artur Hadasz --- subsys/suit/platform/sdfw/src/suit_plat_authenticate.c | 3 +++ sysbuild/Kconfig.suit | 10 ++++++++++ west.yml | 4 ++-- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/subsys/suit/platform/sdfw/src/suit_plat_authenticate.c b/subsys/suit/platform/sdfw/src/suit_plat_authenticate.c index 5dd111ca40be..75dd76444d1f 100644 --- a/subsys/suit/platform/sdfw/src/suit_plat_authenticate.c +++ b/subsys/suit/platform/sdfw/src/suit_plat_authenticate.c @@ -32,6 +32,9 @@ int suit_plat_authenticate_manifest(struct zcbor_string *manifest_component_id, case suit_cose_EdDSA: psa_alg = PSA_ALG_PURE_EDDSA; /* ed25519/curve25519 without internal hashing */ break; + case suit_cose_VS_HashEdDSA: + psa_alg = PSA_ALG_ED25519PH; /* ed25519/curve25519 with internal hashing */ + break; default: return SUIT_ERR_DECODING; } diff --git a/sysbuild/Kconfig.suit b/sysbuild/Kconfig.suit index a397a90eca31..7fa624bef8e5 100644 --- a/sysbuild/Kconfig.suit +++ b/sysbuild/Kconfig.suit @@ -122,6 +122,10 @@ config SUIT_ENVELOPE_ROOT_SIGN_ALG_ECDSA_384 config SUIT_ENVELOPE_ROOT_SIGN_ALG_ECDSA_521 bool "Use the ECDSA algorithm with key length of 521 bits" +config SUIT_ENVELOPE_ROOT_SIGN_ALG_HASH_EDDSA + bool "Use the HashEdDSA algorithm (specifically: ed25519ph)" + select EXPERIMENTAL + endchoice config SUIT_ENVELOPE_ROOT_SIGN_ALG_NAME @@ -130,6 +134,7 @@ config SUIT_ENVELOPE_ROOT_SIGN_ALG_NAME default "es-256" if SUIT_ENVELOPE_ROOT_SIGN_ALG_ECDSA_256 default "es-384" if SUIT_ENVELOPE_ROOT_SIGN_ALG_ECDSA_384 default "es-521" if SUIT_ENVELOPE_ROOT_SIGN_ALG_ECDSA_521 + default "hash-eddsa" if SUIT_ENVELOPE_ROOT_SIGN_ALG_HASH_EDDSA endif # SUIT_ENVELOPE_ROOT_SIGN @@ -257,6 +262,10 @@ config SUIT_ENVELOPE_APP_RECOVERY_SIGN_ALG_ECDSA_384 config SUIT_ENVELOPE_APP_RECOVERY_SIGN_ALG_ECDSA_521 bool "Use the ECDSA algorithm with key length of 521 bits" +config SUIT_ENVELOPE_APP_RECOVERY_SIGN_ALG_HASH_EDDSA + bool "Use the HashEdDSA algorithm (specifically: ed25519ph)" + select EXPERIMENTAL + endchoice config SUIT_ENVELOPE_APP_RECOVERY_SIGN_ALG_NAME @@ -265,6 +274,7 @@ config SUIT_ENVELOPE_APP_RECOVERY_SIGN_ALG_NAME default "es-256" if SUIT_ENVELOPE_APP_RECOVERY_SIGN_ALG_ECDSA_256 default "es-384" if SUIT_ENVELOPE_APP_RECOVERY_SIGN_ALG_ECDSA_384 default "es-521" if SUIT_ENVELOPE_APP_RECOVERY_SIGN_ALG_ECDSA_521 + default "hash-eddsa" if SUIT_ENVELOPE_APP_RECOVERY_SIGN_ALG_HASH_EDDSA endif # SUIT_ENVELOPE_APP_RECOVERY_SIGN diff --git a/west.yml b/west.yml index ad4ea8cbafcd..4b6417de34d4 100644 --- a/west.yml +++ b/west.yml @@ -242,10 +242,10 @@ manifest: upstream-sha: c6eaeda5a1c1c5dbb24dce7e027340cb8893a77b compare-by-default: false - name: suit-generator - revision: b37972cd90c122bb8f384f2509b72fad62c3aa4b + revision: 2e22c176ba50930604519c3f70ee8ae5ba2a09ac path: modules/lib/suit-generator - name: suit-processor - revision: a499bcadceff8877da63a0a140c6a91ff2f87b25 + revision: a3f2b6de5f94d8547211b781bde231017caa9cdc path: modules/lib/suit-processor - name: doc-internal repo-path: doc-internal From 70a6f4f15a29a94666a64c597ede45d7a649e5bd Mon Sep 17 00:00:00 2001 From: Artur Hadasz Date: Thu, 23 Jan 2025 16:19:11 +0100 Subject: [PATCH 2/2] suit: Validate algorithm used for signing Nordic elements can only use EdDSA, not Hashed EdDSA Signed-off-by: Artur Hadasz --- subsys/suit/mci/CMakeLists.txt | 1 + subsys/suit/mci/include/suit_mci.h | 11 ++++++----- subsys/suit/mci/src/suit_mci_nrf54h20.c | 11 +++++++---- subsys/suit/mci/src/suit_mci_nrf9280.c | 10 ++++++---- .../suit/platform/sdfw/src/suit_plat_authenticate.c | 4 ++-- tests/subsys/suit/common/mci_test/mci_test.c | 5 ++++- tests/subsys/suit/mci/prj.conf | 1 + tests/subsys/suit/mci/src/api_positive_scenarios.c | 6 ++++-- tests/subsys/suit/mci/src/sanity.c | 7 ++++--- tests/subsys/suit/unit/mocks/include/mock_suit_mci.h | 5 +++-- 10 files changed, 38 insertions(+), 23 deletions(-) diff --git a/subsys/suit/mci/CMakeLists.txt b/subsys/suit/mci/CMakeLists.txt index 2e0236e3c05c..41358969cc67 100644 --- 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 index 332dd6eb4f79..6abee06c8753 100644 --- 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 index abca51532e06..b14c13ee9c95 100644 --- 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 #include +#include #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 index 9c792b270364..e36cb24869b2 100644 --- 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 index 75dd76444d1f..6459b4ce36ae 100644 --- 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 index b03d30ca49e2..45e55ec5b996 100644 --- 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 index 7dac2b41b3ce..3e89031c8ee0 100644 --- 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 index 5585c6dc31c7..c69b030e3b2b 100644 --- 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 index 9af377f11a56..e9b3db5f3a3f 100644 --- 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 index c5700c875483..9185b06d53bc 100644 --- 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);