ipc: close unauthenticated event-spoofing gap in emit's no-source path
The IPC "emit" method's no-source path took a bare event+data and sent it
straight to emit_tx tagged AdapterSource::System with zero validation of
the event name. Any same-UID process on the socket could send e.g.
{"event":"bread.power.ac.connected",...} and have it delivered to every
Lua subscriber indistinguishable from a real adapter event, since System
is the same tag the daemon uses for its own trusted, Rust-originated
sends (bread.system.startup, bread.profile.activated).
Fix, scoped to the actual threat model (same-UID Unix socket trust means
there's no way to cryptographically distinguish "the real bread-cli
binary" from any other local process, so a generic connection-identity
handshake would be theater):
- New AdapterSource::Manual tag for the no-source emit path. System is
now reserved for daemon-internal, Rust-code-originated sends only and
can never again be produced from data that arrived over the wire.
- The event name is rejected if its top-level dotted segment is one of
the reserved, adapter-owned domains (RESERVED_DOMAINS in
bread-shared/src/apps.rs) -- extended with bluetooth/workspace/window/
monitor, event families the Hyprland and Bluetooth adapters already
publish under but that were missing from that list. Freely-named
custom/test event names are untouched, so `bread emit <name>` and
bread-emit's fire-and-forget single-line-write design keep working
exactly as documented.
Bumped API_VERSION to 1.5.0 and updated Documentation.md's IPC emit
section and Namespaces reserved-domains list accordingly.
Also fixed a subscribe/emit race that surfaced while adding regression
tests for this: events.subscribe's ack is written to the client before
the server task actually registers on the broadcast channel, so a test
that emits immediately after reading the ack can race the registration.
Added a settle delay plus an explicit timeout (instead of an unbounded
read loop) so a future regression fails the test instead of hanging the
whole binary.
This commit is contained in:
parent
96639516b1
commit
7bb6fbb20f
6 changed files with 281 additions and 11 deletions
|
|
@ -45,6 +45,19 @@ impl EventNormalizer {
|
|||
source: raw.source.clone(),
|
||||
data: raw.payload.clone(),
|
||||
}],
|
||||
// `Manual` is never constructed as a `RawEvent::source` in this
|
||||
// codebase — the IPC boundary's unsourced `emit` path builds a
|
||||
// `BreadEvent` directly (see `breadd/src/ipc/mod.rs`), bypassing
|
||||
// this normalizer entirely, exactly as `System` did before it.
|
||||
// This arm exists only so the match stays exhaustive; if that
|
||||
// ever changes, pass-through (like `System`) is the sane default
|
||||
// rather than silently dropping the event.
|
||||
AdapterSource::Manual => vec![BreadEvent {
|
||||
event: raw.kind.clone(),
|
||||
timestamp: raw.timestamp,
|
||||
source: raw.source.clone(),
|
||||
data: raw.payload.clone(),
|
||||
}],
|
||||
};
|
||||
|
||||
out.retain(|ev| self.accept(ev));
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ use std::sync::Arc;
|
|||
use std::time::Instant;
|
||||
|
||||
use anyhow::{anyhow, Result};
|
||||
use bread_shared::apps::{is_known_app, validate_app_namespace};
|
||||
use bread_shared::apps::{event_domain, is_known_app, is_reserved_domain, validate_app_namespace};
|
||||
use bread_shared::{now_unix_ms, AdapterSource, BreadEvent, RawEvent};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use serde_json::{json, Value};
|
||||
|
|
@ -27,7 +27,7 @@ use crate::lua::RuntimeHandle;
|
|||
/// something new-but-additive (a binding, an event, an IPC param); bump the
|
||||
/// major version only for a breaking change, which should not happen inside
|
||||
/// this daemon's v1 lifetime per that section's stated policy.
|
||||
const API_VERSION: &str = "1.4.0";
|
||||
const API_VERSION: &str = "1.5.0";
|
||||
|
||||
#[derive(Clone)]
|
||||
pub struct Server {
|
||||
|
|
@ -323,12 +323,35 @@ impl Server {
|
|||
}
|
||||
Ok(json!({ "emitted": true }))
|
||||
} else {
|
||||
// Unsourced emit: the manual-testing path ("bread emit
|
||||
// <event>", used to poke Lua handlers without unplugging
|
||||
// cables). Tagged `Manual`, never `System` — `System` is
|
||||
// reserved for events the daemon originates itself in
|
||||
// Rust code (e.g. `bread.system.startup` in `serve()`
|
||||
// above), not for anything that arrived over the wire.
|
||||
// A socket client can still name any custom/test event
|
||||
// it likes, but not one whose top-level segment is a
|
||||
// reserved, adapter-owned domain (`bread.power.*`,
|
||||
// `bread.hyprland.*`, ...) — otherwise this path would
|
||||
// let any same-UID process impersonate a real adapter
|
||||
// event with nothing downstream able to tell the
|
||||
// difference.
|
||||
let Some(event) = req.params.get("event").and_then(Value::as_str) else {
|
||||
return Err((id, "missing event name".to_string()));
|
||||
};
|
||||
if let Some(domain) = event_domain(event) {
|
||||
if is_reserved_domain(domain) {
|
||||
return Err((
|
||||
id,
|
||||
format!(
|
||||
"event '{event}' claims the reserved '{domain}' domain — manual emit cannot impersonate an adapter-owned event; use a custom event name, or a sourced emit if this should go through the normalizer"
|
||||
),
|
||||
));
|
||||
}
|
||||
}
|
||||
if self
|
||||
.emit_tx
|
||||
.send(BreadEvent::new(event, AdapterSource::System, data))
|
||||
.send(BreadEvent::new(event, AdapterSource::Manual, data))
|
||||
.is_err()
|
||||
{
|
||||
return Err((id, "emit channel closed".to_string()));
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ use serde_json::{json, Value};
|
|||
use tempfile::TempDir;
|
||||
use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
|
||||
use tokio::net::UnixStream;
|
||||
use tokio::time::sleep;
|
||||
use tokio::time::{sleep, timeout};
|
||||
|
||||
#[tokio::test]
|
||||
async fn ping_and_state_dump_work() -> Result<()> {
|
||||
|
|
@ -99,6 +99,155 @@ async fn emit_without_event_errors() -> Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn emit_without_source_rejects_adapter_owned_event_name() -> Result<()> {
|
||||
let harness = TestHarness::spawn()?;
|
||||
harness.wait_until_ready().await?;
|
||||
|
||||
// A no-`source` emit must not be able to impersonate a real adapter
|
||||
// event by event name alone — "power" is a reserved, adapter-owned
|
||||
// domain (see `bread_shared::apps::RESERVED_DOMAINS`).
|
||||
let result = harness
|
||||
.send_request(
|
||||
"emit",
|
||||
json!({ "event": "bread.power.ac.connected", "data": { "ac_connected": true } }),
|
||||
)
|
||||
.await;
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"manual emit must not be able to claim an adapter-owned event name"
|
||||
);
|
||||
let msg = result.err().unwrap().to_string();
|
||||
assert!(msg.contains("reserved"), "got: {msg}");
|
||||
|
||||
harness.shutdown();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn emit_without_source_rejects_every_adapter_owned_domain() -> Result<()> {
|
||||
let harness = TestHarness::spawn()?;
|
||||
harness.wait_until_ready().await?;
|
||||
|
||||
// Every namespace a real adapter (or the daemon itself) publishes
|
||||
// under must be closed to the unsourced emit path, not just "power".
|
||||
for event in [
|
||||
"bread.power.ac.connected",
|
||||
"bread.network.connected",
|
||||
"bread.device.connected",
|
||||
"bread.bluetooth.device.paired",
|
||||
"bread.hyprland.event",
|
||||
"bread.workspace.changed",
|
||||
"bread.monitor.connected",
|
||||
"bread.window.opened",
|
||||
"bread.system.startup",
|
||||
] {
|
||||
let result = harness
|
||||
.send_request("emit", json!({ "event": event, "data": {} }))
|
||||
.await;
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"expected '{event}' to be rejected on the unsourced emit path"
|
||||
);
|
||||
}
|
||||
|
||||
harness.shutdown();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn emit_without_source_still_allows_custom_event_names() -> Result<()> {
|
||||
let harness = TestHarness::spawn()?;
|
||||
harness.wait_until_ready().await?;
|
||||
|
||||
// The documented `bread emit <custom-name>` debug use case (testing Lua
|
||||
// handlers without unplugging cables) must keep working for any event
|
||||
// name that isn't a reserved, adapter-owned domain.
|
||||
let result = harness
|
||||
.send_request(
|
||||
"emit",
|
||||
json!({ "event": "bread.mymodule.custom_thing", "data": { "n": 1 } }),
|
||||
)
|
||||
.await;
|
||||
assert!(result.is_ok(), "custom event name must still be emittable");
|
||||
assert_eq!(
|
||||
result.unwrap().get("emitted").and_then(Value::as_bool),
|
||||
Some(true)
|
||||
);
|
||||
|
||||
harness.shutdown();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn emit_without_source_is_tagged_manual_not_system() -> Result<()> {
|
||||
let harness = TestHarness::spawn()?;
|
||||
harness.wait_until_ready().await?;
|
||||
|
||||
let stream = UnixStream::connect(harness.socket_path()).await?;
|
||||
let (read_half, mut write_half) = stream.into_split();
|
||||
let subscribe = json!({
|
||||
"id": "sub-manual",
|
||||
"method": "events.subscribe",
|
||||
"params": { "filter": "bread.mymodule.*" }
|
||||
});
|
||||
write_half
|
||||
.write_all(format!("{}\n", serde_json::to_string(&subscribe)?).as_bytes())
|
||||
.await?;
|
||||
|
||||
let mut reader = BufReader::new(read_half).lines();
|
||||
reader
|
||||
.next_line()
|
||||
.await?
|
||||
.ok_or_else(|| anyhow!("missing subscribe ack"))?;
|
||||
|
||||
// The subscribe ack is written to the client before the server task
|
||||
// actually registers on the broadcast channel (see `handle_connection`'s
|
||||
// "events.subscribe" arm — ack first, `stream_events`/`event_tx.subscribe()`
|
||||
// second). A brief settle delay avoids racing that registration under
|
||||
// full-suite parallel load, same as the settle delay already used in
|
||||
// `workflow_reaches_done_via_wait_any_happy_path` above for the same
|
||||
// class of subscribe-then-fire race.
|
||||
sleep(Duration::from_millis(100)).await;
|
||||
|
||||
harness
|
||||
.send_request(
|
||||
"emit",
|
||||
json!({ "event": "bread.mymodule.poked", "data": {} }),
|
||||
)
|
||||
.await?;
|
||||
|
||||
// Bounded by an explicit timeout (not just the `Instant`-deadline-checked
|
||||
// loop used elsewhere in this file) so a real regression here fails the
|
||||
// test in 5s instead of hanging this test binary — and therefore the
|
||||
// whole `cargo test --workspace` run — forever.
|
||||
let event = timeout(Duration::from_secs(5), async {
|
||||
loop {
|
||||
let line = reader
|
||||
.next_line()
|
||||
.await?
|
||||
.ok_or_else(|| anyhow!("event stream closed before match"))?;
|
||||
let event: Value = serde_json::from_str(&line)?;
|
||||
if event.get("event").and_then(Value::as_str) == Some("bread.mymodule.poked") {
|
||||
return Ok::<Value, anyhow::Error>(event);
|
||||
}
|
||||
}
|
||||
})
|
||||
.await
|
||||
.map_err(|_| anyhow!("timed out waiting for bread.mymodule.poked on the stream"))??;
|
||||
// A wire-triggered no-source emit must never be indistinguishable from
|
||||
// a daemon-internal `System` event (e.g. `bread.system.startup`,
|
||||
// `bread.profile.activated`) — it must carry the distinct `Manual` tag.
|
||||
assert_eq!(
|
||||
event.get("source").and_then(Value::as_str),
|
||||
Some("manual"),
|
||||
"no-source emit must be tagged 'manual', not 'system': {event:?}"
|
||||
);
|
||||
|
||||
harness.shutdown();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn emit_with_internal_source_is_rejected() -> Result<()> {
|
||||
let harness = TestHarness::spawn()?;
|
||||
|
|
@ -471,7 +620,12 @@ async fn events_stream_receives_emitted_events() -> Result<()> {
|
|||
"id": "sub-1",
|
||||
"method": "events.subscribe",
|
||||
"params": {
|
||||
"filter": "bread.system.*"
|
||||
// "system" is a reserved, adapter/daemon-owned domain (see
|
||||
// `emit_without_source_rejects_reserved_domain` below) — a
|
||||
// manual emit can no longer use it, so this test's custom
|
||||
// event lives under "custom" instead, same as it always did
|
||||
// for "match"/"nomatch"/"replay"/etc. elsewhere in this file.
|
||||
"filter": "bread.custom.*"
|
||||
}
|
||||
});
|
||||
write_half
|
||||
|
|
@ -497,7 +651,7 @@ async fn events_stream_receives_emitted_events() -> Result<()> {
|
|||
.send_request(
|
||||
"emit",
|
||||
json!({
|
||||
"event": "bread.system.test",
|
||||
"event": "bread.custom.test",
|
||||
"data": { "ok": true }
|
||||
}),
|
||||
)
|
||||
|
|
@ -510,7 +664,7 @@ async fn events_stream_receives_emitted_events() -> Result<()> {
|
|||
break;
|
||||
};
|
||||
let event: Value = serde_json::from_str(&line)?;
|
||||
if event.get("event").and_then(Value::as_str) == Some("bread.system.test") {
|
||||
if event.get("event").and_then(Value::as_str) == Some("bread.custom.test") {
|
||||
got = true;
|
||||
break;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue