diff --git a/src/host/taint/mod.rs b/src/host/taint/mod.rs index a9e1054..a4a79d4 100644 --- a/src/host/taint/mod.rs +++ b/src/host/taint/mod.rs @@ -904,7 +904,10 @@ fn propagate_owner_bridge( /// 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 +/// - A *bounded* tainted reader has a strong key, or a usable PID **backed by +/// a resolved Client** (F11-1 — a node's self-claimed +/// `application.process.id` no longer bounds anything on its own; see +/// [`owner::owner_is_bounded`]), 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 diff --git a/src/host/taint/owner.rs b/src/host/taint/owner.rs index 464d4d0..f592a46 100644 --- a/src/host/taint/owner.rs +++ b/src/host/taint/owner.rs @@ -130,6 +130,22 @@ impl OwnerCtx { fn client_pid(&self, node: &NodeSnapshot) -> Option { self.client_pids.get(&node.props.client_id?).copied() } + + /// Does this node have **protected provenance** — an unambiguous Client + /// yielding `Some(pipewire.sec.pid)`? + /// + /// ⚠️ Read **before** the pipewire-pulse suppression in [`keys_of`], and + /// that ordering is the whole rule (F11-1, below). A Pulse-emulated app's + /// Client resolves to the daemon's PID; the value is then omitted from the + /// bridge keys as too coarse to *group* on, but it is still a protected + /// `pipewire.*` answer to "who is this", so the app keeps its provenance. + /// + /// ❌ Not "a unique Client object exists". A unique Client with + /// `sec_pid = None` satisfies that and carries no protected identity at + /// all, which is exactly the hole [`owner_is_bounded`] closes. + fn client_is_resolved(&self, node: &NodeSnapshot) -> bool { + self.client_pid(node).is_some() + } } /// Which key bridged two legs. Ordered strongest first; the `Ord` derive is @@ -257,10 +273,59 @@ fn keys_of(node: &NodeSnapshot, ctx: &OwnerCtx) -> Vec<(OwnerKey, KeyValue)> { /// the engine must fail closed rather than declare it clean /// (v3.4 §6.1.1, final paragraph). /// -/// # 🔴 OPEN, phase-6 blocking — the key union can *reduce* taint here +/// # F11-1 — CLOSED. A self-claimed PID is not provenance /// -/// **Round 11 review, finding 1. Verified correct; deliberately not fixed in -/// that round.** Round 10 made key 4 a union of the node's +/// **The rule, implemented below:** a strong key (`node.link-group`, +/// `pulse.module.id`) bounds an owner on its own; **key 4 bounds an owner only +/// when the node's Client resolves** — see [`OwnerCtx::client_is_resolved`]. +/// A node whose Client cannot be resolved at all is *unbounded*, whatever +/// `application.process.id` it puts on itself. +/// +/// The five Client cases, which is the matrix this needed (two of them are +/// indistinguishable under the wrong reading of "resolves"): +/// +/// | Client | node's own pid | bounded? | why | +/// | --- | --- | --- | --- | +/// | **absent** | claimed | **no** | nothing corroborates the claim | +/// | **ambiguous** (two Clients, one id) | claimed | **no** | "we do not know who owns this" must not be papered over | +/// | **unique but pid-less** | claimed | **no** | a Client object is not an identity; `sec_pid` is | +/// | **resolved-native** (`sec_pid` = the app's) | absent | **yes** | protected pid, and it *is* key 4 | +/// | **resolved-to-pipewire-pulse** | claimed | **yes** | protected provenance; the daemon pid is suppressed as a *grouping* key only | +/// +/// The last row is what keeps this from being the blunt fix. Applying +/// "self-claims are not sound" without the provenance test unbounds every +/// Pulse-emulated app — their Client's `sec_pid` is the daemon's and +/// suppressed, so the node's own claim is their only per-app identity — which +/// re-triggers the §6.1.1 mass over-exclusion the whole design exists to avoid +/// and empties the eligible half of the §5.1 matrix. +/// +/// **Cost, measured on the live graph** (2026-07-26): **zero**. The +/// before- and after-binaries audited the *same* graph simultaneously — both +/// are read-only observers, which is the only way to A/B a partition without +/// churn between runs — with a tagged producer feeding the default sink, +/// `parec` on its monitor as a real tainted reader (so the sweep was armed, +/// not merely present in the code), and Firefox, `aplay` and `pacat` as +/// bystanders. **181 records each, the same 14 distinct decision states, none +/// exclusive to either side, no `unresolved-owner` on either.** The eligible +/// half stayed non-empty throughout: native (`aplay`), Pulse-emulated +/// (`pacat`) and Firefox all eligible. O5 is unmoved: identical p50 (15 µs) +/// and busy fraction (0.0012), and the after-binary's worst per-record +/// recompute was *lower* (217 µs vs 243 µs — noise, same debug build, same +/// concurrent load). +/// +/// Why it costs nothing here: every real app on this box is either native +/// (Client `sec_pid` = its own pid) or Pulse-emulated (Client `sec_pid` = the +/// daemon's), and **both resolve**. Sweeping all 18 live nodes for the +/// predicate's inputs directly, the only unresolved-Client nodes were +/// `Dummy-Driver` and `Freewheel-Driver`, which carry no pid key to lose; +/// session-manager device nodes are unresolved too (their Client is pid-less) +/// but exception 2 already strips key 4 from them. That is the answer the +/// deferral was waiting for: the rule bites exactly the anomalous shapes, and +/// this host has none. +/// +/// ## The leak it closes (round 11 review, finding 1) +/// +/// Round 10 made key 4 a union of the node's /// `application.process.id` and its Client's `pipewire.sec.pid`, and the claim /// that this was "strictly additive" was too strong: the same key list also /// feeds *this* predicate, so adding a value can move a node from unbounded to @@ -277,55 +342,50 @@ fn keys_of(node: &NodeSnapshot, ctx: &OwnerCtx) -> Vec<(OwnerKey, KeyValue)> { /// 3. Neither the bridge nor the sweep fires, and the output stays eligible /// while re-emitting the call. /// -/// It cannot leak today: `evaluate()` is reached only by the dry-run audit, -/// which creates no links. It becomes live when phase 6 consumes eligibility. +/// Step 2 is now unbounded ⇒ the sweep fires ⇒ the leg is excluded. Note it +/// could not leak *yet* when it was filed — `evaluate()` is reached only by the +/// dry-run audit, which creates no links — and that is why the fix waited for +/// the §5.1 measurement instead of guessing at its cost. /// -/// **Why it is not fixed yet.** The principled repair is provenance: a -/// self-claimed `application.process.id` is not a *sound* bound, only the -/// protected keys are. But applying that bluntly makes every Pulse-emulated -/// app unbounded — their Client's `sec_pid` is the daemon's and suppressed, so -/// the node's own claim is their only per-app identity — which re-triggers the -/// §6.1.1 mass over-exclusion the whole design is built to avoid, and would -/// make the eligible half of the §5.1 matrix empty. +/// ## What this is deliberately NOT /// -/// The targeted rule that closes the path above without that cost: **a node -/// whose Client cannot be resolved at all must not be bounded by its own -/// self-claimed PID.** An ambiguous Client already means "we do not know who -/// owns this", and a self-claim must not paper over it; a Pulse app's Client -/// *is* resolved (to the daemon's PID, then suppressed), so it keeps its -/// bound. Implementing it needs `OwnerCtx` to distinguish "resolved" from -/// "absent", and `OwnerKeyIndex` to carry boundedness separately from the key -/// set, since bridging must keep using the full union. +/// It is not a claim that `application.process.id` is now unused: it still +/// bridges (a self-claim is fine as *evidence that two legs are related* — +/// the fail-closed direction), and a resolved-Client node is still bounded by +/// whichever key-4 value survives suppression. Only *boundedness* — the +/// permission to say "I can enumerate this owner's other legs, so a +/// differently-keyed output is provably someone else" — now demands a +/// `pipewire.*` answer to "who is this". /// -/// ⚠️ Do this **with the §5.1 matrix data in hand**, not before: the whole -/// question is how much over-exclusion the rule actually causes on a real -/// graph, and that is measurable rather than arguable. -/// -/// ## Round 12 — the deferral holds, and "resolved" has a trap in it -/// -/// Codex re-examined this and agreed the deferral is defensible while -/// `evaluate()` is audit-only, and that the rule above closes the recorded path -/// without unbounding normal Pulse-emulated apps — **but only under one -/// reading of "resolves"**, and the wrong reading reintroduces the hole: -/// -/// - ✅ "Resolved" must mean **an unambiguous Client that yields -/// `Some(pipewire.sec.pid)`**, taken *before* the pipewire-pulse suppression -/// step. A Pulse app then still has the daemon's protected PID as -/// provenance, even though that value is omitted from the bridge keys, so it -/// stays bounded and the eligible half survives. -/// - ❌ **Do not** implement it as "a unique Client object exists". A unique -/// Client with `sec_pid = None` would satisfy that test while providing no -/// protected identity at all, leaving exactly the self-claimed-PID hole this -/// rule is meant to close. -/// -/// So the matrix needs five Client cases, not two: **absent**, **ambiguous**, -/// **unique but pid-less**, **resolved-native**, and -/// **resolved-to-pipewire-pulse**. The third is the one that distinguishes the -/// two readings, and it is the row a two-case matrix would silently skip. +/// ⚠️ Bridging must keep using the **full** union, so boundedness is carried +/// separately from the key set in [`OwnerKeyIndex`] rather than being +/// re-derived from it. pub fn owner_is_bounded(node: &NodeSnapshot, ctx: &OwnerCtx) -> bool { - keys_of(node, ctx) - .iter() - .any(|(key, _)| *key != OwnerKey::ClientId) + bounded_by(&keys_of(node, ctx), node, ctx) +} + +/// [`owner_is_bounded`]'s rule, over an already-computed key list. +/// +/// The single implementation: [`OwnerKeyIndex::build`] has the keys in hand and +/// must not recompute them, and two copies of a predicate this load-bearing is +/// how the two spellings drift apart. +fn bounded_by(keys: &[(OwnerKey, KeyValue)], node: &NodeSnapshot, ctx: &OwnerCtx) -> bool { + let mut has_process_key = false; + for (key, _) in keys { + match key { + // Strong keys are per-instance and name the sibling set directly. + OwnerKey::LinkGroup | OwnerKey::PulseModuleId => return true, + OwnerKey::ProcessId => has_process_key = true, + // Never: one process can present two `client.id`s (the measured + // GStreamer refutation, above). + OwnerKey::ClientId => {} + } + } + // F11-1. The key may be the node's own claim, the Client's protected pid, + // or both — `keys_of` does not record which, and it does not need to: a + // resolved Client is provenance for the node *whatever* value key 4 ends + // up carrying, and without one there is no protected identity to stand on. + has_process_key && ctx.client_is_resolved(node) } /// Owner keys computed once per snapshot. @@ -336,16 +396,27 @@ pub fn owner_is_bounded(node: &NodeSnapshot, ctx: &OwnerCtx) -> bool { #[derive(Debug, Default)] pub struct OwnerKeyIndex { keys: BTreeMap>, + /// Nodes whose owner is positively bounded — see [`owner_is_bounded`]. + /// + /// ⚠️ **Stored, not derived from `keys`.** Since F11-1 the predicate needs + /// the node's Client as well as its key list, and the two answers are + /// deliberately different: the full union still bridges, while a + /// self-claimed pid no longer bounds. + bounded: BTreeSet, } impl OwnerKeyIndex { pub fn build(snapshot: &GraphSnapshot, ctx: &OwnerCtx) -> Self { - Self { - keys: snapshot - .nodes() - .map(|node| (node.serial, keys_of(node, ctx))) - .collect(), + let mut keys = BTreeMap::new(); + let mut bounded = BTreeSet::new(); + for node in snapshot.nodes() { + let node_keys = keys_of(node, ctx); + if bounded_by(&node_keys, node, ctx) { + bounded.insert(node.serial); + } + keys.insert(node.serial, node_keys); } + Self { keys, bounded } } /// The strongest key these two nodes share directly, if any. @@ -409,11 +480,10 @@ impl OwnerKeyIndex { }) } - /// See [`owner_is_bounded`]. + /// See [`owner_is_bounded`]. A node outside this snapshot is unbounded, + /// which is the fail-closed answer. pub fn is_bounded(&self, serial: Serial) -> bool { - self.keys - .get(&serial) - .is_some_and(|keys| keys.iter().any(|(key, _)| *key != OwnerKey::ClientId)) + self.bounded.contains(&serial) } } diff --git a/src/host/taint/tests.rs b/src/host/taint/tests.rs index ac0a958..b70da24 100644 --- a/src/host/taint/tests.rs +++ b/src/host/taint/tests.rs @@ -1088,6 +1088,226 @@ fn the_nodes_own_process_id_wins_over_its_clients() { ); } +// ────────────────────────────────────────────────────────────────────── +// F11-1 — a self-claimed pid is not provenance: the five Client cases +// ────────────────────────────────────────────────────────────────────── + +/// The scaffold every F11-1 row needs: peerspeak's call reaching the hardware +/// sink, a **bounded** tainted reader, and an ordinary bystander. +/// +/// ⚠️ The reader must be *bounded* (`sunshine` carries a real pid). An +/// unbounded tainted reader trips `propagate_unresolved_owner`'s other tier, +/// which sweeps **every** output candidate on the box regardless of its own +/// keys — the three "unbounded" rows below would then pass without testing +/// anything. The bystander is the other half of that guard: it is bounded via +/// the ordinary Pulse shape, so an implementation that unbounded everything +/// fails every row instead of passing three of them. +/// +/// Returns the graph, the hardware sink to hang nodes off, and the two nodes +/// every row must name in its partition. +fn armed_with_a_bounded_reader() -> (Graph, NodeRef, NodeRef, NodeRef) { + 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 sunshine = graph.app_node("sunshine", MediaRole::StreamInput, 3_838); + graph.link(hw, sunshine); + let bystander = graph.app_node("mpv", MediaRole::StreamOutput, 9_001); + graph.link(bystander, hw); + (graph, hw, call, bystander) +} + +/// Case 1 of 5 — **Client absent.** A node that names no Client at all has +/// nothing but its own word for who owns it, so it cannot be bounded. +#[test] +fn an_absent_client_leaves_a_self_claimed_pid_unbounded() { + let (mut graph, hw, call, bystander) = armed_with_a_bounded_reader(); + let orphan = graph.node( + "no-client", + MediaRole::StreamOutput, + NodeProps { + process_id: Some(70_001), + ..NodeProps::default() + }, + ); + graph.link(orphan, hw); + + assert_partition( + &run(&graph, &ctx()), + &[("mpv", bystander)], + &[ + ("call", call, "peerspeak-owned"), + ("no-client", orphan, "unresolved-owner"), + ], + ); +} + +/// Case 2 of 5 — **Client ambiguous.** Two live Clients claim the id, so the +/// observer missed a removal and we do not know who owns this node. A +/// self-claimed pid must not paper over that: this is step 2 of the recorded +/// leak path, and before F11-1 the claim bounded the node and spared it. +#[test] +fn an_ambiguous_client_leaves_a_self_claimed_pid_unbounded() { + let (mut graph, hw, call, bystander) = armed_with_a_bounded_reader(); + let shared_id = graph.client(Some(70_010)); + graph.client_with_id(shared_id, Some(70_011)); + let app = graph.node( + "ambiguous-client", + MediaRole::StreamOutput, + NodeProps { + client_id: Some(shared_id), + process_id: Some(70_012), + ..NodeProps::default() + }, + ); + graph.link(app, hw); + + assert_partition( + &run(&graph, &ctx()), + &[("mpv", bystander)], + &[ + ("call", call, "peerspeak-owned"), + ("ambiguous-client", app, "unresolved-owner"), + ], + ); +} + +/// Case 3 of 5 — **Client unique but pid-less**, and *the row that decides +/// which rule is implemented*. +/// +/// A unique Client object exists, so "resolved = a unique Client exists" would +/// call this node bounded — leaving the self-claimed-pid hole wide open under a +/// rule that looks like it closed it. `sec_pid` is what carries protected +/// identity, so `None` means unresolved, and pid-less Clients are ordinary +/// (the session manager's is one). +/// +/// A two-case absent/resolved matrix skips this silently. That is why it is +/// written out. +#[test] +fn a_unique_but_pidless_client_leaves_a_self_claimed_pid_unbounded() { + let (mut graph, hw, call, bystander) = armed_with_a_bounded_reader(); + let pidless = graph.client(None); + let app = graph.node( + "pidless-client", + MediaRole::StreamOutput, + NodeProps { + client_id: Some(pidless), + process_id: Some(70_020), + ..NodeProps::default() + }, + ); + graph.link(app, hw); + + assert_partition( + &run(&graph, &ctx()), + &[("mpv", bystander)], + &[ + ("call", call, "peerspeak-owned"), + ("pidless-client", app, "unresolved-owner"), + ], + ); +} + +/// Case 4 of 5 — **Client resolved, native.** `pipewire.sec.pid` is the app's +/// own, so provenance and key 4 are the same value and the node is bounded +/// without claiming anything itself. +#[test] +fn a_resolved_native_client_bounds_its_node() { + let (mut graph, hw, call, bystander) = armed_with_a_bounded_reader(); + let mpv2 = graph.native_client_node("mpv-native", MediaRole::StreamOutput, 70_030); + graph.link(mpv2, hw); + + assert_partition( + &run(&graph, &ctx()), + &[("mpv", bystander), ("mpv-native", mpv2)], + &[("call", call, "peerspeak-owned")], + ); +} + +/// Case 5 of 5 — **Client resolved to pipewire-pulse.** The row that stops +/// this rule from being the blunt fix. +/// +/// Every Pulse-emulated app looks like this: the Client's `sec_pid` is the +/// daemon's — suppressed as a *grouping* key, because it would fuse fifteen +/// unrelated apps — while the node's own `application.process.id` is the app's. +/// Provenance is read **before** that suppression, so the app keeps its bound +/// and stays eligible. Reading it after would unbound every Pulse app on the +/// box and empty the eligible half of the §5.1 matrix, which is the §6.1.1 +/// catastrophe arriving through the boundedness door. +#[test] +fn a_client_resolving_to_pipewire_pulse_still_bounds_its_node() { + let (mut graph, hw, call, bystander) = armed_with_a_bounded_reader(); + let firefox = graph.app_node("firefox", MediaRole::StreamOutput, 70_040); + graph.link(firefox, hw); + + assert_partition( + &run(&graph, &ctx()), + &[("mpv", bystander), ("firefox", firefox)], + &[("call", call, "peerspeak-owned")], + ); +} + +/// **The recorded leak path, end to end** (round 11 review, finding 1). +/// +/// One process, two Clients. Its reading leg claims the daemon's pid — which +/// exception 1 suppresses — while its Client holds a real protected pid `A`, so +/// the union bounds the reader by `A` and the *unbounded-reader* tier never +/// arms. Its re-emitting leg sits on a second Client whose id is **ambiguous** +/// (one of the two claimants even holds `A`, so this is not "the guess would +/// have been wrong" — it is "a guess is not evidence"), and claims a pid of its +/// own. The two legs share no key, so the bridge does not fire either. +/// +/// Before F11-1 the self-claim bounded the output leg, both tiers stayed quiet, +/// and it re-emitted the call while eligible. Now the leg is unbounded, the +/// bounded-reader tier sweeps it, and `mpv` shows the sweep is still targeted. +#[test] +fn a_self_claimed_pid_cannot_spare_an_output_leg_the_bridge_cannot_reach() { + 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 reader_client = graph.client(Some(80_000)); + let reader = graph.node( + "forwarder-in", + MediaRole::StreamInput, + NodeProps { + client_id: Some(reader_client), + process_id: Some(PULSE_PID), + ..NodeProps::default() + }, + ); + graph.link(hw, reader); + + let ambiguous = graph.client(Some(80_000)); + graph.client_with_id(ambiguous, Some(80_001)); + let output = graph.node( + "forwarder-out", + MediaRole::StreamOutput, + NodeProps { + client_id: Some(ambiguous), + process_id: Some(80_002), + ..NodeProps::default() + }, + ); + graph.link(output, hw); + + let bystander = graph.app_node("mpv", MediaRole::StreamOutput, 9_001); + graph.link(bystander, hw); + + // `mpv` staying eligible is what proves the reader is bounded: an + // unbounded tainted reader sweeps **every** output candidate, `mpv` + // included, and this row would then be testing the wrong tier. + assert_partition( + &run(&graph, &ctx()), + &[("mpv", bystander)], + &[ + ("call", call, "peerspeak-owned"), + ("forwarder-out", output, "unresolved-owner"), + ], + ); +} + #[test] fn an_unknown_pipewire_pulse_pid_over_excludes_rather_than_leaks() { // v3.4 §6.1.2's failure-mode paragraph: if pixelpass cannot identify