Mirrors/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2026-10-02 11:10:54 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(esp-tls): address MR review comments for SE PSA driver

Browse Source 
- esp_tls_mbedtls: require cert when PSA-backed server/client key is set
- esp_tls_mbedtls: drop redundant pk_init/x509_crt_init (calloc handles it)
- psa SE driver: copy callbacks/opaque_key by value (no lifetime coupling)
- psa SE driver: replace atomic CAS with simple null check on register
- psa SE driver: use sig_len from sign callback with bounds validation
- psa SE driver: validate pubkey_len returned by export_pubkey callback
- psa SE driver: check hash sub-alg in RSA PKCS1V15 branch of validate_request
- psa SE driver: align secure_element_register_callbacks doc with value-copy impl
- esp_https_server: initialize server_key in HTTPD_SSL_CONFIG_DEFAULT
- mbedtls: move SECURE_ELEMENT_DRIVER_ENABLED to esp_config.h for parity
  with ESP_ECDSA_DRIVER_ENABLED; drop target_compile_definitions
- docs: fix esp_tls_cfg_t -> esp_http_client_config_t cross-reference
- docs: check psa_import_key() status in ESP-TLS PSA example
- hints/error_output: point at CONFIG_MBEDTLS_SECURE_ELEMENT_DRIVER_ENABLED

