host/taint: close the inverse asymmetric leak; strengthen contracts (Codex round 3)
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 <noreply@anthropic.com>
This commit is contained in:
+57
-12
@@ -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<u32>,
|
||||
keys: &owner::OwnerKeyIndex,
|
||||
edges: &Edges,
|
||||
taint: &mut BTreeMap<Serial, Reason>,
|
||||
) -> 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);
|
||||
}
|
||||
}
|
||||
|
||||
+41
-16
@@ -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,
|
||||
}
|
||||
|
||||
|
||||
@@ -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"),
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user