From 09ded7787ffb5ec08311add388905ff5a59d4c80 Mon Sep 17 00:00:00 2001 From: Aditya Patwardhan Date: Tue, 10 Dec 2024 17:57:26 +0530 Subject: [PATCH] fix(hal): Make the ECDSA countermeasure dynamically applicable This commit makes the ECDSA countermeasure dynamically applicable across different revisions of the ESP32H2 SoC. --- components/esp_hw_support/Kconfig | 14 ------------- components/esp_system/port/cpu_start.c | 8 ++++++- components/hal/Kconfig | 2 +- components/hal/ecdsa_hal.c | 7 ++++++- components/hal/test_apps/ecc/CMakeLists.txt | 2 ++ .../hal/test_apps/ecc/main/CMakeLists.txt | 2 +- components/hal/test_apps/ecc/main/test_ecc.c | 21 ++++++++++++------- .../hal/test_apps/ecc/sdkconfig.defaults | 1 + components/mbedtls/port/ecdsa/ecdsa_alt.c | 10 ++++++--- 9 files changed, 38 insertions(+), 29 deletions(-) diff --git a/components/esp_hw_support/Kconfig b/components/esp_hw_support/Kconfig index a82b83f5c0..418fbbe87b 100644 --- a/components/esp_hw_support/Kconfig +++ b/components/esp_hw_support/Kconfig @@ -346,18 +346,4 @@ menu "Hardware Settings" time point multiplication operations by changing the default ESP-IDF configurations. Performing constant time operations protect the ECC multiplication operations from timing attacks. - orsource "./port/$IDF_TARGET/Kconfig.dcdc" - - orsource "./port/$IDF_TARGET/Kconfig.ldo" - - # Invisible bringup bypass options for esp_hw_support component - config ESP_BRINGUP_BYPASS_CPU_CLK_SETTING - bool - default y if !SOC_CLK_TREE_SUPPORTED - default n - help - This option is only used for new chip bringup, when - clock support isn't done yet. So with this option, - we use xtal on FPGA as the clock source. - endmenu diff --git a/components/esp_system/port/cpu_start.c b/components/esp_system/port/cpu_start.c index e3bc566970..1da903d7f7 100644 --- a/components/esp_system/port/cpu_start.c +++ b/components/esp_system/port/cpu_start.c @@ -249,7 +249,13 @@ static void start_other_core(void) #endif #if CONFIG_ESP_CRYPTO_FORCE_ECC_CONSTANT_TIME_POINT_MUL - if (!esp_efuse_read_field_bit(ESP_EFUSE_ECC_FORCE_CONST_TIME)) { + bool force_constant_time = true; +#if CONFIG_IDF_TARGET_ESP32H2 + if (!ESP_CHIP_REV_ABOVE(efuse_hal_chip_revision(), 102)) { + force_constant_time = false; + } +#endif + if (!esp_efuse_read_field_bit(ESP_EFUSE_ECC_FORCE_CONST_TIME) && force_constant_time) { ESP_EARLY_LOGD(TAG, "Forcefully enabling ECC constant time operations"); esp_err_t err = esp_efuse_write_field_bit(ESP_EFUSE_ECC_FORCE_CONST_TIME); if (err != ESP_OK) { diff --git a/components/hal/Kconfig b/components/hal/Kconfig index aac591bd36..dcd91df039 100644 --- a/components/hal/Kconfig +++ b/components/hal/Kconfig @@ -105,7 +105,7 @@ menu "Hardware Abstraction Layer (HAL) and Low Level (LL)" config HAL_ECDSA_GEN_SIG_CM bool "Enable countermeasure for ECDSA signature generation" - depends on IDF_TARGET_ESP32H2 && ESP32H2_REV_MIN_FULL < 102 + depends on IDF_TARGET_ESP32H2 default n help Enable this option to apply the countermeasure for ECDSA signature operation diff --git a/components/hal/ecdsa_hal.c b/components/hal/ecdsa_hal.c index 31c782ba85..0148841161 100644 --- a/components/hal/ecdsa_hal.c +++ b/components/hal/ecdsa_hal.c @@ -12,6 +12,7 @@ #if CONFIG_HAL_ECDSA_GEN_SIG_CM #include "esp_fault.h" #include "esp_random.h" +#include "soc/chip_revision.h" #endif #define ECDSA_HAL_P192_COMPONENT_LEN 24 @@ -108,7 +109,11 @@ void ecdsa_hal_gen_signature(ecdsa_hal_config_t *conf, const uint8_t *hash, configure_ecdsa_periph(conf); #if CONFIG_HAL_ECDSA_GEN_SIG_CM - ecdsa_hal_gen_signature_with_countermeasure(hash, r_out, s_out, len); + if (!ESP_CHIP_REV_ABOVE(efuse_hal_chip_revision(), 102)) { + ecdsa_hal_gen_signature_with_countermeasure(hash, r_out, s_out, len); + } else { + ecdsa_hal_gen_signature_inner(hash, r_out, s_out, len); + } #else /* CONFIG_HAL_ECDSA_GEN_SIG_CM */ ecdsa_hal_gen_signature_inner(hash, r_out, s_out, len); #endif /* !CONFIG_HAL_ECDSA_GEN_SIG_CM */ diff --git a/components/hal/test_apps/ecc/CMakeLists.txt b/components/hal/test_apps/ecc/CMakeLists.txt index 5ca38f39c4..067ea13764 100644 --- a/components/hal/test_apps/ecc/CMakeLists.txt +++ b/components/hal/test_apps/ecc/CMakeLists.txt @@ -2,6 +2,8 @@ cmake_minimum_required(VERSION 3.16) set(COMPONENTS main) +set(EXTRA_COMPONENT_DIRS "$ENV{IDF_PATH}/tools/unit-test-app/components") include($ENV{IDF_PATH}/tools/cmake/project.cmake) + project(ecc_test) diff --git a/components/hal/test_apps/ecc/main/CMakeLists.txt b/components/hal/test_apps/ecc/main/CMakeLists.txt index 668d5be1ed..98c0c07f90 100644 --- a/components/hal/test_apps/ecc/main/CMakeLists.txt +++ b/components/hal/test_apps/ecc/main/CMakeLists.txt @@ -2,5 +2,5 @@ set(srcs "app_main.c" "test_ecc.c") idf_component_register(SRCS ${srcs} - REQUIRES unity + REQUIRES unity test_utils WHOLE_ARCHIVE) diff --git a/components/hal/test_apps/ecc/main/test_ecc.c b/components/hal/test_apps/ecc/main/test_ecc.c index db3d46d7c1..8f191bebdb 100644 --- a/components/hal/test_apps/ecc/main/test_ecc.c +++ b/components/hal/test_apps/ecc/main/test_ecc.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: CC0-1.0 */ @@ -7,6 +7,8 @@ #include #include #include +#include "ccomp_timer.h" +#include "sys/param.h" #include "sdkconfig.h" #include "esp_log.h" #include "test_params.h" @@ -136,7 +138,7 @@ TEST_CASE("ECC point multiplication on SECP192R1 and SECP256R1", "[ecc][hal]") #if SOC_ECC_CONSTANT_TIME_POINT_MUL || (CONFIG_IDF_TARGET_ESP32H2 && CONFIG_ESP32H2_REV_MIN_FULL >= 102) -#define CONST_TIME_DEVIATION_PERCENT 0.002 +#define CONST_TIME_DEVIATION_THRESHOLD 2 // 0.2 % static void test_ecc_point_mul_inner_constant_time(void) { @@ -152,11 +154,14 @@ static void test_ecc_point_mul_inner_constant_time(void) uint8_t x_res_le[32]; uint8_t y_res_le[32]; - double deviation = 0; + uint32_t deviation = 0; uint32_t elapsed_time, mean_elapsed_time, total_elapsed_time = 0; uint32_t max_time = 0, min_time = UINT32_MAX; int loop_count = 10; + // performing the operation once to warm up the cache + ecc_point_mul(scalar_le, x_le, y_le, 32, 0, x_res_le, y_res_le); + for (int i = 0; i < loop_count; i++) { ccomp_timer_start(); ecc_point_mul(scalar_le, x_le, y_le, 32, 0, x_res_le, y_res_le); @@ -167,9 +172,9 @@ static void test_ecc_point_mul_inner_constant_time(void) total_elapsed_time += elapsed_time; } mean_elapsed_time = total_elapsed_time / loop_count; - deviation = ((double)(max_time - mean_elapsed_time) / mean_elapsed_time); + deviation = (((max_time - mean_elapsed_time) * 1000) / mean_elapsed_time); - TEST_ASSERT_LESS_THAN_DOUBLE(CONST_TIME_DEVIATION_PERCENT, deviation); + TEST_ASSERT_LESS_THAN(CONST_TIME_DEVIATION_THRESHOLD, deviation); /* P192 */ ecc_be_to_le(ecc_p192_scalar, scalar_le, 24); @@ -190,12 +195,12 @@ static void test_ecc_point_mul_inner_constant_time(void) total_elapsed_time += elapsed_time; } mean_elapsed_time = total_elapsed_time / loop_count; - deviation = ((double)(max_time - mean_elapsed_time) / mean_elapsed_time); + deviation = (((max_time - mean_elapsed_time) * 1000) / mean_elapsed_time); - TEST_ASSERT_LESS_THAN_DOUBLE(CONST_TIME_DEVIATION_PERCENT, deviation); + TEST_ASSERT_LESS_THAN(CONST_TIME_DEVIATION_THRESHOLD, deviation); } -TEST(ecc, ecc_point_multiplication_const_time_check_on_SECP192R1_and_SECP256R1) +TEST_CASE("ECC point multiplication constant time check on SECP192R1 and SECP256R1", "[ecc][hal]") { test_ecc_point_mul_inner_constant_time(); } diff --git a/components/hal/test_apps/ecc/sdkconfig.defaults b/components/hal/test_apps/ecc/sdkconfig.defaults index a4ba403514..bad2274c40 100644 --- a/components/hal/test_apps/ecc/sdkconfig.defaults +++ b/components/hal/test_apps/ecc/sdkconfig.defaults @@ -1,2 +1,3 @@ CONFIG_ESP_TASK_WDT_EN=y CONFIG_ESP_TASK_WDT_INIT=n +CONFIG_ESP_CRYPTO_DPA_PROTECTION_AT_STARTUP=n diff --git a/components/mbedtls/port/ecdsa/ecdsa_alt.c b/components/mbedtls/port/ecdsa/ecdsa_alt.c index 8f9158c372..c8b396cf00 100644 --- a/components/mbedtls/port/ecdsa/ecdsa_alt.c +++ b/components/mbedtls/port/ecdsa/ecdsa_alt.c @@ -23,6 +23,8 @@ #if CONFIG_MBEDTLS_HARDWARE_ECDSA_SIGN_CONSTANT_TIME_CM #include "esp_timer.h" +#include "soc/chip_revision.h" +#include "hal/efuse_hal.h" #if CONFIG_ESP_CRYPTO_DPA_PROTECTION_LEVEL_HIGH /* @@ -176,9 +178,11 @@ static int esp_ecdsa_sign(mbedtls_ecp_group *grp, mbedtls_mpi* r, mbedtls_mpi* s #endif ecdsa_hal_gen_signature(&conf, sha_le, r_le, s_le, len); #if CONFIG_MBEDTLS_HARDWARE_ECDSA_SIGN_CONSTANT_TIME_CM - sig_time = esp_timer_get_time() - sig_time; - if (sig_time < ECDSA_CM_FIXED_SIG_TIME) { - esp_rom_delay_us(ECDSA_CM_FIXED_SIG_TIME - sig_time); + if (!ESP_CHIP_REV_ABOVE(efuse_hal_chip_revision(), 102)) { + sig_time = esp_timer_get_time() - sig_time; + if (sig_time < ECDSA_CM_FIXED_SIG_TIME) { + esp_rom_delay_us(ECDSA_CM_FIXED_SIG_TIME - sig_time); + } } #endif process_again = !ecdsa_hal_get_operation_result()