diff --git a/Cargo.lock b/Cargo.lock index 028103b..51eab0f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1407,6 +1407,7 @@ dependencies = [ "omachat-proto", "serde", "serde_json", + "tempfile", "tokio", ] @@ -1558,6 +1559,7 @@ dependencies = [ "omachat-registry-host", "omachat-registry-transport", "omachat-store", + "rustix", "serde", "serde_json", "tempfile", diff --git a/SECURITY.md b/SECURITY.md index 175fbb1..80abf13 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -35,9 +35,21 @@ under the account root. A configured handle is only a local candidate. No global uniqueness, registry freshness, recovery, revocation, or key-transparency claim exists until their complete verification paths ship. -IPC is versioned, length bounded, and carried over a 0600 Unix socket. The -systemd user service restricts writable paths and privileges, but Bluetooth, -network, Unix-socket, and user-session D-Bus access remain necessary. +IPC is versioned, length bounded, and carried over a Unix socket created 0600 +under a restrictive umask. The daemon checks SO_PEERCRED on every accepted +connection and drops any peer whose uid differs from its own effective uid, +root included. Destructive commands (panic erase, registry handle claims) +additionally require a single-use confirmation token with a 120-second +lifetime, minted on request into a 0600 file inside the sealed state +directory and echoed back in a second step. These are deliberate-two-step +and freshness guarantees, not an authorization boundary: same-uid isolation +is not a boundary this daemon can enforce alone. A process running as the +same user can ultimately ptrace the daemon, read its memory, or read the +state directory that holds the confirmation tokens. Protecting the account +from hostile same-user code — including coding agents — requires OS-level +sandboxing of that code, not daemon-side checks. The systemd user service +restricts writable paths and privileges, but Bluetooth, network, +Unix-socket, and user-session D-Bus access remain necessary. ## Metadata and network limits diff --git a/crates/omachat-ctl/Cargo.toml b/crates/omachat-ctl/Cargo.toml index bacec82..c171043 100644 --- a/crates/omachat-ctl/Cargo.toml +++ b/crates/omachat-ctl/Cargo.toml @@ -12,5 +12,8 @@ serde = { version = "=1.0.229", features = ["derive"] } serde_json = "=1.0.151" tokio = { version = "=1.53.1", features = ["io-util", "macros", "net", "rt-multi-thread", "time"] } +[dev-dependencies] +tempfile = "=3.27.0" + [lints] workspace = true diff --git a/crates/omachat-ctl/src/lib.rs b/crates/omachat-ctl/src/lib.rs index e832fe9..44e6768 100644 --- a/crates/omachat-ctl/src/lib.rs +++ b/crates/omachat-ctl/src/lib.rs @@ -74,6 +74,82 @@ impl Client { } } +/// Restated at every panic invocation: erasure is local-only. +pub const PANIC_ERASE_WARNING: &str = "panic erase destroys the local master key and sealed \ +state; it cannot retract messages, keys, or metadata already replicated to relays, peers, or \ +backups"; + +/// Two-phase orchestration for destructive commands. The typed intent +/// (`ERASE`, or the handle echoed to `--confirm`) is checked locally; the +/// daemon-minted single-use token is then fetched out of band from the +/// daemon state directory and echoed back. Non-destructive commands pass +/// straight through. +pub async fn request_with_confirmation( + client: &mut Client, + command: Command, +) -> Result { + match command { + Command::Panic { confirmation } => { + if confirmation != "ERASE" { + return Err(ClientError::ConfirmationRefused( + "panic requires --confirm ERASE".into(), + )); + } + let issued = client.request(Command::RequestPanicConfirmation).await?; + let ResponseOutcome::Ok { ref result } = issued.outcome else { + return Ok(issued); + }; + let token = read_confirmation_token(result)?; + client + .request(Command::Panic { + confirmation: token, + }) + .await + } + Command::ClaimRegistryHandle { + handle, + confirmation, + } => { + if confirmation != handle { + return Err(ClientError::ConfirmationRefused( + "claim-handle requires --confirm HANDLE to echo the handle exactly".into(), + )); + } + let issued = client + .request(Command::RequestRegistryClaimConfirmation { + handle: handle.clone(), + }) + .await?; + let ResponseOutcome::Ok { ref result } = issued.outcome else { + return Ok(issued); + }; + let token = read_confirmation_token(result)?; + client + .request(Command::ClaimRegistryHandle { + handle, + confirmation: token, + }) + .await + } + other => client.request(other).await, + } +} + +fn read_confirmation_token(result: &serde_json::Value) -> Result { + let path = result + .get("token_path") + .and_then(serde_json::Value::as_str) + .ok_or_else(|| { + ClientError::ConfirmationProtocol("daemon response lacked token_path".into()) + })?; + // Token files are tens of bytes; a blocking read keeps the client free + // of a tokio fs feature dependency. + let token = std::fs::read_to_string(path).map_err(|error| { + ClientError::ConfirmationProtocol(format!("token file unreadable: {error}")) + })?; + Ok(token.trim().to_owned()) +} + async fn read_line(stream: &mut UnixStream) -> Result { let mut line = Vec::new(); let mut byte = [0_u8; 1]; @@ -103,7 +179,16 @@ pub enum ClientError { MalformedResponse, VersionMismatch(u16), CorrelationMismatch, - Remote { code: String, message: String }, + Remote { + code: String, + message: String, + }, + /// The locally typed intent (`--confirm` value) did not match; nothing + /// was sent to the daemon. + ConfirmationRefused(String), + /// The daemon's confirmation-token response was malformed or the token + /// file could not be read. + ConfirmationProtocol(String), } impl fmt::Display for ClientError { @@ -120,6 +205,10 @@ impl fmt::Display for ClientError { } Self::CorrelationMismatch => formatter.write_str("daemon response ID does not match"), Self::Remote { code, message } => write!(formatter, "daemon error {code}: {message}"), + Self::ConfirmationRefused(message) => write!(formatter, "refused: {message}"), + Self::ConfirmationProtocol(message) => { + write!(formatter, "confirmation protocol failed: {message}") + } } } } diff --git a/crates/omachat-ctl/src/main.rs b/crates/omachat-ctl/src/main.rs index 5ddb258..31d4917 100644 --- a/crates/omachat-ctl/src/main.rs +++ b/crates/omachat-ctl/src/main.rs @@ -43,7 +43,12 @@ async fn run(mut arguments: Vec) -> Result<(), CliError> { let mut client = Client::connect(socket, DEFAULT_TIMEOUT) .await .map_err(CliError::Client)?; - let response = client.request(command).await.map_err(CliError::Client)?; + if matches!(command, Command::Panic { .. }) { + eprintln!("{}", omachat_ctl::PANIC_ERASE_WARNING); + } + let response = omachat_ctl::request_with_confirmation(&mut client, command) + .await + .map_err(CliError::Client)?; match response.outcome { ResponseOutcome::Ok { result } => { if output_mode == OutputMode::Json { diff --git a/crates/omachat-ctl/tests/confirmation_flow.rs b/crates/omachat-ctl/tests/confirmation_flow.rs new file mode 100644 index 0000000..4241c03 --- /dev/null +++ b/crates/omachat-ctl/tests/confirmation_flow.rs @@ -0,0 +1,120 @@ +//! Two-phase destructive-command orchestration against a scripted stub daemon. + +use omachat_ctl::{Client, ClientError, DEFAULT_TIMEOUT, request_with_confirmation}; +use omachat_proto::ipc::{ + Command, RequestDecoder, Response, ResponseOutcome, VERSION, encode_line, negotiate, +}; +use serde_json::json; +use std::path::Path; +use tempfile::tempdir; +use tokio::{ + io::{AsyncReadExt, AsyncWriteExt}, + net::UnixListener, +}; + +/// Serves one client: hello, then request-panic-confirmation (mints a token +/// file), then panic (accepted only with the minted token). +async fn stub_daemon(listener: UnixListener, token_directory: std::path::PathBuf) { + let (mut stream, _) = listener.accept().await.expect("accept"); + let mut decoder = RequestDecoder::default(); + let mut buffer = [0_u8; 4096]; + let mut minted: Option = None; + loop { + let count = match stream.read(&mut buffer).await { + Ok(0) | Err(_) => return, + Ok(count) => count, + }; + for request in decoder.push(&buffer[..count]).expect("decode request") { + let outcome = match &request.command { + Command::Hello { + minimum_version, + maximum_version, + } => ResponseOutcome::Ok { + result: serde_json::to_value( + negotiate(*minimum_version, *maximum_version).expect("negotiate"), + ) + .expect("hello result"), + }, + Command::RequestPanicConfirmation => { + let token = "a".repeat(64); + let token_path = token_directory.join("panic.token"); + std::fs::write(&token_path, &token).expect("write token file"); + minted = Some(token); + ResponseOutcome::Ok { + result: json!({ + "token_path": token_path.display().to_string(), + "expires_at": 1_u64, + "ttl_seconds": 120_u64, + }), + } + } + Command::Panic { confirmation } => { + assert_eq!( + Some(confirmation.as_str()), + minted.as_deref(), + "client must echo the minted token, not the typed intent" + ); + ResponseOutcome::Ok { + result: json!({"panic": "erased"}), + } + } + command => panic!("unexpected command: {command:?}"), + }; + let response = Response { + version: VERSION, + id: request.id, + outcome, + }; + stream + .write_all(&encode_line(&response).expect("encode response")) + .await + .expect("write response"); + } + } +} + +async fn connect(socket: &Path) -> Client { + Client::connect(socket, DEFAULT_TIMEOUT) + .await + .expect("connect client") +} + +#[tokio::test] +async fn panic_orchestrates_token_request_and_commit() { + let temporary = tempdir().expect("temporary directory"); + let socket = temporary.path().join("stub.sock"); + let listener = UnixListener::bind(&socket).expect("bind stub"); + let server = tokio::spawn(stub_daemon(listener, temporary.path().to_owned())); + let mut client = connect(&socket).await; + let response = request_with_confirmation( + &mut client, + Command::Panic { + confirmation: "ERASE".into(), + }, + ) + .await + .expect("two-phase panic"); + assert!(matches!(response.outcome, ResponseOutcome::Ok { .. })); + drop(client); + server.await.expect("stub daemon"); +} + +#[tokio::test] +async fn mistyped_intent_is_refused_before_any_daemon_interaction() { + let temporary = tempdir().expect("temporary directory"); + let socket = temporary.path().join("stub.sock"); + let listener = UnixListener::bind(&socket).expect("bind stub"); + let server = tokio::spawn(stub_daemon(listener, temporary.path().to_owned())); + let mut client = connect(&socket).await; + let error = request_with_confirmation( + &mut client, + Command::Panic { + confirmation: "erase".into(), + }, + ) + .await + .expect_err("typed intent must be exact"); + assert!(matches!(error, ClientError::ConfirmationRefused(_))); + drop(client); + server.await.expect("stub daemon"); +} diff --git a/crates/omachat-proto/src/ipc.rs b/crates/omachat-proto/src/ipc.rs index 39e18b7..ee22341 100644 --- a/crates/omachat-proto/src/ipc.rs +++ b/crates/omachat-proto/src/ipc.rs @@ -4,7 +4,12 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use std::{error::Error, fmt}; -pub const VERSION: u16 = 1; +/// Version 2 changed `Panic` and `ClaimRegistryHandle` incompatibly: their +/// `confirmation` field is now a daemon-minted single-use token obtained +/// through `RequestPanicConfirmation` / `RequestRegistryClaimConfirmation`, +/// not an in-band constant. A version-1 client's destructive flow no longer +/// works, so negotiation must reject it rather than fail at use time. +pub const VERSION: u16 = 2; pub const MAX_LINE_BYTES: usize = 64 * 1024; pub const MAX_CORRELATION_ID_BYTES: usize = 128; @@ -61,6 +66,15 @@ pub enum Command { handle: String, confirmation: String, }, + /// Mint a single-use, TTL-bounded confirmation token for `Panic`. The + /// token itself travels out of band via a 0600 file in the daemon state + /// directory; the response carries only the file path and expiry. + RequestPanicConfirmation, + /// Mint a single-use, TTL-bounded confirmation token for + /// `ClaimRegistryHandle` on exactly this handle. + RequestRegistryClaimConfirmation { + handle: String, + }, Who { geohash: String, }, @@ -207,6 +221,15 @@ enum StrictRequestWire { id: String, params: RegistryClaimParams, }, + RequestPanicConfirmation { + version: u16, + id: String, + }, + RequestRegistryClaimConfirmation { + version: u16, + id: String, + params: HandleParams, + }, Who { version: u16, id: String, @@ -408,6 +431,18 @@ impl From for Request { confirmation, }, ), + StrictRequestWire::RequestPanicConfirmation { version, id } => { + (version, id, Command::RequestPanicConfirmation) + } + StrictRequestWire::RequestRegistryClaimConfirmation { + version, + id, + params: HandleParams { handle }, + } => ( + version, + id, + Command::RequestRegistryClaimConfirmation { handle }, + ), StrictRequestWire::Who { version, id, diff --git a/crates/omachat-proto/tests/ipc.rs b/crates/omachat-proto/tests/ipc.rs index ca89e19..1eaeb54 100644 --- a/crates/omachat-proto/tests/ipc.rs +++ b/crates/omachat-proto/tests/ipc.rs @@ -39,17 +39,25 @@ fn oversized_malformed_and_unknown_fields_fail_boundedly() { ); assert_eq!(decoder.push(b"not-json\n"), Err(IpcError::MalformedJson)); assert_eq!( - decoder.push(b"{\"version\":1,\"id\":\"x\",\"method\":\"status\",\"extra\":true}\n"), + decoder.push(b"{\"version\":2,\"id\":\"x\",\"method\":\"status\",\"extra\":true}\n"), Err(IpcError::MalformedJson) ); } #[test] fn hello_negotiation_is_explicit() { - assert_eq!(negotiate(1, 1).expect("compatible").version, VERSION); + assert_eq!(negotiate(2, 2).expect("compatible").version, VERSION); + assert_eq!(negotiate(1, 2).expect("ranged").version, VERSION); + assert!( + matches!( + negotiate(1, 1), + Err(IpcError::VersionMismatch { supported: 2, .. }) + ), + "version 1 destructive semantics are gone" + ); assert!(matches!( - negotiate(2, 3), - Err(IpcError::VersionMismatch { supported: 1, .. }) + negotiate(3, 4), + Err(IpcError::VersionMismatch { supported: 2, .. }) )); } @@ -61,46 +69,46 @@ fn requests_preserve_the_flat_wire_format() { minimum_version: 1, maximum_version: 2, }, - r#"{"version":1,"id":"request","method":"hello","params":{"minimum_version":1,"maximum_version":2}}"#, + r#"{"version":2,"id":"request","method":"hello","params":{"minimum_version":1,"maximum_version":2}}"#, ), ( Command::Status, - r#"{"version":1,"id":"request","method":"status"}"#, + r#"{"version":2,"id":"request","method":"status"}"#, ), ( Command::Fingerprint, - r#"{"version":1,"id":"request","method":"fingerprint"}"#, + r#"{"version":2,"id":"request","method":"fingerprint"}"#, ), ( Command::Join { geohash: "u4pruy".into(), }, - r#"{"version":1,"id":"request","method":"join","params":{"geohash":"u4pruy"}}"#, + r#"{"version":2,"id":"request","method":"join","params":{"geohash":"u4pruy"}}"#, ), ( Command::Leave { geohash: "u4pruy".into(), }, - r#"{"version":1,"id":"request","method":"leave","params":{"geohash":"u4pruy"}}"#, + r#"{"version":2,"id":"request","method":"leave","params":{"geohash":"u4pruy"}}"#, ), ( Command::Send { conversation: "general".into(), text: "hello".into(), }, - r#"{"version":1,"id":"request","method":"send","params":{"conversation":"general","text":"hello"}}"#, + r#"{"version":2,"id":"request","method":"send","params":{"conversation":"general","text":"hello"}}"#, ), ( Command::Who { geohash: "u4pruy".into(), }, - r#"{"version":1,"id":"request","method":"who","params":{"geohash":"u4pruy"}}"#, + r#"{"version":2,"id":"request","method":"who","params":{"geohash":"u4pruy"}}"#, ), ( Command::Block { public_key: "pubkey".into(), }, - r#"{"version":1,"id":"request","method":"block","params":{"public_key":"pubkey"}}"#, + r#"{"version":2,"id":"request","method":"block","params":{"public_key":"pubkey"}}"#, ), ( Command::JoinRoom { @@ -108,7 +116,7 @@ fn requests_preserve_the_flat_wire_format() { group_id: "omarchy".into(), invite_code: None, }, - r#"{"version":1,"id":"request","method":"join-room","params":{"relay":"wss://rooms.example","group_id":"omarchy"}}"#, + r#"{"version":2,"id":"request","method":"join-room","params":{"relay":"wss://rooms.example","group_id":"omarchy"}}"#, ), ( Command::JoinRoom { @@ -116,37 +124,37 @@ fn requests_preserve_the_flat_wire_format() { group_id: "omarchy".into(), invite_code: Some("welcome".into()), }, - r#"{"version":1,"id":"request","method":"join-room","params":{"relay":"wss://rooms.example","group_id":"omarchy","invite_code":"welcome"}}"#, + r#"{"version":2,"id":"request","method":"join-room","params":{"relay":"wss://rooms.example","group_id":"omarchy","invite_code":"welcome"}}"#, ), ( Command::LeaveRoom { relay: "wss://rooms.example".into(), group_id: "omarchy".into(), }, - r#"{"version":1,"id":"request","method":"leave-room","params":{"relay":"wss://rooms.example","group_id":"omarchy"}}"#, + r#"{"version":2,"id":"request","method":"leave-room","params":{"relay":"wss://rooms.example","group_id":"omarchy"}}"#, ), ( Command::ListRooms, - r#"{"version":1,"id":"request","method":"list-rooms"}"#, + r#"{"version":2,"id":"request","method":"list-rooms"}"#, ), ( Command::RoomMembers { relay: "wss://rooms.example".into(), group_id: "omarchy".into(), }, - r#"{"version":1,"id":"request","method":"room-members","params":{"relay":"wss://rooms.example","group_id":"omarchy"}}"#, + r#"{"version":2,"id":"request","method":"room-members","params":{"relay":"wss://rooms.example","group_id":"omarchy"}}"#, ), ( Command::Panic { confirmation: "confirm".into(), }, - r#"{"version":1,"id":"request","method":"panic","params":{"confirmation":"confirm"}}"#, + r#"{"version":2,"id":"request","method":"panic","params":{"confirmation":"confirm"}}"#, ), ( Command::Subscribe { topics: vec![Topic::Status, Topic::Messages], }, - r#"{"version":1,"id":"request","method":"subscribe","params":{"topics":["status","messages"]}}"#, + r#"{"version":2,"id":"request","method":"subscribe","params":{"topics":["status","messages"]}}"#, ), ]; @@ -168,7 +176,7 @@ fn requests_preserve_the_flat_wire_format() { #[test] fn request_fields_may_arrive_in_any_order() { let request = serde_json::from_str::( - r#"{"params":{"text":"hello","conversation":"general"},"method":"send","id":"request","version":1}"#, + r#"{"params":{"text":"hello","conversation":"general"},"method":"send","id":"request","version":2}"#, ) .expect("reordered request deserializes"); assert_eq!( @@ -187,15 +195,15 @@ fn request_fields_may_arrive_in_any_order() { #[test] fn requests_reject_noncanonical_arms_and_fields() { let invalid = [ - r#"{"version":1,"id":"x","method":"send"}"#, - r#"{"version":1,"id":"x","method":"send","params":null}"#, - r#"{"version":1,"id":"x","method":"status","params":null}"#, - r#"{"version":1,"id":"x","method":"status","params":{"conversation":"general","text":"hello"}}"#, - r#"{"version":1,"id":"x","method":"send","params":{"conversation":"general","text":"hello","extra":true}}"#, - r#"{"version":1,"id":"x","method":"send","params":{"conversation":"general","conversation":"other","text":"hello"}}"#, - r#"{"version":1,"id":"x","method":"status","extra":true}"#, - r#"{"version":1,"id":"x","method":"status","method":"status"}"#, - r#"{"version":1,"id":"x","method":"unknown"}"#, + r#"{"version":2,"id":"x","method":"send"}"#, + r#"{"version":2,"id":"x","method":"send","params":null}"#, + r#"{"version":2,"id":"x","method":"status","params":null}"#, + r#"{"version":2,"id":"x","method":"status","params":{"conversation":"general","text":"hello"}}"#, + r#"{"version":2,"id":"x","method":"send","params":{"conversation":"general","text":"hello","extra":true}}"#, + r#"{"version":2,"id":"x","method":"send","params":{"conversation":"general","conversation":"other","text":"hello"}}"#, + r#"{"version":2,"id":"x","method":"status","extra":true}"#, + r#"{"version":2,"id":"x","method":"status","method":"status"}"#, + r#"{"version":2,"id":"x","method":"unknown"}"#, ]; for wire in invalid { @@ -218,7 +226,7 @@ fn responses_preserve_the_flat_wire_format_and_null_result() { let encoded = serde_json::to_string(&response).expect("response serializes"); assert_eq!( encoded, - r#"{"version":1,"id":"response-1","status":"ok","result":null}"# + r#"{"version":2,"id":"response-1","status":"ok","result":null}"# ); assert_eq!( serde_json::from_str::(&encoded).expect("null result deserializes"), @@ -237,11 +245,11 @@ fn responses_preserve_the_flat_wire_format_and_null_result() { }; assert_eq!( serde_json::to_string(&error).expect("error serializes"), - r#"{"version":1,"id":"response-2","status":"error","error":{"code":"unavailable","message":"offline"}}"# + r#"{"version":2,"id":"response-2","status":"error","error":{"code":"unavailable","message":"offline"}}"# ); assert_eq!( serde_json::from_str::( - r#"{"error":{"message":"offline","code":"unavailable"},"status":"error","id":"response-2","version":1}"# + r#"{"error":{"message":"offline","code":"unavailable"},"status":"error","id":"response-2","version":2}"# ) .expect("reordered response deserializes"), error @@ -251,18 +259,18 @@ fn responses_preserve_the_flat_wire_format_and_null_result() { #[test] fn responses_reject_missing_wrong_unknown_and_duplicate_arms() { let invalid = [ - r#"{"version":1,"id":"x","status":"ok"}"#, - r#"{"version":1,"id":"x","status":"ok","error":{"code":"internal","message":"failed"}}"#, - r#"{"version":1,"id":"x","status":"ok","result":null,"error":{"code":"internal","message":"failed"}}"#, - r#"{"version":1,"id":"x","status":"error","result":null}"#, - r#"{"version":1,"id":"x","status":"error","error":{"code":"internal","message":"failed"},"result":null}"#, - r#"{"version":1,"id":"x","status":"error"}"#, - r#"{"version":1,"id":"x","status":"error","error":null}"#, - r#"{"version":1,"id":"x","status":"error","error":{"code":"internal","message":"failed","extra":true}}"#, - r#"{"version":1,"id":"x","status":"ok","result":null,"extra":true}"#, - r#"{"version":1,"id":"x","status":"ok","result":null,"result":null}"#, - r#"{"version":1,"id":"x","status":"ok","status":"ok","result":null}"#, - r#"{"version":1,"id":"x","status":"unknown","result":null}"#, + r#"{"version":2,"id":"x","status":"ok"}"#, + r#"{"version":2,"id":"x","status":"ok","error":{"code":"internal","message":"failed"}}"#, + r#"{"version":2,"id":"x","status":"ok","result":null,"error":{"code":"internal","message":"failed"}}"#, + r#"{"version":2,"id":"x","status":"error","result":null}"#, + r#"{"version":2,"id":"x","status":"error","error":{"code":"internal","message":"failed"},"result":null}"#, + r#"{"version":2,"id":"x","status":"error"}"#, + r#"{"version":2,"id":"x","status":"error","error":null}"#, + r#"{"version":2,"id":"x","status":"error","error":{"code":"internal","message":"failed","extra":true}}"#, + r#"{"version":2,"id":"x","status":"ok","result":null,"extra":true}"#, + r#"{"version":2,"id":"x","status":"ok","result":null,"result":null}"#, + r#"{"version":2,"id":"x","status":"ok","status":"ok","result":null}"#, + r#"{"version":2,"id":"x","status":"unknown","result":null}"#, ]; for wire in invalid { @@ -272,3 +280,37 @@ fn responses_reject_missing_wrong_unknown_and_duplicate_arms() { ); } } + +#[test] +fn confirmation_request_commands_round_trip() { + let mut decoder = RequestDecoder::default(); + let requests = decoder + .push(b"{\"version\":2,\"id\":\"1\",\"method\":\"request-panic-confirmation\"}\n") + .expect("decode panic confirmation request"); + assert_eq!(requests.len(), 1); + assert_eq!(requests[0].command, Command::RequestPanicConfirmation); + + let requests = decoder + .push( + b"{\"version\":2,\"id\":\"2\",\"method\":\"request-registry-claim-confirmation\",\"params\":{\"handle\":\"tom\"}}\n", + ) + .expect("decode claim confirmation request"); + assert_eq!( + requests[0].command, + Command::RequestRegistryClaimConfirmation { + handle: "tom".into() + } + ); + + let encoded = encode_line(&Request { + version: VERSION, + id: "3".into(), + command: Command::RequestRegistryClaimConfirmation { + handle: "tom".into(), + }, + }) + .expect("encode claim confirmation request"); + let text = std::str::from_utf8(&encoded).expect("utf8 line"); + assert!(text.contains("\"method\":\"request-registry-claim-confirmation\"")); + assert!(text.contains("\"handle\":\"tom\"")); +} diff --git a/crates/omachat-proto/tests/nip65_ipc_wire.rs b/crates/omachat-proto/tests/nip65_ipc_wire.rs index 2597252..4801047 100644 --- a/crates/omachat-proto/tests/nip65_ipc_wire.rs +++ b/crates/omachat-proto/tests/nip65_ipc_wire.rs @@ -11,7 +11,7 @@ fn decode(line: &str) -> Command { #[test] fn nip65_publication_is_a_strict_parameterless_command() { - let command = decode(r#"{"version":1,"id":"publish","method":"publish-nip65-relays"}"#); + let command = decode(r#"{"version":2,"id":"publish","method":"publish-nip65-relays"}"#); assert_eq!(command, Command::PublishNip65Relays); let encoded = encode_line(&omachat_proto::ipc::Request { @@ -22,13 +22,13 @@ fn nip65_publication_is_a_strict_parameterless_command() { .expect("encode request"); assert_eq!( std::str::from_utf8(&encoded).expect("UTF-8 request"), - "{\"version\":1,\"id\":\"publish\",\"method\":\"publish-nip65-relays\"}\n" + "{\"version\":2,\"id\":\"publish\",\"method\":\"publish-nip65-relays\"}\n" ); let mut decoder = RequestDecoder::default(); assert_eq!( decoder.push( - b"{\"version\":1,\"id\":\"publish\",\"method\":\"publish-nip65-relays\",\"params\":{}}\n" + b"{\"version\":2,\"id\":\"publish\",\"method\":\"publish-nip65-relays\",\"params\":{}}\n" ), Err(IpcError::MalformedJson) ); diff --git a/crates/omachat-tui/src/main.rs b/crates/omachat-tui/src/main.rs index d318963..9bff970 100644 --- a/crates/omachat-tui/src/main.rs +++ b/crates/omachat-tui/src/main.rs @@ -52,16 +52,18 @@ async fn main() -> ExitCode { .get(model.selected) .map(|conversation| conversation.id.as_str()); match parse_input(&line, current) { - Ok(Some(command)) => match client.request(command).await { - Ok(response) => match response.outcome { - ResponseOutcome::Ok { result } => { - model.status = result.to_string(); - model.security_notice_pending = false; - } - ResponseOutcome::Error { error } => model.status = error.message, - }, - Err(error) => model.status = format!("disconnected: {error}"), - }, + Ok(Some(command)) => { + match omachat_ctl::request_with_confirmation(&mut client, command).await { + Ok(response) => match response.outcome { + ResponseOutcome::Ok { result } => { + model.status = result.to_string(); + model.security_notice_pending = false; + } + ResponseOutcome::Error { error } => model.status = error.message, + }, + Err(error) => model.status = format!("disconnected: {error}"), + } + } Ok(None) => {} Err(error) => model.status = error, } diff --git a/crates/omachatd/Cargo.toml b/crates/omachatd/Cargo.toml index 10b9016..e08b45c 100644 --- a/crates/omachatd/Cargo.toml +++ b/crates/omachatd/Cargo.toml @@ -17,6 +17,7 @@ omachat-proto = { path = "../omachat-proto", version = "=0.0.1" } omachat-registry = { path = "../omachat-registry", version = "=0.0.1" } omachat-registry-transport = { path = "../omachat-registry-transport", version = "=0.0.1" } omachat-store = { path = "../omachat-store", version = "=0.0.1" } +rustix = { version = "=1.1.4", features = ["fs", "process"] } serde = { version = "=1.0.229", features = ["derive"] } serde_json = "=1.0.151" tokio = { version = "=1.53.1", features = ["io-util", "macros", "net", "rt-multi-thread", "signal", "sync"] } diff --git a/crates/omachatd/src/confirmation.rs b/crates/omachatd/src/confirmation.rs new file mode 100644 index 0000000..413d875 --- /dev/null +++ b/crates/omachatd/src/confirmation.rs @@ -0,0 +1,250 @@ +//! Single-use, TTL-bounded confirmation tokens for destructive commands. +//! +//! A destructive command (`panic`, `claim-registry-handle`) is a two-phase +//! exchange: the client first requests a confirmation, the daemon mints a +//! random token and places it in a 0600 file inside the 0700 +//! `/confirmations/` directory, and the client must echo that +//! token back within the TTL. This replaces the in-band constant `"ERASE"` +//! (a typo guard, not authorization). Same-uid processes can still read the +//! state directory — this is a deliberate-two-step and freshness guarantee +//! layered on the peer-credential gate, not a hard boundary; see SECURITY.md. + +use crate::CoreError; +use std::{ + collections::HashMap, + fs, + io::Write, + os::unix::fs::{OpenOptionsExt, PermissionsExt}, + path::{Path, PathBuf}, + sync::Mutex, +}; + +/// A confirmation token is useless after two minutes: long enough for an +/// interactive `--confirm` round trip, short enough that a token left in the +/// state directory by an abandoned command is not a standing authorization. +pub const CONFIRMATION_TTL_SECONDS: u64 = 120; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum ConfirmationAction { + PanicErase, + RegistryClaim { handle: String }, +} + +impl ConfirmationAction { + /// One outstanding token per action kind; a claim token additionally + /// pins the exact handle through the pending-entry comparison. + fn file_name(&self) -> &'static str { + match self { + Self::PanicErase => "panic.token", + Self::RegistryClaim { .. } => "registry-claim.token", + } + } +} + +#[derive(Debug)] +struct PendingToken { + action: ConfirmationAction, + token: String, + expires_at: u64, +} + +#[derive(Debug)] +pub struct IssuedConfirmation { + pub token_path: PathBuf, + pub expires_at: u64, +} + +#[derive(Debug, Eq, PartialEq)] +pub enum ConfirmationError { + Missing, + Mismatch, + Expired, +} + +#[derive(Debug)] +pub struct DestructiveConfirmations { + directory: PathBuf, + pending: Mutex>, +} + +impl DestructiveConfirmations { + #[must_use] + pub fn new(state_directory: &Path) -> Self { + Self { + directory: state_directory.join("confirmations"), + pending: Mutex::new(HashMap::new()), + } + } + + /// Mint a fresh token for `action`, replacing any outstanding token of + /// the same kind. The token travels out of band: the caller learns only + /// the path, and must be able to read the daemon's state directory to + /// obtain the value itself. + pub fn issue( + &self, + action: ConfirmationAction, + now: u64, + ) -> Result { + let mut bytes = [0_u8; 32]; + getrandom::fill(&mut bytes).map_err(|_| CoreError::Random)?; + let token = hex::encode(bytes); + fs::create_dir_all(&self.directory).map_err(CoreError::Io)?; + fs::set_permissions(&self.directory, fs::Permissions::from_mode(0o700)) + .map_err(CoreError::Io)?; + let token_path = self.directory.join(action.file_name()); + let mut file = fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .custom_flags(libc::O_CLOEXEC | libc::O_NOFOLLOW) + .open(&token_path) + .map_err(CoreError::Io)?; + file.write_all(token.as_bytes()).map_err(CoreError::Io)?; + let expires_at = now.saturating_add(CONFIRMATION_TTL_SECONDS); + self.pending + .lock() + .expect("confirmation mutex poisoned") + .insert( + action.file_name(), + PendingToken { + action, + token, + expires_at, + }, + ); + Ok(IssuedConfirmation { + token_path, + expires_at, + }) + } + + /// Single use, burn on attempt: the pending entry and the token file are + /// consumed by every redemption attempt for the action kind, matched or + /// not, so a wrong guess costs the outstanding token instead of leaving + /// it available for retries. Tokens are 256-bit random values, so a + /// non-constant-time comparison leaks nothing recoverable within one + /// attempt. + pub fn redeem( + &self, + action: &ConfirmationAction, + presented: &str, + now: u64, + ) -> Result<(), ConfirmationError> { + let removed = self + .pending + .lock() + .expect("confirmation mutex poisoned") + .remove(action.file_name()); + let _ = fs::remove_file(self.directory.join(action.file_name())); + let Some(pending) = removed else { + return Err(ConfirmationError::Missing); + }; + if pending.expires_at < now { + return Err(ConfirmationError::Expired); + } + if &pending.action != action || pending.token != presented { + return Err(ConfirmationError::Mismatch); + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::{ + CONFIRMATION_TTL_SECONDS, ConfirmationAction, ConfirmationError, DestructiveConfirmations, + }; + use std::os::unix::fs::PermissionsExt; + use tempfile::tempdir; + + #[test] + fn issue_writes_a_private_single_use_token() { + let state = tempdir().expect("state directory"); + let confirmations = DestructiveConfirmations::new(state.path()); + let issued = confirmations + .issue(ConfirmationAction::PanicErase, 1_000) + .expect("issue token"); + assert_eq!(issued.expires_at, 1_000 + CONFIRMATION_TTL_SECONDS); + let mode = std::fs::metadata(&issued.token_path) + .expect("token metadata") + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o600); + let token = std::fs::read_to_string(&issued.token_path).expect("token file"); + assert_eq!(token.len(), 64, "32 random bytes hex encoded"); + + confirmations + .redeem(&ConfirmationAction::PanicErase, &token, 1_010) + .expect("first redemption succeeds"); + assert!(!issued.token_path.exists(), "redemption consumes the file"); + assert_eq!( + confirmations.redeem(&ConfirmationAction::PanicErase, &token, 1_010), + Err(ConfirmationError::Missing), + "tokens are single use" + ); + } + + #[test] + fn wrong_token_burns_the_pending_confirmation() { + let state = tempdir().expect("state directory"); + let confirmations = DestructiveConfirmations::new(state.path()); + let issued = confirmations + .issue(ConfirmationAction::PanicErase, 0) + .expect("issue token"); + assert_eq!( + confirmations.redeem(&ConfirmationAction::PanicErase, "ERASE", 1), + Err(ConfirmationError::Mismatch), + "the legacy constant is no longer a confirmation" + ); + let token = std::fs::read_to_string(&issued.token_path); + assert!( + token.is_err(), + "a failed guess consumes the outstanding token" + ); + } + + #[test] + fn expired_tokens_are_rejected() { + let state = tempdir().expect("state directory"); + let confirmations = DestructiveConfirmations::new(state.path()); + let issued = confirmations + .issue(ConfirmationAction::PanicErase, 100) + .expect("issue token"); + let token = std::fs::read_to_string(&issued.token_path).expect("token file"); + assert_eq!( + confirmations.redeem( + &ConfirmationAction::PanicErase, + &token, + issued.expires_at + 1 + ), + Err(ConfirmationError::Expired) + ); + } + + #[test] + fn claim_tokens_are_bound_to_their_handle() { + let state = tempdir().expect("state directory"); + let confirmations = DestructiveConfirmations::new(state.path()); + let issued = confirmations + .issue( + ConfirmationAction::RegistryClaim { + handle: "tom".into(), + }, + 0, + ) + .expect("issue token"); + let token = std::fs::read_to_string(&issued.token_path).expect("token file"); + assert_eq!( + confirmations.redeem( + &ConfirmationAction::RegistryClaim { + handle: "alice".into() + }, + &token, + 1, + ), + Err(ConfirmationError::Mismatch), + "a token minted for one handle must not confirm another" + ); + } +} diff --git a/crates/omachatd/src/core.rs b/crates/omachatd/src/core.rs index 16beb36..12a2425 100644 --- a/crates/omachatd/src/core.rs +++ b/crates/omachatd/src/core.rs @@ -220,6 +220,7 @@ struct CoreInner { config: Mutex, events: EventHub, sequence: AtomicU64, + confirmations: crate::confirmation::DestructiveConfirmations, } #[derive(Clone)] @@ -288,6 +289,7 @@ impl DaemonCore { events: EventHub, ) -> Result { config.validate()?; + let confirmation_root = state_directory.as_ref().to_owned(); let store = Arc::new( SealedStore::open(&state_directory, config.storage_provider.into()) .await @@ -387,6 +389,9 @@ impl DaemonCore { config: Mutex::new(config), events, sequence: AtomicU64::new(1), + confirmations: crate::confirmation::DestructiveConfirmations::new( + &confirmation_root, + ), }), }) } @@ -1694,14 +1699,46 @@ impl DaemonCore { confirmation, } => { let handle = GlobalHandle::parse(&handle).map_err(|_| CoreError::InvalidHandle)?; - if confirmation != handle.as_str() { - return Err(CoreError::RegistryClaimConfirmationRequired); - } + self.inner + .confirmations + .redeem( + &crate::confirmation::ConfirmationAction::RegistryClaim { + handle: handle.as_str().to_owned(), + }, + &confirmation, + unix_time()?, + ) + .map_err(|error| match error { + crate::confirmation::ConfirmationError::Expired => { + CoreError::ConfirmationExpired + } + crate::confirmation::ConfirmationError::Missing + | crate::confirmation::ConfirmationError::Mismatch => { + CoreError::RegistryClaimConfirmationRequired + } + })?; let result = self .claim_configured_registry_handle_active(&handle, unix_time()?) .await?; Ok(registry_claim_value(&handle, &result)) } + Command::RequestPanicConfirmation => { + let issued = self.inner.confirmations.issue( + crate::confirmation::ConfirmationAction::PanicErase, + unix_time()?, + )?; + Ok(confirmation_issue_value(&issued)) + } + Command::RequestRegistryClaimConfirmation { handle } => { + let handle = GlobalHandle::parse(&handle).map_err(|_| CoreError::InvalidHandle)?; + let issued = self.inner.confirmations.issue( + crate::confirmation::ConfirmationAction::RegistryClaim { + handle: handle.as_str().to_owned(), + }, + unix_time()?, + )?; + Ok(confirmation_issue_value(&issued)) + } Command::Who { geohash } => self.who(&geohash), Command::Block { public_key } => self.block(&public_key), Command::JoinRoom { @@ -2501,9 +2538,20 @@ impl DaemonCore { } async fn panic_erase(&self, confirmation: &str) -> Result { - if confirmation != "ERASE" { - return Err(CoreError::ConfirmationRequired); - } + self.inner + .confirmations + .redeem( + &crate::confirmation::ConfirmationAction::PanicErase, + confirmation, + unix_time()?, + ) + .map_err(|error| match error { + crate::confirmation::ConfirmationError::Expired => CoreError::ConfirmationExpired, + crate::confirmation::ConfirmationError::Missing + | crate::confirmation::ConfirmationError::Mismatch => { + CoreError::ConfirmationRequired + } + })?; if !self.inner.panic.begin() { return Err(CoreError::Panicked); } @@ -2921,7 +2969,7 @@ mod relay_list_publication_lifecycle_tests { .await .expect("open configured core"); - core.panic_erase("ERASE") + core.panic_erase(&super::minted_panic_token(&core)) .await .expect("panic erasure completes"); @@ -2970,6 +3018,32 @@ fn panic_unavailable() -> ResponseOutcome { } } +fn confirmation_issue_value(issued: &crate::confirmation::IssuedConfirmation) -> serde_json::Value { + serde_json::json!({ + "token_path": issued.token_path.display().to_string(), + "expires_at": issued.expires_at, + "ttl_seconds": crate::confirmation::CONFIRMATION_TTL_SECONDS, + }) +} + +/// Mint a real panic-confirmation token for tests: destructive commands are +/// no longer authorized by a constant string. +#[cfg(test)] +fn minted_panic_token(core: &DaemonCore) -> String { + let issued = core + .inner + .confirmations + .issue( + crate::confirmation::ConfirmationAction::PanicErase, + unix_time().expect("clock"), + ) + .expect("issue panic token"); + std::fs::read_to_string(issued.token_path) + .expect("token file") + .trim() + .to_owned() +} + fn unix_time() -> Result { SystemTime::now() .duration_since(UNIX_EPOCH) @@ -3185,7 +3259,8 @@ mod tests { let waiting_core = core.clone(); let waiter = tokio::spawn(async move { waiting_core.wait_for_panic_terminal().await }); let panic_core = core.clone(); - let panic = tokio::spawn(async move { panic_core.panic_erase("ERASE").await }); + let panic_token = super::minted_panic_token(&core); + let panic = tokio::spawn(async move { panic_core.panic_erase(&panic_token).await }); tokio::time::timeout(Duration::from_secs(1), async { while core.panic_state() != PanicState::Erasing { @@ -3242,7 +3317,8 @@ mod tests { core.prepare_for_shutdown().await; assert_eq!(core.panic_state(), PanicState::Stopping); - assert!(core.panic_erase("ERASE").await.is_err()); + let token = super::minted_panic_token(&core); + assert!(core.panic_erase(&token).await.is_err()); assert!(temporary.path().exists(), "late panic did not erase state"); } diff --git a/crates/omachatd/src/core_error.rs b/crates/omachatd/src/core_error.rs index 8e2e7a0..9829bff 100644 --- a/crates/omachatd/src/core_error.rs +++ b/crates/omachatd/src/core_error.rs @@ -58,6 +58,7 @@ pub enum CoreError { Random, Subscription, ConfirmationRequired, + ConfirmationExpired, PanicErase, Panicked, RestartRequired, @@ -76,6 +77,7 @@ impl CoreError { | Self::InvalidPublicKey | Self::InvalidMessage => ErrorCode::InvalidRequest, Self::ConfirmationRequired + | Self::ConfirmationExpired | Self::RegistryClaimConfirmationRequired | Self::RegistryHandleConflict | Self::RegistryBindingChanged => ErrorCode::Conflict, @@ -210,9 +212,9 @@ impl fmt::Display for CoreError { } Self::RegistryClaimPreflightUnusable => formatter .write_str("registry preflight did not return usable current account state"), - Self::RegistryClaimConfirmationRequired => { - formatter.write_str("registry handle claim requires exact handle confirmation") - } + Self::RegistryClaimConfirmationRequired => formatter.write_str( + "registry handle claim requires a fresh confirmation token; request one with request-registry-claim-confirmation", + ), Self::RegistryHandleConflict => formatter .write_str("requested handle conflicts with local or authoritative account state"), Self::RegistryBindingChanged => { @@ -236,9 +238,12 @@ impl fmt::Display for CoreError { Self::Clock => formatter.write_str("system clock is before the Unix epoch"), Self::Random => formatter.write_str("secure random generation failed"), Self::Subscription => formatter.write_str("Nostr subscription refresh failed"), - Self::ConfirmationRequired => { - formatter.write_str("panic erase requires exact confirmation ERASE") - } + Self::ConfirmationRequired => formatter.write_str( + "panic erase requires a fresh confirmation token; request one with request-panic-confirmation", + ), + Self::ConfirmationExpired => formatter.write_str( + "confirmation token expired or was consumed; request a new one", + ), Self::PanicErase => { formatter.write_str("panic erase cannot run in this runtime context") } diff --git a/crates/omachatd/src/ipc_server.rs b/crates/omachatd/src/ipc_server.rs index 4fde46e..1a57aa3 100644 --- a/crates/omachatd/src/ipc_server.rs +++ b/crates/omachatd/src/ipc_server.rs @@ -8,7 +8,7 @@ use std::{ fmt, fs, fs::{File, OpenOptions}, future::Future, - os::unix::fs::{FileTypeExt, OpenOptionsExt, PermissionsExt}, + os::unix::fs::{DirBuilderExt, FileTypeExt, OpenOptionsExt, PermissionsExt}, path::{Path, PathBuf}, pin::Pin, sync::{Arc, Mutex}, @@ -79,6 +79,53 @@ pub struct IpcServer { events: EventHub, } +/// Publish the listening socket at its final path only once it is already +/// private. `UnixListener::bind` honours the process umask, so binding +/// directly and chmodding afterwards leaves a window in which another uid +/// can connect (finding #7) whenever the umask is permissive — a manual +/// launch without the packaged unit's `UMask=0077`, for instance. +/// +/// Overriding the umask around `bind` would close that window but is not +/// safe here: umask is process-wide, so it would also strip bits from files +/// and directories created concurrently by other threads. Instead the +/// socket is bound inside a freshly created 0700 staging directory, where +/// no other uid can reach it, tightened to 0600 there, and then renamed +/// into place. `rename` is atomic and preserves the inode and its mode, so +/// the final path never exists in a world-accessible state and clients +/// connect to the same listening socket through it. +fn bind_private_socket(socket_path: &Path) -> Result { + let parent = socket_path.parent().unwrap_or_else(|| Path::new(".")); + let file_name = socket_path + .file_name() + .ok_or(ServerError::OccupiedPath)? + .to_string_lossy() + .into_owned(); + let staging_directory = parent.join(format!(".{file_name}.staging")); + // A stale staging directory can only be ours: the instance lock is held + // by this process before bind runs. Recreation fails closed if another + // process wins the race for the name. + let _ = fs::remove_dir_all(&staging_directory); + fs::DirBuilder::new() + .mode(0o700) + .create(&staging_directory) + .map_err(ServerError::Io)?; + // The requested 0700 is masked, never widened, by the process umask; + // this restores the owner bits an exotic umask could have removed while + // the directory was still empty. + fs::set_permissions(&staging_directory, fs::Permissions::from_mode(0o700)) + .map_err(ServerError::Io)?; + let staged_socket = staging_directory.join("socket"); + let bound = UnixListener::bind(&staged_socket); + let published = bound.and_then(|listener| { + fs::set_permissions(&staged_socket, fs::Permissions::from_mode(0o600)) + .and_then(|()| fs::rename(&staged_socket, socket_path)) + .map(|()| listener) + }); + // The staging directory is transient on every path, including failure. + let _ = fs::remove_dir_all(&staging_directory); + published.map_err(ServerError::Io) +} + impl IpcServer { pub fn bind( socket_path: impl AsRef, @@ -108,9 +155,7 @@ impl IpcServer { } fs::remove_file(&socket_path).map_err(ServerError::Io)?; } - let listener = UnixListener::bind(&socket_path).map_err(ServerError::Io)?; - fs::set_permissions(&socket_path, fs::Permissions::from_mode(0o600)) - .map_err(ServerError::Io)?; + let listener = bind_private_socket(&socket_path)?; Ok(Self { listener, socket_path, @@ -125,6 +170,7 @@ impl IpcServer { let mut clients = JoinSet::new(); let mut terminal_error = None; let (client_shutdown_sender, client_shutdown) = watch::channel(false); + let daemon_euid = rustix::process::geteuid().as_raw(); loop { if *shutdown.borrow() { break; @@ -144,6 +190,12 @@ impl IpcServer { break; } }; + match stream.peer_cred() { + Ok(credentials) if peer_permitted(credentials.uid(), daemon_euid) => {} + // Fail closed: a foreign or unreadable peer gets no + // protocol bytes at all, not even an error frame. + Ok(_) | Err(_) => continue, + } let handler = Arc::clone(&self.handler); let events = self.events.clone(); let client_shutdown = client_shutdown.clone(); @@ -280,6 +332,15 @@ async fn serve_client( } } +/// SO_PEERCRED authorization: only the daemon's own effective uid may speak +/// the protocol. The uid in `UCred` is fixed by the kernel at connect() +/// time and cannot be spoofed by the client. The pid is deliberately not +/// consulted (pid reuse races). This is defense in depth, not a hard +/// boundary: a same-uid process can ptrace the daemon — see SECURITY.md. +fn peer_permitted(peer_uid: u32, daemon_euid: u32) -> bool { + peer_uid == daemon_euid +} + fn protocol_error(code: ErrorCode, error: &impl fmt::Display) -> ResponseOutcome { ResponseOutcome::Error { error: ErrorBody { @@ -317,3 +378,42 @@ impl Error for ServerError { } } } + +#[cfg(test)] +mod tests { + use super::{bind_private_socket, peer_permitted}; + use std::os::unix::fs::PermissionsExt; + use tempfile::tempdir; + + #[tokio::test] + async fn the_published_socket_is_private_and_leaves_no_staging_directory() { + let temporary = tempdir().expect("temporary directory"); + let socket = temporary.path().join("omachat.sock"); + let listener = bind_private_socket(&socket).expect("bind private socket"); + assert_eq!( + std::fs::metadata(&socket) + .expect("socket metadata") + .permissions() + .mode() + & 0o777, + 0o600, + "the socket is 0600 the moment it appears at its final path" + ); + assert!( + !temporary.path().join(".omachat.sock.staging").exists(), + "the staging directory is removed after publication" + ); + drop(listener); + } + + #[test] + fn only_the_daemon_uid_is_permitted() { + assert!(peer_permitted(1000, 1000)); + assert!(!peer_permitted(1001, 1000)); + // Root is not exempted: a root peer can bypass any socket check + // through other means, so accepting it here would only widen the + // daemon's accepted-input surface without adding capability. + assert!(!peer_permitted(0, 1000)); + assert!(peer_permitted(0, 0)); + } +} diff --git a/crates/omachatd/src/lib.rs b/crates/omachatd/src/lib.rs index 206cad8..3c55612 100644 --- a/crates/omachatd/src/lib.rs +++ b/crates/omachatd/src/lib.rs @@ -2,6 +2,7 @@ mod agent_lifecycle_store; mod config; +mod confirmation; mod core; mod core_error; mod dm_delivery_service; @@ -33,6 +34,10 @@ pub use config::{ RelayListPublicationConfig, RelayListPublicationRelayConfig, RoomsConfig, StorageProviderConfig, }; +pub use confirmation::{ + CONFIRMATION_TTL_SECONDS, ConfirmationAction, ConfirmationError, DestructiveConfirmations, + IssuedConfirmation, +}; pub use core::{ DaemonCore, PanicState, RegistryClaimEvidence, RegistryClaimResult, RegistryClaimStatus, }; diff --git a/crates/omachatd/tests/confirmation_ipc.rs b/crates/omachatd/tests/confirmation_ipc.rs new file mode 100644 index 0000000..25d5a4e --- /dev/null +++ b/crates/omachatd/tests/confirmation_ipc.rs @@ -0,0 +1,146 @@ +use omachat_proto::ipc::{Command, Request, ResponseOutcome, VERSION}; +use omachatd::{DaemonConfig, DaemonCore, EventHub, RequestHandler, StorageProviderConfig}; +use std::os::unix::fs::PermissionsExt; +use tempfile::tempdir; + +async fn open_core(state: &std::path::Path) -> DaemonCore { + DaemonCore::open( + state, + DaemonConfig { + storage_provider: StorageProviderConfig::File, + ..DaemonConfig::default() + }, + EventHub::default(), + ) + .await + .expect("open core") +} + +async fn ok_result(core: &DaemonCore, id: &str, command: Command) -> serde_json::Value { + let outcome = core + .handle(Request { + version: VERSION, + id: id.into(), + command, + }) + .await; + let ResponseOutcome::Ok { result } = outcome else { + panic!("expected ok outcome, got {outcome:?}"); + }; + result +} + +#[tokio::test] +async fn panic_confirmation_request_mints_a_private_token_file() { + let temporary = tempdir().expect("temporary directory"); + let core = open_core(temporary.path()).await; + let result = ok_result(&core, "token", Command::RequestPanicConfirmation).await; + let token_path = result + .get("token_path") + .and_then(serde_json::Value::as_str) + .expect("token_path in result"); + assert!(token_path.starts_with(temporary.path().to_str().expect("utf8 path"))); + let mode = std::fs::metadata(token_path) + .expect("token metadata") + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o600); + assert_eq!( + std::fs::read_to_string(token_path) + .expect("token file") + .len(), + 64 + ); + assert!( + result + .get("expires_at") + .and_then(serde_json::Value::as_u64) + .is_some() + ); + assert_eq!( + result + .get("ttl_seconds") + .and_then(serde_json::Value::as_u64), + Some(120) + ); +} + +#[tokio::test] +async fn claim_confirmation_request_validates_the_handle() { + let temporary = tempdir().expect("temporary directory"); + let core = open_core(temporary.path()).await; + let outcome = core + .handle(Request { + version: VERSION, + id: "bad".into(), + command: Command::RequestRegistryClaimConfirmation { + handle: "NOT A HANDLE".into(), + }, + }) + .await; + assert!( + matches!(outcome, ResponseOutcome::Error { .. }), + "invalid handles must not mint tokens" + ); + let result = ok_result( + &core, + "good", + Command::RequestRegistryClaimConfirmation { + handle: "tom".into(), + }, + ) + .await; + assert!( + result + .get("token_path") + .and_then(serde_json::Value::as_str) + .is_some() + ); +} + +#[tokio::test] +async fn the_legacy_erase_constant_no_longer_authorizes_panic() { + let temporary = tempdir().expect("temporary directory"); + let core = open_core(temporary.path()).await; + let outcome = core + .handle(Request { + version: VERSION, + id: "legacy".into(), + command: Command::Panic { + confirmation: "ERASE".into(), + }, + }) + .await; + assert!(matches!(outcome, ResponseOutcome::Error { .. })); + assert!( + !core.is_panicked(), + "a rejected confirmation must not erase" + ); +} + +#[tokio::test] +async fn a_minted_token_authorizes_panic_exactly_once() { + let temporary = tempdir().expect("temporary directory"); + let core = open_core(temporary.path()).await; + let result = ok_result(&core, "token", Command::RequestPanicConfirmation).await; + let token_path = result + .get("token_path") + .and_then(serde_json::Value::as_str) + .expect("token_path") + .to_owned(); + let token = std::fs::read_to_string(&token_path) + .expect("token file") + .trim() + .to_owned(); + let outcome = core + .handle(Request { + version: VERSION, + id: "commit".into(), + command: Command::Panic { + confirmation: token, + }, + }) + .await; + assert!(matches!(outcome, ResponseOutcome::Ok { .. }), "{outcome:?}"); + assert!(core.is_panicked()); +} diff --git a/crates/omachatd/tests/core.rs b/crates/omachatd/tests/core.rs index d06782e..9eff4be 100644 --- a/crates/omachatd/tests/core.rs +++ b/crates/omachatd/tests/core.rs @@ -10,6 +10,30 @@ use tempfile::tempdir; use tokio::net::{TcpListener, TcpStream}; use tokio_tungstenite::{WebSocketStream, accept_async, tungstenite::Message}; +/// Destructive commands require a daemon-minted single-use token; mint one +/// over the same IPC surface the client uses. +async fn minted_panic_token(core: &DaemonCore) -> String { + let outcome = core + .handle(Request { + version: VERSION, + id: "panic-token".into(), + command: Command::RequestPanicConfirmation, + }) + .await; + let ResponseOutcome::Ok { result } = outcome else { + panic!("panic token issuance failed: {outcome:?}"); + }; + let path = result + .get("token_path") + .and_then(serde_json::Value::as_str) + .expect("token_path") + .to_owned(); + std::fs::read_to_string(path) + .expect("token file") + .trim() + .to_owned() +} + async fn command(core: &DaemonCore, command: Command) -> serde_json::Value { match core .handle(Request { @@ -347,10 +371,11 @@ async fn panic_requires_confirmation_erases_state_and_rejects_more_work() { .await; assert!(matches!(denied, ResponseOutcome::Error { .. })); assert_eq!(core.panic_state(), PanicState::Active); + let token = minted_panic_token(&core).await; command( &core, Command::Panic { - confirmation: "ERASE".into(), + confirmation: token, }, ) .await; @@ -387,12 +412,13 @@ async fn panic_cleanup_failure_is_terminal_and_never_reenables_the_daemon() { .expect("open core"); fs::remove_file(temporary.path().join("master.key")).expect("inject key cleanup failure"); + let token = minted_panic_token(&core).await; let failed = core .handle(Request { version: VERSION, id: "panic-failure".into(), command: Command::Panic { - confirmation: "ERASE".into(), + confirmation: token, }, }) .await; @@ -471,13 +497,14 @@ async fn panic_cancels_a_slow_publish_before_erasing_and_emits_no_local_message( .expect("relay event signal"); let panic_core = core.clone(); + let panic_token = minted_panic_token(&core).await; let panic = tokio::spawn(async move { panic_core .handle(Request { version: VERSION, id: "panic-during-send".into(), command: Command::Panic { - confirmation: "ERASE".into(), + confirmation: panic_token, }, }) .await diff --git a/crates/omachatd/tests/dm_inbox_enable.rs b/crates/omachatd/tests/dm_inbox_enable.rs index c50a18f..e412383 100644 --- a/crates/omachatd/tests/dm_inbox_enable.rs +++ b/crates/omachatd/tests/dm_inbox_enable.rs @@ -10,6 +10,31 @@ use omachat_proto::ipc::{Command, Request, ResponseOutcome, Topic, VERSION}; use omachatd::{DaemonConfig, DaemonCore, EventHub, RequestHandler, StorageProviderConfig}; use serde_json::{Value, json}; use tempfile::tempdir; + +/// Destructive commands require a daemon-minted single-use token; mint one +/// over the same IPC surface the client uses. +async fn minted_panic_token(core: &DaemonCore) -> String { + let outcome = core + .handle(Request { + version: VERSION, + id: "panic-token".into(), + command: Command::RequestPanicConfirmation, + }) + .await; + let ResponseOutcome::Ok { result } = outcome else { + panic!("panic token issuance failed: {outcome:?}"); + }; + let path = result + .get("token_path") + .and_then(serde_json::Value::as_str) + .expect("token_path") + .to_owned(); + std::fs::read_to_string(path) + .expect("token file") + .trim() + .to_owned() +} + use tokio::{ net::{TcpListener, TcpStream}, time::timeout, @@ -101,10 +126,11 @@ async fn configured_private_inbox_reaches_ipc_and_quiesces_before_panic_erasure( ); assert_eq!(event.payload["delivery"], "received"); + let token = minted_panic_token(&core).await; let erased = command( &core, Command::Panic { - confirmation: "ERASE".to_owned(), + confirmation: token, }, ) .await; diff --git a/crates/omachatd/tests/ipc_server.rs b/crates/omachatd/tests/ipc_server.rs index a1b38bc..0a39f0a 100644 --- a/crates/omachatd/tests/ipc_server.rs +++ b/crates/omachatd/tests/ipc_server.rs @@ -123,8 +123,8 @@ async fn socket_is_private_and_hello_status_work() { version: VERSION, id: "hello".into(), command: Command::Hello { - minimum_version: 1, - maximum_version: 1, + minimum_version: VERSION, + maximum_version: VERSION, }, }, Request { @@ -368,13 +368,37 @@ async fn panic_response_during_terminal_shutdown( }, ) .await; + // Destructive commands need a daemon-minted single-use token, obtained + // over the same connection before the panic request itself. + let token_response = write_request( + &mut writer, + &mut reader, + Request { + version: VERSION, + id: "token".into(), + command: Command::RequestPanicConfirmation, + }, + ) + .await; + let ResponseOutcome::Ok { result } = token_response.outcome else { + panic!("token issuance failed"); + }; + let token = std::fs::read_to_string( + result + .get("token_path") + .and_then(serde_json::Value::as_str) + .expect("token_path"), + ) + .expect("token file") + .trim() + .to_owned(); writer .write_all( &encode_line(&Request { version: VERSION, id: "panic".into(), command: Command::Panic { - confirmation: "ERASE".into(), + confirmation: token, }, }) .expect("encode panic request"), diff --git a/crates/omachatd/tests/ipc_socket_mode.rs b/crates/omachatd/tests/ipc_socket_mode.rs new file mode 100644 index 0000000..e143650 --- /dev/null +++ b/crates/omachatd/tests/ipc_socket_mode.rs @@ -0,0 +1,112 @@ +//! Finding #7: the socket must never be reachable at its final path in a +//! world-accessible state. A permissive process umask simulates a manual +//! (non-systemd) launch without the packaged unit's UMask=0077, which is +//! exactly the case the old chmod-after-bind sequence left exposed. + +use omachat_proto::ipc::{ + Command, Request, Response, ResponseOutcome, VERSION, encode_line, negotiate, +}; +use omachatd::{EventHub, IpcServer, RequestHandler}; +use rustix::fs::Mode; +use serde_json::{json, to_value}; +use std::{future::Future, os::unix::fs::PermissionsExt, pin::Pin, time::Duration}; +use tempfile::tempdir; +use tokio::{ + io::{AsyncBufReadExt, AsyncWriteExt, BufReader}, + net::UnixStream, + sync::watch, +}; + +struct Handler; + +impl RequestHandler for Handler { + fn handle( + &self, + request: Request, + ) -> Pin + Send + '_>> { + Box::pin(async move { + match request.command { + Command::Hello { + minimum_version, + maximum_version, + } => match negotiate(minimum_version, maximum_version) { + Ok(result) => ResponseOutcome::Ok { + result: to_value(result).expect("hello result"), + }, + Err(error) => ResponseOutcome::Error { + error: omachat_proto::ipc::ErrorBody { + code: omachat_proto::ipc::ErrorCode::VersionMismatch, + message: error.to_string(), + }, + }, + }, + _ => ResponseOutcome::Ok { result: json!({}) }, + } + }) + } +} + +#[tokio::test] +async fn socket_is_published_private_even_under_a_permissive_umask() { + // umask is process-wide; this integration-test binary holds a single + // test, so nothing else in the process depends on it. The daemon must + // not need this to be restrictive. + let previous = rustix::process::umask(Mode::empty()); + let temporary = tempdir().expect("temporary directory"); + let socket = temporary.path().join("omachat.sock"); + let server = IpcServer::bind(&socket, Handler, EventHub::default()).expect("bind IPC server"); + let mode = std::fs::metadata(&socket) + .expect("socket metadata") + .permissions() + .mode(); + assert_eq!( + mode & 0o777, + 0o600, + "socket must already be private when it appears at its final path" + ); + assert!( + !temporary.path().join(".omachat.sock.staging").exists(), + "staging directory must not be left behind" + ); + // The published path must still be the live listening socket: rename + // keeps the inode, so clients connect through it normally. + let (shutdown_sender, shutdown_receiver) = watch::channel(false); + let task = tokio::spawn(server.run(shutdown_receiver)); + let stream = UnixStream::connect(&socket).await.expect("connect"); + let (reader, mut writer) = stream.into_split(); + let mut reader = BufReader::new(reader); + writer + .write_all( + &encode_line(&Request { + version: VERSION, + id: "hello".into(), + command: Command::Hello { + minimum_version: VERSION, + maximum_version: VERSION, + }, + }) + .expect("encode hello"), + ) + .await + .expect("write hello"); + let mut line = String::new(); + tokio::time::timeout(Duration::from_secs(2), reader.read_line(&mut line)) + .await + .expect("hello response timeout") + .expect("read hello response"); + let response: Response = serde_json::from_str(&line).expect("hello response"); + assert!(matches!(response.outcome, ResponseOutcome::Ok { .. })); + + shutdown_sender.send(true).expect("shutdown"); + tokio::time::timeout(Duration::from_secs(2), task) + .await + .expect("server shutdown") + .expect("server task") + .expect("server result"); + let observed = rustix::process::umask(previous); + assert_eq!( + observed, + Mode::empty(), + "binding must not change the process umask" + ); +} diff --git a/crates/omachatd/tests/principal_registry_claim.rs b/crates/omachatd/tests/principal_registry_claim.rs index e364a42..f7ca260 100644 --- a/crates/omachatd/tests/principal_registry_claim.rs +++ b/crates/omachatd/tests/principal_registry_claim.rs @@ -16,6 +16,32 @@ use omachatd::{ use tempfile::tempdir; use tokio::{net::TcpListener, sync::oneshot}; +/// Registry claims require a daemon-minted single-use token bound to the +/// exact handle; mint one over the same IPC surface the client uses. +async fn minted_claim_token(core: &DaemonCore, handle: &str) -> String { + let outcome = core + .handle(Request { + version: VERSION, + id: "claim-token".into(), + command: Command::RequestRegistryClaimConfirmation { + handle: handle.to_owned(), + }, + }) + .await; + let ResponseOutcome::Ok { result } = outcome else { + panic!("claim token issuance failed: {outcome:?}"); + }; + let path = result + .get("token_path") + .and_then(serde_json::Value::as_str) + .expect("token_path") + .to_owned(); + std::fs::read_to_string(path) + .expect("token file") + .trim() + .to_owned() +} + fn now() -> u64 { SystemTime::now() .duration_since(UNIX_EPOCH) @@ -24,13 +50,14 @@ fn now() -> u64 { } async fn claim(core: &DaemonCore) -> serde_json::Value { + let confirmation = minted_claim_token(core, "alice").await; match core .handle(Request { version: VERSION, id: "principal-registry-claim".into(), command: Command::ClaimRegistryHandle { handle: "alice".into(), - confirmation: "alice".into(), + confirmation, }, }) .await diff --git a/crates/omachatd/tests/profile_publication_ipc.rs b/crates/omachatd/tests/profile_publication_ipc.rs index 7588927..594fc98 100644 --- a/crates/omachatd/tests/profile_publication_ipc.rs +++ b/crates/omachatd/tests/profile_publication_ipc.rs @@ -11,6 +11,30 @@ use tokio::net::TcpListener; use tokio::sync::oneshot; use tokio_tungstenite::{accept_async, tungstenite::Message}; +/// Destructive commands require a daemon-minted single-use token; mint one +/// over the same IPC surface the client uses. +async fn minted_panic_token(core: &DaemonCore) -> String { + let outcome = core + .handle(Request { + version: VERSION, + id: "panic-token".into(), + command: Command::RequestPanicConfirmation, + }) + .await; + let ResponseOutcome::Ok { result } = outcome else { + panic!("panic token issuance failed: {outcome:?}"); + }; + let path = result + .get("token_path") + .and_then(serde_json::Value::as_str) + .expect("token_path") + .to_owned(); + std::fs::read_to_string(path) + .expect("token file") + .trim() + .to_owned() +} + #[tokio::test] async fn ipc_publishes_a_device_principal_profile() { let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); @@ -171,13 +195,14 @@ async fn panic_erasure_cancels_profile_publication_before_erasing_keys() { }) }; publication_started.await.unwrap(); + let token = minted_panic_token(&core).await; let panic = tokio::time::timeout( std::time::Duration::from_secs(2), core.handle(Request { version: VERSION, id: "panic-profile".into(), command: Command::Panic { - confirmation: "ERASE".into(), + confirmation: token, }, }), ) diff --git a/crates/omachatd/tests/registry_claim.rs b/crates/omachatd/tests/registry_claim.rs index 5c2fd6c..f84437d 100644 --- a/crates/omachatd/tests/registry_claim.rs +++ b/crates/omachatd/tests/registry_claim.rs @@ -14,6 +14,33 @@ use omachatd::{ RequestHandler, StorageProviderConfig, }; use tempfile::tempdir; + +/// Registry claims require a daemon-minted single-use token bound to the +/// exact handle; mint one over the same IPC surface the client uses. +async fn minted_claim_token(core: &DaemonCore, handle: &str) -> String { + let outcome = core + .handle(Request { + version: VERSION, + id: "claim-token".into(), + command: Command::RequestRegistryClaimConfirmation { + handle: handle.to_owned(), + }, + }) + .await; + let ResponseOutcome::Ok { result } = outcome else { + panic!("claim token issuance failed: {outcome:?}"); + }; + let path = result + .get("token_path") + .and_then(serde_json::Value::as_str) + .expect("token_path") + .to_owned(); + std::fs::read_to_string(path) + .expect("token file") + .trim() + .to_owned() +} + use tokio::{net::TcpListener, sync::oneshot}; fn now() -> u64 { @@ -94,13 +121,14 @@ async fn pending_claim_replays_after_restart_and_clears_after_durable_receipt() ) .await .expect("daemon core"); + let confirmation = minted_claim_token(&core, "alice").await; let result = core .handle(Request { version: VERSION, id: "claim-handle".into(), command: Command::ClaimRegistryHandle { handle: "alice".into(), - confirmation: "alice".into(), + confirmation, }, }) .await; @@ -167,13 +195,16 @@ async fn offline_preflight_never_creates_a_new_claim_intent() { ) .await .expect("daemon core"); + // A token minted for another handle must not confirm this claim, and the + // legacy handle echo is no longer a confirmation at all. + let foreign_token = minted_claim_token(&core, "bob").await; let rejected = core .handle(Request { version: VERSION, id: "wrong-confirmation".into(), command: Command::ClaimRegistryHandle { handle: "alice".into(), - confirmation: "bob".into(), + confirmation: foreign_token, }, }) .await; diff --git a/docs/installation.md b/docs/installation.md index 76330fd..1c0693d 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -6,9 +6,14 @@ inputs for the Omarchy v4.0.1 clean-install gate. ## User service OmaChat's local IPC socket is an account-local control boundary, not an -application sandbox. Socket permissions exclude other Unix users, and the -daemon refuses a concurrent instance, but processes already running as the -same account are inside this trust boundary. Run untrusted desktop software +application sandbox. Socket permissions exclude other Unix users, the daemon +rejects any connection whose peer credentials report a different uid, and it +refuses a concurrent instance, but processes already running as the same +account are inside this trust boundary. Destructive commands (`panic`, +`claim-handle`) require a single-use confirmation token that the daemon mints +into its state directory on request and that expires after 120 seconds, so a +blind one-shot write to the socket cannot erase the account; a same-account +process that can read the state directory can still complete that exchange. Run untrusted desktop software under a separate OS identity or sandbox that cannot access the account's runtime directory. In particular, do not describe the `0600` socket as authenticating individual same-user applications.