281 lines
16 KiB
Markdown
281 lines
16 KiB
Markdown
# TODO — Security audit & performance review (2026-08-23)
|
||
|
||
Findings ordered by severity/impact. Line references verified against working tree.
|
||
Context: pipeline is runCam → mofin → terp → coordinator (+ livestream-cache side channel),
|
||
all communicating via plaintext gob over TCP. Ports assume LAN/Tailscale isolation today;
|
||
every finding marked [NET] matters the moment any port is reachable beyond localhost.
|
||
|
||
---
|
||
|
||
## SECURITY
|
||
|
||
### Critical
|
||
|
||
**S1. Path traversal → arbitrary file write in coordinator** [NET]
|
||
`coordinator/main.go` `handleClip`: `user, camera := splitSource(clp.SourceData)` takes
|
||
attacker-controlled strings straight into
|
||
`filepath.Join(storageDir, user, camera, dateShard, dirName)` then `MkdirAll` +
|
||
writes `clip.mp4`/`clip.json`.
|
||
A crafted `SourceData` such as `"../../../tmp--evil"` escapes the storage root and writes
|
||
an attacker-controlled MP4 (raw pixel bytes) outside it. Even benign-looking names with
|
||
`/` create unintended nested dirs (the test suite itself uses camera name containing `/`).
|
||
Fix: validate both fields against `^[A-Za-z0-9._-]+$` (reject otherwise), reject empty,
|
||
and add a final `strings.HasPrefix(finalPath, storageRoot+sep)` assertion.
|
||
|
||
**S2. Remote process-crash DoS across the whole pipeline** [NET]
|
||
Ingest paths never validate clip internal consistency (`CheckLenCorrelations()` exists in
|
||
frame/frame.go but is called nowhere except tests):
|
||
- `coordinator/main.go insertClip/dirName`: indexes `clp.Guids[0]`, `clp.Timestamps[0]`,
|
||
`clp.PixelMats[0]` unchecked → panic.
|
||
- `mofin/main.go` → `frame.CountChangedPixels`: loops `i < len(PixelMats)-1` indexing
|
||
`c.Guids[i]` → short `Guids` slice panics.
|
||
Any malformed gob Clip crashes the entire receiving process (panic in handler goroutine =
|
||
process exit). One bad client kills surveillance for all cameras.
|
||
Fix: call `clip.CheckLenCorrelations()` immediately after every decode in mofin ingest,
|
||
terp ingest, coordinator handleClip, livestream-cache handleTCPConnection; drop + log on
|
||
failure. Also wrap handlers with `recover()` as defense-in-depth.
|
||
|
||
**S3. TLS private key committed to git**
|
||
`livestream-cache/key.pem` (+ `cert.pem`) tracked in history. Regenerate, move out of repo,
|
||
gitignore, and treat the old key as burned (anyone with repo history can MITM the :8443
|
||
viewer traffic).
|
||
|
||
### High
|
||
|
||
**S4. No transport encryption or authentication on any pipeline hop** [NET]
|
||
mofin (:8083), terp (:8081), coordinator (:8082), livestream-cache ingest (:8085) accept
|
||
plaintext gob from anyone who can reach the port, binding all interfaces (`":8083"`-style
|
||
addresses, not `127.0.0.1`). Consequences: forged footage injected into any stage, live
|
||
view poisoning, clip replay/spoofing, free reconnaissance of camera names via logged
|
||
SourceData. Only livestream-cache's *viewer* port has TLS — and no auth either (see S5).
|
||
Fix options (pick one):
|
||
a. Default binds to `127.0.0.1` + explicit opt-in interface config (cheapest);
|
||
b. Shared-token handshake as first gob message on each conn (cheap, works over Tailscale);
|
||
c. Full mTLS mirroring the existing cert setup (strongest).
|
||
|
||
**S5. Unauthenticated MJPEG viewing**
|
||
`handleMJPEGStream` checks only that `sourceData` exists in `cameraUsers`. Anyone reaching
|
||
:8443 watches every camera feed. Add HTTP BasicAuth (bcrypt-hashed creds in YAML) or a
|
||
bearer token per viewer; return 401 before touching the stream.
|
||
|
||
**S6. Unbounded gob decode → memory-exhaustion DoS** [NET]
|
||
Every listener decodes attacker-sized `[][]byte` payloads with no cap (a single Clip can
|
||
declare gigabytes of pixel data → OOM). Combined with S7 this is trivially scriptable.
|
||
Fix: enforce a max-clip-bytes config; simplest robust route is `io.LimitReader`-wrapped
|
||
conn + length-prefixed framing, or decode then reject if `len(PixelMats) > MaxFrames` /
|
||
`len(frame) > MaxFrameBytes` *before* any processing/allocation beyond the decode itself.
|
||
|
||
**S7. No connection deadlines anywhere (slowloris / goroutine exhaustion)** [NET]
|
||
Zero `SetDeadline` calls repo-wide (verified). Servers hold one goroutine + buffers per
|
||
client indefinitely; a handful of idle sockets starves the process. Also client-side:
|
||
`Clip.Send` has DialTimeout but no write deadline, and `flush()` in runCam calls it inline
|
||
— a stuck receiver freezes that camera's clip building.
|
||
Fix: `conn.SetDeadline(time.Now().Add(N))` after accept and refresh per message; write
|
||
deadline around every send including `Clip.Send`.
|
||
|
||
### Medium
|
||
|
||
**S8. Passphrases exposed via CLI flags and plaintext configs**
|
||
`-passphrase` flags appear in `ps`/`/proc/*/cmdline` for every local user and persist in
|
||
shell history; YAML configs store them in cleartext. Fix: support env var + `-passfile`
|
||
(or keyring prompt), never echo back; document migration.
|
||
Related landmine: legacy `frame.ConsumeMat`/`preframeWrapper.ToFrame` concatenate
|
||
`CfgData.PassHash` INTO `SourceData` — if ever rewired, password hashes leak into clip.json
|
||
metadata and DB rows. Delete the hash-from-metadata behavior now while the code is dead.
|
||
|
||
**S9. Metadata travels in cleartext even when frames are encrypted**
|
||
AES-GCM covers `PixelBytes` only; SourceData (username/camera), GUIDs, timestamps,
|
||
detections, and Comparisons ride plaintext gob. Network observer learns which cameras,
|
||
when, and how active they are. Fix: extend encryption to the serialized metadata block
|
||
(e.g., encrypt a marshalled header alongside pixels) or tunnel the whole gob stream in TLS
|
||
(S4c solves this wholesale).
|
||
|
||
**S10. Livestream-cache trusts whatever arrives for a registered camera** [NET]
|
||
If a camera is registered without a passphrase, any host can inject frames into the live
|
||
view (no origin concept exists). Subsumed by S4b/S4c, but worth an explicit note: the cache
|
||
should reject conns lacking the shared secret even when passphrases are unused.
|
||
|
||
**S11. Hardened servers missing timeouts / limits (HTTPS viewer)**
|
||
`http.Server` created without `ReadHeaderTimeout`, `IdleTimeout`, `MaxHeaderBytes`;
|
||
unlimited concurrent streams. Slowloris applies here too. Add sane timeouts + a
|
||
semaphore-capped stream count.
|
||
|
||
**S12. Footage written world-readable**
|
||
Storage dirs/files default 0755/0644 (`MkdirAll`, VideoWriter output). On multi-user hosts
|
||
any local account can copy surveillance footage. Write with 0750/0640 (umask or explicit
|
||
Chmod after MkdirAll).
|
||
|
||
**S13. MySQL DSN built via fmt.Sprintf**
|
||
Special characters in db-user/password silently corrupt the DSN (and error messages may
|
||
echo it). Use `mysql.Config{...}.FormatDSN()`; load creds from env/file rather than flags.
|
||
|
||
### Low
|
||
|
||
- **S14 Log injection**: raw request paths/hosts flow into log lines (livestream-cache,
|
||
coordinator); sanitize/strip control chars.
|
||
- **S15 Git remote over plain HTTP** (`http://gitea:3000/...`) — switch to SSH or HTTPS
|
||
for push traffic on shared LANs.
|
||
- **S16 oko-run flag interpolation** builds child-process args from unquoted map values —
|
||
harmless today (config is trusted, exec doesn't use a shell) but easy to misuse; quote
|
||
values explicitly.
|
||
- **S17 `Frame.Errors` never populated** — no audit trail for dropped/failed frames;
|
||
populate at capture time (decode failures, drops-on-full-channel) so downstream can
|
||
report data-quality issues.
|
||
|
||
---
|
||
|
||
## PERFORMANCE
|
||
|
||
Ordered by expected impact on the target hardware (Pi Zero W-class).
|
||
|
||
**P1. PBKDF2 with 600k iterations executed PER FRAME** — `frame.Encrypt`
|
||
(frame/frame.go). A 10 s clip at ~50 fps = ~500 KDF derivations per clip, each ~100 ms+
|
||
on a Pi → encryption dominates total CPU and burns through most of the clip interval.
|
||
Fix: derive ONE key per clip — move salt to Clip level (add `Salt []byte` field, derive
|
||
once in mofin, encrypt every frame with distinct random nonces). Backward-compatible
|
||
enough since all stages deploy together. Expected: ~99% reduction in KDF cost.
|
||
|
||
**P2. Haar cascade XML re-loaded for every frame, for every location** —
|
||
`frame.Detect` (frame/detect.go:40-43) constructs `gocv.NewCascadeClassifier` + `.Load()`
|
||
per frame per model path. XML parsing dwarfs actual detection cost.
|
||
Fix: load all classifiers once at terp startup, share the objects across worker
|
||
goroutines (`detectMultiScale` is thread-safe for reads). Expected: order-of-magnitude
|
||
speedup of the classification stage.
|
||
|
||
**P3. Raw, uncompressed pixels end-to-end** — 320×240×3 B ≈ 230 KB/frame; observed clip =
|
||
~500 frames ⇒ ~115 MB per clip held in RAM (builder buffer + gob encode buffer + decode
|
||
side), plus 115 MB bursts on the wire every 10 s per camera. On a Zero W this is the
|
||
memory ceiling. Options in ascending effort:
|
||
a. Cap clip duration/fps for the motion stage (e.g. analyze 5 fps subsample);
|
||
b. Per-frame JPEG (or MJPEG stream) on the wire — ~10–20× size reduction, decode cost
|
||
paid once in mofin;
|
||
c. H.264 chunk streaming (matches what coordinator re-encodes anyway).
|
||
At minimum document the footprint math near the clip builder.
|
||
|
||
**P4. Motion analysis done TWICE** — mofin computes `CountChangedPixels` and stores it in
|
||
`clp.Comparisons`; terp then recomputes identical values (terp/main.go classify loop)
|
||
instead of trusting `clp.Comparisons` when present. Free 2× saving on the AbsDiff stage:
|
||
`if len(clp.Comparisons) > 0 { skip }`.
|
||
|
||
**P5. mofin decrypts then re-encrypts every frame** (fresh salt each time, defeating any
|
||
KDF caching) purely to run motion detection on plaintext. With P1 fixed the re-encrypt is
|
||
cheap, but better: move motion gating upstream into runCam (pre-send) and ship encrypted
|
||
clips untouched through mofin, or have mofin operate on a small thumbnail sub-stream
|
||
(P3a) so bulk pixels never decrypt/re-encrypt.
|
||
|
||
**P6. Livestream JPEG re-encoded per viewer per tick** — `handleMJPEGStream` encodes a
|
||
fresh JPEG from the cached RGBA image for EVERY connected client EVERY 100 ms. N viewers
|
||
= N× encode cost. Fix: cache `[]byte` JPEG per camera; re-encode only when a new frame
|
||
lands (janitor/ingest hook), serve the same bytes to all viewers.
|
||
|
||
**P7. Pure-Go BGR→RGBA pixel loop in frameToImage** — per-pixel Go loop with bounds
|
||
checks per frame. Since gocv is already in the module graph, replace the whole helper
|
||
with `gocv.Mat` construction from bytes + `gocv.IMencode(".jpg", mat, &buf)` — moves
|
||
color conversion AND compression into optimized OpenCV code in one step (subsumes P6's
|
||
encoder too).
|
||
|
||
**P8. Motion comparison allocates heavily per frame-pair** — `compareTo` creates 3 Mats
|
||
(diff/gray/thresh) and converts overlapping frames to Mat twice (frame i used as pair i-1
|
||
tail and pair i head). Fix inside `CountChangedPixels`: keep previous frame's gray Mat
|
||
alive between iterations, reuse scratch Mats with `Mat.CopyTo`/in-place ops, and downscale
|
||
to e.g. 96×72 grayscale before diffing (motion gating needs neither resolution nor color).
|
||
Expected: 10–30× less work in the hottest OpenCV section.
|
||
|
||
**P9. Unbounded goroutine-per-connection/per-clip** in mofin, terp, coordinator handlers.
|
||
One burst of clips = unbounded concurrent 100 MB-scale allocations (compounds S6/P3).
|
||
Cap with a buffered-channel semaphore (configurable workers, default 2–4) per service.
|
||
|
||
**P10. DB writes row-at-a-time** — `insertClip` Execs one INSERT per detection. Prepare
|
||
once per connection + single transaction per clip; enable go-sql-driver interpolateParams
|
||
for small batches. Matters when detections-per-clip grows (YOLO wiring will multiply it).
|
||
|
||
**P11. VideoWriter codec fallback ends at MJPG** — MJPG fallback produces files ~5–10×
|
||
larger than mp4v/H264. Prefer trying `avc1 → mp4v → MJPG`, and make codec/fourcc
|
||
configurable per deployment. Also `computeFPS` uses only the first frame delta — average
|
||
over several deltas to avoid wildly wrong timestamps after capture hiccups.
|
||
|
||
**P12. Dev-loop: oko-run rebuild check stats binary existence only** — stale binaries are
|
||
silently reused after source edits. Compare mtimes (newest .go vs binary) or add a
|
||
`-force-rebuild` flag.
|
||
|
||
---
|
||
|
||
## COMPUTER-VISION PIPELINE ENHANCEMENTS
|
||
|
||
Grouped: correctness first, then accuracy, then capability.
|
||
|
||
### Correctness (do these before trusting any detections)
|
||
|
||
**C1. YOLO head parsed wrong — class information discarded.**
|
||
`DetectYolo` reads only the box/conf channels; ONNX YOLOv8/v11 export layout is
|
||
[1, 84, N] = cx,cy,w,h + 80 class scores, so every detection currently shares one title
|
||
and confidence source. Fix: transpose to [N,84], take argmax class + its score as the
|
||
detection, map through a COCO names table, filter by per-class thresholds.
|
||
|
||
**C2. NMS never applied to YOLO output.** `filterNMS` exists but is unwired — expect
|
||
dozens of duplicate boxes per object. Apply class-aware NMS (IoU ~0.45) after C1.
|
||
|
||
**C3. Model loaded per call in DetectYolo path** (`loadModel` per invocation). Same fix
|
||
as P2: load Net once at startup; `net.Empty()`/forward is the only per-frame op.
|
||
|
||
### Accuracy
|
||
|
||
**C4. Replace Haar fullbody primary detection.** Haar at 320×240 yields heavy false
|
||
positives/negatives. Better ladder: (a) HOG+SVM person detector as cheap upgrade;
|
||
(b) a lightweight DNN detector (MobileNet-SSD / nano-YOLO) reusing the existing ONNX
|
||
plumbing — the infrastructure lands with C1–C3 anyway.
|
||
|
||
**C5. Background subtraction instead of consecutive-frame diff.**
|
||
`gocv.BackgroundSubtractorMOG2` gives: robustness to gradual lighting changes, built-in
|
||
shadow suppression, contour masks for area/centroid features (replacing raw changed-pixel
|
||
counts), and long-term stationary-object suppression. Keep frame-diff as fallback for the
|
||
first seconds after startup (model warm-up).
|
||
|
||
**C6. Clean up the motion signal.** Before/after diffing: Gaussian blur (σ≈3) to kill
|
||
sensor noise; morphological open+close on the threshold mask; minimum-contour-area gate;
|
||
auto-calibrated threshold per camera from rolling idle-period noise statistics (mean+3σ of
|
||
changed pixels) instead of magic constant 30.
|
||
|
||
**C7. ROI masking per camera.** Configurable polygon per view (sky/hedges/road excluded)
|
||
applied to the motion mask — eliminates the classic swaying-trees/headlights false alarms.
|
||
|
||
### Capability
|
||
|
||
**C8. Event-shaped recording.** Today fixed 10 s ticker slices cut events mid-action and
|
||
record dead air. Switch clip builder to: rolling ring buffer (~5 s pre-roll) + trigger on
|
||
motion + record until quiet for N s (+ hysteresis), emitting variable-length event clips.
|
||
Biggest UX win per line of code.
|
||
|
||
**C9. Intra-clip tracking.** IoU-match detections across frames within a clip → stable
|
||
object IDs, direction, dwell time; persist track summaries to MySQL next to detections.
|
||
Enables "person loitered 4 min" queries and dedupes alert storms.
|
||
|
||
**C10. Quality gates.** Variance-of-Laplacian blur check per captured frame; discard
|
||
garbage (exposure glitches) before they poison motion stats; populate `Frame.Errors`
|
||
(S17) with reasons for observability.
|
||
|
||
**C11. Review affordances at coordinator:** save first-detection-frame JPEG thumbnails +
|
||
detection crops beside clip.mp4; add a tiny JSON index endpoint for a future web UI.
|
||
Cheap, transforms triage from "watch every mp4" to scanning a contact sheet.
|
||
|
||
**C12. Retention/GC job.** Storage grows unbounded (only janitor is the live-view cache).
|
||
Age-based pruning + total-quota enforcement in coordinator, config-driven.
|
||
|
||
**C13. Notification hook.** After terp classification, POST event summaries (camera, class,
|
||
track info, thumbnail ref) to a webhook/Ntfy/MQTT — turns the pipeline from recorder into
|
||
alarm system.
|
||
|
||
**C14. Cross-camera correlation (later).** Time-windowed matching of tracks across cameras
|
||
(same user namespace) for entry/exit reasoning.
|
||
|
||
---
|
||
|
||
## Suggested execution order
|
||
|
||
Quick wins first (hours): S2 validation+recover, S1 traversal guard, S5 basic auth,
|
||
P4 (one-liner), P2 (classifier reuse), P6/P7 (jpeg cache + IMencode).
|
||
Then structural (days): P1 KDF-per-clip, S4b token auth, S6/S7 deadlines+caps,
|
||
P8 motion-buffer reuse, C1+C2+C3 YOLO fix.
|
||
Then feature work: C8 event recording, C5 background subtraction, C9 tracking,
|
||
P3 compression decision (needs a bandwidth/storage budget discussion).
|