diff --git a/Cargo.lock b/Cargo.lock index 92b54ed89c..d31e2d1a7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4763,6 +4763,7 @@ dependencies = [ "rustls", "serde", "serde_json", + "tempfile", "thiserror 2.0.20", "tokio", "tokio-stream", @@ -4775,6 +4776,9 @@ name = "openshell-gateway" version = "0.0.0" dependencies = [ "async-trait", + "http 1.4.0", + "http-body-util", + "hyper", "hyper-util", "miette", "nix 0.29.0", diff --git a/crates/openshell-core/src/external_driver_socket.rs b/crates/openshell-core/src/external_driver_socket.rs index e6e1978f83..50993ff83b 100644 --- a/crates/openshell-core/src/external_driver_socket.rs +++ b/crates/openshell-core/src/external_driver_socket.rs @@ -14,6 +14,7 @@ use tokio_stream::Stream; /// Prepare and bind a private Unix socket owned by the current effective UID. pub fn bind_private(path: &Path) -> Result { + crate::unix_socket_path::check(path).map_err(|err| err.to_string())?; let parent = path .parent() .ok_or_else(|| format!("driver socket path '{}' has no parent", path.display()))?; @@ -173,6 +174,20 @@ mod tests { assert!(err.contains("must be a directory, not a symlink")); } + #[test] + fn bind_private_rejects_overlong_path_before_creating_parent() { + let temp_dir = tempfile::tempdir().expect("create temporary directory"); + let parent = temp_dir + .path() + .join("a".repeat(crate::unix_socket_path::MAX_LEN)); + + let err = bind_private(&parent.join("compute.sock")) + .expect_err("overlong socket path must be rejected"); + + assert!(err.contains("-byte limit"), "{err}"); + assert!(!parent.exists()); + } + #[test] fn bind_private_rejects_existing_non_socket() { let temp_dir = tempfile::tempdir().expect("create temporary directory"); diff --git a/crates/openshell-core/src/lib.rs b/crates/openshell-core/src/lib.rs index f16eb7c130..973d4d65cf 100644 --- a/crates/openshell-core/src/lib.rs +++ b/crates/openshell-core/src/lib.rs @@ -69,6 +69,7 @@ pub mod time; pub mod token_grant_failure; pub mod transport_errors; pub mod upload; +pub use openshell_extension_core::unix_socket_path; pub use config::{ AppArmorProfile, Config, ExtensionBindingPolicy, GatewayAuthConfig, diff --git a/crates/openshell-driver-kubernetes-secrets/src/main.rs b/crates/openshell-driver-kubernetes-secrets/src/main.rs index 2c7f13293f..007a3e89f1 100644 --- a/crates/openshell-driver-kubernetes-secrets/src/main.rs +++ b/crates/openshell-driver-kubernetes-secrets/src/main.rs @@ -77,6 +77,7 @@ fn driver_config(args: &Args) -> toml::Table { } fn prepare_socket(socket_path: &Path) -> Result<()> { + openshell_core::unix_socket_path::check(socket_path).into_diagnostic()?; let parent = socket_path.parent().ok_or_else(|| { miette!( "credential driver socket path '{}' has no parent directory", diff --git a/crates/openshell-driver-vault/src/main.rs b/crates/openshell-driver-vault/src/main.rs index c2a4da6eed..ca2d65a177 100644 --- a/crates/openshell-driver-vault/src/main.rs +++ b/crates/openshell-driver-vault/src/main.rs @@ -129,6 +129,7 @@ fn insert_path(config: &mut toml::Table, key: &str, value: Option<&PathBuf>) { } fn prepare_socket(socket_path: &Path) -> Result<()> { + openshell_core::unix_socket_path::check(socket_path).into_diagnostic()?; let parent = socket_path.parent().ok_or_else(|| { miette!( "credential driver socket path '{}' has no parent directory", diff --git a/crates/openshell-driver-vm/README.md b/crates/openshell-driver-vm/README.md index a0371bacfb..6b0753f8af 100644 --- a/crates/openshell-driver-vm/README.md +++ b/crates/openshell-driver-vm/README.md @@ -68,7 +68,7 @@ By default `mise run gateway:vm`: `~/.config/openshell/gateways/vm-dev/metadata.json`. It does not modify the workspace `.env`. - Persists the gateway SQLite DB under `.cache/gateway-vm/gateway.db`. -- Places the VM driver state (per-sandbox `overlay.ext4`, image cache, and `run/compute-driver.sock`) under `/tmp/openshell-vm-driver-$USER-vm-dev/` so the AF_UNIX socket path stays under macOS `SUN_LEN`. +- Places the VM driver state (per-sandbox `overlay.ext4` and image cache) under `/tmp/openshell-vm-driver-$USER-vm-dev/`. - Writes `.cache/gateway-vm/gateway.toml` with `[openshell.drivers.vm].driver_dir = "$PWD/target/debug"` so the freshly built `openshell-driver-vm` is used instead of an older installed copy from `~/.local/libexec/openshell`, `/usr/libexec/openshell`, or `/usr/local/libexec`. - Enables OTLP trace export, OCSF log export, and the agent trace relay to `http://127.0.0.1:4317` only when a local collector is listening there. Otherwise, it omits the OTLP configuration to avoid repeated export failures. @@ -166,7 +166,7 @@ Select the VM driver with `--compute-driver vm`, `OPENSHELL_COMPUTE_DRIVER=vm`, | Configuration key | Default | Purpose | |---|---|---| | `grpc_endpoint` | empty | Required. URL the native host supervisor uses to reach the gateway. Host loopback such as `http://127.0.0.1:` is valid. Legacy guest aliases are normalized to host loopback. This endpoint is never sent into the VM. | -| `state_dir` | `target/openshell-vm-driver` | Per-sandbox overlay disks, console logs, image cache, and private `run/compute-driver.sock` UDS. Relative paths are resolved to absolute paths at driver startup. | +| `state_dir` | `target/openshell-vm-driver` | Per-sandbox overlay disks, console logs, and image cache. Relative paths are resolved to absolute paths at driver startup. | | `driver_dir` | unset | Override the directory searched for `openshell-driver-vm`. | | `default_image` | OpenShell base image | Sandbox image used when a create request omits one. | | `bootstrap_image` | unset | VM runtime image used as the immutable bootstrap root disk. Falls back only to the operator-configured `default_image`; the driver refuses to start when both are empty. | @@ -283,8 +283,8 @@ RUST_LOG=openshell_server=debug,openshell_driver_vm=debug \ mise run gateway:vm ``` -The VM guest's serial console is appended to `//console.log`. Sandbox IDs must match `[A-Za-z0-9._-]{1,128}` before the driver uses them in host paths. The gateway-owned compute-driver socket lives at `/run/compute-driver.sock`; OpenShell creates `run/` with owner-only permissions and removes same-owner stale sockets. On clean shutdown, the gateway sends the managed driver `SIGTERM`, waits up to five seconds for it to flush telemetry and exit, then force-kills it if necessary and removes the socket. UDS clients must match the driver UID and provide the expected gateway process PID by default. Standalone same-UID UDS mode requires the explicit `--allow-same-uid-peer` development flag. TCP mode is disabled by default because it is unauthenticated; use `--allow-unauthenticated-tcp --bind-address 127.0.0.1:50061` only for local development. -The VM serial console is appended to `/sandboxes//rootfs-console.log`. Host-supervisor stdout and stderr are written beside it as `supervisor.log` and `supervisor.err.log`. Sandbox IDs must match `[A-Za-z0-9._-]{1,128}` before the driver uses them in host paths. The gateway-owned compute-driver socket lives at `/run/compute-driver.sock`; OpenShell creates `run/` with owner-only permissions, removes same-owner stale sockets, and the gateway removes the socket on clean shutdown via `ManagedDriverProcess::drop`. UDS clients must match the driver UID and provide the expected gateway process PID by default. Standalone same-UID UDS mode requires the explicit `--allow-same-uid-peer` development flag. TCP mode is disabled by default because it is unauthenticated; use `--allow-unauthenticated-tcp --bind-address 127.0.0.1:50061` only for local development. +The VM guest's serial console is appended to `//console.log`. Sandbox IDs must match `[A-Za-z0-9._-]{1,128}` before the driver uses them in host paths. The gateway-owned compute-driver socket lives in a private, randomly named `/tmp/os-gw--/` directory, independent of `state_dir`. On clean shutdown, the gateway sends the managed driver `SIGTERM`, waits up to five seconds for it to flush telemetry and exit, then force-kills it if necessary and removes the socket directory. UDS clients must match the driver UID and provide the expected gateway process PID by default. Standalone same-UID UDS mode requires the explicit `--allow-same-uid-peer` development flag. TCP mode is disabled by default because it is unauthenticated; use `--allow-unauthenticated-tcp --bind-address 127.0.0.1:50061` only for local development. +The VM serial console is appended to `/sandboxes//rootfs-console.log`. Host-supervisor stdout and stderr are written beside it as `supervisor.log` and `supervisor.err.log`. Sandbox IDs must match `[A-Za-z0-9._-]{1,128}` before the driver uses them in host paths. The gateway-owned compute-driver socket lives in a private, randomly named `/tmp/os-gw--/` directory, which `ManagedDriverProcess::drop` removes. UDS clients must match the driver UID and provide the expected gateway process PID by default. Standalone same-UID UDS mode requires the explicit `--allow-same-uid-peer` development flag. TCP mode is disabled by default because it is unauthenticated; use `--allow-unauthenticated-tcp --bind-address 127.0.0.1:50061` only for local development. ## Network isolation diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 6260a52842..7eb5f94ff7 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -1472,7 +1472,9 @@ impl VmDriver { .env(openshell_core::sandbox_env::SANDBOX, &sandbox.name) .env( openshell_core::sandbox_env::SSH_SOCKET_PATH, - sandbox_socket_dir(&self.socket_root, &sandbox.id).join("ssh.sock"), + sandbox_socket_dir(&self.socket_root, &sandbox.id) + .map_err(|err| Status::internal(format!("sandbox socket dir: {err}")))? + .join("ssh.sock"), ) .env( openshell_core::sandbox_env::PROXY_TLS_DIR, @@ -2082,10 +2084,10 @@ impl VmDriver { } let console_output = state_dir.join("rootfs-console.log"); - create_sandbox_socket_dir(&self.socket_root_fd, &self.socket_root, &sandbox.id) - .map_err(|err| Status::internal(format!("create socket dir failed: {err}")))?; let control_socket = - sandbox_socket_dir(&self.socket_root, &sandbox.id).join(VM_CONTROL_SOCKET); + create_sandbox_socket_dir(&self.socket_root_fd, &self.socket_root, &sandbox.id) + .map_err(|err| Status::internal(format!("create socket dir failed: {err}")))? + .join(VM_CONTROL_SOCKET); let session_id = launch_authentication.supervisor.session_id; let channel_tls = generate_sandbox_tls_material(session_id) .map_err(|error| Status::internal(error.to_string()))?; @@ -6811,11 +6813,20 @@ async fn restrict_owner_only_dir(_path: &Path) -> Result<(), std::io::Error> { } const SOCKET_ROOT_ALLOC_RETRIES: usize = 16; -/// macOS `sun_path` is 104 bytes including the NUL terminator. -const MAX_SUN_PATH_LEN: usize = 103; -fn sandbox_socket_dir(socket_root: &Path, sandbox_id: &str) -> PathBuf { - socket_root.join(sandbox_id) +/// Name of a sandbox's socket directory under the socket root. Sandbox IDs can +/// be 128 bytes, so a fixed-length digest keeps socket paths short. +fn sandbox_socket_leaf(sandbox_id: &str) -> Result { + let digest = openshell_crypto::sha256(sandbox_id.as_bytes()).map_err(std::io::Error::other)?; + let mut leaf = String::with_capacity(32); + for byte in &digest[..16] { + write!(&mut leaf, "{byte:02x}").expect("writing to String cannot fail"); + } + Ok(leaf) +} + +fn sandbox_socket_dir(socket_root: &Path, sandbox_id: &str) -> Result { + Ok(socket_root.join(sandbox_socket_leaf(sandbox_id)?)) } fn allocate_socket_root() -> Result<(PathBuf, OwnedFd), std::io::Error> { @@ -6864,34 +6875,34 @@ fn create_sandbox_socket_dir( socket_root: &Path, sandbox_id: &str, ) -> Result { - let longest = socket_root.join(sandbox_id).join(VM_CONTROL_SOCKET); - if longest.as_os_str().len() > MAX_SUN_PATH_LEN { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - format!( - "socket path {} exceeds sun_path limit ({MAX_SUN_PATH_LEN} bytes)", - longest.display() - ), - )); - } - match rustix::fs::mkdirat(root_fd, sandbox_id, rustix::fs::Mode::from_raw_mode(0o700)) { + let leaf = sandbox_socket_leaf(sandbox_id)?; + let dir = socket_root.join(&leaf); + openshell_core::unix_socket_path::check(&dir.join(VM_CONTROL_SOCKET))?; + match rustix::fs::mkdirat( + root_fd, + leaf.as_str(), + rustix::fs::Mode::from_raw_mode(0o700), + ) { Ok(()) | Err(rustix::io::Errno::EXIST) => {} Err(e) => return Err(e.into()), } - Ok(socket_root.join(sandbox_id)) + Ok(dir) } fn remove_sandbox_socket_dir(root_fd: &OwnedFd, sandbox_id: &str) { + let Ok(leaf) = sandbox_socket_leaf(sandbox_id) else { + return; + }; if let Ok(leaf_fd) = rustix::fs::openat( root_fd, - sandbox_id, + leaf.as_str(), rustix::fs::OFlags::RDONLY | rustix::fs::OFlags::DIRECTORY | rustix::fs::OFlags::NOFOLLOW, rustix::fs::Mode::empty(), ) { let _ = rustix::fs::unlinkat(&leaf_fd, VM_CONTROL_SOCKET, rustix::fs::AtFlags::empty()); let _ = rustix::fs::unlinkat(&leaf_fd, "ssh.sock", rustix::fs::AtFlags::empty()); } - let _ = rustix::fs::unlinkat(root_fd, sandbox_id, rustix::fs::AtFlags::REMOVEDIR); + let _ = rustix::fs::unlinkat(root_fd, leaf.as_str(), rustix::fs::AtFlags::REMOVEDIR); } #[allow(clippy::result_large_err)] @@ -8341,7 +8352,8 @@ mod tests { use tonic::Code; fn test_socket_root() -> (PathBuf, Arc) { - let dir = std::env::temp_dir().join(format!( + // Production socket roots live under /tmp; macOS TMPDIR is too long. + let dir = Path::new("/tmp").join(format!( "os-test-{:016x}", u64::from_ne_bytes(openshell_crypto::random_bytes::<8>().expect("test entropy")) )); @@ -10572,20 +10584,26 @@ mod tests { } #[test] - fn create_sandbox_socket_dir_rejects_over_long_ids() { + fn create_sandbox_socket_dir_accepts_longest_sandbox_id() { let (root, fd) = test_socket_root(); - let err = create_sandbox_socket_dir(&fd, &root, &"x".repeat(128)).unwrap_err(); - assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); - assert!(!root.join("x".repeat(128)).exists()); + let sandbox_id = "x".repeat(128); + let dir = create_sandbox_socket_dir(&fd, &root, &sandbox_id).unwrap(); + assert!(dir.is_dir()); + assert_eq!(dir, sandbox_socket_dir(&root, &sandbox_id).unwrap()); + std::os::unix::net::UnixListener::bind(dir.join(VM_CONTROL_SOCKET)).unwrap(); + remove_sandbox_socket_dir(&fd, &sandbox_id); + assert!(!dir.exists()); let _ = std::fs::remove_dir_all(root); } #[test] fn sandbox_socket_dir_fits_macos_sun_path() { - let sandbox_id = "3eb2ad45-bead-4c2e-bd10-1a4a7f3a2721"; + let sandbox_id = "x".repeat(128); let worst_root = "/tmp/os-4294967295-00000000000000000000000000000000"; - let control = sandbox_socket_dir(Path::new(worst_root), sandbox_id).join(VM_CONTROL_SOCKET); - let ssh = sandbox_socket_dir(Path::new(worst_root), sandbox_id).join("ssh.sock"); + let dir = sandbox_socket_dir(Path::new(worst_root), &sandbox_id).unwrap(); + let control = dir.join(VM_CONTROL_SOCKET); + let ssh = dir.join("ssh.sock"); + assert_ne!(dir, sandbox_socket_dir(Path::new(worst_root), "y").unwrap()); assert!( control.as_os_str().len() < 104, "control socket path exceeds macOS sun_path: {}", diff --git a/crates/openshell-driver-vm/src/main.rs b/crates/openshell-driver-vm/src/main.rs index 0ab4743771..634339a8c8 100644 --- a/crates/openshell-driver-vm/src/main.rs +++ b/crates/openshell-driver-vm/src/main.rs @@ -455,6 +455,7 @@ fn compute_driver_listen_mode(args: &Args) -> std::result::Result Result<(), TransportError if !matches!(&config.server_trust, ExtensionServerTrust::PlatformRoots) && !is_https { return Err(TransportError::CustomCaRequiresHttps); } - if let Some(path) = config.endpoint.strip_prefix("unix://") - && (path.is_empty() || !PathBuf::from(path).is_absolute()) - { - return Err(TransportError::InvalidUnixPath); + if let Some(path) = config.endpoint.strip_prefix("unix://") { + if path.is_empty() || !PathBuf::from(path).is_absolute() { + return Err(TransportError::InvalidUnixPath); + } + crate::unix_socket_path::check(path.as_ref()).map_err(TransportError::UnixPathTooLong)?; } Ok(()) } @@ -271,7 +274,7 @@ mod tests { } #[test] - fn rejects_unsupported_schemes_and_relative_unix_paths() { + fn rejects_unsupported_schemes_and_invalid_unix_paths() { assert!(matches!( validate_config(&ExtensionChannelConfig::new("tcp://middleware:50051")), Err(TransportError::UnsupportedScheme) @@ -280,6 +283,11 @@ mod tests { validate_config(&ExtensionChannelConfig::new("unix://relative.sock")), Err(TransportError::InvalidUnixPath) )); + let long = format!("unix:///{}", "a".repeat(crate::unix_socket_path::MAX_LEN)); + assert!(matches!( + validate_config(&ExtensionChannelConfig::new(long)), + Err(TransportError::UnixPathTooLong(_)) + )); } #[test] diff --git a/crates/openshell-extension-core/src/unix_socket_path.rs b/crates/openshell-extension-core/src/unix_socket_path.rs new file mode 100644 index 0000000000..2bdb900ec9 --- /dev/null +++ b/crates/openshell-extension-core/src/unix_socket_path.rs @@ -0,0 +1,72 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Unix domain socket pathname limit. + +use std::io; +use std::path::Path; + +/// Longest Unix socket pathname, in bytes, without the terminating NUL. +/// +/// `sockaddr_un.sun_path` holds 108 bytes on Linux and 104 on macOS. +/// Abstract socket addresses have different limits and are not pathnames. +pub const MAX_LEN: usize = if cfg!(target_os = "linux") { 107 } else { 103 }; + +/// Reject a socket pathname that does not fit in `sockaddr_un.sun_path`. +/// +/// Call it before creating directories, removing a stale socket or retrying a +/// connection. The error names the path and its length. The kernel error says +/// only that the path must be shorter than `SUN_LEN`. +pub fn check(path: &Path) -> io::Result<()> { + let len = path.as_os_str().len(); + if len <= MAX_LEN { + return Ok(()); + } + Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "Unix socket path '{}' is {len} bytes, over the {MAX_LEN}-byte limit on this platform", + path.display() + ), + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn accepts_the_limit_and_rejects_one_byte_more() { + let at_limit = format!("/{}", "a".repeat(MAX_LEN - 1)); + check(Path::new(&at_limit)).unwrap(); + + let over = format!("{at_limit}a"); + let error = check(Path::new(&over)).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + assert!(error.to_string().contains(&over), "{error}"); + assert!( + error.to_string().contains(&format!( + "{} bytes, over the {MAX_LEN}-byte limit", + MAX_LEN + 1 + )), + "{error}" + ); + } + + #[cfg(unix)] + #[test] + fn limit_matches_what_the_platform_binds() { + // macOS TMPDIR is too long to leave room for the probe name. + let dir = tempfile::Builder::new() + .prefix("os-sock-len-") + .tempdir_in("/tmp") + .unwrap(); + let prefix = dir.path().join("s"); + let fill = MAX_LEN - prefix.as_os_str().len(); + let at_limit = format!("{}{}", prefix.display(), "a".repeat(fill)); + let bound = std::os::unix::net::UnixListener::bind(&at_limit); + let over = std::os::unix::net::UnixListener::bind(format!("{at_limit}b")); + bound.unwrap(); + over.unwrap_err(); + } +} diff --git a/crates/openshell-gateway/Cargo.toml b/crates/openshell-gateway/Cargo.toml index 86249568ba..006ef1c5c9 100644 --- a/crates/openshell-gateway/Cargo.toml +++ b/crates/openshell-gateway/Cargo.toml @@ -28,10 +28,12 @@ openshell-driver-docker = { path = "../openshell-driver-docker", optional = true openshell-driver-kubernetes = { path = "../openshell-driver-kubernetes", optional = true } openshell-driver-podman = { path = "../openshell-driver-podman", optional = true } openshell-policy = { path = "../openshell-policy", optional = true } +openshell-crypto = { workspace = true, optional = true } hyper-util = { workspace = true, optional = true } nix = { workspace = true, optional = true } serde = { workspace = true, optional = true } rustix = { workspace = true, optional = true } +tempfile = { version = "3", optional = true } tonic = { workspace = true, optional = true } tower = { workspace = true, optional = true } tracing = { workspace = true, optional = true } @@ -59,6 +61,8 @@ compute-driver-vm = [ "dep:nix", "dep:serde", "dep:rustix", + "dep:openshell-crypto", + "dep:tempfile", "dep:tonic", "dep:tower", "dep:tracing", @@ -79,6 +83,9 @@ vendored-z3 = ["openshell-server/vendored-z3"] workspace = true [dev-dependencies] +http = { workspace = true } +http-body-util = { workspace = true } +hyper = { workspace = true } openshell-crypto = { workspace = true, default-features = false, features = ["aws-lc"] } rcgen = { workspace = true } tempfile = "3" diff --git a/crates/openshell-gateway/src/vm.rs b/crates/openshell-gateway/src/vm.rs index 2f9ea5cc8e..0b3cab38ca 100644 --- a/crates/openshell-gateway/src/vm.rs +++ b/crates/openshell-gateway/src/vm.rs @@ -39,7 +39,7 @@ use openshell_server::AcquiredRemoteDriverEndpoint; use openshell_server::ManagedDriverProcess; use openshell_server::config_file::OtlpConfig; #[cfg(unix)] -use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt}; +use std::os::unix::fs::{MetadataExt, PermissionsExt}; #[cfg(unix)] use std::path::Path; use std::path::PathBuf; @@ -56,8 +56,9 @@ use tonic::transport::Endpoint; use tower::service_fn; const DRIVER_BIN_NAME: &str = "openshell-driver-vm"; -const COMPUTE_DRIVER_SOCKET_RUN_DIR: &str = "run"; const COMPUTE_DRIVER_SOCKET_NAME: &str = "compute-driver.sock"; +#[cfg(unix)] +const SOCKET_DIR_ALLOC_RETRIES: usize = 16; /// Configuration for launching and talking to the VM compute driver. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] @@ -344,29 +345,44 @@ fn push_unique_path(paths: &mut Vec, path: PathBuf) { } } -/// Path of the Unix domain socket the driver will listen on. -pub fn compute_driver_socket_path(vm_config: &VmComputeConfig) -> PathBuf { - vm_config - .state_dir - .join(COMPUTE_DRIVER_SOCKET_RUN_DIR) - .join(COMPUTE_DRIVER_SOCKET_NAME) +#[cfg(unix)] +fn allocate_compute_driver_socket_dir() -> std::io::Result { + let uid = current_euid(); + // macOS TMPDIR can itself exceed the Unix socket path limit. Use the same + // short base and random private-directory approach as the VM driver. + allocate_compute_driver_socket_dir_with(Path::new("/tmp"), || { + let random = u128::from_ne_bytes( + openshell_crypto::random_bytes::<16>().map_err(std::io::Error::other)?, + ); + Ok(format!("os-gw-{uid}-{random:032x}")) + }) } #[cfg(unix)] -fn prepare_compute_driver_socket_path( - vm_config: &VmComputeConfig, - socket_path: &Path, -) -> Result<()> { - let expected_uid = current_euid(); - prepare_vm_state_dir(&vm_config.state_dir, expected_uid)?; - let parent = socket_path.parent().ok_or_else(|| { - Error::execution(format!( - "vm compute driver socket path '{}' has no parent directory", - socket_path.display() - )) - })?; - prepare_private_socket_dir(parent, expected_uid)?; - remove_stale_socket(socket_path, expected_uid) +fn allocate_compute_driver_socket_dir_with( + base: &Path, + mut gen_name: impl FnMut() -> std::io::Result, +) -> std::io::Result { + for _ in 0..SOCKET_DIR_ALLOC_RETRIES { + match tempfile::Builder::new() + .prefix(&gen_name()?) + .rand_bytes(0) + .permissions(std::fs::Permissions::from_mode(0o700)) + .tempdir_in(base) + { + Ok(dir) => return Ok(dir), + // Never inspect, reuse, or modify an occupied candidate. + Err(err) if err.kind() == ErrorKind::AlreadyExists => {} + Err(err) => return Err(err), + } + } + Err(std::io::Error::new( + ErrorKind::AlreadyExists, + format!( + "failed to allocate vm compute driver socket directory under {} after {SOCKET_DIR_ALLOC_RETRIES} attempts", + base.display() + ), + )) } #[cfg(unix)] @@ -397,23 +413,6 @@ fn prepare_vm_state_dir(state_dir: &Path, expected_uid: u32) -> Result<()> { Ok(()) } -#[cfg(unix)] -fn prepare_private_socket_dir(socket_dir: &Path, expected_uid: u32) -> Result<()> { - std::fs::create_dir_all(socket_dir).map_err(|err| { - Error::execution(format!( - "failed to create vm compute driver socket dir '{}': {err}", - socket_dir.display() - )) - })?; - let _ = checked_directory_metadata(socket_dir, expected_uid, "vm compute driver socket dir")?; - std::fs::set_permissions(socket_dir, std::fs::Permissions::from_mode(0o700)).map_err(|err| { - Error::execution(format!( - "failed to restrict vm compute driver socket dir '{}': {err}", - socket_dir.display() - )) - }) -} - #[cfg(unix)] fn checked_directory_metadata( path: &Path, @@ -450,47 +449,6 @@ fn checked_directory_metadata( Ok(metadata) } -#[cfg(unix)] -fn remove_stale_socket(socket_path: &Path, expected_uid: u32) -> Result<()> { - let metadata = match std::fs::symlink_metadata(socket_path) { - Ok(metadata) => metadata, - Err(err) if err.kind() == ErrorKind::NotFound => return Ok(()), - Err(err) => { - return Err(Error::execution(format!( - "failed to stat vm compute driver socket '{}': {err}", - socket_path.display() - ))); - } - }; - let file_type = metadata.file_type(); - if file_type.is_symlink() { - return Err(Error::execution(format!( - "vm compute driver socket '{}' is a symlink; refusing to remove it", - socket_path.display() - ))); - } - if metadata.uid() != expected_uid { - return Err(Error::execution(format!( - "vm compute driver socket '{}' is owned by uid {} but current euid is {}", - socket_path.display(), - metadata.uid(), - expected_uid - ))); - } - if !file_type.is_socket() { - return Err(Error::execution(format!( - "vm compute driver socket path '{}' exists but is not a Unix socket", - socket_path.display() - ))); - } - std::fs::remove_file(socket_path).map_err(|err| { - Error::execution(format!( - "failed to remove stale vm compute driver socket '{}': {err}", - socket_path.display() - )) - }) -} - #[cfg(unix)] pub fn compute_driver_guest_tls_paths( vm_config: &VmComputeConfig, @@ -535,9 +493,14 @@ pub async fn spawn( ) -> Result { vm_config.validate_configuration()?; let driver_bin = resolve_compute_driver_bin(vm_config)?; - let socket_path = compute_driver_socket_path(vm_config); let guest_tls_paths = compute_driver_guest_tls_paths(vm_config)?; - prepare_compute_driver_socket_path(vm_config, &socket_path)?; + prepare_vm_state_dir(&vm_config.state_dir, current_euid())?; + let socket_dir = allocate_compute_driver_socket_dir().map_err(|err| { + Error::execution(format!( + "failed to allocate vm compute driver socket directory: {err}" + )) + })?; + let socket_path = socket_dir.path().join(COMPUTE_DRIVER_SOCKET_NAME); let mut command = Command::new(&driver_bin); command.kill_on_drop(true); @@ -593,7 +556,8 @@ pub async fn spawn( )) })?; let channel = wait_for_compute_driver(&socket_path, &mut child).await?; - let process = Arc::new(ManagedDriverProcess::new(child, socket_path)); + let process = + Arc::new(ManagedDriverProcess::new(child, socket_path).with_socket_dir(socket_dir)); Ok(AcquiredRemoteDriverEndpoint::managed( "vm", channel, process, )) @@ -796,10 +760,11 @@ async fn connect_compute_driver(socket_path: &Path) -> Result { #[cfg(all(test, unix))] mod tests { use super::{ - VmComputeConfig, append_otlp_args, append_vm_identity_args, append_vm_network_trust_args, + COMPUTE_DRIVER_SOCKET_NAME, SOCKET_DIR_ALLOC_RETRIES, VmComputeConfig, + allocate_compute_driver_socket_dir, allocate_compute_driver_socket_dir_with, + append_otlp_args, append_vm_identity_args, append_vm_network_trust_args, append_vm_proxy_and_spiffe_args, append_vm_rootfs_tar_args, compute_driver_guest_tls_paths, - compute_driver_socket_path, current_euid, prepare_compute_driver_socket_path, - prepare_vm_state_dir, resolve_compute_driver_bin, resolve_driver_search_dirs, + current_euid, prepare_vm_state_dir, resolve_compute_driver_bin, resolve_driver_search_dirs, validate_vm_sandbox_identity, }; use openshell_core::{NetworkSupervisorTrustBundle, UpstreamProxyConfig}; @@ -1251,116 +1216,99 @@ mod tests { } #[test] - fn compute_driver_socket_path_uses_private_run_dir() { - let state_dir = PathBuf::from("/tmp/openshell-vm-state"); - let vm_config = VmComputeConfig { - state_dir: state_dir.clone(), - ..Default::default() - }; - + fn compute_driver_socket_dir_is_private_unique_and_cleans_up() { + let first = allocate_compute_driver_socket_dir().unwrap(); + let second = allocate_compute_driver_socket_dir().unwrap(); + assert_ne!(first.path(), second.path()); + assert_eq!(first.path().parent(), Some(std::path::Path::new("/tmp"))); + let metadata = std::fs::metadata(first.path()).unwrap(); + assert_eq!(metadata.permissions().mode() & 0o777, 0o700); assert_eq!( - compute_driver_socket_path(&vm_config), - state_dir.join("run").join("compute-driver.sock") + std::os::unix::fs::MetadataExt::uid(&metadata), + current_euid() ); + let socket_path = first.path().join(COMPUTE_DRIVER_SOCKET_NAME); + let listener = StdUnixListener::bind(&socket_path).unwrap(); + let directory = first.path().to_path_buf(); + drop(listener); + drop(first); + assert!(!directory.exists()); + assert!(second.path().exists()); } #[test] - fn prepare_compute_driver_socket_path_creates_private_run_dir() { - let dir = tempdir().unwrap(); - let vm_config = VmComputeConfig { - state_dir: dir.path().join("state"), - ..Default::default() - }; - let socket_path = compute_driver_socket_path(&vm_config); - - prepare_compute_driver_socket_path(&vm_config, &socket_path).unwrap(); - - let mode = std::fs::metadata(vm_config.state_dir.join("run")) - .unwrap() - .permissions() - .mode() - & 0o777; - assert_eq!(mode, 0o700); + fn compute_driver_socket_dir_retries_without_touching_occupied_entries() { + let base = tempdir().unwrap(); + let victim = base.path().join("victim"); + std::fs::create_dir(&victim).unwrap(); + std::fs::set_permissions(&victim, std::fs::Permissions::from_mode(0o755)).unwrap(); + std::os::unix::fs::symlink(&victim, base.path().join("attempt-0")).unwrap(); + std::fs::create_dir(base.path().join("attempt-1")).unwrap(); + std::fs::write(base.path().join("attempt-2"), "leave this file alone").unwrap(); + let mut attempts = 0; + let allocated = allocate_compute_driver_socket_dir_with(base.path(), || { + let name = format!("attempt-{attempts}"); + attempts += 1; + Ok(name) + }) + .unwrap(); + assert_eq!(attempts, 4); + assert_eq!(allocated.path(), base.path().join("attempt-3")); + drop(allocated); + assert!(base.path().join("attempt-0").is_symlink()); + assert!(base.path().join("attempt-1").is_dir()); + assert_eq!( + std::fs::read_to_string(base.path().join("attempt-2")).unwrap(), + "leave this file alone" + ); + assert_eq!( + std::fs::metadata(&victim).unwrap().permissions().mode() & 0o777, + 0o755 + ); } #[test] - fn prepare_compute_driver_socket_path_restricts_existing_run_dir() { - let dir = tempdir().unwrap(); - let vm_config = VmComputeConfig { - state_dir: dir.path().join("state"), - ..Default::default() - }; - let run_dir = vm_config.state_dir.join("run"); - std::fs::create_dir_all(&run_dir).unwrap(); - std::fs::set_permissions(&run_dir, std::fs::Permissions::from_mode(0o777)).unwrap(); - let socket_path = compute_driver_socket_path(&vm_config); - - prepare_compute_driver_socket_path(&vm_config, &socket_path).unwrap(); - - let mode = std::fs::metadata(run_dir).unwrap().permissions().mode() & 0o777; - assert_eq!(mode, 0o700); + fn compute_driver_socket_dir_stops_after_collision_limit() { + let base = tempdir().unwrap(); + std::fs::create_dir(base.path().join("occupied")).unwrap(); + let mut attempts = 0; + let error = allocate_compute_driver_socket_dir_with(base.path(), || { + attempts += 1; + Ok("occupied".to_owned()) + }) + .unwrap_err(); + assert_eq!(error.kind(), std::io::ErrorKind::AlreadyExists); + assert_eq!(attempts, SOCKET_DIR_ALLOC_RETRIES); + assert!(base.path().join("occupied").is_dir()); } #[test] - fn prepare_compute_driver_socket_path_restricts_existing_state_dir() { + fn prepare_vm_state_dir_restricts_existing_state_dir() { let dir = tempdir().unwrap(); - let vm_config = VmComputeConfig { - state_dir: dir.path().join("state"), - ..Default::default() - }; - std::fs::create_dir_all(&vm_config.state_dir).unwrap(); - std::fs::set_permissions(&vm_config.state_dir, std::fs::Permissions::from_mode(0o777)) - .unwrap(); - let socket_path = compute_driver_socket_path(&vm_config); + let state_dir = dir.path().join("state"); + std::fs::create_dir(&state_dir).unwrap(); + std::fs::set_permissions(&state_dir, std::fs::Permissions::from_mode(0o777)).unwrap(); - prepare_compute_driver_socket_path(&vm_config, &socket_path).unwrap(); + prepare_vm_state_dir(&state_dir, current_euid()).unwrap(); - let mode = std::fs::metadata(vm_config.state_dir) - .unwrap() - .permissions() - .mode() - & 0o777; + let mode = std::fs::metadata(state_dir).unwrap().permissions().mode() & 0o777; assert_eq!(mode, 0o700); } #[test] - fn prepare_compute_driver_socket_path_rejects_symlinked_state_dir() { + fn prepare_vm_state_dir_rejects_symlinked_state_dir() { let dir = tempdir().unwrap(); let target = dir.path().join("target"); let state_link = dir.path().join("state-link"); - std::fs::create_dir_all(&target).unwrap(); + std::fs::create_dir(&target).unwrap(); std::os::unix::fs::symlink(&target, &state_link).unwrap(); - let vm_config = VmComputeConfig { - state_dir: state_link, - ..Default::default() - }; - let socket_path = compute_driver_socket_path(&vm_config); - let err = prepare_compute_driver_socket_path(&vm_config, &socket_path) + let err = prepare_vm_state_dir(&state_link, current_euid()) .expect_err("symlinked state dir should be rejected") .to_string(); assert!(err.contains("is a symlink")); } - #[test] - fn prepare_compute_driver_socket_path_rejects_symlinked_run_dir() { - let dir = tempdir().unwrap(); - let vm_config = VmComputeConfig { - state_dir: dir.path().join("state"), - ..Default::default() - }; - let target = dir.path().join("run-target"); - std::fs::create_dir_all(&vm_config.state_dir).unwrap(); - std::fs::create_dir_all(&target).unwrap(); - std::os::unix::fs::symlink(&target, vm_config.state_dir.join("run")).unwrap(); - let socket_path = compute_driver_socket_path(&vm_config); - - let err = prepare_compute_driver_socket_path(&vm_config, &socket_path) - .expect_err("symlinked run dir should be rejected") - .to_string(); - assert!(err.contains("is a symlink")); - } - #[test] fn prepare_vm_state_dir_rejects_wrong_owner() { let dir = tempdir().unwrap(); @@ -1378,54 +1326,161 @@ mod tests { assert!(err.contains("is owned by uid")); } - #[test] - fn prepare_compute_driver_socket_path_rejects_symlinked_socket() { - let dir = tempdir().unwrap(); - let vm_config = VmComputeConfig { - state_dir: dir.path().join("state"), - ..Default::default() - }; - let socket_path = compute_driver_socket_path(&vm_config); - std::fs::create_dir_all(socket_path.parent().unwrap()).unwrap(); - std::os::unix::fs::symlink("/tmp/not-a-socket", &socket_path).unwrap(); + fn install_test_driver(driver_dir: &std::path::Path, command: &str) { + let driver = driver_dir.join(super::DRIVER_BIN_NAME); + // This executable fixture uses POSIX sh so it runs on Linux and macOS. + std::fs::write( + &driver, + format!( + r#"#!/bin/sh +while [ "$#" -gt 0 ]; do + case "$1" in + --bind-socket) socket="$2"; shift ;; + --state-dir) state="$2"; shift ;; + --expected-peer-pid) peer="$2"; shift ;; + esac + shift +done +printf '%s' "$socket" > "$state/socket-path" +printf '%s' "$peer" > "$state/peer-pid" +export OPENSHELL_TEST_SOCKET="$socket" +{command} +"# + ), + ) + .unwrap(); + std::fs::set_permissions(driver, std::fs::Permissions::from_mode(0o700)).unwrap(); + } - let err = prepare_compute_driver_socket_path(&vm_config, &socket_path) - .expect_err("symlinked socket should be rejected") - .to_string(); - assert!(err.contains("is a symlink")); + fn test_driver_config(root: &std::path::Path) -> VmComputeConfig { + VmComputeConfig { + driver_dir: Some(root.to_path_buf()), + state_dir: root.join("persistent-state".repeat(10)), + grpc_endpoint: "http://127.0.0.1:8443".to_owned(), + ..Default::default() + } } - #[test] - fn prepare_compute_driver_socket_path_rejects_non_socket_stale_path() { + #[tokio::test] + async fn spawn_accepts_overlong_state_dir_and_cleans_up_managed_socket() { let dir = tempdir().unwrap(); - let vm_config = VmComputeConfig { - state_dir: dir.path().join("state"), - ..Default::default() - }; - let socket_path = compute_driver_socket_path(&vm_config); - std::fs::create_dir_all(socket_path.parent().unwrap()).unwrap(); - std::fs::write(&socket_path, "not a socket").unwrap(); + let vm_config = test_driver_config(dir.path()); + assert!(vm_config.state_dir.as_os_str().len() > 108); + let test_binary = std::env::current_exe().unwrap(); + let quoted_binary = test_binary.to_string_lossy().replace('\'', "'\"'\"'"); + install_test_driver( + dir.path(), + &format!( + "exec '{quoted_binary}' --exact vm::tests::compute_driver_subprocess_fixture --ignored --nocapture" + ), + ); - let err = prepare_compute_driver_socket_path(&vm_config, &socket_path) - .expect_err("regular file should be rejected") - .to_string(); - assert!(err.contains("is not a Unix socket")); + let endpoint = super::spawn("info", "test", &vm_config, None, None) + .await + .unwrap(); + let socket = PathBuf::from( + std::fs::read_to_string(vm_config.state_dir.join("socket-path")).unwrap(), + ); + assert_eq!( + socket.parent().unwrap().parent(), + Some(std::path::Path::new("/tmp")) + ); + assert!(!socket.starts_with(&vm_config.state_dir)); + assert!(socket.exists()); + assert_eq!( + std::fs::read_to_string(vm_config.state_dir.join("peer-pid")).unwrap(), + std::process::id().to_string() + ); + std::fs::write(vm_config.state_dir.join("persistent"), "keep").unwrap(); + + drop(endpoint); + + assert!(!socket.parent().unwrap().exists()); + assert_eq!( + std::fs::read_to_string(vm_config.state_dir.join("persistent")).unwrap(), + "keep" + ); } - #[test] - fn prepare_compute_driver_socket_path_removes_same_owner_stale_socket() { + #[tokio::test] + async fn spawn_removes_socket_dir_when_driver_exits_before_ready() { let dir = tempdir().unwrap(); - let vm_config = VmComputeConfig { - state_dir: dir.path().join("state"), - ..Default::default() - }; - let socket_path = compute_driver_socket_path(&vm_config); - std::fs::create_dir_all(socket_path.parent().unwrap()).unwrap(); - let listener = StdUnixListener::bind(&socket_path).unwrap(); - - prepare_compute_driver_socket_path(&vm_config, &socket_path).unwrap(); + let vm_config = test_driver_config(dir.path()); + install_test_driver(dir.path(), "exit 23"); + let error = super::spawn("info", "test", &vm_config, None, None) + .await + .unwrap_err(); + assert!( + error.to_string().contains("exited before becoming ready"), + "{error}" + ); + let socket = PathBuf::from( + std::fs::read_to_string(vm_config.state_dir.join("socket-path")).unwrap(), + ); + assert!(!socket.parent().unwrap().exists()); + assert!(vm_config.state_dir.exists()); + } - drop(listener); - assert!(!socket_path.exists()); + #[tokio::test] + async fn spawn_removes_socket_dir_when_startup_is_cancelled() { + let dir = tempdir().unwrap(); + let vm_config = test_driver_config(dir.path()); + install_test_driver(dir.path(), "exec sleep 60"); + let config = vm_config.clone(); + let task = + tokio::spawn(async move { super::spawn("info", "test", &config, None, None).await }); + let socket_record = vm_config.state_dir.join("socket-path"); + let socket = tokio::time::timeout(std::time::Duration::from_secs(5), async { + loop { + if let Ok(path) = std::fs::read_to_string(&socket_record) + && !path.is_empty() + { + break PathBuf::from(path); + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + assert!(socket.parent().unwrap().exists()); + task.abort(); + assert!(task.await.unwrap_err().is_cancelled()); + assert!(!socket.parent().unwrap().exists()); + } + + // Re-executed by the startup test as the managed driver process. It serves + // an empty successful GetCapabilities response without booting a VM. + #[tokio::test] + #[ignore = "subprocess fixture for spawn_accepts_overlong_state_dir_and_cleans_up_managed_socket"] + async fn compute_driver_subprocess_fixture() { + use http_body_util::{BodyExt as _, Full}; + use hyper::body::Bytes; + use hyper_util::rt::{TokioExecutor, TokioIo}; + + let socket = std::env::var_os("OPENSHELL_TEST_SOCKET").unwrap(); + let listener = tokio::net::UnixListener::bind(&socket).unwrap(); + std::fs::set_permissions(&socket, std::fs::Permissions::from_mode(0o600)).unwrap(); + loop { + let (stream, _) = listener.accept().await.unwrap(); + tokio::spawn(async move { + let service = hyper::service::service_fn(|_request| async { + let body = + Full::new(Bytes::from_static(&[0, 0, 0, 0, 0])).with_trailers(async { + let mut trailers = http::HeaderMap::new(); + trailers.insert("grpc-status", http::HeaderValue::from_static("0")); + Some(Ok(trailers)) + }); + Ok::<_, std::convert::Infallible>( + http::Response::builder() + .header("content-type", "application/grpc") + .body(body) + .unwrap(), + ) + }); + let _ = hyper::server::conn::http2::Builder::new(TokioExecutor::new()) + .serve_connection(TokioIo::new(stream), service) + .await; + }); + } } } diff --git a/crates/openshell-sandbox-backend/src/runtime_initiated.rs b/crates/openshell-sandbox-backend/src/runtime_initiated.rs index 8599c06eb9..bad3780a2a 100644 --- a/crates/openshell-sandbox-backend/src/runtime_initiated.rs +++ b/crates/openshell-sandbox-backend/src/runtime_initiated.rs @@ -207,6 +207,7 @@ impl Listener { #[cfg(unix)] fn bind_unix(path: &std::path::Path) -> std::io::Result { use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt}; + openshell_core::unix_socket_path::check(path)?; // The driver owns the containing directory. Never replace a live socket, // another user's inode, or a non-socket (including symlinks). match std::fs::symlink_metadata(path) { diff --git a/crates/openshell-sandbox/src/boundary_server.rs b/crates/openshell-sandbox/src/boundary_server.rs index 4eb3f1ec02..bbac980a59 100644 --- a/crates/openshell-sandbox/src/boundary_server.rs +++ b/crates/openshell-sandbox/src/boundary_server.rs @@ -4771,6 +4771,7 @@ mod linux { }) } BoundaryListenerConfig::Unix { socket_path, tls } => { + openshell_core::unix_socket_path::check(socket_path)?; remove_owned_stale_control_socket(socket_path)?; let listener = std::os::unix::net::UnixListener::bind(socket_path)?; // Mutual TLS makes a same-UID pathname replacement a diff --git a/crates/openshell-server/src/compute/driver_config.rs b/crates/openshell-server/src/compute/driver_config.rs index 418ae5b108..7ad0714c69 100644 --- a/crates/openshell-server/src/compute/driver_config.rs +++ b/crates/openshell-server/src/compute/driver_config.rs @@ -197,12 +197,13 @@ fn apply_remote_driver_overrides( } fn validate_remote_driver_config(cfg: &RemoteDriverConfig, name: &str) -> Result<()> { - if !cfg.socket_path.as_os_str().is_empty() { - return Ok(()); + if cfg.socket_path.as_os_str().is_empty() { + return Err(Error::config(format!( + "remote compute driver '{name}' requires socket_path" + ))); } - Err(Error::config(format!( - "remote compute driver '{name}' requires socket_path" - ))) + openshell_core::unix_socket_path::check(&cfg.socket_path) + .map_err(|err| Error::config(format!("remote compute driver '{name}' socket_path: {err}"))) } #[cfg(test)] @@ -510,4 +511,21 @@ socket_path = "/run/openshell/kyma.sock" .contains("remote compute driver 'kyma' requires socket_path") ); } + + #[test] + fn remote_driver_config_rejects_overlong_socket_path() { + let socket_path = PathBuf::from(format!( + "/{}/kyma.sock", + "a".repeat(openshell_core::unix_socket_path::MAX_LEN) + )); + let endpoint_overrides = BTreeMap::from([("kyma".to_string(), socket_path)]); + + let err = remote_driver_config_from_context( + test_context_with_endpoint_overrides(None, &endpoint_overrides), + "kyma", + ) + .unwrap_err(); + + assert!(err.to_string().contains("-byte limit"), "{err}"); + } } diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 2039ef4f7f..1d8239c975 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -559,6 +559,7 @@ pub use openshell_core::ComputeDriverError as ComputeError; pub struct ManagedDriverProcess { child: std::sync::Mutex>, socket_path: PathBuf, + socket_dir: Option, } impl ManagedDriverProcess { @@ -567,9 +568,18 @@ impl ManagedDriverProcess { Self { child: std::sync::Mutex::new(Some(child)), socket_path, + socket_dir: None, } } + /// Keep a private socket directory until the managed process is dropped. + #[cfg(unix)] + #[must_use] + pub fn with_socket_dir(mut self, socket_dir: tempfile::TempDir) -> Self { + self.socket_dir = Some(socket_dir); + self + } + #[cfg(unix)] async fn shutdown(&self) -> Result<(), String> { use nix::errno::Errno; diff --git a/crates/openshell-server/src/credentials.rs b/crates/openshell-server/src/credentials.rs index 17cfe96c28..5281c217a6 100644 --- a/crates/openshell-server/src/credentials.rs +++ b/crates/openshell-server/src/credentials.rs @@ -1059,6 +1059,11 @@ fn parse_driver_table( "[openshell.credential_drivers.{driver_name}] socket_path must be absolute" ))); } + openshell_core::unix_socket_path::check(socket_path).map_err(|err| { + Error::config(format!( + "[openshell.credential_drivers.{driver_name}] socket_path: {err}" + )) + })?; if let Some(command) = command.as_ref() && !command.is_absolute() { @@ -2572,6 +2577,25 @@ socket_path = "vault.sock" assert!(err.to_string().contains("socket_path must be absolute")); } + #[tokio::test] + async fn runtime_rejects_overlong_uds_socket_path() { + let config = Config::new(None).with_credential_drivers(["vault"]); + let file = config_file(&format!( + r#" +[openshell.credential_drivers.vault] +transport = "uds" +socket_path = "/{}/vault.sock" +"#, + "a".repeat(openshell_core::unix_socket_path::MAX_LEN) + )); + + let err = CredentialRuntime::from_config_file(&config, Some(&file)) + .await + .unwrap_err(); + + assert!(err.to_string().contains("-byte limit"), "{err}"); + } + #[tokio::test] async fn runtime_rejects_unknown_transport() { let config = Config::new(None).with_credential_drivers(["vault"]); diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index 5e2d6fc193..f31a617aae 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -123,6 +123,9 @@ fn ssh_server_init( // shared socket retains group access. Linux abstract sockets avoid a // workload-replaceable filesystem inode. let abstract_socket = crate::unix_socket::is_abstract(listen_path); + if !abstract_socket { + openshell_core::unix_socket_path::check(listen_path).into_diagnostic()?; + } if !abstract_socket && let Some(parent) = listen_path.parent() { std::fs::create_dir_all(parent).into_diagnostic()?; #[cfg(unix)] diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index 8b1035beca..d0c5174d06 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -299,6 +299,7 @@ fn prepare_control_readiness_path(path: &std::path::Path) -> Result<()> { "supervisor readiness socket path must be absolute" )); } + openshell_core::unix_socket_path::check(path).into_diagnostic()?; if let Some(parent) = path.parent() { std::fs::create_dir_all(parent) .into_diagnostic() diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index 3d5680301d..937c8cc496 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -659,7 +659,7 @@ gateway under the worktree-specific k3d cluster name; select it with tasks export spans and OCSF log records to the forwarded receiver automatically. -In-process compute drivers read their backend-specific settings from `[openshell.drivers.]`. An external driver's gateway table supplies only its `socket_path`; configure the external driver process itself through that binary's flags or environment variables. A driver that advertises `supports_sandbox_authentication` may authenticate an opaque bootstrap credential through the compute-driver protocol. It must return the same non-empty runtime identity from sandbox creation and credential authentication. Start requests include the previously recorded opaque identity so the driver can preserve the stable resource while rotating generation-specific compute. For `IssueSandboxToken`, the gateway verifies the sandbox exists and the driver/runtime identity matches the durable provisioning record before returning a generation-bound session JWT. The in-process Kubernetes driver reads `service_account_name`, `workspace_mode`, and namespace discovery from `[openshell.drivers.kubernetes]`; an external Kubernetes driver receives the equivalent values through its own CLI or environment contract. +In-process compute drivers read their backend-specific settings from `[openshell.drivers.]`. An external driver's gateway table supplies only its `socket_path`. The gateway rejects a Unix socket path longer than 107 bytes on Linux or 103 bytes on macOS. Configure the external driver process itself through that binary's flags or environment variables. A driver that advertises `supports_sandbox_authentication` may authenticate an opaque bootstrap credential through the compute-driver protocol. It must return the same non-empty runtime identity from sandbox creation and credential authentication. Start requests include the previously recorded opaque identity so the driver can preserve the stable resource while rotating generation-specific compute. For `IssueSandboxToken`, the gateway verifies the sandbox exists and the driver/runtime identity matches the durable provisioning record before returning a generation-bound session JWT. The in-process Kubernetes driver reads `service_account_name`, `workspace_mode`, and namespace discovery from `[openshell.drivers.kubernetes]`; an external Kubernetes driver receives the equivalent values through its own CLI or environment contract. ### OCSF Log Export @@ -863,7 +863,7 @@ The gateway validates snapshot structure and provider-profile semantics. It trea Set `credential_drivers` only when the gateway should store provider secrets in an external credential backend. Provider secrets include injectable credentials and gateway-only refresh material such as OAuth refresh tokens, client secrets, and service-account private keys. OpenShell supports at most one enabled credential driver at a time. When `credential_drivers` is omitted, the gateway uses its default encrypted database credential storage. `credential_drivers = []` is invalid in the TOML file; omit the field for the default encrypted store, or select a backend such as `kubernetes-secrets` or `vault`. -Credential driver tables are backend-owned and live under `[openshell.credential_drivers.]`. Built-in drivers default to in-tree transport, so they do not need a `transport` field. Use `transport = "uds"` with an absolute `socket_path` only for a remote gRPC driver over a Unix domain socket. +Credential driver tables are backend-owned and live under `[openshell.credential_drivers.]`. Built-in drivers default to in-tree transport, so they do not need a `transport` field. Use `transport = "uds"` with an absolute `socket_path` only for a remote gRPC driver over a Unix domain socket. The gateway rejects a `socket_path` longer than 107 bytes on Linux or 103 bytes on macOS. ```toml [openshell.gateway.credential_storage] @@ -1583,6 +1583,8 @@ kid_path = "/etc/openshell/jwt/kid" gateway_id = "openshell" [openshell.drivers.vm] +# Persistent VM state stays here. The gateway keeps the driver control socket +# in a private temporary directory under /tmp, independent of state_dir. state_dir = "/var/lib/openshell/vm" # Where the gateway looks for the openshell-driver-vm subprocess binary. driver_dir = "/usr/local/libexec/openshell" diff --git a/e2e/rust/e2e-vm.sh b/e2e/rust/e2e-vm.sh index 0cb2c46858..84cc508f85 100755 --- a/e2e/rust/e2e-vm.sh +++ b/e2e/rust/e2e-vm.sh @@ -59,7 +59,7 @@ if [ "${E2E_TEST_OVERRIDE}" = "additional_ca" ]; then fi SANDBOX_IMAGE="${OPENSHELL_SANDBOX_IMAGE:-nvcr.io/nvidia/base/ubuntu:24.04}" -# The VM driver places `compute-driver.sock` under `[openshell.drivers.vm].state_dir`. +# This script binds the external VM driver's `compute-driver.sock` under the run state dir. # AF_UNIX SUN_LEN is 104 bytes on macOS (108 on Linux), so paths anchored # in the workspace's `target/` blow the limit on typical developer # machines — e.g. a ~100-char `~/.superset/worktrees/.../target/...` @@ -169,7 +169,7 @@ print(s.getsockname()[1]) s.close()')" # Per-run state dir so concurrent e2e runs don't collide on the UDS or -# sandbox state. The VM driver creates `/compute-driver.sock` +# sandbox state. The external VM driver binds `/compute-driver.sock` # and `/sandboxes//overlay.ext4` under here. Keep the # basename short — see the SUN_LEN comment above. RUN_STATE_DIR="${STATE_DIR_ROOT}/os-vm-e2e-${HOST_PORT}-$$" diff --git a/tasks/scripts/gateway-vm.sh b/tasks/scripts/gateway-vm.sh index f81b5f24ff..d212db34d7 100755 --- a/tasks/scripts/gateway-vm.sh +++ b/tasks/scripts/gateway-vm.sh @@ -13,9 +13,8 @@ # - Plaintext HTTP on 127.0.0.1:18081 # - Gateway installation and CLI registration name "vm-dev" # - Persistent gateway state (SQLite DB) under .cache/gateway-vm -# - Per-sandbox VM driver state (rootfs + compute-driver.sock) under -# /tmp/openshell-vm-driver-- so the AF_UNIX socket -# path stays under macOS SUN_LEN +# - Per-sandbox VM driver state (rootfs and image cache) under +# /tmp/openshell-vm-driver-- # # Common overrides: # OPENSHELL_SERVER_PORT=18091 mise run gateway:vm @@ -287,10 +286,8 @@ if port_is_in_use "${PORT}"; then exit 2 fi -# AF_UNIX SUN_LEN on macOS is 104 bytes; the VM driver places -# `compute-driver.sock` directly under VM_DRIVER_STATE_DIR, so anchor it -# under /tmp instead of `${ROOT}/.cache` (which is typically too long on -# macOS dev boxes with worktree paths). +# Keep VM driver state outside the worktree. The gateway places the driver +# socket in its own /tmp directory, so this path length does not matter. STATE_LABEL="$(printf '%s' "${GATEWAY_NAME}" | tr -cs '[:alnum:]._-' '-')" if [ -z "${STATE_LABEL}" ]; then STATE_LABEL="vm-dev"