From 029f6d4bc3bd66db4f77c2ff04a896be802375c6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 5 Sep 2026 11:13:43 +0200 Subject: [PATCH] [ota] Offer encryption with the api key so enabling it works over OTA --- THREAT_MODEL.md | 33 +++-- esphome/components/esphome/ota/__init__.py | 25 +++- .../components/esphome/ota/ota_esphome.cpp | 32 ++-- esphome/core/defines.h | 1 + esphome/espota2.py | 10 +- tests/component_tests/ota/test_esphome_ota.py | 80 ++++++++++ .../ota/test_esphome_ota_api_key_offer.yaml | 11 ++ ...st_esphome_ota_api_key_offer_password.yaml | 12 ++ .../test_esphome_ota_encryption_required.yaml | 12 ++ .../ota/test_esphome_ota_runtime_api_key.yaml | 10 ++ tests/components/ota/api_key_offer.yaml | 12 ++ .../ota/test-api_key_offer.esp32-idf.yaml | 2 + .../ota/test-api_key_offer.esp8266-ard.yaml | 2 + .../host_ota_api_key_offer_with_password.yaml | 12 ++ .../host_ota_api_key_offers_encryption.yaml | 11 ++ tests/integration/test_host_ota.py | 137 ++++++++++++++++++ tests/unit_tests/test_espota2_noise.py | 55 ++++++- 17 files changed, 425 insertions(+), 32 deletions(-) create mode 100644 tests/component_tests/ota/test_esphome_ota_api_key_offer.yaml create mode 100644 tests/component_tests/ota/test_esphome_ota_api_key_offer_password.yaml create mode 100644 tests/component_tests/ota/test_esphome_ota_encryption_required.yaml create mode 100644 tests/component_tests/ota/test_esphome_ota_runtime_api_key.yaml create mode 100644 tests/components/ota/api_key_offer.yaml create mode 100644 tests/components/ota/test-api_key_offer.esp32-idf.yaml create mode 100644 tests/components/ota/test-api_key_offer.esp8266-ard.yaml create mode 100644 tests/integration/fixtures/host_ota_api_key_offer_with_password.yaml create mode 100644 tests/integration/fixtures/host_ota_api_key_offers_encryption.yaml diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index b4f557e55b..3d820761e2 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -125,26 +125,33 @@ design is optimal or that it will not change. ## OTA update encryption The `esphome` OTA platform optionally encrypts updates with the same Noise -`NNpsk0` pattern the native API uses; one key protects the device. With an -`encryption:` block configured the guarantees are: the firmware image is +`NNpsk0` pattern the native API uses; one key protects the device. A device +whose `api:` block has a static encryption key compiles in the transport and +offers it on every OTA connection, so an uploader presenting the key gets the +guarantees below even without an `ota: encryption:` block; only that block +makes the device require encryption. The guarantees are: the firmware image is confidential in transit, the uploader is authenticated by the pre-shared key, and the plaintext negotiation preceding the handshake is bound into the handshake prologue, so stripping or tampering with it fails the first MAC. -Both ends fail closed with no override: a device built with a key refuses -plaintext uploads, and the CLI refuses to send plaintext when a key is -configured. +With `ota: encryption:` configured both ends fail closed with no override: the +device refuses plaintext uploads, and the CLI refuses to send plaintext when a +key is configured. -Defeating any of that without the key is in scope: a keyed device accepting a -plaintext or downgraded upload, getting past the MAC, or recovering image -contents from captured traffic. +Defeating any of that without the key is in scope: a device that requires +encryption accepting a plaintext or downgraded upload, getting past the MAC, +or recovering image contents from captured traffic. The following are **not** vulnerabilities, by design: -- Plaintext OTA on a device with no `encryption:` block. That is the - documented default, authenticated (if at all) by the OTA password. -- The enablement window: turning encryption on takes one last upload of the - encryption-enabled firmware over the existing plaintext channel, with the - pre-existing plaintext exposure. +- Plaintext OTA on a device with no `ota: encryption:` block, including one + that offers encryption because it has a static api key. That is the + documented default, authenticated (if at all) by the OTA password. An + uploader that takes the offer skips the password; the key authenticates it. +- The enablement window: requiring encryption needs a running firmware that + offers it, which every build with a static api key does. Older firmware + takes one last upload of the offering firmware over the existing plaintext + channel, with the pre-existing plaintext exposure; the upload that turns on + `ota: encryption:` is then already encrypted. - The web OTA `/update` endpoint alongside encryption. The `web_server` component keeps it always reachable, and `captive_portal:` auto-loads it for the fallback AP window; validation warns about both combinations, and diff --git a/esphome/components/esphome/ota/__init__.py b/esphome/components/esphome/ota/__init__.py index 1fec9e5c9b..20d9734c6c 100644 --- a/esphome/components/esphome/ota/__init__.py +++ b/esphome/components/esphome/ota/__init__.py @@ -173,6 +173,19 @@ def _warn_web_server_ota(full_conf: ConfigType) -> None: ) +def _api_static_key(api_conf: ConfigType) -> str | None: + """The api encryption key when it is fixed at build time. + + A keyless api block provisions its key at runtime and the all-zeros key is + the unprovisioned sentinel (the device treats it as no key); neither can + seed the OTA encryption offer. + """ + key = api_conf.get(CONF_ENCRYPTION, {}).get(CONF_KEY) + if not key or is_reserved_key(key): + return None + return key + + def _resolve_encryption_key(encryption_conf: ConfigType, api_conf: ConfigType) -> None: """Resolve the one encryption key per device into the ota block. @@ -268,7 +281,10 @@ FINAL_VALIDATE_SCHEMA = ota_esphome_final_validate def FILTER_SOURCE_FILES() -> list[str]: - """Filter out the noise transport when no ota entry configures encryption.""" + """Filter out the noise transport unless an ota entry configures + encryption or the api has a static key to offer.""" + if _api_static_key(CORE.config.get(CONF_API) or {}) is not None: + return [] for ota_conf in CORE.config.get(CONF_OTA, []): if ( ota_conf.get(CONF_PLATFORM) == CONF_ESPHOME @@ -299,6 +315,13 @@ async def to_code(config: ConfigType) -> None: if (encryption_conf := config.get(CONF_ENCRYPTION)) is not None: # A missing key was resolved from the api component in final validate. key = encryption_conf[CONF_KEY] + cg.add_define("USE_OTA_ENCRYPTION_REQUIRED") + else: + # With only an api key the device offers encryption but still accepts + # plaintext, so a firmware that predates the ota block can be replaced + # over OTA and the next upload with the block is encrypted. + key = _api_static_key(CORE.config.get(CONF_API) or {}) + if key is not None: cg.add_define("USE_OTA_ENCRYPTION") cg.add(var.set_noise_psk(list(decode_encryption_key(key)))) diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index 396a47bc52..b7ea0fa0af 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -106,7 +106,11 @@ void ESPHomeOTAComponent::dump_config() { #endif #ifdef USE_OTA_ENCRYPTION if (this->noise_ctx_.has_psk()) { - ESP_LOGCONFIG(TAG, " Encryption configured"); +#ifdef USE_OTA_ENCRYPTION_REQUIRED + ESP_LOGCONFIG(TAG, " Encryption: required"); +#else + ESP_LOGCONFIG(TAG, " Encryption: offered (api key), plaintext accepted"); +#endif } #endif #ifdef USE_OTA_PARTITIONS @@ -157,6 +161,11 @@ static constexpr uint8_t CLIENT_FEATURE_SUPPORTS_NOISE = 0x08; static constexpr uint8_t SERVER_FEATURE_SUPPORTS_COMPRESSION = 0x01; static constexpr uint8_t SERVER_FEATURE_SUPPORTS_PARTITION_ACCESS = 0x02; static constexpr uint8_t SERVER_FEATURE_SUPPORTS_NOISE = 0x04; +#ifdef USE_OTA_ENCRYPTION +// Noise needs the extended protocol: the prologue binds the 2-byte feature ack +static constexpr uint8_t CLIENT_NOISE_FEATURES = + CLIENT_FEATURE_SUPPORTS_NOISE | CLIENT_FEATURE_SUPPORTS_EXTENDED_PROTOCOL; +#endif void ESPHomeOTAComponent::handle_handshake_() { /// Handle the OTA handshake and authentication. @@ -241,12 +250,11 @@ void ESPHomeOTAComponent::handle_handshake_() { this->ota_features_ = this->handshake_buf_[0]; ESP_LOGV(TAG, "Features: 0x%02X", this->ota_features_); -#ifdef USE_OTA_ENCRYPTION - // Fail closed: with a PSK configured the client must negotiate encryption - // (which requires the extended protocol); refuse plaintext uploads. - static constexpr uint8_t NOISE_REQUIRED_FEATURES = - CLIENT_FEATURE_SUPPORTS_NOISE | CLIENT_FEATURE_SUPPORTS_EXTENDED_PROTOCOL; - if (this->noise_ctx_.has_psk() && (this->ota_features_ & NOISE_REQUIRED_FEATURES) != NOISE_REQUIRED_FEATURES) { +#ifdef USE_OTA_ENCRYPTION_REQUIRED + // Fail closed: an explicit `ota: encryption:` block means the client must + // negotiate encryption; refuse plaintext uploads. A build with only an api + // key offers encryption but lets plaintext through. + if (this->noise_ctx_.has_psk() && (this->ota_features_ & CLIENT_NOISE_FEATURES) != CLIENT_NOISE_FEATURES) { ESP_LOGW(TAG, "Client does not support encryption"); this->send_error_and_cleanup_(ota::OTA_RESPONSE_ERROR_ENCRYPTION_REQUIRED); return; @@ -289,10 +297,12 @@ void ESPHomeOTAComponent::handle_handshake_() { return; } #ifdef USE_OTA_ENCRYPTION - // With a PSK configured the rest of the session runs inside the noise - // transport; the client sends the first handshake frame next, so there - // is nothing to do until data arrives. - if (this->noise_ctx_.has_psk()) { + // When the client took the encryption offer the rest of the session runs + // inside the noise transport, which also authenticates it; the client + // sends the first handshake frame next, so there is nothing to do until + // data arrives. A client without both noise bits continues in plaintext + // (already rejected above when encryption is required). + if (this->noise_ctx_.has_psk() && (this->ota_features_ & CLIENT_NOISE_FEATURES) == CLIENT_NOISE_FEATURES) { // handshake_buf_ still holds the feature ack composed above; a // would-block re-entry lands here without rebuilding it if (!this->noise_start_session_(this->handshake_buf_[1])) { diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 526adf74f0..28ff9f3f2f 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -244,6 +244,7 @@ #define USE_RUNTIME_STATS #define USE_OTA #define USE_OTA_ENCRYPTION +#define USE_OTA_ENCRYPTION_REQUIRED #define USE_OTA_PASSWORD #define USE_OTA_VERSION 2 #define USE_TIME_TIMEZONE diff --git a/esphome/espota2.py b/esphome/espota2.py index ac4cbeeb7c..bfd257e2eb 100644 --- a/esphome/espota2.py +++ b/esphome/espota2.py @@ -537,9 +537,13 @@ def perform_ota( raise OTAError( "An OTA encryption key is configured but the device did not " "offer encryption; refusing to send the image in plaintext. " - "If the running firmware predates OTA encryption, first update " - "it without the 'ota: encryption:' block (over a trusted " - "network or via USB), then restore the block and upload again." + "The running firmware was built before OTA encryption " + "(ESPHome 2026.9.0) or without an 'api: encryption: key'. " + "Install once with the 'ota: encryption:' block removed and " + "the api key kept, that firmware offers encryption, then " + "restore the block and install again; every install after " + "that is encrypted. Use the web_server OTA platform or a " + "serial flash if the device cannot be reached that way." ) # The prologue binds every negotiation byte both sides saw, so any # tampering with the plaintext preamble breaks the handshake. diff --git a/tests/component_tests/ota/test_esphome_ota.py b/tests/component_tests/ota/test_esphome_ota.py index 873f162555..141ce17b05 100644 --- a/tests/component_tests/ota/test_esphome_ota.py +++ b/tests/component_tests/ota/test_esphome_ota.py @@ -2,6 +2,7 @@ from __future__ import annotations +from collections.abc import Callable import logging from typing import Any @@ -11,6 +12,7 @@ from esphome import config_validation as cv from esphome.components.esphome.ota import ( AUTO_LOAD, FILTER_SOURCE_FILES, + _api_static_key, _validate_no_password_with_encryption, ota_esphome_final_validate, ) @@ -386,6 +388,84 @@ def test_filter_source_files_excludes_noise_without_encryption() -> None: CORE.config = old_config +def test_filter_source_files_keeps_noise_for_static_api_key() -> None: + """A static api key makes the device offer encryption, so the transport + compiles even without an ota encryption block.""" + old_config = CORE.config + ota = [_make_ota_config(port=3232)] + try: + CORE.config = {CONF_API: {CONF_ENCRYPTION: {CONF_KEY: API_KEY}}, CONF_OTA: ota} + assert FILTER_SOURCE_FILES() == [] + # A runtime provisioned or all-zeros api key has nothing to offer + CORE.config = {CONF_API: {CONF_ENCRYPTION: {}}, CONF_OTA: ota} + assert FILTER_SOURCE_FILES() == ["ota_esphome_noise.cpp"] + CORE.config = { + CONF_API: {CONF_ENCRYPTION: {CONF_KEY: ZEROS_KEY}}, + CONF_OTA: ota, + } + assert FILTER_SOURCE_FILES() == ["ota_esphome_noise.cpp"] + CORE.config = {CONF_API: {}, CONF_OTA: ota} + assert FILTER_SOURCE_FILES() == ["ota_esphome_noise.cpp"] + finally: + CORE.config = old_config + + +def test_api_static_key() -> None: + """Only a real build-time api key can seed the encryption offer.""" + assert _api_static_key({}) is None + assert _api_static_key({CONF_ENCRYPTION: {}}) is None + assert _api_static_key({CONF_ENCRYPTION: {CONF_KEY: ZEROS_KEY}}) is None + assert _api_static_key({CONF_ENCRYPTION: {CONF_KEY: API_KEY}}) == API_KEY + + +def _defines() -> set[str]: + return {define.name for define in CORE.defines} + + +def test_api_key_offers_encryption_without_requiring_it( + generate_main: Callable[[str], str], +) -> None: + """An api key alone compiles the transport in and sets the psk, but the + device keeps accepting plaintext uploads.""" + main_cpp = generate_main( + "tests/component_tests/ota/test_esphome_ota_api_key_offer.yaml" + ) + assert "USE_OTA_ENCRYPTION" in _defines() + assert "USE_OTA_ENCRYPTION_REQUIRED" not in _defines() + assert "set_noise_psk(" in main_cpp + + +def test_api_key_offer_keeps_password(generate_main: Callable[[str], str]) -> None: + """A password still guards plaintext uploads on an offering device.""" + main_cpp = generate_main( + "tests/component_tests/ota/test_esphome_ota_api_key_offer_password.yaml" + ) + assert {"USE_OTA_ENCRYPTION", "USE_OTA_PASSWORD"} <= _defines() + assert "USE_OTA_ENCRYPTION_REQUIRED" not in _defines() + assert "set_noise_psk(" in main_cpp + assert "set_auth_password(" in main_cpp + + +def test_encryption_block_requires_encryption( + generate_main: Callable[[str], str], +) -> None: + """The ota encryption block is what makes the device refuse plaintext.""" + main_cpp = generate_main( + "tests/component_tests/ota/test_esphome_ota_encryption_required.yaml" + ) + assert {"USE_OTA_ENCRYPTION", "USE_OTA_ENCRYPTION_REQUIRED"} <= _defines() + assert "set_noise_psk(" in main_cpp + + +def test_runtime_api_key_offers_nothing(generate_main: Callable[[str], str]) -> None: + """A key provisioned at runtime is unknown at build time, so no offer.""" + main_cpp = generate_main( + "tests/component_tests/ota/test_esphome_ota_runtime_api_key.yaml" + ) + assert "USE_OTA_ENCRYPTION" not in _defines() + assert "set_noise_psk(" not in main_cpp + + def test_password_with_encryption_rejected() -> None: """The password and encryption options are mutually exclusive.""" config = {CONF_PASSWORD: "pw", CONF_ENCRYPTION: {CONF_KEY: API_KEY}} diff --git a/tests/component_tests/ota/test_esphome_ota_api_key_offer.yaml b/tests/component_tests/ota/test_esphome_ota_api_key_offer.yaml new file mode 100644 index 0000000000..ca26eb9f46 --- /dev/null +++ b/tests/component_tests/ota/test_esphome_ota_api_key_offer.yaml @@ -0,0 +1,11 @@ +esphome: + name: ota-offer + +host: + +api: + encryption: + key: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" + +ota: + - platform: esphome diff --git a/tests/component_tests/ota/test_esphome_ota_api_key_offer_password.yaml b/tests/component_tests/ota/test_esphome_ota_api_key_offer_password.yaml new file mode 100644 index 0000000000..1e23975690 --- /dev/null +++ b/tests/component_tests/ota/test_esphome_ota_api_key_offer_password.yaml @@ -0,0 +1,12 @@ +esphome: + name: ota-offer-password + +host: + +api: + encryption: + key: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" + +ota: + - platform: esphome + password: "superlongpasswordthatnoonewillknow" diff --git a/tests/component_tests/ota/test_esphome_ota_encryption_required.yaml b/tests/component_tests/ota/test_esphome_ota_encryption_required.yaml new file mode 100644 index 0000000000..36690038d8 --- /dev/null +++ b/tests/component_tests/ota/test_esphome_ota_encryption_required.yaml @@ -0,0 +1,12 @@ +esphome: + name: ota-encryption-required + +host: + +api: + encryption: + key: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" + +ota: + - platform: esphome + encryption: diff --git a/tests/component_tests/ota/test_esphome_ota_runtime_api_key.yaml b/tests/component_tests/ota/test_esphome_ota_runtime_api_key.yaml new file mode 100644 index 0000000000..8825335141 --- /dev/null +++ b/tests/component_tests/ota/test_esphome_ota_runtime_api_key.yaml @@ -0,0 +1,10 @@ +esphome: + name: ota-runtime-key + +host: + +api: + encryption: + +ota: + - platform: esphome diff --git a/tests/components/ota/api_key_offer.yaml b/tests/components/ota/api_key_offer.yaml new file mode 100644 index 0000000000..8d1814bf7e --- /dev/null +++ b/tests/components/ota/api_key_offer.yaml @@ -0,0 +1,12 @@ +wifi: + ssid: MySSID + password: password1 + +api: + encryption: + key: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" + +ota: + - platform: esphome + port: 3290 + password: "superlongpasswordthatnoonewillknow" diff --git a/tests/components/ota/test-api_key_offer.esp32-idf.yaml b/tests/components/ota/test-api_key_offer.esp32-idf.yaml new file mode 100644 index 0000000000..ecda625521 --- /dev/null +++ b/tests/components/ota/test-api_key_offer.esp32-idf.yaml @@ -0,0 +1,2 @@ +packages: + ota: !include api_key_offer.yaml diff --git a/tests/components/ota/test-api_key_offer.esp8266-ard.yaml b/tests/components/ota/test-api_key_offer.esp8266-ard.yaml new file mode 100644 index 0000000000..ecda625521 --- /dev/null +++ b/tests/components/ota/test-api_key_offer.esp8266-ard.yaml @@ -0,0 +1,2 @@ +packages: + ota: !include api_key_offer.yaml diff --git a/tests/integration/fixtures/host_ota_api_key_offer_with_password.yaml b/tests/integration/fixtures/host_ota_api_key_offer_with_password.yaml new file mode 100644 index 0000000000..1dedcc9ee1 --- /dev/null +++ b/tests/integration/fixtures/host_ota_api_key_offer_with_password.yaml @@ -0,0 +1,12 @@ +esphome: + name: host-ota-test +host: +api: + encryption: + key: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" +ota: + - platform: esphome + port: __OTA_PORT__ + password: "hunter2" +logger: + level: DEBUG diff --git a/tests/integration/fixtures/host_ota_api_key_offers_encryption.yaml b/tests/integration/fixtures/host_ota_api_key_offers_encryption.yaml new file mode 100644 index 0000000000..e846e4ea56 --- /dev/null +++ b/tests/integration/fixtures/host_ota_api_key_offers_encryption.yaml @@ -0,0 +1,11 @@ +esphome: + name: host-ota-test +host: +api: + encryption: + key: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" +ota: + - platform: esphome + port: __OTA_PORT__ +logger: + level: DEBUG diff --git a/tests/integration/test_host_ota.py b/tests/integration/test_host_ota.py index 4e74814534..feaa97bdfe 100644 --- a/tests/integration/test_host_ota.py +++ b/tests/integration/test_host_ota.py @@ -168,6 +168,143 @@ async def test_host_ota_encrypted( assert proc.pid == pid_before +class _RebootCounter: + """Counts safe reboots so a test can wait for the nth one.""" + + def __init__(self) -> None: + self._seen = asyncio.Event() + self.count = 0 + + def on_log(self, line: str) -> None: + if "Rebooting safely" in line: + self.count += 1 + self._seen.set() + + async def wait(self, count: int, timeout: float = 10.0) -> None: + async with asyncio.timeout(timeout): + while self.count < count: + self._seen.clear() + await self._seen.wait() + + +@pytest.mark.asyncio +async def test_host_ota_api_key_offers_encryption( + yaml_config: str, + write_yaml_config: ConfigWriter, + compile_esphome: CompileFunction, + reserved_tcp_port: tuple[int, socket.socket], +) -> None: + """With only an api key the device takes both a plaintext upload and an + encrypted one using that key, which is the enablement path for + `ota: encryption:`.""" + pytest.importorskip("aioesphomeapi.noise") + api_key = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" + api_port, api_socket = reserved_tcp_port + with _reserve_port() as (ota_port, ota_socket): + yaml_config = yaml_config.replace("__OTA_PORT__", str(ota_port)) + config_path = await write_yaml_config(yaml_config) + binary_path = await compile_esphome(config_path) + api_socket.close() + ota_socket.close() + + loop = asyncio.get_running_loop() + reboots = _RebootCounter() + + async with run_binary(binary_path, line_callback=reboots.on_log) as ( + proc, + lines, + ): + await _wait_for_port(LOCALHOST, api_port, PORT_WAIT_TIMEOUT) + pid_before = proc.pid + + rc, _ = await loop.run_in_executor( + None, espota2.run_ota, LOCALHOST, ota_port, None, binary_path + ) + assert rc == 0, "plaintext upload to an offering device must succeed" + await reboots.wait(1) + await _wait_for_port(LOCALHOST, api_port, PORT_WAIT_TIMEOUT) + assert proc.pid == pid_before + + rc, _ = await loop.run_in_executor( + None, + functools.partial( + espota2.run_ota, + LOCALHOST, + ota_port, + None, + binary_path, + noise_psk=api_key, + ), + ) + assert rc == 0, "encrypted upload with the api key must succeed" + await reboots.wait(2) + await _wait_for_port(LOCALHOST, api_port, PORT_WAIT_TIMEOUT) + assert proc.returncode is None, "process exited instead of execing" + assert proc.pid == pid_before + assert any("Encryption: offered" in line for line in lines) + + +@pytest.mark.asyncio +async def test_host_ota_api_key_offer_with_password( + yaml_config: str, + write_yaml_config: ConfigWriter, + compile_esphome: CompileFunction, + reserved_tcp_port: tuple[int, socket.socket], +) -> None: + """The OTA password still guards plaintext uploads on an offering device + while the api key alone authenticates an encrypted one.""" + pytest.importorskip("aioesphomeapi.noise") + api_key = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=" + api_port, api_socket = reserved_tcp_port + with _reserve_port() as (ota_port, ota_socket): + yaml_config = yaml_config.replace("__OTA_PORT__", str(ota_port)) + config_path = await write_yaml_config(yaml_config) + binary_path = await compile_esphome(config_path) + api_socket.close() + ota_socket.close() + + loop = asyncio.get_running_loop() + reboots = _RebootCounter() + + async with run_binary(binary_path, line_callback=reboots.on_log) as ( + proc, + _lines, + ): + await _wait_for_port(LOCALHOST, api_port, PORT_WAIT_TIMEOUT) + pid_before = proc.pid + + rc, _ = await loop.run_in_executor( + None, espota2.run_ota, LOCALHOST, ota_port, None, binary_path + ) + assert rc == 1, "plaintext upload without the password must fail" + await asyncio.sleep(0.5) + assert proc.returncode is None, "process died on rejected upload" + + rc, _ = await loop.run_in_executor( + None, espota2.run_ota, LOCALHOST, ota_port, "hunter2", binary_path + ) + assert rc == 0, "plaintext upload with the password must succeed" + await reboots.wait(1) + await _wait_for_port(LOCALHOST, api_port, PORT_WAIT_TIMEOUT) + assert proc.pid == pid_before + + rc, _ = await loop.run_in_executor( + None, + functools.partial( + espota2.run_ota, + LOCALHOST, + ota_port, + None, + binary_path, + noise_psk=api_key, + ), + ) + assert rc == 0, "encrypted upload with the api key must succeed" + await reboots.wait(2) + await _wait_for_port(LOCALHOST, api_port, PORT_WAIT_TIMEOUT) + assert proc.pid == pid_before + + @pytest.mark.asyncio async def test_host_ota_rejects_garbage( yaml_config: str, diff --git a/tests/unit_tests/test_espota2_noise.py b/tests/unit_tests/test_espota2_noise.py index 5b43d05530..68cb2b7d3a 100644 --- a/tests/unit_tests/test_espota2_noise.py +++ b/tests/unit_tests/test_espota2_noise.py @@ -10,6 +10,7 @@ when the installed aioesphomeapi predates the noise module. from __future__ import annotations import base64 +from collections.abc import Callable import hashlib import io from pathlib import Path @@ -109,8 +110,17 @@ class FakeEncryptedDevice(threading.Thread): return server_flags = espota2.SERVER_FEATURE_SUPPORTS_NOISE if self.offer_noise else 0 sock.sendall(bytes([espota2.RESPONSE_FEATURE_FLAGS, server_flags])) - if not (self.offer_noise and noise_negotiated): + if noise_negotiated and not self.offer_noise: return # the client fails closed; nothing further arrives + if not noise_negotiated: + # A device that does not require encryption lets a plaintext + # client through + self._transfer( + lambda byte: sock.sendall(bytes([byte])), + lambda length: _recv_exact(sock, length), + lambda remaining: sock.recv(min(remaining, 4096)), + ) + return from cryptography.exceptions import InvalidTag from noise.connection import NoiseConnection @@ -149,6 +159,20 @@ class FakeEncryptedDevice(threading.Thread): assert len(plaintext) == length, "control units must be one per frame" return plaintext + def recv_data(remaining: int) -> bytes: + plaintext = proto.decrypt(_recv_frame(sock)) + assert 0 < len(plaintext) <= espota2.NOISE_MAX_PLAINTEXT + return plaintext + + self._transfer(send_byte, recv_unit, recv_data) + + def _transfer( + self, + send_byte: Callable[[int], None], + recv_unit: Callable[[int], bytes], + recv_data: Callable[[int], bytes], + ) -> None: + """The post-handshake exchange, identical over both transports.""" send_byte(espota2.RESPONSE_AUTH_OK) recv_unit(1) # ota type size = int.from_bytes(recv_unit(4), "big") @@ -159,9 +183,9 @@ class FakeEncryptedDevice(threading.Thread): received = b"" acked = 0 while len(received) < size: - plaintext = proto.decrypt(_recv_frame(sock)) - assert 0 < len(plaintext) <= espota2.NOISE_MAX_PLAINTEXT - received += plaintext + chunk = recv_data(size - len(received)) + assert chunk, "client closed mid-transfer" + received += chunk if self.version >= espota2.OTA_VERSION_2_0: while acked + espota2.UPLOAD_BLOCK_SIZE <= len(received) or ( len(received) == size and acked < size @@ -240,6 +264,29 @@ def test_client_fails_closed_when_device_lacks_encryption() -> None: device.join_and_check() +def test_plaintext_client_accepted_by_offering_device() -> None: + """A device that offers but does not require encryption still takes a + plaintext upload from a client with no key configured.""" + firmware = bytes(range(256)) * 40 + device = FakeEncryptedDevice(offer_noise=True, require_noise=False) + with patch("time.sleep"): + _upload(device, firmware, None) + device.join_and_check() + assert device.received == firmware + + +def test_keyed_client_encrypts_with_offering_device() -> None: + """The upload that turns on `ota: encryption:` is already encrypted when + the running firmware offers it.""" + pytest.importorskip("aioesphomeapi.noise") + firmware = bytes(range(256)) * 40 + device = FakeEncryptedDevice(offer_noise=True, require_noise=False) + with patch("time.sleep"): + _upload(device, firmware, PSK) + device.join_and_check() + assert device.received == firmware + + def test_plaintext_client_gets_encryption_required_error() -> None: """A client without a key gets the device's 0x94 error message.""" device = FakeEncryptedDevice()