From ce27a6e7e09eb295e437e040e4eb9cb01f58b48c Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Fri, 28 Aug 2026 15:43:46 +0200 Subject: [PATCH 1/3] fix(esp_security): Stop ECDSA and Key Manager resets from corrupting concurrent crypto ECDSA enable pulses a reset that also holds SHA in reset, and SHA shares its DMA with AES. Key Manager enable pulses a reset that also covers the XTS-AES flash encryption key-usage selector. Neither path was serialized against those victims, so a hardware ECDSA/HMAC/DS operation could corrupt a concurrent SHA/AES transfer or an in-flight encrypted flash read. - Take the SHA/AES lock inside esp_crypto_ecdsa_lock_acquire(), before MPI, matching the DS lock order (sha_aes < mpi) - Add esp_crypto_key_mgr_enable_periph_clk_no_reset() and switch ECDSA, HMAC and DS to it; they only need the key-usage selector writable - Hold esp_crypto_key_manager_lock across those clock enable/disable pairs so selector writes stay serialized without resetting KM --- .../esp_security/include/esp_crypto_lock.h | 10 +++++++-- .../include/esp_crypto_periph_clk.h | 22 ++++++++++++++++++- components/esp_security/src/esp_crypto_lock.c | 20 ++++++++++++++++- .../esp_security/src/esp_crypto_periph_clk.c | 17 ++++++++++++-- components/esp_security/src/esp_ds.c | 10 +++++++-- components/esp_security/src/esp_hmac.c | 14 +++++++++--- .../esp_ecdsa/psa_crypto_driver_esp_ecdsa.c | 10 +++++++-- 7 files changed, 90 insertions(+), 13 deletions(-) diff --git a/components/esp_security/include/esp_crypto_lock.h b/components/esp_security/include/esp_crypto_lock.h index 76200e66194..5a419f6e9d2 100644 --- a/components/esp_security/include/esp_crypto_lock.h +++ b/components/esp_security/include/esp_crypto_lock.h @@ -110,14 +110,16 @@ void esp_crypto_ecc_lock_release(void); /** * @brief Acquire lock for ECDSA cryptography peripheral * - * Internally also locks the ECC and MPI peripheral, as the ECDSA depends on these peripherals + * Internally also locks the ECC and MPI peripheral, as the ECDSA depends on these peripherals, + * and the SHA/AES peripheral, because the ECDSA reset holds SHA in reset as well */ void esp_crypto_ecdsa_lock_acquire(void); /** * @brief Release lock for ECDSA cryptography peripheral * - * Internally also releases the ECC and MPI peripheral, as the ECDSA depends on these peripherals + * Internally also releases the ECC and MPI peripheral, as the ECDSA depends on these peripherals, + * and the SHA/AES peripheral, because the ECDSA reset holds SHA in reset as well */ void esp_crypto_ecdsa_lock_release(void); #endif /* SOC_ECDSA_SUPPORTED */ @@ -126,12 +128,16 @@ void esp_crypto_ecdsa_lock_release(void); /** * @brief Acquire lock for Key Manager peripheral * + * Must be held across esp_crypto_key_mgr_enable_periph_clk(true/false): that + * helper pulses the Key Manager reset, which also covers the XTS-AES flash + * encryption key-usage selector on targets that deploy FE keys through KM. */ void esp_crypto_key_manager_lock_acquire(void); /** * @brief Release lock for Key Manager peripheral * + * Must be released only after the matching esp_crypto_key_mgr_enable_periph_clk(false). */ void esp_crypto_key_manager_lock_release(void); #endif /* SOC_KEY_MANAGER_SUPPORT_KEY_DEPLOYMENT */ diff --git a/components/esp_security/include/esp_crypto_periph_clk.h b/components/esp_security/include/esp_crypto_periph_clk.h index 1555fce565c..56dfa8f0c16 100644 --- a/components/esp_security/include/esp_crypto_periph_clk.h +++ b/components/esp_security/include/esp_crypto_periph_clk.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -63,10 +63,30 @@ void esp_crypto_ecdsa_enable_periph_clk(bool enable); /** * @brief Enable or disable the Key Manager peripheral clock * + * When enable is true this also pulses the Key Manager reset. The caller must + * hold esp_crypto_key_manager_lock across the matching true/false pair, because + * that reset also covers the XTS-AES flash encryption key-usage selector. + * + * Prefer esp_crypto_key_mgr_enable_periph_clk_no_reset() when the caller only + * needs the key-usage selector writable (ECDSA/HMAC/DS). + * * @param enable true: enable; false: disable */ void esp_crypto_key_mgr_enable_periph_clk(bool enable); +/** + * @brief Enable or disable the Key Manager clocks without resetting the peripheral + * + * Use this when a crypto accelerator only needs to write its own key-usage + * selector. Resetting would drop the XTS-AES flash encryption selector that + * MSPI may be using, and flash DMA does not take the Key Manager lock. + * The caller must still hold esp_crypto_key_manager_lock across the matching + * true/false pair to serialize selector writes. + * + * @param enable true: enable; false: disable + */ +void esp_crypto_key_mgr_enable_periph_clk_no_reset(bool enable); + #ifdef __cplusplus } #endif diff --git a/components/esp_security/src/esp_crypto_lock.c b/components/esp_security/src/esp_crypto_lock.c index 2633e7ac25b..7dd20eff564 100644 --- a/components/esp_security/src/esp_crypto_lock.c +++ b/components/esp_security/src/esp_crypto_lock.c @@ -15,7 +15,9 @@ MPI/RSA: independent ECC: independent HMAC: needs SHA DS: needs HMAC (which needs SHA), AES and MPI -ECDSA: needs ECC and MPI +ECDSA: needs ECC and MPI, and its reset pulse holds SHA (and thus the SHA/AES DMA) in reset +Key Manager: shared key-usage selectors (ECDSA/HMAC/DS/XTS-AES flash); + esp_crypto_key_mgr_enable_periph_clk(true) resets it */ #if !NON_OS_BUILD @@ -140,6 +142,19 @@ void esp_crypto_ecdsa_lock_acquire(void) { _lock_acquire(&s_crypto_ecdsa_lock); esp_crypto_ecc_lock_acquire(); +#if defined(SOC_SHA_SUPPORTED) || defined(SOC_AES_SUPPORTED) + /* Enabling the ECDSA peripheral pulses the ECDSA reset + (esp_crypto_ecdsa_enable_periph_clk() -> ecdsa_ll_reset_register()), and on every + target that has an ECDSA peripheral that reset also holds SHA in reset: see the + "otherwise SHA is held in reset" note in sha_ll_reset_register(). SHA shares its + (G)DMA channel with AES, and the SHA/AES lock is what serializes both of them, so + it has to be held across the pulse. Without it, a hardware ECDSA operation on one + core lands in the middle of an unrelated SHA or AES transfer on the other core, + which completes without an error but yields wrong output. + Taken before the MPI lock to keep the acquisition order of + esp_crypto_ds_lock_acquire() (SHA/AES before MPI) and avoid a lock cycle. */ + esp_crypto_sha_aes_lock_acquire(); +#endif /* defined(SOC_SHA_SUPPORTED) || defined(SOC_AES_SUPPORTED) */ #ifdef SOC_ECDSA_USES_MPI if (ecdsa_ll_is_mpi_required()) { esp_crypto_mpi_lock_acquire(); @@ -154,6 +169,9 @@ void esp_crypto_ecdsa_lock_release(void) esp_crypto_mpi_lock_release(); } #endif /* SOC_ECDSA_USES_MPI */ +#if defined(SOC_SHA_SUPPORTED) || defined(SOC_AES_SUPPORTED) + esp_crypto_sha_aes_lock_release(); +#endif /* defined(SOC_SHA_SUPPORTED) || defined(SOC_AES_SUPPORTED) */ esp_crypto_ecc_lock_release(); _lock_release(&s_crypto_ecdsa_lock); } diff --git a/components/esp_security/src/esp_crypto_periph_clk.c b/components/esp_security/src/esp_crypto_periph_clk.c index cdd65057c12..f07a3307abb 100644 --- a/components/esp_security/src/esp_crypto_periph_clk.c +++ b/components/esp_security/src/esp_crypto_periph_clk.c @@ -155,16 +155,29 @@ void esp_crypto_ecdsa_enable_periph_clk(bool enable) #endif #if SOC_KEY_MANAGER_SUPPORT_KEY_DEPLOYMENT -void esp_crypto_key_mgr_enable_periph_clk(bool enable) +static void key_mgr_configure_periph_clk(bool enable, bool reset) { KEY_MANAGER_RCC_ATOMIC() { esp_crypto_common_clk_enable(enable); key_mgr_ll_power_up(); key_mgr_ll_enable_bus_clock(enable); key_mgr_ll_enable_peripheral_clock(enable); - if (enable) { + if (enable && reset) { key_mgr_ll_reset_register(); } } } + +void esp_crypto_key_mgr_enable_periph_clk(bool enable) +{ + /* Caller must hold esp_crypto_key_manager_lock: this reset also covers + the XTS-AES flash encryption key-usage selector. */ + key_mgr_configure_periph_clk(enable, enable); +} + +void esp_crypto_key_mgr_enable_periph_clk_no_reset(bool enable) +{ + /* Caller must hold esp_crypto_key_manager_lock to serialize selector writes. */ + key_mgr_configure_periph_clk(enable, false); +} #endif diff --git a/components/esp_security/src/esp_ds.c b/components/esp_security/src/esp_ds.c index 3d9dd795cb1..b713ff9558c 100644 --- a/components/esp_security/src/esp_ds.c +++ b/components/esp_security/src/esp_ds.c @@ -269,15 +269,21 @@ static void ds_acquire_enable(void) /* Key Manager holds the key usage selector register(efuse vs own key). Thus, we need to enable the Key Manager peripheral clock to ensure that the key usage selector register is properly set. + Taken after the DS lock (SHA/AES + MPI) so the order matches HMAC/ECDSA: + sha_aes < mpi < key_manager. */ - esp_crypto_key_mgr_enable_periph_clk(true); + esp_crypto_key_manager_lock_acquire(); + /* Clock only: a full KM reset would drop the XTS-AES flash encryption + key-usage selector, and spi_flash DMA does not take the KM lock. */ + esp_crypto_key_mgr_enable_periph_clk_no_reset(true); #endif /* SOC_KEY_MANAGER_DS_KEY_DEPLOY */ } static void ds_disable_release(void) { #if SOC_KEY_MANAGER_DS_KEY_DEPLOY - esp_crypto_key_mgr_enable_periph_clk(false); + esp_crypto_key_mgr_enable_periph_clk_no_reset(false); + esp_crypto_key_manager_lock_release(); #endif /* SOC_KEY_MANAGER_DS_KEY_DEPLOY */ esp_crypto_ds_enable_periph_clk(false); diff --git a/components/esp_security/src/esp_hmac.c b/components/esp_security/src/esp_hmac.c index 3b458f76ff0..d5d1fd157f2 100644 --- a/components/esp_security/src/esp_hmac.c +++ b/components/esp_security/src/esp_hmac.c @@ -79,8 +79,14 @@ esp_err_t esp_hmac_calculate(hmac_key_id_t key_id, /* Key Manager holds the key usage selector register(efuse vs own key). Thus, we need to enable the Key Manager peripheral clock to ensure that the key usage selector register is properly set. + Taken after the HMAC lock (SHA/AES) so the order matches ECDSA/DS: + sha_aes < mpi < key_manager. Do not take it earlier: ECDSA already + holds MPI before KM, and reversing that here would deadlock. */ - esp_crypto_key_mgr_enable_periph_clk(true); + esp_crypto_key_manager_lock_acquire(); + /* Clock only: a full KM reset would drop the XTS-AES flash encryption + key-usage selector, and spi_flash DMA does not take the KM lock. */ + esp_crypto_key_mgr_enable_periph_clk_no_reset(true); #endif /* SOC_KEY_MANAGER_HMAC_KEY_DEPLOY */ hmac_hal_start(); @@ -90,7 +96,8 @@ esp_err_t esp_hmac_calculate(hmac_key_id_t key_id, esp_crypto_sha_enable_periph_clk(false); esp_crypto_hmac_enable_periph_clk(false); #if SOC_KEY_MANAGER_HMAC_KEY_DEPLOY - esp_crypto_key_mgr_enable_periph_clk(false); + esp_crypto_key_mgr_enable_periph_clk_no_reset(false); + esp_crypto_key_manager_lock_release(); #endif // SOC_KEY_MANAGER_HMAC_KEY_DEPLOY esp_crypto_hmac_lock_release(); return ESP_FAIL; @@ -149,7 +156,8 @@ esp_err_t esp_hmac_calculate(hmac_key_id_t key_id, hmac_hal_read_result_256(hmac); #if SOC_KEY_MANAGER_HMAC_KEY_DEPLOY - esp_crypto_key_mgr_enable_periph_clk(false); + esp_crypto_key_mgr_enable_periph_clk_no_reset(false); + esp_crypto_key_manager_lock_release(); #endif /* SOC_KEY_MANAGER_HMAC_KEY_DEPLOY */ esp_crypto_sha_enable_periph_clk(false); diff --git a/components/mbedtls/port/psa_driver/esp_ecdsa/psa_crypto_driver_esp_ecdsa.c b/components/mbedtls/port/psa_driver/esp_ecdsa/psa_crypto_driver_esp_ecdsa.c index f7900c277c9..46849f7a459 100644 --- a/components/mbedtls/port/psa_driver/esp_ecdsa/psa_crypto_driver_esp_ecdsa.c +++ b/components/mbedtls/port/psa_driver/esp_ecdsa/psa_crypto_driver_esp_ecdsa.c @@ -393,8 +393,13 @@ static void esp_ecdsa_acquire_hardware(void) /* Key Manager holds the key usage selector register (efuse vs own key). Thus, we need to enable the Key Manager peripheral clock to ensure that the key usage selector register is properly set. + Taken after the ECDSA lock (which already holds SHA/AES and MPI) so + the order matches HMAC/DS: sha_aes < mpi < key_manager. */ - esp_crypto_key_mgr_enable_periph_clk(true); + esp_crypto_key_manager_lock_acquire(); + /* Clock only: a full KM reset would drop the XTS-AES flash encryption + key-usage selector, and spi_flash DMA does not take the KM lock. */ + esp_crypto_key_mgr_enable_periph_clk_no_reset(true); #endif /* SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY */ #if SOC_ECDSA_USES_MPI @@ -414,7 +419,8 @@ static void esp_ecdsa_release_hardware(void) esp_crypto_ecc_enable_periph_clk(false); #if SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY - esp_crypto_key_mgr_enable_periph_clk(false); + esp_crypto_key_mgr_enable_periph_clk_no_reset(false); + esp_crypto_key_manager_lock_release(); #endif /* SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY */ #if SOC_ECDSA_USES_MPI From 6f6e2aa93c14abf4e43f0ea6dc13a451ae765017 Mon Sep 17 00:00:00 2001 From: "radek.tandler" Date: Mon, 31 Aug 2026 15:44:41 +0200 Subject: [PATCH 2/3] fix(mbedtls): Fix mbedtls testapps false memory leaks by lazy mutex creation --- .../persistent_storage_format/main/app_main.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/components/mbedtls/test_apps/persistent_storage_format/main/app_main.c b/components/mbedtls/test_apps/persistent_storage_format/main/app_main.c index 42463fd9382..75e933aec9f 100644 --- a/components/mbedtls/test_apps/persistent_storage_format/main/app_main.c +++ b/components/mbedtls/test_apps/persistent_storage_format/main/app_main.c @@ -10,10 +10,28 @@ #include "esp_newlib.h" #include "memory_checks.h" #include "nvs_flash.h" +#include "psa/crypto.h" #include "unity.h" +#include "test_persistent_format.h" + +/* First ITS access caches the NVS psa_its namespace (and related one-shot + * PSA storage state). Prime it before leak accounting so consume tests are + * not charged ~1.2 KB against the 1200-byte critical threshold — the same + * pattern mbedtls_ut uses for AES interrupt allocation. */ +static void prime_psa_its(psa_key_id_t id) +{ + psa_key_attributes_t attr = PSA_KEY_ATTRIBUTES_INIT; + (void)psa_get_key_attributes(id, &attr); + psa_reset_key_attributes(&attr); + (void)psa_purge_key(id); +} void setUp(void) { + prime_psa_its(ESP_PERSISTENT_FIXTURE_DS_KEY_ID); + prime_psa_its(ESP_PERSISTENT_FIXTURE_HMAC_KEY_ID); + prime_psa_its(ESP_PERSISTENT_FIXTURE_ECDSA_KEY_ID); + test_utils_record_free_mem(); test_utils_set_leak_level(CONFIG_UNITY_CRITICAL_LEAK_LEVEL_GENERAL, ESP_LEAK_TYPE_CRITICAL, ESP_COMP_LEAK_GENERAL); From de3a510a18a4db4c193848e42c2762f67b1873c1 Mon Sep 17 00:00:00 2001 From: "harshal.patil" Date: Thu, 3 Sep 2026 19:08:48 +0530 Subject: [PATCH 3/3] fix(esp_security): cover the crypto reset coupling in the driver locks A peripheral's reset also resets the ones it occupies, so a lock has to cover both. Gate the ECDSA MPI lock on SOC_ECDSA_USES_MPI rather than the runtime ecdsa_ll_is_mpi_required() and set that capability on C5, lock the Key Manager path in esp_key_mgr.c, clean HMAC after its reset, and enable DS before the primitives its reset covers. --- .../esp32c5/include/hal/ecdsa_ll.h | 10 ++- components/esp_security/src/esp_crypto_lock.c | 78 +++++++++++-------- .../esp_security/src/esp_crypto_periph_clk.c | 1 + components/esp_security/src/esp_ds.c | 7 +- components/esp_security/src/esp_key_mgr.c | 46 +++++++---- .../esp32c5/include/soc/Kconfig.soc_caps.in | 4 + components/soc/esp32c5/include/soc/soc_caps.h | 1 + components/soc/esp32h2/include/soc/soc_caps.h | 2 +- .../soc/esp32h21/include/soc/soc_caps.h | 1 + components/soc/esp32p4/include/soc/soc_caps.h | 2 +- 10 files changed, 100 insertions(+), 52 deletions(-) diff --git a/components/esp_hal_security/esp32c5/include/hal/ecdsa_ll.h b/components/esp_hal_security/esp32c5/include/hal/ecdsa_ll.h index 031df896ddb..5ff24638c3e 100644 --- a/components/esp_hal_security/esp32c5/include/hal/ecdsa_ll.h +++ b/components/esp_hal_security/esp32c5/include/hal/ecdsa_ll.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -445,6 +445,14 @@ __attribute__((always_inline)) static inline void ecdsa_ll_set_ecdsa_key_blk(ecd } } +/** + * @brief Check if the ECDSA peripheral uses MPI module's memory + */ +static inline bool ecdsa_ll_is_mpi_required(void) +{ + return false; +} + /** * @brief Check if the ECDSA peripheral is supported on this chip revision * For ESP32-C5, ECDSA is always supported diff --git a/components/esp_security/src/esp_crypto_lock.c b/components/esp_security/src/esp_crypto_lock.c index 7dd20eff564..f98b267d03a 100644 --- a/components/esp_security/src/esp_crypto_lock.c +++ b/components/esp_security/src/esp_crypto_lock.c @@ -8,16 +8,41 @@ #include "esp_crypto_lock.h" -/* Lock overview: -SHA: peripheral independent, but DMA is shared with AES -AES: peripheral independent, but DMA is shared with SHA -MPI/RSA: independent -ECC: independent -HMAC: needs SHA -DS: needs HMAC (which needs SHA), AES and MPI -ECDSA: needs ECC and MPI, and its reset pulse holds SHA (and thus the SHA/AES DMA) in reset -Key Manager: shared key-usage selectors (ECDSA/HMAC/DS/XTS-AES flash); - esp_crypto_key_mgr_enable_periph_clk(true) resets it +/* Lock overview. + + Two separate relations decide what a lock must cover: + + 1. Functional dependency - which peripherals an operation drives: + SHA: independent, but DMA is shared with AES + AES: independent, but DMA is shared with SHA + MPI/RSA: independent + ECC: independent + HMAC: needs SHA + DS: needs HMAC (which needs SHA), AES and MPI + ECDSA: needs ECC, SHA where the K value is derived deterministically or + the Z value is taken from SHA rather than supplied, and MPI on + some targets + + 2. Reset coupling - which peripherals are also reset when this one's RST_EN is + pulsed, because the hardware reset tree is shared: + AES/SHA/MPI/ECC: itself only + HMAC: HMAC, SHA + DS: DS, AES, SHA, MPI + ECDSA: ECDSA, SHA, ECC, and MPI where SOC_ECDSA_USES_MPI + KM: KM, AES, ECC + + A lock must cover the union of both. The reset coupling is why the ECDSA lock + takes the SHA/AES and MPI locks even though an ECDSA operation does not + necessarily use those engines. + + The Key Manager holds key usage selectors shared by ECDSA, HMAC, DS and the + XTS-AES engines. The accelerator paths take the Key Manager lock around the + clock enable that lets those selectors be written; only the Key Manager's own + driver resets the peripheral, because that reset is one of the couplings above. + + + Acquisition order, which every path must follow to stay deadlock-free: + DS -> ECDSA -> HMAC -> ECC -> SHA/AES -> MPI -> Key Manager */ #if !NON_OS_BUILD @@ -49,9 +74,6 @@ static _lock_t s_crypto_ecc_lock; #ifdef SOC_ECDSA_SUPPORTED /* Lock for ECDSA peripheral */ static _lock_t s_crypto_ecdsa_lock; -#if SOC_ECDSA_USES_MPI -#include "hal/ecdsa_ll.h" -#endif /* SOC_ECDSA_USES_MPI */ #endif /* SOC_ECDSA_SUPPORTED */ #if SOC_KEY_MANAGER_SUPPORT_KEY_DEPLOYMENT @@ -143,32 +165,22 @@ void esp_crypto_ecdsa_lock_acquire(void) _lock_acquire(&s_crypto_ecdsa_lock); esp_crypto_ecc_lock_acquire(); #if defined(SOC_SHA_SUPPORTED) || defined(SOC_AES_SUPPORTED) - /* Enabling the ECDSA peripheral pulses the ECDSA reset - (esp_crypto_ecdsa_enable_periph_clk() -> ecdsa_ll_reset_register()), and on every - target that has an ECDSA peripheral that reset also holds SHA in reset: see the - "otherwise SHA is held in reset" note in sha_ll_reset_register(). SHA shares its - (G)DMA channel with AES, and the SHA/AES lock is what serializes both of them, so - it has to be held across the pulse. Without it, a hardware ECDSA operation on one - core lands in the middle of an unrelated SHA or AES transfer on the other core, - which completes without an error but yields wrong output. - Taken before the MPI lock to keep the acquisition order of - esp_crypto_ds_lock_acquire() (SHA/AES before MPI) and avoid a lock cycle. */ + /* The ECDSA reset holds SHA, which shares its DMA with AES. Taken before MPI + to keep esp_crypto_ds_lock_acquire()'s order. */ esp_crypto_sha_aes_lock_acquire(); #endif /* defined(SOC_SHA_SUPPORTED) || defined(SOC_AES_SUPPORTED) */ -#ifdef SOC_ECDSA_USES_MPI - if (ecdsa_ll_is_mpi_required()) { - esp_crypto_mpi_lock_acquire(); - } -#endif /* SOC_ECDSA_USES_MPI */ + /* Unconditional under the cap: the reset coupling is present whether or not + this revision needs the MPI engine. */ +#if (SOC_MPI_SUPPORTED && SOC_ECDSA_USES_MPI) + esp_crypto_mpi_lock_acquire(); +#endif /* (SOC_MPI_SUPPORTED && SOC_ECDSA_USES_MPI) */ } void esp_crypto_ecdsa_lock_release(void) { -#ifdef SOC_ECDSA_USES_MPI - if (ecdsa_ll_is_mpi_required()) { - esp_crypto_mpi_lock_release(); - } -#endif /* SOC_ECDSA_USES_MPI */ +#if (SOC_MPI_SUPPORTED && SOC_ECDSA_USES_MPI) + esp_crypto_mpi_lock_release(); +#endif /* (SOC_MPI_SUPPORTED && SOC_ECDSA_USES_MPI) */ #if defined(SOC_SHA_SUPPORTED) || defined(SOC_AES_SUPPORTED) esp_crypto_sha_aes_lock_release(); #endif /* defined(SOC_SHA_SUPPORTED) || defined(SOC_AES_SUPPORTED) */ diff --git a/components/esp_security/src/esp_crypto_periph_clk.c b/components/esp_security/src/esp_crypto_periph_clk.c index f07a3307abb..d8ed0ebb6cb 100644 --- a/components/esp_security/src/esp_crypto_periph_clk.c +++ b/components/esp_security/src/esp_crypto_periph_clk.c @@ -123,6 +123,7 @@ void esp_crypto_hmac_enable_periph_clk(bool enable) hmac_ll_enable_bus_clock(enable); if (enable) { hmac_ll_reset_register(); + hmac_ll_clean(); } } } diff --git a/components/esp_security/src/esp_ds.c b/components/esp_security/src/esp_ds.c index b713ff9558c..368162349a8 100644 --- a/components/esp_security/src/esp_ds.c +++ b/components/esp_security/src/esp_ds.c @@ -259,11 +259,12 @@ static void ds_acquire_enable(void) { esp_crypto_ds_lock_acquire(); - // We also enable SHA and HMAC here. SHA is used by HMAC, HMAC is used by DS. + /* DS first: its reset also resets AES, SHA and MPI, so anything enabled + before it would be reset again here. */ + esp_crypto_ds_enable_periph_clk(true); esp_crypto_hmac_enable_periph_clk(true); esp_crypto_sha_enable_periph_clk(true); esp_crypto_mpi_enable_periph_clk(true); - esp_crypto_ds_enable_periph_clk(true); #if SOC_KEY_MANAGER_DS_KEY_DEPLOY /* Key Manager holds the key usage selector register(efuse vs own key). @@ -286,10 +287,10 @@ static void ds_disable_release(void) esp_crypto_key_manager_lock_release(); #endif /* SOC_KEY_MANAGER_DS_KEY_DEPLOY */ - esp_crypto_ds_enable_periph_clk(false); esp_crypto_mpi_enable_periph_clk(false); esp_crypto_sha_enable_periph_clk(false); esp_crypto_hmac_enable_periph_clk(false); + esp_crypto_ds_enable_periph_clk(false); esp_crypto_ds_lock_release(); } diff --git a/components/esp_security/src/esp_key_mgr.c b/components/esp_security/src/esp_key_mgr.c index 2540573f457..8dcf083f37f 100644 --- a/components/esp_security/src/esp_key_mgr.c +++ b/components/esp_security/src/esp_key_mgr.c @@ -118,13 +118,27 @@ static void esp_key_mgr_release_key_lock(esp_key_mgr_key_type_t key_type) } #endif /* NON_OS_BUILD */ +/* The Key Manager reset also resets AES and ECC, and the sequences guarded here + drive the state machine and write the shared key usage selector. Callers of + esp_key_mgr_acquire_hardware()/release_hardware() hold all three locks. */ +static void key_mgr_crypto_lock_acquire(void) +{ + esp_crypto_ecc_lock_acquire(); + esp_crypto_sha_aes_lock_acquire(); + esp_crypto_key_manager_lock_acquire(); +} + +static void key_mgr_crypto_lock_release(void) +{ + esp_crypto_key_manager_lock_release(); + esp_crypto_sha_aes_lock_release(); + esp_crypto_ecc_lock_release(); +} + static void esp_key_mgr_acquire_hardware(bool deployment_mode) { if (deployment_mode) { - // We only need explicit locks in the deployment mode - esp_crypto_ecc_lock_acquire(); - esp_crypto_sha_aes_lock_acquire(); - esp_crypto_key_manager_lock_acquire(); + key_mgr_crypto_lock_acquire(); // The KM peripheral uses the external ECC block for the ECDH0/ECDH1 // scalar multiplications; its bus clock must be on, otherwise the KM // deploys an incorrect key. @@ -132,7 +146,6 @@ static void esp_key_mgr_acquire_hardware(bool deployment_mode) esp_crypto_ecc_enable_periph_clk(true); #endif } - // Reset the Key Manager Clock esp_crypto_key_mgr_enable_periph_clk(true); } @@ -142,13 +155,12 @@ static void esp_key_mgr_release_hardware(bool deployment_mode) #if SOC_ECC_SUPPORTED esp_crypto_ecc_enable_periph_clk(false); #endif - esp_crypto_key_manager_lock_release(); - esp_crypto_sha_aes_lock_release(); - esp_crypto_ecc_lock_release(); } - - // Reset the Key Manager Clock esp_crypto_key_mgr_enable_periph_clk(false); + + if (deployment_mode) { + key_mgr_crypto_lock_release(); + } } /** @@ -604,12 +616,12 @@ esp_err_t esp_key_mgr_activate_key(esp_key_mgr_key_recovery_info_t *key_recovery esp_key_mgr_acquire_key_lock(key_type); + key_mgr_crypto_lock_acquire(); esp_key_mgr_acquire_hardware(false); esp_err_t esp_ret = key_mgr_recover_key(&key_recovery_config); if (esp_ret != ESP_OK) { ESP_LOGE(TAG, "Failed to recover key"); - esp_key_mgr_release_key_lock(key_type); goto cleanup; } @@ -619,7 +631,6 @@ esp_err_t esp_key_mgr_activate_key(esp_key_mgr_key_recovery_info_t *key_recovery esp_ret = key_mgr_recover_key(&key_recovery_config); if (esp_ret != ESP_OK) { ESP_LOGE(TAG, "Failed to recover key"); - esp_key_mgr_release_key_lock(key_type); goto cleanup; } } @@ -627,19 +638,28 @@ esp_err_t esp_key_mgr_activate_key(esp_key_mgr_key_recovery_info_t *key_recovery // Set the Key Manager Static Register to use own key for the respective key type key_mgr_hal_set_key_usage(key_type, ESP_KEY_MGR_USE_OWN_KEY); + /* Released here: nothing after this point drives the peripheral. */ + key_mgr_crypto_lock_release(); + ESP_LOGD(TAG, "Key activation for type %d successful", key_type); return ESP_OK; cleanup: ESP_LOGE(TAG, "Key activation failed"); esp_key_mgr_release_hardware(false); + key_mgr_crypto_lock_release(); + esp_key_mgr_release_key_lock(key_type); return esp_ret; } esp_err_t esp_key_mgr_deactivate_key(esp_key_mgr_key_type_t key_type) { - esp_key_mgr_release_key_lock(key_type); + key_mgr_crypto_lock_acquire(); esp_key_mgr_release_hardware(false); + key_mgr_crypto_lock_release(); + + esp_key_mgr_release_key_lock(key_type); + ESP_LOGD(TAG, "Key deactivation successful for type %d", key_type); return ESP_OK; } diff --git a/components/soc/esp32c5/include/soc/Kconfig.soc_caps.in b/components/soc/esp32c5/include/soc/Kconfig.soc_caps.in index 2741a627a3c..47f8781ca0a 100644 --- a/components/soc/esp32c5/include/soc/Kconfig.soc_caps.in +++ b/components/soc/esp32c5/include/soc/Kconfig.soc_caps.in @@ -931,6 +931,10 @@ config SOC_ECDSA_SUPPORT_DETERMINISTIC_MODE bool default y +config SOC_ECDSA_USES_MPI + bool + default y + config SOC_ECDSA_SUPPORT_HW_DETERMINISTIC_LOOP bool default y diff --git a/components/soc/esp32c5/include/soc/soc_caps.h b/components/soc/esp32c5/include/soc/soc_caps.h index 4f28358f4b2..eda95a4ca22 100644 --- a/components/soc/esp32c5/include/soc/soc_caps.h +++ b/components/soc/esp32c5/include/soc/soc_caps.h @@ -384,6 +384,7 @@ /*--------------------------- ECDSA CAPS ---------------------------------------*/ #define SOC_ECDSA_SUPPORT_EXPORT_PUBKEY (1) #define SOC_ECDSA_SUPPORT_DETERMINISTIC_MODE (1) +#define SOC_ECDSA_USES_MPI (1) /*!< ECDSA shares MPI's reset domain: v1.0 dropped ECDSA's use of RSA but kept the clkrst coupling, so the MPI lock is still required */ #define SOC_ECDSA_SUPPORT_HW_DETERMINISTIC_LOOP (1) #define SOC_ECDSA_SUPPORT_CURVE_P384 (1) #define SOC_ECDSA_SUPPORT_CURVE_SPECIFIC_KEY_PURPOSES (1) /*!< Support individual key purposes for different ECDSA curves (P192, P256, P384) */ diff --git a/components/soc/esp32h2/include/soc/soc_caps.h b/components/soc/esp32h2/include/soc/soc_caps.h index 8e08c1b4c6f..af51f2553d3 100644 --- a/components/soc/esp32h2/include/soc/soc_caps.h +++ b/components/soc/esp32h2/include/soc/soc_caps.h @@ -429,7 +429,7 @@ #define SOC_ECC_CONSTANT_TIME_POINT_MUL 1 /*------------------------- ECDSA CAPS -------------------------*/ -#define SOC_ECDSA_USES_MPI (1) +#define SOC_ECDSA_USES_MPI (1) /*!< ECDSA reuses the MPI operand memory below rev v1.2, and shares MPI's reset domain on every revision */ #define SOC_ECDSA_SUPPORT_DETERMINISTIC_MODE (1) #define SOC_ECDSA_SUPPORT_HW_DETERMINISTIC_LOOP (1) #define SOC_ECDSA_P192_CURVE_DEFAULT_DISABLED (1) diff --git a/components/soc/esp32h21/include/soc/soc_caps.h b/components/soc/esp32h21/include/soc/soc_caps.h index 49f329fb98b..ac809a885da 100644 --- a/components/soc/esp32h21/include/soc/soc_caps.h +++ b/components/soc/esp32h21/include/soc/soc_caps.h @@ -408,6 +408,7 @@ #define SOC_ECDSA_SUPPORT_DETERMINISTIC_MODE (1) #define SOC_ECDSA_SUPPORT_HW_DETERMINISTIC_LOOP (1) #define SOC_ECDSA_P192_CURVE_DEFAULT_DISABLED (1) +// #define SOC_ECDSA_USES_MPI 1 // TODO: [ESP32H21] IDF-16142 /*-------------------------- UART CAPS ---------------------------------------*/ // ESP32-H21 has 2 UARTs diff --git a/components/soc/esp32p4/include/soc/soc_caps.h b/components/soc/esp32p4/include/soc/soc_caps.h index c5340114575..9d94437b4fc 100644 --- a/components/soc/esp32p4/include/soc/soc_caps.h +++ b/components/soc/esp32p4/include/soc/soc_caps.h @@ -496,7 +496,7 @@ #define SOC_ECDSA_SUPPORT_EXPORT_PUBKEY (1) #define SOC_ECDSA_SUPPORT_DETERMINISTIC_MODE (1) #define SOC_ECDSA_SUPPORT_HW_DETERMINISTIC_LOOP (1) -#define SOC_ECDSA_USES_MPI (1) +#define SOC_ECDSA_USES_MPI (1) /*!< ECDSA shares MPI's reset domain, so the MPI lock is required even though ECDSA uses neither the MPI engine nor its memory */ #define SOC_ECDSA_SUPPORT_CURVE_P384 (1) #define SOC_ECDSA_SUPPORT_CURVE_SPECIFIC_KEY_PURPOSES (1) /*!< Support individual key purposes for different ECDSA curves (P192, P256, P384) */