From 228f8894a90b997a22fe310771dd60feb7074033 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 7 Sep 2026 12:33:50 +0200 Subject: [PATCH] [noise] Consume the spare key inside the handshake and let noise own its define NoiseResponderHandshake::init() now hands the slot straight to noise-c and wipes it, so the api and ota call sites are unchanged, nothing copies the key pair and no transport has to remember the wipe. The slot compiles under USE_NOISE_SPARE_EPHEMERAL, which the api component enables as the refiller, instead of the noise component keying on an api define. The per tick check sits in loop() with the refill out of line, and the grace predicate lives next to the handshake timeout it mirrors. --- esphome/components/api/__init__.py | 2 + esphome/components/api/api_connection.cpp | 6 ++ esphome/components/api/api_connection.h | 9 +-- .../components/api/api_frame_helper_noise.cpp | 6 +- esphome/components/api/api_server.cpp | 7 +- esphome/components/api/api_server.h | 8 +-- .../esphome/ota/ota_esphome_noise.cpp | 17 +---- esphome/components/noise/__init__.py | 5 ++ esphome/components/noise/noise.cpp | 34 ++++++---- esphome/components/noise/noise.h | 21 +++--- esphome/components/noise/noise_handshake.cpp | 17 ++--- esphome/components/noise/noise_handshake.h | 8 +-- esphome/core/defines.h | 1 + tests/components/noise/__init__.py | 3 +- .../components/noise/test_noise_handshake.cpp | 68 ++++++------------- 15 files changed, 89 insertions(+), 123 deletions(-) diff --git a/esphome/components/api/__init__.py b/esphome/components/api/__init__.py index 6202e127bf..79dbd77175 100644 --- a/esphome/components/api/__init__.py +++ b/esphome/components/api/__init__.py @@ -13,6 +13,7 @@ from esphome.components.logger import request_log_listener from esphome.components.noise import ( # noqa: F401 ENCRYPTION_SCHEMA, decode_encryption_key, + enable_spare_ephemeral, encryption_schema, new_psk_progmem, validate_encryption_key, @@ -603,6 +604,7 @@ async def to_code(config: ConfigType) -> None: # and plaintext disabled. Only a factory reset can remove it. cg.add_define("USE_API_PLAINTEXT") cg.add_define("USE_API_NOISE") + enable_spare_ephemeral() else: cg.add_define("USE_API_PLAINTEXT") diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index da4b7d7702..313783863e 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -77,6 +77,8 @@ static constexpr uint32_t KEEPALIVE_DISCONNECT_TIMEOUT = (KEEPALIVE_TIMEOUT_MS * // WiFi (-70 dBm+), TCP retransmissions push real-world handshake times to // 28-30s. See https://github.com/esphome/esphome/issues/14999 static constexpr uint32_t HANDSHAKE_TIMEOUT_MS = 60000; +// How long a new connection holds off the spare ephemeral refill +static constexpr uint32_t CONNECT_GRACE_MS = 1000; static constexpr auto ESPHOME_VERSION_REF = StringRef::from_lit(ESPHOME_VERSION); @@ -250,6 +252,10 @@ void APIConnection::begin_iterator_(ActiveIterator type) { } } +bool APIConnection::is_still_connecting(uint32_t now) { + return !this->is_authenticated() && now - this->last_traffic_ < CONNECT_GRACE_MS; +} + void APIConnection::loop() { if (this->flags_.next_close) { // requested a disconnect - don't close socket here, let APIServer::loop() do it diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index 04938a00fa..206778282e 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -316,15 +316,12 @@ class APIConnection final : public APIServerConnectionBase { void on_noise_encryption_set_key_request(const NoiseEncryptionSetKeyRequest &msg); #endif - static constexpr uint32_t CONNECT_GRACE_MS = 1000; bool is_authenticated() { return static_cast(this->flags_.connection_state) == ConnectionState::AUTHENTICATED; } - // A connection that is still setting up within its grace period; an older - // unauthenticated one is a stale half open client and no longer counts - bool is_still_connecting(uint32_t now) { - return !this->is_authenticated() && now - this->last_traffic_ < CONNECT_GRACE_MS; - } + // Unauthenticated and within its grace period; an older unauthenticated + // connection is a stale half open client and no longer counts + bool is_still_connecting(uint32_t now); bool is_connection_setup() { return static_cast(this->flags_.connection_state) == ConnectionState::CONNECTED || this->is_authenticated(); diff --git a/esphome/components/api/api_frame_helper_noise.cpp b/esphome/components/api/api_frame_helper_noise.cpp index d18a722c9e..29b2858aee 100644 --- a/esphome/components/api/api_frame_helper_noise.cpp +++ b/esphome/components/api/api_frame_helper_noise.cpp @@ -10,7 +10,6 @@ #include "proto.h" #include #include -#include #ifdef USE_ESP8266 #include @@ -549,10 +548,7 @@ APIError APINoiseFrameHelper::write_frame_(const uint8_t *data, uint16_t len) { * @return 0 on success, -1 on error (check errno) */ APIError APINoiseFrameHelper::init_handshake_() { - noise::ephemeral_keypair_t spare; - const uint8_t *ephemeral = noise::take_spare_ephemeral(spare) ? spare.data() : nullptr; - int err = this->handshake_.init(this->ctx_, prologue_.data(), prologue_.size(), ephemeral); - sodium_memzero(spare.data(), spare.size()); + int err = this->handshake_.init(this->ctx_, prologue_.data(), prologue_.size()); APIError aerr = handle_noise_error_(err, LOG_STR("noise_handshake_init"), APIError::HANDSHAKESTATE_SETUP_FAILED); if (aerr != APIError::OK) return aerr; diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index 438283e826..d82875b6a4 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -144,7 +144,10 @@ void APIServer::setup() { void APIServer::loop() { #ifdef USE_API_NOISE - this->prepare_spare_ephemeral_(); + // Only the flag test is inline; refilling is the rare path + if (!noise::has_spare_ephemeral()) { + this->refill_spare_ephemeral_(); + } #endif // Accept new clients only if the socket exists and has incoming connections if (this->socket_ && this->socket_->ready()) { @@ -199,7 +202,7 @@ void APIServer::loop() { #ifdef USE_API_NOISE // Refill only while no api client is still connecting; an OTA handshake is // not visible here and just pays the refill it triggered. -void APIServer::prepare_spare_ephemeral_slow_() { +void APIServer::refill_spare_ephemeral_() { if (!network::is_connected()) { return; } diff --git a/esphome/components/api/api_server.h b/esphome/components/api/api_server.h index 5e5f472c1f..6127d0dec7 100644 --- a/esphome/components/api/api_server.h +++ b/esphome/components/api/api_server.h @@ -364,13 +364,7 @@ class APIServer final : public Component, #endif #ifdef USE_API_NOISE - // Polled every loop tick: only the flag test stays inline - void prepare_spare_ephemeral_() { - if (!noise::has_spare_ephemeral()) { - this->prepare_spare_ephemeral_slow_(); - } - } - void prepare_spare_ephemeral_slow_(); + void refill_spare_ephemeral_(); noise::NoiseContext noise_ctx_; #ifndef USE_API_NOISE_PSK_FROM_YAML SavedNoisePsk saved_psk_{}; // backs noise_ctx_ for a runtime provisioned key diff --git a/esphome/components/esphome/ota/ota_esphome_noise.cpp b/esphome/components/esphome/ota/ota_esphome_noise.cpp index 747f99053c..7401413d6d 100644 --- a/esphome/components/esphome/ota/ota_esphome_noise.cpp +++ b/esphome/components/esphome/ota/ota_esphome_noise.cpp @@ -8,7 +8,6 @@ #include #include -#include #ifdef USE_ESP8266 #include @@ -66,21 +65,9 @@ bool ESPHomeOTAComponent::noise_start_session_(uint8_t server_feature_flags) { *p++ = ota::OTA_RESPONSE_FEATURE_FLAGS; *p++ = server_feature_flags; - // Only the api server refills the spare - const uint8_t *ephemeral = nullptr; -#ifdef USE_API_NOISE - noise::ephemeral_keypair_t spare; - if (this->noise_ != nullptr && noise::take_spare_ephemeral(spare)) { - ephemeral = spare.data(); - } -#endif // The caller only starts a session when the context holds a key - int err = this->noise_ == nullptr - ? NOISE_ERROR_NO_MEMORY - : this->noise_->handshake.init(this->noise_context_(), prologue, sizeof(prologue), ephemeral); -#ifdef USE_API_NOISE - sodium_memzero(spare.data(), spare.size()); -#endif + int err = this->noise_ == nullptr ? NOISE_ERROR_NO_MEMORY + : this->noise_->handshake.init(this->noise_context_(), prologue, sizeof(prologue)); if (err != 0) { // Raw noise codes throughout: the name table would cost flash in builds // where only the OTA uses noise diff --git a/esphome/components/noise/__init__.py b/esphome/components/noise/__init__.py index 4de706120e..474759cc20 100644 --- a/esphome/components/noise/__init__.py +++ b/esphome/components/noise/__init__.py @@ -86,6 +86,11 @@ def encryption_schema(config: ConfigType | None) -> ConfigType: return ENCRYPTION_SCHEMA(config) +def enable_spare_ephemeral() -> None: + """Compile the spare ephemeral key slot; the component that refills it calls this.""" + cg.add_define("USE_NOISE_SPARE_EPHEMERAL") + + async def to_code(config: ConfigType) -> None: cg.add_define("USE_NOISE") cg.add_library("esphome/noise-c", "0.1.24") diff --git a/esphome/components/noise/noise.cpp b/esphome/components/noise/noise.cpp index b2c6a3af8b..8dc661336e 100644 --- a/esphome/components/noise/noise.cpp +++ b/esphome/components/noise/noise.cpp @@ -26,39 +26,45 @@ void NoiseContext::load_psk(psk_t &out) const { progmem_memcpy(out.data(), this->psk_, out.size()); } -#ifdef USE_API_NOISE -static uint8_t spare_ephemeral[EPHEMERAL_KEYPAIR_SIZE]; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -bool spare_ephemeral_ready = false; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +#ifdef USE_NOISE_SPARE_EPHEMERAL +static constexpr size_t PRIVATE_KEY_SIZE = 32; +static constexpr size_t PUBLIC_KEY_SIZE = 32; +// Private key then public key +static uint8_t + spare_ephemeral[PRIVATE_KEY_SIZE + PUBLIC_KEY_SIZE]; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +bool spare_ephemeral_ready = false; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) void prepare_spare_ephemeral() { // A partial fill must never look ready spare_ephemeral_ready = false; uint8_t *private_key = spare_ephemeral; - uint8_t *public_key = spare_ephemeral + EPHEMERAL_PRIVATE_KEY_SIZE; - // Same steps as noise-c's curve25519 keygen; on RNG failure the slot stays + uint8_t *public_key = spare_ephemeral + PRIVATE_KEY_SIZE; + // Same steps as noise-c's curve25519 keygen; on failure the slot stays // empty and the handshake generates its own key - if (!random_bytes(private_key, EPHEMERAL_PRIVATE_KEY_SIZE)) { + if (!random_bytes(private_key, PRIVATE_KEY_SIZE)) { return; } private_key[0] &= 0xF8; - private_key[EPHEMERAL_PRIVATE_KEY_SIZE - 1] = (private_key[EPHEMERAL_PRIVATE_KEY_SIZE - 1] & 0x7F) | 0x40; + private_key[PRIVATE_KEY_SIZE - 1] = (private_key[PRIVATE_KEY_SIZE - 1] & 0x7F) | 0x40; if (crypto_scalarmult_curve25519_base(public_key, private_key) != 0) { - sodium_memzero(spare_ephemeral, EPHEMERAL_KEYPAIR_SIZE); + sodium_memzero(spare_ephemeral, sizeof(spare_ephemeral)); return; } spare_ephemeral_ready = true; } -bool take_spare_ephemeral(ephemeral_keypair_t &out) { +int consume_spare_ephemeral(NoiseHandshakeState *state) { if (!spare_ephemeral_ready) { - return false; + return 0; } - std::memcpy(out.data(), spare_ephemeral, EPHEMERAL_KEYPAIR_SIZE); - sodium_memzero(spare_ephemeral, EPHEMERAL_KEYPAIR_SIZE); + // noise-c keeps its own copy, so the slot is wiped either way + int err = noise_handshakestate_set_local_ephemeral(state, spare_ephemeral, PRIVATE_KEY_SIZE, + spare_ephemeral + PRIVATE_KEY_SIZE, PUBLIC_KEY_SIZE); + sodium_memzero(spare_ephemeral, sizeof(spare_ephemeral)); spare_ephemeral_ready = false; - return true; + return err; } -#endif // USE_API_NOISE +#endif // USE_NOISE_SPARE_EPHEMERAL const LogString *noise_err_to_logstr(int err) { if (err == NOISE_ERROR_NO_MEMORY) diff --git a/esphome/components/noise/noise.h b/esphome/components/noise/noise.h index ca103275ba..97e54f4292 100644 --- a/esphome/components/noise/noise.h +++ b/esphome/components/noise/noise.h @@ -6,6 +6,9 @@ #include #include "esphome/core/log.h" +// noise-c handshake state; the full definition lives in +typedef struct NoiseHandshakeState_s NoiseHandshakeState; + namespace esphome::noise { using psk_t = std::array; @@ -38,23 +41,19 @@ class NoiseContext { /// Convert a noise error code to a readable error const LogString *noise_err_to_logstr(int err); -// Spare key pair layout: private key then public key -static constexpr size_t EPHEMERAL_PRIVATE_KEY_SIZE = 32; -static constexpr size_t EPHEMERAL_PUBLIC_KEY_SIZE = 32; -static constexpr size_t EPHEMERAL_KEYPAIR_SIZE = EPHEMERAL_PRIVATE_KEY_SIZE + EPHEMERAL_PUBLIC_KEY_SIZE; -using ephemeral_keypair_t = std::array; - -#ifdef USE_API_NOISE +#ifdef USE_NOISE_SPARE_EPHEMERAL // One responder ephemeral key pair generated ahead of time (about 60 ms on -// ESP8266), refilled by the api server while idle, shared by every noise -// transport; an empty slot means the handshake generates its own key. +// ESP8266), refilled by the api server while idle and consumed by the next +// handshake of any noise transport; an empty slot means the handshake +// generates its own key. extern bool spare_ephemeral_ready; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) // Polled every api loop tick, so it must inline inline bool has_spare_ephemeral() { return spare_ephemeral_ready; } /// Fill the slot; blocks for the base point multiply void prepare_spare_ephemeral(); -/// Move the slot into out and empty it; false (out untouched) when empty -bool take_spare_ephemeral(ephemeral_keypair_t &out); +/// Hand the slot's key pair to a handshake that has not started and wipe the +/// slot; 0 when the slot was empty or the key was taken, else the noise-c error +int consume_spare_ephemeral(NoiseHandshakeState *state); #endif // Shared wire format for the noise transports (api and ota): every frame is diff --git a/esphome/components/noise/noise_handshake.cpp b/esphome/components/noise/noise_handshake.cpp index 3def02708c..55fcf04358 100644 --- a/esphome/components/noise/noise_handshake.cpp +++ b/esphome/components/noise/noise_handshake.cpp @@ -20,8 +20,7 @@ NoiseResponderHandshake::~NoiseResponderHandshake() { } } -int NoiseResponderHandshake::init(const NoiseContext &ctx, const uint8_t *prologue, size_t prologue_len, - const uint8_t *ephemeral_keypair) { +int NoiseResponderHandshake::init(const NoiseContext &ctx, const uint8_t *prologue, size_t prologue_len) { if (this->handshake_ != nullptr) { noise_handshakestate_free(this->handshake_); this->handshake_ = nullptr; @@ -58,15 +57,13 @@ int NoiseResponderHandshake::init(const NoiseContext &ctx, const uint8_t *prolog HANDSHAKE_STEP_LOG("noise_handshakestate_set_prologue", err); return this->fail_init_(err); } - if (ephemeral_keypair != nullptr) { - err = noise_handshakestate_set_local_ephemeral(this->handshake_, ephemeral_keypair, EPHEMERAL_PRIVATE_KEY_SIZE, - ephemeral_keypair + EPHEMERAL_PRIVATE_KEY_SIZE, - EPHEMERAL_PUBLIC_KEY_SIZE); - // Not fatal: the handshake generates its own key instead - if (err != 0) { - HANDSHAKE_STEP_LOG("noise_handshakestate_set_local_ephemeral", err); - } +#ifdef USE_NOISE_SPARE_EPHEMERAL + err = consume_spare_ephemeral(this->handshake_); + // Not fatal: the handshake generates its own key instead + if (err != 0) { + HANDSHAKE_STEP_LOG("noise_handshakestate_set_local_ephemeral", err); } +#endif err = noise_handshakestate_start(this->handshake_); if (err != 0) { HANDSHAKE_STEP_LOG("noise_handshakestate_start", err); diff --git a/esphome/components/noise/noise_handshake.h b/esphome/components/noise/noise_handshake.h index dd53e6db14..ef556a26b0 100644 --- a/esphome/components/noise/noise_handshake.h +++ b/esphome/components/noise/noise_handshake.h @@ -37,11 +37,9 @@ class NoiseResponderHandshake { NoiseResponderHandshake &operator=(const NoiseResponderHandshake &) = delete; /// Create and start the handshake with the context's PSK and the prologue. - /// A repeated call frees the previous handshake state and starts over. - /// ephemeral_keypair, when set, is a take_spare_ephemeral() key pair used - /// instead of generating one; a refused pair falls back to generating. - [[nodiscard]] int init(const NoiseContext &ctx, const uint8_t *prologue, size_t prologue_len, - const uint8_t *ephemeral_keypair = nullptr); + /// A repeated call frees the previous handshake state and starts over. A + /// spare ephemeral key, when one is ready, is used instead of generating. + [[nodiscard]] int init(const NoiseContext &ctx, const uint8_t *prologue, size_t prologue_len); /// ACTION_FAILED is the catch-all: returned before init(), after split() /// has released the state, and when noise-c reports a failed handshake. [[nodiscard]] Action action() const; diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 9dd1e0ced6..bd8d938b7c 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -230,6 +230,7 @@ #define USE_IMPROV_SERIAL_NEXT_URL #define USE_MD5 #define USE_NOISE +#define USE_NOISE_SPARE_EPHEMERAL #define USE_SHA256 #ifndef USE_RP2 // no MQTT backend or esp_wireguard library on RP2 #define USE_MQTT diff --git a/tests/components/noise/__init__.py b/tests/components/noise/__init__.py index d21f123031..aeae988f44 100644 --- a/tests/components/noise/__init__.py +++ b/tests/components/noise/__init__.py @@ -10,7 +10,6 @@ def override_manifest(manifest: ComponentManifestOverride) -> None: async def to_code_testing(config): await real_to_code(config) - # The spare ephemeral slot only exists in builds with an encrypted api - cg.add_define("USE_API_NOISE") + cg.add_define("USE_NOISE_SPARE_EPHEMERAL") manifest.to_code = to_code_testing diff --git a/tests/components/noise/test_noise_handshake.cpp b/tests/components/noise/test_noise_handshake.cpp index 14556bbf11..d7ad8cdba6 100644 --- a/tests/components/noise/test_noise_handshake.cpp +++ b/tests/components/noise/test_noise_handshake.cpp @@ -3,7 +3,6 @@ #include #include -#include #include "esphome/components/noise/noise.h" #include "esphome/components/noise/noise_handshake.h" @@ -158,59 +157,36 @@ TEST(NoiseResponderHandshakeTest, FullHandshakeAndTransportRoundTrip) { noise_cipherstate_free(recv_cipher); } -TEST(SpareEphemeralTest, EmptySlotHandsOutNothing) { - ephemeral_keypair_t out; - // Drain whatever an earlier test left behind, then the slot must stay empty - take_spare_ephemeral(out); - EXPECT_FALSE(has_spare_ephemeral()); - EXPECT_FALSE(take_spare_ephemeral(out)); -} - -TEST(SpareEphemeralTest, KeyPairIsHandedOutExactlyOnce) { - ephemeral_keypair_t out; - take_spare_ephemeral(out); - prepare_spare_ephemeral(); - ASSERT_TRUE(has_spare_ephemeral()); - ASSERT_TRUE(take_spare_ephemeral(out)); - // Taken once: the slot is empty and a second take gets nothing - EXPECT_FALSE(has_spare_ephemeral()); - EXPECT_FALSE(take_spare_ephemeral(out)); - - // The pair is consistent: the public half is the base point multiple of the private half - uint8_t check[EPHEMERAL_PUBLIC_KEY_SIZE]; - ASSERT_EQ(crypto_scalarmult_curve25519_base(check, out.data()), 0); - EXPECT_EQ(std::memcmp(check, out.data() + EPHEMERAL_PRIVATE_KEY_SIZE, EPHEMERAL_PUBLIC_KEY_SIZE), 0); -} - -TEST(SpareEphemeralTest, SuppliedKeyPairCompletesHandshakeAndIsTheKeyOnTheWire) { - ephemeral_keypair_t spare; - take_spare_ephemeral(spare); - prepare_spare_ephemeral(); - ASSERT_TRUE(take_spare_ephemeral(spare)); - +// Drive one full NNpsk0 handshake between a fresh initiator and responder +static void run_handshake(NoiseResponderHandshake &responder) { const psk_t psk = make_psk(7); - NoiseResponderHandshake responder; - ASSERT_EQ(responder.init(ctx_for(psk), PROLOGUE, sizeof(PROLOGUE), spare.data()), 0); - + ASSERT_EQ(responder.init(ctx_for(psk), PROLOGUE, sizeof(PROLOGUE)), 0); Initiator initiator(psk, PROLOGUE, sizeof(PROLOGUE)); uint8_t msg[MAX_HANDSHAKE_SIZE]; size_t msg_len = initiator.write_message(msg, sizeof(msg)); ASSERT_EQ(responder.read_message(msg, msg_len), 0); - size_t reply_len = 0; ASSERT_EQ(responder.write_message(msg, sizeof(msg), reply_len), 0); - // The responder's message starts with its ephemeral public key - ASSERT_GE(reply_len, static_cast(EPHEMERAL_PUBLIC_KEY_SIZE)); - EXPECT_EQ(std::memcmp(msg, spare.data() + EPHEMERAL_PRIVATE_KEY_SIZE, EPHEMERAL_PUBLIC_KEY_SIZE), 0); - ASSERT_EQ(initiator.read_message(msg, reply_len), 0); - initiator.split(); - NoiseCipherState *send_cipher = nullptr; - NoiseCipherState *recv_cipher = nullptr; - ASSERT_EQ(responder.split(send_cipher, recv_cipher), 0); - ASSERT_NE(send_cipher, nullptr); - noise_cipherstate_free(send_cipher); - noise_cipherstate_free(recv_cipher); + ASSERT_EQ(responder.action(), Action::ACTION_SPLIT); +} + +TEST(SpareEphemeralTest, EmptySlotLeavesHandshakeToGenerate) { + NoiseResponderHandshake responder; + run_handshake(responder); + EXPECT_FALSE(has_spare_ephemeral()); +} + +TEST(SpareEphemeralTest, SlotIsConsumedByExactlyOneHandshake) { + prepare_spare_ephemeral(); + ASSERT_TRUE(has_spare_ephemeral()); + NoiseResponderHandshake first; + run_handshake(first); + // Consumed: the next handshake finds no spare and still completes + EXPECT_FALSE(has_spare_ephemeral()); + NoiseResponderHandshake second; + run_handshake(second); + EXPECT_FALSE(has_spare_ephemeral()); } TEST(NoiseResponderHandshakeTest, ReInitRestartsHandshake) {