From 72d694ddc596140031b7f4ec5c87022ce1420f92 Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Fri, 11 Sep 2026 17:17:13 +1200 Subject: [PATCH] [esp32] Respect user sdkconfig values in the TLS trims, keep EC key parsing Review follow-ups: the FINAL reconcile job now uses the set-if-absent helper so explicit sdkconfig_options win, and skips the whole TLS role choice when the user set any of its members. The EC public-key parsing options stay enabled because they decide whether a peer certificate with a compressed point or explicit curve parameters parses. Adds an ESP32-C6 http_request compile test so CI links the vasprintf wrap, and suppresses clang-tidy's no-malloc check on the allocation that vasprintf's contract requires. --- esphome/components/esp32/__init__.py | 33 ++++++++++++++----- esphome/components/esp32/vasprintf_stubs.cpp | 3 +- .../config/mbedtls_tls_user_sdkconfig.yaml | 17 ++++++++++ tests/component_tests/esp32/test_esp32.py | 19 +++++++++++ .../http_request/test.esp32-c6-idf.yaml | 4 +++ 5 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 tests/component_tests/esp32/config/mbedtls_tls_user_sdkconfig.yaml create mode 100644 tests/components/http_request/test.esp32-c6-idf.yaml diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index 4b73a22e97..bb1381f8f5 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -1856,6 +1856,7 @@ def require_mbedtls_tls_extras() -> None: Call this from components that need AES-CCM, deterministic ECDSA signing, static RSA/ECDH key exchange, TLS renegotiation or session tickets. + A user-supplied sdkconfig_options value is never overridden either. OpenThread uses CCM and deterministic ECDSA directly. """ CORE.data[KEY_ESP32][KEY_MBEDTLS_TLS_EXTRAS_REQUIRED] = True @@ -2339,9 +2340,11 @@ async def _reconcile_certificate_bundle_sdkconfig() -> None: # negotiates. Static RSA and static ECDH key exchange have no forward secrecy # and are gone in TLS 1.3, renegotiation is deprecated, esp-tls never enables # session tickets, AES-CCM ciphersuites are not offered by web servers, and -# the EC key parsing extras and deterministic ECDSA only matter when signing -# with a private key. Together they cost ~11 KB of flash whenever TLS is -# linked (http_request, mqtt). +# deterministic ECDSA only matters when signing with a private key. Together +# they cost ~10 KB of flash whenever TLS is linked (http_request, mqtt). +# The EC public key parsing extras stay enabled: they decide whether a peer +# certificate with a compressed point or explicit curve parameters parses, +# which no component can know ahead of time. MBEDTLS_TLS_EXTRA_OPTIONS = ( "CONFIG_MBEDTLS_KEY_EXCHANGE_RSA", "CONFIG_MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA", @@ -2350,11 +2353,18 @@ MBEDTLS_TLS_EXTRA_OPTIONS = ( "CONFIG_MBEDTLS_CLIENT_SSL_SESSION_TICKETS", "CONFIG_MBEDTLS_SERVER_SSL_SESSION_TICKETS", "CONFIG_MBEDTLS_CCM_C", - "CONFIG_MBEDTLS_PK_PARSE_EC_EXTENDED", - "CONFIG_MBEDTLS_PK_PARSE_EC_COMPRESSED", "CONFIG_MBEDTLS_ECDSA_DETERMINISTIC", ) +# Members of the mbedTLS "TLS Protocol Role" Kconfig choice. Setting one +# member is only valid when the user has not already chosen another. +MBEDTLS_TLS_ROLE_OPTIONS = ( + "CONFIG_MBEDTLS_TLS_SERVER_AND_CLIENT", + "CONFIG_MBEDTLS_TLS_SERVER_ONLY", + "CONFIG_MBEDTLS_TLS_CLIENT_ONLY", + "CONFIG_MBEDTLS_TLS_DISABLED", +) + @coroutine_with_priority(CoroPriority.FINAL) async def _reconcile_mbedtls_tls_sdkconfig( @@ -2365,15 +2375,22 @@ async def _reconcile_mbedtls_tls_sdkconfig( Runs at FINAL priority so every require_mbedtls_tls_server() and require_mbedtls_tls_extras() call has happened. Only the server-side handshake (~7 KB) is a separate option; nothing in ESPHome accepts TLS - connections, but OpenThread's DTLS commissioner does. + connections, but OpenThread's DTLS commissioner does. A user-supplied + sdkconfig_options value always wins; for the TLS role choice, any member + the user set leaves the whole choice alone so the pair cannot conflict. """ data = CORE.data[KEY_ESP32] - if disable_tls_server and not data.get(KEY_MBEDTLS_TLS_SERVER_REQUIRED, False): + sdkconfig = data[KEY_SDKCONFIG_OPTIONS] + if ( + disable_tls_server + and not data.get(KEY_MBEDTLS_TLS_SERVER_REQUIRED, False) + and not any(option in sdkconfig for option in MBEDTLS_TLS_ROLE_OPTIONS) + ): add_idf_sdkconfig_option("CONFIG_MBEDTLS_TLS_CLIENT_ONLY", True) add_idf_sdkconfig_option("CONFIG_MBEDTLS_TLS_SERVER_AND_CLIENT", False) if disable_tls_extras and not data.get(KEY_MBEDTLS_TLS_EXTRAS_REQUIRED, False): for option in MBEDTLS_TLS_EXTRA_OPTIONS: - add_idf_sdkconfig_option(option, False) + set_idf_sdkconfig_default(option, False) @coroutine_with_priority(CoroPriority.FINAL) diff --git a/esphome/components/esp32/vasprintf_stubs.cpp b/esphome/components/esp32/vasprintf_stubs.cpp index f53b2d2c77..308a58ebda 100644 --- a/esphome/components/esp32/vasprintf_stubs.cpp +++ b/esphome/components/esp32/vasprintf_stubs.cpp @@ -37,7 +37,8 @@ int __wrap_vasprintf(char **strp, const char *fmt, va_list ap) { if (len < 0) { return len; } - char *buf = static_cast(malloc(static_cast(len) + 1)); + // vasprintf's contract is a malloc'd buffer the caller releases with free() + char *buf = static_cast(malloc(static_cast(len) + 1)); // NOLINT(cppcoreguidelines-no-malloc) if (buf == nullptr) { return -1; } diff --git a/tests/component_tests/esp32/config/mbedtls_tls_user_sdkconfig.yaml b/tests/component_tests/esp32/config/mbedtls_tls_user_sdkconfig.yaml new file mode 100644 index 0000000000..44ff047a48 --- /dev/null +++ b/tests/component_tests/esp32/config/mbedtls_tls_user_sdkconfig.yaml @@ -0,0 +1,17 @@ +esphome: + name: test + +esp32: + board: esp32dev + framework: + type: esp-idf + sdkconfig_options: + CONFIG_MBEDTLS_TLS_SERVER_AND_CLIENT: y + CONFIG_MBEDTLS_CCM_C: y + +wifi: + ssid: "test_ssid" + password: "test_password" + +http_request: + verify_ssl: true diff --git a/tests/component_tests/esp32/test_esp32.py b/tests/component_tests/esp32/test_esp32.py index 491b8501bb..c58350a23f 100644 --- a/tests/component_tests/esp32/test_esp32.py +++ b/tests/component_tests/esp32/test_esp32.py @@ -1374,6 +1374,25 @@ def test_mbedtls_tls_trim_sdkconfig( assert {sdkconfig.get(name) for name in MBEDTLS_TLS_EXTRA_OPTIONS} == {extras} +def test_mbedtls_tls_user_sdkconfig_wins( + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], +) -> None: + """A user-set TLS role member leaves the whole choice alone; other user values are kept.""" + generate_main(component_config_path("mbedtls_tls_user_sdkconfig.yaml")) + sdkconfig = CORE.data[KEY_ESP32][KEY_SDKCONFIG_OPTIONS] + assert sdkconfig.get("CONFIG_MBEDTLS_TLS_CLIENT_ONLY") is None + role = sdkconfig["CONFIG_MBEDTLS_TLS_SERVER_AND_CLIENT"] + assert isinstance(role, RawSdkconfigValue) and role.value == "y" + ccm = sdkconfig["CONFIG_MBEDTLS_CCM_C"] + assert isinstance(ccm, RawSdkconfigValue) and ccm.value == "y" + assert { + sdkconfig.get(name) + for name in MBEDTLS_TLS_EXTRA_OPTIONS + if name != "CONFIG_MBEDTLS_CCM_C" + } == {False} + + def test_mbedtls_tls_openthread_requires_server_and_extras( generate_main: Callable[[str | Path], str], component_config_path: Callable[[str], Path], diff --git a/tests/components/http_request/test.esp32-c6-idf.yaml b/tests/components/http_request/test.esp32-c6-idf.yaml new file mode 100644 index 0000000000..ee2f5aa59b --- /dev/null +++ b/tests/components/http_request/test.esp32-c6-idf.yaml @@ -0,0 +1,4 @@ +substitutions: + verify_ssl: "true" + +<<: !include common.yaml