feat(vmm): pass pre-opened tap chardevs and drop VM privileges via systemd - #1047
Draft
Leechael wants to merge 20 commits into
Draft
Conversation
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.
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.
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.
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=<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.
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.
…working 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.
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[:<name>]|default` for manual recovery; custom mode stays on the RPC, where the caller supplies the device path.
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.
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.
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.
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.
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.
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:<name>. 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack
Stacked on #1022 (
codex/experimental-systemd-vm-processes→next).Merge after #1022. Base is the parent branch, not
next.Problem
Production needs two things the Supervisor launch path cannot do together:
/dev/tap7498from macvtap), without handing QEMU a raw netdev string that embeds an fd number the VMM does not control.cvm.userwithoutsudo -u, which closes every descriptor above stderr before exec and would destroy any pre-opened chardev.Supervisor also has no way to open files for a child or set
User=on the process. Launching anyway would start a VM whose netdev points at an unrelated fd, or run as root after asking to be confined.Fix
Build on the experimental systemd process manager from #1022:
networking.open_file(manifest-only,mode = "custom", mutually exclusive withnetdev) generates-netdev tap,id=netN,fd=M.OpenFile=/ LISTEN_FDS order starting at fd 3. Paths are validated so they cannot injectOpenFile=field separators or%specifiers.OpenFile=properties and, whencvm.useris set,User=instead of asudoprefix.cvm.networking.open_fileis rejected; a chardev names one device and must stay per-NIC.cvm.userkeeps working: sudo gets#UID, systemd gets a bare UID.User=, the VMM chowns the swtpm state tree so the unprivileged launcher can create the socket and update TPM state.open_fileis omitted from Networking JSON so existing manifests stay byte-compatible when rewritten.ProcessConfig.user/open_filesdefault in the bon builder and are skipped when empty on the wire.Verification
cargo fmt --all -- --checkcargo clippy --target x86_64-unknown-linux-musl -p dstack-vmm -p supervisor -- -D warnings --allow unused_variablesprekon the touched filesdstack-vmmunit tests under Linux musl binary in Docker: 124 passedNot yet run on a real node:
systemd-run --property=OpenFile= --property=User=smoke test with a live macvtap chardev and a TPM-backed VM.Test plan
cvm.pm = "systemd"(or"auto"on a fresh deploy) andcvm.userset, start a non-TPM VM whose NIC usesmode = "custom"+open_file = "/dev/tapN"and confirm QEMU attaches to that chardevUser=and orderedOpenFile=properties, and QEMU args containtap,id=netN,fd=3(thenfd=4for a second open_file NIC)open_file(and Supervisor + non-emptyProcessConfig.user) fail closed before spawn--dry-runis enough whenopen_fileor systemdUser=is requiredUser=can createswtpm.sockand update state (ownership handoff)cvm.user = "#1000"still works on Supervisor (sudo -u #1000) and systemd (User=1000)