feat(vmm): pass pre-opened tap chardevs and drop VM privileges via systemd - #1047
Draft
Leechael wants to merge 13 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.
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)