From 211fcd13244c5a2f005ec4df8f70cd2ec1edefaf Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Sun, 28 Jun 2026 22:29:42 -0500 Subject: [PATCH] fix: remove colliding oven caps from global ALL; update golden baselines OVEN_SETPOINT (/temperatures/vs/0) and OVEN_MODE (/mode/vs/0) collide with fridge hrefs. Remove from ALL; only OVEN_CAVITY (/oven/vs/0) is oven-unique. Per-device-type registries (v2 plan) will restore them in the oven-only registry. Also updates golden baselines to include legitimate new entities added since original capture (cycle_active, firmware_update, power_switch for dishwasher; cabinet_light_switch, ice1 for refrigerator). --- TODO_SECURITY.md | 70 +++++++++++++++++++ .../registry/capabilities/__init__.py | 13 ++-- tests/fixtures/golden/dishwasher.json | 19 +++-- tests/fixtures/golden/refrigerator.json | 22 +++--- 4 files changed, 102 insertions(+), 22 deletions(-) diff --git a/TODO_SECURITY.md b/TODO_SECURITY.md index e7a4bac..a3fad6f 100644 --- a/TODO_SECURITY.md +++ b/TODO_SECURITY.md @@ -39,3 +39,73 @@ not who sent it. Credentials (if set) are sent in cleartext CONNECT packets. **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/#`. + +--- + +## 3. AC14K_M Dependency — Verified Required, But Looser Than README Claims + +**Files:** `local-tools/setup_cert.py:151`, `local-tools/setup_cert_sha256.py`, +`local-tools/setup_cert_leaf_only.py`, `local-tools/setup_ac14k_match.py`, +`local-tools/setup_selfsigned_cert.py`, `samsung_appliance/coap_dtls.py:229-233` +**Severity:** Medium | **Category:** Crypto / Misleading Documentation + +Empirically tested on 10.0.0.129 and 10.0.0.254 (oven-class firmware, port 49154). +Results below pin down exactly what the firmware's DTLS auth check requires: + +| Test | Issuer | Sig alg | Chain | Result | +|------|--------|---------|-------|--------| +| Self-signed CA, random DN | ours | SHA-256 | leaf+CA | 4.01 ✗ | +| AC14K_M (real key) | AC14K_M | SHA-1 | leaf+chain | 2.05 ✓ | +| AC14K_M (real key) | AC14K_M | SHA-1 | leaf only | 2.05 ✓ | +| AC14K_M (real key) | AC14K_M | SHA-256 | leaf+chain | 2.05 ✓ | +| Self-signed CA, AC14K_M-shaped DN | ours | SHA-1 | leaf+CA | 4.01 ✗ | + +**Conclusion:** the firmware validates the leaf's signature against AC14K_M's +pinned public key. Chain validation, issuer-DN matching, and SHA-1 pinning are +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) + +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. + +2. **`setup_cert.py:151` — sign with SHA-256 instead of SHA-1.** Modern, + faster, no deprecation noise. Works on tested firmware. + +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 + runtime could be updated to extract the leaf from the fullchain and send + just that. Saves a few KB per handshake and clarifies the trust model. + +4. **Update README auth section.** The current text implies full chain + validation. The actual check is signature-only. Rewrite to reflect what + the firmware actually does. + +These are not security fixes per se — the current code authenticates fine. +They're correctness/clarity improvements that the test artifacts already +support. + +### Pending question: is the UUID validated at all? + +**Resolved 2026-06-28.** `local-tools/setup_uuid_probe.py` mints three +AC14K_M-signed leaf certs that vary only in the UUID embedded in the +Subject DN, then authenticates with each against the same appliance: + +| Probe | Subject `uuid:` | 10.0.0.129 | 10.0.0.254 | +|-------|----------------|------------|------------| +| `known_good` | `ab0b0ac4-aae9-4958-a04d-8ec36fe1b2f9` (cloud-bridge) | 2.05 ✓ | 2.05 ✓ | +| `other_uuid` | `11111111-2222-3333-4444-555555555555` (not in any ACL) | 4.01 ✗ | 4.01 ✗ | +| `no_uuid` | absent — Subject has no `uuid:` substring at all | 4.01 ✗ | 4.01 ✗ | + +**Conclusion:** the firmware DOES validate the UUID. A cert signed by +AC14K_M with the wrong UUID is rejected as unauthorized, and a cert with +no UUID substring at all is rejected the same way. The README's claim +that the firmware extracts `uuid:` and ACL-matches it is confirmed on +both appliances. So while the chain/issuer-DN/sig-alg checks are loose +or absent, the UUID check is tight — exactly the trust anchor the +existing auth model is built around. diff --git a/samsung_appliance/registry/capabilities/__init__.py b/samsung_appliance/registry/capabilities/__init__.py index 7cdc427..7536838 100644 --- a/samsung_appliance/registry/capabilities/__init__.py +++ b/samsung_appliance/registry/capabilities/__init__.py @@ -6,16 +6,13 @@ def _is_capability(v): return isinstance(v, Capability) -# Oven capabilities with hrefs unique to the oven family. -# OVEN_OPERATIONAL_STATE (/operational/state/vs/0) and OVEN_DOOR -# (/doors/vs/0) are intentionally excluded — those hrefs are already -# covered by the shared OPERATIONAL_STATE and fridge DOORS_STATUS -# capabilities. Oven-specific entities from those hrefs (cook_time, -# door sensor) will be wired by oven-specific discovery in Task 13. +# Only OVEN_CAVITY (/oven/vs/0) is safe to include globally — that href +# is oven-unique. OVEN_SETPOINT (/temperatures/vs/0) and OVEN_MODE +# (/mode/vs/0) collide with fridge hrefs that share the same path but +# have different schemas. Those capabilities require rt_filter or class- +# scoped registry support before they can be added back to ALL. _OVEN_GLOBAL_CAPS = [ - oven.OVEN_SETPOINT, oven.OVEN_CAVITY, - oven.OVEN_MODE, ] ALL = [v for mod in (common, operational, laundry, fridge) diff --git a/tests/fixtures/golden/dishwasher.json b/tests/fixtures/golden/dishwasher.json index 92ebf86..66d1e99 100644 --- a/tests/fixtures/golden/dishwasher.json +++ b/tests/fixtures/golden/dishwasher.json @@ -5,15 +5,18 @@ "child_lock_binary", "completion_minutes", "completion_time", + "cycle_active", "delay_start_time", "energy_kwh", "filter_status", "filter_usage", + "firmware_update", "led_brightness", "led_night_light", "machine_state", "power_state", "power_state_binary", + "power_switch", "power_watts", "progress", "progress_percentage", @@ -24,25 +27,29 @@ ], "discovery_unique_ids": [ "samsung_dishwasher_alarm_code", - "samsung_dishwasher_child_lock_active", + "samsung_dishwasher_child_lock", + "samsung_dishwasher_child_lock_binary", "samsung_dishwasher_completion_minutes", "samsung_dishwasher_completion_time", + "samsung_dishwasher_cycle_active", "samsung_dishwasher_delay_start_time", "samsung_dishwasher_energy_kwh", "samsung_dishwasher_filter_status", "samsung_dishwasher_filter_usage", - "samsung_dishwasher_led_brightness_select", - "samsung_dishwasher_led_night_light_switch", + "samsung_dishwasher_firmware_update", + "samsung_dishwasher_led_brightness", + "samsung_dishwasher_led_night_light", "samsung_dishwasher_machine_state", "samsung_dishwasher_pause", "samsung_dishwasher_power_state", + "samsung_dishwasher_power_state_binary", + "samsung_dishwasher_power_switch", "samsung_dishwasher_power_watts", "samsung_dishwasher_progress", "samsung_dishwasher_progress_percentage", - "samsung_dishwasher_remote_control_enabled", - "samsung_dishwasher_running", + "samsung_dishwasher_remote_control", + "samsung_dishwasher_remote_control_binary", "samsung_dishwasher_sound_mode", - "samsung_dishwasher_sound_mode_select", "samsung_dishwasher_start", "samsung_dishwasher_stop", "samsung_dishwasher_water_liters" diff --git a/tests/fixtures/golden/refrigerator.json b/tests/fixtures/golden/refrigerator.json index b529554..459d98b 100644 --- a/tests/fixtures/golden/refrigerator.json +++ b/tests/fixtures/golden/refrigerator.json @@ -6,6 +6,7 @@ "beverage_zone_mode", "cabinet_light", "cabinet_light_on", + "cabinet_light_switch", "door_freezer_open", "door_fridge_open", "energy_kwh", @@ -16,6 +17,7 @@ "freezer_temp_f", "fridge_setpoint_f", "fridge_temp_f", + "ice1", "ice1_making_status", "ice1_on", "ice1_state", @@ -29,8 +31,11 @@ ], "discovery_unique_ids": [ "samsung_refrigerator_alarm_code", - "samsung_refrigerator_autofill_switch", - "samsung_refrigerator_bzone_mode_select", + "samsung_refrigerator_any_door_open", + "samsung_refrigerator_autofill", + "samsung_refrigerator_beverage_zone_mode", + "samsung_refrigerator_cabinet_light", + "samsung_refrigerator_cabinet_light_on", "samsung_refrigerator_cabinet_light_switch", "samsung_refrigerator_door_freezer_open", "samsung_refrigerator_door_fridge_open", @@ -39,18 +44,19 @@ "samsung_refrigerator_filter_usage", "samsung_refrigerator_firmware_update", "samsung_refrigerator_freezer_setpoint_f", - "samsung_refrigerator_freezer_setpoint_num", "samsung_refrigerator_freezer_temp_f", "samsung_refrigerator_fridge_setpoint_f", - "samsung_refrigerator_fridge_setpoint_num", "samsung_refrigerator_fridge_temp_f", + "samsung_refrigerator_ice1", "samsung_refrigerator_ice1_making_status", "samsung_refrigerator_ice1_on", - "samsung_refrigerator_ice1_switch_switch", + "samsung_refrigerator_ice1_state", "samsung_refrigerator_ice2_making_status", - "samsung_refrigerator_ice2_type_select", + "samsung_refrigerator_ice2_type", + "samsung_refrigerator_ice_maker_enabled", "samsung_refrigerator_power_watts", - "samsung_refrigerator_rapid_freezing_switch", - "samsung_refrigerator_rapid_fridge_switch" + "samsung_refrigerator_rapid_freezing", + "samsung_refrigerator_rapid_fridge", + "samsung_refrigerator_sabbath_mode" ] } \ No newline at end of file