From 9ea01eb7c41b3217dbc5b09cac7de65945287813 Mon Sep 17 00:00:00 2001 From: Sachin Billore Date: Mon, 13 Jan 2025 09:23:47 +0530 Subject: [PATCH] fix(esp_tee): Input validation for secure services --- components/esp_tee/include/esp_tee.h | 1 + .../esp_tee/include/private/esp_tee_binary.h | 7 + components/esp_tee/src/esp_tee_config.c | 5 +- .../esp_tee/subproject/main/CMakeLists.txt | 1 + .../main/core/esp_secure_dispatcher.c | 110 +++++++ .../main/core/esp_secure_services.c | 283 +++++++++++------- 6 files changed, 297 insertions(+), 110 deletions(-) create mode 100644 components/esp_tee/subproject/main/core/esp_secure_dispatcher.c diff --git a/components/esp_tee/include/esp_tee.h b/components/esp_tee/include/esp_tee.h index 3afd53b8bc..c22d887b49 100644 --- a/components/esp_tee/include/esp_tee.h +++ b/components/esp_tee/include/esp_tee.h @@ -52,6 +52,7 @@ typedef struct { void *ns_int_handler; void *ns_iram_end; void *ns_irom_end; + void *ns_drom_start; void *ns_drom_end; } __attribute__((aligned(4))) __attribute__((__packed__)) esp_tee_config_t; diff --git a/components/esp_tee/include/private/esp_tee_binary.h b/components/esp_tee/include/private/esp_tee_binary.h index 0599531a8a..4870db5b95 100644 --- a/components/esp_tee/include/private/esp_tee_binary.h +++ b/components/esp_tee/include/private/esp_tee_binary.h @@ -37,6 +37,9 @@ extern "C" { #define SOC_S_IROM_HIGH (SOC_IROM_LOW + SOC_S_IDROM_SIZE) #define SOC_S_DROM_LOW (SOC_DROM_LOW) #define SOC_S_DROM_HIGH (SOC_DROM_LOW + SOC_S_IDROM_SIZE) +/* REE I/DRAM */ +#define SOC_NS_IDRAM_START (SOC_S_DRAM_END) +#define SOC_NS_IDRAM_END (SOC_DIRAM_DRAM_HIGH) #define SOC_MMU_TOTAL_SIZE (SOC_DRAM0_CACHE_ADDRESS_HIGH - SOC_DRAM0_CACHE_ADDRESS_LOW) #define SOC_MMU_END_VADDR (SOC_DROM_LOW + SOC_MMU_TOTAL_SIZE) @@ -47,6 +50,10 @@ extern "C" { #include #include +/* Secure Service table */ +typedef void (*secure_service_t)(void); +extern const secure_service_t tee_secure_service_table[]; + /** * @brief TEE initialization function called by the bootloader at boot time. * Performs secure system initialization before switching to the REE. diff --git a/components/esp_tee/src/esp_tee_config.c b/components/esp_tee/src/esp_tee_config.c index fce4c7ddf8..eec0210f9e 100644 --- a/components/esp_tee/src/esp_tee_config.c +++ b/components/esp_tee/src/esp_tee_config.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -15,6 +15,8 @@ extern uint32_t _u2m_switch(int argc, va_list ap); extern uint32_t _iram_end; /* REE IROM end */ extern uint32_t _instruction_reserved_end; +/* REE DROM start */ +extern uint32_t _rodata_reserved_start; /* REE DROM end */ extern uint32_t _rodata_reserved_end; @@ -31,5 +33,6 @@ esp_tee_config_t esp_tee_app_config __attribute__((section(".esp_tee_app_cfg"))) .ns_entry_addr = &_u2m_switch, .ns_iram_end = &_iram_end, .ns_irom_end = &_instruction_reserved_end, + .ns_drom_start = &_rodata_reserved_start, .ns_drom_end = &_rodata_reserved_end, }; diff --git a/components/esp_tee/subproject/main/CMakeLists.txt b/components/esp_tee/subproject/main/CMakeLists.txt index ebdf46b7c0..8eb3d1c693 100644 --- a/components/esp_tee/subproject/main/CMakeLists.txt +++ b/components/esp_tee/subproject/main/CMakeLists.txt @@ -15,6 +15,7 @@ set(include) set(srcs "core/esp_tee_init.c" "core/esp_tee_intr.c" "core/esp_secure_services.c" + "core/esp_secure_dispatcher.c" "core/esp_secure_service_table.c") # Arch specific implementation for TEE diff --git a/components/esp_tee/subproject/main/core/esp_secure_dispatcher.c b/components/esp_tee/subproject/main/core/esp_secure_dispatcher.c new file mode 100644 index 0000000000..51df15a5d9 --- /dev/null +++ b/components/esp_tee/subproject/main/core/esp_secure_dispatcher.c @@ -0,0 +1,110 @@ +/* + * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include "esp_log.h" +#include "esp_tee.h" +#include "secure_service_num.h" + +#define ESP_TEE_MAX_INPUT_ARG 10 + +static const char *TAG = "esp_tee_sec_disp"; + +/** + * @brief Entry point to the TEE binary during secure service call. It decipher the call and dispatch it + * to corresponding Secure Service API in secure world. + * TODO: Fix the assembly routine here for compatibility with all levels of compiler optimizations + */ +#pragma GCC push_options +#pragma GCC optimize ("Og") + +int esp_tee_service_dispatcher(int argc, va_list ap) +{ + if (argc > ESP_TEE_MAX_INPUT_ARG) { + ESP_LOGE(TAG, "Input arguments overflow! Received %d, Permitted %d", + argc, ESP_TEE_MAX_INPUT_ARG); + return -1; + } + + int ret = -1; + void *fp_secure_service; + uint32_t argv[ESP_TEE_MAX_INPUT_ARG], *argp; + + uint32_t sid = va_arg(ap, uint32_t); + argc--; + + if (sid >= MAX_SECURE_SERVICES) { + ESP_LOGE(TAG, "Invalid Service ID!"); + va_end(ap); + return -1; + } + + fp_secure_service = (void *)tee_secure_service_table[sid]; + + for (int i = 0; i < argc; i++) { + argv[i] = va_arg(ap, uint32_t); + } + argp = &argv[0]; + va_end(ap); + + asm volatile( + "mv t0, %1 \n" + "beqz t0, service_call \n" + + "lw a0, 0(%3) \n" + "addi t0, t0, -1 \n" + "beqz t0, service_call \n" + + "lw a1, 4(%3) \n" + "addi t0, t0, -1 \n" + "beqz t0, service_call \n" + + "lw a2, 8(%3) \n" + "addi t0, t0, -1 \n" + "beqz t0, service_call \n" + + "lw a3, 12(%3) \n" + "addi t0, t0, -1 \n" + "beqz t0, service_call \n" + + "lw a4, 16(%3) \n" + "addi t0, t0, -1 \n" + "beqz t0, service_call \n" + + "lw a5, 20(%3) \n" + "addi t0, t0, -1 \n" + "beqz t0, service_call \n" + + "lw a6, 24(%3) \n" + "addi t0, t0, -1 \n" + "beqz t0, service_call \n" + + "lw a7, 28(%3) \n" + "addi t0, t0, -1 \n" + "beqz t0, service_call \n" + + "addi %3, %3, 32 \n" + "mv t2, sp \n" + "loop: \n" + "lw t1, 0(%3) \n" + "sw t1, 0(t2) \n" + "addi t0, t0, -1 \n" + "addi t2, t2, 4 \n" + "addi %3, %3, 4 \n" + "bnez t0, loop \n" + + "service_call: \n" + "mv t1, %2 \n" + "jalr 0(t1) \n" + "mv %0, a0 \n" + : "=r"(ret) + : "r"(argc), "r"(fp_secure_service), "r"(argp) + : "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7", "t0", "t1", "t2" + ); + + return ret; +} +#pragma GCC pop_options diff --git a/components/esp_tee/subproject/main/core/esp_secure_services.c b/components/esp_tee/subproject/main/core/esp_secure_services.c index 99a8491049..e90fdb6b30 100644 --- a/components/esp_tee/subproject/main/core/esp_secure_services.c +++ b/components/esp_tee/subproject/main/core/esp_secure_services.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 */ @@ -7,6 +7,7 @@ #include "esp_cpu.h" #include "esp_efuse.h" +#include "esp_fault.h" #include "esp_flash.h" #include "esp_flash_encrypt.h" #include "esp_rom_efuse.h" @@ -23,7 +24,6 @@ #include "sha/sha_dma.h" #include "esp_tee.h" -#include "secure_service_num.h" #include "esp_tee_intr.h" #include "esp_tee_aes_intr.h" #include "esp_tee_rv_utils.h" @@ -33,13 +33,19 @@ #include "esp_tee_ota_ops.h" #include "esp_attestation.h" -#define ESP_TEE_MAX_INPUT_ARG 10 - -static const char *TAG = "esp_tee_sec_srv"; - -typedef void (*secure_service_t)(void); - -extern const secure_service_t tee_secure_service_table[]; +FORCE_INLINE_ATTR bool is_valid_ree_address(const void *ree_addr) +{ + return ((((size_t)ree_addr >= SOC_NS_IDRAM_START) && + ((size_t)ree_addr < SOC_NS_IDRAM_END) + ) || + ((ree_addr >= esp_tee_app_config.ns_drom_start) && + ((size_t)ree_addr < SOC_S_MMU_MMAP_RESV_START_VADDR) + ) || + (((size_t)ree_addr >= SOC_RTC_DATA_LOW) && + ((size_t)ree_addr < SOC_RTC_DATA_HIGH) + ) + ); +} void _ss_invalid_secure_service(void) { @@ -107,6 +113,15 @@ bool _ss_efuse_hal_get_disable_wafer_version_major(void) void _ss_efuse_hal_get_mac(uint8_t *mac) { + bool valid_addr = ((is_valid_ree_address((void *)mac)) & + (is_valid_ree_address((void *)(mac + 6)))); + + if (!valid_addr) { + return; + } + + ESP_FAULT_ASSERT(valid_addr); + efuse_hal_get_mac(mac); } @@ -169,6 +184,15 @@ int _ss_esp_aes_crypt_cbc(esp_aes_context *ctx, const unsigned char *input, unsigned char *output) { + bool valid_addr = ((is_valid_ree_address((void *)input) && is_valid_ree_address((void *)output)) & + (is_valid_ree_address((void *)(input + length)) && is_valid_ree_address((void *)(output + length)))); + + if (!valid_addr) { + return -1; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_aes_crypt_cbc(ctx, mode, length, iv, input, output); } @@ -180,6 +204,15 @@ int _ss_esp_aes_crypt_cfb128(esp_aes_context *ctx, const unsigned char *input, unsigned char *output) { + bool valid_addr = ((is_valid_ree_address((void *)input) && is_valid_ree_address((void *)output)) & + (is_valid_ree_address((void *)(input + length)) && is_valid_ree_address((void *)(output + length)))); + + if (!valid_addr) { + return -1; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_aes_crypt_cfb128(ctx, mode, length, iv_off, iv, input, output); } @@ -190,6 +223,15 @@ int _ss_esp_aes_crypt_cfb8(esp_aes_context *ctx, const unsigned char *input, unsigned char *output) { + bool valid_addr = ((is_valid_ree_address((void *)input) && is_valid_ree_address((void *)output)) & + (is_valid_ree_address((void *)(input + length)) && is_valid_ree_address((void *)(output + length)))); + + if (!valid_addr) { + return -1; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_aes_crypt_cfb8(ctx, mode, length, iv, input, output); } @@ -201,6 +243,15 @@ int _ss_esp_aes_crypt_ctr(esp_aes_context *ctx, const unsigned char *input, unsigned char *output) { + bool valid_addr = ((is_valid_ree_address((void *)input) && is_valid_ree_address((void *)output)) & + (is_valid_ree_address((void *)(input + length)) && is_valid_ree_address((void *)(output + length)))); + + if (!valid_addr) { + return -1; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_aes_crypt_ctr(ctx, length, nc_off, nonce_counter, stream_block, input, output); } @@ -209,6 +260,15 @@ int _ss_esp_aes_crypt_ecb(esp_aes_context *ctx, const unsigned char input[16], unsigned char output[16]) { + bool valid_addr = ((is_valid_ree_address((void *)input) && is_valid_ree_address((void *)output)) & + (is_valid_ree_address((void *)(input + 16)) && is_valid_ree_address((void *)(output + 16)))); + + if (!valid_addr) { + return -1; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_aes_crypt_ecb(ctx, mode, input, output); } @@ -219,6 +279,15 @@ int _ss_esp_aes_crypt_ofb(esp_aes_context *ctx, const unsigned char *input, unsigned char *output) { + bool valid_addr = ((is_valid_ree_address((void *)input) && is_valid_ree_address((void *)output)) & + (is_valid_ree_address((void *)(input + length)) && is_valid_ree_address((void *)(output + length)))); + + if (!valid_addr) { + return -1; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_aes_crypt_ofb(ctx, length, iv_off, iv, input, output); } @@ -226,12 +295,35 @@ int _ss_esp_aes_crypt_ofb(esp_aes_context *ctx, void _ss_esp_sha(esp_sha_type sha_type, const unsigned char *input, size_t ilen, unsigned char *output) { + bool valid_addr = ((is_valid_ree_address((void *)input) && is_valid_ree_address((void *)output)) & + (is_valid_ree_address((void *)(input + ilen)))); + + if (!valid_addr) { + return; + } + + ESP_FAULT_ASSERT(valid_addr); + esp_sha(sha_type, input, ilen, output); } int _ss_esp_sha_dma(esp_sha_type sha_type, const void *input, uint32_t ilen, const void *buf, uint32_t buf_len, bool is_first_block) { + bool valid_addr = (is_valid_ree_address((void *)input) && + is_valid_ree_address((void *)((char *)input + ilen)) + ); + if (buf_len) { + valid_addr &= (is_valid_ree_address((void *)buf) && + is_valid_ree_address((void *)((char *)buf + buf_len)) + ); + } + + if (!valid_addr) { + return -1; + } + ESP_FAULT_ASSERT(valid_addr); + return esp_sha_dma(sha_type, input, ilen, buf, buf_len, is_first_block); } @@ -254,6 +346,15 @@ int _ss_esp_tee_ota_begin(void) int _ss_esp_tee_ota_write(uint32_t rel_offset, void *data, size_t size) { + bool valid_addr = ((is_valid_ree_address((void *)data)) & + (is_valid_ree_address((void *)((char *)data + size)))); + + if (!valid_addr) { + return -1; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_tee_ota_write(rel_offset, data, size); } @@ -276,23 +377,76 @@ esp_err_t _ss_esp_tee_sec_storage_gen_key(uint16_t slot_id, uint8_t key_type) esp_err_t _ss_esp_tee_sec_storage_get_signature(uint16_t slot_id, uint8_t *hash, size_t hlen, esp_tee_sec_storage_sign_t *out_sign) { + bool valid_addr = ((is_valid_ree_address((void *)hash) && is_valid_ree_address((void *)out_sign)) & + (is_valid_ree_address((void *)(hash + hlen)) && + is_valid_ree_address((void *)((char *)out_sign + sizeof(esp_tee_sec_storage_sign_t))))); + + if (!valid_addr) { + return ESP_ERR_INVALID_ARG; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_tee_sec_storage_get_signature(slot_id, hash, hlen, out_sign); } esp_err_t _ss_esp_tee_sec_storage_get_pubkey(uint16_t slot_id, esp_tee_sec_storage_pubkey_t *pubkey) { + bool valid_addr = ((is_valid_ree_address((void *)pubkey)) & + (is_valid_ree_address((void *)((char *)pubkey + sizeof(esp_tee_sec_storage_pubkey_t))))); + + if (!valid_addr) { + return ESP_ERR_INVALID_ARG; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_tee_sec_storage_get_pubkey(slot_id, pubkey); } esp_err_t _ss_esp_tee_sec_storage_encrypt(uint16_t slot_id, uint8_t *input, uint8_t len, uint8_t *aad, uint16_t aad_len, uint8_t *tag, uint16_t tag_len, uint8_t *output) { + bool valid_addr = (is_valid_ree_address((void *)input) && is_valid_ree_address((void *)tag) && + is_valid_ree_address((void *)output)); + + valid_addr &= (is_valid_ree_address((void *)(input + len)) && is_valid_ree_address((void *)(tag + tag_len)) && + is_valid_ree_address((void *)(output + len)) + ); + + if (aad) { + valid_addr &= (is_valid_ree_address((void *)aad) && is_valid_ree_address((void *)(aad + aad_len))); + } + + if (!valid_addr) { + return ESP_ERR_INVALID_ARG; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_tee_sec_storage_encrypt(slot_id, input, len, aad, aad_len, tag, tag_len, output); } esp_err_t _ss_esp_tee_sec_storage_decrypt(uint16_t slot_id, uint8_t *input, uint8_t len, uint8_t *aad, uint16_t aad_len, uint8_t *tag, uint16_t tag_len, uint8_t *output) { + bool valid_addr = (is_valid_ree_address((void *)input) && is_valid_ree_address((void *)tag) && + is_valid_ree_address((void *)output)); + + valid_addr &= (is_valid_ree_address((void *)(input + len)) && is_valid_ree_address((void *)(tag + tag_len)) && + is_valid_ree_address((void *)(output + len)) + ); + + if (aad) { + valid_addr &= (is_valid_ree_address((void *)aad) && is_valid_ree_address((void *)(aad + aad_len))); + } + + if (!valid_addr) { + return ESP_ERR_INVALID_ARG; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_tee_sec_storage_decrypt(slot_id, input, len, aad, aad_len, tag, tag_len, output); } @@ -311,11 +465,21 @@ esp_err_t _ss_esp_tee_sec_storage_clear_slot(uint16_t slot_id) esp_err_t _ss_esp_tee_att_generate_token(const uint32_t nonce, const uint32_t client_id, const char *psa_cert_ref, uint8_t *token_buf, const size_t token_buf_size, uint32_t *token_len) { + bool valid_addr = (is_valid_ree_address((void *)psa_cert_ref) && is_valid_ree_address((void *)token_buf) && + is_valid_ree_address((void *)token_len)); + + valid_addr &= (is_valid_ree_address((void *)((char *)psa_cert_ref + 32)) && is_valid_ree_address((void *)((char *)token_buf + token_buf_size))); + + if (!valid_addr) { + return ESP_ERR_INVALID_ARG; + } + + ESP_FAULT_ASSERT(valid_addr); + return esp_att_generate_token(nonce, client_id, psa_cert_ref, token_buf, token_buf_size, token_len); } /* ---------------------------------------------- MMU HAL ------------------------------------------------- */ - void _ss_mmu_hal_map_region(uint32_t mmu_id, mmu_target_t mem_type, uint32_t vaddr, uint32_t paddr, uint32_t len, uint32_t *out_len) { @@ -359,102 +523,3 @@ bool _ss_mmu_hal_paddr_to_vaddr(uint32_t mmu_id, uint32_t paddr, mmu_target_t ta ESP_FAULT_ASSERT(!paddr_chk); return mmu_hal_paddr_to_vaddr(mmu_id, paddr, target, type, out_vaddr); } - -/* ---------------------------------------------- Secure Service Dispatcher ------------------------------------------------- */ - -/** - * @brief Entry point to the TEE binary during secure service call. It decipher the call and dispatch it - * to corresponding Secure Service API in secure world. - * TODO: Fix the assembly routine here for compatibility with all levels of compiler optimizations - */ -#pragma GCC push_options -#pragma GCC optimize ("Og") - -int esp_tee_service_dispatcher(int argc, va_list ap) -{ - if (argc > ESP_TEE_MAX_INPUT_ARG) { - ESP_LOGE(TAG, "Input arguments overflow! Received %d, Permitted %d", - argc, ESP_TEE_MAX_INPUT_ARG); - return -1; - } - - int ret = -1; - void *fp_secure_service; - uint32_t argv[ESP_TEE_MAX_INPUT_ARG], *argp; - - uint32_t sid = va_arg(ap, uint32_t); - argc--; - - if (sid >= MAX_SECURE_SERVICES) { - ESP_LOGE(TAG, "Invalid Service ID!"); - va_end(ap); - return -1; - } - - fp_secure_service = (void *)tee_secure_service_table[sid]; - - for (int i = 0; i < argc; i++) { - argv[i] = va_arg(ap, uint32_t); - } - argp = &argv[0]; - va_end(ap); - - asm volatile( - "mv t0, %1 \n" - "beqz t0, service_call \n" - - "lw a0, 0(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - - "lw a1, 4(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - - "lw a2, 8(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - - "lw a3, 12(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - - "lw a4, 16(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - - "lw a5, 20(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - - "lw a6, 24(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - - "lw a7, 28(%3) \n" - "addi t0, t0, -1 \n" - "beqz t0, service_call \n" - - "addi %3, %3, 32 \n" - "mv t2, sp \n" - "loop: \n" - "lw t1, 0(%3) \n" - "sw t1, 0(t2) \n" - "addi t0, t0, -1 \n" - "addi t2, t2, 4 \n" - "addi %3, %3, 4 \n" - "bnez t0, loop \n" - - "service_call: \n" - "mv t1, %2 \n" - "jalr 0(t1) \n" - "mv %0, a0 \n" - : "=r"(ret) - : "r"(argc), "r"(fp_secure_service), "r"(argp) - : "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7", "t0", "t1", "t2" - ); - - return ret; -} - -#pragma GCC pop_options