From aa754d40b83d3a19d92070ebd13019217787d3d1 Mon Sep 17 00:00:00 2001 From: Aditya Patwardhan Date: Wed, 28 Aug 2024 11:38:00 +0530 Subject: [PATCH] fix(security): Fixed flash encryption for esp32p4 The flash encryption on esp32p4 was broken due to code related to key manager not being executed when key manager support was disabled on esp32p4 target. This commit fixes that behaviour Additionally, the atomic env enablement for key_mgr_ll_enable_peripheral_clock was fixed. --- .../src/flash_encryption/flash_encrypt.c | 35 ++++++++++++------- components/esp_system/port/cpu_start.c | 19 +++++++--- components/hal/ecdsa_hal.c | 19 +++++++--- .../hal/esp32p4/include/hal/key_mgr_ll.h | 17 +++++---- components/hal/include/hal/key_mgr_types.h | 7 +--- .../esp32p4/include/soc/Kconfig.soc_caps.in | 8 +++++ components/soc/esp32p4/include/soc/soc_caps.h | 4 ++- 7 files changed, 71 insertions(+), 38 deletions(-) diff --git a/components/bootloader_support/src/flash_encryption/flash_encrypt.c b/components/bootloader_support/src/flash_encryption/flash_encrypt.c index 850fcb99841f..85697dbeb3b3 100644 --- a/components/bootloader_support/src/flash_encryption/flash_encrypt.c +++ b/components/bootloader_support/src/flash_encryption/flash_encrypt.c @@ -16,11 +16,15 @@ #include "esp_log.h" #include "hal/wdt_hal.h" -#if SOC_KEY_MANAGER_SUPPORTED -#include "hal/key_mgr_hal.h" -#include "hal/mspi_timing_tuning_ll.h" +#if SOC_KEY_MANAGER_FE_KEY_DEPLOY || CONFIG_IDF_TARGET_ESP32C5 +#if CONFIG_IDF_TARGET_ESP32C5 #include "soc/keymng_reg.h" -#endif +#include "soc/pcr_reg.h" +#else /* CONFIG_IDF_TARGET_ESP32C5 */ +#include "hal/key_mgr_ll.h" +#include "hal/mspi_timing_tuning_ll.h" +#endif /* !CONFIG_IDF_TARGET_ESP32C5 */ +#endif /* SOC_KEY_MANAGER_FE_KEY_DEPLOY */ #ifdef CONFIG_SOC_EFUSE_CONSISTS_OF_ONE_KEY_BLOCK #include "soc/sensitive_reg.h" @@ -217,18 +221,25 @@ static esp_err_t check_and_generate_encryption_keys(void) ESP_LOGI(TAG, "Using pre-loaded flash encryption key in efuse"); } -#if SOC_KEY_MANAGER_SUPPORTED -#if CONFIG_IDF_TARGET_ESP32C5 && SOC_KEY_MANAGER_SUPPORTED - // TODO: [ESP32C5] IDF-8622 find a more proper place for these codes - REG_SET_BIT(KEYMNG_STATIC_REG, KEYMNG_USE_EFUSE_KEY_FLASH); +#if SOC_KEY_MANAGER_FE_KEY_DEPLOY || CONFIG_IDF_TARGET_ESP32C5 +#if CONFIG_IDF_TARGET_ESP32C5 + REG_SET_FIELD(KEYMNG_STATIC_REG, KEYMNG_USE_EFUSE_KEY, 2); REG_SET_BIT(PCR_MSPI_CLK_CONF_REG, PCR_MSPI_AXI_RST_EN); REG_CLR_BIT(PCR_MSPI_CLK_CONF_REG, PCR_MSPI_AXI_RST_EN); -#endif +#else /* CONFIG_IDF_TARGET_ESP32C5 */ + // Enable and reset key manager + // To suppress build errors about spinlock's __DECLARE_RCC_ATOMIC_ENV + int __DECLARE_RCC_ATOMIC_ENV __attribute__ ((unused)); + key_mgr_ll_enable_bus_clock(true); + key_mgr_ll_enable_peripheral_clock(true); + key_mgr_ll_reset_register(); + while (key_mgr_ll_get_state() != ESP_KEY_MGR_STATE_IDLE) { + }; // Force Key Manager to use eFuse key for XTS-AES operation - key_mgr_hal_set_key_usage(ESP_KEY_MGR_XTS_AES_128_KEY, ESP_KEY_MGR_USE_EFUSE_KEY); + key_mgr_ll_set_key_usage(ESP_KEY_MGR_XTS_AES_128_KEY, ESP_KEY_MGR_USE_EFUSE_KEY); _mspi_timing_ll_reset_mspi(); -#endif - +#endif /* !CONFIG_IDF_TARGET_ESP32C5 */ +#endif /* SOC_KEY_MANAGER_FE_KEY_DEPLOY */ return ESP_OK; } diff --git a/components/esp_system/port/cpu_start.c b/components/esp_system/port/cpu_start.c index 649007cdfaac..6fbe2ff95d3c 100644 --- a/components/esp_system/port/cpu_start.c +++ b/components/esp_system/port/cpu_start.c @@ -71,8 +71,8 @@ #include "soc/hp_sys_clkrst_reg.h" #endif -#if SOC_KEY_MANAGER_SUPPORTED -#include "hal/key_mgr_hal.h" +#if SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY || SOC_KEY_MANAGER_FE_KEY_DEPLOY +#include "hal/key_mgr_ll.h" #endif #include "esp_private/rtc_clk.h" @@ -309,13 +309,22 @@ static void start_other_core(void) } #endif -#if SOC_KEY_MANAGER_SUPPORTED // The following operation makes the Key Manager to use eFuse key for ECDSA and XTS-AES operation by default // This is to keep the default behavior same as the other chips // If the Key Manager configuration is already locked then following operation does not have any effect - key_mgr_hal_set_key_usage(ESP_KEY_MGR_ECDSA_KEY, ESP_KEY_MGR_USE_EFUSE_KEY); - key_mgr_hal_set_key_usage(ESP_KEY_MGR_XTS_AES_128_KEY, ESP_KEY_MGR_USE_EFUSE_KEY); +#if SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY || SOC_KEY_MANAGER_FE_KEY_DEPLOY + // Enable key manager clock + // Using ll APIs which do not require critical section + _key_mgr_ll_enable_bus_clock(true); + _key_mgr_ll_enable_peripheral_clock(true); +#if SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY + key_mgr_ll_set_key_usage(ESP_KEY_MGR_ECDSA_KEY, ESP_KEY_MGR_USE_EFUSE_KEY); #endif +#if SOC_KEY_MANAGER_FE_KEY_DEPLOY + key_mgr_ll_set_key_usage(ESP_KEY_MGR_XTS_AES_128_KEY, ESP_KEY_MGR_USE_EFUSE_KEY); +#endif +#endif /* SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY || SOC_KEY_MANAGER_FE_KEY_DEPLOY */ + ets_set_appcpu_boot_addr((uint32_t)call_start_cpu1); bool cpus_up = false; diff --git a/components/hal/ecdsa_hal.c b/components/hal/ecdsa_hal.c index 552fb423af06..73302b1f43f1 100644 --- a/components/hal/ecdsa_hal.c +++ b/components/hal/ecdsa_hal.c @@ -9,7 +9,11 @@ #include "hal/ecdsa_hal.h" #include "hal/efuse_hal.h" -#ifdef SOC_KEY_MANAGER_SUPPORTED +#if CONFIG_IDF_TARGET_ESP32C5 +#include "soc/keymng_reg.h" +#endif + +#ifdef SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY #include "hal/key_mgr_hal.h" #endif @@ -19,16 +23,21 @@ static void configure_ecdsa_periph(ecdsa_hal_config_t *conf) { - if (conf->use_km_key == 0) { efuse_hal_set_ecdsa_key(conf->efuse_key_blk); -#if SOC_KEY_MANAGER_SUPPORTED - key_mgr_hal_set_key_usage(ESP_KEY_MGR_ECDSA_KEY, ESP_KEY_MGR_USE_EFUSE_KEY); + +#if CONFIG_IDF_TARGET_ESP32C5 + REG_SET_FIELD(KEYMNG_STATIC_REG, KEYMNG_USE_EFUSE_KEY, 1); +#endif + +#if SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY + // Force Key Manager to use eFuse key for XTS-AES operation + key_mgr_ll_set_key_usage(ESP_KEY_MGR_ECDSA_KEY, ESP_KEY_MGR_USE_EFUSE_KEY); #endif } #if SOC_KEY_MANAGER_SUPPORTED else { - key_mgr_hal_set_key_usage(ESP_KEY_MGR_ECDSA_KEY, ESP_KEY_MGR_USE_OWN_KEY); + key_mgr_ll_set_key_usage(ESP_KEY_MGR_ECDSA_KEY, ESP_KEY_MGR_USE_OWN_KEY); } #endif diff --git a/components/hal/esp32p4/include/hal/key_mgr_ll.h b/components/hal/esp32p4/include/hal/key_mgr_ll.h index 1d6d05eba57c..15923a67ffa0 100644 --- a/components/hal/esp32p4/include/hal/key_mgr_ll.h +++ b/components/hal/esp32p4/include/hal/key_mgr_ll.h @@ -10,9 +10,7 @@ ******************************************************************************/ #pragma once -#include "soc/soc_caps.h" -#if SOC_KEY_MANAGER_SUPPORTED #include #include #include @@ -21,7 +19,6 @@ #include "hal/key_mgr_types.h" #include "soc/keymng_reg.h" #include "soc/hp_sys_clkrst_struct.h" -#include "soc/soc_caps.h" #ifdef __cplusplus extern "C" { @@ -29,29 +26,32 @@ extern "C" { /** * @brief Enable the bus clock for Key Manager peripheral - * + * Note: Please use key_mgr_ll_enable_bus_clock which requires the critical section + * and do not use _key_mgr_ll_enable_bus_clock * @param true to enable, false to disable */ -static inline void key_mgr_ll_enable_bus_clock(bool enable) +static inline void _key_mgr_ll_enable_bus_clock(bool enable) { HP_SYS_CLKRST.soc_clk_ctrl1.reg_key_manager_sys_clk_en = enable; } /// use a macro to wrap the function, force the caller to use it in a critical section /// the critical section needs to declare the __DECLARE_RCC_ATOMIC_ENV variable in advance -#define key_mgr_ll_enable_bus_clock(...) (void)__DECLARE_RCC_ATOMIC_ENV; key_mgr_ll_enable_bus_clock(__VA_ARGS__) +#define key_mgr_ll_enable_bus_clock(...) (void)__DECLARE_RCC_ATOMIC_ENV; _key_mgr_ll_enable_bus_clock(__VA_ARGS__) /** * @brief Enable the peripheral clock for Key Manager * + * Note: Please use key_mgr_ll_enable_peripheral_clock which requires the critical section + * and do not use _key_mgr_ll_enable_peripheral_clock * @param true to enable, false to disable */ -static inline void key_mgr_ll_enable_peripheral_clock(bool enable) +static inline void _key_mgr_ll_enable_peripheral_clock(bool enable) { HP_SYS_CLKRST.peri_clk_ctrl25.reg_crypto_km_clk_en = enable; } -#define key_mgr_ll_enable_peripheral_clock(...) (void)__DECLARE_RCC_ATOMIC_ENV; key_mgr_ll_enable_bus_clock(__VA_ARGS__) +#define key_mgr_ll_enable_peripheral_clock(...) (void)__DECLARE_RCC_ATOMIC_ENV; _key_mgr_ll_enable_peripheral_clock(__VA_ARGS__) /** * @brief Reset the Key Manager peripheral */ @@ -345,4 +345,3 @@ static inline uint32_t key_mgr_ll_get_date_info(void) #ifdef __cplusplus } #endif -#endif diff --git a/components/hal/include/hal/key_mgr_types.h b/components/hal/include/hal/key_mgr_types.h index 82064e58d4a0..31ea39aeb4dc 100644 --- a/components/hal/include/hal/key_mgr_types.h +++ b/components/hal/include/hal/key_mgr_types.h @@ -5,9 +5,6 @@ */ #pragma once -#include "soc/soc_caps.h" - -#if SOC_KEY_MANAGER_SUPPORTED #include #include #include @@ -24,7 +21,7 @@ extern "C" { */ typedef enum { ESP_KEY_MGR_STATE_IDLE = 0, /* Key Manager is idle */ - ESP_KEY_MGR_STATE_LOAD = 1, /* Key Manager is ready to recieve input */ + ESP_KEY_MGR_STATE_LOAD = 1, /* Key Manager is ready to receive input */ ESP_KEY_MGR_STATE_GAIN = 2, /* Key Manager is ready to provide output */ ESP_KEY_MGR_STATE_BUSY = 3, /* Key Manager is busy */ } esp_key_mgr_state_t; @@ -114,5 +111,3 @@ typedef struct WORD_ALIGNED_ATTR PACKED_ATTR { #ifdef __cplusplus } #endif - -#endif diff --git a/components/soc/esp32p4/include/soc/Kconfig.soc_caps.in b/components/soc/esp32p4/include/soc/Kconfig.soc_caps.in index 1201d5df2412..e55ced27f023 100644 --- a/components/soc/esp32p4/include/soc/Kconfig.soc_caps.in +++ b/components/soc/esp32p4/include/soc/Kconfig.soc_caps.in @@ -1483,6 +1483,14 @@ config SOC_EFUSE_ECDSA_KEY bool default y +config SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY + bool + default y + +config SOC_KEY_MANAGER_FE_KEY_DEPLOY + bool + default y + config SOC_SECURE_BOOT_V2_RSA bool default y diff --git a/components/soc/esp32p4/include/soc/soc_caps.h b/components/soc/esp32p4/include/soc/soc_caps.h index 452329f0195e..71d88b890a22 100644 --- a/components/soc/esp32p4/include/soc/soc_caps.h +++ b/components/soc/esp32p4/include/soc/soc_caps.h @@ -582,6 +582,9 @@ #define SOC_EFUSE_DIS_DOWNLOAD_MSPI 1 #define SOC_EFUSE_ECDSA_KEY 1 +/*-------------------------- Key Manager CAPS----------------------------*/ +#define SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY 1 /*!< Key manager responsible to deploy ECDSA key */ +#define SOC_KEY_MANAGER_FE_KEY_DEPLOY 1 /*!< Key manager responsible to deploy Flash Encryption key */ /*-------------------------- Secure Boot CAPS----------------------------*/ #define SOC_SECURE_BOOT_V2_RSA 1 #define SOC_SECURE_BOOT_V2_ECC 1 @@ -595,7 +598,6 @@ #define SOC_FLASH_ENCRYPTION_XTS_AES_OPTIONS 1 #define SOC_FLASH_ENCRYPTION_XTS_AES_128 1 #define SOC_FLASH_ENCRYPTION_XTS_AES_256 1 - /*-------------------------- MEMPROT CAPS ------------------------------------*/ /*-------------------------- UART CAPS ---------------------------------------*/