From de8e48bee7dc8bf9ae7a9561f27c5b7394217b3e Mon Sep 17 00:00:00 2001 From: Ultradesu Date: Fri, 2 Oct 2026 14:41:58 +0100 Subject: [PATCH] Let the agent grant the control socket to a group (0660) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For a GUI (or a CLI) running as a different user to reach the agent, the control socket needs to be group-accessible. This is set by the agent when it binds the socket — correct from creation, not patched afterwards. - ipc::ControlSocketAccess { Private, Group(gid) }. Private is the existing owner-only 0600. Group sets the socket's group to the gid and mode 0660, and creates the socket's parent directory 0755 so the group can traverse to it; if the group cannot be set it falls back to owner-only rather than leaving it wrong. The chown uses std::os::unix::fs::chown — no unsafe, no new lib dependency. - ControlSocket::bind takes the access policy (Windows accepts it and keeps its pipe default for now; a pipe security descriptor is a later follow-up). - CLI: `tsunagi up --control-group ` (env TSUNAGI_CONTROL_GROUP) resolves the group to its gid via nix (safe wrapper, unix-only dep) and passes Group(gid); without it the socket stays Private. A name that does not resolve is reported, and the socket stays owner-only. - control_socket() now serves the system path (default_control_socket_path) on macOS too, not just Windows: /var/run/tsunagi/agent.sock, a runtime directory the GUI can reach, rather than the owner-only state directory. A custom --state-dir still keeps its own per-user socket. So the packaged/daemon agent runs e.g. `tsunagi up --control-group tsunagi` and a user in that group talks to it; a plain per-user agent is unchanged (0600). Checks: fmt, clippy --workspace --all-targets -D warnings, tsunagi lib tests and the wg-quic local_control tests pass on macOS. Co-Authored-By: Claude Opus 4.8 --- Cargo.lock | 1 + crates/tsunagi-cli/Cargo.toml | 5 ++ crates/tsunagi-cli/src/main.rs | 53 +++++++++++++++- crates/tsunagi-wg-quic/tests/local_control.rs | 36 +++++++---- crates/tsunagi/src/ipc/mod.rs | 15 +++++ crates/tsunagi/src/ipc/unix.rs | 61 ++++++++++++++++--- crates/tsunagi/src/ipc/windows.rs | 15 ++++- 7 files changed, 161 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6229c7e..e70f9df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4245,6 +4245,7 @@ dependencies = [ "clap", "iroh", "netwatch", + "nix 0.31.3", "simple-dns", "tempfile", "tokio", diff --git a/crates/tsunagi-cli/Cargo.toml b/crates/tsunagi-cli/Cargo.toml index 2e86f75..00253ad 100644 --- a/crates/tsunagi-cli/Cargo.toml +++ b/crates/tsunagi-cli/Cargo.toml @@ -33,6 +33,11 @@ anstyle = "1.0" # Local interface addresses, for the diagnostics in `status`. netwatch = "0.19.3" +# Resolving a group name to its gid for `up --control-group`, through a safe +# wrapper (no unsafe of our own). Unix only. +[target.'cfg(unix)'.dependencies] +nix = { version = "0.31", features = ["user"] } + [dev-dependencies] tokio = { version = "1.53", features = ["rt", "rt-multi-thread", "sync", "time", "macros", "process"] } tempfile.workspace = true diff --git a/crates/tsunagi-cli/src/main.rs b/crates/tsunagi-cli/src/main.rs index e8f4e4b..a0b8700 100644 --- a/crates/tsunagi-cli/src/main.rs +++ b/crates/tsunagi-cli/src/main.rs @@ -482,6 +482,17 @@ struct UpArgs { /// Control socket to serve. Derived from the state directory by default. #[arg(long, env = "TSUNAGI_CONTROL_SOCKET")] control_socket: Option, + + /// Group whose members may use the control socket (Unix, mode 0660), so a + /// GUI or CLI running as another user can reach the agent. Without it the + /// socket is owner-only (0600). The group must already exist. + #[arg( + long, + value_name = "GROUP", + env = "TSUNAGI_CONTROL_GROUP", + help_heading = "System" + )] + control_group: Option, } /// Reads the shared secret from an argument or a file. @@ -748,6 +759,31 @@ async fn run(command: Command) -> Result<(), Box> { } } +/// Resolves the control-socket access policy from an optional group name. +/// +/// The group must already exist (packaging/post-install creates it); a name +/// that cannot be resolved is an error the caller reports rather than a silent +/// fallback that would leave a GUI unable to connect without explanation. +#[cfg(unix)] +fn control_socket_access(group: Option<&str>) -> Result { + match group { + None => Ok(tsunagi::ipc::ControlSocketAccess::Private), + Some(name) => { + let group = nix::unistd::Group::from_name(name) + .map_err(|err| format!("cannot look up group `{name}`: {err}"))? + .ok_or_else(|| format!("group `{name}` does not exist; create it first"))?; + Ok(tsunagi::ipc::ControlSocketAccess::Group(group.gid.as_raw())) + } + } +} + +#[cfg(not(unix))] +fn control_socket_access( + _group: Option<&str>, +) -> Result { + Ok(tsunagi::ipc::ControlSocketAccess::Private) +} + /// Path of the local control socket for a state directory. fn control_socket( paths: &StoragePaths, @@ -757,7 +793,10 @@ fn control_socket( if let Some(path) = override_path { return path.clone(); } - #[cfg(windows)] + // On Windows and macOS the agent runs as a system service and serves at a + // fixed system path (a runtime directory the GUI can reach), not inside the + // owner-only state directory. + #[cfg(any(windows, target_os = "macos"))] if !_is_custom_state_dir { return tsunagi::ipc::default_control_socket_path(); } @@ -3859,7 +3898,17 @@ async fn up(args: UpArgs) -> Result<(), Box> { }); let is_custom_state = args.paths.state_dir.is_some(); let path = control_socket(&paths, args.control_socket.as_ref(), is_custom_state); - match tsunagi::ipc::ControlSocket::bind(path, source).await { + let access = match control_socket_access(args.control_group.as_deref()) { + Ok(access) => access, + Err(reason) => { + eprintln!( + "warning: cannot grant the control socket to a group ({reason}); \ + it will be owner-only" + ); + tsunagi::ipc::ControlSocketAccess::Private + } + }; + match tsunagi::ipc::ControlSocket::bind(path, source, access).await { Ok(socket) => { println!(" control {}", socket.path().display()); Some(socket) diff --git a/crates/tsunagi-wg-quic/tests/local_control.rs b/crates/tsunagi-wg-quic/tests/local_control.rs index 8b193e4..38eba8d 100644 --- a/crates/tsunagi-wg-quic/tests/local_control.rs +++ b/crates/tsunagi-wg-quic/tests/local_control.rs @@ -12,7 +12,7 @@ use std::time::Duration; use tempfile::TempDir; use tsunagi::dataplane::IpPlugin; use tsunagi::discovery::SharedMemoryDiscovery; -use tsunagi::ipc::{ControlSocket, request_status}; +use tsunagi::ipc::{ControlSocket, ControlSocketAccess, request_status}; use tsunagi::ipc::{StatusReport, control_socket_path}; use tsunagi::overlay::MemoryTunFactory; use tsunagi::testing::{config_with, network, wait_for_peers, wait_until}; @@ -130,9 +130,13 @@ async fn a_client_sees_the_agent_and_its_overlay() { // A short path: a Unix socket address is limited to about 100 bytes. let socket_path = dir_a.path().join("agent.sock"); - let control = ControlSocket::bind(&socket_path, source(agent.clone(), plugin.clone())) - .await - .unwrap(); + let control = ControlSocket::bind( + &socket_path, + source(agent.clone(), plugin.clone()), + ControlSocketAccess::Private, + ) + .await + .unwrap(); let report = wait_until("the overlay is reported as up", || { let socket_path = socket_path.clone(); @@ -193,13 +197,13 @@ async fn a_leftover_socket_file_is_replaced_but_a_live_one_is_not() { // A file with nobody listening is a leftover from a crash. std::fs::write(&path, b"stale").unwrap(); - let first = ControlSocket::bind(&path, Arc::clone(&empty)) + let first = ControlSocket::bind(&path, Arc::clone(&empty), ControlSocketAccess::Private) .await .unwrap(); assert!(request_status(&path).await.is_ok()); // A live socket is not stolen from the agent that owns it. - let second = ControlSocket::bind(&path, Arc::clone(&empty)).await; + let second = ControlSocket::bind(&path, Arc::clone(&empty), ControlSocketAccess::Private).await; assert!( matches!(second, Err(tsunagi::Error::StateLocked { .. })), "a second agent must not take over a live control socket" @@ -321,9 +325,13 @@ async fn a_client_can_leave_a_network_through_the_running_agent() { wait_for_peers(&agent, network_id, 1).await; let socket_path = dir.path().join("control.sock"); - let control = ControlSocket::bind(&socket_path, Arc::new(Control(agent.clone()))) - .await - .unwrap(); + let control = ControlSocket::bind( + &socket_path, + Arc::new(Control(agent.clone())), + ControlSocketAccess::Private, + ) + .await + .unwrap(); let report = tsunagi::ipc::leave_network(&socket_path, &network_id.to_string()) .await @@ -361,9 +369,13 @@ async fn a_client_can_add_a_network_to_a_running_agent() { agent.join_network(&first, &first_secret).await.unwrap(); let socket_path = dir.path().join("control.sock"); - let control = ControlSocket::bind(&socket_path, Arc::new(Control(agent.clone()))) - .await - .unwrap(); + let control = ControlSocket::bind( + &socket_path, + Arc::new(Control(agent.clone())), + ControlSocketAccess::Private, + ) + .await + .unwrap(); let report = tsunagi::ipc::join_network( &socket_path, diff --git a/crates/tsunagi/src/ipc/mod.rs b/crates/tsunagi/src/ipc/mod.rs index 689788d..70d6d9c 100644 --- a/crates/tsunagi/src/ipc/mod.rs +++ b/crates/tsunagi/src/ipc/mod.rs @@ -44,6 +44,21 @@ use windows as transport; #[cfg(any(unix, windows))] pub use transport::ControlSocket; +/// Who may open the local control socket. +/// +/// The default is owner-only. A system service shares its socket with a GUI or +/// CLI running as another user by granting a group; the agent sets that on the +/// socket when it binds it, so the access is correct from creation rather than +/// patched afterwards. +#[derive(Debug, Clone)] +pub enum ControlSocketAccess { + /// Owner only — mode `0600`. The per-user agent's default. + Private, + /// The owner and members of this group — the socket's group is set to this + /// gid and the mode to `0660`. The caller resolves a group name to its gid. + Group(u32), +} + use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; diff --git a/crates/tsunagi/src/ipc/unix.rs b/crates/tsunagi/src/ipc/unix.rs index d8922a9..15f4930 100644 --- a/crates/tsunagi/src/ipc/unix.rs +++ b/crates/tsunagi/src/ipc/unix.rs @@ -22,8 +22,8 @@ use crate::error::{Error, Result}; // The transport-agnostic surface, re-exported so this module is a complete // view of the local control interface on its own. pub use super::{ - CONTROL_PROTOCOL, EXCHANGE_TIMEOUT, ReportSource, is_serving, join_network, leave_network, - request_status, set_active, set_dns, set_hostname, + CONTROL_PROTOCOL, ControlSocketAccess, EXCHANGE_TIMEOUT, ReportSource, is_serving, + join_network, leave_network, request_status, set_active, set_dns, set_hostname, }; /// Serves the local control interface on a Unix socket. @@ -39,11 +39,15 @@ impl ControlSocket { /// A socket file left behind by a crashed agent is replaced, but only /// after checking that nothing is listening on it, so two live agents /// never fight over one path. - pub async fn bind(path: impl AsRef, source: Arc) -> Result { + pub async fn bind( + path: impl AsRef, + source: Arc, + access: ControlSocketAccess, + ) -> Result { let path = path.as_ref().to_path_buf(); if let Some(parent) = path.parent() { - crate::storage::create_dir(parent)?; + create_parent(parent, &access)?; } if path.exists() { @@ -61,7 +65,7 @@ impl ControlSocket { path: path.clone(), source, })?; - restrict(&path)?; + apply_access(&path, &access)?; let task = tokio::spawn(serve(listener, source)); Ok(Self { @@ -94,9 +98,48 @@ impl Drop for ControlSocket { } } -fn restrict(path: &Path) -> Result<()> { +/// Creates the socket's parent directory. +/// +/// A group-shared socket must sit in a directory the group can traverse, so it +/// is `0755`; a private socket keeps the owner-only state-directory default. +fn create_parent(parent: &Path, access: &ControlSocketAccess) -> Result<()> { + match access { + ControlSocketAccess::Private => crate::storage::create_dir(parent), + ControlSocketAccess::Group(_) => { + std::fs::create_dir_all(parent).map_err(|source| Error::Io { + path: parent.to_path_buf(), + source, + })?; + set_mode(parent, 0o755) + } + } +} + +/// Applies the access policy to the bound socket. +/// +/// Private is owner-only `0600`. Group sets the socket's group to the gid and +/// the mode to `0660`; if the group cannot be set — the agent is not a member, +/// say — it falls back to owner-only rather than leaving the group wrong, so +/// the socket is never more open than intended. +fn apply_access(path: &Path, access: &ControlSocketAccess) -> Result<()> { + match access { + ControlSocketAccess::Private => set_mode(path, 0o600), + ControlSocketAccess::Group(gid) => { + if let Err(source) = std::os::unix::fs::chown(path, None, Some(*gid)) { + tracing::warn!( + path = %path.display(), gid, %source, + "cannot set the control socket group; keeping it owner-only" + ); + return set_mode(path, 0o600); + } + set_mode(path, 0o660) + } + } +} + +fn set_mode(path: &Path, mode: u32) -> Result<()> { use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)).map_err(|source| { + std::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)).map_err(|source| { Error::Io { path: path.to_path_buf(), source, @@ -170,7 +213,9 @@ mod tests { let source: Arc = Arc::new(|| -> BoxFuture<'static, StatusReport> { Box::pin(async { StatusReport::default() }) }); - let control = ControlSocket::bind(&path, source).await.unwrap(); + let control = ControlSocket::bind(&path, source, ControlSocketAccess::Private) + .await + .unwrap(); let answer = exchange(&path, &Request::Status, EXCHANGE_TIMEOUT) .await diff --git a/crates/tsunagi/src/ipc/windows.rs b/crates/tsunagi/src/ipc/windows.rs index d7a5dfb..da842fa 100644 --- a/crates/tsunagi/src/ipc/windows.rs +++ b/crates/tsunagi/src/ipc/windows.rs @@ -40,8 +40,8 @@ use tokio::task::JoinHandle; use crate::error::{Error, Result}; pub use super::{ - CONTROL_PROTOCOL, EXCHANGE_TIMEOUT, ReportSource, is_serving, join_network, leave_network, - request_status, set_active, set_dns, set_hostname, + CONTROL_PROTOCOL, ControlSocketAccess, EXCHANGE_TIMEOUT, ReportSource, is_serving, + join_network, leave_network, request_status, set_active, set_dns, set_hostname, }; /// `ERROR_ACCESS_DENIED`: creating a first instance finds an existing pipe, @@ -149,7 +149,16 @@ impl ControlSocket { /// Claiming the first instance of the name is what detects a second agent: /// if the name already exists, the create is refused and that is reported /// as the state being locked, so two live agents never share one pipe. - pub async fn bind(path: impl AsRef, source: Arc) -> Result { + /// + /// `_access` is accepted for parity with the Unix socket; granting a group + /// here means a pipe security descriptor and is a separate follow-up, so for + /// now the pipe keeps its default (creator plus the built-in access the + /// `PipeSecurityAttributes` set). + pub async fn bind( + path: impl AsRef, + source: Arc, + _access: ControlSocketAccess, + ) -> Result { let path = path.as_ref().to_path_buf(); let name = pipe_name(&path);