From e5c3063deee1cefdbf630b3662da1d3a344c2732 Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Wed, 5 Aug 2026 19:46:53 +0530 Subject: [PATCH] fix(esp_tee): Snapshot input arguments in TEE memory before secure service execution - Also fix the `tee_cli_app` build failure due to TEE heap size overflow --- components/esp_security/src/esp_ds.c | 42 +- .../main/core/esp_secure_services.c | 80 ++-- .../main/core/esp_secure_services_iram.c | 388 ++++++++++-------- .../main/include/esp_tee_memory_utils.h | 15 + .../tee_cli_app/sdkconfig.ci.minimal_tee | 4 +- .../tee_cli_app/sdkconfig.ci.release | 4 - components/mbedtls/port/ecc/esp_ecc.c | 11 +- 7 files changed, 319 insertions(+), 225 deletions(-) diff --git a/components/esp_security/src/esp_ds.c b/components/esp_security/src/esp_ds.c index 9a7d8fd42ec..3d9dd795cb1 100644 --- a/components/esp_security/src/esp_ds.c +++ b/components/esp_security/src/esp_ds.c @@ -334,11 +334,12 @@ esp_err_t esp_ds_start_sign(const void *message, return ESP_ERR_INVALID_ARG; } - if (!(data->rsa_length == ESP_DS_RSA_1024 - || data->rsa_length == ESP_DS_RSA_2048 - || data->rsa_length == ESP_DS_RSA_3072 + const uint32_t rsa_length = data->rsa_length; + if (!(rsa_length == ESP_DS_RSA_1024 + || rsa_length == ESP_DS_RSA_2048 + || rsa_length == ESP_DS_RSA_3072 #if SOC_RSA_MAX_BIT_LEN == 4096 - || data->rsa_length == ESP_DS_RSA_4096 + || rsa_length == ESP_DS_RSA_4096 #endif )) { return ESP_ERR_INVALID_ARG; @@ -393,7 +394,7 @@ esp_err_t esp_ds_start_sign(const void *message, return ESP_ERR_NO_MEM; } - size_t rsa_len = (data->rsa_length + 1) * 4; + size_t rsa_len = (rsa_length + 1) * 4; ds_hal_write_private_key_params(data->c); ds_hal_configure_iv((uint32_t *)data->iv); ds_hal_write_message(message, rsa_len); @@ -426,22 +427,31 @@ esp_err_t esp_ds_finish_sign(void *signature, esp_ds_context_t *esp_ds_ctx) } const esp_ds_data_t *data = (const esp_ds_data_t *)esp_ds_ctx->data; - unsigned rsa_len = (data->rsa_length + 1) * 4; + esp_err_t return_value = ESP_ERR_INVALID_ARG; while (ds_hal_busy()) { } - ds_signature_check_t sig_check_result = ds_hal_read_result((uint8_t *) signature, (size_t) rsa_len); + uint32_t rsa_length = data->rsa_length; + if (rsa_length == ESP_DS_RSA_1024 + || rsa_length == ESP_DS_RSA_2048 + || rsa_length == ESP_DS_RSA_3072 +#if SOC_DS_SIGNATURE_MAX_BIT_LEN == 4096 + || rsa_length == ESP_DS_RSA_4096 +#endif + ) { + unsigned rsa_len = (rsa_length + 1) * 4; - esp_err_t return_value = ESP_OK; + ds_signature_check_t res = ds_hal_read_result((uint8_t *) signature, (size_t) rsa_len); - if (sig_check_result == DS_SIGNATURE_MD_FAIL || sig_check_result == DS_SIGNATURE_PADDING_AND_MD_FAIL) { - esp_ds_zeroize(signature, rsa_len); - return_value = ESP_ERR_HW_CRYPTO_DS_INVALID_DIGEST; - } - - if (sig_check_result == DS_SIGNATURE_PADDING_FAIL) { - esp_ds_zeroize(signature, rsa_len); - return_value = ESP_ERR_HW_CRYPTO_DS_INVALID_PADDING; + if (res == DS_SIGNATURE_MD_FAIL || res == DS_SIGNATURE_PADDING_AND_MD_FAIL) { + esp_ds_zeroize(signature, rsa_len); + return_value = ESP_ERR_HW_CRYPTO_DS_INVALID_DIGEST; + } else if (res == DS_SIGNATURE_PADDING_FAIL) { + esp_ds_zeroize(signature, rsa_len); + return_value = ESP_ERR_HW_CRYPTO_DS_INVALID_PADDING; + } else if (res == DS_SIGNATURE_OK) { + return_value = ESP_OK; + } } #if !ESP_TEE_BUILD 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 2a335938407..38acc0e49b0 100644 --- a/components/esp_tee/subproject/main/core/esp_secure_services.c +++ b/components/esp_tee/subproject/main/core/esp_secure_services.c @@ -71,7 +71,8 @@ int _ss_esp_aes_crypt_cbc(esp_aes_context *ctx, } ESP_FAULT_ASSERT(valid_addr); - return esp_aes_crypt_cbc(ctx, mode, length, iv, input, output); + esp_aes_context ctx_local = *ctx; + return esp_aes_crypt_cbc(&ctx_local, mode, length, iv, input, output); } int _ss_esp_aes_crypt_cfb128(esp_aes_context *ctx, @@ -93,7 +94,8 @@ int _ss_esp_aes_crypt_cfb128(esp_aes_context *ctx, } ESP_FAULT_ASSERT(valid_addr); - return esp_aes_crypt_cfb128(ctx, mode, length, iv_off, iv, input, output); + esp_aes_context ctx_local = *ctx; + return esp_aes_crypt_cfb128(&ctx_local, mode, length, iv_off, iv, input, output); } int _ss_esp_aes_crypt_cfb8(esp_aes_context *ctx, @@ -113,7 +115,8 @@ int _ss_esp_aes_crypt_cfb8(esp_aes_context *ctx, } ESP_FAULT_ASSERT(valid_addr); - return esp_aes_crypt_cfb8(ctx, mode, length, iv, input, output); + esp_aes_context ctx_local = *ctx; + return esp_aes_crypt_cfb8(&ctx_local, mode, length, iv, input, output); } int _ss_esp_aes_crypt_ctr(esp_aes_context *ctx, @@ -136,7 +139,8 @@ int _ss_esp_aes_crypt_ctr(esp_aes_context *ctx, } ESP_FAULT_ASSERT(valid_addr); - return esp_aes_crypt_ctr(ctx, length, nc_off, nonce_counter, stream_block, input, output); + esp_aes_context ctx_local = *ctx; + return esp_aes_crypt_ctr(&ctx_local, length, nc_off, nonce_counter, stream_block, input, output); } int _ss_esp_aes_crypt_ecb(esp_aes_context *ctx, @@ -153,7 +157,8 @@ int _ss_esp_aes_crypt_ecb(esp_aes_context *ctx, } ESP_FAULT_ASSERT(valid_addr); - return esp_aes_crypt_ecb(ctx, mode, input, output); + esp_aes_context ctx_local = *ctx; + return esp_aes_crypt_ecb(&ctx_local, mode, input, output); } int _ss_esp_aes_crypt_ofb(esp_aes_context *ctx, @@ -174,7 +179,8 @@ int _ss_esp_aes_crypt_ofb(esp_aes_context *ctx, } ESP_FAULT_ASSERT(valid_addr); - return esp_aes_crypt_ofb(ctx, length, iv_off, iv, input, output); + esp_aes_context ctx_local = *ctx; + return esp_aes_crypt_ofb(&ctx_local, length, iv_off, iv, input, output); } #endif @@ -401,8 +407,10 @@ esp_err_t _ss_esp_ds_sign(const void *message, return ESP_ERR_INVALID_ARG; } - size_t n = get_ds_msg_sign_len(data->rsa_length); - valid_addr &= (n > 0) && esp_tee_buf_in_ree(message, n) && esp_tee_buf_in_ree(signature, n); + const size_t n_max = SOC_DS_SIGNATURE_MAX_BIT_LEN / 8; + valid_addr &= (get_ds_msg_sign_len(data->rsa_length) > 0) && + esp_tee_buf_in_ree(message, n_max) && + esp_tee_buf_in_ree(signature, n_max); #if CONFIG_SECURE_TEE_SEC_STG_MODE_RELEASE valid_addr &= (key_id != (hmac_key_id_t)CONFIG_SECURE_TEE_SEC_STG_EFUSE_HMAC_KEY_ID); @@ -422,15 +430,21 @@ esp_err_t _ss_esp_ds_start_sign(const void *message, hmac_key_id_t key_id, esp_ds_context_t **esp_ds_ctx) { - bool valid_addr = (esp_tee_buf_in_ree(esp_ds_ctx, sizeof(esp_ds_context_t *)) && - esp_tee_buf_in_ree(*esp_ds_ctx, sizeof(esp_ds_context_t)) && + if (!esp_tee_buf_in_ree(esp_ds_ctx, sizeof(esp_ds_context_t *))) { + return ESP_ERR_INVALID_ARG; + } + + esp_ds_context_t *ds_ctx = *esp_ds_ctx; + const size_t n_max = SOC_DS_SIGNATURE_MAX_BIT_LEN / 8; + + bool valid_addr = (esp_tee_buf_in_ree(ds_ctx, sizeof(esp_ds_context_t)) && esp_tee_buf_in_ree(data, sizeof(esp_ds_data_t))); if (!valid_addr) { return ESP_ERR_INVALID_ARG; } - size_t n = get_ds_msg_sign_len(data->rsa_length); - valid_addr &= (n > 0) && esp_tee_buf_in_ree(message, n); + valid_addr &= (get_ds_msg_sign_len(data->rsa_length) > 0) && + esp_tee_buf_in_ree(message, n_max); #if CONFIG_SECURE_TEE_SEC_STG_MODE_RELEASE valid_addr &= (key_id != (hmac_key_id_t)CONFIG_SECURE_TEE_SEC_STG_EFUSE_HMAC_KEY_ID); @@ -442,7 +456,12 @@ esp_err_t _ss_esp_ds_start_sign(const void *message, } ESP_FAULT_ASSERT(valid_addr); - return esp_ds_start_sign(message, data, key_id, esp_ds_ctx); + esp_err_t err = esp_ds_start_sign(message, data, key_id, &ds_ctx); + if (err == ESP_OK) { + *esp_ds_ctx = ds_ctx; + } + + return err; } bool _ss_esp_ds_is_busy(void) @@ -452,14 +471,16 @@ bool _ss_esp_ds_is_busy(void) esp_err_t _ss_esp_ds_finish_sign(void *signature, esp_ds_context_t *esp_ds_ctx) { - const size_t max_sign = SOC_DS_SIGNATURE_MAX_BIT_LEN / 8; - bool valid_addr = (esp_tee_buf_in_ree(signature, max_sign) && + const size_t n_max = SOC_DS_SIGNATURE_MAX_BIT_LEN / 8; + bool valid_addr = (esp_tee_buf_in_ree(signature, n_max) && esp_tee_buf_in_ree(esp_ds_ctx, sizeof(esp_ds_context_t))); if (!valid_addr) { return ESP_ERR_INVALID_ARG; } - const esp_ds_data_t *data = (const esp_ds_data_t *)esp_ds_ctx->data; + const esp_ds_context_t ctx_local = *esp_ds_ctx; + + const esp_ds_data_t *data = (const esp_ds_data_t *)ctx_local.data; valid_addr &= esp_tee_buf_in_ree(data, sizeof(esp_ds_data_t)) && (get_ds_msg_sign_len(data->rsa_length) > 0); @@ -468,7 +489,7 @@ esp_err_t _ss_esp_ds_finish_sign(void *signature, esp_ds_context_t *esp_ds_ctx) } ESP_FAULT_ASSERT(valid_addr); - return esp_ds_finish_sign(signature, esp_ds_ctx); + return esp_ds_finish_sign(signature, (esp_ds_context_t *)&ctx_local); } esp_err_t _ss_esp_ds_encrypt_params(esp_ds_data_t *data, @@ -583,15 +604,11 @@ int _ss_esp_tee_ota_end(void) /* ---------------------------------------------- Secure Storage ------------------------------------------------- */ -/* NOTE: The key-name pointers here (cfg->id/ctx->key_id) are REE-supplied, NULL-terminated - * NVS key names used read-only for key lookup (NVS compares them with strncmp bounded to - * NVS_KEY_NAME_MAX_SIZE-1) — never written through, never used as a register base. - * Pointing one at TEE memory yields at most a load-fault DoS or a useless presence oracle, - * so they are left unchecked. Argument checks cost code size and add latency to every - * service call, so we keep only the ones that close a real REE->TEE read/write/control-flow gap. - */ esp_err_t _ss_esp_tee_sec_storage_clear_key(const char *key_id) { + char id_buf[NVS_KEY_NAME_MAX_SIZE]; + tee_snapshot_ree_str(&key_id, id_buf, sizeof(id_buf)); + bool valid_arg = !esp_tee_sec_storage_is_key_tee_owned(key_id); if (!valid_arg) { return ESP_ERR_INVALID_ARG; @@ -603,15 +620,22 @@ esp_err_t _ss_esp_tee_sec_storage_clear_key(const char *key_id) esp_err_t _ss_esp_tee_sec_storage_gen_key(const esp_tee_sec_storage_key_cfg_t *cfg) { - bool valid_arg = esp_tee_buf_in_ree(cfg, sizeof(esp_tee_sec_storage_key_cfg_t)) && - !(cfg->flags & SEC_STORAGE_FLAG_TEE_ONLY) && - !esp_tee_sec_storage_is_key_tee_owned(cfg->id); + if (!esp_tee_buf_in_ree(cfg, sizeof(esp_tee_sec_storage_key_cfg_t))) { + return ESP_ERR_INVALID_ARG; + } + + esp_tee_sec_storage_key_cfg_t cfg_local = *cfg; + char id_buf[NVS_KEY_NAME_MAX_SIZE]; + tee_snapshot_ree_str(&cfg_local.id, id_buf, sizeof(id_buf)); + + bool valid_arg = !(cfg_local.flags & SEC_STORAGE_FLAG_TEE_ONLY) && + !esp_tee_sec_storage_is_key_tee_owned(cfg_local.id); if (!valid_arg) { return ESP_ERR_INVALID_ARG; } ESP_FAULT_ASSERT(valid_arg); - return esp_tee_sec_storage_gen_key(cfg); + return esp_tee_sec_storage_gen_key(&cfg_local); } /* ---------------------------------------------- PSA Attestation ------------------------------------------------- */ diff --git a/components/esp_tee/subproject/main/core/esp_secure_services_iram.c b/components/esp_tee/subproject/main/core/esp_secure_services_iram.c index cf7d56e98b6..4a0917712d4 100644 --- a/components/esp_tee/subproject/main/core/esp_secure_services_iram.c +++ b/components/esp_tee/subproject/main/core/esp_secure_services_iram.c @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ #include +#include #include #include "esp_err.h" @@ -167,70 +168,90 @@ void _ss_wdt_hal_init(wdt_hal_context_t *hal, wdt_inst_t wdt_inst, uint32_t pres } ESP_FAULT_ASSERT(valid_addr); - wdt_hal_init(hal, wdt_inst, prescaler, enable_intr); + wdt_hal_context_t hal_local; + wdt_hal_init(&hal_local, wdt_inst, prescaler, enable_intr); + + *hal = hal_local; } void _ss_wdt_hal_deinit(wdt_hal_context_t *hal) { - bool valid_addr = (esp_tee_buf_in_ree(hal, sizeof(wdt_hal_context_t)) && - is_wdt_dev_valid(hal->mwdt_dev)); + if (!esp_tee_buf_in_ree(hal, sizeof(wdt_hal_context_t))) { + return; + } + wdt_hal_context_t hal_snap = *hal; + + bool valid_addr = is_wdt_dev_valid(hal_snap.mwdt_dev); if (!valid_addr) { return; } ESP_FAULT_ASSERT(valid_addr); - wdt_hal_deinit(hal); + wdt_hal_deinit(&hal_snap); } /* ---------------------------------------------- Secure Storage ------------------------------------------------- */ -/* NOTE: The key-name pointers here (cfg->id/ctx->key_id) are REE-supplied, NULL-terminated - * NVS key names used read-only for key lookup (NVS compares them with strncmp bounded to - * NVS_KEY_NAME_MAX_SIZE-1) — never written through, never used as a register base. - * Pointing one at TEE memory yields at most a load-fault DoS or a useless presence oracle, - * so they are left unchecked. Argument checks cost code size and add latency to every - * service call, so we keep only the ones that close a real REE->TEE read/write/control-flow gap. - * The buffers alongside these ARE validated, since the TEE reads/writes them. - */ esp_err_t _ss_esp_tee_sec_storage_ecdsa_sign(const esp_tee_sec_storage_key_cfg_t *cfg, const uint8_t *hash, size_t hlen, esp_tee_sec_storage_ecdsa_sign_t *out_sign) { - bool valid_arg = (esp_tee_buf_in_ree(cfg, sizeof(esp_tee_sec_storage_key_cfg_t)) && - esp_tee_buf_in_ree(hash, hlen) && + if (!esp_tee_buf_in_ree(cfg, sizeof(esp_tee_sec_storage_key_cfg_t))) { + return ESP_ERR_INVALID_ARG; + } + + esp_tee_sec_storage_key_cfg_t cfg_local = *cfg; + char id_buf[NVS_KEY_NAME_MAX_SIZE]; + tee_snapshot_ree_str(&cfg_local.id, id_buf, sizeof(id_buf)); + + bool valid_arg = (esp_tee_buf_in_ree(hash, hlen) && esp_tee_buf_in_ree(out_sign, sizeof(esp_tee_sec_storage_ecdsa_sign_t)) && - !esp_tee_sec_storage_is_key_tee_owned(cfg->id)); + !esp_tee_sec_storage_is_key_tee_owned(cfg_local.id)); if (!valid_arg) { return ESP_ERR_INVALID_ARG; } ESP_FAULT_ASSERT(valid_arg); - return esp_tee_sec_storage_ecdsa_sign(cfg, hash, hlen, out_sign); + return esp_tee_sec_storage_ecdsa_sign(&cfg_local, hash, hlen, out_sign); } esp_err_t _ss_esp_tee_sec_storage_ecdsa_get_pubkey(const esp_tee_sec_storage_key_cfg_t *cfg, esp_tee_sec_storage_ecdsa_pubkey_t *out_pubkey) { - bool valid_arg = (esp_tee_buf_in_ree(cfg, sizeof(esp_tee_sec_storage_key_cfg_t)) && - esp_tee_buf_in_ree(out_pubkey, sizeof(esp_tee_sec_storage_ecdsa_pubkey_t)) && - !esp_tee_sec_storage_is_key_tee_owned(cfg->id)); + if (!esp_tee_buf_in_ree(cfg, sizeof(esp_tee_sec_storage_key_cfg_t))) { + return ESP_ERR_INVALID_ARG; + } + + esp_tee_sec_storage_key_cfg_t cfg_local = *cfg; + char id_buf[NVS_KEY_NAME_MAX_SIZE]; + tee_snapshot_ree_str(&cfg_local.id, id_buf, sizeof(id_buf)); + + bool valid_arg = (esp_tee_buf_in_ree(out_pubkey, sizeof(esp_tee_sec_storage_ecdsa_pubkey_t)) && + !esp_tee_sec_storage_is_key_tee_owned(cfg_local.id)); if (!valid_arg) { return ESP_ERR_INVALID_ARG; } ESP_FAULT_ASSERT(valid_arg); - return esp_tee_sec_storage_ecdsa_get_pubkey(cfg, out_pubkey); + return esp_tee_sec_storage_ecdsa_get_pubkey(&cfg_local, out_pubkey); } esp_err_t _ss_esp_tee_sec_storage_aead_encrypt(const esp_tee_sec_storage_aead_ctx_t *ctx, uint8_t *iv, size_t iv_len, uint8_t *tag, size_t tag_len, uint8_t *output) { - bool valid_arg = (esp_tee_buf_in_ree(ctx, sizeof(esp_tee_sec_storage_aead_ctx_t)) && - esp_tee_buf_in_ree(ctx->input, ctx->input_len) && + if (!esp_tee_buf_in_ree(ctx, sizeof(esp_tee_sec_storage_aead_ctx_t))) { + return ESP_ERR_INVALID_ARG; + } + + esp_tee_sec_storage_aead_ctx_t ctx_local = *ctx; + char id_buf[NVS_KEY_NAME_MAX_SIZE]; + tee_snapshot_ree_str(&ctx_local.key_id, id_buf, sizeof(id_buf)); + + bool valid_arg = (esp_tee_buf_in_ree(ctx_local.input, ctx_local.input_len) && esp_tee_buf_in_ree(iv, iv_len) && esp_tee_buf_in_ree(tag, tag_len) && - esp_tee_buf_in_ree(output, ctx->input_len) && - !esp_tee_sec_storage_is_key_tee_owned(ctx->key_id)); + esp_tee_buf_in_ree(output, ctx_local.input_len) && + !esp_tee_sec_storage_is_key_tee_owned(ctx_local.key_id)); - if (ctx->aad_len != 0) { - valid_arg &= esp_tee_buf_in_ree(ctx->aad, ctx->aad_len); + if (ctx_local.aad_len != 0) { + valid_arg &= esp_tee_buf_in_ree(ctx_local.aad, ctx_local.aad_len); } if (!valid_arg) { @@ -238,20 +259,27 @@ esp_err_t _ss_esp_tee_sec_storage_aead_encrypt(const esp_tee_sec_storage_aead_ct } ESP_FAULT_ASSERT(valid_arg); - return esp_tee_sec_storage_aead_encrypt(ctx, iv, iv_len, tag, tag_len, output); + return esp_tee_sec_storage_aead_encrypt(&ctx_local, iv, iv_len, tag, tag_len, output); } esp_err_t _ss_esp_tee_sec_storage_aead_decrypt(const esp_tee_sec_storage_aead_ctx_t *ctx, const uint8_t *iv, size_t iv_len, const uint8_t *tag, size_t tag_len, uint8_t *output) { - bool valid_arg = (esp_tee_buf_in_ree(ctx, sizeof(esp_tee_sec_storage_aead_ctx_t)) && - esp_tee_buf_in_ree(ctx->input, ctx->input_len) && + if (!esp_tee_buf_in_ree(ctx, sizeof(esp_tee_sec_storage_aead_ctx_t))) { + return ESP_ERR_INVALID_ARG; + } + + esp_tee_sec_storage_aead_ctx_t ctx_local = *ctx; + char id_buf[NVS_KEY_NAME_MAX_SIZE]; + tee_snapshot_ree_str(&ctx_local.key_id, id_buf, sizeof(id_buf)); + + bool valid_arg = (esp_tee_buf_in_ree(ctx_local.input, ctx_local.input_len) && esp_tee_buf_in_ree(iv, iv_len) && esp_tee_buf_in_ree(tag, tag_len) && - esp_tee_buf_in_ree(output, ctx->input_len) && - !esp_tee_sec_storage_is_key_tee_owned(ctx->key_id)); + esp_tee_buf_in_ree(output, ctx_local.input_len) && + !esp_tee_sec_storage_is_key_tee_owned(ctx_local.key_id)); - if (ctx->aad_len != 0) { - valid_arg &= esp_tee_buf_in_ree(ctx->aad, ctx->aad_len); + if (ctx_local.aad_len != 0) { + valid_arg &= esp_tee_buf_in_ree(ctx_local.aad, ctx_local.aad_len); } if (!valid_arg) { @@ -259,23 +287,28 @@ esp_err_t _ss_esp_tee_sec_storage_aead_decrypt(const esp_tee_sec_storage_aead_ct } ESP_FAULT_ASSERT(valid_arg); - return esp_tee_sec_storage_aead_decrypt(ctx, iv, iv_len, tag, tag_len, output); + return esp_tee_sec_storage_aead_decrypt(&ctx_local, iv, iv_len, tag, tag_len, output); } esp_err_t _ss_esp_tee_sec_storage_ecdsa_sign_pbkdf2(const esp_tee_sec_storage_pbkdf2_ctx_t *ctx, const uint8_t *hash, size_t hlen, esp_tee_sec_storage_ecdsa_sign_t *out_sign, esp_tee_sec_storage_ecdsa_pubkey_t *out_pubkey) { - bool valid_addr = (esp_tee_buf_in_ree(ctx, sizeof(esp_tee_sec_storage_pbkdf2_ctx_t)) && - esp_tee_buf_in_ree(hash, hlen) && + if (!esp_tee_buf_in_ree(ctx, sizeof(esp_tee_sec_storage_pbkdf2_ctx_t))) { + return ESP_ERR_INVALID_ARG; + } + + const esp_tee_sec_storage_pbkdf2_ctx_t ctx_local = *ctx; + + bool valid_addr = (esp_tee_buf_in_ree(hash, hlen) && esp_tee_buf_in_ree(out_sign, sizeof(esp_tee_sec_storage_ecdsa_sign_t)) && esp_tee_buf_in_ree(out_pubkey, sizeof(esp_tee_sec_storage_ecdsa_pubkey_t)) && - esp_tee_buf_in_ree(ctx->salt, ctx->salt_len)); + esp_tee_buf_in_ree(ctx_local.salt, ctx_local.salt_len)); if (!valid_addr) { return ESP_ERR_INVALID_ARG; } ESP_FAULT_ASSERT(valid_addr); - return esp_tee_sec_storage_ecdsa_sign_pbkdf2(ctx, hash, hlen, out_sign, out_pubkey); + return esp_tee_sec_storage_ecdsa_sign_pbkdf2(&ctx_local, hash, hlen, out_sign, out_pubkey); } /* ---------------------------------------------- MMU HAL ------------------------------------------------- */ @@ -382,30 +415,6 @@ static bool is_flash_addr_readable(uint32_t paddr, uint32_t len) return !esp_tee_flash_check_prange_in_tee_region(paddr, len); } -static bool is_spi_host_in_ree(spi_flash_host_inst_t *host) -{ - const spi_flash_hal_context_t *ctx = (const spi_flash_hal_context_t *)host; - - return (esp_tee_buf_in_ree(host, sizeof(spi_flash_hal_context_t)) && - ctx->spi == spi_flash_ll_get_hw(SPI1_HOST)); -} - -static bool is_spi_trans_valid(spi_flash_host_inst_t *host, spi_flash_trans_t *trans) -{ - if (!is_spi_host_in_ree(host) || !esp_tee_buf_in_ree(trans, sizeof(spi_flash_trans_t))) { - return false; - } - - bool valid_addr = true; - if (trans->mosi_len != 0) { - valid_addr &= esp_tee_buf_in_ree(trans->mosi_data, trans->mosi_len); - } - if (trans->miso_len != 0) { - valid_addr &= esp_tee_buf_in_ree(trans->miso_data, trans->miso_len); - } - return valid_addr; -} - static bool is_spi_cmd_addr_ok(uint32_t addr_bitlen, uint32_t address, uint32_t mosi_len, uint32_t miso_len) { if (addr_bitlen == 0) { @@ -430,206 +439,246 @@ static const spi_flash_host_driver_t tee_host_driver = { .configure_host_io_mode = spi_flash_hal_configure_host_io_mode, }; -static inline const spi_flash_host_driver_t *tee_substitute_host_driver(spi_flash_host_inst_t *host) +static spi_flash_host_inst_t *tee_own_host(const spi_flash_host_inst_t *host, spi_flash_hal_context_t *snap) { - const spi_flash_host_driver_t *orig = host->driver; - host->driver = &tee_host_driver; - return orig; + if (!esp_tee_buf_in_ree(host, sizeof(spi_flash_hal_context_t))) { + return NULL; + } + + *snap = *(const spi_flash_hal_context_t *)host; + + /* Reject a host aimed at another peripheral rather than silently retargeting it */ + if (snap->spi != spi_flash_ll_get_hw(SPI1_HOST)) { + return NULL; + } + + snap->inst.driver = &tee_host_driver; + snap->spi = spi_flash_ll_get_hw(SPI1_HOST); + + return &snap->inst; } uint32_t _ss_spi_flash_hal_check_status(spi_flash_host_inst_t *host) { - bool valid_addr = is_spi_host_in_ree(host); - - if (!valid_addr) { + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { return 0; } - ESP_FAULT_ASSERT(valid_addr); + ESP_FAULT_ASSERT(tee_host != NULL); - return spi_flash_hal_check_status(host); + return spi_flash_hal_check_status(tee_host); } esp_err_t _ss_spi_flash_hal_common_command(spi_flash_host_inst_t *host, spi_flash_trans_t *trans) { - bool trans_valid = is_spi_trans_valid(host, trans); + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { + return ESP_ERR_INVALID_ARG; + } + ESP_FAULT_ASSERT(tee_host != NULL); + + if (!esp_tee_buf_in_ree(trans, sizeof(spi_flash_trans_t))) { + return ESP_ERR_INVALID_ARG; + } + + spi_flash_trans_t trans_snap = *trans; + + bool trans_valid = true; + if (trans_snap.mosi_len != 0) { + trans_valid &= esp_tee_buf_in_ree(trans_snap.mosi_data, trans_snap.mosi_len); + } + if (trans_snap.miso_len != 0) { + trans_valid &= esp_tee_buf_in_ree(trans_snap.miso_data, trans_snap.miso_len); + } + trans_valid &= is_spi_cmd_addr_ok(trans_snap.address_bitlen, trans_snap.address, + trans_snap.mosi_len, trans_snap.miso_len); if (!trans_valid) { + ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, trans_snap.address); return ESP_ERR_INVALID_ARG; } ESP_FAULT_ASSERT(trans_valid); - bool addr_ok = is_spi_cmd_addr_ok(trans->address_bitlen, trans->address, trans->mosi_len, trans->miso_len); - if (!addr_ok) { - ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, trans->address); - return ESP_ERR_INVALID_ARG; - } - ESP_FAULT_ASSERT(addr_ok); - - const spi_flash_host_driver_t *orig = tee_substitute_host_driver(host); - esp_err_t r = spi_flash_hal_common_command(host, trans); - host->driver = orig; - return r; + return spi_flash_hal_common_command(tee_host, &trans_snap); } esp_err_t _ss_spi_flash_hal_device_config(spi_flash_host_inst_t *host) { - bool valid_addr = is_spi_host_in_ree(host); - - if (!valid_addr) { + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { return ESP_ERR_INVALID_ARG; } - ESP_FAULT_ASSERT(valid_addr); + ESP_FAULT_ASSERT(tee_host != NULL); - return spi_flash_hal_device_config(host); + return spi_flash_hal_device_config(tee_host); } void _ss_spi_flash_hal_erase_block(spi_flash_host_inst_t *host, uint32_t start_address) { - bool valid_addr = (is_spi_host_in_ree(host) && - start_address <= FLASH_ADDR_MAX_24BIT && - is_flash_addr_writable(start_address, FLASH_BLOCK_SIZE)); + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { + return; + } + ESP_FAULT_ASSERT(tee_host != NULL); + bool valid_addr = (start_address <= FLASH_ADDR_MAX_24BIT && + is_flash_addr_writable(start_address, FLASH_BLOCK_SIZE)); if (!valid_addr) { ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, start_address); return; } ESP_FAULT_ASSERT(valid_addr); - const spi_flash_host_driver_t *orig = tee_substitute_host_driver(host); - spi_flash_hal_erase_block(host, start_address); - host->driver = orig; + spi_flash_hal_erase_block(tee_host, start_address); } void _ss_spi_flash_hal_erase_sector(spi_flash_host_inst_t *host, uint32_t start_address) { - bool valid_addr = (is_spi_host_in_ree(host) && - start_address <= FLASH_ADDR_MAX_24BIT && - is_flash_addr_writable(start_address, FLASH_SECTOR_SIZE)); + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { + return; + } + ESP_FAULT_ASSERT(tee_host != NULL); + bool valid_addr = (start_address <= FLASH_ADDR_MAX_24BIT && + is_flash_addr_writable(start_address, FLASH_SECTOR_SIZE)); if (!valid_addr) { ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, start_address); return; } ESP_FAULT_ASSERT(valid_addr); - const spi_flash_host_driver_t *orig = tee_substitute_host_driver(host); - spi_flash_hal_erase_sector(host, start_address); - host->driver = orig; + spi_flash_hal_erase_sector(tee_host, start_address); } void _ss_spi_flash_hal_program_page(spi_flash_host_inst_t *host, const void *buffer, uint32_t address, uint32_t length) { - bool valid_addr = (is_spi_host_in_ree(host) && - address <= FLASH_ADDR_MAX_24BIT && + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { + return; + } + ESP_FAULT_ASSERT(tee_host != NULL); + + bool valid_addr = (address <= FLASH_ADDR_MAX_24BIT && is_flash_addr_writable(address, length) && esp_tee_buf_in_ree(buffer, length)); - if (!valid_addr) { ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, address); return; } ESP_FAULT_ASSERT(valid_addr); - const spi_flash_host_driver_t *orig = tee_substitute_host_driver(host); - spi_flash_hal_program_page(host, buffer, address, length); - host->driver = orig; + spi_flash_hal_program_page(tee_host, buffer, address, length); } esp_err_t _ss_spi_flash_hal_read(spi_flash_host_inst_t *host, void *buffer, uint32_t address, uint32_t read_len) { - bool valid_addr = (is_spi_host_in_ree(host) && - is_flash_addr_readable(address, read_len) && - esp_tee_buf_in_ree(buffer, read_len)); + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { + return ESP_ERR_INVALID_ARG; + } + ESP_FAULT_ASSERT(tee_host != NULL); + bool valid_addr = (is_flash_addr_readable(address, read_len) && + esp_tee_buf_in_ree(buffer, read_len)); if (!valid_addr) { ESP_LOGD(TAG, "[%s] Illegal flash access at 0x%08x", __func__, address); return ESP_ERR_INVALID_ARG; } ESP_FAULT_ASSERT(valid_addr); - const spi_flash_host_driver_t *orig = tee_substitute_host_driver(host); - esp_err_t r = spi_flash_hal_read(host, buffer, address, read_len); - host->driver = orig; - return r; + return spi_flash_hal_read(tee_host, buffer, address, read_len); } void _ss_spi_flash_hal_resume(spi_flash_host_inst_t *host) { - bool valid_addr = is_spi_host_in_ree(host); - - if (!valid_addr) { + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { return; } - ESP_FAULT_ASSERT(valid_addr); + ESP_FAULT_ASSERT(tee_host != NULL); - const spi_flash_host_driver_t *orig = tee_substitute_host_driver(host); - spi_flash_hal_resume(host); - host->driver = orig; + spi_flash_hal_resume(tee_host); } esp_err_t _ss_spi_flash_hal_set_write_protect(spi_flash_host_inst_t *host, bool wp) { - bool valid_addr = is_spi_host_in_ree(host); - - if (!valid_addr) { + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { return ESP_ERR_INVALID_ARG; } - ESP_FAULT_ASSERT(valid_addr); + ESP_FAULT_ASSERT(tee_host != NULL); - const spi_flash_host_driver_t *orig = tee_substitute_host_driver(host); - esp_err_t r = spi_flash_hal_set_write_protect(host, wp); - host->driver = orig; - return r; + return spi_flash_hal_set_write_protect(tee_host, wp); } esp_err_t _ss_spi_flash_hal_setup_read_suspend(spi_flash_host_inst_t *host, const spi_flash_sus_cmd_conf *sus_conf) { - bool valid_addr = (is_spi_host_in_ree(host) && - esp_tee_buf_in_ree(sus_conf, sizeof(spi_flash_sus_cmd_conf))); - - if (!valid_addr) { + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { return ESP_ERR_INVALID_ARG; } - ESP_FAULT_ASSERT(valid_addr); + ESP_FAULT_ASSERT(tee_host != NULL); - return spi_flash_hal_setup_read_suspend(host, sus_conf); + if (!esp_tee_buf_in_ree(sus_conf, sizeof(spi_flash_sus_cmd_conf))) { + return ESP_ERR_INVALID_ARG; + } + + const spi_flash_sus_cmd_conf sus_snap = *sus_conf; + return spi_flash_hal_setup_read_suspend(tee_host, &sus_snap); } bool _ss_spi_flash_hal_supports_direct_read(spi_flash_host_inst_t *host, const void *p) { - bool valid_addr = (is_spi_host_in_ree(host) && esp_tee_ptr_in_ree(p)); - - if (!valid_addr) { + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { return false; } - ESP_FAULT_ASSERT(valid_addr); + ESP_FAULT_ASSERT(tee_host != NULL); - return spi_flash_hal_supports_direct_read(host, p); + if (!esp_tee_ptr_in_ree(p)) { + return false; + } + + return spi_flash_hal_supports_direct_read(tee_host, p); } bool _ss_spi_flash_hal_supports_direct_write(spi_flash_host_inst_t *host, const void *p) { - bool valid_addr = (is_spi_host_in_ree(host) && esp_tee_ptr_in_ree(p)); - - if (!valid_addr) { + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { return false; } - ESP_FAULT_ASSERT(valid_addr); + ESP_FAULT_ASSERT(tee_host != NULL); - return spi_flash_hal_supports_direct_write(host, p); + if (!esp_tee_ptr_in_ree(p)) { + return false; + } + + return spi_flash_hal_supports_direct_write(tee_host, p); } void _ss_spi_flash_hal_suspend(spi_flash_host_inst_t *host) { - bool valid_addr = is_spi_host_in_ree(host); - - if (!valid_addr) { + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { return; } - ESP_FAULT_ASSERT(valid_addr); + ESP_FAULT_ASSERT(tee_host != NULL); - const spi_flash_host_driver_t *orig = tee_substitute_host_driver(host); - spi_flash_hal_suspend(host); - host->driver = orig; + spi_flash_hal_suspend(tee_host); } /* ---------------------------------------------- SPI Flash Extras ------------------------------------------------- */ @@ -680,38 +729,41 @@ uint32_t _ss_bootloader_flash_execute_command_common( esp_err_t _ss_memspi_host_flush_cache(spi_flash_host_inst_t *host, uint32_t addr, uint32_t size) { - bool valid_addr = (is_spi_host_in_ree(host) && - is_flash_addr_readable(addr, size)); - - if (!valid_addr) { + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { return ESP_ERR_INVALID_ARG; } - ESP_FAULT_ASSERT(valid_addr); + ESP_FAULT_ASSERT(tee_host != NULL); - return memspi_host_flush_cache(host, addr, size); + if (!is_flash_addr_readable(addr, size)) { + return ESP_ERR_INVALID_ARG; + } + + return memspi_host_flush_cache(tee_host, addr, size); } esp_err_t _ss_spi_flash_chip_generic_config_host_io_mode(esp_flash_t *chip, uint32_t flags) { - spi_flash_host_inst_t *host = NULL; - bool valid_addr = (esp_tee_buf_in_ree(chip, sizeof(struct esp_flash_t)) && - is_spi_host_in_ree((host = chip->host))); - - if (!valid_addr) { + if (!esp_tee_buf_in_ree(chip, sizeof(struct esp_flash_t))) { return ESP_ERR_INVALID_ARG; } - ESP_FAULT_ASSERT(valid_addr); + + spi_flash_host_inst_t *const host = chip->host; + spi_flash_hal_context_t host_snap; + spi_flash_host_inst_t *tee_host = tee_own_host(host, &host_snap); + if (tee_host == NULL) { + return ESP_ERR_INVALID_ARG; + } + ESP_FAULT_ASSERT(tee_host != NULL); esp_flash_t chip_snap = { - .host = host, + .host = tee_host, .read_mode = chip->read_mode, .hpm_dummy_ena = chip->hpm_dummy_ena, }; - const spi_flash_host_driver_t *orig = tee_substitute_host_driver(host); - esp_err_t r = spi_flash_chip_generic_config_host_io_mode(&chip_snap, flags); - host->driver = orig; - return r; + return spi_flash_chip_generic_config_host_io_mode(&chip_snap, flags); } #if CONFIG_IDF_TARGET_ESP32C5 diff --git a/components/esp_tee/subproject/main/include/esp_tee_memory_utils.h b/components/esp_tee/subproject/main/include/esp_tee_memory_utils.h index 0d52b8fc636..903c36f15ec 100644 --- a/components/esp_tee/subproject/main/include/esp_tee_memory_utils.h +++ b/components/esp_tee/subproject/main/include/esp_tee_memory_utils.h @@ -7,6 +7,7 @@ #include #include +#include #include "esp_attr.h" #include "soc/soc.h" #include "soc/ext_mem_defs.h" @@ -42,6 +43,20 @@ FORCE_INLINE_ATTR bool esp_tee_ptr_in_ree(const void *p) return esp_tee_buf_in_ree(p, 4); } +/* NOTE: re-points a REE string argument at a TEE-resident copy */ +FORCE_INLINE_ATTR void tee_snapshot_ree_str(const char **name, char *buf, size_t buf_len) +{ + const char *src = *name; + if (src == NULL || buf_len == 0) { + return; + } + + memcpy(buf, src, buf_len); + buf[buf_len - 1] = '\0'; + + *name = buf; +} + #ifdef __cplusplus } #endif diff --git a/components/esp_tee/test_apps/tee_cli_app/sdkconfig.ci.minimal_tee b/components/esp_tee/test_apps/tee_cli_app/sdkconfig.ci.minimal_tee index 2cff4b58aa5..07779a34e2b 100644 --- a/components/esp_tee/test_apps/tee_cli_app/sdkconfig.ci.minimal_tee +++ b/components/esp_tee/test_apps/tee_cli_app/sdkconfig.ci.minimal_tee @@ -5,8 +5,8 @@ CONFIG_SECURE_TEE_SEC_STG_EFUSE_HMAC_KEY_ID=5 # Reducing TEE I/DRAM sizes # 24KB CONFIG_SECURE_TEE_IRAM_SIZE=0x6000 -# 17KB -CONFIG_SECURE_TEE_DRAM_SIZE=0x4400 +# 18KB +CONFIG_SECURE_TEE_DRAM_SIZE=0x4800 # Disable TEE logs (also disable all panic logs) CONFIG_SECURE_TEE_DEBUG_MODE=n diff --git a/components/esp_tee/test_apps/tee_cli_app/sdkconfig.ci.release b/components/esp_tee/test_apps/tee_cli_app/sdkconfig.ci.release index b6b8fd811d3..e504eeea6ed 100644 --- a/components/esp_tee/test_apps/tee_cli_app/sdkconfig.ci.release +++ b/components/esp_tee/test_apps/tee_cli_app/sdkconfig.ci.release @@ -1,10 +1,6 @@ # NOTE: This sdkconfig is intended solely for CI build purposes - to verify ESP-TEE # builds across various configurations - and is not intended for production use. -# Reducing TEE IRAM size -# 31KB -CONFIG_SECURE_TEE_IRAM_SIZE=0x7C00 - # TEE Secure Storage: Release mode CONFIG_SECURE_TEE_SEC_STG_MODE_RELEASE=y CONFIG_SECURE_TEE_SEC_STG_EFUSE_HMAC_KEY_ID=5 diff --git a/components/mbedtls/port/ecc/esp_ecc.c b/components/mbedtls/port/ecc/esp_ecc.c index 59a558fd2e7..1e7ecc2535f 100644 --- a/components/mbedtls/port/ecc/esp_ecc.c +++ b/components/mbedtls/port/ecc/esp_ecc.c @@ -75,21 +75,18 @@ int esp_ecc_point_multiply(const ecc_point_t *point, const uint8_t *scalar, ecc_ int esp_ecc_point_verify(const ecc_point_t *point) { int result; + const unsigned len = point->len; - /* point->len drives a fixed-stride MMIO write loop in the HAL; an unvalidated oversized - * value (attacker-controlled via the TEE secure service) walks past the ECC register block - * and can reach other peripheral registers (CWE-787). Reject non-curve lengths up front and - * return 0 (point not verified) -- the fail-safe value for this routine. */ - if (point->len != P192_LEN && point->len != P256_LEN + if (len != P192_LEN && len != P256_LEN #if SOC_ECC_SUPPORT_CURVE_P384 - && point->len != P384_LEN + && len != P384_LEN #endif ) { return 0; } esp_ecc_acquire_hardware(); - ecc_hal_write_verify_param(point->x, point->y, point->len); + ecc_hal_write_verify_param(point->x, point->y, len); ecc_hal_set_mode(ECC_MODE_VERIFY); ecc_hal_start_calc();