Security: the --control server requires an auth token and closes bad connections (#777)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

crates/mcp/src/backend.rs

@@ -25,14 +25,22 @@ const MAX_REPLY: u64 = 256 * 1024 * 1024;
 /// A running GridCraft app, reached through its loopback control port.
 pub struct Remote {
     addr: String,
+    token: String,
     conn: Option<(BufReader<TcpStream>, TcpStream)>,
     next_id: u64,
 }
 
 impl Remote {
-    /// Connect to `addr` (`127.0.0.1:7979`), failing fast when nothing is listening.
+    /// Connect to `addr` (`127.0.0.1:7979`), failing fast when nothing is listening. The app
+    /// requires a token (#777): `GRIDCRAFT_CONTROL_TOKEN`, else the file the app wrote for this port.
     pub fn connect(addr: &str) -> std::io::Result<Self> {
-        let mut r = Self { addr: addr.to_string(), conn: None, next_id: 1 };
+        let token = crate::control_auth::client_token(addr).map_err(|e| std::io::Error::new(std::io::ErrorKind::PermissionDenied, e))?;
+        Self::connect_with_token(addr, token)
+    }
+
+    /// Like [`Remote::connect`] with a token the caller already has.
+    pub fn connect_with_token(addr: &str, token: String) -> std::io::Result<Self> {
+        let mut r = Self { addr: addr.to_string(), token, conn: None, next_id: 1 };
         r.reconnect()?;
         Ok(r)
     }
@@ -51,7 +59,9 @@ impl Remote {
                     s.set_read_timeout(Some(IO_TIMEOUT)).ok();
                     s.set_write_timeout(Some(IO_TIMEOUT)).ok();
                     let read = s.try_clone()?;
-                    self.conn = Some((BufReader::new(read), s));
+                    let mut conn = (BufReader::new(read), s);
+                    Self::authenticate(&mut conn, &self.token)?;
+                    self.conn = Some(conn);
                     return Ok(());
                 }
                 Err(e) => last = e,
@@ -60,6 +70,25 @@ impl Remote {
         Err(last)
     }
 
+    /// The handshake every connection starts with: the app closes one that doesn't send the token first.
+    fn authenticate(conn: &mut (BufReader<TcpStream>, TcpStream), token: &str) -> std::io::Result<()> {
+        let (reader, writer) = conn;
+        writer.write_all(crate::control_auth::auth_line(0, token).as_bytes())?;
+        writer.write_all(b"\n")?;
+        writer.flush()?;
+        let mut reply = String::new();
+        let n = std::io::Read::take(reader, 64 * 1024).read_line(&mut reply)?;
+        let ok = n > 0 && serde_json::from_str::<Value>(reply.trim()).ok().and_then(|v| v.get("ok").and_then(Value::as_bool)) == Some(true);
+        if ok {
+            Ok(())
+        } else {
+            Err(std::io::Error::new(
+                std::io::ErrorKind::PermissionDenied,
+                "the app refused the control token (is GRIDCRAFT_CONTROL_TOKEN or the app's token file for this port out of date?)",
+            ))
+        }
+    }
+
     fn roundtrip(&mut self, line: &str) -> std::io::Result<String> {
         if self.conn.is_none() {
             self.reconnect()?;

crates/mcp/src/control_auth.rs

@@ -0,0 +1,135 @@
+//! Authentication of the control channel (#777).
+//!
+//! Anything on the machine, and a web page that POSTs `text/plain` to `127.0.0.1:<port>`, can
+//! reach a loopback port. So the first line of every connection must be
+//! `{"id":…,"method":"auth","params":{"token":"…"}}`, with the secret the app made at start. The
+//! app keeps it in memory and writes it to a file only the user can read (or the user chooses it
+//! with `GRIDCRAFT_CONTROL_TOKEN`); [`Remote`](crate::Remote) reads it from the same places.
+
+use std::path::PathBuf;
+
+use serde_json::{Value, json};
+
+/// Environment variable holding a token chosen by the user. The app uses it instead of making
+/// one (and writes no file); clients use it instead of reading the file.
+pub const TOKEN_ENV: &str = "GRIDCRAFT_CONTROL_TOKEN";
+/// The method of the handshake line.
+pub const AUTH_METHOD: &str = "auth";
+/// Shortest token the app accepts from the user (a made one has 256 bits).
+pub const MIN_TOKEN_LEN: usize = 16;
+/// Longest first line the app reads from a peer that hasn't authenticated yet.
+pub const MAX_AUTH_LINE: usize = 4096;
+
+/// Compare two secrets without stopping at the first difference. Unequal lengths are unequal
+/// at once (the length of a token isn't secret).
+pub fn constant_time_eq(a: &[u8], b: &[u8]) -> bool {
+    if a.len() != b.len() {
+        return false;
+    }
+    let mut diff = 0u8;
+    for (x, y) in a.iter().zip(b) {
+        diff |= x ^ y;
+    }
+    // Keep the optimiser from turning the loop into an early exit.
+    std::hint::black_box(diff) == 0
+}
+
+/// A made token as lowercase hex (two characters per byte).
+pub fn hex(bytes: &[u8]) -> String {
+    const DIGITS: &[u8; 16] = b"0123456789abcdef";
+    let mut s = String::with_capacity(bytes.len().saturating_mul(2));
+    for b in bytes {
+        s.push(char::from(DIGITS[usize::from(b >> 4)]));
+        s.push(char::from(DIGITS[usize::from(b & 15)]));
+    }
+    s
+}
+
+/// The handshake line a client sends first.
+pub fn auth_line(id: u64, token: &str) -> String {
+    json!({"id": id, "method": AUTH_METHOD, "params": {"token": token}}).to_string()
+}
+
+/// Check a connection's first line. `Ok` is the reply to send before serving further requests;
+/// `Err` is the reply for a refused connection, which the caller sends and then closes. The
+/// refusal is the same whatever was wrong, so it tells a stranger nothing.
+pub fn check_first_line(line: &str, token: &str) -> Result<Value, Value> {
+    let refused = || json!({"ok": false, "error": "authentication required"});
+    let msg: Value = serde_json::from_str(line.trim()).map_err(|_| refused())?;
+    if msg.get("method").and_then(Value::as_str) != Some(AUTH_METHOD) {
+        return Err(refused());
+    }
+    let given = msg.get("params").and_then(|p| p.get("token")).and_then(Value::as_str).unwrap_or("");
+    // Always compare, so a missing token costs the same as a wrong one.
+    if constant_time_eq(given.as_bytes(), token.as_bytes()) && !token.is_empty() {
+        Ok(json!({"id": msg.get("id").cloned().unwrap_or(Value::Null), "ok": true, "result": {"authenticated": true}}))
+    } else {
+        Err(refused())
+    }
+}
+
+/// Where the app writes the token it made for `port`, and where clients look for it: a folder
+/// only the user can open (`$XDG_RUNTIME_DIR/gridcraft`, else `~/.cache/gridcraft`; on Windows
+/// `%USERPROFILE%\.gridcraft`), file `control-<port>.token`. `None` when no such folder is known.
+pub fn token_path(port: u16) -> Option<PathBuf> {
+    let dir = if cfg!(windows) {
+        PathBuf::from(std::env::var_os("USERPROFILE").filter(|v| !v.is_empty())?).join(".gridcraft")
+    } else if let Some(run) = std::env::var_os("XDG_RUNTIME_DIR").filter(|v| !v.is_empty()) {
+        PathBuf::from(run).join("gridcraft")
+    } else {
+        PathBuf::from(std::env::var_os("HOME").filter(|v| !v.is_empty())?).join(".cache").join("gridcraft")
+    };
+    Some(dir.join(format!("control-{port}.token")))
+}
+
+/// The port of a control address (`127.0.0.1:7979` → 7979).
+fn port_of(addr: &str) -> Option<u16> {
+    addr.rsplit(':').next()?.parse().ok()
+}
+
+/// The token a client uses for `addr`: [`TOKEN_ENV`] if set, else the file the app wrote.
+pub fn client_token(addr: &str) -> Result<String, String> {
+    if let Ok(t) = std::env::var(TOKEN_ENV)
+        && !t.trim().is_empty()
+    {
+        return Ok(t.trim().to_string());
+    }
+    let path = port_of(addr).and_then(token_path).ok_or_else(|| format!("no control token: set {TOKEN_ENV}"))?;
+    match std::fs::read_to_string(&path) {
+        Ok(t) if !t.trim().is_empty() => Ok(t.trim().to_string()),
+        _ => Err(format!("no control token: set {TOKEN_ENV}, or start the app on this machine and user (it writes {})", path.display())),
+    }
+}
+
+/// Write `token` to [`token_path`] for the app: the folder is made private (mode 0700 on Unix),
+/// an older file is replaced, and the new one is created readable by the user only (mode 0600).
+/// Returns the path.
+pub fn write_token_file(port: u16, token: &str) -> std::io::Result<PathBuf> {
+    use std::io::Write;
+    let path = token_path(port).ok_or_else(|| std::io::Error::new(std::io::ErrorKind::NotFound, "no user folder for the control token"))?;
+    let dir = path.parent().ok_or_else(|| std::io::Error::other("token path has no folder"))?;
+    std::fs::create_dir_all(dir)?;
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        // Only our own folder: leave a shared one (`$XDG_RUNTIME_DIR` itself) as it is.
+        std::fs::set_permissions(dir, std::fs::Permissions::from_mode(0o700))?;
+    }
+    // Replace rather than open: a leftover file may have looser permissions, or be a link.
+    match std::fs::remove_file(&path) {
+        Ok(()) => {}
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
+        Err(e) => return Err(e),
+    }
+    let mut opts = std::fs::OpenOptions::new();
+    opts.write(true).create_new(true);
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::OpenOptionsExt;
+        opts.mode(0o600);
+    }
+    let mut f = opts.open(&path)?;
+    f.write_all(token.as_bytes())?;
+    f.write_all(b"\n")?;
+    Ok(path)
+}

crates/mcp/src/lib.rs

@@ -15,6 +15,7 @@
 #![forbid(unsafe_code)]
 
 mod backend;
+pub mod control_auth;
 mod headless;
 mod server;
 mod tools;

crates/mcp/src/tests.rs

@@ -363,6 +363,11 @@ fn remote_backend_over_tcp() {
             let stream = stream.unwrap();
             let mut w = stream.try_clone().unwrap();
             let mut r = BufReader::new(stream);
+            // Every connection starts with the token, as the app requires.
+            let mut first = String::new();
+            r.read_line(&mut first).unwrap();
+            let hello = crate::control_auth::check_first_line(&first, "secret-token-0123456789").unwrap();
+            w.write_all(format!("{hello}\n").as_bytes()).unwrap();
             loop {
                 let mut line = String::new();
                 if r.read_line(&mut line).unwrap_or(0) == 0 {
@@ -382,7 +387,7 @@ fn remote_backend_over_tcp() {
     });
     let addr = control_addr(&port.to_string());
     assert_eq!(addr, format!("127.0.0.1:{port}"));
-    let mut s = Server::new(Box::new(Remote::connect(&addr).unwrap()));
+    let mut s = Server::new(Box::new(Remote::connect_with_token(&addr, "secret-token-0123456789".into()).unwrap()));
     assert_eq!(ok(&mut s, "evaluate_formula", json!({"formula": "=1+1"}))["result"], json!(2.0));
     // Second call reconnects transparently.
     ok(&mut s, "set_cell", json!({"cell": "A1", "input": "7"}));
@@ -391,7 +396,49 @@ fn remote_backend_over_tcp() {
     assert!(msg.contains("unknown command"), "{msg}");
     drop(s);
     handle.join().unwrap();
-    assert!(Remote::connect("127.0.0.1:1").is_err());
+    assert!(Remote::connect_with_token("127.0.0.1:1", "x".into()).is_err());
+}
+
+#[test]
+fn control_auth_handshake_and_compare() {
+    use crate::control_auth::{auth_line, check_first_line, constant_time_eq, hex};
+    let token = "0123456789abcdef0123456789abcdef";
+    let ok = check_first_line(&auth_line(7, token), token).unwrap();
+    assert_eq!((ok["id"].clone(), ok["ok"].clone()), (json!(7), json!(true)));
+    // Wrong or missing token, another method, and non-JSON are all refused the same way.
+    let wrong = check_first_line(&auth_line(1, "0123456789abcdef0123456789abcdee"), token).unwrap_err();
+    for bad in [
+        auth_line(1, ""),
+        r#"{"id":1,"method":"auth","params":{}}"#.into(),
+        r#"{"id":1,"method":"app.save","params":{"path":"/tmp/x","token":"0123456789abcdef0123456789abcdef"}}"#.into(),
+        "POST / HTTP/1.1".into(),
+        String::new(),
+    ] {
+        assert_eq!(check_first_line(&bad, token).unwrap_err(), wrong, "{bad}");
+    }
+    // An empty configured token never matches an empty given one.
+    assert!(check_first_line(&auth_line(1, ""), "").is_err());
+    assert!(constant_time_eq(b"abc", b"abc"));
+    assert!(!constant_time_eq(b"abc", b"abd"));
+    assert!(!constant_time_eq(b"abc", b"abcd"));
+    assert_eq!(hex(&[0x00, 0xab, 0xff]), "00abff");
+}
+
+#[cfg(unix)]
+#[test]
+fn control_token_file_is_private() {
+    use std::os::unix::fs::PermissionsExt;
+    // Unique port per run so parallel tests don't share a file; the folder is the real one.
+    let port = 40_000 + (std::process::id() % 20_000) as u16;
+    let path = crate::control_auth::write_token_file(port, "tok-0123456789abcdef").unwrap();
+    assert_eq!(std::fs::metadata(&path).unwrap().permissions().mode() & 0o777, 0o600);
+    assert_eq!(std::fs::metadata(path.parent().unwrap()).unwrap().permissions().mode() & 0o777, 0o700);
+    // A looser leftover is replaced, not reused.
+    std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o666)).unwrap();
+    crate::control_auth::write_token_file(port, "tok-1123456789abcdef").unwrap();
+    assert_eq!(std::fs::metadata(&path).unwrap().permissions().mode() & 0o777, 0o600);
+    assert_eq!(std::fs::read_to_string(&path).unwrap().trim(), "tok-1123456789abcdef");
+    std::fs::remove_file(&path).unwrap();
 }
 
 #[test]