mirror of
https://github.com/ruvnet/RuView
synced 2026-07-31 18:51:42 +00:00
fix(bfld security): close HIGH privacy-bypass in process_to_frame (identity surface leaked despite restrictive class) + JSON-injection (#1075)
* fix(bfld): route process_to_frame payload through PrivacyGate (ADR-141 privacy bypass) BfldPipeline::process_to_frame stamped the frame header with the active privacy class but serialized the caller-supplied BfldPayload UNCHANGED via BfldFrame::from_payload. This let a frame labeled Anonymous(2) or Restricted(3) carry the full identity-leaky compressed_angle_matrix (+ amplitude/phase proxies, csi_delta) that PrivacyGate::demote is documented and tested (privacy_gate_demote.rs) to strip at exactly those classes. A NetworkSink accepts class >= Derived(1), so such a frame would publish the beamforming angle matrix — the identity surface — across the node boundary despite its restrictive class byte. The class byte lied about payload content. Fix: after building the frame at the active class, apply PrivacyGate::demote to the same class. demote() strips sections by target-class threshold (independent of any class transition), so a same-class demote performs no class change but brings the payload into policy compliance. Research classes (Raw/Derived) keep the full payload — demote is a no-op there. Pinned by three fails-on-old tests in pipeline_to_frame.rs: - process_to_frame_at_anonymous_strips_identity_leaky_sections (FAILED pre-fix) - process_to_frame_in_privacy_mode_strips_amplitude_and_phase (FAILED pre-fix) - process_to_frame_at_derived_preserves_full_payload (guards against over-strip) The pre-existing round-trip test is updated to assert the gated payload. Co-Authored-By: claude-flow <ruv@ruv.net> * fix(bfld): JSON-escape zone_id in MQTT state-topic payload render_events emitted the zone_activity payload as format!("\"{zone}\"") with no escaping, while ha_discovery.rs already escapes operator-controlled strings via push_str_field. A zone name containing a double-quote or backslash therefore produced malformed / injectable JSON on the state topic that Home Assistant parses (e.g. zone `a"b` -> payload `"a"b"`). Fix: add json_string_literal() mirroring ha_discovery's escaping (", \, \n, \r, \t, control chars) and use it for the zone payload. Value-identical for normal zone names (living_room etc.). Pinned by zone_payload_escapes_json_metacharacters (FAILED pre-fix); the existing zone_payload_is_json_string_with_quotes still passes unchanged. Co-Authored-By: claude-flow <ruv@ruv.net> * docs(adr-141): record bfld privacy+security review findings + CHANGELOG Document the two fixed bugs (process_to_frame privacy-bypass; zone_id JSON injection) and the dimensions confirmed clean (event-field gating, witness/hash framing, fail-closed) in ADR-141, plus CHANGELOG [Unreleased] Security/Fixed entries. Co-Authored-By: claude-flow <ruv@ruv.net>
This commit is contained in:
@@ -629,3 +629,23 @@ is characterized as a valid non-demoting mode (`single_node_cycle_is_well_formed
|
||||
|
||||
The related witness domain-separation fix from the same review is recorded in
|
||||
ADR-137 (the witness folds `effective_class`, so the demotion is auditable).
|
||||
## Security & Privacy Review (2026-06-14)
|
||||
|
||||
Beyond-SOTA privacy+security review of `wifi-densepose-bfld` (the crate was not in the ADR-154–159 sweep). Two real bugs fixed (each pinned by a fails-on-old test), several dimensions confirmed clean.
|
||||
|
||||
### Findings
|
||||
|
||||
| # | Severity | Site | Issue | Fix | Pinned by |
|
||||
|---|----------|------|-------|-----|-----------|
|
||||
| 1 | **privacy-bypass (HIGH)** | `pipeline.rs::process_to_frame` | The documented wire-bytes production path stamped the frame header with the active `PrivacyClass` but serialized the caller's `BfldPayload` **unchanged** via `BfldFrame::from_payload` — never routing through `PrivacyGate::demote`. A frame labeled `Anonymous`(2)/`Restricted`(3) carried the full `compressed_angle_matrix` (identity surface) + amplitude/phase + `csi_delta`. A `NetworkSink` accepts class ≥ `Derived`(1), so the identity surface could cross the node boundary despite the restrictive class byte — the byte lied about content. | Apply `PrivacyGate::demote(frame, active_class)` after construction: a same-class transition that strips the sections the class forbids; `Raw`/`Derived` keep the full payload. | `tests/pipeline_to_frame.rs::process_to_frame_at_anonymous_strips_identity_leaky_sections`, `…_in_privacy_mode_strips_amplitude_and_phase` (both FAILED pre-fix); `…_at_derived_preserves_full_payload` (over-strip guard) |
|
||||
| 2 | **PII/injection (MEDIUM)** | `mqtt_topics.rs::render_events` | `zone_activity` payload built as `format!("\"{zone}\"")` with no JSON escaping (while `ha_discovery.rs` already escapes). A zone name with `"`/`\` produced malformed/injectable JSON on the HA state topic. | `json_string_literal()` escaper mirroring `ha_discovery::push_str_field`. Value-identical for normal zone names. | `tests/mqtt_topic_routing.rs::zone_payload_escapes_json_metacharacters` (FAILED pre-fix) |
|
||||
|
||||
### Dimensions confirmed clean (with evidence)
|
||||
|
||||
- **Event-field privacy gating** — `BfldEvent::apply_privacy_gating` nulls `identity_risk_score` + `rf_signature_hash` at `Restricted`, and `serde(skip_serializing_if = "Option::is_none")` omits them entirely. `render_events`/`render_discovery_payloads` refuse class < `Anonymous` (stricter than the `sink.rs` `NetworkKind` `MIN_CLASS = Derived` — defense in depth toward less leakage). Covered by `event_privacy_gating.rs`, `mqtt_topic_routing.rs`, `ha_discovery.rs`.
|
||||
- **Witness/hash framing (the engine `witness_of` bug class)** — CLEAN. `SignatureHasher::compute` prefixes a **fixed 4-byte** `day_epoch` then a **fixed-width canonical-f32** feature block (`IdentityFeatures`: Embedding = `EMBEDDING_DIM*4`, RiskFactors = 16 B). `PrivacyAttestationProof::compute` hashes a fixed 32-byte `prev_hash` + three fixed 1-byte values. No variable-length operator-influenceable string is concatenated into any digest — no length-prefix-framing collision is possible.
|
||||
- **Fail-closed** — `payload.rs::from_bytes` rejects truncated/overflowing/trailing-byte sections (`checked_add`, bounds checks); `frame.rs::from_bytes` validates magic/version/length/CRC; `PrivacyClass::try_from` rejects unknown bytes; `identity_risk::score` maps NaN/degenerate factors → 0.0 (privacy-conservative). The `from_score(NaN) → Accept` choice is a documented, deliberate publish-aggregate-only fallback (NaN never reaches it from `score()`); risk-driven NaN cannot leak identity because identity gating is class-byte-driven, not risk-driven.
|
||||
|
||||
### Observation (not a bug)
|
||||
|
||||
The ADR-141 control plane (`PrivacyMode`/`PrivacyModeRegistry`) is **not yet wired into the emit path** — the emitter/pipeline enforce the raw `PrivacyClass` directly; the registry is exported + unit-tested but advisory. This matches the "Integration glue — not yet on the live path" status above. The class-byte enforcement (emitter + event + renderers + the now-fixed `process_to_frame`) is the live guarantee. Wiring the registry is the documented next step.
|
||||
|
||||
Reference in New Issue
Block a user