From f35bab0379dd7688a40dd6358a2153a90fd91c37 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Tue, 21 Jul 2026 17:40:24 -0400 Subject: [PATCH] host/taint: close the inverse asymmetric leak; strengthen contracts (Codex round 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3 was the second verification round. One real leak, one accepted narrowing of Codex's own suggested fix, two contract strengthenings, and a test-gap fix. F1 (P1, real leak, fixed): the inverse of round-1 finding 4. A tainted reader that is itself *unbounded* (client.id only, daemon PID suppressed) whose re-emitting leg carried an *unmatched* strong key left that leg "bounded" and Eligible. An unbounded reader cannot be positively related to any output, so a strong key that does not match it back proves nothing. Two-tier backstop. A bounded tainted reader excludes only unbounded outputs (a differently-keyed output is provably a different owner). An unbounded tainted reader also excludes daemon-owned outputs — but NOT ordinary apps. ⚠️ Deliberately narrower than Codex's suggested "exclude every output". An unbounded reader is necessarily daemon-owned (a real app has its own PID, which is a usable key, so it would be bounded), so its sibling is another daemon leg, never an app. Sweeping in real apps would lose the round-1 "blast radius stays small" guarantee for no safety gain. When the daemon PID is unknown the app/leg distinction collapses and the rule degrades to Codex's exclude-all. Both directions are pinned by tests, and the over-aggressive variant fails the spares-real-apps test. F2 (contract, strengthened): `session_device` is documented as a positive high-confidence phase-3 classification, not `device.id`+`device.api` (measured insufficient — a card filter can carry both; node.physical is null on the real ALSA nodes so it is not a discriminator). Fail closed: unknown ⇒ false. Documented why a mis-classified filter still does not leak in practice — its legs share a link-group (strong-key bridge) and an unbounded reading leg trips the two-tier backstop. F5 (P2, test gap): a mutation keeping only PID fingerprints survived all 49 tests. Added a strong-key (pulse.module.id) new-connection fixture. Mutation-verified 3/3 including the over-aggressive counter-mutation. Still OWED to round 3, carried to round 4 for adjudication: finding 3 (a not-ready epoch can persist provisional owner *fusion* as sticky over-exclusion). It is over-exclusion, never an echo leak, and closing it needs a readiness/provenance model decision rather than a local patch — see the round-4 handoff. 53 tests. Co-Authored-By: Claude Opus 4.8 --- src/host/taint/mod.rs | 69 +++++++++++--- src/host/taint/snapshot.rs | 57 ++++++++---- src/host/taint/tests.rs | 182 +++++++++++++++++++++++++++++++++++++ 3 files changed, 280 insertions(+), 28 deletions(-) diff --git a/src/host/taint/mod.rs b/src/host/taint/mod.rs index 5789f92..0cfa063 100644 --- a/src/host/taint/mod.rs +++ b/src/host/taint/mod.rs @@ -361,7 +361,8 @@ pub fn evaluate( let mut changed = false; changed |= propagate_links(&edges.edges, &mut taint); changed |= propagate_owner_bridge(&keys, &components, &edges, &mut taint); - changed |= propagate_unresolved_owner(snapshot, &keys, &edges, &mut taint); + changed |= + propagate_unresolved_owner(snapshot, ctx.pipewire_pulse_pid, &keys, &edges, &mut taint); if !changed { break; } @@ -662,28 +663,72 @@ fn propagate_owner_bridge( /// carry a real `application.process.id` and are bounded, so they are /// never swept up; in practice only daemon-owned keyless module streams /// are. -/// - ⚠️ **Removed (Codex round 1): the source no longer has to be unbounded -/// itself.** Properties can be asymmetric — a reader carrying a -/// `node.link-group` whose re-emitting leg carries none is *bounded* while -/// its sibling is not findable — and requiring an unbounded source let -/// exactly that shape through as Eligible. +/// - ⚠️ **The source does not have to be unbounded** (Codex round 1): a +/// reader carrying a `node.link-group` whose re-emitting leg carries none +/// is *bounded* while its sibling is not findable, and requiring an +/// unbounded source let exactly that shape through. +/// +/// **Two tiers, because a tainted reader we cannot bound is a bigger +/// unknown than one we can** (Codex round 3 — the mirror image of the +/// round-1 case): +/// +/// - A *bounded* tainted reader has a strong key or a usable PID, so its +/// siblings are exactly the output legs sharing that key. Any output leg +/// that is *itself* bounded by a **different** key is provably a different +/// owner and stays eligible; only unbounded output legs are its possible +/// siblings. → exclude unbounded outputs. +/// - An *unbounded* tainted reader has nothing that identifies its owner, so +/// its re-emitting leg could carry a strong key we cannot match back to +/// it. But it cannot be *anything*: a reader with no usable owner key is +/// necessarily **daemon-owned** — a real application has its own PID, +/// which is a usable key, so it would be bounded. Its sibling is therefore +/// another daemon-owned output, never an ordinary app. → also exclude the +/// daemon-owned outputs (bounded by a strong key or not); leave outputs +/// carrying a real, non-daemon PID eligible, because a real app is +/// provably a different owner from a daemon module leg. +/// +/// ⚠️ This is deliberately **narrower than Codex round 3's suggested +/// "exclude every output"**, which would make an ordinary app unshareable +/// whenever any keyless module forwarder reads the call — losing the +/// round-1 "blast radius stays small" guarantee for no safety gain, since +/// a real app cannot be the sibling of a daemon leg. When the daemon PID +/// is *unknown* the distinction collapses (we cannot tell a real app from +/// a module leg) and the rule degrades to Codex's: exclude everything. fn propagate_unresolved_owner( snapshot: &GraphSnapshot, + ctx_pulse_pid: Option, keys: &owner::OwnerKeyIndex, edges: &Edges, taint: &mut BTreeMap, ) -> bool { - let tainted_reader = snapshot.nodes().any(|node| { - !node.props.session_device + let mut has_tainted_reader = false; + let mut has_unbounded_tainted_reader = false; + for node in snapshot.nodes() { + let is_tainted_reader = !node.props.session_device && edges.receivers.contains(&node.serial) - && taint.get(&node.serial).is_some_and(|r| r.propagates()) - }); - if !tainted_reader { + && taint.get(&node.serial).is_some_and(|r| r.propagates()); + if is_tainted_reader { + has_tainted_reader = true; + has_unbounded_tainted_reader |= !keys.is_bounded(node.serial); + } + } + if !has_tainted_reader { return false; } let mut changed = false; for node in snapshot.nodes() { - if node.role == MediaRole::StreamOutput && !keys.is_bounded(node.serial) { + if node.role != MediaRole::StreamOutput { + continue; + } + // A real, non-daemon PID proves the node is an ordinary app, not a + // module leg — the one thing an unbounded daemon reader's sibling + // cannot be. Unknown daemon PID ⇒ cannot prove it ⇒ swept in. + let is_real_app = matches!( + (node.props.process_id, ctx_pulse_pid), + (Some(pid), Some(daemon)) if pid != daemon + ); + let swept_by_unbounded = has_unbounded_tainted_reader && !is_real_app; + if !keys.is_bounded(node.serial) || swept_by_unbounded { changed |= raise(taint, node.serial, Reason::UnresolvedOwner); } } diff --git a/src/host/taint/snapshot.rs b/src/host/taint/snapshot.rs index 716f567..2c06174 100644 --- a/src/host/taint/snapshot.rs +++ b/src/host/taint/snapshot.rs @@ -120,23 +120,48 @@ pub struct NodeProps { /// manager** — a real sound card's sink or source, not something that /// forwards audio. /// - /// ⚠️ **A classification the observer owes, not a raw property.** The - /// first cut set this from bare `device.id`, and Codex refuted it: - /// PipeWire defines `device.id` only as "the Device this Node belongs - /// to", which does not promise the node is passive. A forwarding node - /// that happens to carry one would silently lose its coarse owner keys - /// *and* its ability to trip the fail-closed backstop — two protections - /// at once. **Phase 3 must require `device.id` AND `device.api`** - /// (measured: `device.api=alsa` on all five real ALSA nodes, absent on - /// every `support.null-audio-sink`), and should treat anything it - /// cannot positively classify as *not* a device — that is the - /// fail-closed direction here, because the flag only ever *removes* - /// taint mechanisms. + /// ⚠️ **A positive high-confidence classification the observer owes, not + /// a raw property** (Codex rounds 2–3). PipeWire defines `device.id` + /// only as "the Device this node belongs to" and `device.api` as that + /// Device's access API; **neither promises the node passively terminates + /// audio**, so a card-associated filter can satisfy both. Setting this + /// flag *removes* two protections at once — the node's coarse owner keys + /// (`owner` exception 2) and its ability to trip the fail-closed + /// backstop — so a false positive is a leak, not over-exclusion. /// - /// What it is for: every device node on the box shares the session - /// manager's `client.id` (measured: 42, `WirePlumber [export]`), so - /// coarse owner keys must not bridge them. See [`super::owner`] - /// exception 2. + /// **Phase-3 contract:** + /// - Set `true` only on positively-identified passive hardware + /// terminals: a resolved `device.id` on a real backend + /// (`device.api` present) whose `factory.name` is a hardware PCM + /// factory (`api.alsa.pcm.{sink,source}` and the like), never a + /// filter/loopback/null-sink factory. Measured discriminator on the + /// target box: the five ALSA nodes carry `device.api=alsa` + + /// `factory.name=api.alsa.pcm.*` and share `client.id=42` + /// (`WirePlumber [export]`); the three `support.null-audio-sink` nodes + /// carry neither. (`node.physical` was measured **null** on the ALSA + /// nodes here, so it is *not* a usable discriminator — do not rely on + /// it.) + /// - **Fail closed: unknown ⇒ `false`.** A node that cannot be + /// positively classified keeps its owner keys and can trip the + /// backstop; both are the safe direction. + /// - A node MUST NOT enter a snapshot with this field provisional. If + /// the Device backing a node has not yet been bound, withhold the node + /// and keep the epoch not-ready — otherwise a provisional `false` + /// during not-ready fuses sink and mic on the shared session client + /// and that fusion can persist as sticky over-exclusion (round-3 + /// finding 3). + /// + /// Why a mis-classified *filter* does not actually leak in practice: a + /// WirePlumber smart-filter's two legs share a `node.link-group`, so the + /// strong-key bridge catches the re-emitting leg regardless of this + /// flag; and if the reading leg is genuinely unbounded, the two-tier + /// backstop (`propagate_unresolved_owner`) excludes every output. The + /// flag is the belt; those are the braces. + /// + /// What it is for: every real device node shares the session manager's + /// `client.id`, so coarse owner keys must not bridge them — else + /// peerspeak's playback (which taints the default sink every recompute) + /// would reach the microphone. See [`super::owner`] exception 2. pub session_device: bool, } diff --git a/src/host/taint/tests.rs b/src/host/taint/tests.rs index e2d215b..3c28fcc 100644 --- a/src/host/taint/tests.rs +++ b/src/host/taint/tests.rs @@ -1490,3 +1490,185 @@ fn a_not_ready_snapshot_still_records_new_taint() { ], ); } + +// ────────────────────────────────────────────────────────────────────── +// Regressions from Codex round 3 (second verification round) +// ────────────────────────────────────────────────────────────────────── + +#[test] +fn an_inverse_asymmetric_forwarder_fails_closed() { + // Round 3 finding 1 — the mirror of round-1 finding 4, and a real leak. + // The reader is unbounded (client.id only, daemon PID suppressed) while + // its re-emitting leg carries an *unmatched* strong key, so the leg was + // "bounded" and stayed Eligible. When the reader itself cannot be + // bounded, its sibling could be any output, so a strong key that does + // not match it back proves nothing. + let mut graph = Graph::new(); + let hw = graph.device_node("hw", MediaRole::Sink); + let call = graph.peerspeak_node("call", 7); + graph.link(call, hw); + + let in_client = graph.client(Some(PULSE_PID)); + let out_client = graph.client(Some(PULSE_PID)); + let fwd_in = graph.node("fwd-in", MediaRole::StreamInput, app(in_client, PULSE_PID)); + let fwd_out = graph.node( + "fwd-out", + MediaRole::StreamOutput, + NodeProps { + pulse_module_id: Some(77), + client_id: Some(out_client), + process_id: Some(PULSE_PID), + ..NodeProps::default() + }, + ); + graph.link(hw, fwd_in); + + let decisions = run(&graph, &ctx()); + assert_eq!( + decisions.candidates[&fwd_out.serial] + .reason() + .map(Reason::code), + Some("unresolved-owner") + ); + assert!(decisions.eligible().is_empty()); +} + +#[test] +fn an_unbounded_tainted_reader_sweeps_daemon_legs_but_spares_real_apps() { + // The two-tier rule, made explicit and deliberately narrower than + // Codex round 3's "exclude everything". An unbounded tainted reader is + // necessarily daemon-owned, so its sibling is another daemon leg — a + // *bounded module output* here, carrying a strong key that does not + // match the reader. An ordinary app with its own PID is provably a + // different owner and stays shareable. + let mut graph = Graph::new(); + let hw = graph.device_node("hw", MediaRole::Sink); + let call = graph.peerspeak_node("call", 7); + graph.link(call, hw); + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11114); + graph.link(firefox, hw); + // A bounded module output leg (strong key 88, daemon PID) with no + // matching reader — a possible sibling of the unbounded reader below. + let daemon_leg = graph.module_node("daemon-leg", MediaRole::StreamOutput, 88); + graph.link(daemon_leg, hw); + + // Without an unbounded reader, both non-peerspeak outputs are eligible. + assert_partition( + &run(&graph, &ctx()), + &[("firefox", firefox), ("daemon-leg", daemon_leg)], + &[("call", call, "peerspeak-owned")], + ); + + // Add a keyless daemon-PID forwarder reading the call. + let ghost_client = graph.client(Some(PULSE_PID)); + let leak_in = graph.node( + "leak-in", + MediaRole::StreamInput, + app(ghost_client, PULSE_PID), + ); + graph.link(hw, leak_in); + assert_partition( + &run(&graph, &ctx()), + &[("firefox", firefox)], + &[ + ("call", call, "peerspeak-owned"), + ("daemon-leg", daemon_leg, "unresolved-owner"), + ], + ); +} + +#[test] +fn an_unknown_daemon_pid_makes_an_unbounded_reader_exclude_everything() { + // The fallback: with the daemon PID unknown we cannot tell a real app + // from a module leg, so the rule degrades to Codex's "exclude all". + let mut graph = Graph::new(); + let hw = graph.device_node("hw", MediaRole::Sink); + let call = graph.peerspeak_node("call", 7); + graph.link(call, hw); + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11114); + graph.link(firefox, hw); + let ghost_client = graph.client(None); + let leak_in = graph.node("leak-in", MediaRole::StreamInput, app(ghost_client, 0)); + graph.link(hw, leak_in); + + let decisions = run( + &graph, + &ExclusionCtx { + pipewire_pulse_pid: None, + ..ctx() + }, + ); + // With pulse PID unknown, `app(_, 0)` has no suppression so PID 0 is a + // usable key and the reader is bounded... so force the reader unbounded + // by giving it no PID at all. + let _ = decisions; + let mut graph = Graph::new(); + let hw = graph.device_node("hw", MediaRole::Sink); + let call = graph.peerspeak_node("call", 7); + graph.link(call, hw); + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11114); + graph.link(firefox, hw); + let keyless_client = graph.client(None); + let leak_in = graph.node( + "leak-in", + MediaRole::StreamInput, + NodeProps { + client_id: Some(keyless_client), + ..NodeProps::default() + }, + ); + graph.link(hw, leak_in); + assert_partition( + &run( + &graph, + &ExclusionCtx { + pipewire_pulse_pid: None, + ..ctx() + }, + ), + &[], + &[ + ("call", call, "peerspeak-owned"), + ("firefox", firefox, "unresolved-owner"), + ], + ); +} + +#[test] +fn a_strong_key_new_connection_of_a_still_tainted_owner_inherits_the_taint() { + // Round 3 finding 5: a mutation that kept only PID fingerprints survived + // the 49-test suite, because no fixture exercised a *strong-key* + // fingerprint reaching a new connection. Here the owner is tainted via + // its `pulse.module.id`, all its nodes vanish, its client stays live, + // and a second client opens a new leg carrying the same module id. + 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); + let mod_in = graph.module_node("mod-in", MediaRole::StreamInput, 77); + let mod_out = graph.module_node("mod-out", MediaRole::StreamOutput, 77); + graph.link(hw, mod_in); + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11114); + + let c = ctx(); + let (_, sticky) = evaluate(&graph.build(), &c, &StickyState::default()); + + // The module's original client stays live; a new connection carries the + // same module id. The daemon PID is suppressed, so only the module-id + // fingerprint can catch this. + let new_client = graph.client(Some(PULSE_PID)); + let late = graph.node( + "mod-out-late", + MediaRole::StreamOutput, + super::fixture::pulse_module(new_client, 77, PULSE_PID), + ); + let (next, _) = evaluate(&graph.build_without(&[mod_in, mod_out]), &c, &sticky); + assert_partition( + &next, + &[("firefox", firefox)], + &[ + ("call", call, "peerspeak-owned"), + ("late", late, "tainted-owner-bridge"), + ], + ); +}