diff --git a/Cargo.lock b/Cargo.lock --- a/Cargo.lock +++ b/Cargo.lock @@ -1302,6 +1302,7 @@ dependencies = [ "eframe", "egui", "fmv-macos-events", + "getrandom 0.4.3", "log", "muda", "objc2 0.6.5", diff --git a/apps/designcraft-cli/src/main.rs b/apps/designcraft-cli/src/main.rs --- a/apps/designcraft-cli/src/main.rs +++ b/apps/designcraft-cli/src/main.rs @@ -144,6 +144,21 @@ fn main() -> ExitCode { } } +/// The control token a client presents: `DESIGNCRAFT_CONTROL_TOKEN`, else the file the app wrote +/// in its profile directory, named by `DESIGNCRAFT_CONTROL_TOKEN_FILE`. The app prints where that +/// file is when it opens the channel (#1585). +fn control_token() -> Option { + if let Ok(t) = std::env::var("DESIGNCRAFT_CONTROL_TOKEN") { + let t = t.trim().to_string(); + if !t.is_empty() { + return Some(t); + } + } + let path = std::env::var_os("DESIGNCRAFT_CONTROL_TOKEN_FILE")?; + let t = std::fs::read_to_string(path).ok()?.trim().to_string(); + (!t.is_empty()).then_some(t) +} + fn report(r: Result<(), String>) -> ExitCode { match r { Ok(()) => ExitCode::SUCCESS, @@ -223,7 +238,7 @@ so drop --connect (or use --connect without the roots)" Some(c) => { let addr = control_addr(&c); Box::new( - Remote::connect(&addr) + Remote::connect_with_token(&addr, control_token()) .map_err(|e| format!("cannot connect to the DesignCraft app at {addr}: {e} (start it with `designcraft --control PORT`)"))?, ) } @@ -506,7 +521,7 @@ fn script(args: &[String]) -> Result<(), String> { Some(c) => { let addr = control_addr(c); Box::new( - Remote::connect(&addr) + Remote::connect_with_token(&addr, control_token()) .map_err(|e| format!("cannot connect to the DesignCraft app at {addr}: {e} (start it with `designcraft --control PORT`)"))?, ) } @@ -567,7 +582,7 @@ fn app(args: &[String]) -> Result<(), String> { } } let addr = control_addr(&port); - let mut r = Remote::connect(&addr) + let mut r = Remote::connect_with_token(&addr, control_token()) .map_err(|e| format!("cannot connect to the DesignCraft app at {addr}: {e} (start it with `designcraft --control {port}`)"))?; let json_arg = |s: Option<&String>| -> Result { s.map_or(Ok(json!({})), |t| serde_json::from_str(t).map_err(|e| format!("bad JSON: {e}"))) }; diff --git a/apps/designcraft/Cargo.toml b/apps/designcraft/Cargo.toml --- a/apps/designcraft/Cargo.toml +++ b/apps/designcraft/Cargo.toml @@ -11,6 +11,8 @@ default-run = "designcraft" workspace = true [dependencies] +# The control channel's per-launch token comes from the OS random source (#1585). +getrandom = "0.4" designcraft-engine = { workspace = true } designcraft-ui-egui = { workspace = true } eframe = { workspace = true, features = ["default_fonts", "wgpu", "accesskit", "wayland", "x11", "links", "clipboard"] } diff --git a/apps/designcraft/src/control_server.rs b/apps/designcraft/src/control_server.rs --- a/apps/designcraft/src/control_server.rs +++ b/apps/designcraft/src/control_server.rs @@ -23,7 +23,7 @@ pub const MAX_LINE: usize = 4 * 1024 * 1024; /// Connections served at once; further ones get an error line and are closed. pub const MAX_CONNECTIONS: usize = 16; -pub fn start(port: u16, ctx: egui::Context) -> Receiver { +pub fn start(port: u16, token: String, ctx: egui::Context) -> Receiver { let (tx, rx) = channel::(); let listener = match TcpListener::bind(("127.0.0.1", port)) { Ok(l) => l, @@ -33,11 +33,11 @@ pub fn start(port: u16, ctx: egui::Context) -> Receiver { } }; eprintln!("designcraft: control server listening on 127.0.0.1:{port}"); - std::thread::spawn(move || accept_loop(listener, tx, ctx)); + std::thread::spawn(move || accept_loop(listener, tx, ctx, token)); rx } -fn accept_loop(listener: TcpListener, tx: Sender, ctx: egui::Context) { +fn accept_loop(listener: TcpListener, tx: Sender, ctx: egui::Context, token: String) { let open = Arc::new(AtomicUsize::new(0)); for mut stream in listener.incoming().flatten() { if open.fetch_add(1, Ordering::SeqCst) >= MAX_CONNECTIONS { @@ -48,16 +48,17 @@ fn accept_loop(listener: TcpListener, tx: Sender, ctx: egui::Con let tx = tx.clone(); let ctx = ctx.clone(); let open = Arc::clone(&open); + let token = token.clone(); std::thread::spawn(move || { - serve(stream, &tx, &ctx); + serve(stream, &tx, &ctx, &token); open.fetch_sub(1, Ordering::SeqCst); }); } } -fn serve(stream: TcpStream, tx: &Sender, ctx: &egui::Context) { +fn serve(stream: TcpStream, tx: &Sender, ctx: &egui::Context, token: &str) { let Ok(read) = stream.try_clone() else { return }; - serve_lines(read, stream, |method, params| { + serve_lines_with_token(read, stream, token, |method, params| { let (req, rrx) = ControlRequest::new(method, params); tx.send(req).ok()?; ctx.request_repaint(); @@ -126,6 +127,49 @@ fn read_bounded_line(reader: &mut impl BufRead, buf: &mut Vec, max: usize) - Ok(true) } +/// [`serve_lines`] behind the launch's token: the first line of a connection must be the token +/// and nothing else, so a process that cannot read the token file runs nothing (#1585). +/// +/// The reply to the token line says only whether it was accepted. A wrong one closes the +/// connection, as any other line that is not a request does. +fn serve_lines_with_token(read: R, mut out: W, token: &str, handle: impl FnMut(String, Value) -> Option) { + let mut reader = BufReader::new(read); + let mut buf = Vec::new(); + loop { + match read_bounded_line(&mut reader, &mut buf, MAX_LINE) { + Ok(false) => return, + Err(e) => { + let _ = writeln!(out, "{}", json!({"ok": false, "error": e})).and_then(|()| out.flush()); + return; + } + Ok(true) => {} + } + let text = String::from_utf8_lossy(&buf); + let text = text.trim(); + if text.is_empty() { + continue; + } + // The token may come alone on the line or as `{"token": "..."}`, so a client can send it + // as JSON like everything else on this channel. + let given = match serde_json::from_str::(text) { + Ok(v) => v.get("token").and_then(Value::as_str).map(str::to_string).unwrap_or_default(), + Err(_) => text.to_string(), + }; + if crate::control_token::matches(token, &given) { + let _ = writeln!(out, "{}", json!({"ok": true})).and_then(|()| out.flush()); + break; + } + let _ = writeln!( + out, + "{}", + json!({"ok": false, "error": "the control channel needs this launch's token as the first line; closing the connection"}) + ) + .and_then(|()| out.flush()); + return; + } + serve_lines(reader, out, handle); +} + /// Serve requests from `read`, answering on `out`, until the peer closes, sends something that is /// not a request, or `handle` returns `None` (app gone). `handle` gets `(method, params)` and /// returns the reply object (the request's `id` is added here). @@ -171,6 +215,54 @@ mod tests { (replies, calls) } + /// The channel reaches every command, so a connection runs nothing until it has presented + /// this launch's token (#1585). + #[test] + fn nothing_runs_until_the_token_is_presented() { + const TOKEN: &str = "0123456789abcdef0123456789abcdef"; + let serve = |input: &str| { + let mut out = Vec::new(); + let mut calls: Vec = Vec::new(); + serve_lines_with_token(input.as_bytes(), &mut out, TOKEN, |m, _p| { + calls.push(m); + Some(json!({"ok": true, "result": null})) + }); + let replies: Vec = + String::from_utf8(out).unwrap().lines().filter(|l| !l.is_empty()).map(|l| serde_json::from_str(l).unwrap()).collect(); + (replies, calls) + }; + let request = r#"{"id":1,"method":"engine.execute","params":{}}"#; + + // The token, then a request: the token is acknowledged and the request runs. + let (replies, calls) = serve(&format!("{{\"token\":\"{TOKEN}\"}}\n{request}\n")); + assert_eq!(calls, ["engine.execute"], "the request ran"); + assert_eq!(replies[0]["ok"], json!(true), "the token was acknowledged"); + assert_eq!(replies[1]["id"], json!(1), "and the reply carries the request's id"); + + // The bare token on a line works too, for a client that is not sending JSON yet. + let (_, calls) = serve(&format!("{TOKEN}\n{request}\n")); + assert_eq!(calls, ["engine.execute"]); + + // No token: the request never reaches the engine, and the connection closes. + let (replies, calls) = serve(&format!("{request}\n{request}\n")); + assert!(calls.is_empty(), "nothing ran without the token"); + assert_eq!(replies.len(), 1, "one error and the connection is closed"); + assert_eq!(replies[0]["ok"], json!(false)); + assert!(replies[0]["error"].as_str().unwrap_or_default().contains("token"), "{:?}", replies[0]); + + // A wrong token, including one that is a prefix of the real one: nothing runs. + for wrong in [&TOKEN[..31], "0123456789abcdef0123456789abcdee", "", "{\"token\":null}"] { + let (replies, calls) = serve(&format!("{wrong}\n{request}\n")); + assert!(calls.is_empty(), "nothing ran for {wrong:?}"); + assert_eq!(replies[0]["ok"], json!(false), "{wrong:?}"); + } + + // A browser's cross-origin request still gets one error and no execution. + let (replies, calls) = serve("GET / HTTP/1.1\r\n"); + assert!(calls.is_empty()); + assert_eq!(replies[0]["ok"], json!(false)); + } + #[test] fn requests_are_answered_in_order_with_ids() { let (r, calls) = run(b"{\"id\":1,\"method\":\"a\"}\n\n{\"id\":2,\"method\":\"b\",\"params\":{}}\r\n{\"method\":\"c\"}"); @@ -266,7 +358,7 @@ mod tests { let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap(); let addr = listener.local_addr().unwrap(); let (tx, _rx) = channel::(); - std::thread::spawn(move || accept_loop(listener, tx, egui::Context::default())); + std::thread::spawn(move || accept_loop(listener, tx, egui::Context::default(), "token-for-the-test".into())); // Idle connections are accepted in order and held open. let held: Vec = (0..16).map(|_| TcpStream::connect(addr).unwrap()).collect(); let mut extra = TcpStream::connect(addr).unwrap(); diff --git a/apps/designcraft/src/control_token.rs b/apps/designcraft/src/control_token.rs new file mode 100644 --- /dev/null +++ b/apps/designcraft/src/control_token.rs @@ -0,0 +1,105 @@ +//! The token a control-channel client presents before it can run anything. +//! +//! `--control ` reaches every engine command: opening, saving and exporting with arbitrary +//! paths, and quitting without saving. The channel is opt-in and loopback only, but loopback is +//! not a boundary between users -- on a shared machine, or beside other local software, any +//! process that can connect could act with the user's file permissions (#1585). +//! +//! Each launch makes a fresh token, writes it to the profile directory readable only by its owner, +//! and prints where it is. A connection's first line must be that token. + +use std::io::Write; +use std::path::{Path, PathBuf}; + +/// A client may pass the token here instead of reading the file. +pub const TOKEN_ENV: &str = "DESIGNCRAFT_CONTROL_TOKEN"; +/// The file's name inside the profile directory. +pub const TOKEN_FILE: &str = "control-token"; +/// Token length in hex characters (128 bits). +const TOKEN_HEX: usize = 32; + +/// A fresh token from the operating system's random source, or `None` when it cannot be read -- +/// in which case the channel must not start, rather than start without a token. +pub fn new_token() -> Option { + let mut bytes = [0u8; TOKEN_HEX / 2]; + getrandom::fill(&mut bytes).ok()?; + Some(bytes.iter().map(|b| format!("{b:02x}")).collect()) +} + +/// `/control-token`. +pub fn token_path(config_dir: &Path) -> PathBuf { + config_dir.join(TOKEN_FILE) +} + +/// Write `token` where only its owner can read it, replacing any previous one. +pub fn write_token(config_dir: &Path, token: &str) -> std::io::Result { + std::fs::create_dir_all(config_dir)?; + let path = token_path(config_dir); + // Create with the mode set, rather than writing and then narrowing: a token must never exist + // readable, however briefly. + let mut options = std::fs::OpenOptions::new(); + options.write(true).create(true).truncate(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let mut file = options.open(&path)?; + file.write_all(token.as_bytes())?; + file.flush()?; + Ok(path) +} + +/// Whether `given` is `expected`, compared without giving away where they differ. +pub fn matches(expected: &str, given: &str) -> bool { + let (a, b) = (expected.as_bytes(), given.as_bytes()); + // The length is not a secret; the bytes are, so every one of them is looked at. + if a.len() != b.len() { + return false; + } + a.iter().zip(b).fold(0u8, |acc, (x, y)| acc | (x ^ y)) == 0 +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_token_is_128_random_bits_in_hex() { + let a = new_token().expect("the OS random source"); + let b = new_token().expect("the OS random source"); + assert_eq!(a.len(), TOKEN_HEX); + assert!(a.chars().all(|c| c.is_ascii_hexdigit())); + assert_ne!(a, b, "a fresh token each time"); + } + + #[test] + fn a_token_matches_only_itself() { + let t = "0123456789abcdef0123456789abcdef"; + assert!(matches(t, t)); + assert!(!matches(t, &t[..31]), "a prefix is not the token"); + assert!(!matches(t, &format!("{t}0")), "nor is a longer string"); + assert!(!matches(t, "0123456789abcdef0123456789abcdee"), "nor one byte off"); + assert!(!matches(t, ""), "nor nothing"); + } + + #[test] + fn the_token_file_is_written_for_its_owner_only() { + let dir = std::env::temp_dir().join(format!("dc-token-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let token = new_token().unwrap(); + let path = write_token(&dir, &token).expect("the token is written"); + assert_eq!(std::fs::read_to_string(&path).unwrap(), token); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600, "only its owner can read the token: {mode:o}"); + } + // Writing again replaces it, and leaves no second file behind. + let next = new_token().unwrap(); + write_token(&dir, &next).unwrap(); + assert_eq!(std::fs::read_to_string(&path).unwrap(), next); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/apps/designcraft/src/main.rs b/apps/designcraft/src/main.rs --- a/apps/designcraft/src/main.rs +++ b/apps/designcraft/src/main.rs @@ -16,6 +16,7 @@ #[cfg(target_os = "macos")] mod apple_events; mod control_server; +mod control_token; mod gpu; mod gpu_limits; mod logging; @@ -575,8 +576,26 @@ fn main() -> std::process::ExitCode { app.integrated_titlebar = cfg!(target_os = "macos"); app.custom_titlebar = cfg!(target_os = "windows"); if let Some(port) = control_port { - let rx = control_server::start(port, cc.egui_ctx.clone()); - app = app.with_control(rx); + // The channel reaches every command, so it opens only with a token a client + // has to read from the profile directory (#1585). Without a random source + // there is no token, and the channel does not open at all: starting it + // unauthenticated would be the thing the token is for. + match control_token::new_token() { + Some(token) => { + match profile::config_dir().map(|dir| control_token::write_token(&dir, &token)) { + Some(Ok(path)) => eprintln!("designcraft: the control token is in {}", path.display()), + Some(Err(e)) => { + eprintln!("designcraft: the control token could not be written: {e} (pass it with {})", control_token::TOKEN_ENV) + } + None => { + eprintln!("designcraft: no profile directory for the control token (pass it with {})", control_token::TOKEN_ENV) + } + } + let rx = control_server::start(port, token, cc.egui_ctx.clone()); + app = app.with_control(rx); + } + None => eprintln!("designcraft: the control server did not start: no random source for its token"), + } } if sample { let _ = app.run("file.newSample", serde_json::json!({})); diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -28,12 +28,22 @@ pub struct Remote { addr: String, conn: Option<(BufReader, TcpStream)>, next_id: u64, + /// The launch's control token, sent as the first line of every connection (#1585). + token: Option, } impl Remote { /// Connect to `addr` (`127.0.0.1:7979`), failing fast when nothing is listening. + /// + /// The app's control channel asks for the launch's token first (#1585), so pass it with + /// [`Self::connect_with_token`]; this stays for callers talking to a channel without one. pub fn connect(addr: &str) -> std::io::Result { - let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1 }; + Self::connect_with_token(addr, None) + } + + /// [`Self::connect`], presenting `token` as the first line of the connection. + pub fn connect_with_token(addr: &str, token: Option) -> std::io::Result { + let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1, token }; r.reconnect()?; Ok(r) } @@ -53,6 +63,7 @@ impl Remote { s.set_read_timeout(Some(Duration::from_secs(90))).ok(); let read = s.try_clone()?; self.conn = Some((BufReader::new(read), s)); + self.greet()?; return Ok(()); } Err(e) => last = e, @@ -61,6 +72,31 @@ impl Remote { Err(last) } + /// Present the token, if there is one, and read the app's answer. A channel that asks for a + /// token and does not get one closes, so a clear error here beats a mystery later. + fn greet(&mut self) -> std::io::Result<()> { + let Some(token) = self.token.clone() else { return Ok(()) }; + let Some((reader, writer)) = self.conn.as_mut() else { + return Err(std::io::Error::new(std::io::ErrorKind::NotConnected, "no connection")); + }; + writeln!(writer, "{}", serde_json::json!({"token": token}))?; + writer.flush()?; + let mut line = String::new(); + reader.read_line(&mut line)?; + let ok = serde_json::from_str::(line.trim()) + .ok() + .and_then(|v| v.get("ok").and_then(serde_json::Value::as_bool)) + .unwrap_or(false); + if ok { + return Ok(()); + } + self.conn = None; + Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "the app refused the control token (it is in the profile directory, or pass DESIGNCRAFT_CONTROL_TOKEN)", + )) + } + fn prepare_connection(&mut self) -> std::io::Result<()> { if let Some((reader, writer)) = self.conn.take() { writer.set_nonblocking(true)?; diff --git a/docs/control-protocol.md b/docs/control-protocol.md --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -3,6 +3,24 @@ Start the app with `--control ` (or `DESIGNCRAFT_CONTROL_PORT`). The server listens on `127.0.0.1` only and speaks JSON lines: one request object per line, one reply per line. +**The first line of every connection is this launch's token.** The channel reaches every command +-- open, save and export with any path, and quitting without saving -- and loopback is not a +boundary between users, so a connection runs nothing until it presents the token: + +```json +{"token": "0f8c…"} +{"ok": true} +``` + +A bare token on its own line is accepted too. A wrong or missing token gets one error reply and +the connection is closed, like any other line that is not a request. + +Each launch makes a fresh 128-bit token and writes it to the profile directory as `control-token`, +readable only by its owner (mode 600 on unix); the app prints the path on startup. Clients: +`designcraft-cli mcp --connect` reads `DESIGNCRAFT_CONTROL_TOKEN`, else the file named by +`DESIGNCRAFT_CONTROL_TOKEN_FILE`. Without a random source for a token the channel does not open at +all, rather than opening without one. + For an isolated native test profile, set `DESIGNCRAFT_CONFIG_DIR` to a dedicated directory before launch. UI/engine preferences and the `Recovery` directory then live under that root. An unset override preserves the usual platform paths (or `DesignCraftData` next to the executable when a @@ -22,8 +40,9 @@ nothing sent after it on that connection runs. That covers text that isn't JSON, an object without `method`, invalid UTF-8, and a line longer than 4 MiB. An HTTP request (for example a web page's cross-origin `fetch` to `127.0.0.1:`) therefore can't smuggle a command in its body: its request line is rejected first. At most 16 connections are served at once; further ones get an error -line and are closed. Clients that get an error reply should reconnect. The port has no authentication, -so only enable it while you use it. Transport: `apps/designcraft/src/control_server.rs`. +line and are closed. Clients that get an error reply should reconnect. The token above is what +stands between the channel and any other local process, so still only enable the port while you +use it. Transport: `apps/designcraft/src/control_server.rs`. ```json {"id": 1, "method": "engine.execute", "params": {"command": "frame.create", "params": {"rect": [36, 36, 300, 200], "content": "text"}}}