bhetherman/localthings
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

remove TODO_SECURITY.md — all findings resolved

Browse Source
This commit is contained in:
Marc Billow
2026-06-30 16:34:10 -05:00
parent d5dc92e673
commit ee779c955b
1 changed files with 0 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files
TODO_SECURITY.md
-86
View File
@@ -1,86 +0,0 @@
# Security TODOs
Findings from initial security review (2026-06-27). These are hardening issues,
not malicious code — the repo is safe to run. Fix before exposing to untrusted networks.
---
## 1. DTLS Certificate Verification Disabled ✅ RESOLVED
`certs/ocf_root_ca.pem` (Samsung Electronics OCF Root CA) bundled at
`samsung_appliance/ocf_root_ca.pem`. `coap_dtls.py` now uses
`SSL.VERIFY_PEER` + `ctx.load_verify_locations(_OCF_ROOT_CA)`.
ARP-spoof mitigation: the appliance's cert must chain to the Samsung OCF Root CA.
---
## 2. MQTT — No TLS, No Publisher Authentication ✅ RESOLVED
MQTT bridge removed. The integration now communicates directly with HA
via the `localthings` custom component — no MQTT broker in the path.
---
## 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 ✅ RESOLVED
1. **`coap_dtls.py` — `@SECLEVEL=0` dropped.** Cipher string is now
`ECDHE-ECDSA-AES128-GCM-SHA256` with no seclevel override.
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
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.