diff --git a/esphome/components/api/api_frame_helper_noise.cpp b/esphome/components/api/api_frame_helper_noise.cpp index cb3fafe15a..0f975b23a2 100644 --- a/esphome/components/api/api_frame_helper_noise.cpp +++ b/esphome/components/api/api_frame_helper_noise.cpp @@ -549,10 +549,10 @@ APIError APINoiseFrameHelper::write_frame_(const uint8_t *data, uint16_t len) { * @return 0 on success, -1 on error (check errno) */ APIError APINoiseFrameHelper::init_handshake_() { - uint8_t spare[noise::EPHEMERAL_KEYPAIR_SIZE]; - const uint8_t *ephemeral = noise::take_spare_ephemeral(spare) ? spare : nullptr; + noise::ephemeral_keypair_t spare; + const uint8_t *ephemeral = noise::take_spare_ephemeral(spare) ? spare.data() : nullptr; int err = this->handshake_.init(this->ctx_.get_psk(), prologue_.data(), prologue_.size(), ephemeral); - sodium_memzero(spare, sizeof(spare)); + sodium_memzero(spare.data(), spare.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 1f6aa9d763..25667513c8 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -193,10 +193,13 @@ void APIServer::loop() { #ifdef USE_API_NOISE // Refill the spare ephemeral key while nobody is waiting for it: not before -// the network is up, and not while a client is still in its handshake, since -// the key generation blocks the loop for its duration. On ESP8266 that is -// about 60 ms, over the 50 ms blocking warning; the same generation used to -// run inside the handshake, where it tripped the warning for longer. +// the network is up, and not while an api client is still in its handshake +// (an OTA handshake is not visible here; it took the previous spare and +// pays the refill instead, no worse than generating its own key). The +// generation blocks the loop for about 60 ms on ESP8266, over the 50 ms +// blocking warning, so the api component's warning threshold ratchets up +// on the first refill; the same generation used to run inside the +// handshake, where it tripped the warning for longer. void APIServer::prepare_spare_ephemeral_() { if (noise::has_spare_ephemeral() || !network::is_connected()) { return; diff --git a/esphome/components/esphome/ota/ota_esphome_noise.cpp b/esphome/components/esphome/ota/ota_esphome_noise.cpp index f937ea8299..7115c5779d 100644 --- a/esphome/components/esphome/ota/ota_esphome_noise.cpp +++ b/esphome/components/esphome/ota/ota_esphome_noise.cpp @@ -75,14 +75,14 @@ bool ESPHomeOTAComponent::noise_start_session_(uint8_t server_feature_flags) { // The api server keeps the spare; without an encrypted api there is none const uint8_t *ephemeral = nullptr; #ifdef USE_API_NOISE - uint8_t spare[noise::EPHEMERAL_KEYPAIR_SIZE]; + noise::ephemeral_keypair_t spare; if (noise::take_spare_ephemeral(spare)) { - ephemeral = spare; + ephemeral = spare.data(); } #endif int err = this->noise_->handshake.init(this->noise_ctx_.get_psk(), prologue, sizeof(prologue), ephemeral); #ifdef USE_API_NOISE - sodium_memzero(spare, sizeof(spare)); + sodium_memzero(spare.data(), spare.size()); #endif if (err != 0) { ESP_LOGW(TAG, "Handshake init: %s", LOG_STR_ARG(noise::noise_err_to_logstr(err))); diff --git a/esphome/components/noise/noise.cpp b/esphome/components/noise/noise.cpp index a38b6cf368..4831616b13 100644 --- a/esphome/components/noise/noise.cpp +++ b/esphome/components/noise/noise.cpp @@ -38,11 +38,11 @@ void prepare_spare_ephemeral() { spare_ephemeral_ready = true; } -bool take_spare_ephemeral(uint8_t *out) { +bool take_spare_ephemeral(ephemeral_keypair_t &out) { if (!spare_ephemeral_ready) { return false; } - std::memcpy(out, spare_ephemeral, EPHEMERAL_KEYPAIR_SIZE); + std::memcpy(out.data(), spare_ephemeral, EPHEMERAL_KEYPAIR_SIZE); sodium_memzero(spare_ephemeral, EPHEMERAL_KEYPAIR_SIZE); spare_ephemeral_ready = false; return true; diff --git a/esphome/components/noise/noise.h b/esphome/components/noise/noise.h index 18a0da8d8f..9d6ff259aa 100644 --- a/esphome/components/noise/noise.h +++ b/esphome/components/noise/noise.h @@ -42,6 +42,7 @@ const LogString *noise_err_to_logstr(int err); 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 // A responder ephemeral key pair generated ahead of time. The base point @@ -53,9 +54,9 @@ static constexpr size_t EPHEMERAL_KEYPAIR_SIZE = EPHEMERAL_PRIVATE_KEY_SIZE + EP bool has_spare_ephemeral(); /// Generate a key pair into the slot; blocks for the base point multiply. void prepare_spare_ephemeral(); -/// Move the slot's key pair into out (EPHEMERAL_KEYPAIR_SIZE bytes) and empty -/// the slot. Returns false, leaving out untouched, when the slot is empty. -bool take_spare_ephemeral(uint8_t *out); +/// Move the slot's key pair into out and empty the slot. Returns false, +/// leaving out untouched, when the slot is empty. +bool take_spare_ephemeral(ephemeral_keypair_t &out); #endif // Shared wire format for the noise transports (api and ota): every frame is diff --git a/tests/components/noise/test_noise_handshake.cpp b/tests/components/noise/test_noise_handshake.cpp index 822a1eabaa..e2bf7eb89f 100644 --- a/tests/components/noise/test_noise_handshake.cpp +++ b/tests/components/noise/test_noise_handshake.cpp @@ -151,7 +151,7 @@ TEST(NoiseResponderHandshakeTest, FullHandshakeAndTransportRoundTrip) { } TEST(SpareEphemeralTest, EmptySlotHandsOutNothing) { - uint8_t out[EPHEMERAL_KEYPAIR_SIZE]; + 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()); @@ -159,7 +159,7 @@ TEST(SpareEphemeralTest, EmptySlotHandsOutNothing) { } TEST(SpareEphemeralTest, KeyPairIsHandedOutExactlyOnce) { - uint8_t out[EPHEMERAL_KEYPAIR_SIZE]; + ephemeral_keypair_t out; take_spare_ephemeral(out); prepare_spare_ephemeral(); ASSERT_TRUE(has_spare_ephemeral()); @@ -170,19 +170,19 @@ TEST(SpareEphemeralTest, KeyPairIsHandedOutExactlyOnce) { // 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), 0); - EXPECT_EQ(std::memcmp(check, out + EPHEMERAL_PRIVATE_KEY_SIZE, EPHEMERAL_PUBLIC_KEY_SIZE), 0); + 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) { - uint8_t spare[EPHEMERAL_KEYPAIR_SIZE]; + ephemeral_keypair_t spare; take_spare_ephemeral(spare); prepare_spare_ephemeral(); ASSERT_TRUE(take_spare_ephemeral(spare)); const psk_t psk = make_psk(7); NoiseResponderHandshake responder; - ASSERT_EQ(responder.init(psk, PROLOGUE, sizeof(PROLOGUE), spare), 0); + ASSERT_EQ(responder.init(psk, PROLOGUE, sizeof(PROLOGUE), spare.data()), 0); Initiator initiator(psk, PROLOGUE, sizeof(PROLOGUE)); uint8_t msg[MAX_HANDSHAKE_SIZE]; @@ -193,7 +193,7 @@ TEST(SpareEphemeralTest, SuppliedKeyPairCompletesHandshakeAndIsTheKeyOnTheWire) 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 + EPHEMERAL_PRIVATE_KEY_SIZE, EPHEMERAL_PUBLIC_KEY_SIZE), 0); + 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();