diff --git a/src/host/taint/mod.rs b/src/host/taint/mod.rs index dd6e3af..bd39aa6 100644 --- a/src/host/taint/mod.rs +++ b/src/host/taint/mod.rs @@ -52,39 +52,45 @@ //! disappeared; a key that reappears after full teardown is a new owner and //! starts clean. //! -//! ## Documented v1 limitation — buffered audio across a full teardown of an -//! ## *unbounded* reader (Codex phase-2 round 5) +//! ## ⚠️ KNOWN OPEN GAP — buffered audio across a full PipeWire teardown of +//! ## a still-live process (Codex phase-2 rounds 5–6) — DESIGN DECISION OWED //! -//! The "clear on full teardown" boundary is the design's, verbatim -//! (v3.4 §6.1.3: "a key that reappears after full teardown is a new owner -//! and starts clean"). It leaves one theoretical echo: an **unbounded** -//! reader — one carrying no `node.link-group`, no `pulse.module.id`, and no -//! usable PID — reads the call into an internal buffer, tears down *all* its -//! PipeWire objects while keeping that buffer, then reconnects with fresh -//! objects and replays. With every member serial gone and no owner key to -//! fingerprint, nothing links the new owner to the old, and if no reader is -//! live in that epoch the [backstop](propagate_unresolved_owner) does not -//! fire either — so the replayed leg is eligible. +//! **This is an in-threat-model echo gap, not an outside-the-model one — an +//! earlier version of this note wrongly scoped it to keyless streams.** //! -//! **Left as a documented v1 limitation, not fixed, on three grounds:** -//! 1. **Outside the threat model.** v3.4 §2 states the model is "don't echo -//! the user's own call back at them," *not* "defend against a hostile -//! local process." A stream that exposes none of PID / module id / -//! link-group is a malformed or deliberately identity-hiding stream; real -//! software (apps carry a PID, pactl modules a module id, PipeWire -//! filters a link-group, GStreamer a PID) does not produce one. -//! 2. **It contradicts the design's own boundary** (§6.1.3, quoted above), -//! so closing it is a *design change*, not a bug fix. -//! 3. **No proportionate fix exists.** The only closed-form fix is a -//! whole-share hammer — once any unbounded reader is seen, exclude every -//! output for the rest of the share — which makes the desktop -//! unshareable on the mere appearance of one keyless stream. The reachable -//! cases (a reader live *now*) are already covered by the backstop's -//! exclude-everything tier. +//! The scenario, entirely with a real PID-bearing app (a recorder, a DAW, +//! a GStreamer pipeline): it reads the call into an application buffer, +//! **fully** tears down its PipeWire Node *and* Client while keeping that +//! buffer, then — still the same live process — opens a fresh Client and a +//! `Stream/Output/Audio` and replays. Every old serial is gone, so +//! [`seed_sticky`] refuses to apply the remembered PID fingerprint (the +//! fingerprint is lifetime-scoped to a live serial member, because bare keys +//! recycle); no reader is live in the new epoch, so the backstop does not +//! fire; the replayed leg is eligible. //! -//! Owed to the design doc as a round-8 note (impl plan §11); if field -//! testing ever surfaces a real unbounded reader, revisit with -//! process-generation evidence rather than the hammer. +//! It is real and reachable by non-adversarial software. It also sits +//! exactly on the design's stated boundary (v3.4 §6.1.3: "a key that +//! reappears after full teardown is a new owner and starts clean"), so +//! closing it is a **design change**, not a local bug fix: +//! +//! - **Option A — accept as a documented v1 limitation.** Contrived in +//! practice (most apps hold their PipeWire connection open for their +//! lifetime; the round-2 fix already covers the common +//! idle-a-client-and-open-another case), never a *silent* correctness +//! regression since it is written down, and phase 5's dry run would show +//! it. But it is a known echo path, which sits badly against the feature's +//! fail-closed ethos. +//! - **Option B — process-generation lifetime.** Key the fingerprint's +//! lifetime on the owning **process** being alive — PID + `/proc` start +//! time (or a pidfd) to defeat PID reuse — instead of on a live PipeWire +//! object. Phase 3 supplies process liveness; §6.1.3's node/client-only +//! lifetime definition is revised. Closes the PID-bearing case; the truly +//! keyless sub-case (no PID at all) genuinely *is* outside the threat +//! model and stays a documented limit. +//! +//! The choice is the designer's (it revises the security surface). Until it +//! is made, `a_fingerprint_does_not_outlive_its_owner` encodes Option A's +//! behaviour — flip it if B is chosen. Owed to the design doc as round 8. // Phase 2 lands the engine behind its own test surface and nothing else: // the registry observer that will feed it is phase 3, so in a non-test diff --git a/src/host/taint/tests.rs b/src/host/taint/tests.rs index 0a887cb..f8ad8d1 100644 --- a/src/host/taint/tests.rs +++ b/src/host/taint/tests.rs @@ -1745,3 +1745,64 @@ fn a_local_root_receiver_bridges_without_an_inbound_link() { ); assert_tainted(&decisions, sink, "pixelpass-owned"); } + +#[test] +fn an_ambiguous_client_id_remembers_every_claimant_for_stickiness() { + // Round 6 finding 2: nothing pinned the ambiguous-Client branch, so a + // mutation remembering only the first claimant survived. Two live + // clients claim one global id; the tainted owner's node references it. + // If we remember only one and it is the one that later disappears, the + // still-live claimant that reopens an output escapes. + use super::snapshot::{ClientSnapshot, GlobalId}; + + let mut graph = Graph::new(); + let hw = graph.device_node("hw-sink", MediaRole::Sink); + let call = graph.peerspeak_node("peerspeak", 7); + graph.link(call, hw); + + // Two clients share one global id (the observer saw an id collision). + let shared_id = graph.dangling_id(); + let client_a = graph.client_with_id(shared_id, Some(PULSE_PID)); + let _client_b = graph.client_with_id(shared_id, Some(PULSE_PID)); + assert_eq!(client_a, shared_id); + + // The tainted reader references that (ambiguous) client id, no PID/keys. + let reader = graph.node( + "reader", + MediaRole::StreamInput, + NodeProps { + client_id: Some(shared_id), + ..NodeProps::default() + }, + ); + let out = graph.node( + "out", + MediaRole::StreamOutput, + NodeProps { + client_id: Some(shared_id), + ..NodeProps::default() + }, + ); + graph.link(hw, reader); + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11114); + + let c = ctx(); + let (first, sticky) = evaluate(&graph.build(), &c, &StickyState::default()); + assert_eq!( + first.candidates[&out.serial].reason().map(Reason::code), + Some("tainted-owner-bridge") + ); + // Both claimants must be remembered, or a mutation keeping only one + // could drop the surviving owner. At least both client serials appear. + let client_members: usize = sticky + .owners + .iter() + .flat_map(|o| o.members.iter()) + .filter(|m| matches!(m, super::ObjectRef::Client(_))) + .count(); + assert!( + client_members >= 2, + "both ambiguous-id clients should be remembered: {sticky:#?}" + ); + let _ = (ClientSnapshot { serial: Serial(0), id: GlobalId(0), sec_pid: None }, firefox); +}