From a8dc36aca3f69e1e6d7bf6d59121cf2f71fc25b2 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Wed, 12 Aug 2026 13:06:14 +0800 Subject: [PATCH 01/20] feat(vmm): generate QEMU netdev from a pre-opened tap chardev An external net daemon can create a macvtap interface for a VM and expose it as a character device such as /dev/tap7498. QEMU cannot open that node itself under the VMM's launch model, but it can use one that is already open: `-netdev tap,id=netN,fd=M`. This adds the manifest side of that path. Design notes: - `Networking.open_file` is a per-NIC option, not a host default. `cvm.networking.open_file` is rejected during config validation and `resolve_networking` overwrites rather than merges it, because the value names one specific device: inheriting it would attach every NIC of every VM to the same tap. - It pairs with `mode = "custom"` and is mutually exclusive with `netdev`. The netdev string is generated rather than operator-supplied because the descriptor number is decided by the process manager, not by the manifest. - Descriptor numbering follows the LISTEN_FDS convention systemd uses for `OpenFile=`: entries are handed over in declaration order starting at fd 3. `open_files` collects the paths in NIC order and `open_file_fd` derives a NIC's number from how many earlier NICs also asked for one, so both sides agree without a runtime handshake. - `validate_open_file` is deliberately narrow. `:` separates the fields of a systemd `OpenFile=` property and `%` starts a specifier expansion, so either character would change what the property means instead of naming a device. - ProcessConfig carries the paths to the process manager. The field is skipped when empty, so existing Supervisor records and requests keep serializing byte-identically. Two combinations are rejected instead of silently launching a VM whose netdev points at an unrelated descriptor: - `cvm.user`, which prefixes QEMU with sudo; sudo closes every descriptor above stderr before exec. - swtpm, which puts vm-launcher between the process manager and QEMU. vm-launcher would inherit the descriptors and leak them into swtpm, and nothing keeps their numbers stable across its own file operations. --- dstack/supervisor/client/src/main.rs | 1 + dstack/supervisor/src/process.rs | 9 +++ dstack/vmm/src/app.rs | 2 + dstack/vmm/src/app/network.rs | 105 ++++++++++++++++++++++++++- dstack/vmm/src/app/qemu.rs | 86 ++++++++++++++++++++-- dstack/vmm/src/config.rs | 54 ++++++++++++++ dstack/vmm/src/main_service.rs | 3 + 7 files changed, 252 insertions(+), 8 deletions(-) diff --git a/dstack/supervisor/client/src/main.rs b/dstack/supervisor/client/src/main.rs index 16d8c61a5..d6ed9177e 100644 --- a/dstack/supervisor/client/src/main.rs +++ b/dstack/supervisor/client/src/main.rs @@ -74,6 +74,7 @@ async fn main() -> Result<()> { pidfile: String::new(), cid: None, note: String::new(), + open_files: Vec::new(), }; print_json(&client.deploy(&config).await?)?; } diff --git a/dstack/supervisor/src/process.rs b/dstack/supervisor/src/process.rs index 94e0c61e5..42cce1b10 100644 --- a/dstack/supervisor/src/process.rs +++ b/dstack/supervisor/src/process.rs @@ -46,6 +46,15 @@ pub struct ProcessConfig { pub cid: Option, #[serde(default)] pub note: String, + /// Files the process manager opens before exec and passes to the process + /// as inherited file descriptors, in declaration order starting at fd 3. + /// + /// Only the VMM's systemd backend implements this. Supervisor rejects a + /// config that sets it rather than starting a process without the file + /// descriptors it asked for. Skipped when empty so existing records and + /// requests keep serializing byte-identically. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub open_files: Vec, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/dstack/vmm/src/app.rs b/dstack/vmm/src/app.rs index bc88f0d1e..4c4abf55d 100644 --- a/dstack/vmm/src/app.rs +++ b/dstack/vmm/src/app.rs @@ -2166,6 +2166,7 @@ mod tests { dhcp_start: String::new(), restrict: false, netdev: String::new(), + open_file: String::new(), }]; workdir.put_manifest(&manifest)?; @@ -2428,6 +2429,7 @@ mod tests { dhcp_start: String::new(), restrict: false, netdev: String::new(), + open_file: String::new(), }]; let user_manifest = test_manifest(2048); let image = test_tdx_image(true); diff --git a/dstack/vmm/src/app/network.rs b/dstack/vmm/src/app/network.rs index 87925d9a6..72cdb9c17 100644 --- a/dstack/vmm/src/app/network.rs +++ b/dstack/vmm/src/app/network.rs @@ -10,7 +10,9 @@ use anyhow::{bail, Result}; use sha2::{Digest, Sha256}; use super::Manifest; -use crate::config::{CvmConfig, Networking, NetworkingMode}; +use crate::config::{ + validate_open_file, CvmConfig, Networking, NetworkingMode, SD_LISTEN_FDS_START, +}; pub(crate) fn resolve_networking(networking: &Networking, cfg: &CvmConfig) -> Networking { let mut resolved = cfg.networking.clone(); @@ -31,6 +33,9 @@ pub(crate) fn resolve_networking(networking: &Networking, cfg: &CvmConfig) -> Ne if !networking.netdev.is_empty() { resolved.netdev = networking.netdev.clone(); } + // Not merged from the host defaults: a pre-opened chardev names one + // device and belongs to exactly one NIC. + resolved.open_file = networking.open_file.clone(); resolved } @@ -47,6 +52,17 @@ pub(crate) fn resolved_networks(manifest: &Manifest, cfg: &CvmConfig) -> Vec Result<()> { + if !networking.open_file.is_empty() { + validate_open_file("networking.open_file", &networking.open_file)?; + if networking.mode != NetworkingMode::Custom { + bail!("networking.open_file requires mode = \"custom\""); + } + if !networking.netdev.is_empty() { + // The netdev string is generated from the inherited fd number, + // which only the process manager knows. + bail!("networking.open_file and networking.netdev are mutually exclusive"); + } + } if networking.mode != NetworkingMode::Bridge { return Ok(()); } @@ -69,6 +85,32 @@ pub(crate) fn validate_resolved_networks(networks: &[Networking]) -> Result<()> Ok(()) } +/// Chardev paths the process manager must open before exec, in NIC order. +/// +/// The order is the contract: systemd hands the files to the service in +/// declaration order, so entry `i` of this list arrives as fd +/// `SD_LISTEN_FDS_START + i`. +pub(crate) fn open_files(networks: &[Networking]) -> Vec { + networks + .iter() + .filter(|networking| !networking.open_file.is_empty()) + .map(|networking| networking.open_file.clone()) + .collect() +} + +/// File descriptor the NIC at `index` receives, or `None` if it does not use a +/// pre-opened chardev. +pub(crate) fn open_file_fd(networks: &[Networking], index: usize) -> Option { + if networks.get(index)?.open_file.is_empty() { + return None; + } + let preceding = networks[..index] + .iter() + .filter(|networking| !networking.open_file.is_empty()) + .count(); + Some(SD_LISTEN_FDS_START + preceding as u32) +} + /// Derives a deterministic, locally administered unicast MAC address. /// /// Index zero preserves the legacy single-NIC derivation. Later interfaces @@ -95,7 +137,66 @@ pub(crate) fn mac_address_for_vm_index(vm_id: &str, prefix: &[u8], index: usize) #[cfg(test)] mod tests { - use super::mac_address_for_vm_index; + use super::{ + mac_address_for_vm_index, open_file_fd, open_files, validate_resolved_network, Networking, + NetworkingMode, + }; + + fn open_file_network(path: &str) -> Networking { + Networking { + mode: NetworkingMode::Custom, + bridge: String::new(), + mac_prefix: String::new(), + net: String::new(), + dhcp_start: String::new(), + restrict: false, + netdev: String::new(), + open_file: path.into(), + } + } + + #[test] + fn open_file_descriptors_are_numbered_in_nic_order() { + let mut networks = vec![ + open_file_network(""), + open_file_network("/dev/tap10"), + open_file_network(""), + open_file_network("/dev/tap11"), + ]; + networks[0].mode = NetworkingMode::User; + networks[2].mode = NetworkingMode::Bridge; + + assert_eq!(open_files(&networks), ["/dev/tap10", "/dev/tap11"]); + assert_eq!(open_file_fd(&networks, 0), None); + assert_eq!(open_file_fd(&networks, 1), Some(3)); + assert_eq!(open_file_fd(&networks, 2), None); + assert_eq!(open_file_fd(&networks, 3), Some(4)); + assert_eq!(open_file_fd(&networks, 4), None); + } + + #[test] + fn open_file_networks_are_validated() { + validate_resolved_network(&open_file_network("/dev/tap7498")).unwrap(); + + for path in [ + "dev/tap7498", + "/dev/tap 7498", + "/dev/tap7498:foo", + "/dev/tap7498,vhost=on", + "/dev/%i/tap7498", + ] { + validate_resolved_network(&open_file_network(path)).unwrap_err(); + } + + let mut wrong_mode = open_file_network("/dev/tap7498"); + wrong_mode.mode = NetworkingMode::Bridge; + wrong_mode.bridge = "br0".into(); + validate_resolved_network(&wrong_mode).unwrap_err(); + + let mut with_netdev = open_file_network("/dev/tap7498"); + with_netdev.netdev = "tap,id=net0,fd=3".into(); + validate_resolved_network(&with_netdev).unwrap_err(); + } #[test] fn primary_mac_keeps_legacy_derivation_and_later_nics_are_distinct() { diff --git a/dstack/vmm/src/app/qemu.rs b/dstack/vmm/src/app/qemu.rs index 67115c0fe..a3ace3ce5 100644 --- a/dstack/vmm/src/app/qemu.rs +++ b/dstack/vmm/src/app/qemu.rs @@ -9,7 +9,10 @@ use super::{ hugepage_numa_nodes, image::Image, mr_config::{snp_host_data, tdx_mr_config_id}, - network::{mac_address_for_vm_index, resolved_networks, validate_resolved_networks}, + network::{ + mac_address_for_vm_index, open_file_fd, open_files, resolved_networks, + validate_resolved_networks, + }, pci_numa_node, round_up, GpuConfig, VmWorkDir, }; use crate::{ @@ -354,6 +357,14 @@ impl VmConfig { let Some(socket) = prepared.swtpm_socket.as_deref() else { return Ok(vec![process]); }; + if !process.open_files.is_empty() { + // The swtpm path puts vm-launcher between the process manager and + // QEMU. vm-launcher would inherit the descriptors and leak them + // into swtpm as well, and nothing keeps their numbers stable + // across the launcher's own file operations, so QEMU could be + // handed an unrelated fd. Reject instead of guessing. + bail!("networking.open_file is not supported for VMs that use swtpm"); + } let swtpm_path = prepared .swtpm_path .as_ref() @@ -414,6 +425,9 @@ impl VmConfig { pidfile: process.pidfile, cid: process.cid, note: process.note, + // Rejected above: file descriptor passing does not survive the + // vm-launcher indirection. + open_files: Vec::new(), }; Ok(vec![launcher]) } @@ -628,12 +642,18 @@ impl QemuCommandBuilder<'_> { } } NetworkingMode::Custom => { - if !networking.netdev.contains(&format!("id={net_id}")) { - bail!( - "custom networking netdev must contain id={net_id} for interface index {index}" - ); + if let Some(fd) = open_file_fd(&self.prepared.networks, index) { + // The chardev is opened by the process manager, so the + // fd number is the only handle QEMU gets. + format!("tap,id={net_id},fd={fd}") + } else { + if !networking.netdev.contains(&format!("id={net_id}")) { + bail!( + "custom networking netdev must contain id={net_id} for interface index {index}" + ); + } + networking.netdev.clone() } - networking.netdev.clone() } }; command.arg("-netdev").arg(netdev); @@ -781,6 +801,7 @@ impl QemuCommandBuilder<'_> { fn process_config(&self, command: Command) -> Result { let workdir = &self.prepared.workdir; + let open_files = open_files(&self.prepared.networks); let mut arguments = vec![self.cfg.qemu_path.to_string_lossy().to_string()]; arguments.extend( command @@ -791,6 +812,13 @@ impl QemuCommandBuilder<'_> { arguments.splice(0..0, ["taskset", "-c", cpus].into_iter().map(String::from)); } if !self.cfg.user.is_empty() { + if !open_files.is_empty() { + // sudo closes every descriptor above stderr before exec, so + // QEMU would be told to use an fd that no longer exists. + bail!( + "networking.open_file requires cvm.user to be empty: sudo closes inherited file descriptors" + ); + } arguments.splice( 0..0, ["sudo", "-u", &self.cfg.user].into_iter().map(String::from), @@ -819,6 +847,7 @@ impl QemuCommandBuilder<'_> { pidfile: workdir.pid_file().to_string_lossy().to_string(), cid: Some(self.vm.cid), note, + open_files, }) } } @@ -1265,5 +1294,50 @@ mod tests { .args .windows(2) .any(|args| args == ["-tpmdev", "emulator,id=tpm0,chardev=chrtpm"])); + + // Pre-opened chardevs. The first NIC keeps user networking, so the + // two NICs that ask for a chardev take the first two descriptors + // systemd hands over. + prepared.swtpm_socket = None; + prepared.networks.push(config.cvm.networking.clone()); + prepared.networks[0].mode = NetworkingMode::User; + for (index, path) in [(1, "/dev/tap7498"), (2, "/dev/tap7499")] { + let networking = &mut prepared.networks[index]; + networking.mode = NetworkingMode::Custom; + networking.bridge = String::new(); + networking.netdev = String::new(); + networking.open_file = path.into(); + } + let process = QemuCommandBuilder { + vm: &vm, + cfg: &config.cvm, + gpus: &GpuConfig::default(), + prepared: &prepared, + } + .build() + .unwrap(); + assert!(process + .args + .windows(2) + .any(|args| args == ["-netdev", "tap,id=net1,fd=3"])); + assert!(process + .args + .windows(2) + .any(|args| args == ["-netdev", "tap,id=net2,fd=4"])); + assert_eq!(process.open_files, ["/dev/tap7498", "/dev/tap7499"]); + + // sudo closes inherited descriptors, so the combination is rejected + // instead of launching QEMU against an fd that no longer exists. + let mut sudo_config = config.clone(); + sudo_config.cvm.user = "qemu".into(); + let error = QemuCommandBuilder { + vm: &vm, + cfg: &sudo_config.cvm, + gpus: &GpuConfig::default(), + prepared: &prepared, + } + .build() + .unwrap_err(); + assert!(error.to_string().contains("cvm.user"), "{error:#}"); } } diff --git a/dstack/vmm/src/config.rs b/dstack/vmm/src/config.rs index f3ca78d3b..b9fe3cacd 100644 --- a/dstack/vmm/src/config.rs +++ b/dstack/vmm/src/config.rs @@ -798,6 +798,13 @@ fn validate_networking(networking: &Networking) -> Result<()> { "cvm.networking.mac_prefix must contain 1 to 3 two-digit hexadecimal bytes" ); } + // A pre-opened chardev names one specific host device, so it belongs to a + // single VM NIC. Inheriting it as a host-wide default would attach every + // VM to the same tap device. + anyhow::ensure!( + networking.open_file.is_empty(), + "cvm.networking.open_file must be set per VM NIC, not as a host-wide default" + ); match networking.mode { NetworkingMode::Bridge => anyhow::ensure!( !networking.bridge.trim().is_empty(), @@ -812,6 +819,29 @@ fn validate_networking(networking: &Networking) -> Result<()> { Ok(()) } +/// First file descriptor systemd hands to a service, per the LISTEN_FDS +/// convention shared by socket activation and `OpenFile=`. +pub const SD_LISTEN_FDS_START: u32 = 3; + +/// Validates a `open_file` path before it reaches a systemd unit property. +/// +/// systemd parses `OpenFile=` as `path:fdname:options` and expands `%` +/// specifiers, so those characters would change the meaning of the property +/// rather than name a device. The check is deliberately conservative: the only +/// intended values are host device nodes such as `/dev/tap7498`. +pub fn validate_open_file(name: &str, path: &str) -> Result<()> { + anyhow::ensure!( + path.starts_with('/'), + "{name} must be an absolute path: {path}" + ); + anyhow::ensure!( + path.bytes() + .all(|byte| byte.is_ascii_graphic() && !matches!(byte, b':' | b',' | b'%' | b'\\')), + "{name} must not contain whitespace or any of ':' ',' '%' '\\': {path}" + ); + Ok(()) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] #[serde(rename_all = "lowercase")] pub enum NetworkingMode { @@ -849,6 +879,19 @@ pub struct Networking { // ── Custom fields ────────────────────────────────────────────── #[serde(default)] pub netdev: String, + + // ── Pre-opened chardev ───────────────────────────────────────── + /// Absolute path to an already existing tap character device, e.g. + /// `/dev/tap7498` for a macvtap interface created by an external net + /// daemon. The process manager opens it before exec and QEMU inherits it + /// as a file descriptor, so the netdev becomes `tap,id=netN,fd=M`. + /// + /// Only the systemd process manager can pass file descriptors, so this is + /// rejected on every other launch path instead of being silently dropped: + /// QEMU would otherwise open an unrelated fd and attach the guest to the + /// wrong network. + #[serde(default)] + pub open_file: String, } impl Networking { @@ -1157,6 +1200,17 @@ mod tests { assert_eq!(parse("auto"), ProcessManagerBackend::Auto); } + #[test] + fn host_wide_open_file_is_rejected() { + let mut config = default_config(); + config.cvm.networking.open_file = "/dev/tap7498".into(); + assert!(config + .validate() + .unwrap_err() + .to_string() + .contains("open_file")); + } + #[test] fn config_validation_rejects_invalid_static_invariants() { let mut config = default_config(); diff --git a/dstack/vmm/src/main_service.rs b/dstack/vmm/src/main_service.rs index ef52eed4c..d8daedfe0 100644 --- a/dstack/vmm/src/main_service.rs +++ b/dstack/vmm/src/main_service.rs @@ -370,6 +370,9 @@ fn networking_from_proto(proto: &rpc::NetworkingConfig) -> Result Date: Wed, 12 Aug 2026 13:06:26 +0800 Subject: [PATCH 02/20] feat(vmm): open VM chardevs from the systemd transient unit systemd 253+ can open files for a service before exec and pass them as inherited descriptors, which is exactly what a NIC with `open_file` needs. Each path becomes an `OpenFile=` property on the transient unit, in the order the VMM collected them across the VM's NICs. The properties carry no fdname and no `graceful` option on purpose: a missing or unopenable device must fail the unit start. Skipping it would shift every later descriptor down by one and hand QEMU somebody else's file. The paths are re-validated here as well, because a process record can outlive the manifest that produced it. Building the systemd-run argument list moved into `run_args` so the rendered properties can be asserted without a live systemd. Backends that cannot pass descriptors reject the process before anything is spawned rather than launching QEMU against a descriptor that was never opened. --- dstack/vmm/src/process_manager.rs | 159 ++++++++++++++++++++++++------ 1 file changed, 128 insertions(+), 31 deletions(-) diff --git a/dstack/vmm/src/process_manager.rs b/dstack/vmm/src/process_manager.rs index e102dd2ee..f30450384 100644 --- a/dstack/vmm/src/process_manager.rs +++ b/dstack/vmm/src/process_manager.rs @@ -15,6 +15,8 @@ use tokio::process::Command; use tokio::sync::RwLock; use tracing::warn; +use crate::config::validate_open_file; + #[derive(Clone)] pub enum ProcessManager { Supervisor(SupervisorClient), @@ -62,7 +64,10 @@ impl ProcessManager { pub async fn deploy(&self, config: &ProcessConfig) -> Result<()> { match self { - Self::Supervisor(client) => client.deploy(config).await, + Self::Supervisor(client) => { + ensure_no_open_files(config)?; + client.deploy(config).await + } Self::Systemd(manager) => manager.deploy(config).await, Self::Auto(manager) => manager.deploy(config).await, } @@ -101,6 +106,19 @@ impl ProcessManager { } } +/// Rejects a process that needs pre-opened file descriptors on a backend that +/// cannot pass them. Launching it anyway would leave QEMU pointing at whatever +/// the fd number happens to be, so this fails before anything is spawned. +fn ensure_no_open_files(config: &ProcessConfig) -> Result<()> { + if !config.open_files.is_empty() { + bail!( + "process {} requires pre-opened files, which only the systemd process manager supports; set cvm.pm = \"systemd\" or \"auto\"", + config.id + ); + } + Ok(()) +} + pub struct AutoProcessManager { systemd: Arc, supervisor: Option, @@ -355,47 +373,63 @@ impl SystemdProcessManager { Ok(output) } - async fn launch(&self, config: &ProcessConfig) -> Result<()> { - let unit = self.unit(&config.id); - // Failed transient units remain loaded until reset and otherwise - // prevent automatic restart from reusing the unit name. - let mut reset = Command::new("systemctl"); - reset.arg("reset-failed").arg(&unit); - let _ = reset.output().await; - let mut command = Command::new("systemd-run"); - command - .arg("--quiet") - .arg("--unit") - .arg(&unit) - .arg("--service-type=exec") - .arg("--property=KillMode=mixed") - .arg("--property=KillSignal=SIGTERM") - .arg("--property=SendSIGKILL=yes") - .arg(format!("--property=TimeoutStopSec={}", self.stop_timeout)) - .arg("--property=ExitType=cgroup") - .arg("--property=Restart=no") - .arg(format!("--description=dstack VM process {}", config.id)); + fn run_args(&self, config: &ProcessConfig, unit: &str) -> Result> { + let mut args = vec![ + "--quiet".into(), + "--unit".into(), + unit.to_string(), + "--service-type=exec".into(), + "--property=KillMode=mixed".into(), + "--property=KillSignal=SIGTERM".into(), + "--property=SendSIGKILL=yes".into(), + format!("--property=TimeoutStopSec={}", self.stop_timeout), + "--property=ExitType=cgroup".into(), + "--property=Restart=no".into(), + format!("--description=dstack VM process {}", config.id), + ]; if !config.cwd.is_empty() { - command.arg(format!("--working-directory={}", config.cwd)); + args.push(format!("--working-directory={}", config.cwd)); } if config.stdout.is_empty() { - command.arg("--property=StandardOutput=null"); + args.push("--property=StandardOutput=null".into()); } else { - command.arg(format!( + args.push(format!( "--property=StandardOutput=append:{}", config.stdout )); } if config.stderr.is_empty() { - command.arg("--property=StandardError=null"); + args.push("--property=StandardError=null".into()); } else { - command.arg(format!("--property=StandardError=append:{}", config.stderr)); + args.push(format!("--property=StandardError=append:{}", config.stderr)); } for (key, value) in &config.env { - command.arg(format!("--setenv={key}={value}")); + args.push(format!("--setenv={key}={value}")); + } + // systemd opens these before exec and passes them in declaration + // order starting at fd 3, which is what the QEMU netdev arguments + // reference. No fdname and no `graceful` option: a missing device must + // fail the unit instead of shifting every later descriptor by one. + for path in &config.open_files { + validate_open_file("open_files entry", path)?; + args.push(format!("--property=OpenFile={path}")); } - command.arg("--").arg(&config.command).args(&config.args); + args.push("--".into()); + args.push(config.command.clone()); + args.extend(config.args.iter().cloned()); + Ok(args) + } + + async fn launch(&self, config: &ProcessConfig) -> Result<()> { + let unit = self.unit(&config.id); + // Failed transient units remain loaded until reset and otherwise + // prevent automatic restart from reusing the unit name. + let mut reset = Command::new("systemctl"); + reset.arg("reset-failed").arg(&unit); + let _ = reset.output().await; + let mut command = Command::new("systemd-run"); + command.args(self.run_args(config, &unit)?); Self::command(command, "systemd-run").await?; if !config.pidfile.is_empty() { @@ -544,6 +578,30 @@ mod tests { #[test] fn unit_names_are_stable_and_do_not_embed_process_ids() { + let (_dir, manager) = test_manager(); + assert_eq!(manager.unit("vm/one"), manager.unit("vm/one")); + assert_ne!(manager.unit("vm/one"), manager.unit("vm-two")); + assert!(!manager.unit("vm/one").contains("vm/one")); + } + + fn test_config(open_files: &[&str]) -> ProcessConfig { + ProcessConfig { + id: "vm/one".into(), + name: "vm".into(), + command: "/usr/bin/qemu".into(), + args: vec!["-netdev".into(), "tap,id=net0,fd=3".into()], + env: HashMap::new(), + cwd: String::new(), + stdout: String::new(), + stderr: String::new(), + pidfile: String::new(), + cid: None, + note: String::new(), + open_files: open_files.iter().map(|path| path.to_string()).collect(), + } + } + + fn test_manager() -> (tempfile::TempDir, SystemdProcessManager) { let dir = tempfile::tempdir().unwrap(); let manager = SystemdProcessManager::new( dir.path().to_path_buf(), @@ -551,9 +609,48 @@ mod tests { "infinity".into(), ) .unwrap(); - assert_eq!(manager.unit("vm/one"), manager.unit("vm/one")); - assert_ne!(manager.unit("vm/one"), manager.unit("vm-two")); - assert!(!manager.unit("vm/one").contains("vm/one")); + (dir, manager) + } + + #[test] + fn renders_open_files_as_ordered_unit_properties() { + let (_dir, manager) = test_manager(); + let args = manager + .run_args( + &test_config(&["/dev/tap7498", "/dev/tap7499"]), + "unit.service", + ) + .unwrap(); + let properties = args + .iter() + .take_while(|arg| *arg != "--") + .filter_map(|arg| arg.strip_prefix("--property=OpenFile=")) + .collect::>(); + assert_eq!(properties, ["/dev/tap7498", "/dev/tap7499"]); + assert_eq!(args.last().unwrap(), "tap,id=net0,fd=3"); + + assert!(!manager + .run_args(&test_config(&[]), "unit.service") + .unwrap() + .iter() + .any(|arg| arg.contains("OpenFile"))); + } + + #[test] + fn rejects_open_files_that_would_change_the_unit_property() { + let (_dir, manager) = test_manager(); + for path in ["relative/tap", "/dev/tap:0", "/dev/%i/tap"] { + manager + .run_args(&test_config(&[path]), "unit.service") + .unwrap_err(); + } + } + + #[test] + fn supervisor_backend_rejects_open_files() { + ensure_no_open_files(&test_config(&[])).unwrap(); + let error = ensure_no_open_files(&test_config(&["/dev/tap7498"])).unwrap_err(); + assert!(error.to_string().contains("systemd"), "{error:#}"); } #[test] From 23b5bf3f1b4176561d9571a31146edce9eb1d963 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Wed, 12 Aug 2026 13:06:26 +0800 Subject: [PATCH 03/20] fix: reject pre-opened chardevs where descriptors cannot be passed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Supervisor spawns processes without pre-opened descriptors and one-shot mode execs QEMU directly, so both would start a VM whose netdev refers to a descriptor that does not exist — or worse, to an unrelated file that happens to occupy the number. Supervisor rejects the deploy at its own API rather than relying on the VMM to filter, since it is a general-purpose process runner with other callers. One-shot mirrors the existing libvirt-filtering guard and still allows --dry-run, which only prints the command. --- dstack/supervisor/src/supervisor.rs | 6 ++++++ dstack/vmm/src/one_shot.rs | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/dstack/supervisor/src/supervisor.rs b/dstack/supervisor/src/supervisor.rs index 378013c05..a73c0acd0 100644 --- a/dstack/supervisor/src/supervisor.rs +++ b/dstack/supervisor/src/supervisor.rs @@ -59,6 +59,12 @@ impl Supervisor { if id.is_empty() { return Err(anyhow::anyhow!("Process ID is empty")); } + if !config.open_files.is_empty() { + // Supervisor spawns processes without pre-opened file descriptors, + // so honoring the rest of the config would start a process that is + // missing the files it depends on. + bail!("open_files is not supported by supervisor"); + } if self .info(&id) .is_some_and(|info| info.state.status.is_running()) diff --git a/dstack/vmm/src/one_shot.rs b/dstack/vmm/src/one_shot.rs index c71bee58f..46ef7e733 100644 --- a/dstack/vmm/src/one_shot.rs +++ b/dstack/vmm/src/one_shot.rs @@ -290,6 +290,16 @@ Compose file content (first 200 chars): ); } + if !dry_run + && resolved_networks(&manifest, &config.cvm) + .iter() + .any(|network| !network.open_file.is_empty()) + { + anyhow::bail!( + "one-shot execution cannot pass pre-opened file descriptors to QEMU; run the VMM server with cvm.pm = \"systemd\" or use --dry-run" + ); + } + let process_configs = vm_builder_config .config_qemu(&workdir_path, &config.cvm, &gpus) .context("Failed to build QEMU configuration")?; From 739c99db9a13885ea456b7e9897ffda35c023d43 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Wed, 12 Aug 2026 19:55:41 +0800 Subject: [PATCH 04/20] feat(vmm): drop VM privileges through the systemd unit instead of sudo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production must not launch VMs through sudo. The systemd backend can do the privilege drop itself, so when `cvm.user` is set and the effective process manager is systemd (`cvm.pm = "systemd"` or `"auto"`, whose new deploys always land on systemd), QEMU is exec'd directly and the transient unit carries `--property=User=`. Supervisor has no such mechanism and keeps the sudo prefix unchanged. `taskset` wrapping is untouched in both cases. This makes `open_file` plus `cvm.user` the normal production combination, so the rejection added for it is now scoped to the supervisor backend, where sudo's closefrom behaviour still destroys the descriptors before QEMU starts. OpenFile= vs the privilege drop: systemd.exec(5) states "The file or socket is opened by the service manager and the file descriptor is passed to the service", and the drop to `User=` happens in the forked child just before exec. The chardev is therefore opened with the manager's privileges, which is what makes this useful: a root-owned /dev/tapN does not have to be chowned to the QEMU user. This is the documented contract rather than an observed one — worth a `systemd-run --property=OpenFile= --property=User=` smoke test on the node before relying on it in production. Mechanics: - ProcessConfig carries `user`, skipped when empty so existing Supervisor records and requests keep serializing byte-identically. Supervisor rejects a non-empty value at its own API instead of running a VM as root that asked to be confined. - The value is validated against a POSIX-user-name charset before it becomes a unit property, so it cannot introduce `%` specifier expansion or extra property syntax. `cvm.user` is checked at config load as well. - One-shot mode rejects a non-empty user: it creates no unit and the command has no sudo prefix, so it would run QEMU with the VMM's own privileges. - The swtpm path passes the user through to the vm-launcher unit, so vm-launcher and the swtpm and QEMU children it spawns all run unprivileged. Under Supervisor only QEMU dropped privileges, so this is a behaviour change for TPM-backed VMs that needs a run on a real node. --- dstack/supervisor/client/src/main.rs | 1 + dstack/supervisor/src/process.rs | 8 +++ dstack/supervisor/src/supervisor.rs | 6 +++ dstack/vmm/src/app/qemu.rs | 74 +++++++++++++++++++++----- dstack/vmm/src/config.rs | 38 ++++++++++++++ dstack/vmm/src/one_shot.rs | 9 ++++ dstack/vmm/src/process_manager.rs | 78 ++++++++++++++++++++++------ 7 files changed, 186 insertions(+), 28 deletions(-) diff --git a/dstack/supervisor/client/src/main.rs b/dstack/supervisor/client/src/main.rs index d6ed9177e..b076a8ddb 100644 --- a/dstack/supervisor/client/src/main.rs +++ b/dstack/supervisor/client/src/main.rs @@ -74,6 +74,7 @@ async fn main() -> Result<()> { pidfile: String::new(), cid: None, note: String::new(), + user: String::new(), open_files: Vec::new(), }; print_json(&client.deploy(&config).await?)?; diff --git a/dstack/supervisor/src/process.rs b/dstack/supervisor/src/process.rs index 42cce1b10..595c583d9 100644 --- a/dstack/supervisor/src/process.rs +++ b/dstack/supervisor/src/process.rs @@ -46,6 +46,14 @@ pub struct ProcessConfig { pub cid: Option, #[serde(default)] pub note: String, + /// User the process manager runs the process as. + /// + /// Only the VMM's systemd backend implements this, by dropping privileges + /// in the transient unit. Supervisor rejects a config that sets it rather + /// than running the process with its own privileges. Skipped when empty so + /// existing records and requests keep serializing byte-identically. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub user: String, /// Files the process manager opens before exec and passes to the process /// as inherited file descriptors, in declaration order starting at fd 3. /// diff --git a/dstack/supervisor/src/supervisor.rs b/dstack/supervisor/src/supervisor.rs index a73c0acd0..18d7c528d 100644 --- a/dstack/supervisor/src/supervisor.rs +++ b/dstack/supervisor/src/supervisor.rs @@ -59,6 +59,12 @@ impl Supervisor { if id.is_empty() { return Err(anyhow::anyhow!("Process ID is empty")); } + if !config.user.is_empty() { + // Supervisor runs processes with its own privileges. Starting the + // process anyway would run a VM as root that asked to be confined + // to an unprivileged user. + bail!("user is not supported by supervisor"); + } if !config.open_files.is_empty() { // Supervisor spawns processes without pre-opened file descriptors, // so honoring the rest of the config would start a process that is diff --git a/dstack/vmm/src/app/qemu.rs b/dstack/vmm/src/app/qemu.rs index a3ace3ce5..0a66d29dc 100644 --- a/dstack/vmm/src/app/qemu.rs +++ b/dstack/vmm/src/app/qemu.rs @@ -19,6 +19,7 @@ use crate::{ app::Manifest, config::{ CvmConfig, CvmPlatform, NetworkFilterMode, Networking, NetworkingMode, ProcessAnnotation, + ProcessManagerBackend, }, netd::{tap_name, InterfaceIdentity}, vm_launcher::{ChildCommand, LaunchSpec}, @@ -425,6 +426,9 @@ impl VmConfig { pidfile: process.pidfile, cid: process.cid, note: process.note, + // The launcher unit owns the privilege drop, so vm-launcher and + // the swtpm and QEMU children it spawns all run as this user. + user: process.user, // Rejected above: file descriptor passing does not survive the // vm-launcher indirection. open_files: Vec::new(), @@ -811,18 +815,26 @@ impl QemuCommandBuilder<'_> { if let Some(cpus) = &self.prepared.numa_cpus { arguments.splice(0..0, ["taskset", "-c", cpus].into_iter().map(String::from)); } + // The systemd backend drops privileges in the unit itself, so QEMU is + // exec'd directly. Supervisor has no such mechanism and keeps the sudo + // prefix, which is also why it cannot pass file descriptors: sudo + // closes every descriptor above stderr before exec, so QEMU would be + // told to use an fd that no longer exists. + let mut user = String::new(); if !self.cfg.user.is_empty() { - if !open_files.is_empty() { - // sudo closes every descriptor above stderr before exec, so - // QEMU would be told to use an fd that no longer exists. - bail!( - "networking.open_file requires cvm.user to be empty: sudo closes inherited file descriptors" + if self.cfg.pm == ProcessManagerBackend::Supervisor { + if !open_files.is_empty() { + bail!( + "networking.open_file requires cvm.pm = \"systemd\" or \"auto\" when cvm.user is set: sudo closes inherited file descriptors" + ); + } + arguments.splice( + 0..0, + ["sudo", "-u", &self.cfg.user].into_iter().map(String::from), ); + } else { + user = self.cfg.user.clone(); } - arguments.splice( - 0..0, - ["sudo", "-u", &self.cfg.user].into_iter().map(String::from), - ); } let command = arguments.remove(0); @@ -847,6 +859,7 @@ impl QemuCommandBuilder<'_> { pidfile: workdir.pid_file().to_string_lossy().to_string(), cid: Some(self.vm.cid), note, + user, open_files, }) } @@ -1036,7 +1049,8 @@ mod tests { use crate::app::image::{Image, ImageInfo}; use crate::app::{needs_swtpm, GpuConfig, Manifest, PortMapping, VmVolume, VmWorkDir}; use crate::config::{ - Config, CvmPlatform, NetworkFilterMode, NetworkingMode, Protocol, DEFAULT_CONFIG, + Config, CvmPlatform, NetworkFilterMode, NetworkingMode, ProcessManagerBackend, Protocol, + DEFAULT_CONFIG, }; use crate::netd::{tap_name, InterfaceIdentity}; use dstack_types::{KeyProviderKind, TeeVariant}; @@ -1326,8 +1340,25 @@ mod tests { .any(|args| args == ["-netdev", "tap,id=net2,fd=4"])); assert_eq!(process.open_files, ["/dev/tap7498", "/dev/tap7499"]); - // sudo closes inherited descriptors, so the combination is rejected - // instead of launching QEMU against an fd that no longer exists. + // systemd drops privileges in the unit, so QEMU is exec'd directly and + // keeps the descriptors it was handed. + let mut systemd_config = config.clone(); + systemd_config.cvm.pm = ProcessManagerBackend::Systemd; + systemd_config.cvm.user = "qemu".into(); + let process = QemuCommandBuilder { + vm: &vm, + cfg: &systemd_config.cvm, + gpus: &GpuConfig::default(), + prepared: &prepared, + } + .build() + .unwrap(); + assert_eq!(process.user, "qemu"); + assert_eq!(process.command, "/not-installed/qemu-system-x86_64"); + assert!(!process.args.iter().any(|arg| arg == "sudo")); + + // Supervisor has no privilege-drop mechanism and falls back to sudo, + // which closes the descriptors before QEMU starts. let mut sudo_config = config.clone(); sudo_config.cvm.user = "qemu".into(); let error = QemuCommandBuilder { @@ -1338,6 +1369,23 @@ mod tests { } .build() .unwrap_err(); - assert!(error.to_string().contains("cvm.user"), "{error:#}"); + assert!(error.to_string().contains("cvm.pm"), "{error:#}"); + + // Without a pre-opened chardev, Supervisor keeps the sudo prefix. + for networking in &mut prepared.networks { + networking.open_file = String::new(); + networking.mode = NetworkingMode::User; + } + let process = QemuCommandBuilder { + vm: &vm, + cfg: &sudo_config.cvm, + gpus: &GpuConfig::default(), + prepared: &prepared, + } + .build() + .unwrap(); + assert_eq!(process.command, "sudo"); + assert_eq!(&process.args[..2], ["-u", "qemu"]); + assert!(process.user.is_empty()); } } diff --git a/dstack/vmm/src/config.rs b/dstack/vmm/src/config.rs index b9fe3cacd..9d22b4040 100644 --- a/dstack/vmm/src/config.rs +++ b/dstack/vmm/src/config.rs @@ -727,6 +727,9 @@ impl Config { } validate_networking(&self.cvm.networking)?; + if !self.cvm.user.is_empty() { + validate_unit_user("cvm.user", &self.cvm.user)?; + } if self.cvm.pm != ProcessManagerBackend::Systemd { anyhow::ensure!( !self.supervisor.sock.trim().is_empty(), @@ -829,6 +832,25 @@ pub const SD_LISTEN_FDS_START: u32 = 3; /// specifiers, so those characters would change the meaning of the property /// rather than name a device. The check is deliberately conservative: the only /// intended values are host device nodes such as `/dev/tap7498`. +/// Validates a user name before it reaches a systemd unit property. +/// +/// `User=` takes a user name or a numeric UID. The charset is kept to what a +/// POSIX user name can contain so the value cannot introduce a `%` specifier +/// expansion or extra property syntax. +pub fn validate_unit_user(name: &str, user: &str) -> Result<()> { + anyhow::ensure!(!user.is_empty(), "{name} must not be empty"); + anyhow::ensure!( + !user.starts_with('-'), + "{name} must not start with '-': {user}" + ); + anyhow::ensure!( + user.bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'-' | b'.')), + "{name} must contain only alphanumerics, '_', '-' and '.': {user}" + ); + Ok(()) +} + pub fn validate_open_file(name: &str, path: &str) -> Result<()> { anyhow::ensure!( path.starts_with('/'), @@ -1211,6 +1233,22 @@ mod tests { .contains("open_file")); } + #[test] + fn unit_user_names_are_validated() { + validate_unit_user("cvm.user", "qemu-1.user_x").unwrap(); + for user in ["", "-qemu", "qemu:0", "qemu user", "%i", "qemu$"] { + validate_unit_user("cvm.user", user).unwrap_err(); + } + + let mut config = default_config(); + config.cvm.user = "qemu:0".into(); + assert!(config + .validate() + .unwrap_err() + .to_string() + .contains("cvm.user")); + } + #[test] fn config_validation_rejects_invalid_static_invariants() { let mut config = default_config(); diff --git a/dstack/vmm/src/one_shot.rs b/dstack/vmm/src/one_shot.rs index 46ef7e733..deb15f31d 100644 --- a/dstack/vmm/src/one_shot.rs +++ b/dstack/vmm/src/one_shot.rs @@ -322,6 +322,15 @@ Compose file content (first 200 chars): println!("# QEMU Command:"); println!("{}", full_command.join(" ")); + if !dry_run && !process_config.user.is_empty() { + // Privileges are dropped by the systemd unit, which one-shot mode does + // not create, and the command carries no sudo prefix either. Running it + // here would start QEMU with the VMM's own privileges. + anyhow::bail!( + "one-shot execution cannot drop privileges to cvm.user with cvm.pm = \"systemd\" or \"auto\"; use --dry-run or cvm.pm = \"supervisor\"" + ); + } + if dry_run { println!("# Dry run mode - QEMU command not executed"); println!( diff --git a/dstack/vmm/src/process_manager.rs b/dstack/vmm/src/process_manager.rs index f30450384..5b6e2b752 100644 --- a/dstack/vmm/src/process_manager.rs +++ b/dstack/vmm/src/process_manager.rs @@ -15,7 +15,7 @@ use tokio::process::Command; use tokio::sync::RwLock; use tracing::warn; -use crate::config::validate_open_file; +use crate::config::{validate_open_file, validate_unit_user}; #[derive(Clone)] pub enum ProcessManager { @@ -65,7 +65,7 @@ impl ProcessManager { pub async fn deploy(&self, config: &ProcessConfig) -> Result<()> { match self { Self::Supervisor(client) => { - ensure_no_open_files(config)?; + ensure_supervisor_supported(config)?; client.deploy(config).await } Self::Systemd(manager) => manager.deploy(config).await, @@ -106,15 +106,23 @@ impl ProcessManager { } } -/// Rejects a process that needs pre-opened file descriptors on a backend that -/// cannot pass them. Launching it anyway would leave QEMU pointing at whatever -/// the fd number happens to be, so this fails before anything is spawned. -fn ensure_no_open_files(config: &ProcessConfig) -> Result<()> { - if !config.open_files.is_empty() { - bail!( - "process {} requires pre-opened files, which only the systemd process manager supports; set cvm.pm = \"systemd\" or \"auto\"", - config.id - ); +/// Rejects a process asking for something Supervisor cannot provide. +/// +/// Supervisor spawns processes with its own privileges and without pre-opened +/// file descriptors. Launching anyway would run a VM as root that asked to be +/// confined, or leave QEMU pointing at whatever the fd number happens to be, +/// so this fails before anything is spawned. +fn ensure_supervisor_supported(config: &ProcessConfig) -> Result<()> { + for (what, unsupported) in [ + ("pre-opened files", !config.open_files.is_empty()), + ("a dedicated user", !config.user.is_empty()), + ] { + if unsupported { + bail!( + "process {} requires {what}, which only the systemd process manager supports; set cvm.pm = \"systemd\" or \"auto\"", + config.id + ); + } } Ok(()) } @@ -407,10 +415,20 @@ impl SystemdProcessManager { for (key, value) in &config.env { args.push(format!("--setenv={key}={value}")); } + if !config.user.is_empty() { + validate_unit_user("user", &config.user)?; + args.push(format!("--property=User={}", config.user)); + } // systemd opens these before exec and passes them in declaration // order starting at fd 3, which is what the QEMU netdev arguments // reference. No fdname and no `graceful` option: a missing device must // fail the unit instead of shifting every later descriptor by one. + // + // systemd.exec(5): "The file or socket is opened by the service + // manager and the file descriptor is passed to the service." The open + // therefore happens with the manager's privileges, before the `User=` + // drop that lands just before exec, so a root-owned chardev such as + // /dev/tapN does not have to be chowned to the QEMU user. for path in &config.open_files { validate_open_file("open_files entry", path)?; args.push(format!("--property=OpenFile={path}")); @@ -585,6 +603,10 @@ mod tests { } fn test_config(open_files: &[&str]) -> ProcessConfig { + test_config_as("", open_files) + } + + fn test_config_as(user: &str, open_files: &[&str]) -> ProcessConfig { ProcessConfig { id: "vm/one".into(), name: "vm".into(), @@ -597,6 +619,7 @@ mod tests { pidfile: String::new(), cid: None, note: String::new(), + user: user.into(), open_files: open_files.iter().map(|path| path.to_string()).collect(), } } @@ -647,10 +670,35 @@ mod tests { } #[test] - fn supervisor_backend_rejects_open_files() { - ensure_no_open_files(&test_config(&[])).unwrap(); - let error = ensure_no_open_files(&test_config(&["/dev/tap7498"])).unwrap_err(); - assert!(error.to_string().contains("systemd"), "{error:#}"); + fn renders_the_privilege_drop_as_a_unit_property() { + let (_dir, manager) = test_manager(); + let args = manager + .run_args(&test_config_as("qemu", &["/dev/tap7498"]), "unit.service") + .unwrap(); + assert!(args.iter().any(|arg| arg == "--property=User=qemu")); + // The privilege drop replaces the sudo prefix rather than joining it. + assert!(!args.iter().any(|arg| arg == "sudo")); + + assert!(!manager + .run_args(&test_config(&[]), "unit.service") + .unwrap() + .iter() + .any(|arg| arg.contains("User="))); + + for user in ["qemu:0", "qemu user", "%i", "-qemu"] { + manager + .run_args(&test_config_as(user, &[]), "unit.service") + .unwrap_err(); + } + } + + #[test] + fn supervisor_backend_rejects_what_it_cannot_provide() { + ensure_supervisor_supported(&test_config(&[])).unwrap(); + for config in [test_config(&["/dev/tap7498"]), test_config_as("qemu", &[])] { + let error = ensure_supervisor_supported(&config).unwrap_err(); + assert!(error.to_string().contains("systemd"), "{error:#}"); + } } #[test] From 8c9e0051ff0cb695003d4f46ea9cd8d04704353c Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Thu, 13 Aug 2026 00:10:47 +0800 Subject: [PATCH 05/20] fix(supervisor): default ProcessConfig builder fields for user and open_files --- dstack/supervisor/src/process.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dstack/supervisor/src/process.rs b/dstack/supervisor/src/process.rs index 595c583d9..6286d0087 100644 --- a/dstack/supervisor/src/process.rs +++ b/dstack/supervisor/src/process.rs @@ -53,6 +53,7 @@ pub struct ProcessConfig { /// than running the process with its own privileges. Skipped when empty so /// existing records and requests keep serializing byte-identically. #[serde(default, skip_serializing_if = "String::is_empty")] + #[builder(default)] pub user: String, /// Files the process manager opens before exec and passes to the process /// as inherited file descriptors, in declaration order starting at fd 3. @@ -62,6 +63,7 @@ pub struct ProcessConfig { /// descriptors it asked for. Skipped when empty so existing records and /// requests keep serializing byte-identically. #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[builder(default)] pub open_files: Vec, } From 2d16a9c813ea8cc9f5d591a8aa0d7a22ef95d52b Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Thu, 13 Aug 2026 00:10:48 +0800 Subject: [PATCH 06/20] fix(vmm): accept numeric cvm.user for sudo and systemd --- dstack/vmm/src/app.rs | 8 +- dstack/vmm/src/app/qemu.rs | 55 +++++++++-- dstack/vmm/src/config.rs | 146 +++++++++++++++++++++++------- dstack/vmm/src/process_manager.rs | 18 +++- 4 files changed, 175 insertions(+), 52 deletions(-) diff --git a/dstack/vmm/src/app.rs b/dstack/vmm/src/app.rs index 4c4abf55d..f57cf0683 100644 --- a/dstack/vmm/src/app.rs +++ b/dstack/vmm/src/app.rs @@ -22,7 +22,7 @@ use dstack_vmm_rpc::{ use fs_err as fs; use guest_api::client::DefaultClient as GuestClient; use id_pool::IdPool; -use nix::unistd::{Uid, User}; +use nix::unistd::Uid; use or_panic::ResultOrPanic; use ra_rpc::client::RaClient; use serde::{Deserialize, Serialize}; @@ -521,11 +521,7 @@ impl App { let qemu_uid = if self.config.cvm.user.is_empty() { Uid::effective().as_raw() } else { - User::from_name(&self.config.cvm.user) - .context("failed to resolve QEMU user")? - .with_context(|| format!("QEMU user {} does not exist", self.config.cvm.user))? - .uid - .as_raw() + qemu::resolve_cvm_user(&self.config.cvm.user)?.uid.as_raw() }; let mut prepared = Vec::new(); for (nic_index, network) in networks.iter().enumerate() { diff --git a/dstack/vmm/src/app/qemu.rs b/dstack/vmm/src/app/qemu.rs index 0a66d29dc..2f2a825fb 100644 --- a/dstack/vmm/src/app/qemu.rs +++ b/dstack/vmm/src/app/qemu.rs @@ -18,8 +18,8 @@ use super::{ use crate::{ app::Manifest, config::{ - CvmConfig, CvmPlatform, NetworkFilterMode, Networking, NetworkingMode, ProcessAnnotation, - ProcessManagerBackend, + parse_unit_user, CvmConfig, CvmPlatform, NetworkFilterMode, Networking, NetworkingMode, + ProcessAnnotation, ProcessManagerBackend, UnitUser, }, netd::{tap_name, InterfaceIdentity}, vm_launcher::{ChildCommand, LaunchSpec}, @@ -28,7 +28,7 @@ use anyhow::{bail, Context, Result}; use bon::Builder; use dstack_types::shared_filenames::HOST_SHARED_DISK_LABEL; use fs_err as fs; -use nix::unistd::User; +use nix::unistd::{Uid, User}; use serde::Serialize; use std::collections::HashMap; use std::os::unix::fs::PermissionsExt; @@ -312,6 +312,17 @@ fn prepare_data_disk(vm: &VmConfig, workdir: &VmWorkDir, cfg: &CvmConfig) -> Res Ok(()) } +pub(crate) fn resolve_cvm_user(user: &str) -> Result { + match parse_unit_user("cvm.user", user)? { + UnitUser::Name(name) => User::from_name(&name) + .context("failed to resolve QEMU user")? + .with_context(|| format!("QEMU user {name} does not exist")), + UnitUser::Uid(uid) => User::from_uid(Uid::from_raw(uid)) + .context("failed to resolve QEMU user")? + .with_context(|| format!("QEMU user uid {uid} does not exist")), + } +} + fn prepare_shared_dir(workdir: &VmWorkDir) -> Result<()> { let shared_dir = workdir.shared_dir(); if !shared_dir.exists() { @@ -373,9 +384,7 @@ impl VmConfig { let (socket_uid, socket_gid) = if cfg.user.is_empty() { (unsafe { libc::geteuid() }, unsafe { libc::getegid() }) } else { - let user = User::from_name(&cfg.user) - .context("failed to resolve QEMU user")? - .with_context(|| format!("QEMU user {} does not exist", cfg.user))?; + let user = resolve_cvm_user(&cfg.user)?; (user.uid.as_raw(), user.gid.as_raw()) }; @@ -822,18 +831,21 @@ impl QemuCommandBuilder<'_> { // told to use an fd that no longer exists. let mut user = String::new(); if !self.cfg.user.is_empty() { + let unit_user = parse_unit_user("cvm.user", &self.cfg.user)?; if self.cfg.pm == ProcessManagerBackend::Supervisor { if !open_files.is_empty() { bail!( "networking.open_file requires cvm.pm = \"systemd\" or \"auto\" when cvm.user is set: sudo closes inherited file descriptors" ); } + let sudo_user = unit_user.sudo_value(); arguments.splice( 0..0, - ["sudo", "-u", &self.cfg.user].into_iter().map(String::from), + ["sudo", "-u", &sudo_user].into_iter().map(String::from), ); } else { - user = self.cfg.user.clone(); + // systemd User= takes a bare name or decimal UID, not sudo's #UID. + user = unit_user.systemd_value(); } } @@ -1387,5 +1399,32 @@ mod tests { assert_eq!(process.command, "sudo"); assert_eq!(&process.args[..2], ["-u", "qemu"]); assert!(process.user.is_empty()); + + // Numeric UIDs keep sudo's #UID form and systemd's bare digits. + sudo_config.cvm.user = "#1000".into(); + let process = QemuCommandBuilder { + vm: &vm, + cfg: &sudo_config.cvm, + gpus: &GpuConfig::default(), + prepared: &prepared, + } + .build() + .unwrap(); + assert_eq!(process.command, "sudo"); + assert_eq!(&process.args[..2], ["-u", "#1000"]); + + let mut uid_config = config.clone(); + uid_config.cvm.pm = ProcessManagerBackend::Systemd; + uid_config.cvm.user = "#1000".into(); + let process = QemuCommandBuilder { + vm: &vm, + cfg: &uid_config.cvm, + gpus: &GpuConfig::default(), + prepared: &prepared, + } + .build() + .unwrap(); + assert_eq!(process.user, "1000"); + assert!(!process.args.iter().any(|arg| arg == "sudo")); } } diff --git a/dstack/vmm/src/config.rs b/dstack/vmm/src/config.rs index 9d22b4040..f20ab3358 100644 --- a/dstack/vmm/src/config.rs +++ b/dstack/vmm/src/config.rs @@ -333,7 +333,10 @@ pub struct CvmConfig { pub qmp_socket: bool, /// GPU configuration pub gpu: GpuConfig, - /// Use sudo to run the VM + /// User the VM process runs as. Empty keeps the VMM's own privileges. + /// Supervisor prefixes QEMU with `sudo -u`; systemd sets `User=` on the + /// transient unit. Accepts a POSIX user name, a numeric UID, or sudo's + /// `#UID` form. pub user: String, /// Auto restart configuration @@ -824,43 +827,91 @@ fn validate_networking(networking: &Networking) -> Result<()> { /// First file descriptor systemd hands to a service, per the LISTEN_FDS /// convention shared by socket activation and `OpenFile=`. -pub const SD_LISTEN_FDS_START: u32 = 3; +pub(crate) const SD_LISTEN_FDS_START: u32 = 3; + +/// A `cvm.user` value after syntax checks, before looking the account up. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum UnitUser { + /// POSIX user name for `User=` / `sudo -u`. + Name(String), + /// Numeric UID. systemd takes the bare digits; sudo needs `#UID`. + Uid(u32), +} + +impl UnitUser { + /// Value for a systemd `User=` property. + pub(crate) fn systemd_value(&self) -> String { + match self { + Self::Name(name) => name.clone(), + Self::Uid(uid) => uid.to_string(), + } + } + + /// Value for `sudo -u`. + pub(crate) fn sudo_value(&self) -> String { + match self { + Self::Name(name) => name.clone(), + Self::Uid(uid) => format!("#{uid}"), + } + } +} -/// Validates a `open_file` path before it reaches a systemd unit property. +/// Parses a user name or numeric UID before it reaches sudo or a unit property. +/// +/// Accepts a POSIX user name, a bare decimal UID (systemd `User=`), or sudo's +/// `#UID` form. The charset for names excludes `%` and property separators so +/// the value cannot expand as a systemd specifier or inject extra syntax. +pub(crate) fn parse_unit_user(name: &str, user: &str) -> Result { + if user.is_empty() { + bail!("{name} must not be empty"); + } + if let Some(digits) = user.strip_prefix('#') { + if digits.is_empty() || !digits.bytes().all(|byte| byte.is_ascii_digit()) { + bail!("{name} must be '#' when it starts with '#': {user}"); + } + let uid = digits + .parse::() + .with_context(|| format!("{name} contains an out-of-range uid: {user}"))?; + return Ok(UnitUser::Uid(uid)); + } + if user.bytes().all(|byte| byte.is_ascii_digit()) { + let uid = user + .parse::() + .with_context(|| format!("{name} contains an out-of-range uid: {user}"))?; + return Ok(UnitUser::Uid(uid)); + } + if user.starts_with('-') { + bail!("{name} must not start with '-': {user}"); + } + if !user + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'-' | b'.')) + { + bail!("{name} must contain only alphanumerics, '_', '-' and '.': {user}"); + } + Ok(UnitUser::Name(user.to_string())) +} + +pub(crate) fn validate_unit_user(name: &str, user: &str) -> Result<()> { + parse_unit_user(name, user).map(|_| ()) +} + +/// Validates an `open_file` path before it reaches a systemd unit property. /// /// systemd parses `OpenFile=` as `path:fdname:options` and expands `%` /// specifiers, so those characters would change the meaning of the property /// rather than name a device. The check is deliberately conservative: the only /// intended values are host device nodes such as `/dev/tap7498`. -/// Validates a user name before it reaches a systemd unit property. -/// -/// `User=` takes a user name or a numeric UID. The charset is kept to what a -/// POSIX user name can contain so the value cannot introduce a `%` specifier -/// expansion or extra property syntax. -pub fn validate_unit_user(name: &str, user: &str) -> Result<()> { - anyhow::ensure!(!user.is_empty(), "{name} must not be empty"); - anyhow::ensure!( - !user.starts_with('-'), - "{name} must not start with '-': {user}" - ); - anyhow::ensure!( - user.bytes() - .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'-' | b'.')), - "{name} must contain only alphanumerics, '_', '-' and '.': {user}" - ); - Ok(()) -} - -pub fn validate_open_file(name: &str, path: &str) -> Result<()> { - anyhow::ensure!( - path.starts_with('/'), - "{name} must be an absolute path: {path}" - ); - anyhow::ensure!( - path.bytes() - .all(|byte| byte.is_ascii_graphic() && !matches!(byte, b':' | b',' | b'%' | b'\\')), - "{name} must not contain whitespace or any of ':' ',' '%' '\\': {path}" - ); +pub(crate) fn validate_open_file(name: &str, path: &str) -> Result<()> { + if !path.starts_with('/') { + bail!("{name} must be an absolute path: {path}"); + } + if !path + .bytes() + .all(|byte| byte.is_ascii_graphic() && !matches!(byte, b':' | b',' | b'%' | b'\\')) + { + bail!("{name} must not contain whitespace or any of ':' ',' '%' '\\': {path}"); + } Ok(()) } @@ -1236,7 +1287,34 @@ mod tests { #[test] fn unit_user_names_are_validated() { validate_unit_user("cvm.user", "qemu-1.user_x").unwrap(); - for user in ["", "-qemu", "qemu:0", "qemu user", "%i", "qemu$"] { + assert_eq!( + parse_unit_user("cvm.user", "#1000").unwrap(), + UnitUser::Uid(1000) + ); + assert_eq!( + parse_unit_user("cvm.user", "1000").unwrap(), + UnitUser::Uid(1000) + ); + assert_eq!( + parse_unit_user("cvm.user", "#1000").unwrap().sudo_value(), + "#1000" + ); + assert_eq!( + parse_unit_user("cvm.user", "#1000") + .unwrap() + .systemd_value(), + "1000" + ); + for user in [ + "", + "#", + "#-1", + "-qemu", + "qemu:0", + "qemu user", + "%i", + "qemu$", + ] { validate_unit_user("cvm.user", user).unwrap_err(); } @@ -1247,6 +1325,8 @@ mod tests { .unwrap_err() .to_string() .contains("cvm.user")); + config.cvm.user = "#1000".into(); + config.validate().unwrap(); } #[test] diff --git a/dstack/vmm/src/process_manager.rs b/dstack/vmm/src/process_manager.rs index 5b6e2b752..7ffb7941e 100644 --- a/dstack/vmm/src/process_manager.rs +++ b/dstack/vmm/src/process_manager.rs @@ -15,7 +15,7 @@ use tokio::process::Command; use tokio::sync::RwLock; use tracing::warn; -use crate::config::{validate_open_file, validate_unit_user}; +use crate::config::{parse_unit_user, validate_open_file}; #[derive(Clone)] pub enum ProcessManager { @@ -416,15 +416,18 @@ impl SystemdProcessManager { args.push(format!("--setenv={key}={value}")); } if !config.user.is_empty() { - validate_unit_user("user", &config.user)?; - args.push(format!("--property=User={}", config.user)); + // ProcessConfig.user is already normalized to a systemd User= + // value (name or bare UID). Re-parse to reject anything that + // would still inject property syntax. + let user = parse_unit_user("user", &config.user)?; + args.push(format!("--property=User={}", user.systemd_value())); } // systemd opens these before exec and passes them in declaration // order starting at fd 3, which is what the QEMU netdev arguments // reference. No fdname and no `graceful` option: a missing device must // fail the unit instead of shifting every later descriptor by one. // - // systemd.exec(5): "The file or socket is opened by the service + // systemd.service(5): "The file or socket is opened by the service // manager and the file descriptor is passed to the service." The open // therefore happens with the manager's privileges, before the `User=` // drop that lands just before exec, so a root-owned chardev such as @@ -685,7 +688,12 @@ mod tests { .iter() .any(|arg| arg.contains("User="))); - for user in ["qemu:0", "qemu user", "%i", "-qemu"] { + let args = manager + .run_args(&test_config_as("1000", &[]), "unit.service") + .unwrap(); + assert!(args.iter().any(|arg| arg == "--property=User=1000")); + + for user in ["qemu:0", "qemu user", "%i", "-qemu", "#"] { manager .run_args(&test_config_as(user, &[]), "unit.service") .unwrap_err(); From f66f609ef0734e6763e241a83ffb4d65e11de230 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Thu, 13 Aug 2026 00:10:48 +0800 Subject: [PATCH 07/20] fix(vmm): chown swtpm state under systemd User= --- dstack/vmm/Cargo.toml | 2 +- dstack/vmm/src/app/qemu.rs | 32 +++++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/dstack/vmm/Cargo.toml b/dstack/vmm/Cargo.toml index 941802b05..da08ca72f 100644 --- a/dstack/vmm/Cargo.toml +++ b/dstack/vmm/Cargo.toml @@ -25,7 +25,7 @@ sha2.workspace = true hex.workspace = true fs-err.workspace = true getrandom = { workspace = true, features = ["std"] } -nix = { workspace = true, features = ["user"] } +nix = { workspace = true, features = ["fs", "user"] } dirs.workspace = true which.workspace = true clap = { workspace = true, features = ["derive", "string"] } diff --git a/dstack/vmm/src/app/qemu.rs b/dstack/vmm/src/app/qemu.rs index 2f2a825fb..9161f178d 100644 --- a/dstack/vmm/src/app/qemu.rs +++ b/dstack/vmm/src/app/qemu.rs @@ -28,7 +28,7 @@ use anyhow::{bail, Context, Result}; use bon::Builder; use dstack_types::shared_filenames::HOST_SHARED_DISK_LABEL; use fs_err as fs; -use nix::unistd::{Uid, User}; +use nix::unistd::{chown, Uid, User}; use serde::Serialize; use std::collections::HashMap; use std::os::unix::fs::PermissionsExt; @@ -246,6 +246,14 @@ impl PreparedQemuLaunch { .context("tpm key provider requested but swtpm is not installed")?; let state_dir = workdir.swtpm_state_dir(); fs::create_dir_all(&state_dir).context("failed to create swtpm state directory")?; + // systemd drops privileges for the whole launcher unit, including + // swtpm. Hand the state directory over so socket creation and TPM + // state updates are not denied on a root-owned path. Existing + // files from earlier root-owned boots are included. + if !cfg.user.is_empty() && cfg.pm != ProcessManagerBackend::Supervisor { + let user = resolve_cvm_user(&cfg.user)?; + chown_tree_to_user(&state_dir, &user)?; + } let socket = workdir.swtpm_socket(); if socket.exists() { fs::remove_file(&socket).context("failed to remove stale swtpm socket")?; @@ -323,6 +331,28 @@ pub(crate) fn resolve_cvm_user(user: &str) -> Result { } } +/// Makes `path` and its contents writable by the unprivileged VM user. +/// +/// Under systemd the transient unit drops privileges before exec, so paths the +/// VMM created as root must be handed over before launch. Supervisor keeps +/// root for the launcher/swtpm path and only sudo's QEMU, so it does not need +/// this. +fn chown_tree_to_user(path: &Path, user: &User) -> Result<()> { + chown(path, Some(user.uid), Some(user.gid)) + .with_context(|| format!("failed to chown {}", path.display()))?; + if !path.is_dir() { + return Ok(()); + } + for entry in fs::read_dir(path) + .with_context(|| format!("failed to read directory {}", path.display()))? + { + let entry = + entry.with_context(|| format!("failed to read entry under {}", path.display()))?; + chown_tree_to_user(&entry.path(), user)?; + } + Ok(()) +} + fn prepare_shared_dir(workdir: &VmWorkDir) -> Result<()> { let shared_dir = workdir.shared_dir(); if !shared_dir.exists() { From bf9f80fb5927138cb385df748ea4452b6c838089 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Thu, 13 Aug 2026 00:10:48 +0800 Subject: [PATCH 08/20] fix(vmm): stop open_file from inheriting host custom netdev --- dstack/vmm/src/app/network.rs | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/dstack/vmm/src/app/network.rs b/dstack/vmm/src/app/network.rs index 72cdb9c17..b8aee06cf 100644 --- a/dstack/vmm/src/app/network.rs +++ b/dstack/vmm/src/app/network.rs @@ -30,12 +30,15 @@ pub(crate) fn resolve_networking(networking: &Networking, cfg: &CvmConfig) -> Ne if !networking.dhcp_start.is_empty() { resolved.dhcp_start = networking.dhcp_start.clone(); } - if !networking.netdev.is_empty() { - resolved.netdev = networking.netdev.clone(); - } // Not merged from the host defaults: a pre-opened chardev names one - // device and belongs to exactly one NIC. + // device and belongs to exactly one NIC. When set, it also owns the + // netdev string (generated later from the fd number), so even an empty + // NIC netdev must replace a host-wide custom default. resolved.open_file = networking.open_file.clone(); + let replace_netdev = !networking.open_file.is_empty() || !networking.netdev.is_empty(); + if replace_netdev { + resolved.netdev = networking.netdev.clone(); + } resolved } @@ -198,6 +201,24 @@ mod tests { validate_resolved_network(&with_netdev).unwrap_err(); } + #[test] + fn open_file_does_not_inherit_host_custom_netdev() { + use rocket::figment::{providers::Format, providers::Toml, Figment}; + + let mut cfg: crate::config::Config = + Figment::from(Toml::string(crate::config::DEFAULT_CONFIG)) + .extract() + .unwrap(); + cfg.cvm.networking.mode = NetworkingMode::Custom; + cfg.cvm.networking.netdev = "tap,id=net0,ifname=legacy,script=no".into(); + + let nic = open_file_network("/dev/tap7498"); + let resolved = super::resolve_networking(&nic, &cfg.cvm); + assert_eq!(resolved.open_file, "/dev/tap7498"); + assert!(resolved.netdev.is_empty()); + validate_resolved_network(&resolved).unwrap(); + } + #[test] fn primary_mac_keeps_legacy_derivation_and_later_nics_are_distinct() { assert_eq!( From fd1b8b3c7c336f67d678a1a497298164ed60b276 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Thu, 13 Aug 2026 00:10:48 +0800 Subject: [PATCH 09/20] fix(vmm): omit empty open_file from networking json --- dstack/vmm/src/config.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/dstack/vmm/src/config.rs b/dstack/vmm/src/config.rs index f20ab3358..63a95068a 100644 --- a/dstack/vmm/src/config.rs +++ b/dstack/vmm/src/config.rs @@ -963,7 +963,7 @@ pub struct Networking { /// rejected on every other launch path instead of being silently dropped: /// QEMU would otherwise open an unrelated fd and attach the guest to the /// wrong network. - #[serde(default)] + #[serde(default, skip_serializing_if = "String::is_empty")] pub open_file: String, } @@ -1329,6 +1329,22 @@ mod tests { config.validate().unwrap(); } + #[test] + fn empty_open_file_is_omitted_from_json() { + let networking = Networking { + mode: NetworkingMode::User, + bridge: String::new(), + mac_prefix: String::new(), + net: String::new(), + dhcp_start: String::new(), + restrict: false, + netdev: String::new(), + open_file: String::new(), + }; + let value = serde_json::to_value(&networking).unwrap(); + assert!(value.get("open_file").is_none()); + } + #[test] fn config_validation_rejects_invalid_static_invariants() { let mut config = default_config(); From c2d970a345fd188430e4e11232d9c7e409929ee2 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Thu, 13 Aug 2026 00:10:48 +0800 Subject: [PATCH 10/20] fix(vmm): clarify one-shot dry-run limits for open_file and user --- dstack/vmm/src/one_shot.rs | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/dstack/vmm/src/one_shot.rs b/dstack/vmm/src/one_shot.rs index deb15f31d..09b13896a 100644 --- a/dstack/vmm/src/one_shot.rs +++ b/dstack/vmm/src/one_shot.rs @@ -290,13 +290,12 @@ Compose file content (first 200 chars): ); } - if !dry_run - && resolved_networks(&manifest, &config.cvm) - .iter() - .any(|network| !network.open_file.is_empty()) - { + let needs_open_files = resolved_networks(&manifest, &config.cvm) + .iter() + .any(|network| !network.open_file.is_empty()); + if !dry_run && needs_open_files { anyhow::bail!( - "one-shot execution cannot pass pre-opened file descriptors to QEMU; run the VMM server with cvm.pm = \"systemd\" or use --dry-run" + "one-shot execution cannot pass pre-opened file descriptors to QEMU; run the VMM server with cvm.pm = \"systemd\"" ); } @@ -322,21 +321,35 @@ Compose file content (first 200 chars): println!("# QEMU Command:"); println!("{}", full_command.join(" ")); - if !dry_run && !process_config.user.is_empty() { + let needs_systemd_user = !process_config.user.is_empty(); + if !dry_run && needs_systemd_user { // Privileges are dropped by the systemd unit, which one-shot mode does // not create, and the command carries no sudo prefix either. Running it // here would start QEMU with the VMM's own privileges. anyhow::bail!( - "one-shot execution cannot drop privileges to cvm.user with cvm.pm = \"systemd\" or \"auto\"; use --dry-run or cvm.pm = \"supervisor\"" + "one-shot execution cannot drop privileges to cvm.user with cvm.pm = \"systemd\" or \"auto\"; use cvm.pm = \"supervisor\" or run the VMM server" ); } if dry_run { println!("# Dry run mode - QEMU command not executed"); - println!( - "# To execute, run: --one-shot {} (without --dry-run)", - vm_config_path - ); + if needs_open_files { + println!( + "# This command needs pre-opened file descriptors from systemd OpenFile=; \ + run the VMM server with cvm.pm = \"systemd\" instead of removing --dry-run" + ); + } else if needs_systemd_user { + println!( + "# This command expects systemd User={}; run the VMM server with cvm.pm = \"systemd\" \ + or \"auto\", or set cvm.pm = \"supervisor\" so one-shot can use sudo", + process_config.user + ); + } else { + println!( + "# To execute, run: --one-shot {} (without --dry-run)", + vm_config_path + ); + } } else { println!("# Executing QEMU..."); From 9dfa196a0b8be36ea2677a2556b8f8f1ffa26f23 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Thu, 13 Aug 2026 00:10:48 +0800 Subject: [PATCH 11/20] docs: document OpenFile and User on the systemd process manager --- docs/experimental-systemd-vm-processes.md | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/docs/experimental-systemd-vm-processes.md b/docs/experimental-systemd-vm-processes.md index e2a7549c1..4fd693426 100644 --- a/docs/experimental-systemd-vm-processes.md +++ b/docs/experimental-systemd-vm-processes.md @@ -57,8 +57,20 @@ KillSignal=SIGTERM SendSIGKILL=yes TimeoutStopSec= Restart=no +User= # when cvm.user is set +OpenFile=/dev/tapN # one entry per NIC with networking.open_file ``` +`User=` replaces the Supervisor `sudo -u` path. systemd opens each `OpenFile=` +path with the manager's privileges and hands the descriptors to the service in +declaration order starting at fd 3, which is what QEMU's generated +`-netdev tap,id=netN,fd=M` arguments expect. That combination needs systemd +253 or newer. + +When both are set, the chardev stays root-owned on the host and the unit still +runs QEMU unprivileged. For software-TPM VMs the whole launcher unit runs as +`cvm.user`, so the VMM chowns the swtpm state directory before start. + The existing launcher remains responsible for swtpm readiness and graceful child shutdown. systemd owns the final cgroup lifetime. A stop request is submitted asynchronously so the VMM can report a VM as stopping while QEMU is @@ -88,8 +100,10 @@ atomic property handling and event-driven state updates. ## Limitations -- The host must run systemd with support for `ExitType=cgroup` and - `StandardOutput=append:`. +- The host must run systemd 253+ with support for `OpenFile=`, + `ExitType=cgroup`, and `StandardOutput=append:`. +- `networking.open_file` is manifest-only, requires `mode = "custom"`, and is + rejected with Supervisor, one-shot execution, and swtpm-backed VMs. - The VMM must be authorized to create and stop system services. - Transient services inherit the systemd manager environment rather than the VMM environment. Variables in `ProcessConfig.env` are forwarded; unrelated From 8cd77b2af6d7208c3ac1de43941d596459ef2765 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Thu, 13 Aug 2026 01:47:40 +0800 Subject: [PATCH 12/20] fix(vmm): stop swtpm chown from following symlinks chown_tree_to_user handed the swtpm state directory to the unprivileged cvm.user before a systemd-managed launch using chown(2) plus Path::is_dir(), both of which follow symlinks, and recursed through fs::read_dir. The state directory is owned by (and writable by) cvm.user between boots, so a symlink planted there was followed on the next launch, letting code already running as cvm.user redirect a root chown onto an arbitrary host path (CWE-59) -- an escalation along the exact path the privilege drop is meant to contain. Walk the tree without following symlinks: fchownat the top path with AT_SYMLINK_NOFOLLOW, then descend only through directories opened with O_NOFOLLOW|O_DIRECTORY, performing every chown and every openat relative to the trusted directory fd instead of by re-resolving a path. Operating relative to an already-opened fd also closes the TOCTOU where an intermediate path component is swapped for a symlink between check and use. Add tests that plant a dangling symlink and a symlinked directory in the tree and assert the walk succeeds by chowning the link itself rather than chasing the missing target; both fail against the previous implementation. Enable the nix "dir" feature for the directory-fd traversal. --- dstack/vmm/Cargo.toml | 2 +- dstack/vmm/src/app/qemu.rs | 132 +++++++++++++++++++++++++++++++++---- 2 files changed, 122 insertions(+), 12 deletions(-) diff --git a/dstack/vmm/Cargo.toml b/dstack/vmm/Cargo.toml index da08ca72f..4efbfc1b1 100644 --- a/dstack/vmm/Cargo.toml +++ b/dstack/vmm/Cargo.toml @@ -25,7 +25,7 @@ sha2.workspace = true hex.workspace = true fs-err.workspace = true getrandom = { workspace = true, features = ["std"] } -nix = { workspace = true, features = ["fs", "user"] } +nix = { workspace = true, features = ["fs", "user", "dir"] } dirs.workspace = true which.workspace = true clap = { workspace = true, features = ["derive", "string"] } diff --git a/dstack/vmm/src/app/qemu.rs b/dstack/vmm/src/app/qemu.rs index 9161f178d..7198b8796 100644 --- a/dstack/vmm/src/app/qemu.rs +++ b/dstack/vmm/src/app/qemu.rs @@ -28,9 +28,14 @@ use anyhow::{bail, Context, Result}; use bon::Builder; use dstack_types::shared_filenames::HOST_SHARED_DISK_LABEL; use fs_err as fs; -use nix::unistd::{chown, Uid, User}; +use nix::dir::Dir; +use nix::errno::Errno; +use nix::fcntl::{openat, AtFlags, OFlag}; +use nix::sys::stat::Mode; +use nix::unistd::{fchownat, Uid, User}; use serde::Serialize; use std::collections::HashMap; +use std::os::fd::{AsRawFd, RawFd}; use std::os::unix::fs::PermissionsExt; use std::{ fs::Permissions, @@ -331,24 +336,80 @@ pub(crate) fn resolve_cvm_user(user: &str) -> Result { } } -/// Makes `path` and its contents writable by the unprivileged VM user. +const DIR_OFLAGS: OFlag = OFlag::O_RDONLY + .union(OFlag::O_NOFOLLOW) + .union(OFlag::O_DIRECTORY) + .union(OFlag::O_CLOEXEC); + +/// Makes `path` and its contents owned by the unprivileged VM user. /// /// Under systemd the transient unit drops privileges before exec, so paths the /// VMM created as root must be handed over before launch. Supervisor keeps /// root for the launcher/swtpm path and only sudo's QEMU, so it does not need /// this. +/// +/// The walk never follows a symlink: every entry is chowned and every descent +/// happens relative to an `O_NOFOLLOW`-opened directory fd. The state directory +/// is writable by the unprivileged user between boots, so a symlink planted +/// there must not be able to redirect a root chown onto an arbitrary host path +/// (CWE-59). fn chown_tree_to_user(path: &Path, user: &User) -> Result<()> { - chown(path, Some(user.uid), Some(user.gid)) - .with_context(|| format!("failed to chown {}", path.display()))?; - if !path.is_dir() { - return Ok(()); - } - for entry in fs::read_dir(path) - .with_context(|| format!("failed to read directory {}", path.display()))? - { + // Chown the top path itself without following a symlink. + fchownat( + None, + path, + Some(user.uid), + Some(user.gid), + AtFlags::AT_SYMLINK_NOFOLLOW, + ) + .with_context(|| format!("failed to chown {}", path.display()))?; + + // Open the directory itself without following symlinks. A non-directory or + // a symlink has nothing to descend into and was already chowned above. + let dir_fd = match openat(None, path, DIR_OFLAGS, Mode::empty()) { + Ok(fd) => fd, + Err(Errno::ENOTDIR | Errno::ELOOP) => return Ok(()), + Err(err) => return Err(err).with_context(|| format!("failed to open {}", path.display())), + }; + chown_dir_contents(dir_fd, path, user) +} + +/// Chowns every entry reachable through `dir_fd`, taking ownership of it (the +/// fd is closed when the `Dir` drops). Every chown and descent is performed +/// relative to the trusted fd rather than by re-resolving a path, so a symlink +/// anywhere below `path` cannot redirect the walk outside the tree. +fn chown_dir_contents(dir_fd: RawFd, path: &Path, user: &User) -> Result<()> { + let mut dir = Dir::from_fd(dir_fd) + .with_context(|| format!("failed to read directory {}", path.display()))?; + let raw = dir.as_raw_fd(); + for entry in dir.iter() { let entry = entry.with_context(|| format!("failed to read entry under {}", path.display()))?; - chown_tree_to_user(&entry.path(), user)?; + let name = entry.file_name(); + let bytes = name.to_bytes(); + if bytes == b"." || bytes == b".." { + continue; + } + // Chown the entry relative to the trusted dir fd, never following a + // symlink, so a planted link cannot redirect the chown to its target. + fchownat( + Some(raw), + name, + Some(user.uid), + Some(user.gid), + AtFlags::AT_SYMLINK_NOFOLLOW, + ) + .with_context(|| format!("failed to chown entry under {}", path.display()))?; + // Descend only into a real subdirectory, opened without following + // symlinks. ELOOP means the entry is a symlink; ENOTDIR a regular file. + match openat(Some(raw), name, DIR_OFLAGS, Mode::empty()) { + Ok(child) => chown_dir_contents(child, path, user)?, + Err(Errno::ENOTDIR | Errno::ELOOP) => {} + Err(err) => { + return Err(err) + .with_context(|| format!("failed to open entry under {}", path.display())) + } + } } Ok(()) } @@ -1457,4 +1518,53 @@ mod tests { assert_eq!(process.user, "1000"); assert!(!process.args.iter().any(|arg| arg == "sudo")); } + + fn chown_test_user() -> nix::unistd::User { + // The swtpm chown must succeed against a real account. Targeting the + // current uid keeps the chown a permitted no-op whether the suite runs + // as root or unprivileged, so the tests below assert traversal shape, + // not privilege. + nix::unistd::User::from_uid(nix::unistd::Uid::current()) + .unwrap() + .expect("current uid resolves to a user") + } + + #[test] + fn chown_tree_does_not_follow_a_symlink() { + // A symlink whose target does not exist must be chowned as the link + // itself. Following it would chase the missing target and fail — the + // exact primitive that let a planted link redirect a root chown onto + // an arbitrary host path. + let dir = tempfile::tempdir().unwrap(); + std::os::unix::fs::symlink("/nonexistent/dstack-chown-victim", dir.path().join("link")) + .unwrap(); + super::chown_tree_to_user(dir.path(), &chown_test_user()) + .expect("must chown the symlink itself, not follow it"); + } + + #[test] + fn chown_tree_does_not_descend_into_a_symlinked_dir() { + // A symlink to a directory must not be walked into: the pointed-to + // directory holds a dangling link, so descending would chase it and + // fail. + let dir = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + std::os::unix::fs::symlink("/nonexistent/inner-victim", outside.path().join("inner")) + .unwrap(); + std::os::unix::fs::symlink(outside.path(), dir.path().join("dirlink")).unwrap(); + super::chown_tree_to_user(dir.path(), &chown_test_user()) + .expect("must not descend into a symlinked directory"); + } + + #[test] + fn chown_tree_still_walks_a_real_tree() { + // Regression guard: the hardening must keep chowning real nested + // entries rather than stop at the top directory. + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir(dir.path().join("sub")).unwrap(); + std::fs::write(dir.path().join("sub/file"), b"x").unwrap(); + std::fs::write(dir.path().join("top"), b"y").unwrap(); + super::chown_tree_to_user(dir.path(), &chown_test_user()) + .expect("must chown a normal tree"); + } } From 22a100fb3bddd1edcf8b4e9a77ad89b65e4c950a Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Thu, 13 Aug 2026 09:17:16 +0800 Subject: [PATCH 13/20] refactor(vmm): make open_file/netdev handling explicit in resolve_networking The netdev merge relied on an implicit trick: it entered a `replace_netdev` branch whenever a NIC set open_file OR netdev, then assigned `networking.netdev.clone()` -- which for a pure open_file NIC is empty, so the assignment happened to clear the inherited host netdev. The condition tested open_file while the assignment only used netdev, so the clear worked by coincidence of emptiness rather than by stated intent. Split it into two mutually exclusive branches: an open_file NIC with no netdev explicitly clears the inherited host netdev (its real netdev is generated from the fd number later); a NIC with a netdev takes that netdev; neither inherits. Behaviour is unchanged across all four open_file/netdev combinations, including open_file+netdev set together: that pair is still carried through so validate_resolved_network rejects it as mutually exclusive, rather than letting open_file silently win and drop the user's netdev. --- dstack/vmm/src/app/network.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/dstack/vmm/src/app/network.rs b/dstack/vmm/src/app/network.rs index b8aee06cf..e19d4942a 100644 --- a/dstack/vmm/src/app/network.rs +++ b/dstack/vmm/src/app/network.rs @@ -30,15 +30,21 @@ pub(crate) fn resolve_networking(networking: &Networking, cfg: &CvmConfig) -> Ne if !networking.dhcp_start.is_empty() { resolved.dhcp_start = networking.dhcp_start.clone(); } - // Not merged from the host defaults: a pre-opened chardev names one - // device and belongs to exactly one NIC. When set, it also owns the - // netdev string (generated later from the fd number), so even an empty - // NIC netdev must replace a host-wide custom default. + // A pre-opened chardev names one host device and belongs to exactly one + // NIC, so it is taken from the NIC verbatim and never inherited from the + // host defaults. resolved.open_file = networking.open_file.clone(); - let replace_netdev = !networking.open_file.is_empty() || !networking.netdev.is_empty(); - if replace_netdev { + if !networking.open_file.is_empty() && networking.netdev.is_empty() { + // The chardev generates this NIC's netdev from the fd number later, so + // drop any inherited host netdev rather than leak a stale default. + resolved.netdev = String::new(); + } else if !networking.netdev.is_empty() { + // An explicit NIC netdev overrides the host default. A netdev set + // together with open_file is kept here, not dropped, so that + // validate_resolved_network rejects the conflicting pair. resolved.netdev = networking.netdev.clone(); } + // Neither set: inherit the host netdev unchanged. resolved } From bb73be03cec40fad4f8e83c993dc812ee62a354c Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Sat, 15 Aug 2026 00:13:50 +0800 Subject: [PATCH 14/20] feat(vmm): drive custom networking over the VM RPC NetworkingConfig only carried mode and bridge_name, and the RPC rejected "custom" outright, so a tap created by an external net daemon could only be attached by hand-editing vm-manifest.json. That edit bypasses the VMM's in-memory VM state, so it takes effect only after the whole VMM service restarts -- the failure mode we hit in production, where a manifest already said bridge while every start still rendered slirp. Carry netdev and open_file through the proto instead, and let CreateVm and UpdateVm resolve them with the same validation the manifest path already uses. A netdev or open_file that arrives with the wrong mode is rejected rather than dropped: silently ignoring it would attach the guest to the node default network while the caller believes it asked for its own. open_file additionally fails fast on a Supervisor node, because only systemd can hand a descriptor to QEMU -- the launcher already refuses this, and rejecting it here keeps a VM from being stored in a shape that can only fail at start time. Updating networking on a running VM now errors. QEMU fixes its netdev at exec, so persisting a manifest the running guest does not use would report success for a network that did not change, and the VMM keeps no pending state that would let it apply the change later. GetInfo reports netdev and open_file per mode, so a configuration round trip keeps only the fields its mode accepts. vmm-cli gains `update --net user|bridge[:]|default` for manual recovery; custom mode stays on the RPC, where the caller supplies the device path. --- dstack/vmm/rpc/proto/vmm_rpc.proto | 18 +++- dstack/vmm/src/app/vm_info.rs | 13 +++ dstack/vmm/src/main_service.rs | 153 ++++++++++++++++++++++++----- dstack/vmm/src/vmm-cli.py | 33 +++++++ 4 files changed, 191 insertions(+), 26 deletions(-) diff --git a/dstack/vmm/rpc/proto/vmm_rpc.proto b/dstack/vmm/rpc/proto/vmm_rpc.proto index 734086e2e..24269b8ff 100644 --- a/dstack/vmm/rpc/proto/vmm_rpc.proto +++ b/dstack/vmm/rpc/proto/vmm_rpc.proto @@ -126,10 +126,18 @@ message VmConfiguration { // Per-VM networking configuration. message NetworkingConfig { - // Networking mode: "bridge", "user" + // Networking mode: "bridge", "user", or "custom" string mode = 1; - // Per-VM bridge interface name. Empty = node default bridge. + // Per-VM bridge interface name. Empty = node default bridge. Bridge mode only. string bridge_name = 2; + // Explicit QEMU netdev string. Custom mode only, mutually exclusive with + // open_file. + string netdev = 3; + // Absolute path to a tap character device an external net daemon already + // created, e.g. "/dev/tap7498". The process manager opens it before exec and + // QEMU inherits it as a file descriptor. Custom mode only, mutually exclusive + // with netdev, and only the systemd process manager can pass the descriptor. + string open_file = 4; } // Requested GPU layout for a CVM. @@ -186,7 +194,11 @@ message UpdateVmRequest { optional string image = 17; // Disable or re-enable TEE for an existing VM. optional bool no_tee = 18; - // Optional update networking. + // Optional update networking. Rejected while the VM is running: QEMU fixes + // its netdev at exec, so stop the VM, update, then start it. This RPC is the + // supported way to change networking; editing vm-manifest.json on disk + // bypasses the VMM's in-memory state and only takes effect once the whole + // VMM service restarts. bool update_networking = 19; // Networking list. Empty + update_networking=true resets to node default. repeated NetworkingConfig networks = 20; diff --git a/dstack/vmm/src/app/vm_info.rs b/dstack/vmm/src/app/vm_info.rs index 79d1a78b9..59419ff5f 100644 --- a/dstack/vmm/src/app/vm_info.rs +++ b/dstack/vmm/src/app/vm_info.rs @@ -50,6 +50,7 @@ fn networking_backend_name(mode: NetworkingMode) -> &'static str { } fn networking_to_proto(networking: &Networking) -> pb::NetworkingConfig { + let is_custom = networking.mode == NetworkingMode::Custom; pb::NetworkingConfig { mode: networking_mode_name(networking.mode).into(), bridge_name: if networking.mode == NetworkingMode::Bridge { @@ -57,6 +58,18 @@ fn networking_to_proto(networking: &Networking) -> pb::NetworkingConfig { } else { String::new() }, + // Reported per mode so the round trip of a configuration keeps only the + // fields that mode actually accepts. + netdev: if is_custom { + networking.netdev.clone() + } else { + String::new() + }, + open_file: if is_custom { + networking.open_file.clone() + } else { + String::new() + }, } } diff --git a/dstack/vmm/src/main_service.rs b/dstack/vmm/src/main_service.rs index d8daedfe0..402678786 100644 --- a/dstack/vmm/src/main_service.rs +++ b/dstack/vmm/src/main_service.rs @@ -28,7 +28,7 @@ use crate::app::{ needs_swtpm, resolve_networking, validate_resolved_network, validate_resolved_networks, App, AttachMode, GpuConfig, GpuSpec, Manifest, PortMapping, VmWorkDir, }; -use crate::config::{CvmConfig, Networking, NetworkingMode}; +use crate::config::{CvmConfig, Networking, NetworkingMode, ProcessManagerBackend}; fn hex_sha256(data: &str) -> String { use sha2::Digest; @@ -351,17 +351,26 @@ fn resolve_volume_source(base: &Path, source: &str) -> Result { fn networking_from_proto(proto: &rpc::NetworkingConfig) -> Result> { let bridge = proto.bridge_name.trim().to_string(); + let netdev = proto.netdev.trim().to_string(); + let open_file = proto.open_file.trim().to_string(); + let custom_fields_set = !netdev.is_empty() || !open_file.is_empty(); let mode = match proto.mode.as_str() { "bridge" => NetworkingMode::Bridge, "user" => NetworkingMode::User, - "" if bridge.is_empty() => return Ok(None), - "" => bail!("networking mode is required when bridge is set"), - "custom" => bail!("custom networking mode is manifest-only"), + "custom" => NetworkingMode::Custom, + "" if bridge.is_empty() && !custom_fields_set => return Ok(None), + "" => bail!("networking mode is required when bridge, netdev, or open_file is set"), other => bail!("unsupported networking mode '{other}'"), }; if mode != NetworkingMode::Bridge && !bridge.is_empty() { bail!("bridge_name is only valid for bridge networking mode"); } + // Rejected rather than dropped: a netdev or open_file that silently went + // nowhere would attach the guest to the node default network while the + // caller believes it asked for its own. + if mode != NetworkingMode::Custom && custom_fields_set { + bail!("netdev and open_file are only valid for custom networking mode"); + } Ok(Some(Networking { mode, bridge, @@ -369,10 +378,8 @@ fn networking_from_proto(proto: &rpc::NetworkingConfig) -> Result Result> { + for networking in networks { + ensure_open_file_supported(&networking.open_file, cvm_config)?; + } let resolved = networks .iter() .map(|networking| resolve_networking(networking, cvm_config)) @@ -400,6 +410,21 @@ fn resolve_requested_networks( Ok(resolved) } +/// Rejects a pre-opened chardev on a node that cannot hand one to QEMU. +/// +/// Only systemd passes file descriptors, so Supervisor would launch QEMU +/// against whatever fd number happens to be free and attach the guest to an +/// unrelated network. The launcher already refuses this; failing here means the +/// VM is never stored in a shape that can only fail at start time. +fn ensure_open_file_supported(open_file: &str, cvm_config: &CvmConfig) -> Result<()> { + if open_file.is_empty() || cvm_config.pm != ProcessManagerBackend::Supervisor { + return Ok(()); + } + bail!( + "networking.open_file requires the systemd process manager; set cvm.pm = \"systemd\" or \"auto\"" + ) +} + fn has_host_bridge_interface() -> bool { let Ok(entries) = fs::read_dir("/sys/class/net") else { return false; @@ -717,14 +742,19 @@ impl VmmRpc for RpcHandler { .info(&request.id) .await? .is_some_and(|info| info.state.status.is_running()); - if !is_running { - let runtime_networks = vm_work_dir.runtime_networks(); - self.app - .remove_filtered_networks(&request.id, &runtime_networks) - .await - .context("failed to remove previous filtered networking")?; - vm_work_dir.clear_runtime_networks()?; + // QEMU's netdev is fixed at exec, so accepting this for a running + // VM would persist a manifest the running guest does not use and + // report success for a network that did not change. The VMM keeps + // no pending state, so the caller stops the VM first. + if is_running { + bail!("networking can only be updated while the VM is stopped"); } + let runtime_networks = vm_work_dir.runtime_networks(); + self.app + .remove_filtered_networks(&request.id, &runtime_networks) + .await + .context("failed to remove previous filtered networking")?; + vm_work_dir.clear_runtime_networks()?; manifest.networks = networks; } let compose_file = fs::read_to_string(vm_work_dir.app_compose_path()) @@ -1223,7 +1253,7 @@ mod tests { let mut request = test_vm_configuration(); request.networks = vec![rpc::NetworkingConfig { mode: "user".to_string(), - bridge_name: String::new(), + ..Default::default() }]; let manifest = create_manifest_from_vm_config(request, &test_cvm_config()).unwrap(); @@ -1238,6 +1268,7 @@ mod tests { let err = networks_from_proto(&[rpc::NetworkingConfig { mode: "user".to_string(), bridge_name: "dstack-br0".to_string(), + ..Default::default() }]) .unwrap_err(); @@ -1246,9 +1277,59 @@ mod tests { #[test] fn repeated_networks_rejects_empty_entries() { + let err = networks_from_proto(&[rpc::NetworkingConfig::default()]).unwrap_err(); + + assert!(err.to_string().contains("networking mode is required")); + } + + #[test] + fn custom_networking_carries_netdev_and_open_file() { + let with_netdev = networks_from_proto(&[rpc::NetworkingConfig { + mode: "custom".to_string(), + netdev: "tap,id=net0,ifname=tap-a,script=no".to_string(), + ..Default::default() + }]) + .unwrap(); + assert_eq!(with_netdev[0].mode, NetworkingMode::Custom); + assert_eq!(with_netdev[0].netdev, "tap,id=net0,ifname=tap-a,script=no"); + + let with_open_file = networks_from_proto(&[rpc::NetworkingConfig { + mode: "custom".to_string(), + open_file: "/dev/tap7498".to_string(), + ..Default::default() + }]) + .unwrap(); + assert_eq!(with_open_file[0].open_file, "/dev/tap7498"); + assert!(with_open_file[0].netdev.is_empty()); + } + + #[test] + fn custom_fields_are_rejected_outside_custom_mode() { + for proto in [ + rpc::NetworkingConfig { + mode: "bridge".to_string(), + bridge_name: "dstack-br0".to_string(), + netdev: "tap,id=net0".to_string(), + ..Default::default() + }, + rpc::NetworkingConfig { + mode: "user".to_string(), + open_file: "/dev/tap7498".to_string(), + ..Default::default() + }, + ] { + let err = networks_from_proto(&[proto]).unwrap_err(); + assert!(err + .to_string() + .contains("only valid for custom networking mode")); + } + } + + #[test] + fn custom_fields_without_a_mode_are_rejected_instead_of_ignored() { let err = networks_from_proto(&[rpc::NetworkingConfig { - mode: String::new(), - bridge_name: String::new(), + open_file: "/dev/tap7498".to_string(), + ..Default::default() }]) .unwrap_err(); @@ -1256,14 +1337,40 @@ mod tests { } #[test] - fn repeated_networks_rejects_custom_entries() { - let err = networks_from_proto(&[rpc::NetworkingConfig { + fn netdev_and_open_file_are_mutually_exclusive() { + let networks = networks_from_proto(&[rpc::NetworkingConfig { mode: "custom".to_string(), - bridge_name: String::new(), + netdev: "tap,id=net0".to_string(), + open_file: "/dev/tap7498".to_string(), + ..Default::default() }]) - .unwrap_err(); + .unwrap(); + let mut config = test_cvm_config(); + config.pm = ProcessManagerBackend::Systemd; + + let err = resolve_requested_networks(&networks, &config).unwrap_err(); + + assert!(err.to_string().contains("mutually exclusive")); + } + + #[test] + fn open_file_is_rejected_on_a_supervisor_node() { + let mut config = test_cvm_config(); + config.pm = ProcessManagerBackend::Supervisor; + let networks = networks_from_proto(&[rpc::NetworkingConfig { + mode: "custom".to_string(), + open_file: "/dev/tap7498".to_string(), + ..Default::default() + }]) + .unwrap(); + + let err = resolve_requested_networks(&networks, &config).unwrap_err(); + assert!(err.to_string().contains("systemd process manager")); - assert!(err.to_string().contains("custom networking mode")); + config.pm = ProcessManagerBackend::Systemd; + assert!(ensure_open_file_supported("/dev/tap7498", &config).is_ok()); + config.pm = ProcessManagerBackend::Auto; + assert!(ensure_open_file_supported("/dev/tap7498", &config).is_ok()); } #[test] diff --git a/dstack/vmm/src/vmm-cli.py b/dstack/vmm/src/vmm-cli.py index 1a533434f..d38767a2a 100755 --- a/dstack/vmm/src/vmm-cli.py +++ b/dstack/vmm/src/vmm-cli.py @@ -1030,6 +1030,7 @@ def update_vm( no_gpus: bool = False, kms_urls: Optional[List[str]] = None, no_tee: Optional[bool] = None, + net: Optional[str] = None, ) -> None: """Update multiple aspects of a VM in one command.""" # Validate: --env-file requires --kms-url @@ -1162,6 +1163,27 @@ def update_vm( upgrade_params["kms_urls"] = kms_urls updates.append(f"KMS URLs ({len(kms_urls)})") + # Networking only takes effect at the next start, and the VMM rejects + # this for a running VM: stop the VM before updating it. + if net is not None: + upgrade_params["update_networking"] = True + if net == "default": + upgrade_params["networks"] = [] + updates.append("networking (node default)") + else: + mode, _, bridge_name = net.partition(":") + if mode not in ("user", "bridge"): + raise Exception( + f"--net must be user, bridge[:], or default, got {net!r}" + ) + if bridge_name and mode != "bridge": + raise Exception("--net bridge name is only valid for bridge mode") + networking = {"mode": mode} + if bridge_name: + networking["bridge_name"] = bridge_name + upgrade_params["networks"] = [networking] + updates.append(f"networking ({net})") + # handle port updates - only update if --port or --no-ports is specified if no_ports or ports is not None: if no_ports: @@ -1972,6 +1994,16 @@ def _patched_format_help(): help="Detach all GPUs from the VM", ) + update_parser.add_argument( + "--net", + type=str, + help=( + "Networking mode: user, bridge, bridge:, or default to " + "fall back to the node configuration. Requires a stopped VM. " + "Custom mode is set through the UpdateVm RPC, not this flag." + ), + ) + # TDX toggle tee_group = update_parser.add_mutually_exclusive_group() tee_group.add_argument( @@ -2077,6 +2109,7 @@ def _patched_format_help(): no_gpus=args.no_gpus if hasattr(args, "no_gpus") else False, kms_urls=args.kms_url, no_tee=args.no_tee, + net=args.net, ) elif args.command == "kms": if not args.kms_action: From aff93c1631cea5b121d16f197d6ce379fdce6d24 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Sat, 15 Aug 2026 00:14:34 +0800 Subject: [PATCH 15/20] feat(vmm): bound caller-named bridges with cvm.bridge_allowlist bridge_name was checked only for existence under /sys/class/net, so any caller able to reach the RPC could attach any VM to any bridge on the host, including one belonging to another tenant. The guest then joins that L2 domain, leases from its DHCP, and reaches its VMs, and no host firewall rule fires against it: those rules are keyed by bridge, and from the kernel's view the guest is a legitimate member of the wrong network. With multi-tenant VPCs on one host this is the single parameter that crosses a tenant boundary. Gate it behind cvm.bridge_allowlist_enabled with a separate cvm.bridge_allowlist of names, where a trailing `*` matches a prefix. The toggle is kept apart from the list so that "enabled with an empty list" is an explicit deny-all: folding the two together would make a mistyped list silently degrade into no check at all, which is the wrong direction for a boundary. Default off, so existing nodes keep working until an operator opts in. Only an explicitly named bridge is checked. An empty bridge_name inherits the node default, which the operator already chose, so the default does not have to appear in its own allowlist. This narrows an existing check -- the bridge must exist -- into "the bridge must be allowed". It stays a resource boundary rather than an authorization policy, so the VMM still only renders an already-resolved spec and leaves tenant authorization to the control plane. --- dstack/vmm/src/config.rs | 22 ++++++++++++++ dstack/vmm/src/main_service.rs | 54 ++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/dstack/vmm/src/config.rs b/dstack/vmm/src/config.rs index 63a95068a..6652055e0 100644 --- a/dstack/vmm/src/config.rs +++ b/dstack/vmm/src/config.rs @@ -372,6 +372,28 @@ pub struct CvmConfig { #[serde(default)] pub network_filter: NetworkFilterConfig, + /// Restricts which bridges a VM may name through `bridge_name`. + /// + /// Without it a caller can attach any VM to any existing bridge on the + /// host, including another tenant's: the guest then joins that L2 domain, + /// leases from its DHCP, and reaches its VMs. Host firewall rules are keyed + /// by bridge, so none of them fire — the guest is a legitimate member of + /// the wrong network. Off by default to keep existing nodes working; turn + /// it on wherever one host carries more than one tenant. + /// + /// This is a resource boundary, not an authorization policy: it narrows + /// "the bridge must exist" to "the bridge must be allowed", so the VMM + /// still only renders an already-resolved spec. + #[serde(default)] + pub bridge_allowlist_enabled: bool, + + /// Bridge names a VM may request when `bridge_allowlist_enabled` is set. + /// A trailing `*` matches a prefix, e.g. `vpc-*`. Kept separate from the + /// toggle so that "enabled with an empty list" is an explicit deny-all + /// instead of an ambiguous "no list means no check". + #[serde(default)] + pub bridge_allowlist: Vec, + /// Stable namespace for TAP names when several VMMs share one host. /// An empty value is derived from the absolute run directory. #[serde(default)] diff --git a/dstack/vmm/src/main_service.rs b/dstack/vmm/src/main_service.rs index 402678786..dd4ee7733 100644 --- a/dstack/vmm/src/main_service.rs +++ b/dstack/vmm/src/main_service.rs @@ -400,6 +400,7 @@ fn resolve_requested_networks( cvm_config: &CvmConfig, ) -> Result> { for networking in networks { + ensure_bridge_allowed(&networking.bridge, cvm_config)?; ensure_open_file_supported(&networking.open_file, cvm_config)?; } let resolved = networks @@ -410,6 +411,25 @@ fn resolve_requested_networks( Ok(resolved) } +/// Checks a caller-named bridge against `cvm.bridge_allowlist`. +/// +/// Only an explicitly requested bridge is checked. An empty name inherits the +/// node default, which the operator already chose, so putting the default in +/// the allowlist is not required. +fn ensure_bridge_allowed(requested: &str, cvm_config: &CvmConfig) -> Result<()> { + if !cvm_config.bridge_allowlist_enabled || requested.is_empty() { + return Ok(()); + } + if cvm_config + .bridge_allowlist + .iter() + .any(|pattern| bridge_pattern_matches(pattern, requested)) + { + return Ok(()); + } + bail!("bridge '{requested}' is not in cvm.bridge_allowlist"); +} + /// Rejects a pre-opened chardev on a node that cannot hand one to QEMU. /// /// Only systemd passes file descriptors, so Supervisor would launch QEMU @@ -425,6 +445,13 @@ fn ensure_open_file_supported(open_file: &str, cvm_config: &CvmConfig) -> Result ) } +fn bridge_pattern_matches(pattern: &str, bridge: &str) -> bool { + match pattern.strip_suffix('*') { + Some(prefix) => bridge.starts_with(prefix), + None => pattern == bridge, + } +} + fn has_host_bridge_interface() -> bool { let Ok(entries) = fs::read_dir("/sys/class/net") else { return false; @@ -1373,6 +1400,33 @@ mod tests { assert!(ensure_open_file_supported("/dev/tap7498", &config).is_ok()); } + #[test] + fn bridge_allowlist_only_applies_to_an_explicitly_named_bridge() { + let mut config = test_cvm_config(); + assert!(ensure_bridge_allowed("anything", &config).is_ok()); + + config.bridge_allowlist_enabled = true; + config.bridge_allowlist = vec!["dstack-br0".to_string(), "vpc-*".to_string()]; + + // The node default is the operator's own choice, so it stays allowed + // without being listed. + assert!(ensure_bridge_allowed("", &config).is_ok()); + assert!(ensure_bridge_allowed("dstack-br0", &config).is_ok()); + assert!(ensure_bridge_allowed("vpc-tenant-a", &config).is_ok()); + + let err = ensure_bridge_allowed("docker0", &config).unwrap_err(); + assert!(err.to_string().contains("not in cvm.bridge_allowlist")); + } + + #[test] + fn enabled_allowlist_with_no_entries_denies_every_named_bridge() { + let mut config = test_cvm_config(); + config.bridge_allowlist_enabled = true; + + assert!(ensure_bridge_allowed("dstack-br0", &config).is_err()); + assert!(ensure_bridge_allowed("", &config).is_ok()); + } + #[test] fn resolve_volume_source_rejects_escape_symlink_and_qemu_metachars() -> Result<()> { let tmp = tempfile::tempdir()?; From 9365bd06bc463b3b2cceb170a0b6760dc9a63b57 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Sat, 15 Aug 2026 00:53:57 +0800 Subject: [PATCH 16/20] feat(vmm): run the systemd backend against the user manager The systemd process manager invoked systemd-run and systemctl bare, so every call landed on the system manager, which accepts it only from root or through a polkit rule. Production runs the VMM as an unprivileged account -- prod7 has a system unit with User=phala -- so cvm.pm = "systemd" was unreachable there without granting root or writing a polkit rule whose object is a sha256-derived unit name that no sensible rule can match. That in turn blocked networking.open_file, since passing a pre-opened tap descriptor to QEMU is something only systemd can do. Add cvm.systemd.user_manager to target the caller's own manager instead. The flag is off by default, so existing nodes keep talking to the system manager. --user is placed ahead of the subcommand, because systemctl accepts `--user stop` but not `stop --user`, and XDG_RUNTIME_DIR is derived from the effective uid when absent: a VMM started as a system service inherits no session environment and would otherwise fail with a bus error that names nothing useful. cvm.user is rejected under a user manager rather than passed through. A user manager cannot change uid and ignores User=, so forwarding it would start QEMU with the VMM's own privileges right after an operator asked to confine it. Note for whoever wires this up: the user manager also changes who opens networking.open_file. The system manager opens the chardev as root before dropping to User=; the user manager opens it as the VMM's own account, so an external net daemon must create /dev/tapN owned by that account. --- dstack/vmm/src/config.rs | 17 ++++++ dstack/vmm/src/main.rs | 1 + dstack/vmm/src/process_manager.rs | 91 +++++++++++++++++++++++++++++-- 3 files changed, 103 insertions(+), 6 deletions(-) diff --git a/dstack/vmm/src/config.rs b/dstack/vmm/src/config.rs index 6652055e0..314de7432 100644 --- a/dstack/vmm/src/config.rs +++ b/dstack/vmm/src/config.rs @@ -548,6 +548,22 @@ pub struct SystemdConfig { pub state_dir: PathBuf, #[serde(default = "default_systemd_stop_timeout")] pub stop_timeout: String, + /// Drive the calling user's own systemd manager (`systemd-run --user`) + /// instead of the system manager. + /// + /// The system manager only accepts these calls from root or through a + /// polkit rule, so a VMM running as an unprivileged user cannot use the + /// systemd backend at all without one of the two. The user manager needs + /// neither, at the cost of requiring lingering for the account + /// (`loginctl enable-linger `) so the manager outlives the login + /// session, and of dropping `cvm.user`: a user manager cannot change uid. + /// + /// It also changes who opens `networking.open_file`. The system manager + /// opens the chardev as root before dropping to `User=`; the user manager + /// opens it as the VMM's own account, so the device must already be owned + /// by it. + #[serde(default)] + pub user_manager: bool, } impl Default for SystemdConfig { @@ -556,6 +572,7 @@ impl Default for SystemdConfig { unit_prefix: default_systemd_unit_prefix(), state_dir: default_systemd_state_dir(), stop_timeout: default_systemd_stop_timeout(), + user_manager: false, } } } diff --git a/dstack/vmm/src/main.rs b/dstack/vmm/src/main.rs index da640e196..47b71a671 100644 --- a/dstack/vmm/src/main.rs +++ b/dstack/vmm/src/main.rs @@ -342,6 +342,7 @@ async fn main() -> Result<()> { config.systemd.state_dir.clone(), config.systemd.unit_prefix.clone(), config.systemd.stop_timeout.clone(), + config.systemd.user_manager, ) }; let supervisor_config = &config.supervisor; diff --git a/dstack/vmm/src/process_manager.rs b/dstack/vmm/src/process_manager.rs index 7ffb7941e..946d63294 100644 --- a/dstack/vmm/src/process_manager.rs +++ b/dstack/vmm/src/process_manager.rs @@ -16,6 +16,7 @@ use tokio::sync::RwLock; use tracing::warn; use crate::config::{parse_unit_user, validate_open_file}; +use nix::unistd::geteuid; #[derive(Clone)] pub enum ProcessManager { @@ -33,11 +34,13 @@ impl ProcessManager { state_dir: PathBuf, unit_prefix: String, stop_timeout: String, + user_manager: bool, ) -> Result> { Ok(Arc::new(SystemdProcessManager::new( state_dir, unit_prefix, stop_timeout, + user_manager, )?)) } @@ -320,10 +323,16 @@ pub struct SystemdProcessManager { state_dir: PathBuf, unit_prefix: String, stop_timeout: String, + user_manager: bool, } impl SystemdProcessManager { - fn new(state_dir: PathBuf, unit_prefix: String, stop_timeout: String) -> Result { + fn new( + state_dir: PathBuf, + unit_prefix: String, + stop_timeout: String, + user_manager: bool, + ) -> Result { anyhow::ensure!( !unit_prefix.is_empty(), "systemd unit prefix must not be empty" @@ -339,9 +348,28 @@ impl SystemdProcessManager { state_dir, unit_prefix, stop_timeout, + user_manager, }) } + /// Builds a `systemd-run`/`systemctl` invocation aimed at the configured + /// manager. + /// + /// `--user` selects the caller's own manager, which is reached over the bus + /// at `$XDG_RUNTIME_DIR/bus`. A VMM started as a system service inherits no + /// session environment, so the runtime directory is filled in from the + /// effective uid rather than left to fail with an unhelpful bus error. + fn systemd_command(&self, program: &str) -> Command { + let mut command = Command::new(program); + if self.user_manager { + command.arg("--user"); + if std::env::var_os("XDG_RUNTIME_DIR").is_none() { + command.env("XDG_RUNTIME_DIR", format!("/run/user/{}", geteuid())); + } + } + command + } + fn key(id: &str) -> String { hex::encode(Sha256::digest(id.as_bytes())) } @@ -416,6 +444,13 @@ impl SystemdProcessManager { args.push(format!("--setenv={key}={value}")); } if !config.user.is_empty() { + // A user manager runs everything as its own account and silently + // ignores User=, so accepting it would start QEMU with the VMM's + // privileges after the operator asked to confine it. + anyhow::ensure!( + !self.user_manager, + "cvm.user is not supported with cvm.systemd.user_manager: a user manager cannot change uid" + ); // ProcessConfig.user is already normalized to a systemd User= // value (name or bare UID). Re-parse to reject anything that // would still inject property syntax. @@ -446,10 +481,10 @@ impl SystemdProcessManager { let unit = self.unit(&config.id); // Failed transient units remain loaded until reset and otherwise // prevent automatic restart from reusing the unit name. - let mut reset = Command::new("systemctl"); + let mut reset = self.systemd_command("systemctl"); reset.arg("reset-failed").arg(&unit); let _ = reset.output().await; - let mut command = Command::new("systemd-run"); + let mut command = self.systemd_command("systemd-run"); command.args(self.run_args(config, &unit)?); Self::command(command, "systemd-run").await?; @@ -490,7 +525,7 @@ impl SystemdProcessManager { .await? .is_some_and(|info| info.state.status.is_running()) { - let mut command = Command::new("systemctl"); + let mut command = self.systemd_command("systemctl"); command.arg("stop").arg("--no-block").arg(self.unit(id)); if let Err(error) = Self::command(command, "systemctl stop").await { // The unit may have exited and been collected between the @@ -519,7 +554,7 @@ impl SystemdProcessManager { if record.started { bail!("process is started"); } - let mut command = Command::new("systemctl"); + let mut command = self.systemd_command("systemctl"); command.arg("reset-failed").arg(self.unit(id)); let _ = command.output().await; fs_err::remove_file(self.record_path(id)).context("failed to remove process record") @@ -567,7 +602,7 @@ impl SystemdProcessManager { async fn info_from_record(&self, record: ProcessRecord) -> Result { let unit = self.unit(&record.config.id); - let mut command = Command::new("systemctl"); + let mut command = self.systemd_command("systemctl"); command .arg("show") .arg(&unit) @@ -597,6 +632,49 @@ impl SystemdProcessManager { mod tests { use super::*; + fn test_manager_with(user_manager: bool) -> (tempfile::TempDir, SystemdProcessManager) { + let dir = tempfile::tempdir().unwrap(); + let manager = SystemdProcessManager::new( + dir.path().to_path_buf(), + "dstack-vm".into(), + "infinity".into(), + user_manager, + ) + .unwrap(); + (dir, manager) + } + + #[test] + fn user_manager_selects_the_calling_users_systemd() { + let (_system_dir, system) = test_manager_with(false); + let (_user_dir, user) = test_manager_with(true); + + let args = |command: &tokio::process::Command| { + command + .as_std() + .get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>() + }; + + assert!(args(&system.systemd_command("systemctl")).is_empty()); + // Ahead of the subcommand: `systemctl --user stop` is accepted, + // `systemctl stop --user` is not. + assert_eq!(args(&user.systemd_command("systemctl")), ["--user"]); + assert_eq!(args(&user.systemd_command("systemd-run")), ["--user"]); + } + + #[test] + fn user_manager_rejects_a_dedicated_user_instead_of_ignoring_it() { + let (_dir, manager) = test_manager_with(true); + + let err = manager + .run_args(&test_config_as("qemu", &[]), "unit.service") + .unwrap_err(); + + assert!(err.to_string().contains("cannot change uid")); + } + #[test] fn unit_names_are_stable_and_do_not_embed_process_ids() { let (_dir, manager) = test_manager(); @@ -633,6 +711,7 @@ mod tests { dir.path().to_path_buf(), "dstack-vm".into(), "infinity".into(), + false, ) .unwrap(); (dir, manager) From 7edd194fb7de785136d7d5fc93fcfc78a28b8a0d Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Sat, 15 Aug 2026 02:06:26 +0800 Subject: [PATCH 17/20] test(vmm): cover open_file descriptor passing on a real user manager The unit tests only prove which arguments are rendered. Whether systemd actually opens the chardev and hands the descriptor to the child -- the entire premise of networking.open_file, where an external net daemon creates the tap and QEMU only ever receives an fd -- cannot be shown without a running service manager. Add that check as an ignored test, so a sandbox without a session bus still runs the suite clean and a host with a user manager can prove the mechanism on demand: cargo test -p dstack-vmm -- --ignored user_manager_passes_open_file It waits for the unit to exit rather than for the output file to appear. The shell redirect creates that file before the script writes anything, so watching the path reads a half-written file and the assertion fails against output that is merely incomplete. Verified on a host with a live user manager: LISTEN_FDS=1, fd 3 resolves to the passed file, and the child reads its contents. The same host refuses `systemd-run` without `--user` for an unprivileged account with "Interactive authentication required", which is what makes the user manager path necessary rather than merely convenient. --- dstack/vmm/src/process_manager.rs | 72 +++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/dstack/vmm/src/process_manager.rs b/dstack/vmm/src/process_manager.rs index 946d63294..f3f1c3d34 100644 --- a/dstack/vmm/src/process_manager.rs +++ b/dstack/vmm/src/process_manager.rs @@ -644,6 +644,78 @@ mod tests { (dir, manager) } + /// Proves that a descriptor opened by the user manager reaches the child, + /// which is the whole point of `networking.open_file`: an external net + /// daemon creates the tap device, and QEMU only ever receives the fd. + /// + /// Ignored by default because it needs a real environment -- a live systemd + /// user manager for the calling account, reachable at + /// `$XDG_RUNTIME_DIR/bus` -- which a container or CI sandbox usually lacks. + /// It writes nothing outside a temporary directory and cleans up its unit. + /// + /// Run it on a host that has one: + /// + /// ```text + /// cargo test -p dstack-vmm -- --ignored user_manager_passes_open_file + /// ``` + #[tokio::test] + #[ignore = "requires a live systemd user manager on the host"] + async fn user_manager_passes_open_file_descriptor_to_the_child() { + let dir = tempfile::tempdir().unwrap(); + let payload = dir.path().join("payload"); + fs_err::write(&payload, b"payload-through-fd").unwrap(); + let observed_path = dir.path().join("observed"); + let manager = SystemdProcessManager::new( + dir.path().join("state"), + "dstack-vm-test".into(), + "infinity".into(), + true, + ) + .unwrap(); + + let mut env = HashMap::new(); + env.insert("OBSERVED".to_string(), observed_path.display().to_string()); + let config = ProcessConfig { + env, + command: "/bin/sh".into(), + // systemd hands the file over as fd 3, the number the QEMU netdev + // arguments reference. + args: vec![ + "-c".into(), + "{ echo listen_fds=$LISTEN_FDS; cat <&3; } > $OBSERVED".into(), + ], + ..test_config(&[payload.display().to_string().as_str()]) + }; + manager.deploy(&config).await.expect("deploy"); + + // Wait for the unit to exit rather than for the file to appear: the + // shell redirect creates it before the script has written anything, so + // watching the path reads a half-written file. + let mut status = None; + for _ in 0..50 { + let info = manager.info(&config.id).await.expect("info"); + match info.map(|info| info.state.status) { + Some(ProcessStatus::Running) => { + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + other => { + status = other; + break; + } + } + } + let observed = fs_err::read_to_string(&observed_path).unwrap_or_default(); + let _ = manager.stop(&config.id).await; + let _ = manager.remove(&config.id).await; + + assert!( + matches!(status, Some(ProcessStatus::Exited(0))), + "unit ended as {status:?}, output {observed:?}" + ); + assert!(observed.contains("listen_fds=1"), "got {observed:?}"); + assert!(observed.contains("payload-through-fd"), "got {observed:?}"); + } + #[test] fn user_manager_selects_the_calling_users_systemd() { let (_system_dir, system) = test_manager_with(false); From 104846cba3187a031455a40fc4072cceaf4e8704 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Sat, 15 Aug 2026 02:23:11 +0800 Subject: [PATCH 18/20] fix(vmm): name the user manager option by its real config path The option lives in the top-level [systemd] section, not under [cvm], so the message an operator hits when cvm.user meets a user manager pointed at cvm.systemd.user_manager, a path that does not exist. The preceding commit message has the same mistake. Document the flag in vmm.toml as well: everything else in [systemd] is discoverable there, and an option that only exists in a Rust doc comment is one an operator has no way to find. The entry states the three things that bite -- lingering is required, cvm.user is refused, and open_file devices are opened as this account -- including that a fresh macvtap /dev/tapN is root-owned and has to be chowned to the VMM's account, which was confirmed on a real device rather than inferred. --- dstack/vmm/src/config.rs | 3 ++- dstack/vmm/src/process_manager.rs | 2 +- dstack/vmm/vmm.toml | 7 +++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/dstack/vmm/src/config.rs b/dstack/vmm/src/config.rs index 314de7432..bbe9bf994 100644 --- a/dstack/vmm/src/config.rs +++ b/dstack/vmm/src/config.rs @@ -561,7 +561,8 @@ pub struct SystemdConfig { /// It also changes who opens `networking.open_file`. The system manager /// opens the chardev as root before dropping to `User=`; the user manager /// opens it as the VMM's own account, so the device must already be owned - /// by it. + /// by it. A freshly created macvtap `/dev/tapN` is root-owned, so whatever + /// creates it has to chown it to the VMM's account first. #[serde(default)] pub user_manager: bool, } diff --git a/dstack/vmm/src/process_manager.rs b/dstack/vmm/src/process_manager.rs index f3f1c3d34..9c7124012 100644 --- a/dstack/vmm/src/process_manager.rs +++ b/dstack/vmm/src/process_manager.rs @@ -449,7 +449,7 @@ impl SystemdProcessManager { // privileges after the operator asked to confine it. anyhow::ensure!( !self.user_manager, - "cvm.user is not supported with cvm.systemd.user_manager: a user manager cannot change uid" + "cvm.user is not supported with systemd.user_manager: a user manager cannot change uid" ); // ProcessConfig.user is already normalized to a systemd User= // value (name or bare UID). Re-parse to reject anything that diff --git a/dstack/vmm/vmm.toml b/dstack/vmm/vmm.toml index 74826f634..57126ae8d 100644 --- a/dstack/vmm/vmm.toml +++ b/dstack/vmm/vmm.toml @@ -202,6 +202,13 @@ state_dir = "" # guests can spend hours tearing down encrypted memory, so the safe default is # unbounded. Set a systemd time span such as "30min" to enable escalation. stop_timeout = "infinity" +# Drive the calling user's own systemd manager instead of the system one. The +# system manager only accepts these calls from root or through a polkit rule, +# so an unprivileged VMM needs this to use cvm.pm = "systemd" at all. Requires +# lingering for the account (loginctl enable-linger ), rules out cvm.user +# (a user manager cannot change uid), and makes networking.open_file devices +# open as this account rather than as root, so they must be owned by it. +user_manager = false [host_api] ident = "dstack VMM" From ab59f62ffa13a8deb37d01232892c910f2d13df6 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Sat, 15 Aug 2026 10:42:17 +0800 Subject: [PATCH 19/20] docs(vmm): document the bridge allowlist in the shipped config Every other [cvm] option is discoverable in vmm.toml, so an operator looking for a way to stop tenants from naming each other's bridges had no reason to believe one exists. Spell out what the check does, why the default is off, that an empty bridge_name inherits the node default and stays allowed, and that enabled-with-an-empty-list means deny-all. --- dstack/vmm/vmm.toml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dstack/vmm/vmm.toml b/dstack/vmm/vmm.toml index 57126ae8d..e98c697d3 100644 --- a/dstack/vmm/vmm.toml +++ b/dstack/vmm/vmm.toml @@ -107,6 +107,16 @@ product_name = "dstack" # chassis_serial = "" # chassis_asset_tag = "" +# Restrict which bridges a VM may name through NetworkingConfig.bridge_name. +# Off by default. Without it any caller reaching the RPC can attach a VM to any +# bridge on the host, including another tenant's: the guest joins that L2 +# domain and no host firewall rule fires, because those rules are keyed by +# bridge and the guest is a legitimate member of the wrong network. An empty +# name inherits cvm.networking.bridge and is always allowed. A trailing "*" +# matches a prefix. Enabled with an empty list is an explicit deny-all. +bridge_allowlist_enabled = false +bridge_allowlist = [] + [cvm.networking] mode = "user" From 18c3fe6af25d57e4b7bca16f0522b594b77f2d91 Mon Sep 17 00:00:00 2001 From: Leechael Yim Date: Sat, 15 Aug 2026 12:44:38 +0800 Subject: [PATCH 20/20] feat(vmm-cli): accept the same --net syntax on deploy and update deploy only took the fixed choices bridge|user, so a VM could be created on a named bridge over the RPC but not from the CLI, while update accepted bridge:. Two spellings of the same option is the kind of gap someone finds mid-incident. Extract the parsing into parse_net_spec and use it from both. deploy now also sends the repeated networks field rather than the singular networking one: the repeated field wins over it in the VMM, and every other caller has moved. Custom mode stays out of the flag on purpose. It carries a netdev string or a device path that an external net daemon owns and created, so it belongs to that caller rather than to a hand-typed argument, and the help text on both subcommands now says where it does belong. --- dstack/vmm/src/vmm-cli.py | 56 ++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/dstack/vmm/src/vmm-cli.py b/dstack/vmm/src/vmm-cli.py index d38767a2a..161b21e29 100755 --- a/dstack/vmm/src/vmm-cli.py +++ b/dstack/vmm/src/vmm-cli.py @@ -320,6 +320,28 @@ def encrypt_env(envs, hex_public_key: str) -> str: return result.hex() +def parse_net_spec(net: str) -> Optional[dict]: + """Parse a --net value into a NetworkingConfig, or None for the node default. + + Custom mode is deliberately absent: it carries a netdev string or a device + path that an external net daemon owns, so it belongs to the caller that + created the device, not to a hand-typed flag. + """ + if net == "default": + return None + mode, _, bridge_name = net.partition(":") + if mode not in ("user", "bridge"): + raise Exception( + f"--net must be user, bridge[:], or default, got {net!r}" + ) + if bridge_name and mode != "bridge": + raise Exception("--net bridge name is only valid for bridge mode") + networking = {"mode": mode} + if bridge_name: + networking["bridge_name"] = bridge_name + return networking + + def parse_port_mapping(port_str: str) -> Dict: """Parse a port mapping string into a dictionary.""" parts = port_str.split(":") @@ -920,7 +942,10 @@ def create_vm(self, args) -> None: if args.gateway_url: params["gateway_urls"] = args.gateway_url if args.net: - params["networking"] = {"mode": args.net} + networking = parse_net_spec(args.net) + # The repeated field wins over the singular one and is the shape + # every other caller now uses. + params["networks"] = [networking] if networking else [] app_id = args.app_id or self.calc_app_id(compose_content) print(f"App ID: {app_id}") @@ -1166,23 +1191,12 @@ def update_vm( # Networking only takes effect at the next start, and the VMM rejects # this for a running VM: stop the VM before updating it. if net is not None: + networking = parse_net_spec(net) upgrade_params["update_networking"] = True - if net == "default": - upgrade_params["networks"] = [] - updates.append("networking (node default)") - else: - mode, _, bridge_name = net.partition(":") - if mode not in ("user", "bridge"): - raise Exception( - f"--net must be user, bridge[:], or default, got {net!r}" - ) - if bridge_name and mode != "bridge": - raise Exception("--net bridge name is only valid for bridge mode") - networking = {"mode": mode} - if bridge_name: - networking["bridge_name"] = bridge_name - upgrade_params["networks"] = [networking] - updates.append(f"networking ({net})") + upgrade_params["networks"] = [networking] if networking else [] + updates.append( + "networking (node default)" if networking is None else f"networking ({net})" + ) # handle port updates - only update if --port or --no-ports is specified if no_ports or ports is not None: @@ -1853,8 +1867,12 @@ def _patched_format_help(): ) deploy_parser.add_argument( "--net", - choices=["bridge", "user"], - help="Networking mode (default: use global config)", + type=str, + help=( + "Networking mode: user, bridge, bridge:, or default to " + "use the node configuration. Custom mode is set through the CreateVm " + "RPC, not this flag." + ), ) # Images command