mirror of
https://github.com/esphome/esphome.git
synced 2026-07-10 08:55:36 +00:00
Apply review cleanups: hot path branch placement, shared zero sentinel, provisioning gates that survive plaintext removal
This commit is contained in:
@@ -210,10 +210,10 @@ void APIConnection::upgrade_helper_to_noise_() {
|
||||
uint8_t header[3];
|
||||
uint8_t header_len = plaintext->get_consumed_header(header);
|
||||
auto *noise = new APINoiseFrameHelper(plaintext->release_socket_for_switch(), this->parent_->get_noise_ctx());
|
||||
// Carry over the peername-based client name (Hello has not arrived yet)
|
||||
const char *name = plaintext->get_client_name();
|
||||
noise->set_client_name(name, strlen(name));
|
||||
this->helper_.reset(noise); // destroys the plaintext helper
|
||||
// Restore the peername-based client name (Hello has not arrived yet)
|
||||
char peername[socket::SOCKADDR_STR_LEN];
|
||||
noise->set_client_name(noise->get_peername_to(peername), strlen(peername));
|
||||
APIError err = noise->init_from_handoff(header, header_len);
|
||||
if (err != APIError::OK) {
|
||||
this->fatal_error_with_log_(LOG_STR("Noise handoff failed"), err);
|
||||
@@ -278,12 +278,16 @@ void APIConnection::loop() {
|
||||
if (err == APIError::WOULD_BLOCK) {
|
||||
// No more data available
|
||||
break;
|
||||
#if defined(USE_API_NOISE) && defined(USE_API_PLAINTEXT)
|
||||
} else if (err == APIError::PROTOCOL_SWITCH_TO_NOISE) {
|
||||
this->upgrade_helper_to_noise_();
|
||||
return;
|
||||
#endif
|
||||
} else if (err != APIError::OK) {
|
||||
#if defined(USE_API_NOISE) && defined(USE_API_PLAINTEXT)
|
||||
// Checked inside the error branch to keep the hot err == OK path
|
||||
// free of it; this can only fire on the first bytes of a plaintext
|
||||
// helper on an unprovisioned device
|
||||
if (err == APIError::PROTOCOL_SWITCH_TO_NOISE) {
|
||||
this->upgrade_helper_to_noise_();
|
||||
return;
|
||||
}
|
||||
#endif
|
||||
this->fatal_error_with_log_(LOG_STR("Reading failed"), err);
|
||||
return;
|
||||
} else {
|
||||
@@ -1888,10 +1892,10 @@ bool APIConnection::send_device_info_response_() {
|
||||
#endif
|
||||
#ifdef USE_API_NOISE
|
||||
resp.api_encryption_supported = true;
|
||||
#ifdef USE_API_PLAINTEXT
|
||||
// Dual build (encryption compiled in, no key from YAML): while no key is
|
||||
// set, the key can be provisioned over a zero-PSK Noise connection instead
|
||||
// of plaintext
|
||||
#ifndef USE_API_NOISE_PSK_FROM_YAML
|
||||
// No key from YAML: while no key is set, the key can be provisioned over a
|
||||
// zero-PSK Noise connection. Gated on the YAML define (not the plaintext
|
||||
// one) so this advertisement survives the plaintext removal in 2027.2.0.
|
||||
resp.api_encryption_provisionable = !this->parent_->get_noise_ctx().has_psk();
|
||||
#endif
|
||||
#endif
|
||||
@@ -2071,10 +2075,9 @@ bool APIConnection::send_noise_encryption_set_key_response_(const NoiseEncryptio
|
||||
}
|
||||
} else if (base64_decode(msg.key, msg.key_len, psk.data(), psk.size()) != psk.size()) {
|
||||
ESP_LOGW(TAG, "Invalid encryption key length");
|
||||
} else if (psk == psk_t{}) {
|
||||
// The all-zeros key is reserved: it marks the device as unprovisioned and
|
||||
// serves as the well-known provisioning PSK. Accepting it would report
|
||||
// success without enabling encryption (or silently clear an existing key).
|
||||
} else if (APINoiseContext::is_all_zeros(psk)) {
|
||||
// Accepting the reserved provisioning PSK would report success without
|
||||
// enabling encryption (or silently clear an existing key)
|
||||
ESP_LOGW(TAG, "Rejecting all-zero encryption key");
|
||||
} else if (!this->parent_->save_noise_psk(psk, true)) {
|
||||
ESP_LOGW(TAG, "Failed to save encryption key");
|
||||
@@ -2084,8 +2087,7 @@ bool APIConnection::send_noise_encryption_set_key_response_(const NoiseEncryptio
|
||||
if (this->helper_->frame_footer_size() == 0) {
|
||||
// Plaintext transport has no frame footer; Noise always has the MAC footer.
|
||||
// Remove after 2027.2.0 together with plaintext support on keyless devices.
|
||||
ESP_LOGW(TAG, "Key was received on an unencrypted connection; this is deprecated and will stop "
|
||||
"working in 2027.2.0. Update Home Assistant to provision the key encrypted");
|
||||
ESP_LOGW(TAG, "Key received over plaintext; deprecated, will be removed in 2027.2.0");
|
||||
}
|
||||
#endif
|
||||
}
|
||||
|
||||
@@ -97,11 +97,8 @@ const LogString *api_error_to_logstr(APIError err) {
|
||||
return LOG_STR("BAD_HANDSHAKE_ERROR_BYTE");
|
||||
}
|
||||
#endif
|
||||
#if defined(USE_API_NOISE) && defined(USE_API_PLAINTEXT)
|
||||
else if (err == APIError::PROTOCOL_SWITCH_TO_NOISE) {
|
||||
return LOG_STR("PROTOCOL_SWITCH_TO_NOISE");
|
||||
}
|
||||
#endif
|
||||
// PROTOCOL_SWITCH_TO_NOISE is intercepted in APIConnection::loop() before
|
||||
// any logging can happen, so it intentionally has no entry here.
|
||||
return LOG_STR("UNKNOWN");
|
||||
}
|
||||
|
||||
|
||||
@@ -10,13 +10,20 @@ using psk_t = std::array<uint8_t, 32>;
|
||||
|
||||
class APINoiseContext {
|
||||
public:
|
||||
// The all-zeros PSK is reserved: it marks the device as unprovisioned and
|
||||
// doubles as the well-known provisioning PSK that unprovisioned devices
|
||||
// accept for Noise handshakes (passive-sniffing protection only, no
|
||||
// authentication). It is never a valid real key.
|
||||
static bool is_all_zeros(const psk_t &psk) {
|
||||
uint8_t acc = 0;
|
||||
for (uint8_t b : psk) {
|
||||
acc |= b;
|
||||
}
|
||||
return acc == 0;
|
||||
}
|
||||
void set_psk(psk_t psk) {
|
||||
this->psk_ = psk;
|
||||
bool has_psk = false;
|
||||
for (auto i : psk) {
|
||||
has_psk |= i;
|
||||
}
|
||||
this->has_psk_ = has_psk;
|
||||
this->has_psk_ = !is_all_zeros(psk);
|
||||
}
|
||||
const psk_t &get_psk() const { return this->psk_; }
|
||||
bool has_psk() const { return this->has_psk_; }
|
||||
|
||||
@@ -107,9 +107,10 @@ void MDNSComponent::compile_records_(StaticVector<MDNSService, MDNS_SERVICE_COUN
|
||||
txt_count++; // network
|
||||
#endif
|
||||
#ifdef USE_API_NOISE
|
||||
const bool api_has_psk = api::global_api_server->get_noise_ctx().has_psk();
|
||||
txt_count++; // api_encryption or api_encryption_supported
|
||||
#ifdef USE_API_PLAINTEXT
|
||||
if (!api::global_api_server->get_noise_ctx().has_psk()) {
|
||||
#ifndef USE_API_NOISE_PSK_FROM_YAML
|
||||
if (!api_has_psk) {
|
||||
txt_count++; // api_provisioning
|
||||
}
|
||||
#endif
|
||||
@@ -168,13 +169,13 @@ void MDNSComponent::compile_records_(StaticVector<MDNSService, MDNS_SERVICE_COUN
|
||||
MDNS_STATIC_CONST_CHAR(TXT_API_ENCRYPTION, "api_encryption");
|
||||
MDNS_STATIC_CONST_CHAR(TXT_API_ENCRYPTION_SUPPORTED, "api_encryption_supported");
|
||||
MDNS_STATIC_CONST_CHAR(NOISE_ENCRYPTION, "Noise_NNpsk0_25519_ChaChaPoly_SHA256");
|
||||
bool has_psk = api::global_api_server->get_noise_ctx().has_psk();
|
||||
const char *encryption_key = has_psk ? TXT_API_ENCRYPTION : TXT_API_ENCRYPTION_SUPPORTED;
|
||||
const char *encryption_key = api_has_psk ? TXT_API_ENCRYPTION : TXT_API_ENCRYPTION_SUPPORTED;
|
||||
txt_records.push_back({MDNS_STR(encryption_key), MDNS_STR(NOISE_ENCRYPTION)});
|
||||
#ifdef USE_API_PLAINTEXT
|
||||
if (!has_psk) {
|
||||
// Unprovisioned dual-build device: advertise that the encryption key can
|
||||
// be provisioned over a zero-PSK Noise connection (no plaintext needed)
|
||||
#ifndef USE_API_NOISE_PSK_FROM_YAML
|
||||
if (!api_has_psk) {
|
||||
// Unprovisioned device without a YAML key: advertise that the encryption
|
||||
// key can be provisioned over a zero-PSK Noise connection. Gated on the
|
||||
// YAML define so this survives the plaintext removal in 2027.2.0.
|
||||
MDNS_STATIC_CONST_CHAR(TXT_API_PROVISIONING, "api_provisioning");
|
||||
MDNS_STATIC_CONST_CHAR(VALUE_ZERO_PSK, "zero-psk");
|
||||
txt_records.push_back({MDNS_STR(TXT_API_PROVISIONING), MDNS_STR(VALUE_ZERO_PSK)});
|
||||
|
||||
@@ -1,6 +0,0 @@
|
||||
esphome:
|
||||
name: zero-psk-rejects-test
|
||||
host:
|
||||
api:
|
||||
encryption:
|
||||
logger:
|
||||
@@ -36,8 +36,38 @@ async def test_api_zero_psk_provisioning(
|
||||
run_compiled: RunCompiledFunction,
|
||||
api_client_connected: APIClientConnectedFactory,
|
||||
) -> None:
|
||||
"""Provision a key over the zero-PSK channel and verify the switch to it."""
|
||||
"""Exercise the reject paths, then provision a key over the zero-PSK channel."""
|
||||
async with run_compiled(yaml_config):
|
||||
# --- Pre-provisioning reject paths (device state is unchanged) ---
|
||||
|
||||
# A wrong (non-zero) PSK fails against the zero provisioning PSK
|
||||
with pytest.raises(InvalidEncryptionKeyAPIError):
|
||||
async with api_client_connected(
|
||||
noise_psk=base64.b64encode(b"w" * 32).decode(), timeout=5
|
||||
) as client:
|
||||
await client.device_info()
|
||||
|
||||
# A plaintext client and a zero-PSK client can be connected at the
|
||||
# same time while the device is unprovisioned
|
||||
async with (
|
||||
api_client_connected() as plaintext_client,
|
||||
api_client_connected(noise_psk=ZERO_PSK) as noise_client,
|
||||
):
|
||||
plaintext_info = await plaintext_client.device_info()
|
||||
noise_info = await noise_client.device_info()
|
||||
# Both transports advertise provisioning support so old and new
|
||||
# clients can decide how to provision
|
||||
assert plaintext_info.api_encryption_provisionable is True
|
||||
assert noise_info.api_encryption_provisionable is True
|
||||
|
||||
# The all-zeros key is reserved as the provisioning PSK and is
|
||||
# rejected on both transports
|
||||
zero_key = base64.b64encode(bytes(32))
|
||||
assert await noise_client.noise_encryption_set_key(zero_key) is False
|
||||
assert await plaintext_client.noise_encryption_set_key(zero_key) is False
|
||||
|
||||
# --- Provision over the zero-PSK channel ---
|
||||
|
||||
# The unprovisioned device accepts the all-zeros PSK; the handshake's
|
||||
# ephemeral-ephemeral DH encrypts everything that follows
|
||||
async with api_client_connected(noise_psk=ZERO_PSK) as client:
|
||||
@@ -68,45 +98,6 @@ async def test_api_zero_psk_provisioning(
|
||||
await client.device_info()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_api_zero_psk_provisioning_rejects(
|
||||
yaml_config: str,
|
||||
run_compiled: RunCompiledFunction,
|
||||
api_client_connected: APIClientConnectedFactory,
|
||||
) -> None:
|
||||
"""Exercise the paths that must not provision a key."""
|
||||
async with run_compiled(yaml_config):
|
||||
# A wrong (non-zero) PSK fails against the zero provisioning PSK
|
||||
with pytest.raises(InvalidEncryptionKeyAPIError):
|
||||
async with api_client_connected(
|
||||
noise_psk=base64.b64encode(b"w" * 32).decode(), timeout=5
|
||||
) as client:
|
||||
await client.device_info()
|
||||
|
||||
# A plaintext client and a zero-PSK client can be connected at the
|
||||
# same time while the device is unprovisioned
|
||||
async with (
|
||||
api_client_connected() as plaintext_client,
|
||||
api_client_connected(noise_psk=ZERO_PSK) as noise_client,
|
||||
):
|
||||
plaintext_info = await plaintext_client.device_info()
|
||||
noise_info = await noise_client.device_info()
|
||||
# Both transports advertise provisioning support so old and new
|
||||
# clients can decide how to provision
|
||||
assert plaintext_info.api_encryption_provisionable is True
|
||||
assert noise_info.api_encryption_provisionable is True
|
||||
|
||||
# The all-zeros key is reserved as the provisioning PSK and is
|
||||
# rejected on both transports
|
||||
zero_key = base64.b64encode(bytes(32))
|
||||
assert await noise_client.noise_encryption_set_key(zero_key) is False
|
||||
assert await plaintext_client.noise_encryption_set_key(zero_key) is False
|
||||
|
||||
# Nothing was provisioned: the zero PSK still works
|
||||
async with api_client_connected(noise_psk=ZERO_PSK) as client:
|
||||
assert (await client.device_info()).api_encryption_provisionable is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_api_zero_psk_provisioning_plaintext(
|
||||
yaml_config: str,
|
||||
@@ -125,7 +116,7 @@ async def test_api_zero_psk_provisioning_plaintext(
|
||||
await asyncio.sleep(KEY_ACTIVATION_DELAY)
|
||||
|
||||
# The deprecation warning was logged
|
||||
assert any("unencrypted connection" in line for line in log_lines)
|
||||
assert any("deprecated" in line for line in log_lines)
|
||||
|
||||
# The new key works; the zero PSK does not
|
||||
async with api_client_connected(noise_psk=NEW_KEY.decode()) as client:
|
||||
|
||||
Reference in New Issue
Block a user