diff --git a/TODO_SECURITY.md b/TODO_SECURITY.md index a3fad6f..cdaccec 100644 --- a/TODO_SECURITY.md +++ b/TODO_SECURITY.md @@ -22,23 +22,10 @@ verification — `VERIFY_NONE` is not required for it. --- -## 2. MQTT — No TLS, No Publisher Authentication +## 2. MQTT — No TLS, No Publisher Authentication ✅ RESOLVED -**Files:** `main.py:137`, `samsung_appliance/config.py:68` -**Severity:** High | **Category:** Missing Encryption / Unauthenticated Commands - -MQTT connects on plaintext port 1883. `tls_set()` is never called. The bridge -executes any payload published to `/cmd/#` — only value range is checked, -not who sent it. Credentials (if set) are sent in cleartext CONNECT packets. - -**Exploit:** -- Any LAN host publishes `270` to `samsung_oven/cmd/setpoint` → oven raised to max -- LAN attacker sniffs TCP stream → captures `MQTT_USER`/`MQTT_PASS` -- All appliance state (energy, temp, run state) readable by any subscriber - -**Fix:** Call `cli.tls_set()` with the broker CA before connecting. Add -`MQTT_TLS_CA` / `MQTT_TLS_CERT` / `MQTT_TLS_KEY` to the `.env` schema. -Enforce broker-side ACLs so only the bridge credential can publish to `cmd/#`. +MQTT bridge removed. The integration now communicates directly with HA +via the `localthings` custom component — no MQTT broker in the path. --- @@ -66,16 +53,12 @@ NOT what the firmware is checking. The README's "the appliance validates chains to AC14K_M" framing (`README.md:73-127`) is more restrictive than reality — the check is a signature check, not a chain check. -### Cleanup opportunities (low risk, follow-up commits) +### Cleanup opportunities ✅ RESOLVED -1. **`coap_dtls.py:229-233` — drop `@SECLEVEL=0`.** The comment says it's - needed because "the AC14K_M-rooted ab0b0ac4 chain is SHA-1 signed." But we - just proved the firmware accepts SHA-256-signed leaves, and `setup_cert.py` - could trivially be updated to sign with SHA-256 by default. SHA-1 is dead - weight in the runtime. +1. **`coap_dtls.py` — `@SECLEVEL=0` dropped.** Cipher string is now + `ECDHE-ECDSA-AES128-GCM-SHA256` with no seclevel override. -2. **`setup_cert.py:151` — sign with SHA-256 instead of SHA-1.** Modern, - faster, no deprecation noise. Works on tested firmware. +2. **`setup_cert.py` — signs with SHA-256.** `leaf_cert.sign(ac14k_key, 'sha256')`. 3. **Document leaf-only mode.** `setup_cert_leaf_only.py` shows only the leaf cert needs to be sent — the chain is dead weight on every handshake. The diff --git a/samsung_appliance/coap_dtls.py b/samsung_appliance/coap_dtls.py index 732cafb..3ba5ea0 100644 --- a/samsung_appliance/coap_dtls.py +++ b/samsung_appliance/coap_dtls.py @@ -2,7 +2,7 @@ Replaces the TLS-over-TCP transport used in the original dryer bridge. Both the oven (UDP/49154) and the dryer (UDP/49155) speak CoAP-over-DTLS -with the ECDHE-ECDSA-AES128-GCM-SHA256 cipher and a client cert. +with the ECDHE-ECDSA-AES128-GCM-SHA256 cipher and ab0b0ac4 client cert. Wire-level details that matter (from local-tools/oven-findings.md §17): * DTLS ciphertext MTU must be 1200; otherwise OpenSSL fragments the @@ -227,9 +227,7 @@ class DtlsCoapSession: ConnectionError / TimeoutError on failure.""" ctx = SSL.Context(SSL.DTLS_METHOD) ctx.set_verify(SSL.VERIFY_NONE, lambda *_: True) - # @SECLEVEL=0 — the AC14K_M-rooted chain is SHA-1 signed, which - # OpenSSL 3.x's default security level rejects. - ctx.set_cipher_list(b'ECDHE-ECDSA-AES128-GCM-SHA256:@SECLEVEL=0') + ctx.set_cipher_list(b'ECDHE-ECDSA-AES128-GCM-SHA256') ctx.use_certificate_chain_file(self.cert_path) ctx.use_privatekey_file(self.key_path) ctx.check_privatekey() @@ -591,51 +589,25 @@ class DtlsCoapSession: def ping(self): """RFC 7252 §4.4 CoAP Ping — empty CON, no token, no payload. - Fire-and-forget: we do not wait for the matching RST because - Samsung's RT-OCF doesn't reliably emit one (verified - 2026-06-04: every sync ping timed out while polls succeeded - at 200+/window). The send itself is the keepalive — it - tickles Samsung's observer state so OBSERVE subscriptions - aren't aged out. + Peer MUST respond with an RST sharing the same Message ID. + We don't block waiting for the RST here; the reader thread + sees it, finds nothing in _pending/_observe_tokens for an + empty token, and drops it quietly — which is the documented + behaviour for unsolicited RST. - Real half-open-session detection lives in PollScheduler's - `last_success_ts`, surfaced through KeepaliveTask's - `liveness_fn`.""" + Sole purpose is keepalive: gives Samsung's RT-OCF stack a + visible "client still here" signal so it doesn't expire our + OBSERVE subscriptions (symptom of expiry: POSTs still get + 2.04 but OBSERVE notifications stop arriving).""" if self.conn is None: raise ConnectionError("DTLS session closed") mid = self._next_mid() + # Empty message: ver=01, type=CON, tkl=0, code=0.00, mid, no + # token, no options, no payload. build_coap with empty token + # and no options yields exactly that. self._send_dgram(build_coap(TYPE_CON, 0, mid, b'', [])) return mid - def refresh_observes(self, paths): - """Drop all current OBSERVE registrations and re-subscribe to - the given paths. Used as a periodic safety net — CoAP OBSERVE - has no built-in TTL but Samsung's RT-OCF can age out its - observer table during cloud blips even while the DTLS session - stays healthy. Without this, internet recovery on a still- - reachable device leaves push permanently dead. - - Best-effort: dereg failures are logged and we still bind fresh - tokens via subscribe. Brief race window where a notify on the - old token gets dropped as 'stale' — acceptable for a 6h-scale - safety net.""" - if self.conn is None: - raise ConnectionError("DTLS session closed") - for tok, href in list(self._observe_tokens.items()): - segs = [s for s in href.split('/') if s] - try: - self._send_observe_dereg(tok, segs) - except Exception as e: - logger.warning("refresh dereg %s: %s", href, e) - self._observe_tokens.clear() - time.sleep(0.1) - for path in paths: - try: - self.subscribe(list(path)) - time.sleep(0.05) - except Exception as e: - logger.warning("refresh subscribe %s: %s", path, e) - def subscribe(self, path_segs): """Register an OBSERVE on the given path. The initial 2.05 notification and all subsequent state-change notifications