From beaa37a2e6d6b593d6c0c2c6a2a6ef9f42dba8e2 Mon Sep 17 00:00:00 2001 From: "harshal.patil" Date: Fri, 24 Apr 2026 17:03:26 +0530 Subject: [PATCH] fix(esp_security): guard key manager APIs against unsupported chip revs On ESP32-P4 rev < 3.0, Key Manager is software-disabled, but the public esp_key_mgr.h APIs had no runtime check. Calls using HMAC/DS/PSRAM key types fell through to HAL_ASSERT("Unsupported ...") paths in key_mgr_ll.h. Gate each public API with key_mgr_ll_is_supported() and return ESP_ERR_NOT_SUPPORTED cleanly instead. --- components/esp_security/src/esp_key_mgr.c | 19 +++++++++++- .../test_apps/.build-test-rules.yml | 4 --- .../crypto_drivers/main/test_key_mgr.c | 29 ++++++++++++++++++- .../crypto_drivers/pytest_crypto_drivers.py | 1 - .../main/key_manager/test_key_manager.c | 3 ++ .../hal/test_apps/crypto/pytest_crypto.py | 18 ++++++++++-- ...config.ci.long_aes_operations_esp32p4_rev1 | 8 +++++ 7 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 components/hal/test_apps/crypto/sdkconfig.ci.long_aes_operations_esp32p4_rev1 diff --git a/components/esp_security/src/esp_key_mgr.c b/components/esp_security/src/esp_key_mgr.c index 43e5720d09c..866dadae468 100644 --- a/components/esp_security/src/esp_key_mgr.c +++ b/components/esp_security/src/esp_key_mgr.c @@ -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 */ @@ -17,6 +17,7 @@ #include "esp_efuse.h" #include "hal/key_mgr_types.h" #include "hal/key_mgr_hal.h" +#include "hal/key_mgr_ll.h" #include "hal/huk_types.h" #include "hal/huk_hal.h" #include "rom/key_mgr.h" @@ -350,6 +351,10 @@ static esp_err_t key_mgr_deploy_key_aes_mode(aes_deploy_config_t *config) esp_err_t esp_key_mgr_deploy_key_in_aes_mode(const esp_key_mgr_aes_key_config_t *key_config, esp_key_mgr_key_recovery_info_t *key_recovery_info) { + if (!key_mgr_ll_is_supported()) { + return ESP_ERR_NOT_SUPPORTED; + } + if (key_config == NULL || key_recovery_info == NULL) { return ESP_ERR_INVALID_ARG; } @@ -498,6 +503,10 @@ static esp_err_t key_mgr_recover_key(key_recovery_config_t *config) esp_err_t esp_key_mgr_activate_key(esp_key_mgr_key_recovery_info_t *key_recovery_info) { + if (!key_mgr_ll_is_supported()) { + return ESP_ERR_NOT_SUPPORTED; + } + if (key_recovery_info == NULL) { return ESP_ERR_INVALID_ARG; } @@ -681,6 +690,10 @@ static esp_err_t key_mgr_deploy_key_ecdh0_mode(ecdh0_deploy_config_t *config) esp_err_t esp_key_mgr_deploy_key_in_ecdh0_mode(const esp_key_mgr_ecdh0_key_config_t *key_config, esp_key_mgr_key_recovery_info_t *key_info, esp_key_mgr_ecdh0_info_t *ecdh0_key_info) { + if (!key_mgr_ll_is_supported()) { + return ESP_ERR_NOT_SUPPORTED; + } + if (key_config == NULL || key_info == NULL || ecdh0_key_info == NULL) { return ESP_ERR_INVALID_ARG; } @@ -846,6 +859,10 @@ static esp_err_t key_mgr_deploy_key_random_mode(random_deploy_config_t *config) esp_err_t esp_key_mgr_deploy_key_in_random_mode(const esp_key_mgr_random_key_config_t *key_config, esp_key_mgr_key_recovery_info_t *key_recovery_info) { + if (!key_mgr_ll_is_supported()) { + return ESP_ERR_NOT_SUPPORTED; + } + if (key_config == NULL || key_recovery_info == NULL) { return ESP_ERR_INVALID_ARG; } diff --git a/components/esp_security/test_apps/.build-test-rules.yml b/components/esp_security/test_apps/.build-test-rules.yml index 5a6cdf0da0e..6802faf7539 100644 --- a/components/esp_security/test_apps/.build-test-rules.yml +++ b/components/esp_security/test_apps/.build-test-rules.yml @@ -3,9 +3,5 @@ components/esp_security/test_apps/crypto_drivers: enable: - if: ((SOC_HMAC_SUPPORTED == 1) or (SOC_DIG_SIGN_SUPPORTED == 1)) or (SOC_KEY_MANAGER_SUPPORTED == 1) - disable_test: - - if: IDF_TARGET == "esp32p4" - temporary: true - reason: p4 rev3 migration # TODO: IDF-14418 depends_components: - esp_security diff --git a/components/esp_security/test_apps/crypto_drivers/main/test_key_mgr.c b/components/esp_security/test_apps/crypto_drivers/main/test_key_mgr.c index a2f9c01181e..fba27b59e33 100644 --- a/components/esp_security/test_apps/crypto_drivers/main/test_key_mgr.c +++ b/components/esp_security/test_apps/crypto_drivers/main/test_key_mgr.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Unlicense OR CC0-1.0 */ @@ -48,6 +48,13 @@ typedef struct { static const char *TAG = "key_mgr_test"; +#define SKIP_IF_KEY_MGR_NOT_SUPPORTED() \ + do { \ + if (!key_mgr_ll_is_supported()) { \ + TEST_IGNORE_MESSAGE("Key Manager not supported on this chip"); \ + } \ + } while (0) + #define ENCRYPTED_DATA_SIZE 128 static const uint8_t plaintext_data[ENCRYPTED_DATA_SIZE] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, @@ -140,6 +147,8 @@ static esp_err_t test_xts_aes_key(bool verify) TEST_CASE("Key Manager AES mode: XTS-AES-128 key deployment", "[hw_crypto] [key_mgr]") { + SKIP_IF_KEY_MGR_NOT_SUPPORTED(); + esp_key_mgr_aes_key_config_t *key_config = calloc(1, sizeof(esp_key_mgr_aes_key_config_t)); TEST_ASSERT_NOT_NULL(key_config); @@ -163,6 +172,8 @@ TEST_CASE("Key Manager AES mode: XTS-AES-128 key deployment", "[hw_crypto] [key_ TEST_CASE("Key Manager ECDH0 mode: XTS-AES-128 key deployment", "[hw_crypto] [key_mgr]") { + SKIP_IF_KEY_MGR_NOT_SUPPORTED(); + esp_key_mgr_ecdh0_key_config_t *key_config = calloc(1, sizeof(esp_key_mgr_ecdh0_key_config_t)); TEST_ASSERT_NOT_NULL(key_config); @@ -187,6 +198,8 @@ TEST_CASE("Key Manager ECDH0 mode: XTS-AES-128 key deployment", "[hw_crypto] [ke TEST_CASE("Key Manager Random mode: XTS-AES-128 key deployment", "[hw_crypto] [key_mgr]") { + SKIP_IF_KEY_MGR_NOT_SUPPORTED(); + esp_key_mgr_random_key_config_t *key_config = calloc(1, sizeof(esp_key_mgr_random_key_config_t)); TEST_ASSERT_NOT_NULL(key_config); @@ -208,6 +221,8 @@ TEST_CASE("Key Manager Random mode: XTS-AES-128 key deployment", "[hw_crypto] [k #if SOC_KEY_MANAGER_ECDSA_KEY_DEPLOY TEST_CASE("Key Manager random mode: ECDSA key deployment", "[hw_crypto] [key_mgr]") { + SKIP_IF_KEY_MGR_NOT_SUPPORTED(); + esp_key_mgr_random_key_config_t *key_config = calloc(1, sizeof(esp_key_mgr_random_key_config_t)); TEST_ASSERT_NOT_NULL(key_config); @@ -238,6 +253,8 @@ static esp_err_t test_hmac_key(bool verify) TEST_CASE("Key Manager AES mode: HMAC key deployment", "[hw_crypto] [key_mgr]") { + SKIP_IF_KEY_MGR_NOT_SUPPORTED(); + esp_key_mgr_aes_key_config_t *key_config = calloc(1, sizeof(esp_key_mgr_aes_key_config_t)); TEST_ASSERT_NOT_NULL(key_config); @@ -261,6 +278,8 @@ TEST_CASE("Key Manager AES mode: HMAC key deployment", "[hw_crypto] [key_mgr]") TEST_CASE("Key Manager ECDH0 mode: HMAC key deployment", "[hw_crypto] [key_mgr]") { + SKIP_IF_KEY_MGR_NOT_SUPPORTED(); + esp_key_mgr_ecdh0_key_config_t *key_config = calloc(1, sizeof(esp_key_mgr_ecdh0_key_config_t)); TEST_ASSERT_NOT_NULL(key_config); @@ -285,6 +304,8 @@ TEST_CASE("Key Manager ECDH0 mode: HMAC key deployment", "[hw_crypto] [key_mgr]" TEST_CASE("Key Manager random mode: HMAC key deployment", "[hw_crypto] [key_mgr]") { + SKIP_IF_KEY_MGR_NOT_SUPPORTED(); + esp_key_mgr_random_key_config_t *key_config = calloc(1, sizeof(esp_key_mgr_random_key_config_t)); TEST_ASSERT_NOT_NULL(key_config); @@ -333,6 +354,8 @@ static esp_err_t test_ds_key(void) TEST_CASE("Key Manager AES mode: DS key deployment", "[hw_crypto] [key_mgr]") { + SKIP_IF_KEY_MGR_NOT_SUPPORTED(); + esp_key_mgr_aes_key_config_t *key_config = calloc(1, sizeof(esp_key_mgr_aes_key_config_t)); TEST_ASSERT_NOT_NULL(key_config); @@ -356,6 +379,8 @@ TEST_CASE("Key Manager AES mode: DS key deployment", "[hw_crypto] [key_mgr]") TEST_CASE("Key Manager ECDH0 mode: DS key deployment", "[hw_crypto] [key_mgr]") { + SKIP_IF_KEY_MGR_NOT_SUPPORTED(); + esp_key_mgr_ecdh0_key_config_t *key_config = calloc(1, sizeof(esp_key_mgr_ecdh0_key_config_t)); TEST_ASSERT_NOT_NULL(key_config); @@ -380,6 +405,8 @@ TEST_CASE("Key Manager ECDH0 mode: DS key deployment", "[hw_crypto] [key_mgr]") TEST_CASE("Key Manager random mode: DS key deployment", "[hw_crypto] [key_mgr]") { + SKIP_IF_KEY_MGR_NOT_SUPPORTED(); + esp_key_mgr_random_key_config_t *key_config = calloc(1, sizeof(esp_key_mgr_random_key_config_t)); TEST_ASSERT_NOT_NULL(key_config); diff --git a/components/esp_security/test_apps/crypto_drivers/pytest_crypto_drivers.py b/components/esp_security/test_apps/crypto_drivers/pytest_crypto_drivers.py index 59dc1fdf66d..8e697da9af4 100644 --- a/components/esp_security/test_apps/crypto_drivers/pytest_crypto_drivers.py +++ b/components/esp_security/test_apps/crypto_drivers/pytest_crypto_drivers.py @@ -9,6 +9,5 @@ from pytest_embedded_idf.utils import idf_parametrize @idf_parametrize( 'target', ['esp32s2', 'esp32s3', 'esp32c3', 'esp32c6', 'esp32h2', 'esp32p4', 'esp32c5'], indirect=['target'] ) -@pytest.mark.temp_skip_ci(targets=['esp32p4'], reason='p4 rev3 migration # TODO: IDF-14418') def test_crypto_drivers(dut: Dut) -> None: dut.run_all_single_board_cases(timeout=180) diff --git a/components/hal/test_apps/crypto/main/key_manager/test_key_manager.c b/components/hal/test_apps/crypto/main/key_manager/test_key_manager.c index 121b49a6c87..a14349e7abb 100644 --- a/components/hal/test_apps/crypto/main/key_manager/test_key_manager.c +++ b/components/hal/test_apps/crypto/main/key_manager/test_key_manager.c @@ -425,6 +425,9 @@ TEST_GROUP(key_manager); TEST_SETUP(key_manager) { + if (!key_mgr_ll_is_supported()) { + TEST_IGNORE_MESSAGE("Key Manager not supported on this chip"); + } test_utils_record_free_mem(); TEST_ESP_OK(test_utils_set_leak_level(800, ESP_LEAK_TYPE_CRITICAL, ESP_COMP_LEAK_GENERAL)); } diff --git a/components/hal/test_apps/crypto/pytest_crypto.py b/components/hal/test_apps/crypto/pytest_crypto.py index 59861cf05c7..064fcb8cd29 100644 --- a/components/hal/test_apps/crypto/pytest_crypto.py +++ b/components/hal/test_apps/crypto/pytest_crypto.py @@ -109,12 +109,24 @@ def test_ecdsa_key( raise -@pytest.mark.generic -@pytest.mark.parametrize('config', ['long_aes_operations'], indirect=True) -@idf_parametrize('target', ['supported_targets'], indirect=['target']) def test_crypto_long_aes_operations(dut: Dut) -> None: # if the env variable IDF_FPGA_ENV is set, we would need a longer timeout # as tests for efuses burning security peripherals would be run timeout = 600 if os.environ.get('IDF_ENV_FPGA') else 60 dut.expect('Tests finished', timeout=timeout) + + +@pytest.mark.generic +@pytest.mark.parametrize('config', ['long_aes_operations'], indirect=True) +@idf_parametrize('target', ['supported_targets'], indirect=['target']) +def test_crypto_long_aes_operations_generic(dut: Dut) -> None: + test_crypto_long_aes_operations(dut) + + +@pytest.mark.generic +@pytest.mark.esp32p4_rev1 +@pytest.mark.parametrize('config', ['long_aes_operations_esp32p4_rev1'], indirect=True) +@idf_parametrize('target', ['esp32p4'], indirect=['target']) +def test_crypto_long_aes_operations_esp32p4_rev1(dut: Dut) -> None: + test_crypto_long_aes_operations(dut) diff --git a/components/hal/test_apps/crypto/sdkconfig.ci.long_aes_operations_esp32p4_rev1 b/components/hal/test_apps/crypto/sdkconfig.ci.long_aes_operations_esp32p4_rev1 new file mode 100644 index 00000000000..b957bce25f9 --- /dev/null +++ b/components/hal/test_apps/crypto/sdkconfig.ci.long_aes_operations_esp32p4_rev1 @@ -0,0 +1,8 @@ +# +# Example Configuration +# +CONFIG_IDF_TARGET="esp32p4" +CONFIG_ESP32P4_SELECTS_REV_LESS_V3=y + +CONFIG_CRYPTO_TESTAPP_USE_AES_INTERRUPT=y +# end of Example Configuration