From 2183084ec80f20a9f8c0bf7a1f7caabe7f9642a0 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Tue, 21 Jul 2026 17:54:08 -0400 Subject: [PATCH] host/taint: concede the unbounded-reader rule; pin link-group fingerprints (Codex round 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 4 adjudicated my three round-3 pushbacks. Codex ruled: F2 bool seam sufficient (YES), F3 accepted as a phase-3 contract not a phase-2 blocker (YES) — but my F1 narrowing was unsound (NO), with a clean counterexample. F1 (conceded): I had narrowed "unbounded tainted reader ⇒ exclude every output" to spare outputs carrying a real, non-daemon PID, arguing an unbounded reader must be daemon-owned. Codex refuted it: `application.process.id` is optional and client-controlled, so one real process can present NO pid on its reading leg (unbounded) and a real pid on its output leg — the narrowing spares that output and leaks the call. App properties cannot carry a soundness argument; only `pipewire.*` has protected identity. Reverted to the broad rule: an unbounded tainted reader excludes the whole candidate universe. Added the exact counterexample as a test (`a_real_app_with_no_pid_on_its_reader_leg...`) and kept a bounded-reader test to show the round-1 blast-radius guarantee still holds for the bounded tier. F2 (doc corrected): removed the "a mis-classified filter is still braced" claim — Codex showed a filter with no shared strong key, wrongly marked `session_device`, cannot trip the backstop from its reading leg and leaks through a differently-keyed output. A false positive is now documented as leak-capable; the only defence is the correct positive classifier. F3 (link-group fingerprint, pinned): a mutation dropping LinkGroup fingerprints survived all 53 tests, because the strong-key fingerprint test used pulse.module.id. Added a link-group new-connection test. Mutation-verified 2/2. 55 tests. Phase-2 open item is now only F3-as-phase-3-contract, which Codex accepted is not a phase-2 blocker. Co-Authored-By: Claude Opus 4.8 --- src/host/taint/mod.rs | 50 ++++------ src/host/taint/snapshot.rs | 15 +-- src/host/taint/tests.rs | 191 +++++++++++++++++++++++-------------- 3 files changed, 149 insertions(+), 107 deletions(-) diff --git a/src/host/taint/mod.rs b/src/host/taint/mod.rs index 0cfa063..480b4f2 100644 --- a/src/host/taint/mod.rs +++ b/src/host/taint/mod.rs @@ -361,8 +361,7 @@ 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, ctx.pipewire_pulse_pid, &keys, &edges, &mut taint); + changed |= propagate_unresolved_owner(snapshot, &keys, &edges, &mut taint); if !changed { break; } @@ -678,25 +677,25 @@ fn propagate_owner_bridge( /// 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. +/// its re-emitting leg could be **any** output on the box, and no property +/// on an output leg can prove it is unrelated. → exclude every output +/// candidate. /// -/// ⚠️ 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. +/// ⚠️ I tried to narrow this to "daemon-owned outputs only", on the +/// theory that an unbounded reader must be daemon-owned (a real app has a +/// PID, which would bound it) so a real-PID output is provably a different +/// owner. **Codex refuted it (round 4):** `application.process.id` is +/// optional and client-controlled, so a real process can present *no* PID +/// on its reading leg (unbounded) and a real PID on its output leg — one +/// owner, spared by the narrowing, leaking the call. Only `pipewire.*` +/// properties have protected identity; app properties cannot carry a +/// soundness argument. So: exclude everything. The trigger is genuinely +/// anomalous — a keyless reader actively consuming the call; EasyEffects +/// and loopbacks carry a `node.link-group` and are *bounded*, so they do +/// not trip this tier — and phase 5's dry run surfaces it before it can +/// gate anything real. fn propagate_unresolved_owner( snapshot: &GraphSnapshot, - ctx_pulse_pid: Option, keys: &owner::OwnerKeyIndex, edges: &Edges, taint: &mut BTreeMap, @@ -717,18 +716,9 @@ fn propagate_unresolved_owner( } let mut changed = false; for node in snapshot.nodes() { - 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 { + if node.role == MediaRole::StreamOutput + && (has_unbounded_tainted_reader || !keys.is_bounded(node.serial)) + { changed |= raise(taint, node.serial, Reason::UnresolvedOwner); } } diff --git a/src/host/taint/snapshot.rs b/src/host/taint/snapshot.rs index 2c06174..328a12a 100644 --- a/src/host/taint/snapshot.rs +++ b/src/host/taint/snapshot.rs @@ -151,12 +151,15 @@ pub struct NodeProps { /// 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. + /// ⚠️ **A false positive is leak-capable — do not treat it as braced.** + /// I claimed a mis-classified filter could not leak because its legs + /// share a `node.link-group` (strong-key bridge) or trip the unbounded + /// backstop. Codex refuted it (round 4): a filter *without* a shared + /// strong key, marked `session_device=true`, cannot activate the + /// backstop from its reading leg, so a differently-keyed re-emitting leg + /// leaks. Those braces catch *some* shapes, not all. The only real + /// defence is a correct classifier — hence "positive high-confidence" + /// and "fail closed to false" above, without exception. /// /// What it is for: every real device node shares the session manager's /// `client.id`, so coarse owner keys must not bridge them — else diff --git a/src/host/taint/tests.rs b/src/host/taint/tests.rs index 3c28fcc..3b6ac86 100644 --- a/src/host/taint/tests.rs +++ b/src/host/taint/tests.rs @@ -683,11 +683,17 @@ fn firefox_three_cases() { } #[test] -fn an_unbounded_module_forwarder_fails_closed() { +fn an_unbounded_module_forwarder_fails_closed_and_sweeps_the_desktop() { // v3.4 §12: "a module forwarder with neither link-group nor // pulse.module.id ⇒ unresolved ⇒ excluded". Its process id is the // daemon's (so key 4 is suppressed) and its legs carry different // `client.id`s, so nothing can enumerate its siblings. + // + // ⚠️ Because the reading leg is itself *unbounded*, the whole desktop's + // output is swept — an ordinary app is NOT spared (Codex round 4: a + // real app can present no PID on its reader leg, so "unbounded ⇒ not an + // app" is unsound; over-exclude instead). The trigger is anomalous: a + // keyless reader actively consuming the call. let mut graph = Graph::new(); let hw = graph.device_node("hw-sink", MediaRole::Sink); let call = graph.peerspeak_node("peerspeak", 7); @@ -703,8 +709,35 @@ fn an_unbounded_module_forwarder_fails_closed() { ); graph.link(hw, fwd_in); - // The blast radius must stay small: an ordinary app is bounded by its - // own PID and must not be swept up. + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11114); + graph.link(firefox, hw); + + assert_partition( + &run(&graph, &ctx()), + &[], + &[ + ("call", call, "peerspeak-owned"), + ("fwd-out", fwd_out, "unresolved-owner"), + ("firefox", firefox, "unresolved-owner"), + ], + ); +} + +#[test] +fn a_bounded_tainted_reader_leaves_ordinary_apps_alone() { + // The blast-radius guarantee survives for the *bounded* tier (round-1 + // finding 4): a tainted reader with a real strong key excludes only the + // unbounded output legs that could share its identity, not real apps. + // Here an EasyEffects-shaped reader (link-group) reads the call, and its + // own re-emitting leg is excluded via the bridge — but firefox is not. + 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 ee_in = graph.group_node("ee-in", MediaRole::StreamInput, "ee", 5000); + let ee_out = graph.group_node("ee-out", MediaRole::StreamOutput, "ee", 5000); + graph.link(hw, ee_in); + graph.link(ee_out, hw); let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11114); graph.link(firefox, hw); @@ -713,7 +746,7 @@ fn an_unbounded_module_forwarder_fails_closed() { &[("firefox", firefox)], &[ ("call", call, "peerspeak-owned"), - ("fwd-out", fwd_out, "unresolved-owner"), + ("ee-out", ee_out, "tainted-owner-bridge"), ], ); } @@ -1534,81 +1567,30 @@ fn an_inverse_asymmetric_forwarder_fails_closed() { } #[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. +fn an_unbounded_tainted_reader_excludes_every_output() { + // Round 4: conceded to Codex. An unbounded tainted reader could be a + // real app that simply exposed no PID on its reader leg, so no output + // property proves independence — exclude the whole candidate universe. 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. + // A bounded module leg — even a strong key does not spare it. 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. + // Without an unbounded reader, both 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); + // Add a keyless reader with no PID at all — the exact shape the round-3 + // narrowing would have mishandled. + let keyless_client = graph.client(Some(PULSE_PID)); let leak_in = graph.node( "leak-in", MediaRole::StreamInput, @@ -1619,17 +1601,45 @@ fn an_unknown_daemon_pid_makes_an_unbounded_reader_exclude_everything() { ); graph.link(hw, leak_in); assert_partition( - &run( - &graph, - &ExclusionCtx { - pipewire_pulse_pid: None, - ..ctx() - }, - ), + &run(&graph, &ctx()), &[], &[ ("call", call, "peerspeak-owned"), ("firefox", firefox, "unresolved-owner"), + ("daemon-leg", daemon_leg, "unresolved-owner"), + ], + ); +} + +#[test] +fn a_real_app_with_no_pid_on_its_reader_leg_does_not_leak() { + // Codex round 4's exact counterexample to the narrowing I tried: one + // process, reader leg with no PID (unbounded), output leg with a real + // PID. The narrowing spared the output; the broad rule excludes it. + 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 reader = graph.node( + "reader", + MediaRole::StreamInput, + NodeProps { + client_id: Some(in_client), + ..NodeProps::default() + }, + ); + let leaky_out = graph.node("leaky-out", MediaRole::StreamOutput, app(out_client, 4321)); + graph.link(hw, reader); + + assert_partition( + &run(&graph, &ctx()), + &[], + &[ + ("call", call, "peerspeak-owned"), + ("leaky-out", leaky_out, "unresolved-owner"), ], ); } @@ -1672,3 +1682,42 @@ fn a_strong_key_new_connection_of_a_still_tainted_owner_inherits_the_taint() { ], ); } + +#[test] +fn a_link_group_new_connection_of_a_still_tainted_owner_inherits_the_taint() { + // Round 4 finding 3: a mutation dropping LinkGroup fingerprints (keeping + // only module/PID) survived the 53-test suite, because the strong-key + // fingerprint test used `pulse.module.id`, not `node.link-group`. Here a + // link-group owner reads the call, its nodes vanish, its client stays + // live, and a new client opens a leg with the same link-group and a + // suppressed PID — only the link-group fingerprint can catch it. + 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 g_in = graph.group_node("g-in", MediaRole::StreamInput, "filter-1", 6200); + let g_out = graph.group_node("g-out", MediaRole::StreamOutput, "filter-1", 6200); + graph.link(hw, g_in); + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 11114); + + let c = ctx(); + let (_, sticky) = evaluate(&graph.build(), &c, &StickyState::default()); + + // New connection, same link-group, daemon PID (suppressed) so no usable + // PID remains — the module/PID fingerprints cannot reach it. + let new_client = graph.client(Some(PULSE_PID)); + let late = graph.node( + "g-out-late", + MediaRole::StreamOutput, + super::fixture::link_group("filter-1", new_client, PULSE_PID), + ); + let (next, _) = evaluate(&graph.build_without(&[g_in, g_out]), &c, &sticky); + assert_partition( + &next, + &[("firefox", firefox)], + &[ + ("call", call, "peerspeak-owned"), + ("late", late, "tainted-owner-bridge"), + ], + ); +}