(cherry picked from commit 08b567ef3b)
This commit is contained in:
Aditya Patwardhan
2026-07-09 11:17:02 +05:30
parent 6d7d89378c
commit 9c1dcca1af
14 changed files with 100 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
components/esp-tls/esp_tls_mbedtls.c
+34 -34
View File
@@ -763,8 +763,11 @@ static esp_err_t set_server_config(esp_tls_cfg_server_t *cfg, esp_tls_t *tls)
return esp_ret;
}
} else if (cfg->server_key != NULL && cfg->server_key->source == ESP_KEY_SOURCE_PSA) {
if (cfg->servercert_buf == NULL) {
ESP_LOGE(TAG, "Server certificate is required when using a PSA-backed server key");
return ESP_ERR_INVALID_ARG;
}
mbedtls_svc_key_id_t key_id = cfg->server_key->psa.key_id;
mbedtls_pk_init(&tls->serverkey);
ret = mbedtls_pk_wrap_psa(&tls->serverkey, key_id);
if (ret != 0) {
ESP_LOGE(TAG, "mbedtls_pk_wrap_psa returned -0x%04X", -ret);
@@ -772,22 +775,19 @@ static esp_err_t set_server_config(esp_tls_cfg_server_t *cfg, esp_tls_t *tls)
ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_MBEDTLS, -ret);
return ESP_ERR_MBEDTLS_PK_PARSE_KEY_FAILED;
}
if (cfg->servercert_buf != NULL) {
mbedtls_x509_crt_init(&tls->servercert);
ret = mbedtls_x509_crt_parse(&tls->servercert, cfg->servercert_buf, cfg->servercert_bytes);
if (ret < 0) {
ESP_LOGE(TAG, "mbedtls_x509_crt_parse returned -0x%04X", -ret);
mbedtls_print_error_msg(ret);
ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_MBEDTLS, -ret);
return ESP_ERR_MBEDTLS_X509_CRT_PARSE_FAILED;
}
ret = mbedtls_ssl_conf_own_cert(&tls->conf, &tls->servercert, &tls->serverkey);
if (ret != 0) {
ESP_LOGE(TAG, "mbedtls_ssl_conf_own_cert returned -0x%04X", -ret);
mbedtls_print_error_msg(ret);
ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_MBEDTLS, -ret);
return ESP_ERR_MBEDTLS_SSL_CONF_OWN_CERT_FAILED;
}
ret = mbedtls_x509_crt_parse(&tls->servercert, cfg->servercert_buf, cfg->servercert_bytes);
if (ret < 0) {
ESP_LOGE(TAG, "mbedtls_x509_crt_parse returned -0x%04X", -ret);
mbedtls_print_error_msg(ret);
ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_MBEDTLS, -ret);
return ESP_ERR_MBEDTLS_X509_CRT_PARSE_FAILED;
}
ret = mbedtls_ssl_conf_own_cert(&tls->conf, &tls->servercert, &tls->serverkey);
if (ret != 0) {
ESP_LOGE(TAG, "mbedtls_ssl_conf_own_cert returned -0x%04X", -ret);
mbedtls_print_error_msg(ret);
ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_MBEDTLS, -ret);
return ESP_ERR_MBEDTLS_SSL_CONF_OWN_CERT_FAILED;
}
} else if (cfg->use_ecdsa_peripheral) {
#ifdef CONFIG_MBEDTLS_HARDWARE_ECDSA_SIGN
@@ -1034,8 +1034,11 @@ esp_err_t set_client_config(const char *hostname, size_t hostlen, esp_tls_cfg_t
return esp_ret;
}
} else if (cfg->client_key != NULL && cfg->client_key->source == ESP_KEY_SOURCE_PSA) {
if (cfg->clientcert_buf == NULL) {
ESP_LOGE(TAG, "Client certificate is required when using a PSA-backed client key");
return ESP_ERR_INVALID_ARG;
}
mbedtls_svc_key_id_t key_id = cfg->client_key->psa.key_id;
mbedtls_pk_init(&tls->clientkey);
ret = mbedtls_pk_wrap_psa(&tls->clientkey, key_id);
if (ret != 0) {
ESP_LOGE(TAG, "mbedtls_pk_wrap_psa returned -0x%04X", -ret);
@@ -1043,22 +1046,19 @@ esp_err_t set_client_config(const char *hostname, size_t hostlen, esp_tls_cfg_t
ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_MBEDTLS, -ret);
return ESP_ERR_MBEDTLS_PK_PARSE_KEY_FAILED;
}
if (cfg->clientcert_buf != NULL) {
mbedtls_x509_crt_init(&tls->clientcert);
ret = mbedtls_x509_crt_parse(&tls->clientcert, cfg->clientcert_buf, cfg->clientcert_bytes);
if (ret < 0) {
ESP_LOGE(TAG, "mbedtls_x509_crt_parse returned -0x%04X", -ret);
mbedtls_print_error_msg(ret);
ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_MBEDTLS, -ret);
return ESP_ERR_MBEDTLS_X509_CRT_PARSE_FAILED;
}
ret = mbedtls_ssl_conf_own_cert(&tls->conf, &tls->clientcert, &tls->clientkey);
if (ret != 0) {
ESP_LOGE(TAG, "mbedtls_ssl_conf_own_cert returned -0x%04X", -ret);
mbedtls_print_error_msg(ret);
ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_MBEDTLS, -ret);
return ESP_ERR_MBEDTLS_SSL_CONF_OWN_CERT_FAILED;
}
ret = mbedtls_x509_crt_parse(&tls->clientcert, cfg->clientcert_buf, cfg->clientcert_bytes);
if (ret < 0) {
ESP_LOGE(TAG, "mbedtls_x509_crt_parse returned -0x%04X", -ret);
mbedtls_print_error_msg(ret);
ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_MBEDTLS, -ret);
return ESP_ERR_MBEDTLS_X509_CRT_PARSE_FAILED;
}
ret = mbedtls_ssl_conf_own_cert(&tls->conf, &tls->clientcert, &tls->clientkey);
if (ret != 0) {
ESP_LOGE(TAG, "mbedtls_ssl_conf_own_cert returned -0x%04X", -ret);
mbedtls_print_error_msg(ret);
ESP_INT_EVENT_TRACKER_CAPTURE(tls->error_handle, ESP_TLS_ERR_TYPE_MBEDTLS, -ret);
return ESP_ERR_MBEDTLS_SSL_CONF_OWN_CERT_FAILED;
}
} else if (cfg->ds_data != NULL) {
#ifdef CONFIG_ESP_TLS_USE_DS_PERIPHERAL
components/esp-tls/hints.yml
+1 -1
View File
@@ -3,4 +3,4 @@
hint: "The struct 'esp_tls_t' has now been made private - its elements can be only be accessed/modified through respective getter/setter functions. Please refer to the migration guide for more information."
-
re: "fatal error: .*atca_mbedtls_wrap\\.h: No such file or directory"
hint: "To use CONFIG_ESP_TLS_USE_SECURE_ELEMENT option, please install `esp-cryptoauthlib` using 'idf.py add-dependency espressif/esp-cryptoauthlib'"
hint: "To use the ATECC608A secure element, enable CONFIG_MBEDTLS_SECURE_ELEMENT_DRIVER_ENABLED and install `esp-cryptoauthlib` using 'idf.py add-dependency espressif/esp-cryptoauthlib'"
components/esp-tls/test_apps/README.md
+2 -2
View File
@@ -1,2 +1,2 @@
| Supported Targets | ESP32-C3 |
| ----------------- | -------- |
| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-C5 | ESP32-C6 | ESP32-C61 | ESP32-H2 | ESP32-H21 | ESP32-H4 | ESP32-P4 | ESP32-S2 | ESP32-S3 | ESP32-S31 |
| ----------------- | ----- | -------- | -------- | -------- | -------- | --------- | -------- | --------- | -------- | -------- | -------- | -------- | --------- |
components/esp_https_server/include/esp_https_server.h
+1
View File
@@ -218,6 +218,7 @@ typedef struct httpd_ssl_config httpd_ssl_config_t;
HTTPD_SSL_CONFIG_CLIENT_AUTH_OPTIONAL_INIT \
.prvtkey_pem = NULL, \
.prvtkey_len = 0, \
.server_key = NULL, \
.use_ecdsa_peripheral = false, \
.ecdsa_key_efuse_blk = 0, \
.ecdsa_key_efuse_blk_high = 0, \
components/mbedtls/CMakeLists.txt
-2
View File
@@ -481,8 +481,6 @@ if(CONFIG_MBEDTLS_SECURE_ELEMENT_DRIVER_ENABLED)
target_sources(tfpsacrypto PRIVATE
"${COMPONENT_DIR}/port/psa_driver/secure_element/psa_crypto_driver_secure_element.c")
target_include_directories(tfpsacrypto PUBLIC "${COMPONENT_DIR}/port/psa_driver/include")
target_compile_definitions(tfpsacrypto PRIVATE
SECURE_ELEMENT_DRIVER_ENABLED)
endif()
if(CONFIG_COMPILER_STATIC_ANALYZER AND CMAKE_C_COMPILER_ID STREQUAL "GNU")
components/mbedtls/port/include/mbedtls/esp_config.h
+3 -2
View File
@@ -267,8 +267,9 @@
#endif
#endif
/* SECURE_ELEMENT_DRIVER_ENABLED is set via target_compile_definitions in
* CMakeLists.txt when CONFIG_MBEDTLS_SECURE_ELEMENT_DRIVER_ENABLED is set. */
#ifdef CONFIG_MBEDTLS_SECURE_ELEMENT_DRIVER_ENABLED
#define SECURE_ELEMENT_DRIVER_ENABLED
#endif
#ifdef CONFIG_MBEDTLS_HARDWARE_ECC
#ifdef CONFIG_MBEDTLS_ECC_OTHER_CURVES_SOFT_FALLBACK
components/mbedtls/port/psa_driver/include/psa_crypto_driver_secure_element.h
+5 -3
View File
@@ -113,10 +113,12 @@ typedef struct {
* @brief Register secure element callbacks
*
* Must be called once during application initialization, before any PSA
* operations targeting PSA_KEY_LOCATION_SECURE_ELEMENT. Uses atomic
* compare-and-swap so only the first call succeeds.
* operations targeting PSA_KEY_LOCATION_SECURE_ELEMENT. Only the first
* call succeeds; subsequent calls return PSA_ERROR_BAD_STATE.
*
* @param callbacks Pointer to callback table (must remain valid for program lifetime)
* @param callbacks Pointer to callback table. The contents are copied
* internally, so the struct need not remain valid after
* this call returns.
* @return PSA_SUCCESS on success
* @return PSA_ERROR_BAD_STATE if callbacks were already registered
* @return PSA_ERROR_INVALID_ARGUMENT if callbacks is NULL
components/mbedtls/port/psa_driver/include/psa_crypto_driver_secure_element_contexts.h
+1 -1
View File
@@ -56,7 +56,7 @@ typedef struct {
uint8_t sha[SECURE_ELEMENT_MAX_KEY_BYTES];
size_t key_len;
size_t sha_len;
secure_element_opaque_key_t *opaque_key;
secure_element_opaque_key_t opaque_key;
unsigned int alg;
} secure_element_opaque_sign_hash_operation_t;
components/mbedtls/port/psa_driver/secure_element/psa_crypto_driver_secure_element.c
+37 -14
View File
@@ -18,10 +18,9 @@
*/
#include <string.h>
#include <stdatomic.h>
#include "sdkconfig.h"
#ifdef SECURE_ELEMENT_DRIVER_ENABLED
#ifdef CONFIG_MBEDTLS_SECURE_ELEMENT_DRIVER_ENABLED
#include "esp_log.h"
#include "psa_crypto_driver_secure_element.h"
@@ -30,8 +29,10 @@ static const char *TAG = "psa_crypto_driver_secure_element";
#define UNCOMPRESSED_POINT_FORMAT 0x04
/* Runtime-registered SE callbacks (set once via secure_element_register_callbacks) */
static const secure_element_callbacks_t *s_se_callbacks = NULL;
/* Runtime-registered SE callbacks (set once via secure_element_register_callbacks).
* We keep a value copy so the caller's struct lifetime does not matter. */
static secure_element_callbacks_t s_se_callbacks;
static const secure_element_callbacks_t *s_se_callbacks_ptr = NULL;
psa_status_t secure_element_register_callbacks(const secure_element_callbacks_t *callbacks)
{
@@ -44,14 +45,14 @@ psa_status_t secure_element_register_callbacks(const secure_element_callbacks_t
return PSA_ERROR_INVALID_ARGUMENT;
}
/* Atomic compare-and-swap: only the first registration succeeds */
const secure_element_callbacks_t *expected = NULL;
if (!atomic_compare_exchange_strong((volatile _Atomic(const secure_element_callbacks_t *) *)&s_se_callbacks,
&expected, callbacks)) {
if (s_se_callbacks_ptr != NULL) {
ESP_LOGE(TAG, "Secure element callbacks already registered");
return PSA_ERROR_BAD_STATE;
}
s_se_callbacks = *callbacks;
s_se_callbacks_ptr = &s_se_callbacks;
return PSA_SUCCESS;
}
@@ -60,7 +61,7 @@ psa_status_t secure_element_register_callbacks(const secure_element_callbacks_t
*/
static inline const secure_element_callbacks_t *se_get_callbacks(void)
{
return s_se_callbacks;
return s_se_callbacks_ptr;
}
/**
@@ -93,6 +94,11 @@ static psa_status_t validate_request(psa_algorithm_t alg, const psa_key_attribut
if (!PSA_ALG_IS_RSA_PKCS1V15_SIGN(alg)) {
return PSA_ERROR_NOT_SUPPORTED;
}
/* If registered with a specific hash, check it matches */
psa_algorithm_t reg_hash = PSA_ALG_SIGN_GET_HASH(registered_alg);
if (reg_hash != PSA_ALG_ANY_HASH && reg_hash != PSA_ALG_SIGN_GET_HASH(alg)) {
return PSA_ERROR_NOT_SUPPORTED;
}
} else if (registered_alg != alg) {
return PSA_ERROR_NOT_SUPPORTED;
}
@@ -336,7 +342,7 @@ psa_status_t secure_element_opaque_sign_hash_start(
memset(operation, 0, sizeof(secure_element_opaque_sign_hash_operation_t));
operation->key_len = component_len;
memcpy(operation->sha, hash, component_len);
operation->opaque_key = (secure_element_opaque_key_t *) key_buffer;
memcpy(&operation->opaque_key, key_buffer, sizeof(secure_element_opaque_key_t));
operation->alg = alg;
operation->sha_len = hash_length;
@@ -367,7 +373,7 @@ psa_status_t secure_element_opaque_sign_hash_complete(
/* Sign using registered SE callback */
uint8_t sig[2 * SECURE_ELEMENT_MAX_KEY_BYTES];
size_t sig_len = 0;
psa_status_t status = cbs->sign(operation->opaque_key->slot_id,
psa_status_t status = cbs->sign(operation->opaque_key.slot_id,
operation->sha, operation->sha_len,
sig, sizeof(sig), &sig_len);
@@ -376,9 +382,18 @@ psa_status_t secure_element_opaque_sign_hash_complete(
return status;
}
if (sig_len == 0 || sig_len > sizeof(sig)) {
ESP_LOGE(TAG, "SE returned invalid signature length: %zu", sig_len);
return PSA_ERROR_GENERIC_ERROR;
}
if (sig_len > signature_size) {
return PSA_ERROR_BUFFER_TOO_SMALL;
}
/* Copy signature to output (R || S format, big-endian - matches PSA) */
memcpy(signature, sig, 2 * component_len);
*signature_length = 2 * component_len;
memcpy(signature, sig, sig_len);
*signature_length = sig_len;
return PSA_SUCCESS;
}
@@ -463,6 +478,14 @@ psa_status_t secure_element_opaque_export_public_key(
return status;
}
/* Callback must have written exactly 2 * key_len bytes (X || Y) - reject anything else
* to avoid copying uninitialized stack memory into the caller's buffer. */
if (pubkey_len != 2 * key_len) {
ESP_LOGE(TAG, "SE export_pubkey returned %u bytes, expected %u",
(unsigned)pubkey_len, (unsigned)(2 * key_len));
return PSA_ERROR_HARDWARE_FAILURE;
}
/* Format: uncompressed point (0x04 followed by x and y coordinates) */
data[0] = UNCOMPRESSED_POINT_FORMAT;
memcpy(data + 1, pubkey, key_len); /* X coordinate */
@@ -484,4 +507,4 @@ size_t secure_element_opaque_size_function(
return sizeof(secure_element_opaque_key_t);
}
#endif /* SECURE_ELEMENT_DRIVER_ENABLED */
#endif /* CONFIG_MBEDTLS_SECURE_ELEMENT_DRIVER_ENABLED */