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-02 11:41:49 +05:30
parent 9b185eba9f
commit 72b2e4f4fd
14 changed files with 100 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
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 */