fix(hue): stop leaking entertainment activation, and make the re-pair signal reachable on CLIP v2 - #174
Merged
Merged
Conversation
… signal reachable on CLIP v2
Five defects found by a read-only audit of the Hue path.
1. A failed DTLS bring-up left the area activated. `spawn_hue_dtls_sender`
activated entertainment mode, then propagated any `connect_dtls` error with
`?` — the sender thread that owns cleanup never spawned, `uses_dtls` was
recorded as false, and every later cleanup site filters on that flag, so the
stop PUT was never sent for the rest of the process lifetime. The bridge kept
`active_streamer` and the next start failed its readiness gate. Activation is
now rolled back through the dedupe token on that path.
2. The re-pair classifier could not match any CLIP v2 body. It required a
top-level array with `error.type == 1`, which is the v1 envelope; v2 returns
`{"errors":[{"description":...}]}` with no `type`. Every `/clip/v2/*` caller
was therefore unable to reach `AuthInvalid`, so a revoked application key
surfaced as a retryable transient fault and the user was told to keep
retrying instead of re-pairing. The classifier now also accepts the v2 shape,
and 401 joins 403 as an auth-bearing status. Non-Hue bodies still map to
`Transient`, so a reverse-proxy 401 cannot trigger a re-pair.
3. `validate_hue_credentials` collapsed `AuthInvalid` into the transport arm, so
a bridge that explicitly rejected the key was reported to the user as
"bridge offline". It now returns `HUE_CREDENTIAL_INVALID`.
4. The HTTPS→HTTP fallback added for Bridge Pro (#167) applied to the pairing
POST, whose response carries the DTLS `clientkey`. An attacker who blackholed
TCP/443 could force the downgrade and read the PSK. The fallback is now
opt-in per call site, off for pairing and credential validation, and only
fires on connect-level failures — a TLS handshake failure is exactly what an
attacker would manufacture, so it stays fatal.
5. Cloud discovery used the bridge client, which disables certificate
verification for the bridge's self-signed cert. `discovery.meethue.com` is a
public CA-signed endpoint that tells us which IP to trust, so it now uses a
verifying client.
Four new tests cover the v2 classifier in both directions.
… stop faking a 3 s shutdown Two more from the same audit. get_hue_area_channels was the one bridge call in commands/hue/ that used error_for_status() instead of the shared classifier, so a 403 became a raw reqwest string rather than AUTH_INVALID_RE_PAIR_REQUIRED. The frontend catch turns any failure there into an empty channel list, so an expired application key rendered as 'this area has no channels' — no error, no re-pair prompt. Every no-op sender returned a fresh shutdown signal that nothing could ever fire, so once one was stored, every stop_hue_stream blocked the full 3 s timeout and reported HUE_STOP_TIMEOUT_PARTIAL with a Retry hint for a stream that never started. They now return a pre-signalled one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five defects from a read-only audit of the Hue path. All independently verified against the source before patching.
1. Failed DTLS bring-up left the area activated — the phantom-streamer source
sender.rs:311activates entertainment mode,:314then propagated anyconnect_dtlserror with?. The sender thread that owns the cleanup never spawns,uses_dtlsis recorded false, and every later cleanup site filters on that flag (commands.rs:286,:358,reconnect.rs:359) — so the stop PUT is never sent for the remaining process lifetime. The bridge holdsactive_streamerand the next start fails its readiness gate withHUE_STREAM_NOT_READY_ACTIVE_STREAMER.Activation is now rolled back through the existing dedupe token on that path.
2. The re-pair classifier could not match any CLIP v2 body
hue_http.rsrequired a top-level array whose first element haserror.type == 1— the CLIP v1 envelope. CLIP v2 returns an object:{"errors":[{"description":"..."}]}, notypefield.Consequence:
fetch_entertainment_payloadandfetch_room_payloadcould never produceAuthInvalid, soAUTH_INVALID_RE_PAIR_REQUIREDwas dead on every/clip/v2/*path. A revoked application key surfaced asHUE_AREA_LIST_FAILED, which routes to the transient retry ladder — the user was told to retry forever instead of re-pair.The contract was under-triggering, not over-triggering. Fixed by accepting the v2 shape as well, and adding 401 alongside 403. The whitelist discipline is preserved: a non-Hue body (reverse proxy, captive portal) still maps to
Transient, so nobody gets nudged into an unnecessary re-pair.3. An explicit auth rejection was reported as "bridge offline"
validate_hue_credentialsdidErr(fault) => Err(fault.to_string()), erasing the fault type, then collapsed everything intoHUE_CREDENTIAL_CHECK_FAILED. The frontend maps that tobridgeOfflineTitle. Now returnsHUE_CREDENTIAL_INVALID— contract-neutral, the code already existed.4. The Bridge Pro HTTP fallback was a PSK downgrade vector
The fallback added in #169 applied to the pairing POST, whose response carries the DTLS
clientkey. An on-path attacker who blackholes TCP/443 forces the downgrade and reads the PSK in cleartext.The fallback is now opt-in per call site — off for pairing and credential validation, on only for
/api/config— and fires only onis_connect()failures. A TLS handshake failure is precisely what an attacker manufactures, so it stays fatal instead of downgrading.5. Cloud discovery ran without certificate verification
hue_http_client()disables verification for the bridge's self-signed certificate, and the same client was used forhttps://discovery.meethue.com/— a public CA-signed endpoint whose whole job is telling us which IP to trust. Split into a verifyinghue_cloud_http_client().Validation
cargo fmt --check,cargo clippy --all-targets --all-features -D warnings,cargo test --all-features -- --test-threads=1(315 + 32, up from 311 + 28). Four new tests cover the v2 classifier in both directions, including that a non-Hue 401 body staysTransient.Not covered here
Two findings from the same audit are deferred: the reconnect monitor dead-ending after one failed readiness check (
reconnect.rs:399), and the HTTP fallback sender exceeding the bridge's per-light rate budget by roughly 20× (sender.rs:467). Both need more thought than a mechanical patch.None of this is verified against real hardware. The v2 body shape is settled by the spec; whether a revoked key returns 401 or 403, and the exact
descriptionwording, is not.