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 diff --git a/dstack/supervisor/client/src/main.rs b/dstack/supervisor/client/src/main.rs index 16d8c61a5..b076a8ddb 100644 --- a/dstack/supervisor/client/src/main.rs +++ b/dstack/supervisor/client/src/main.rs @@ -74,6 +74,8 @@ 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 94e0c61e5..6286d0087 100644 --- a/dstack/supervisor/src/process.rs +++ b/dstack/supervisor/src/process.rs @@ -46,6 +46,25 @@ 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")] + #[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. + /// + /// 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")] + #[builder(default)] + pub open_files: Vec, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/dstack/supervisor/src/supervisor.rs b/dstack/supervisor/src/supervisor.rs index 378013c05..18d7c528d 100644 --- a/dstack/supervisor/src/supervisor.rs +++ b/dstack/supervisor/src/supervisor.rs @@ -59,6 +59,18 @@ 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 + // 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/Cargo.toml b/dstack/vmm/Cargo.toml index 941802b05..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 = ["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/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.rs b/dstack/vmm/src/app.rs index bc88f0d1e..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() { @@ -2166,6 +2162,7 @@ mod tests { dhcp_start: String::new(), restrict: false, netdev: String::new(), + open_file: String::new(), }]; workdir.put_manifest(&manifest)?; @@ -2428,6 +2425,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..e19d4942a 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(); @@ -28,9 +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(); } - if !networking.netdev.is_empty() { + // 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(); + 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 } @@ -47,6 +61,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 +94,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 +146,84 @@ 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 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() { diff --git a/dstack/vmm/src/app/qemu.rs b/dstack/vmm/src/app/qemu.rs index 67115c0fe..7198b8796 100644 --- a/dstack/vmm/src/app/qemu.rs +++ b/dstack/vmm/src/app/qemu.rs @@ -9,13 +9,17 @@ 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::{ app::Manifest, config::{ - CvmConfig, CvmPlatform, NetworkFilterMode, Networking, NetworkingMode, ProcessAnnotation, + parse_unit_user, CvmConfig, CvmPlatform, NetworkFilterMode, Networking, NetworkingMode, + ProcessAnnotation, ProcessManagerBackend, UnitUser, }, netd::{tap_name, InterfaceIdentity}, vm_launcher::{ChildCommand, LaunchSpec}, @@ -24,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::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, @@ -242,6 +251,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")?; @@ -308,6 +325,95 @@ 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")), + } +} + +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 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()))?; + 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(()) +} + fn prepare_shared_dir(workdir: &VmWorkDir) -> Result<()> { let shared_dir = workdir.shared_dir(); if !shared_dir.exists() { @@ -354,6 +460,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() @@ -361,9 +475,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()) }; @@ -414,6 +526,12 @@ 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(), }; Ok(vec![launcher]) } @@ -628,12 +746,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 +905,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 @@ -790,11 +915,29 @@ 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() { - arguments.splice( - 0..0, - ["sudo", "-u", &self.cfg.user].into_iter().map(String::from), - ); + 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", &sudo_user].into_iter().map(String::from), + ); + } else { + // systemd User= takes a bare name or decimal UID, not sudo's #UID. + user = unit_user.systemd_value(); + } } let command = arguments.remove(0); @@ -819,6 +962,8 @@ impl QemuCommandBuilder<'_> { pidfile: workdir.pid_file().to_string_lossy().to_string(), cid: Some(self.vm.cid), note, + user, + open_files, }) } } @@ -1007,7 +1152,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}; @@ -1265,5 +1411,160 @@ 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"]); + + // 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, "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/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 { + vm: &vm, + cfg: &sudo_config.cvm, + gpus: &GpuConfig::default(), + prepared: &prepared, + } + .build() + .unwrap_err(); + 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()); + + // 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")); + } + + 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("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/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("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/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"); } } 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/config.rs b/dstack/vmm/src/config.rs index f3ca78d3b..bbe9bf994 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 @@ -369,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)] @@ -523,6 +548,23 @@ 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. 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, } impl Default for SystemdConfig { @@ -531,6 +573,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, } } } @@ -727,6 +770,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(), @@ -798,6 +844,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 +865,96 @@ 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(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}"), + } + } +} + +/// 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`. +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(()) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] #[serde(rename_all = "lowercase")] pub enum NetworkingMode { @@ -849,6 +992,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, skip_serializing_if = "String::is_empty")] + pub open_file: String, } impl Networking { @@ -1157,6 +1313,78 @@ 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 unit_user_names_are_validated() { + validate_unit_user("cvm.user", "qemu-1.user_x").unwrap(); + 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(); + } + + let mut config = default_config(); + config.cvm.user = "qemu:0".into(); + assert!(config + .validate() + .unwrap_err() + .to_string() + .contains("cvm.user")); + config.cvm.user = "#1000".into(); + 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(); 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/main_service.rs b/dstack/vmm/src/main_service.rs index ef52eed4c..dd4ee7733 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,7 +378,8 @@ fn networking_from_proto(proto: &rpc::NetworkingConfig) -> Result Result> { + for networking in networks { + ensure_bridge_allowed(&networking.bridge, cvm_config)?; + ensure_open_file_supported(&networking.open_file, cvm_config)?; + } let resolved = networks .iter() .map(|networking| resolve_networking(networking, cvm_config)) @@ -397,6 +411,47 @@ 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 +/// 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 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; @@ -714,14 +769,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()) @@ -1220,7 +1280,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(); @@ -1235,6 +1295,7 @@ mod tests { let err = networks_from_proto(&[rpc::NetworkingConfig { mode: "user".to_string(), bridge_name: "dstack-br0".to_string(), + ..Default::default() }]) .unwrap_err(); @@ -1243,9 +1304,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(); @@ -1253,14 +1364,67 @@ 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")); + + 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] + 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!(err.to_string().contains("custom networking mode")); + assert!(ensure_bridge_allowed("dstack-br0", &config).is_err()); + assert!(ensure_bridge_allowed("", &config).is_ok()); } #[test] diff --git a/dstack/vmm/src/one_shot.rs b/dstack/vmm/src/one_shot.rs index c71bee58f..09b13896a 100644 --- a/dstack/vmm/src/one_shot.rs +++ b/dstack/vmm/src/one_shot.rs @@ -290,6 +290,15 @@ Compose file content (first 200 chars): ); } + 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\"" + ); + } + let process_configs = vm_builder_config .config_qemu(&workdir_path, &config.cvm, &gpus) .context("Failed to build QEMU configuration")?; @@ -312,12 +321,35 @@ Compose file content (first 200 chars): println!("# QEMU Command:"); println!("{}", full_command.join(" ")); + 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 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..."); diff --git a/dstack/vmm/src/process_manager.rs b/dstack/vmm/src/process_manager.rs index e102dd2ee..9c7124012 100644 --- a/dstack/vmm/src/process_manager.rs +++ b/dstack/vmm/src/process_manager.rs @@ -15,6 +15,9 @@ use tokio::process::Command; 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 { Supervisor(SupervisorClient), @@ -31,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, )?)) } @@ -62,7 +67,10 @@ impl ProcessManager { pub async fn deploy(&self, config: &ProcessConfig) -> Result<()> { match self { - Self::Supervisor(client) => client.deploy(config).await, + Self::Supervisor(client) => { + ensure_supervisor_supported(config)?; + client.deploy(config).await + } Self::Systemd(manager) => manager.deploy(config).await, Self::Auto(manager) => manager.deploy(config).await, } @@ -101,6 +109,27 @@ impl ProcessManager { } } +/// 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(()) +} + pub struct AutoProcessManager { systemd: Arc, supervisor: Option, @@ -294,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" @@ -313,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())) } @@ -355,47 +409,83 @@ 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}")); + } + 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 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. + 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.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 + // /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}")); } - 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 = self.systemd_command("systemctl"); + reset.arg("reset-failed").arg(&unit); + let _ = reset.output().await; + let mut command = self.systemd_command("systemd-run"); + command.args(self.run_args(config, &unit)?); Self::command(command, "systemd-run").await?; if !config.pidfile.is_empty() { @@ -435,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 @@ -464,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") @@ -512,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) @@ -542,20 +632,234 @@ impl SystemdProcessManager { mod tests { use super::*; - #[test] - fn unit_names_are_stable_and_do_not_embed_process_ids() { + 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) + } + + /// 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); + 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(); 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 { + test_config_as("", open_files) + } + + fn test_config_as(user: &str, 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(), + user: user.into(), + 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(), + "dstack-vm".into(), + "infinity".into(), + false, + ) + .unwrap(); + (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 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="))); + + 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(); + } + } + + #[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] fn maps_systemd_states_to_process_status() { let state = |properties, started| state_from_systemd_properties(properties, started).0; diff --git a/dstack/vmm/src/vmm-cli.py b/dstack/vmm/src/vmm-cli.py index 1a533434f..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}") @@ -1030,6 +1055,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 +1188,16 @@ 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: + networking = parse_net_spec(net) + upgrade_params["update_networking"] = True + 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: if no_ports: @@ -1831,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 @@ -1972,6 +2012,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 +2127,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: diff --git a/dstack/vmm/vmm.toml b/dstack/vmm/vmm.toml index 74826f634..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" @@ -202,6 +212,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"