From 13aff0b2164ce0859a204dde0adfb749c1bea222 Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Thu, 10 Apr 2025 17:21:00 +0530 Subject: [PATCH] fix(security): Fixed coverity warnings related to the `esp_tee` component - Also, disable the SECP192R1 curve (Mbed TLS config) when TEE Secure Storage does not require it --- .../tee_sec_storage/tee_sec_storage.c | 9 +++---- .../subproject/main/core/esp_tee_init.c | 24 ++++++++++++------- .../mbedtls/esp_tee/esp_tee_mbedtls_config.h | 2 ++ components/mbedtls/port/ecdsa/ecdsa_alt.c | 10 ++++++-- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/components/esp_tee/subproject/components/tee_sec_storage/tee_sec_storage.c b/components/esp_tee/subproject/components/tee_sec_storage/tee_sec_storage.c index ead8eff497..012813e963 100644 --- a/components/esp_tee/subproject/components/tee_sec_storage/tee_sec_storage.c +++ b/components/esp_tee/subproject/components/tee_sec_storage/tee_sec_storage.c @@ -511,12 +511,15 @@ esp_err_t esp_tee_sec_storage_get_signature(uint16_t slot_id, esp_tee_sec_storag mbedtls_ecp_keypair_init(&priv_key); mbedtls_ecdsa_init(&sign_ctx); + size_t key_len = 0; int ret = -1; if (key_type == ESP_SEC_STG_KEY_ECDSA_SECP256R1) { ret = mbedtls_ecp_read_key(MBEDTLS_ECP_DP_SECP256R1, &priv_key, keyctx.ecdsa_secp256r1.priv_key, sizeof(keyctx.ecdsa_secp256r1.priv_key)); + key_len = ECDSA_SECP256R1_KEY_LEN; #if CONFIG_SECURE_TEE_SEC_STG_SUPPORT_SECP192R1_SIGN } else if (key_type == ESP_SEC_STG_KEY_ECDSA_SECP192R1) { ret = mbedtls_ecp_read_key(MBEDTLS_ECP_DP_SECP192R1, &priv_key, keyctx.ecdsa_secp192r1.priv_key, sizeof(keyctx.ecdsa_secp192r1.priv_key)); + key_len = ECDSA_SECP192R1_KEY_LEN; #endif } else { ESP_LOGE(TAG, "Unsupported key type for signature generation"); @@ -547,12 +550,6 @@ esp_err_t esp_tee_sec_storage_get_signature(uint16_t slot_id, esp_tee_sec_storag memset(out_sign, 0x00, sizeof(esp_tee_sec_storage_sign_t)); - size_t key_len = (key_type == ESP_SEC_STG_KEY_ECDSA_SECP256R1) ? ECDSA_SECP256R1_KEY_LEN : -#if CONFIG_SECURE_TEE_SEC_STG_SUPPORT_SECP192R1_SIGN - ECDSA_SECP192R1_KEY_LEN; -#else - 0; -#endif ret = mbedtls_mpi_write_binary(&r, out_sign->sign_r, key_len); if (ret != 0) { err = ESP_FAIL; diff --git a/components/esp_tee/subproject/main/core/esp_tee_init.c b/components/esp_tee/subproject/main/core/esp_tee_init.c index ddd12b4596..01e129bc98 100644 --- a/components/esp_tee/subproject/main/core/esp_tee_init.c +++ b/components/esp_tee/subproject/main/core/esp_tee_init.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -24,15 +24,15 @@ /* TEE symbols */ extern uint32_t _tee_stack; extern uint32_t _tee_intr_stack_bottom; -extern uint32_t _tee_heap_start; -extern uint32_t _tee_heap_end; extern uint32_t _tee_bss_start; extern uint32_t _tee_bss_end; extern uint32_t _sec_world_entry; extern uint32_t _tee_s_intr_handler; -#define TEE_HEAP_SIZE (((uint32_t)&_tee_heap_end - (uint32_t)&_tee_heap_start)) +extern uint8_t _tee_heap_start[]; +extern uint8_t _tee_heap_end[]; +#define TEE_HEAP_SIZE ((size_t)(_tee_heap_end - _tee_heap_start)) static const char *TAG = "esp_tee_init"; @@ -131,8 +131,12 @@ void __attribute__((noreturn)) esp_tee_init(uint32_t ree_entry_addr, uint32_t re /* TEE compatibility check and App config data initialization. */ tee_init_app_config(); - /* TEE Secure World heap initialization. */ - assert(esp_tee_heap_init(((void *)&_tee_heap_start), TEE_HEAP_SIZE) == ESP_OK); + /* TEE heap initialization. */ + esp_err_t err = esp_tee_heap_init((void *)_tee_heap_start, TEE_HEAP_SIZE); + if (err != ESP_OK) { + ESP_LOGE(TAG, "Failed to setup the TEE heap!"); + abort(); + } /* SoC specific secure initialization. */ esp_tee_soc_secure_sys_init(); @@ -148,7 +152,7 @@ void __attribute__((noreturn)) esp_tee_init(uint32_t ree_entry_addr, uint32_t re ((void *)&_tee_heap_start), TEE_HEAP_SIZE, TEE_HEAP_SIZE / 1024, "RAM"); /* Setting up the permissible flash operation address range */ - esp_err_t err = esp_tee_flash_setup_prot_ctx(tee_boot_part); + err = esp_tee_flash_setup_prot_ctx(tee_boot_part); if (err != ESP_OK) { ESP_LOGE(TAG, "Failed to setup the TEE flash memory protection!"); abort(); @@ -156,7 +160,11 @@ void __attribute__((noreturn)) esp_tee_init(uint32_t ree_entry_addr, uint32_t re ESP_FAULT_ASSERT(err == ESP_OK); /* Setting up the running non-secure app partition as per the address provided by the bootloader */ - assert(esp_tee_flash_set_running_ree_partition(ree_drom_addr) == ESP_OK); + err = esp_tee_flash_set_running_ree_partition(ree_drom_addr); + if (err != ESP_OK) { + ESP_LOGE(TAG, "Failed to setup the active REE partition!"); + abort(); + } tee_print_app_info(); diff --git a/components/mbedtls/esp_tee/esp_tee_mbedtls_config.h b/components/mbedtls/esp_tee/esp_tee_mbedtls_config.h index 3914ab29be..738d3b6586 100644 --- a/components/mbedtls/esp_tee/esp_tee_mbedtls_config.h +++ b/components/mbedtls/esp_tee/esp_tee_mbedtls_config.h @@ -42,7 +42,9 @@ #define MBEDTLS_ASN1_PARSE_C #define MBEDTLS_BIGNUM_C #define MBEDTLS_ECP_DP_SECP256R1_ENABLED +#if CONFIG_SECURE_TEE_SEC_STG_SUPPORT_SECP192R1_SIGN #define MBEDTLS_ECP_DP_SECP192R1_ENABLED +#endif #define MBEDTLS_ECP_C #define MBEDTLS_ECDSA_C diff --git a/components/mbedtls/port/ecdsa/ecdsa_alt.c b/components/mbedtls/port/ecdsa/ecdsa_alt.c index f3a7721fa7..5fca558a6f 100644 --- a/components/mbedtls/port/ecdsa/ecdsa_alt.c +++ b/components/mbedtls/port/ecdsa/ecdsa_alt.c @@ -258,7 +258,9 @@ int esp_ecdsa_privkey_load_pk_context(mbedtls_pk_context *key_ctx, int efuse_blk mbedtls_pk_init(key_ctx); pk_info = mbedtls_pk_info_from_type(MBEDTLS_PK_ECDSA); - mbedtls_pk_setup(key_ctx, pk_info); + if (mbedtls_pk_setup(key_ctx, pk_info) != 0) { + return -1; + } keypair = mbedtls_pk_ec(*key_ctx); return esp_ecdsa_privkey_load_mpi(&(keypair->MBEDTLS_PRIVATE(d)), efuse_blk); @@ -466,7 +468,11 @@ int esp_ecdsa_tee_set_pk_context(mbedtls_pk_context *key_ctx, esp_ecdsa_pk_conf_ mbedtls_pk_init(key_ctx); pk_info = mbedtls_pk_info_from_type(MBEDTLS_PK_ECDSA); - mbedtls_pk_setup(key_ctx, pk_info); + ret = mbedtls_pk_setup(key_ctx, pk_info); + if (ret != 0) { + ESP_LOGE(TAG, "Failed to setup pk context, mbedtls_pk_setup() returned %d", ret); + return ret; + } keypair = mbedtls_pk_ec(*key_ctx); mbedtls_mpi_init(&(keypair->MBEDTLS_PRIVATE(d)));