From 1b1ca3c141ff0742e11e8c350f2b370a405062da Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Fri, 6 Nov 2026 18:55:36 -0800 Subject: [PATCH 01/10] host: render cold boots from runtime and VM configuration Derive whole cold-boot arguments from Runtime and VmConfig, borrowing only the caller-chosen output names. Retain the console device and configured NIC, lower block devices by path, and check the complete Windows command-line limit on every host. Move the portable counting helpers to config while retaining the legacy launch paths and shared argument assembly. Add the reserved configuration-only check and document status-bearing guest power-off behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0325d032-38a9-4c7f-9899-687a4ec93a51 --- nvx-rs/README.md | 38 ++- nvx-rs/public-api/linux.txt | 5 + nvx-rs/public-api/windows.txt | 5 + nvx-rs/src/host/config/mod.rs | 340 ++++++++++++++++++++- nvx-rs/src/host/config/tests.rs | 516 +++++++++++++++++++++++++++++++- nvx-rs/src/host/launch.rs | 147 ++------- nvx-rs/src/host/launch/tests.rs | 66 +--- nvx-rs/src/host/runtime/mod.rs | 14 +- 8 files changed, 924 insertions(+), 207 deletions(-) diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 020bf75cf..a9bd76452 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -126,7 +126,7 @@ changes only when the module's purpose does. | [`nvx::host::runtime`](#nvxhostruntime) | `Runtime`: what every launch shares, the runtime's files and their identity, the hypervisor and its probe, and the launch options | | [`nvx::host::vm`](#nvxhostvm) | The directory of a VM, private to the current user, with the fixed names of the files and endpoints of its launches | | [`nvx::host::snapshot`](#nvxhostsnapshot) | Snapshot compatibility keys, strict manifests, and restore-time eligibility | -| [`nvx::host::config`](#nvxhostconfig) | `VmConfig` and `CaptureConfig`: VM/capture configuration, cold-boot kernel tokens and snapshot identity, without host I/O | +| [`nvx::host::config`](#nvxhostconfig) | `VmConfig` and `CaptureConfig`: pure configuration, cold-boot kernel/OpenVMM arguments, portable length checks, and capture identity | | [`nvx::host::restore`](#nvxhostrestore) | A caller-driven restore to Ready without I/O or a clock, and its peer-verified restore-ready listener | | [`nvx::host::artifacts`](#nvxhostartifacts) | Locating the OpenVMM executable and the guest artifacts that a VM runs, and their runtime identity | | [`nvx::host::launch`](#nvxhostlaunch) | `LaunchSpec` and `VmmLaunch`: the OpenVMM command line of a managed microVM, its checks, and the inputs that it shares with an adapter's launch | @@ -428,11 +428,13 @@ generation tells stale messages from foreign ones. `Runtime`: what every launch shares, the paths of the OpenVMM executable, guest kernel, and guest initramfs, their `RuntimeIdentity`, the hypervisor, an optional CPU profile, whether OpenVMM breaks away from the caller's Windows job -object, and the `LaunchOptions`. Those are what OpenVMM does when the guest -powers off, a `GuestPowerAction`, by default `Exit`, which ends the VM, or +object, and the `LaunchOptions`. Those are what OpenVMM does on a plain guest +power-off or hibernate event, a `GuestPowerAction`, by default `Exit`, or `Halt` or `Reset`, and how long OpenVMM waits for the host to authenticate on -the control console. The boot console is not among them: each VM attaches its -own. +the control console, in whole milliseconds from 1 to 60000. A power-off that +reports an exit status always exits OpenVMM with that status, regardless of +the selected action. The boot console is not among the options: each VM +attaches its own. `RuntimeIdentity` labels the three files with the `ArtifactIdentity` tokens that their install or staging metadata gives, such as `sha256:` and a digest, which @@ -558,6 +560,29 @@ entries. The table's `HostMap` and its wire codecs are accessible through command-line budget, including the terminating NUL. The legacy launch checks continue to use it. +`VmConfig::openvmm_arguments` renders a cold boot's whole OpenVMM argument list +from `Runtime` and `VmConfig`. Its borrowed control endpoint, report path, and +optional boot-console endpoint are already chosen output names, not additional +launch settings. It opens no path, allocates no endpoint, and starts no process. +It renders processors, memory without narrowing it, kernel and initramfs paths, +the kernel command line, layered-root block devices by path, network and export +arguments, console attachment, the CPU profile, and every `LaunchOptions` value. +A configured NIC remains attached even under deny-all policy. A listener needs +its supplied console name; a disconnected attachment retains the device and +discards its output. The renderer emits neither `--microvm-lifecycle` nor +`--microvm-workload-identity`. The whole-config entry point is temporarily +public: SL2.8 makes it internal when `host::vm::create` calls it. + +`VmConfig::check_command_line()` checks the configuration's known arguments on +every host, before any host change, with 2048 UTF-16 code units reserved for the +runtime's files, launch options, and VM output names. The reserve foresees file +and endpoint names of up to 260 code units, not arbitrary runtime paths or +CPU-profile strings. The whole renderer checks the complete actual command +line without a reserve too. `PROCESS_COMMAND_LINE_LIMIT`, +`process_command_line_units` and `fit_process_command_line` now belong to +`nvx::host::config`; their existing `nvx::host::launch` paths remain usable, +with the same Windows quoting and terminating-NUL count. + A `Boot` may also hold a `CaptureConfig`, what a capture of the VM adds to the VM's configuration, which keeps the rest, such as the NIC, the console, and the export: a `DiskCapture`, so far only `Fresh`, the `BlockGeometry`, length and @@ -679,7 +704,8 @@ specification's. `process_command_line_units` counts the Windows command line that runs a program with its arguments, in UTF-16 code units, as Windows quotes them, and `fit_process_command_line` checks it against `PROCESS_COMMAND_LINE_LIMIT`, on -every host, with the reserve that its caller keeps. +every host, with the reserve that its caller keeps. The helpers live in +`nvx::host::config` and remain re-exported here for existing callers. `LaunchSpec::check_process_command_line`, which a caller runs before it provisions an instance, keeps 1024 code units for what may change before the instance starts, whose own check keeps none. diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index 36e58a4fe..ed3f314d2 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -1788,6 +1788,7 @@ field nvx::host::config::NetworkConfig::guest_network field nvx::host::config::NetworkConfig::policy struct nvx::host::config::NicConfig field nvx::host::config::NicConfig::guest_network +const nvx::host::config::PROCESS_COMMAND_LINE_LIMIT enum nvx::host::config::RootConfig variant nvx::host::config::RootConfig::Blockless variant nvx::host::config::RootConfig::Layered @@ -1802,15 +1803,19 @@ field nvx::host::config::UpperConfig::Scratch::path struct nvx::host::config::VmConfig field nvx::host::config::VmConfig::boot method nvx::host::config::VmConfig::check +method nvx::host::config::VmConfig::check_command_line field nvx::host::config::VmConfig::console field nvx::host::config::VmConfig::filesystem method nvx::host::config::VmConfig::kernel_command_line method nvx::host::config::VmConfig::mapping_table field nvx::host::config::VmConfig::memory_mib field nvx::host::config::VmConfig::network +method nvx::host::config::VmConfig::openvmm_arguments field nvx::host::config::VmConfig::processors field nvx::host::config::VmConfig::root field nvx::host::config::VmConfig::view +fn nvx::host::config::fit_process_command_line +fn nvx::host::config::process_command_line_units mod nvx::host::filesystem struct nvx::host::filesystem::Bind field nvx::host::filesystem::Bind::child diff --git a/nvx-rs/public-api/windows.txt b/nvx-rs/public-api/windows.txt index f5176e097..2a2a3f121 100644 --- a/nvx-rs/public-api/windows.txt +++ b/nvx-rs/public-api/windows.txt @@ -711,6 +711,7 @@ field nvx::host::config::NetworkConfig::guest_network field nvx::host::config::NetworkConfig::policy struct nvx::host::config::NicConfig field nvx::host::config::NicConfig::guest_network +const nvx::host::config::PROCESS_COMMAND_LINE_LIMIT enum nvx::host::config::RootConfig variant nvx::host::config::RootConfig::Blockless variant nvx::host::config::RootConfig::Layered @@ -725,15 +726,19 @@ field nvx::host::config::UpperConfig::Scratch::path struct nvx::host::config::VmConfig field nvx::host::config::VmConfig::boot method nvx::host::config::VmConfig::check +method nvx::host::config::VmConfig::check_command_line field nvx::host::config::VmConfig::console field nvx::host::config::VmConfig::filesystem method nvx::host::config::VmConfig::kernel_command_line method nvx::host::config::VmConfig::mapping_table field nvx::host::config::VmConfig::memory_mib field nvx::host::config::VmConfig::network +method nvx::host::config::VmConfig::openvmm_arguments field nvx::host::config::VmConfig::processors field nvx::host::config::VmConfig::root field nvx::host::config::VmConfig::view +fn nvx::host::config::fit_process_command_line +fn nvx::host::config::process_command_line_units mod nvx::host::filesystem struct nvx::host::filesystem::Bind field nvx::host::filesystem::Bind::child diff --git a/nvx-rs/src/host/config/mod.rs b/nvx-rs/src/host/config/mod.rs index e17df793c..999778e13 100644 --- a/nvx-rs/src/host/config/mod.rs +++ b/nvx-rs/src/host/config/mod.rs @@ -3,21 +3,26 @@ //! of it, its [`NetworkConfig`], the host paths that it exports, its [`ConsoleAttachment`], and //! what only its cold boot takes, a [`Boot`], with what a capture of the VM adds, a //! [`CaptureConfig`], whose [`CaptureIdentity`] names the capture's snapshots. +//! [`VmConfig::openvmm_arguments`] renders a whole cold launch from that configuration and a +//! [`Runtime`], with caller-supplied output names, without opening paths or allocating endpoints. use std::collections::BTreeMap; +use std::ffi::{OsStr, OsString}; use std::fmt; use std::ops::RangeInclusive; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; +use std::time::Duration; use sha2::{Digest, Sha256}; use super::filesystem::{ExportPlan, ReservedTargets}; -use super::network::NetworkPolicy; +use super::network::{self, NetworkPolicy}; use super::platform::parse_hex32; -use super::runtime::Runtime; +use super::runtime::{GuestPowerAction, Runtime}; use crate::activation::{self, Journal, MAX_HOSTNAME_BYTES, OverlayMode, View}; use crate::contract; use crate::error::{Error, Result}; +use crate::hypervisor::Hypervisor; use crate::net::Ipv4Cidr; use crate::protocol::cmdline::{self, Parameter}; use crate::protocol::maps::HostMap; @@ -27,6 +32,14 @@ use crate::protocol::view; /// Bytes of the x86 kernel command line available to caller tokens, including the NUL. pub const COMMAND_LINE_BUDGET: usize = 1024; +/// Longest Windows process command line, in UTF-16 code units, including the terminating NUL. +/// +/// OpenVMM's command line is held to it on every host, so a policy behaves alike on all of them. +pub const PROCESS_COMMAND_LINE_LIMIT: usize = 32_767; + +/// Room for the runtime's files and the VM's output paths, as legacy adapter planning reserves. +pub(crate) const OPENVMM_COMMAND_LINE_RESERVE: usize = 2048; + /// The guest address of a VM's NIC at a cold boot whose configuration names none, in the form /// that [`Ipv4Cidr::parse`] reads. pub const DEFAULT_GUEST_NETWORK: &str = "10.0.0.2/24"; @@ -422,6 +435,171 @@ impl VmConfig { Ok(tokens.join(" ")) } + /// Renders a cold boot's complete OpenVMM arguments from this configuration and `runtime`. + /// + /// `endpoint`, `report` and `console_endpoint` are already chosen output names, not launch + /// settings: this method creates no directory, file, listener, or process, and uses paths as + /// given. A [`ConsoleAttachment::Listener`] needs a nonempty `console_endpoint`; a + /// [`ConsoleAttachment::Disconnected`] needs none and retains the console device with a + /// discarded attachment. The control endpoint always listens and authenticates from stdin. + /// The caller runs `runtime.openvmm` with these arguments. + /// + /// This whole-config entry point is temporarily public. SL2.8 makes it internal when + /// `host::vm::create` calls it. Individual device lowerings stay internal. Neither managed + /// lifecycle nor workload-identity flags are rendered; the caller owns its guest tokens. + /// `runtime.windows_breakaway` is process-launch behavior, not an OpenVMM argument. + /// + /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused + /// configuration, no [`Boot`], empty or NUL-containing required names or runtime paths, an + /// invalid authentication timeout, or a complete command line beyond the Windows limit. + /// The complete limit check runs on every host, without a reserve. + pub fn openvmm_arguments( + &self, + runtime: &Runtime, + endpoint: &str, + report: &Path, + console_endpoint: Option<&str>, + ) -> Result> { + let command_line = self.kernel_command_line()?; + check_endpoint(endpoint, "endpoint")?; + for (name, path) in [ + ("Runtime.openvmm", runtime.openvmm.as_path()), + ("Runtime.kernel", runtime.kernel.as_path()), + ("Runtime.initramfs", runtime.initramfs.as_path()), + ("report", report), + ] { + if path.as_os_str().is_empty() || path.as_os_str().to_string_lossy().contains('\0') { + return Err(Error::unsupported(format!( + "{name} must name a path without a NUL" + ))); + } + } + let console = match (self.console, console_endpoint) { + (ConsoleAttachment::Listener, Some(endpoint)) => { + check_endpoint(endpoint, "console_endpoint")?; + format!("listen={endpoint}") + } + (ConsoleAttachment::Disconnected, None) => "none".to_owned(), + (ConsoleAttachment::Listener, None) => { + return Err(Error::unsupported( + "a console listener needs console_endpoint", + )); + } + (ConsoleAttachment::Disconnected, Some(_)) => { + return Err(Error::unsupported( + "a disconnected console takes no console_endpoint", + )); + } + }; + let mut arguments = self.device_arguments()?; + arguments.extend(boot_arguments( + runtime.hypervisor, + self.memory_mib, + &runtime.kernel, + &runtime.initramfs, + &command_line, + )); + arguments.extend(console_arguments(&console, endpoint, report)); + if let Some(profile) = &runtime.cpu_profile { + if profile.is_empty() { + return Err(Error::unsupported("Runtime.cpu_profile must not be empty")); + } + arguments.extend(["--cpu-profile".into(), profile.as_str().into()]); + } + let shutdown = match runtime.options.guest_shutdown { + GuestPowerAction::Exit => "exit", + GuestPowerAction::Halt => "halt", + GuestPowerAction::Reset => "reset", + }; + arguments.extend(["--guest-shutdown-action".into(), shutdown.into()]); + if let Some(timeout) = runtime.options.control_auth_timeout { + if timeout < Duration::from_millis(1) + || timeout > Duration::from_secs(60) + || timeout.subsec_nanos() % 1_000_000 != 0 + { + return Err(Error::unsupported( + "LaunchOptions.control_auth_timeout must be whole milliseconds from 1 to 60000", + )); + } + arguments.extend([ + "--microvm-control-auth-timeout-ms".into(), + timeout.as_millis().to_string().into(), + ]); + } + check_argument_nuls(&arguments)?; + fit_process_command_line(runtime.openvmm.as_os_str(), &arguments, 0)?; + Ok(arguments) + } + + /// Checks the configuration's part of a cold boot's OpenVMM command line on every host, + /// without reading or changing the host, with 2048 UTF-16 code units reserved for the + /// runtime's files, launch options, and the VM's output paths. + /// + /// Like legacy adapter planning, the reserve foresees runtime-file and output names of up to + /// 260 code units each, Windows' `MAX_PATH`. It does not certify arbitrary runtime paths or + /// CPU-profile names: [`openvmm_arguments`](Self::openvmm_arguments) checks the complete + /// command line with the actual runtime and output names, without a reserve. + /// A listener's backend prefix is counted here; its endpoint name stays in the reserve. + /// + /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused + /// configuration or no [`Boot`], an unlowerable network policy, a NUL-containing argument, + /// or insufficient room for the reserve. + pub fn check_command_line(&self) -> Result<()> { + let command_line = self.kernel_command_line()?; + let mut arguments = self.device_arguments()?; + arguments.extend([ + "--memory".into(), + format!("{}M", self.memory_mib).into(), + "--cmdline".into(), + command_line.into(), + "--virtio-console".into(), + match self.console { + ConsoleAttachment::Listener => "listen=", + ConsoleAttachment::Disconnected => "none", + } + .into(), + ]); + check_argument_nuls(&arguments)?; + fit_arguments(&arguments, OPENVMM_COMMAND_LINE_RESERVE) + } + + /// Renders only the configuration's devices; no runtime or output name is needed. + fn device_arguments(&self) -> Result> { + let mut arguments = vec![ + "--machine".into(), + "microvm".into(), + "--processors".into(), + self.processors.to_string().into(), + ]; + if let RootConfig::Layered(root) = &self.root { + let mut image = OsString::from("distro:file:"); + image.push(&root.image); + image.push(",ro"); + arguments.extend(["--microvm-sandbox-block".into(), image]); + if let UpperConfig::Scratch { path, .. } = &root.upper { + let mut scratch = OsString::from("scratch:file:"); + scratch.push(path); + arguments.extend(["--microvm-sandbox-block".into(), scratch]); + } + } + if let Some(network) = &self.network { + let guest_network = network.guest_network.map_or_else( + || DEFAULT_GUEST_NETWORK.to_owned(), + |address| address.to_string(), + ); + let mut nic = network::openvmm_arguments(Some(&network.policy), &guest_network)?; + if nic.is_empty() { + // A configured NIC remains present even under the deny-all posture. + nic = network::template_arguments(&guest_network); + } + arguments.extend(nic.into_iter().map(OsString::from)); + } + if let Some(export) = &self.filesystem { + arguments.extend(export.openvmm_arguments().into_iter().map(OsString::from)); + } + Ok(arguments) + } + /// Checks the whole configuration without reading the host: it opens no path, and checks no /// support of the host. /// @@ -686,5 +864,161 @@ impl Identifying { } } +/// Common cold-boot inputs, shared with the legacy VmmLaunch without narrowing its public types. +pub(crate) fn boot_arguments( + hypervisor: Hypervisor, + memory_mib: u64, + kernel: &Path, + initrd: &Path, + kernel_command_line: &str, +) -> Vec { + let memory = format!("{memory_mib}M"); + [ + OsStr::new("--single-process"), + OsStr::new("--hypervisor"), + OsStr::new(hypervisor.as_str()), + OsStr::new("--memory"), + OsStr::new(&memory), + OsStr::new("--kernel"), + kernel.as_os_str(), + OsStr::new("--initrd"), + initrd.as_os_str(), + OsStr::new("--cmdline"), + OsStr::new(kernel_command_line), + ] + .map(OsStr::to_os_string) + .into() +} + +/// Common console, control authentication, and report arguments of a cold launch. +pub(crate) fn console_arguments(console: &str, endpoint: &str, report: &Path) -> Vec { + let control = format!("listen={endpoint}"); + [ + OsStr::new("--virtio-console"), + OsStr::new(console), + OsStr::new("--microvm-control-console"), + OsStr::new(&control), + OsStr::new("--microvm-control-auth-stdin"), + OsStr::new("--microvm-report"), + report.as_os_str(), + ] + .map(OsStr::to_os_string) + .into() +} + +fn check_endpoint(endpoint: &str, name: &str) -> Result<()> { + if endpoint.is_empty() || endpoint.contains('\0') { + return Err(Error::unsupported(format!( + "{name} must name a nonempty endpoint without a NUL", + ))); + } + Ok(()) +} + +fn check_argument_nuls(arguments: &[OsString]) -> Result<()> { + if arguments + .iter() + .any(|argument| argument.to_string_lossy().contains('\0')) + { + return Err(Error::unsupported( + "OpenVMM's arguments must not contain a NUL", + )); + } + Ok(()) +} + +/// Returns the length in UTF-16 code units, including the terminating NUL, of the Windows command +/// line that runs `program` with `arguments`, quoted the way the Windows platform quotes it: the +/// program always in quotes, and each argument in quotes if it is empty or holds a space or a +/// tab, with a backslash before each quote that it holds and a backslash before each backslash +/// that precedes a quote, its closing quote included, and one space between them. +/// +/// It is the length that [`PROCESS_COMMAND_LINE_LIMIT`] bounds, whatever the host: +/// [`fit_process_command_line`] checks it. +pub fn process_command_line_units(program: &OsStr, arguments: &[OsString]) -> usize { + argument_units(program, true) + arguments_units(arguments) +} + +/// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) unless the Windows command +/// line that runs `program`, such as OpenVMM, with `arguments`, as [`process_command_line_units`] +/// counts it, leaves `reserve` code units of [`PROCESS_COMMAND_LINE_LIMIT`] unused: none for the +/// complete command line of a launch about to spawn OpenVMM, and some for what may change before +/// then, such as the paths of the artifacts. +/// +/// The error says how long the command line is, and how to shorten it. +pub fn fit_process_command_line( + program: &OsStr, + arguments: &[OsString], + reserve: usize, +) -> Result<()> { + let length = process_command_line_units(program, arguments); + let held = if reserve == 0 { + String::new() + } else { + format!(", with {reserve} held in reserve") + }; + fit("OpenVMM's command line", length, reserve, &held) +} + +/// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) unless OpenVMM's +/// `arguments`, which a plan renders, without the program and the rest of the launch, leave +/// `reserve` code units of [`PROCESS_COMMAND_LINE_LIMIT`] to that rest. +pub(crate) fn fit_arguments(arguments: &[OsString], reserve: usize) -> Result<()> { + let held = format!(", with {reserve} held in reserve for the rest of the launch"); + fit( + "OpenVMM's arguments", + arguments_units(arguments), + reserve, + &held, + ) +} + +/// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported), saying that `subject` +/// would take `length` code units, `held` describing `reserve`, unless they leave `reserve` of +/// [`PROCESS_COMMAND_LINE_LIMIT`] unused. +fn fit(subject: &str, length: usize, reserve: usize, held: &str) -> Result<()> { + if length.saturating_add(reserve) <= PROCESS_COMMAND_LINE_LIMIT { + return Ok(()); + } + Err(Error::unsupported(format!( + "{subject} would take {length} of the {PROCESS_COMMAND_LINE_LIMIT} characters that a \ + Windows process command line allows{held}; map, hide, or allow fewer or shorter \ + filesystem paths, or use fewer network rules" + ))) +} + +/// Returns the length in UTF-16 code units of `arguments` on a Windows command line, quoted as +/// [`process_command_line_units`] quotes them, each with the space before it. +pub(crate) fn arguments_units(arguments: &[OsString]) -> usize { + arguments + .iter() + .map(|argument| argument_units(argument, false)) + .sum() +} + +/// Returns the length in UTF-16 code units of `argument` on a Windows command line, quoted as +/// [`process_command_line_units`] quotes it, always if it is the `program`, with one separating +/// space, or the terminating NUL after the last argument. +fn argument_units(argument: &OsStr, program: bool) -> usize { + let text = argument.to_string_lossy(); + let quoted = program || text.is_empty() || text.contains([' ', '\t']); + let mut length = 1 + text.encode_utf16().count() + if quoted { 2 } else { 0 }; + let mut backslashes = 0; + for character in text.chars() { + match character { + '\\' => backslashes += 1, + '"' => { + length += backslashes + 1; + backslashes = 0; + } + _ => backslashes = 0, + } + } + if quoted { + length += backslashes; + } + length +} + #[cfg(test)] mod tests; diff --git a/nvx-rs/src/host/config/tests.rs b/nvx-rs/src/host/config/tests.rs index d2ea319e0..df7551c82 100644 --- a/nvx-rs/src/host/config/tests.rs +++ b/nvx-rs/src/host/config/tests.rs @@ -10,7 +10,7 @@ use crate::activation::MachineIdMode; use crate::host::artifacts::Artifacts; use crate::host::filesystem::{Bind, ExportChild, Owner}; use crate::host::launch::LaunchSpec; -use crate::host::network::Access; +use crate::host::network::{Access, EgressRule, Protocol}; use crate::host::runtime::{ArtifactIdentity, GuestPowerAction, LaunchOptions, RuntimeIdentity}; use crate::host::sandbox::{SandboxDistro, SandboxPlan, SandboxSettings, SandboxUpper}; use crate::hypervisor::Hypervisor; @@ -1100,3 +1100,517 @@ fn public_maps_codec_keeps_the_protocol_root_reexports_usable() { crate::protocol::decode_maps_payload(&payloads[0]).unwrap(); assert_eq!(decoded.maps, table); } + +fn runtime() -> Runtime { + let artifact = ArtifactIdentity::new("fixture").unwrap(); + Runtime { + openvmm: missing("openvmm"), + kernel: missing("vmlinux"), + initramfs: missing("initramfs.cpio.gz"), + identity: RuntimeIdentity::new(artifact.clone(), artifact.clone(), artifact), + hypervisor: Hypervisor::Kvm, + cpu_profile: None, + windows_breakaway: false, + options: LaunchOptions::default(), + } +} + +fn cold(config: VmConfig) -> VmConfig { + VmConfig { + boot: Some(Boot { + command_line: "quiet nvx_entrypoint=/bin/work nvx_arg=\"a b\"".to_owned(), + capture: None, + }), + ..config + } +} + +fn values<'a>(arguments: &'a [OsString], option: &str) -> Vec<&'a OsStr> { + arguments + .windows(2) + .filter(|pair| pair[0] == option) + .map(|pair| pair[1].as_os_str()) + .collect() +} + +#[test] +fn cold_boot_arguments_select_the_configured_guest_without_managed_tokens() { + for hypervisor in [Hypervisor::Kvm, Hypervisor::Mshv, Hypervisor::Whp] { + for console in [ConsoleAttachment::Listener, ConsoleAttachment::Disconnected] { + let config = cold(VmConfig { + processors: 4, + memory_mib: u64::from(u32::MAX) + 1, + console, + ..blockless() + }); + let runtime = Runtime { + hypervisor, + ..runtime() + }; + let endpoint = "/missing/control.sock"; + let report = Path::new("/missing/outcome.json"); + let console_endpoint = + (console == ConsoleAttachment::Listener).then_some("/missing/console.sock"); + let arguments = config + .openvmm_arguments(&runtime, endpoint, report, console_endpoint) + .unwrap(); + let console_backend = if console_endpoint.is_some() { + "listen=/missing/console.sock" + } else { + "none" + }; + let expected: Vec = [ + OsStr::new("--machine"), OsStr::new("microvm"), + OsStr::new("--processors"), OsStr::new("4"), + OsStr::new("--single-process"), + OsStr::new("--hypervisor"), OsStr::new(hypervisor.as_str()), + OsStr::new("--memory"), OsStr::new("4294967296M"), + OsStr::new("--kernel"), runtime.kernel.as_os_str(), + OsStr::new("--initrd"), runtime.initramfs.as_os_str(), + OsStr::new("--cmdline"), + OsStr::new("nvx_hostname=626f78 nvx_machine_id=image quiet nvx_entrypoint=/bin/work nvx_arg=\"a b\""), + OsStr::new("--virtio-console"), OsStr::new(console_backend), + OsStr::new("--microvm-control-console"), OsStr::new("listen=/missing/control.sock"), + OsStr::new("--microvm-control-auth-stdin"), + OsStr::new("--microvm-report"), report.as_os_str(), + OsStr::new("--guest-shutdown-action"), OsStr::new("exit"), + ].into_iter().map(OsStr::to_os_string).collect(); + assert_eq!(arguments, expected); + for forbidden in [ + "--microvm-lifecycle", + "--microvm-workload-identity", + "--microvm-sandbox-block", + ] { + assert!(!arguments.iter().any(|argument| argument == forbidden)); + } + } + } +} + +#[test] +fn cold_boot_arguments_lower_layered_roots_networks_and_exports_by_path() { + for format in [ + RootImage::erofs(None), + RootImage::gpt(Some(Uuid::from_bytes([7; 16]))), + ] { + for upper in [ + UpperConfig::Ram { + max_memory_percent: 37, + }, + UpperConfig::Scratch { + path: missing("scratch with spaces.ext4"), + journal: Journal::Forbidden, + }, + ] { + for address in [None, Some(Ipv4Cidr::parse("172.16.8.2/24").unwrap())] { + let config = cold(VmConfig { + root: RootConfig::Layered(LayeredRoot { + image: missing("root with spaces.img"), + format, + gpt_first_partition_bound: (format.format == ImageFormat::Gpt) + .then_some(65536), + upper: upper.clone(), + overlay: OverlayMode::UntrustedLowers, + }), + network: Some(NetworkConfig { + guest_network: address, + policy: NetworkPolicy::new(Access::Deny) + .with_allow(EgressRule::to("192.0.2.0/24").on_port(Protocol::Tcp, 443)), + }), + filesystem: Some(export("/work")), + ..blockless() + }); + let arguments = config + .openvmm_arguments( + &runtime(), + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .unwrap(); + let mut image = OsString::from("distro:file:"); + image.push(missing("root with spaces.img")); + image.push(",ro"); + let mut blocks = vec![image]; + if let UpperConfig::Scratch { path, .. } = &upper { + let mut scratch = OsString::from("scratch:file:"); + scratch.push(path); + blocks.push(scratch); + } + assert_eq!( + values(&arguments, "--microvm-sandbox-block"), + blocks.iter().map(OsString::as_os_str).collect::>() + ); + assert_eq!( + values(&arguments, "--cmdline"), + [OsStr::new(&config.kernel_command_line().unwrap())] + ); + assert_eq!( + values(&arguments, "--net"), + [OsStr::new(if address.is_some() { + "172.16.8.2/24" + } else { + DEFAULT_GUEST_NETWORK + })] + ); + assert_eq!( + values(&arguments, "--network-profile"), + [OsStr::new("portable")] + ); + assert_eq!(values(&arguments, "--network-egress"), [OsStr::new("deny")]); + assert_eq!( + values(&arguments, "--network-egress-allow"), + [OsStr::new("192.0.2.0/24:tcp:443")] + ); + assert_eq!( + values(&arguments, "--mount-aggregate"), + [OsStr::new(crate::host::filesystem::GUEST_EXPORT)] + ); + let mut child = OsString::from("0,"); + child.push(missing("export")); + child.push(",ro"); + assert_eq!(values(&arguments, "--mount-child"), [child.as_os_str()]); + for forbidden in ["--microvm-lifecycle", "--microvm-workload-identity"] { + assert!(!arguments.iter().any(|argument| argument == forbidden)); + } + } + } + } +} + +#[test] +fn an_explicit_deny_all_network_keeps_its_nic() { + let mut config = cold(blockless()); + config.network = Some(NetworkConfig { + guest_network: None, + policy: NetworkPolicy::new(Access::Deny), + }); + let arguments = config + .openvmm_arguments( + &runtime(), + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .unwrap(); + assert_eq!( + values(&arguments, "--net"), + [OsStr::new(DEFAULT_GUEST_NETWORK)] + ); + for option in ["--network-egress", "--network-ingress", "--host-loopback"] { + assert_eq!(values(&arguments, option), [OsStr::new("deny")]); + } +} + +#[test] +fn cold_boot_arguments_render_every_launch_option_without_process_side_effects() { + for (action, text) in [ + (GuestPowerAction::Exit, "exit"), + (GuestPowerAction::Halt, "halt"), + (GuestPowerAction::Reset, "reset"), + ] { + for timeout in [ + None, + Some(Duration::from_millis(1)), + Some(Duration::from_millis(59999)), + Some(Duration::from_millis(60000)), + ] { + for profile in [None, Some("auto".to_owned())] { + let mut runtime = Runtime { + cpu_profile: profile.clone(), + options: LaunchOptions { + guest_shutdown: action, + control_auth_timeout: timeout, + }, + ..runtime() + }; + let config = cold(blockless()); + let render = |runtime: &Runtime| { + config + .openvmm_arguments( + runtime, + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .unwrap() + }; + let arguments = render(&runtime); + assert_eq!( + values(&arguments, "--guest-shutdown-action"), + [OsStr::new(text)] + ); + assert_eq!( + values(&arguments, "--cpu-profile"), + profile + .as_deref() + .map(OsStr::new) + .into_iter() + .collect::>() + ); + let milliseconds = timeout.map(|value| value.as_millis().to_string()); + assert_eq!( + values(&arguments, "--microvm-control-auth-timeout-ms"), + milliseconds + .as_deref() + .map(OsStr::new) + .into_iter() + .collect::>() + ); + runtime.windows_breakaway = true; + assert_eq!(arguments, render(&runtime)); + } + } + } +} + +#[test] +fn authentication_timeout_refuses_fractional_and_out_of_range_milliseconds() { + for timeout in [ + Duration::ZERO, + Duration::from_nanos(1), + Duration::from_micros(999), + Duration::from_nanos(1_000_001), + Duration::from_nanos(60_000_000_001), + Duration::from_millis(60001), + Duration::from_secs(u64::MAX), + ] { + let runtime = Runtime { + options: LaunchOptions { + control_auth_timeout: Some(timeout), + ..LaunchOptions::default() + }, + ..runtime() + }; + let error = cold(blockless()) + .openvmm_arguments( + &runtime, + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + assert!( + error.to_string().contains("control_auth_timeout"), + "{error}" + ); + } +} + +#[test] +fn required_output_names_and_cold_boot_inputs_are_refused_without_host_io() { + let config = cold(blockless()); + let runtime = runtime(); + for (endpoint, report, console) in [ + ("", Path::new("/missing/report"), Some("/missing/console")), + ( + "/missing/control\0", + Path::new("/missing/report"), + Some("/missing/console"), + ), + ("/missing/control", Path::new(""), Some("/missing/console")), + ("/missing/control", Path::new("/missing/report"), None), + ("/missing/control", Path::new("/missing/report"), Some("")), + ( + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console\0"), + ), + ] { + assert_eq!( + config + .openvmm_arguments(&runtime, endpoint, report, console) + .unwrap_err() + .kind(), + ErrorKind::Unsupported + ); + } + assert_eq!( + blockless() + .openvmm_arguments( + &runtime, + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console") + ) + .unwrap_err() + .kind(), + ErrorKind::Unsupported + ); + assert_eq!( + blockless().check_command_line().unwrap_err().kind(), + ErrorKind::Unsupported + ); + let disconnected = cold(VmConfig { + console: ConsoleAttachment::Disconnected, + ..blockless() + }); + assert!( + disconnected + .openvmm_arguments( + &runtime, + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console") + ) + .is_err() + ); + let invalid = cold(VmConfig { + processors: 0, + ..blockless() + }); + assert!( + invalid + .openvmm_arguments( + &runtime, + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console") + ) + .is_err() + ); + assert!(invalid.check_command_line().is_err()); +} + +#[test] +fn configuration_command_lines_keep_the_same_reserve_on_every_host() { + let hidden = |padding| { + let mut plan = export("/work"); + let root = if cfg!(windows) { + r"D:\host\work" + } else { + "/host/work" + }; + plan.children[0].root = PathBuf::from(root).join("r".repeat(padding)); + plan.children[0].denied = (0..128) + .map(|index| { + plan.children[0] + .root + .join(format!("{index:03}-{}", "x".repeat(120))) + }) + .collect(); + plan.children[0].denied_identities = vec![None; 128]; + cold(VmConfig { + filesystem: Some(plan), + ..blockless() + }) + }; + hidden(80).check_command_line().unwrap(); + let reserved = hidden(95); + let error = reserved.check_command_line().unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + assert!( + error.to_string().contains("2048 held in reserve"), + "{error}" + ); + reserved + .openvmm_arguments( + &runtime(), + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .unwrap(); + let oversized = hidden(110); + assert_eq!( + oversized.check_command_line().unwrap_err().kind(), + ErrorKind::Unsupported + ); + assert_eq!( + oversized + .openvmm_arguments( + &runtime(), + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .unwrap_err() + .kind(), + ErrorKind::Unsupported + ); + let config = cold(blockless()); + config.check_command_line().unwrap(); + let runtime = Runtime { + openvmm: PathBuf::from("x".repeat(PROCESS_COMMAND_LINE_LIMIT)), + ..runtime() + }; + assert!( + config + .openvmm_arguments( + &runtime, + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .is_err() + ); +} + +#[test] +fn process_command_lines_are_measured_as_windows_quotes_them() { + let cases: [(&str, &[&str], &str); 8] = [ + ( + r"C:\Program Files\openvmm.exe", + &["--a", "b c", ""], + r#""C:\Program Files\openvmm.exe" --a "b c" """#, + ), + ("x", &[r#"say "hi""#], r#""x" "say \"hi\"""#), + ("x", &[r#"a"b"#], r#""x" a\"b"#), + ("x", &[r"dir\ with\"], r#""x" "dir\ with\\""#), + ("x", &[r"a\\b", "é"], r#""x" a\\b é"#), + ("", &[""], r#""" """#), + ("x", &["a\tb"], "\"x\" \"a\tb\""), + ("x", &["\u{1f680}"], "\"x\" \u{1f680}"), + ]; + for (program, arguments, line) in cases { + let arguments: Vec = arguments.iter().map(OsString::from).collect(); + assert_eq!( + process_command_line_units(OsStr::new(program), &arguments), + line.encode_utf16().count() + 1, + "{line}" + ); + } +} + +#[test] +fn a_process_command_line_fits_while_it_leaves_its_reserve() { + // Any program and arguments, such as an adapter's: a command line of the limit fits, + // one more code unit does not, and a reserve takes its part of the limit. + let arguments: Vec = ["--a", "b c"].map(OsString::from).into(); + let base = process_command_line_units(OsStr::new("/o"), &arguments); + let program = |extra: usize| OsString::from(format!("/o{}", "o".repeat(extra))); + let fitting = program(PROCESS_COMMAND_LINE_LIMIT - base); + assert_eq!( + process_command_line_units(&fitting, &arguments), + PROCESS_COMMAND_LINE_LIMIT + ); + fit_process_command_line(&fitting, &arguments, 0).unwrap(); + let held = fit_process_command_line(&fitting, &arguments, 1).unwrap_err(); + assert_eq!(held.kind(), ErrorKind::Unsupported); + assert!( + held.to_string() + .contains("would take 32767 of the 32767 characters"), + "{held}" + ); + assert!( + held.to_string().contains("with 1 held in reserve"), + "{held}" + ); + let longer = program(PROCESS_COMMAND_LINE_LIMIT - base + 1); + let error = fit_process_command_line(&longer, &arguments, 0).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + // A reserve past the limit leaves no room for any command line. + assert!(fit_process_command_line(OsStr::new("o"), &[], usize::MAX).is_err()); + // The arguments alone, which a plan renders, take what they take on the command line. + let alone = process_command_line_units(OsStr::new(""), &arguments) + - process_command_line_units(OsStr::new(""), &[]); + let reserve = PROCESS_COMMAND_LINE_LIMIT - alone; + fit_arguments(&arguments, reserve).unwrap(); + let held = fit_arguments(&arguments, reserve + 1).unwrap_err(); + assert!( + held.to_string() + .contains("held in reserve for the rest of the launch"), + "{held}" + ); +} diff --git a/nvx-rs/src/host/launch.rs b/nvx-rs/src/host/launch.rs index 8dd6da639..1bd263e99 100644 --- a/nvx-rs/src/host/launch.rs +++ b/nvx-rs/src/host/launch.rs @@ -16,12 +16,12 @@ //! [`PreparedLaunch::adapter_arguments`](super::sandbox::PreparedLaunch::adapter_arguments) holds //! an adapter's command line to the limit too. -use std::ffi::{OsStr, OsString}; +use std::ffi::OsString; use std::path::{Path, PathBuf}; use std::time::Duration; use super::artifacts::Artifacts; -pub use super::config::COMMAND_LINE_BUDGET; +use super::config; use super::filesystem::ExportPlan; use super::network::{self, NetworkPolicy}; use crate::error::{Error, Result}; @@ -30,12 +30,11 @@ use crate::net::{is_valid_guest_network, is_valid_hostname}; use crate::protocol::WORKLOAD_ACCOUNT_TOKEN; use crate::protocol::cmdline::{self, Parameter}; -pub use super::config::DEFAULT_GUEST_NETWORK; - -/// Longest Windows process command line, in UTF-16 code units, including the terminating NUL. -/// -/// OpenVMM's command line is held to it on every host, so a policy behaves alike on all of them. -pub const PROCESS_COMMAND_LINE_LIMIT: usize = 32_767; +pub(crate) use super::config::fit_arguments; +pub use super::config::{ + COMMAND_LINE_BUDGET, DEFAULT_GUEST_NETWORK, PROCESS_COMMAND_LINE_LIMIT, + fit_process_command_line, process_command_line_units, +}; /// Part of that limit kept for what may change between provision and start, such as the /// configured OpenVMM and guest paths. const PROCESS_COMMAND_LINE_RESERVE: usize = 1024; @@ -338,7 +337,7 @@ impl VmmLaunch<'_> { /// plan: a quoted final path of up to `MAX_PATH` behind `\\?\` in place of a path of at least /// 4. A launch whose inputs are longer may still not fit, which its complete check refuses /// before OpenVMM starts. - pub const RESERVE: usize = 2048; + pub const RESERVE: usize = config::OPENVMM_COMMAND_LINE_RESERVE; /// Returns the arguments that name the launch's inputs, with the kernel command line /// `kernel_command_line`: the VM in one process, `--single-process`, on the hypervisor, @@ -349,30 +348,19 @@ impl VmmLaunch<'_> { /// OpenVMM reads from its standard input, `--microvm-control-auth-stdin`, and the outcome /// report, `--microvm-report`. pub fn arguments(&self, kernel_command_line: &str) -> Vec { - let memory = format!("{}M", self.memory_mib); - let control = format!("listen={}", self.endpoint); - [ - OsStr::new("--single-process"), - OsStr::new("--hypervisor"), - OsStr::new(self.hypervisor.as_str()), - OsStr::new("--memory"), - OsStr::new(&memory), - OsStr::new("--kernel"), - self.kernel.as_os_str(), - OsStr::new("--initrd"), - self.initrd.as_os_str(), - OsStr::new("--cmdline"), - OsStr::new(kernel_command_line), - OsStr::new("--virtio-console"), - OsStr::new("none"), - OsStr::new("--microvm-control-console"), - OsStr::new(&control), - OsStr::new("--microvm-control-auth-stdin"), - OsStr::new("--microvm-report"), - self.report.as_os_str(), - ] - .map(OsStr::to_os_string) - .into() + let mut arguments = config::boot_arguments( + self.hypervisor, + u64::from(self.memory_mib), + self.kernel, + self.initrd, + kernel_command_line, + ); + arguments.extend(config::console_arguments( + "none", + self.endpoint, + self.report, + )); + arguments } } @@ -464,98 +452,5 @@ pub fn timeout_problem(value: Duration, positive: bool) -> Option { }) } -/// Returns the length in UTF-16 code units, including the terminating NUL, of the Windows command -/// line that runs `program` with `arguments`, quoted the way the Windows platform quotes it: the -/// program always in quotes, and each argument in quotes if it is empty or holds a space or a -/// tab, with a backslash before each quote that it holds and a backslash before each backslash -/// that precedes a quote, its closing quote included, and one space between them. -/// -/// It is the length that [`PROCESS_COMMAND_LINE_LIMIT`] bounds, whatever the host: -/// [`fit_process_command_line`] checks it. -pub fn process_command_line_units(program: &OsStr, arguments: &[OsString]) -> usize { - argument_units(program, true) + arguments_units(arguments) -} - -/// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) unless the Windows command -/// line that runs `program`, such as OpenVMM, with `arguments`, as [`process_command_line_units`] -/// counts it, leaves `reserve` code units of [`PROCESS_COMMAND_LINE_LIMIT`] unused: none for the -/// complete command line of a launch about to spawn OpenVMM, and some for what may change before -/// then, such as the paths of the artifacts. -/// -/// The error says how long the command line is, and how to shorten it. -pub fn fit_process_command_line( - program: &OsStr, - arguments: &[OsString], - reserve: usize, -) -> Result<()> { - let length = process_command_line_units(program, arguments); - let held = if reserve == 0 { - String::new() - } else { - format!(", with {reserve} held in reserve") - }; - fit("OpenVMM's command line", length, reserve, &held) -} - -/// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) unless OpenVMM's -/// `arguments`, which a plan renders, without the program and the rest of the launch, leave -/// `reserve` code units of [`PROCESS_COMMAND_LINE_LIMIT`] to that rest. -pub(crate) fn fit_arguments(arguments: &[OsString], reserve: usize) -> Result<()> { - let held = format!(", with {reserve} held in reserve for the rest of the launch"); - fit( - "OpenVMM's arguments", - arguments_units(arguments), - reserve, - &held, - ) -} - -/// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported), saying that `subject` -/// would take `length` code units, `held` describing `reserve`, unless they leave `reserve` of -/// [`PROCESS_COMMAND_LINE_LIMIT`] unused. -fn fit(subject: &str, length: usize, reserve: usize, held: &str) -> Result<()> { - if length.saturating_add(reserve) <= PROCESS_COMMAND_LINE_LIMIT { - return Ok(()); - } - Err(Error::unsupported(format!( - "{subject} would take {length} of the {PROCESS_COMMAND_LINE_LIMIT} characters that a \ - Windows process command line allows{held}; map, hide, or allow fewer or shorter \ - filesystem paths, or use fewer network rules" - ))) -} - -/// Returns the length in UTF-16 code units of `arguments` on a Windows command line, quoted as -/// [`process_command_line_units`] quotes them, each with the space before it. -fn arguments_units(arguments: &[OsString]) -> usize { - arguments - .iter() - .map(|argument| argument_units(argument, false)) - .sum() -} - -/// Returns the length in UTF-16 code units of `argument` on a Windows command line, quoted as -/// [`process_command_line_units`] quotes it, always if it is the `program`, with one separating -/// space, or the terminating NUL after the last argument. -fn argument_units(argument: &OsStr, program: bool) -> usize { - let text = argument.to_string_lossy(); - let quoted = program || text.is_empty() || text.contains([' ', '\t']); - let mut length = 1 + text.encode_utf16().count() + if quoted { 2 } else { 0 }; - let mut backslashes = 0; - for character in text.chars() { - match character { - '\\' => backslashes += 1, - '"' => { - length += backslashes + 1; - backslashes = 0; - } - _ => backslashes = 0, - } - } - if quoted { - length += backslashes; - } - length -} - #[cfg(test)] mod tests; diff --git a/nvx-rs/src/host/launch/tests.rs b/nvx-rs/src/host/launch/tests.rs index 371817c6f..2cc5fa0ca 100644 --- a/nvx-rs/src/host/launch/tests.rs +++ b/nvx-rs/src/host/launch/tests.rs @@ -2,6 +2,7 @@ use proptest::prelude::*; use super::*; use crate::ErrorKind; +use crate::host::config::arguments_units; use crate::host::filesystem::{Bind, ExportChild, Owner}; use crate::protocol::cmdline::tests::model; @@ -114,29 +115,6 @@ fn mappings_add_the_export_and_the_mapping_count() { ); } -#[test] -fn process_command_lines_are_measured_as_windows_quotes_them() { - let cases: [(&str, &[&str], &str); 5] = [ - ( - r"C:\Program Files\openvmm.exe", - &["--a", "b c", ""], - r#""C:\Program Files\openvmm.exe" --a "b c" """#, - ), - ("x", &[r#"say "hi""#], r#""x" "say \"hi\"""#), - ("x", &[r#"a"b"#], r#""x" a\"b"#), - ("x", &[r"dir\ with\"], r#""x" "dir\ with\\""#), - ("x", &[r"a\\b", "é"], r#""x" a\\b é"#), - ]; - for (program, arguments, line) in cases { - let arguments: Vec = arguments.iter().map(OsString::from).collect(); - assert_eq!( - process_command_line_units(OsStr::new(program), &arguments), - line.encode_utf16().count() + 1, - "{line}" - ); - } -} - #[test] fn oversized_process_command_lines_are_rejected() { let mut spec = spec(); @@ -200,48 +178,6 @@ fn launches_may_use_the_reserve_that_provisioning_keeps() { ); } -#[test] -fn a_process_command_line_fits_while_it_leaves_its_reserve() { - // Any program and arguments, such as an adapter's: a command line of the limit fits, - // one more code unit does not, and a reserve takes its part of the limit. - let arguments: Vec = ["--a", "b c"].map(OsString::from).into(); - let base = process_command_line_units(OsStr::new("/o"), &arguments); - let program = |extra: usize| OsString::from(format!("/o{}", "o".repeat(extra))); - let fitting = program(PROCESS_COMMAND_LINE_LIMIT - base); - assert_eq!( - process_command_line_units(&fitting, &arguments), - PROCESS_COMMAND_LINE_LIMIT - ); - fit_process_command_line(&fitting, &arguments, 0).unwrap(); - let held = fit_process_command_line(&fitting, &arguments, 1).unwrap_err(); - assert_eq!(held.kind(), ErrorKind::Unsupported); - assert!( - held.to_string() - .contains("would take 32767 of the 32767 characters"), - "{held}" - ); - assert!( - held.to_string().contains("with 1 held in reserve"), - "{held}" - ); - let longer = program(PROCESS_COMMAND_LINE_LIMIT - base + 1); - let error = fit_process_command_line(&longer, &arguments, 0).unwrap_err(); - assert_eq!(error.kind(), ErrorKind::Unsupported); - // A reserve past the limit leaves no room for any command line. - assert!(fit_process_command_line(OsStr::new("o"), &[], usize::MAX).is_err()); - // The arguments alone, which a plan renders, take what they take on the command line. - let alone = process_command_line_units(OsStr::new(""), &arguments) - - process_command_line_units(OsStr::new(""), &[]); - let reserve = PROCESS_COMMAND_LINE_LIMIT - alone; - fit_arguments(&arguments, reserve).unwrap(); - let held = fit_arguments(&arguments, reserve + 1).unwrap_err(); - assert!( - held.to_string() - .contains("held in reserve for the rest of the launch"), - "{held}" - ); -} - #[test] fn every_launch_names_its_own_inputs_alike() { let launch = VmmLaunch { diff --git a/nvx-rs/src/host/runtime/mod.rs b/nvx-rs/src/host/runtime/mod.rs index 90ba8edd2..9310b0342 100644 --- a/nvx-rs/src/host/runtime/mod.rs +++ b/nvx-rs/src/host/runtime/mod.rs @@ -47,22 +47,24 @@ pub struct Runtime { /// attaches its own. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct LaunchOptions { - /// What OpenVMM does when the guest powers off. + /// What OpenVMM does on a plain guest power-off or hibernate event. A guest power-off that + /// reports an exit status always exits OpenVMM with that status, regardless of this action. pub guest_shutdown: GuestPowerAction, /// How long OpenVMM waits for the host to authenticate on the control console, or `None` for - /// OpenVMM's default. + /// OpenVMM's default. The cold-boot renderer accepts whole milliseconds from 1 to 60000. pub control_auth_timeout: Option, } -/// What OpenVMM does when the guest powers off. +/// What OpenVMM does on a plain guest power-off or hibernate event. A power-off with an exit +/// status always exits OpenVMM with that status instead. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)] pub enum GuestPowerAction { - /// OpenVMM exits, which ends the VM: the default, on which stopping a VM relies. + /// OpenVMM exits, which ends the VM: the default. #[default] Exit, - /// OpenVMM halts the VM and keeps running. + /// OpenVMM halts the VM and keeps running, unless the power-off reports an exit status. Halt, - /// OpenVMM restarts the guest. + /// OpenVMM restarts the guest, unless the power-off reports an exit status. Reset, } From 6d27d82db2252b306f46dc8d2857b6bee6d8c819 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Fri, 6 Nov 2026 19:36:50 -0800 Subject: [PATCH 02/10] host: document cold-boot argument validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0325d032-38a9-4c7f-9899-687a4ec93a51 --- nvx-rs/src/host/config/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nvx-rs/src/host/config/mod.rs b/nvx-rs/src/host/config/mod.rs index 999778e13..9a23f302c 100644 --- a/nvx-rs/src/host/config/mod.rs +++ b/nvx-rs/src/host/config/mod.rs @@ -906,6 +906,7 @@ pub(crate) fn console_arguments(console: &str, endpoint: &str, report: &Path) -> .into() } +/// Refuses empty or NUL-containing caller-selected endpoint names. fn check_endpoint(endpoint: &str, name: &str) -> Result<()> { if endpoint.is_empty() || endpoint.contains('\0') { return Err(Error::unsupported(format!( @@ -915,6 +916,7 @@ fn check_endpoint(endpoint: &str, name: &str) -> Result<()> { Ok(()) } +/// Checks OpenVMM arguments for NULs that cannot be passed to a process. fn check_argument_nuls(arguments: &[OsString]) -> Result<()> { if arguments .iter() From c33e256de664e9b97f4e159701606a762b344b7d Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Fri, 6 Nov 2026 22:07:14 -0800 Subject: [PATCH 03/10] host: reject unrepresentable cold-boot disk paths Reuse the pure disk-path validator for layered image and scratch arguments before either configuration preflight or whole rendering returns. Move the shared guard into config and retain its sandbox re-export, preserving diagnostics and host module layering. Add a regression for disk delimiters and bracket balance through both entry points, including valid literal paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0325d032-38a9-4c7f-9899-687a4ec93a51 --- nvx-rs/src/host/config/mod.rs | 31 ++++++++++++-- nvx-rs/src/host/config/tests.rs | 76 +++++++++++++++++++++++++++++++++ nvx-rs/src/host/sandbox/mod.rs | 22 +--------- 3 files changed, 105 insertions(+), 24 deletions(-) diff --git a/nvx-rs/src/host/config/mod.rs b/nvx-rs/src/host/config/mod.rs index 9a23f302c..63005cfd0 100644 --- a/nvx-rs/src/host/config/mod.rs +++ b/nvx-rs/src/host/config/mod.rs @@ -450,8 +450,9 @@ impl VmConfig { /// `runtime.windows_breakaway` is process-launch behavior, not an OpenVMM argument. /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused - /// configuration, no [`Boot`], empty or NUL-containing required names or runtime paths, an - /// invalid authentication timeout, or a complete command line beyond the Windows limit. + /// configuration, no [`Boot`], empty or NUL-containing required names or runtime paths, + /// block paths with disk-option delimiters or unbalanced brackets, an invalid authentication + /// timeout, or a complete command line beyond the Windows limit. /// The complete limit check runs on every host, without a reserve. pub fn openvmm_arguments( &self, @@ -543,7 +544,8 @@ impl VmConfig { /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused /// configuration or no [`Boot`], an unlowerable network policy, a NUL-containing argument, - /// or insufficient room for the reserve. + /// a block path that OpenVMM's disk syntax cannot hold literally, or insufficient room + /// for the reserve. pub fn check_command_line(&self) -> Result<()> { let command_line = self.kernel_command_line()?; let mut arguments = self.device_arguments()?; @@ -572,11 +574,13 @@ impl VmConfig { self.processors.to_string().into(), ]; if let RootConfig::Layered(root) = &self.root { + reject_disk_path(&root.image, &root.image.display().to_string())?; let mut image = OsString::from("distro:file:"); image.push(&root.image); image.push(",ro"); arguments.extend(["--microvm-sandbox-block".into(), image]); if let UpperConfig::Scratch { path, .. } = &root.upper { + reject_disk_path(path, &path.display().to_string())?; let mut scratch = OsString::from("scratch:file:"); scratch.push(path); arguments.extend(["--microvm-sandbox-block".into(), scratch]); @@ -864,6 +868,27 @@ impl Identifying { } } +/// Refuses a disk path that OpenVMM's `--microvm-sandbox-block` value could not hold: one with +/// a comma, semicolon, or improperly nested brackets. The message shows `path` as `shown`. +pub(crate) fn reject_disk_path(path: &Path, shown: &str) -> Result<()> { + let depth = path + .to_string_lossy() + .bytes() + .try_fold(0_usize, |depth, byte| match byte { + b'[' => Some(depth + 1), + b']' => depth.checked_sub(1), + b',' | b';' => None, + _ => Some(depth), + }); + if depth != Some(0) { + return Err(Error::unsupported(format!( + "disk paths containing commas, semicolons, or unbalanced brackets are unsupported: \ + {shown}" + ))); + } + Ok(()) +} + /// Common cold-boot inputs, shared with the legacy VmmLaunch without narrowing its public types. pub(crate) fn boot_arguments( hypervisor: Hypervisor, diff --git a/nvx-rs/src/host/config/tests.rs b/nvx-rs/src/host/config/tests.rs index df7551c82..f66ac47af 100644 --- a/nvx-rs/src/host/config/tests.rs +++ b/nvx-rs/src/host/config/tests.rs @@ -1278,6 +1278,82 @@ fn cold_boot_arguments_lower_layered_roots_networks_and_exports_by_path() { } } +#[test] +fn cold_boot_arguments_refuse_disk_syntax_in_image_and_scratch_paths() { + let runtime = runtime(); + for name in [ + "disk;direct", + "disk;create=1M", + "disk,ro", + "disk[", + "disk]", + "disk][", + ] { + for scratch in [false, true] { + let mut root = erofs(); + if scratch { + root.upper = UpperConfig::Scratch { + path: missing(name), + journal: Journal::Forbidden, + }; + } else { + root.image = missing(name); + } + let config = cold(layered(root)); + for result in [ + config.check_command_line(), + config + .openvmm_arguments( + &runtime, + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .map(|_| ()), + ] { + let error = result.unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + assert!( + error + .to_string() + .contains("commas, semicolons, or unbalanced brackets"), + "{name}: {error}" + ); + } + } + } + + let image = missing("image with spaces[1].erofs"); + let scratch = missing("scratch with [nested[1]] brackets.ext4"); + let config = cold(layered(LayeredRoot { + image: image.clone(), + upper: UpperConfig::Scratch { + path: scratch.clone(), + journal: Journal::Forbidden, + }, + ..erofs() + })); + config.check_command_line().unwrap(); + let arguments = config + .openvmm_arguments( + &runtime, + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .unwrap(); + let blocks: Vec = [ + format!("distro:file:{},ro", image.display()), + format!("scratch:file:{}", scratch.display()), + ] + .map(OsString::from) + .into(); + assert_eq!( + values(&arguments, "--microvm-sandbox-block"), + blocks.iter().map(OsString::as_os_str).collect::>() + ); +} + #[test] fn an_explicit_deny_all_network_keeps_its_nic() { let mut config = cold(blockless()); diff --git a/nvx-rs/src/host/sandbox/mod.rs b/nvx-rs/src/host/sandbox/mod.rs index c602b6813..cfa0f54e9 100644 --- a/nvx-rs/src/host/sandbox/mod.rs +++ b/nvx-rs/src/host/sandbox/mod.rs @@ -75,6 +75,7 @@ pub use self::plan::{ pub use self::prepared::PreparedLaunch; use crate::error::{Error, ErrorCode, Result}; use crate::host::artifacts::require_existing; +pub(crate) use crate::host::config::reject_disk_path; use crate::host::filesystem::ExportPlan; use crate::net::is_valid_hostname; use crate::protocol::cmdline; @@ -162,27 +163,6 @@ pub fn require_file(path: &Path, description: &str) -> Result<()> { require_existing(path, description, ErrorCode::Unavailable) } -/// Refuses a disk path that OpenVMM's `--microvm-sandbox-block` value could not hold: one with -/// a comma, semicolon, or improperly nested brackets. The message shows `path` as `shown`. -pub(crate) fn reject_disk_path(path: &Path, shown: &str) -> Result<()> { - let depth = path - .to_string_lossy() - .bytes() - .try_fold(0_usize, |depth, byte| match byte { - b'[' => Some(depth + 1), - b']' => depth.checked_sub(1), - b',' | b';' => None, - _ => Some(depth), - }); - if depth != Some(0) { - return Err(refused(format!( - "disk paths containing commas, semicolons, or unbalanced brackets are unsupported: \ - {shown}" - ))); - } - Ok(()) -} - /// The extensions of the files that OpenVMM does not attach as the raw image of a sandbox block /// device: it opens a `.vhd`, `.vhdx`, or `.vmgs` file in its container format, refuses to write /// an `.iso` file, and refuses all four for the block devices of a snapshot. From 3403bf3820780aa06d56bd3338786e88a247a187 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Fri, 6 Nov 2026 22:57:19 -0800 Subject: [PATCH 04/10] host: refuse unsupported control-console kernel syntax Reject double quotes and standalone -- only in the cold renderer and command-line preflight, which always describe a control-console launch. Preserve caller-owned configuration checking and kernel projection, and replace only the cold-renderer fixture with a compatible token. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0325d032-38a9-4c7f-9899-687a4ec93a51 --- nvx-rs/src/host/config/mod.rs | 27 +++++++++++++--- nvx-rs/src/host/config/tests.rs | 56 +++++++++++++++++++++++++++++++-- 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/nvx-rs/src/host/config/mod.rs b/nvx-rs/src/host/config/mod.rs index 63005cfd0..8ebb8d2a1 100644 --- a/nvx-rs/src/host/config/mod.rs +++ b/nvx-rs/src/host/config/mod.rs @@ -452,7 +452,8 @@ impl VmConfig { /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused /// configuration, no [`Boot`], empty or NUL-containing required names or runtime paths, /// block paths with disk-option delimiters or unbalanced brackets, an invalid authentication - /// timeout, or a complete command line beyond the Windows limit. + /// timeout, kernel text with double quotes or the standalone `--` delimiter, or a complete + /// command line beyond the Windows limit. /// The complete limit check runs on every host, without a reserve. pub fn openvmm_arguments( &self, @@ -461,7 +462,7 @@ impl VmConfig { report: &Path, console_endpoint: Option<&str>, ) -> Result> { - let command_line = self.kernel_command_line()?; + let command_line = self.control_kernel_command_line()?; check_endpoint(endpoint, "endpoint")?; for (name, path) in [ ("Runtime.openvmm", runtime.openvmm.as_path()), @@ -544,10 +545,10 @@ impl VmConfig { /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused /// configuration or no [`Boot`], an unlowerable network policy, a NUL-containing argument, - /// a block path that OpenVMM's disk syntax cannot hold literally, or insufficient room - /// for the reserve. + /// a block path that OpenVMM's disk syntax cannot hold literally, kernel text with double + /// quotes or the standalone `--` delimiter, or insufficient room for the reserve. pub fn check_command_line(&self) -> Result<()> { - let command_line = self.kernel_command_line()?; + let command_line = self.control_kernel_command_line()?; let mut arguments = self.device_arguments()?; arguments.extend([ "--memory".into(), @@ -565,6 +566,22 @@ impl VmConfig { fit_arguments(&arguments, OPENVMM_COMMAND_LINE_RESERVE) } + /// Renders kernel text without the double quotes or delimiter OpenVMM's control console refuses. + fn control_kernel_command_line(&self) -> Result { + let command_line = self.kernel_command_line()?; + if command_line.contains('"') + || command_line + .split_ascii_whitespace() + .any(|token| token == "--") + { + return Err(Error::unsupported( + "boot.command_line cannot contain double quotes or a standalone -- with the \ + OpenVMM control console", + )); + } + Ok(command_line) + } + /// Renders only the configuration's devices; no runtime or output name is needed. fn device_arguments(&self) -> Result> { let mut arguments = vec![ diff --git a/nvx-rs/src/host/config/tests.rs b/nvx-rs/src/host/config/tests.rs index f66ac47af..8257027a3 100644 --- a/nvx-rs/src/host/config/tests.rs +++ b/nvx-rs/src/host/config/tests.rs @@ -1118,7 +1118,7 @@ fn runtime() -> Runtime { fn cold(config: VmConfig) -> VmConfig { VmConfig { boot: Some(Boot { - command_line: "quiet nvx_entrypoint=/bin/work nvx_arg=\"a b\"".to_owned(), + command_line: "quiet nvx_entrypoint=/bin/work nvx_arg=ab".to_owned(), capture: None, }), ..config @@ -1133,6 +1133,58 @@ fn values<'a>(arguments: &'a [OsString], option: &str) -> Vec<&'a OsStr> { .collect() } +#[test] +fn cold_boot_command_lines_refuse_native_control_console_syntax() { + let runtime = runtime(); + for console in [ConsoleAttachment::Listener, ConsoleAttachment::Disconnected] { + for (caller, refused) in [ + (r#"nvx_arg="a b""#, true), + ("note=\"", true), + ("--", true), + ("quiet -- init", true), + ("quiet\t--\r\ninit", true), + ("nvx_arg='a'", false), + ("nvx_arg=--", false), + ("--not-a-delimiter", false), + ("--=1", false), + ("quiet\t--not-a-delimiter\nnote=ok", false), + ] { + let config = VmConfig { + boot: Some(Boot { + command_line: caller.to_owned(), + }), + console, + ..blockless() + }; + config.check().unwrap(); + let command_line = config.kernel_command_line().unwrap(); + assert!(command_line.ends_with(caller)); + let rendered = config.openvmm_arguments( + &runtime, + "/missing/control", + Path::new("/missing/report"), + (console == ConsoleAttachment::Listener).then_some("/missing/console"), + ); + if refused { + for result in [config.check_command_line(), rendered.map(|_| ())] { + let error = result.unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + assert!( + error.to_string().contains("control console"), + "{caller:?}: {error}" + ); + } + } else { + config.check_command_line().unwrap(); + assert_eq!( + values(&rendered.unwrap(), "--cmdline"), + [OsStr::new(&command_line)] + ); + } + } + } +} + #[test] fn cold_boot_arguments_select_the_configured_guest_without_managed_tokens() { for hypervisor in [Hypervisor::Kvm, Hypervisor::Mshv, Hypervisor::Whp] { @@ -1168,7 +1220,7 @@ fn cold_boot_arguments_select_the_configured_guest_without_managed_tokens() { OsStr::new("--kernel"), runtime.kernel.as_os_str(), OsStr::new("--initrd"), runtime.initramfs.as_os_str(), OsStr::new("--cmdline"), - OsStr::new("nvx_hostname=626f78 nvx_machine_id=image quiet nvx_entrypoint=/bin/work nvx_arg=\"a b\""), + OsStr::new("nvx_hostname=626f78 nvx_machine_id=image quiet nvx_entrypoint=/bin/work nvx_arg=ab"), OsStr::new("--virtio-console"), OsStr::new(console_backend), OsStr::new("--microvm-control-console"), OsStr::new("listen=/missing/control.sock"), OsStr::new("--microvm-control-auth-stdin"), From 73fed78fe91eff9c599a4dcf484fd697a221d3fd Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Fri, 6 Nov 2026 23:59:55 -0800 Subject: [PATCH 05/10] host: refuse unrepresentable native argument names Require Unicode path-based block arguments and distinct boot/control listeners, using the declared WHP runtime for ASCII-case-insensitive pipe names. Preserve configuration/kernel projection and the legacy disk-path guard; add no host I/O or canonicalization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0325d032-38a9-4c7f-9899-687a4ec93a51 --- nvx-rs/src/host/config/mod.rs | 40 ++++++++--- nvx-rs/src/host/config/tests.rs | 113 ++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 10 deletions(-) diff --git a/nvx-rs/src/host/config/mod.rs b/nvx-rs/src/host/config/mod.rs index 8ebb8d2a1..2b93e8f78 100644 --- a/nvx-rs/src/host/config/mod.rs +++ b/nvx-rs/src/host/config/mod.rs @@ -442,6 +442,7 @@ impl VmConfig { /// given. A [`ConsoleAttachment::Listener`] needs a nonempty `console_endpoint`; a /// [`ConsoleAttachment::Disconnected`] needs none and retains the console device with a /// discarded attachment. The control endpoint always listens and authenticates from stdin. + /// Listener names must be distinct, with ASCII-case-insensitive comparison for WHP. /// The caller runs `runtime.openvmm` with these arguments. /// /// This whole-config entry point is temporarily public. SL2.8 makes it internal when @@ -451,9 +452,9 @@ impl VmConfig { /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused /// configuration, no [`Boot`], empty or NUL-containing required names or runtime paths, - /// block paths with disk-option delimiters or unbalanced brackets, an invalid authentication - /// timeout, kernel text with double quotes or the standalone `--` delimiter, or a complete - /// command line beyond the Windows limit. + /// duplicate listener names, block paths that are not Unicode or hold disk-option delimiters + /// or unbalanced brackets, an invalid authentication timeout, kernel text with double quotes + /// or the standalone `--` delimiter, or a complete command line beyond the Windows limit. /// The complete limit check runs on every host, without a reserve. pub fn openvmm_arguments( &self, @@ -477,9 +478,17 @@ impl VmConfig { } } let console = match (self.console, console_endpoint) { - (ConsoleAttachment::Listener, Some(endpoint)) => { - check_endpoint(endpoint, "console_endpoint")?; - format!("listen={endpoint}") + (ConsoleAttachment::Listener, Some(console_endpoint)) => { + check_endpoint(console_endpoint, "console_endpoint")?; + if console_endpoint == endpoint + || (runtime.hypervisor == Hypervisor::Whp + && console_endpoint.eq_ignore_ascii_case(endpoint)) + { + return Err(Error::unsupported( + "boot and control console listeners require distinct endpoints", + )); + } + format!("listen={console_endpoint}") } (ConsoleAttachment::Disconnected, None) => "none".to_owned(), (ConsoleAttachment::Listener, None) => { @@ -545,8 +554,9 @@ impl VmConfig { /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused /// configuration or no [`Boot`], an unlowerable network policy, a NUL-containing argument, - /// a block path that OpenVMM's disk syntax cannot hold literally, kernel text with double - /// quotes or the standalone `--` delimiter, or insufficient room for the reserve. + /// a non-Unicode block path or one that OpenVMM's disk syntax cannot hold literally, kernel + /// text with double quotes or the standalone `--` delimiter, or insufficient room for the + /// reserve. pub fn check_command_line(&self) -> Result<()> { let command_line = self.control_kernel_command_line()?; let mut arguments = self.device_arguments()?; @@ -591,13 +601,23 @@ impl VmConfig { self.processors.to_string().into(), ]; if let RootConfig::Layered(root) = &self.root { - reject_disk_path(&root.image, &root.image.display().to_string())?; + let scratch = match &root.upper { + UpperConfig::Scratch { path, .. } => Some(path), + UpperConfig::Ram { .. } => None, + }; + for path in [Some(&root.image), scratch].into_iter().flatten() { + if path.to_str().is_none() { + return Err(Error::unsupported( + "block paths must be Unicode for OpenVMM's string arguments", + )); + } + reject_disk_path(path, &path.display().to_string())?; + } let mut image = OsString::from("distro:file:"); image.push(&root.image); image.push(",ro"); arguments.extend(["--microvm-sandbox-block".into(), image]); if let UpperConfig::Scratch { path, .. } = &root.upper { - reject_disk_path(path, &path.display().to_string())?; let mut scratch = OsString::from("scratch:file:"); scratch.push(path); arguments.extend(["--microvm-sandbox-block".into(), scratch]); diff --git a/nvx-rs/src/host/config/tests.rs b/nvx-rs/src/host/config/tests.rs index 8257027a3..916d0f4e6 100644 --- a/nvx-rs/src/host/config/tests.rs +++ b/nvx-rs/src/host/config/tests.rs @@ -1185,6 +1185,119 @@ fn cold_boot_command_lines_refuse_native_control_console_syntax() { } } +#[test] +fn native_argument_listener_names_are_distinct() { + let config = cold(blockless()); + let endpoint = r"\\.\pipe\nvx-console"; + for hypervisor in [Hypervisor::Kvm, Hypervisor::Mshv, Hypervisor::Whp] { + let runtime = Runtime { + hypervisor, + ..runtime() + }; + for console_endpoint in [endpoint, r"\\.\PIPE\NVX-CONSOLE", r"\\.\pipe\nvx-boot"] { + let rendered = config.openvmm_arguments( + &runtime, + endpoint, + Path::new("/missing/report"), + Some(console_endpoint), + ); + if console_endpoint == endpoint + || (hypervisor == Hypervisor::Whp + && console_endpoint.eq_ignore_ascii_case(endpoint)) + { + let error = rendered.unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + assert!(error.to_string().contains("distinct endpoints")); + } else { + let arguments = rendered.unwrap(); + assert_eq!( + values(&arguments, "--virtio-console"), + [OsStr::new(&format!("listen={console_endpoint}"))] + ); + } + } + let disconnected = VmConfig { + console: ConsoleAttachment::Disconnected, + ..config.clone() + }; + let arguments = disconnected + .openvmm_arguments(&runtime, endpoint, Path::new("/missing/report"), None) + .unwrap(); + assert_eq!(values(&arguments, "--virtio-console"), [OsStr::new("none")]); + } +} + +#[cfg(any(unix, windows))] +#[test] +fn native_argument_paths_require_unicode() { + #[cfg(unix)] + let invalid = { + use std::os::unix::ffi::OsStringExt; + PathBuf::from(OsString::from_vec(vec![b'i', 0xff])) + }; + #[cfg(windows)] + let invalid = { + use std::os::windows::ffi::OsStringExt; + PathBuf::from(OsString::from_wide(&[105, 0xd800])) + }; + for scratch in [false, true] { + let mut root = erofs(); + if scratch { + root.upper = UpperConfig::Scratch { + path: invalid.clone(), + journal: Journal::Forbidden, + }; + } else { + root.image = invalid.clone(); + } + let config = cold(layered(root)); + config.check().unwrap(); + config.kernel_command_line().unwrap(); + for result in [ + config.check_command_line(), + config + .openvmm_arguments( + &runtime(), + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .map(|_| ()), + ] { + let error = result.unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + assert!(error.to_string().contains("Unicode")); + } + } + + let image = PathBuf::from("image-\u{e9} with [nested[1]].erofs"); + let scratch = PathBuf::from("scratch-\u{f1} with spaces.ext4"); + let config = cold(layered(LayeredRoot { + image: image.clone(), + upper: UpperConfig::Scratch { + path: scratch.clone(), + journal: Journal::Forbidden, + }, + ..erofs() + })); + config.check_command_line().unwrap(); + let arguments = config + .openvmm_arguments( + &runtime(), + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .unwrap(); + assert_eq!( + values(&arguments, "--microvm-sandbox-block"), + [ + OsStr::new(&format!("distro:file:{},ro", image.display())), + OsStr::new(&format!("scratch:file:{}", scratch.display())), + ] + ); +} + #[test] fn cold_boot_arguments_select_the_configured_guest_without_managed_tokens() { for hypervisor in [Hypervisor::Kvm, Hypervisor::Mshv, Hypervisor::Whp] { From 6e709c0e97cc868eb7fc4d154144c7a4a27ffd7d Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 17:30:22 -0800 Subject: [PATCH 06/10] host: reject non-raw block path extensions Reuse pure block-path validation in both configuration entry points. Reject lower-case container backend extensions, including VMDK, and reject ISO only for writable scratch while preserving native case-sensitive raw paths and the legacy sandbox alias. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0325d032-38a9-4c7f-9899-687a4ec93a51 --- nvx-rs/src/host/config/mod.rs | 46 ++++++++++++++++---- nvx-rs/src/host/config/tests.rs | 74 +++++++++++++++++++++++++++++++++ nvx-rs/src/host/sandbox/mod.rs | 26 +----------- 3 files changed, 113 insertions(+), 33 deletions(-) diff --git a/nvx-rs/src/host/config/mod.rs b/nvx-rs/src/host/config/mod.rs index 2b93e8f78..71e5f9903 100644 --- a/nvx-rs/src/host/config/mod.rs +++ b/nvx-rs/src/host/config/mod.rs @@ -452,9 +452,10 @@ impl VmConfig { /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused /// configuration, no [`Boot`], empty or NUL-containing required names or runtime paths, - /// duplicate listener names, block paths that are not Unicode or hold disk-option delimiters - /// or unbalanced brackets, an invalid authentication timeout, kernel text with double quotes - /// or the standalone `--` delimiter, or a complete command line beyond the Windows limit. + /// duplicate listener names, block paths that are not Unicode, hold disk-option delimiters or + /// unbalanced brackets, or select a non-raw OpenVMM backend, an invalid authentication timeout, + /// kernel text with double quotes or the standalone `--` delimiter, or a complete command line + /// beyond the Windows limit. /// The complete limit check runs on every host, without a reserve. pub fn openvmm_arguments( &self, @@ -554,9 +555,9 @@ impl VmConfig { /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused /// configuration or no [`Boot`], an unlowerable network policy, a NUL-containing argument, - /// a non-Unicode block path or one that OpenVMM's disk syntax cannot hold literally, kernel - /// text with double quotes or the standalone `--` delimiter, or insufficient room for the - /// reserve. + /// a non-Unicode block path, one that OpenVMM's disk syntax cannot hold literally, or one whose + /// extension selects a non-raw backend, kernel text with double quotes or the standalone `--` + /// delimiter, or insufficient room for the reserve. pub fn check_command_line(&self) -> Result<()> { let command_line = self.control_kernel_command_line()?; let mut arguments = self.device_arguments()?; @@ -605,13 +606,18 @@ impl VmConfig { UpperConfig::Scratch { path, .. } => Some(path), UpperConfig::Ram { .. } => None, }; - for path in [Some(&root.image), scratch].into_iter().flatten() { + for (path, writable) in [(Some(&root.image), false), (scratch, true)] + .into_iter() + .filter_map(|(path, writable)| path.map(|path| (path, writable))) + { if path.to_str().is_none() { return Err(Error::unsupported( "block paths must be Unicode for OpenVMM's string arguments", )); } - reject_disk_path(path, &path.display().to_string())?; + let shown = path.display().to_string(); + reject_disk_path(path, &shown)?; + reject_container_backend(path, &shown, writable)?; } let mut image = OsString::from("distro:file:"); image.push(&root.image); @@ -926,6 +932,30 @@ pub(crate) fn reject_disk_path(path: &Path, shown: &str) -> Result<()> { Ok(()) } +/// Extensions for which OpenVMM selects a container backend instead of a raw file. +const CONTAINER_EXTENSIONS: [&str; 4] = ["vhd", "vhdx", "vmgs", "vmdk"]; + +/// Refuses a block path when OpenVMM would not attach it as the requested raw image. +fn reject_container_backend(path: &Path, shown: &str, writable: bool) -> Result<()> { + let name = path.file_name().map(|name| name.to_string_lossy()); + let extension = match name.as_deref().unwrap_or_default().rsplit_once('.') { + Some((stem, extension)) if !stem.is_empty() => extension, + _ => "", + }; + if CONTAINER_EXTENSIONS.contains(&extension) || (writable && extension == "iso") { + return Err(Error::unsupported(format!( + "disk paths ending in .{extension} are unsupported, since OpenVMM would not attach \ + them as raw images: {shown}" + ))); + } + Ok(()) +} + +/// Refuses a legacy sandbox path that OpenVMM would not attach as a raw block image. +pub(crate) fn reject_container_path(path: &Path, shown: &str) -> Result<()> { + reject_container_backend(path, shown, true) +} + /// Common cold-boot inputs, shared with the legacy VmmLaunch without narrowing its public types. pub(crate) fn boot_arguments( hypervisor: Hypervisor, diff --git a/nvx-rs/src/host/config/tests.rs b/nvx-rs/src/host/config/tests.rs index 916d0f4e6..ab4515ffa 100644 --- a/nvx-rs/src/host/config/tests.rs +++ b/nvx-rs/src/host/config/tests.rs @@ -1519,6 +1519,80 @@ fn cold_boot_arguments_refuse_disk_syntax_in_image_and_scratch_paths() { ); } +#[test] +fn cold_boot_arguments_refuse_non_raw_block_extensions() { + let runtime = runtime(); + for (extension, scratch_only) in [ + ("vhd", false), + ("vhdx", false), + ("vmgs", false), + ("vmdk", false), + ("iso", true), + ] { + for scratch in [false, true] { + if scratch_only && !scratch { + continue; + } + let name = format!("disk.{extension}"); + let mut root = erofs(); + if scratch { + root.upper = UpperConfig::Scratch { + path: missing(&name), + journal: Journal::Forbidden, + }; + } else { + root.image = missing(&name); + } + let config = cold(layered(root)); + for result in [ + config.check_command_line(), + config + .openvmm_arguments( + &runtime, + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .map(|_| ()), + ] { + let error = result.unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + assert!(error.to_string().contains(&format!(".{extension}"))); + assert!(error.to_string().contains("raw image")); + } + } + } + + for (image, scratch) in [ + ("distro.iso", "scratch.ext4"), + ("distro.VHD", "scratch.ISO"), + ] { + let config = cold(layered(LayeredRoot { + image: missing(image), + upper: UpperConfig::Scratch { + path: missing(scratch), + journal: Journal::Forbidden, + }, + ..erofs() + })); + config.check_command_line().unwrap(); + let arguments = config + .openvmm_arguments( + &runtime, + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .unwrap(); + let image_block = format!("distro:file:{},ro", missing(image).display()); + let scratch_block = format!("scratch:file:{}", missing(scratch).display()); + assert_eq!( + values(&arguments, "--microvm-sandbox-block"), + [OsStr::new(&image_block), OsStr::new(&scratch_block),] + ); + } +} + #[test] fn an_explicit_deny_all_network_keeps_its_nic() { let mut config = cold(blockless()); diff --git a/nvx-rs/src/host/sandbox/mod.rs b/nvx-rs/src/host/sandbox/mod.rs index cfa0f54e9..95c31f2a3 100644 --- a/nvx-rs/src/host/sandbox/mod.rs +++ b/nvx-rs/src/host/sandbox/mod.rs @@ -75,7 +75,7 @@ pub use self::plan::{ pub use self::prepared::PreparedLaunch; use crate::error::{Error, ErrorCode, Result}; use crate::host::artifacts::require_existing; -pub(crate) use crate::host::config::reject_disk_path; +pub(crate) use crate::host::config::{reject_container_path, reject_disk_path}; use crate::host::filesystem::ExportPlan; use crate::net::is_valid_hostname; use crate::protocol::cmdline; @@ -163,30 +163,6 @@ pub fn require_file(path: &Path, description: &str) -> Result<()> { require_existing(path, description, ErrorCode::Unavailable) } -/// The extensions of the files that OpenVMM does not attach as the raw image of a sandbox block -/// device: it opens a `.vhd`, `.vhdx`, or `.vmgs` file in its container format, refuses to write -/// an `.iso` file, and refuses all four for the block devices of a snapshot. -const CONTAINER_EXTENSIONS: [&str; 4] = ["vhd", "vhdx", "vmgs", "iso"]; - -/// Refuses the image at `path`, which the message shows as `shown`, if OpenVMM would not attach it -/// as a raw image: if the name of `path` has an extension of [`CONTAINER_EXTENSIONS`]. The -/// extension is the text after the name's last dot, unless that dot starts the name, and compares -/// case-sensitively, as OpenVMM reads and compares it. -pub(crate) fn reject_container_path(path: &Path, shown: &str) -> Result<()> { - let name = path.file_name().map(|name| name.to_string_lossy()); - let extension = match name.as_deref().unwrap_or_default().rsplit_once('.') { - Some((stem, extension)) if !stem.is_empty() => extension, - _ => "", - }; - if CONTAINER_EXTENSIONS.contains(&extension) { - return Err(refused(format!( - "disk paths ending in .{extension} are unsupported, since OpenVMM would not attach \ - them as raw images: {shown}" - ))); - } - Ok(()) -} - /// The logical block size that OpenVMM presents for a sandbox block device, in bytes, of which the /// device's length is a nonzero multiple. const LOGICAL_BLOCK_SIZE: u32 = 512; From 3a25b6bc1c8f8fa8ed9e13d8b9a85125916e10dc Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 17:51:24 -0800 Subject: [PATCH 07/10] host: preserve legacy VMDK sandbox paths Keep VMDK rejection on the new cold-boot configuration renderer, where OpenVMM selects its VMDK backend, without broadening the established sandbox, template, and clone snapshot policy. Add a regression for the legacy wrapper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0325d032-38a9-4c7f-9899-687a4ec93a51 --- nvx-rs/src/host/config/mod.rs | 22 +++++++++++++++------- nvx-rs/src/host/config/tests.rs | 10 ++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/nvx-rs/src/host/config/mod.rs b/nvx-rs/src/host/config/mod.rs index 71e5f9903..d6f96ccf3 100644 --- a/nvx-rs/src/host/config/mod.rs +++ b/nvx-rs/src/host/config/mod.rs @@ -617,7 +617,7 @@ impl VmConfig { } let shown = path.display().to_string(); reject_disk_path(path, &shown)?; - reject_container_backend(path, &shown, writable)?; + reject_container_backend(path, &shown, writable, true)?; } let mut image = OsString::from("distro:file:"); image.push(&root.image); @@ -932,17 +932,25 @@ pub(crate) fn reject_disk_path(path: &Path, shown: &str) -> Result<()> { Ok(()) } -/// Extensions for which OpenVMM selects a container backend instead of a raw file. -const CONTAINER_EXTENSIONS: [&str; 4] = ["vhd", "vhdx", "vmgs", "vmdk"]; +/// Extensions for which every affected OpenVMM flow selects a container backend. +const CONTAINER_EXTENSIONS: [&str; 3] = ["vhd", "vhdx", "vmgs"]; /// Refuses a block path when OpenVMM would not attach it as the requested raw image. -fn reject_container_backend(path: &Path, shown: &str, writable: bool) -> Result<()> { +fn reject_container_backend( + path: &Path, + shown: &str, + writable: bool, + reject_vmdk: bool, +) -> Result<()> { let name = path.file_name().map(|name| name.to_string_lossy()); let extension = match name.as_deref().unwrap_or_default().rsplit_once('.') { Some((stem, extension)) if !stem.is_empty() => extension, _ => "", }; - if CONTAINER_EXTENSIONS.contains(&extension) || (writable && extension == "iso") { + if CONTAINER_EXTENSIONS.contains(&extension) + || (reject_vmdk && extension == "vmdk") + || (writable && extension == "iso") + { return Err(Error::unsupported(format!( "disk paths ending in .{extension} are unsupported, since OpenVMM would not attach \ them as raw images: {shown}" @@ -951,9 +959,9 @@ fn reject_container_backend(path: &Path, shown: &str, writable: bool) -> Result< Ok(()) } -/// Refuses a legacy sandbox path that OpenVMM would not attach as a raw block image. +/// Preserves the legacy sandbox refusal set used by snapshot and clone flows. pub(crate) fn reject_container_path(path: &Path, shown: &str) -> Result<()> { - reject_container_backend(path, shown, true) + reject_container_backend(path, shown, true, false) } /// Common cold-boot inputs, shared with the legacy VmmLaunch without narrowing its public types. diff --git a/nvx-rs/src/host/config/tests.rs b/nvx-rs/src/host/config/tests.rs index ab4515ffa..a9d124608 100644 --- a/nvx-rs/src/host/config/tests.rs +++ b/nvx-rs/src/host/config/tests.rs @@ -1593,6 +1593,16 @@ fn cold_boot_arguments_refuse_non_raw_block_extensions() { } } +#[test] +fn legacy_container_path_preserves_its_extension_policy() { + for extension in ["vhd", "vhdx", "vmgs", "iso"] { + let name = format!("disk.{extension}"); + let error = super::reject_container_path(Path::new(&name), &name).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + } + super::reject_container_path(Path::new("disk.vmdk"), "disk.vmdk").unwrap(); +} + #[test] fn an_explicit_deny_all_network_keeps_its_nic() { let mut config = cold(blockless()); From f31a71eb0acddf3e4f85203b64089d9956aace86 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sun, 8 Nov 2026 07:52:43 -0800 Subject: [PATCH 08/10] host: adapt cold-boot fixtures to capture metadata Reuse the inherited runtime fixture instead of defining it twice, and explicitly leave capture disabled in the native control-console refusal cases. Keep the existing paths, launch options and assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0325d032-38a9-4c7f-9899-687a4ec93a51 --- nvx-rs/src/host/config/tests.rs | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/nvx-rs/src/host/config/tests.rs b/nvx-rs/src/host/config/tests.rs index a9d124608..7b118da64 100644 --- a/nvx-rs/src/host/config/tests.rs +++ b/nvx-rs/src/host/config/tests.rs @@ -1101,20 +1101,6 @@ fn public_maps_codec_keeps_the_protocol_root_reexports_usable() { assert_eq!(decoded.maps, table); } -fn runtime() -> Runtime { - let artifact = ArtifactIdentity::new("fixture").unwrap(); - Runtime { - openvmm: missing("openvmm"), - kernel: missing("vmlinux"), - initramfs: missing("initramfs.cpio.gz"), - identity: RuntimeIdentity::new(artifact.clone(), artifact.clone(), artifact), - hypervisor: Hypervisor::Kvm, - cpu_profile: None, - windows_breakaway: false, - options: LaunchOptions::default(), - } -} - fn cold(config: VmConfig) -> VmConfig { VmConfig { boot: Some(Boot { @@ -1152,6 +1138,7 @@ fn cold_boot_command_lines_refuse_native_control_console_syntax() { let config = VmConfig { boot: Some(Boot { command_line: caller.to_owned(), + capture: None, }), console, ..blockless() From ac93c908665c2491e1b600a58fd8b59166b8abe2 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sun, 8 Nov 2026 08:31:03 -0800 Subject: [PATCH 09/10] host: refuse captures in cold-boot argument rendering Refuse capture-bearing configurations in the shared pure path used by both temporary argument entry points rather than silently omit their devices and resource capacities. Preserve configuration validation and kernel projection, and cover both refusals with a regression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0325d032-38a9-4c7f-9899-687a4ec93a51 --- nvx-rs/README.md | 5 +++++ nvx-rs/src/host/config/mod.rs | 28 ++++++++++++++++++++-------- nvx-rs/src/host/config/tests.rs | 22 ++++++++++++++++++++++ 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/nvx-rs/README.md b/nvx-rs/README.md index a9bd76452..61713dfb0 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -573,6 +573,11 @@ discards its output. The renderer emits neither `--microvm-lifecycle` nor `--microvm-workload-identity`. The whole-config entry point is temporarily public: SL2.8 makes it internal when `host::vm::create` calls it. +Both argument entry points refuse a `Boot` with a capture as `Unsupported` +rather than silently omit its devices and resource capacities. +`VmConfig::check` and `kernel_command_line` retain their existing behavior for +valid capture configurations. + `VmConfig::check_command_line()` checks the configuration's known arguments on every host, before any host change, with 2048 UTF-16 code units reserved for the runtime's files, launch options, and VM output names. The reserve foresees file diff --git a/nvx-rs/src/host/config/mod.rs b/nvx-rs/src/host/config/mod.rs index d6f96ccf3..1d7d18f63 100644 --- a/nvx-rs/src/host/config/mod.rs +++ b/nvx-rs/src/host/config/mod.rs @@ -435,7 +435,8 @@ impl VmConfig { Ok(tokens.join(" ")) } - /// Renders a cold boot's complete OpenVMM arguments from this configuration and `runtime`. + /// Renders a non-capture cold boot's complete OpenVMM arguments from this configuration and + /// `runtime`. /// /// `endpoint`, `report` and `console_endpoint` are already chosen output names, not launch /// settings: this method creates no directory, file, listener, or process, and uses paths as @@ -451,8 +452,9 @@ impl VmConfig { /// `runtime.windows_breakaway` is process-launch behavior, not an OpenVMM argument. /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused - /// configuration, no [`Boot`], empty or NUL-containing required names or runtime paths, - /// duplicate listener names, block paths that are not Unicode, hold disk-option delimiters or + /// configuration, no [`Boot`], a capture-bearing boot, empty or NUL-containing required names + /// or runtime paths, duplicate listener names, block paths that are not Unicode, hold + /// disk-option delimiters or /// unbalanced brackets, or select a non-raw OpenVMM backend, an invalid authentication timeout, /// kernel text with double quotes or the standalone `--` delimiter, or a complete command line /// beyond the Windows limit. @@ -543,8 +545,8 @@ impl VmConfig { Ok(arguments) } - /// Checks the configuration's part of a cold boot's OpenVMM command line on every host, - /// without reading or changing the host, with 2048 UTF-16 code units reserved for the + /// Checks the configuration's part of a non-capture cold boot's OpenVMM command line on every + /// host, without reading or changing the host, with 2048 UTF-16 code units reserved for the /// runtime's files, launch options, and the VM's output paths. /// /// Like legacy adapter planning, the reserve foresees runtime-file and output names of up to @@ -554,8 +556,9 @@ impl VmConfig { /// A listener's backend prefix is counted here; its endpoint name stays in the reserve. /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused - /// configuration or no [`Boot`], an unlowerable network policy, a NUL-containing argument, - /// a non-Unicode block path, one that OpenVMM's disk syntax cannot hold literally, or one whose + /// configuration or no [`Boot`], a capture-bearing boot, an unlowerable network policy, a + /// NUL-containing argument, a non-Unicode block path, one that OpenVMM's disk syntax cannot + /// hold literally, or one whose /// extension selects a non-raw backend, kernel text with double quotes or the standalone `--` /// delimiter, or insufficient room for the reserve. pub fn check_command_line(&self) -> Result<()> { @@ -577,9 +580,18 @@ impl VmConfig { fit_arguments(&arguments, OPENVMM_COMMAND_LINE_RESERVE) } - /// Renders kernel text without the double quotes or delimiter OpenVMM's control console refuses. + /// Renders a non-capture boot's kernel text that OpenVMM's control console accepts. fn control_kernel_command_line(&self) -> Result { let command_line = self.kernel_command_line()?; + if self + .boot + .as_ref() + .is_some_and(|boot| boot.capture.is_some()) + { + return Err(Error::unsupported( + "cold-boot argument rendering does not support capture configurations", + )); + } if command_line.contains('"') || command_line .split_ascii_whitespace() diff --git a/nvx-rs/src/host/config/tests.rs b/nvx-rs/src/host/config/tests.rs index 7b118da64..2657077f7 100644 --- a/nvx-rs/src/host/config/tests.rs +++ b/nvx-rs/src/host/config/tests.rs @@ -1119,6 +1119,28 @@ fn values<'a>(arguments: &'a [OsString], option: &str) -> Vec<&'a OsStr> { .collect() } +#[test] +fn cold_boot_arguments_refuse_capture_configurations() { + let config = captured(); + config.check().unwrap(); + config.kernel_command_line().unwrap(); + for result in [ + config.check_command_line(), + config + .openvmm_arguments( + &runtime(), + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .map(|_| ()), + ] { + let error = result.unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + assert!(error.to_string().contains("capture")); + } +} + #[test] fn cold_boot_command_lines_refuse_native_control_console_syntax() { let runtime = runtime(); From d86792ecfdea34dbc9ade4f8dd79d54759055dab Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sun, 8 Nov 2026 11:32:39 -0800 Subject: [PATCH 10/10] host: refuse unrepresentable cold-boot export and listener names Refuse non-Unicode export child roots before string lowering and serial option delimiters in caller-selected control and boot-console names. Keep parent configuration checks and legacy launch behavior unchanged. Add focused regressions for both argument entry points and listener roles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0325d032-38a9-4c7f-9899-687a4ec93a51 --- nvx-rs/README.md | 6 +- nvx-rs/src/host/config/mod.rs | 25 +++++-- nvx-rs/src/host/config/tests.rs | 113 ++++++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 8 deletions(-) diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 61713dfb0..a7a093985 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -569,12 +569,16 @@ the kernel command line, layered-root block devices by path, network and export arguments, console attachment, the CPU profile, and every `LaunchOptions` value. A configured NIC remains attached even under deny-all policy. A listener needs its supplied console name; a disconnected attachment retains the device and -discards its output. The renderer emits neither `--microvm-lifecycle` nor +discards its output. Listener names are nonempty and contain no NUL, comma, or +equals sign, which OpenVMM's serial options cannot hold literally. +The renderer emits neither `--microvm-lifecycle` nor `--microvm-workload-identity`. The whole-config entry point is temporarily public: SL2.8 makes it internal when `host::vm::create` calls it. Both argument entry points refuse a `Boot` with a capture as `Unsupported` rather than silently omit its devices and resource capacities. +They also refuse non-Unicode export child roots instead of lossily rendering +a different host path. `VmConfig::check` and `kernel_command_line` retain their existing behavior for valid capture configurations. diff --git a/nvx-rs/src/host/config/mod.rs b/nvx-rs/src/host/config/mod.rs index 1d7d18f63..fc5fc6bda 100644 --- a/nvx-rs/src/host/config/mod.rs +++ b/nvx-rs/src/host/config/mod.rs @@ -443,7 +443,8 @@ impl VmConfig { /// given. A [`ConsoleAttachment::Listener`] needs a nonempty `console_endpoint`; a /// [`ConsoleAttachment::Disconnected`] needs none and retains the console device with a /// discarded attachment. The control endpoint always listens and authenticates from stdin. - /// Listener names must be distinct, with ASCII-case-insensitive comparison for WHP. + /// Listener names must be distinct, with ASCII-case-insensitive comparison for WHP, and + /// contain neither commas nor equals signs, which delimit OpenVMM's serial options. /// The caller runs `runtime.openvmm` with these arguments. /// /// This whole-config entry point is temporarily public. SL2.8 makes it internal when @@ -453,7 +454,8 @@ impl VmConfig { /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused /// configuration, no [`Boot`], a capture-bearing boot, empty or NUL-containing required names - /// or runtime paths, duplicate listener names, block paths that are not Unicode, hold + /// or runtime paths, serial-option delimiters in listener names, duplicate listener names, + /// non-Unicode export child roots, block paths that are not Unicode, hold /// disk-option delimiters or /// unbalanced brackets, or select a non-raw OpenVMM backend, an invalid authentication timeout, /// kernel text with double quotes or the standalone `--` delimiter, or a complete command line @@ -557,8 +559,8 @@ impl VmConfig { /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for a refused /// configuration or no [`Boot`], a capture-bearing boot, an unlowerable network policy, a - /// NUL-containing argument, a non-Unicode block path, one that OpenVMM's disk syntax cannot - /// hold literally, or one whose + /// NUL-containing argument, a non-Unicode export child root or block path, one that OpenVMM's + /// disk syntax cannot hold literally, or one whose /// extension selects a non-raw backend, kernel text with double quotes or the standalone `--` /// delimiter, or insufficient room for the reserve. pub fn check_command_line(&self) -> Result<()> { @@ -654,6 +656,15 @@ impl VmConfig { arguments.extend(nic.into_iter().map(OsString::from)); } if let Some(export) = &self.filesystem { + if export + .children + .iter() + .any(|child| child.root.to_str().is_none()) + { + return Err(Error::unsupported( + "filesystem export child roots must be Unicode for OpenVMM's string arguments", + )); + } arguments.extend(export.openvmm_arguments().into_iter().map(OsString::from)); } Ok(arguments) @@ -1018,11 +1029,11 @@ pub(crate) fn console_arguments(console: &str, endpoint: &str, report: &Path) -> .into() } -/// Refuses empty or NUL-containing caller-selected endpoint names. +/// Refuses caller-selected endpoint names that OpenVMM's serial options cannot hold literally. fn check_endpoint(endpoint: &str, name: &str) -> Result<()> { - if endpoint.is_empty() || endpoint.contains('\0') { + if endpoint.is_empty() || endpoint.contains(['\0', ',', '=']) { return Err(Error::unsupported(format!( - "{name} must name a nonempty endpoint without a NUL", + "{name} must name a nonempty endpoint without a NUL, comma, or equals sign", ))); } Ok(()) diff --git a/nvx-rs/src/host/config/tests.rs b/nvx-rs/src/host/config/tests.rs index 2657077f7..ac8f140c8 100644 --- a/nvx-rs/src/host/config/tests.rs +++ b/nvx-rs/src/host/config/tests.rs @@ -1236,6 +1236,60 @@ fn native_argument_listener_names_are_distinct() { } } +#[test] +fn native_argument_listener_names_refuse_serial_delimiters() { + let config = cold(blockless()); + let runtime = runtime(); + for delimiter in [',', '='] { + let invalid = format!("/missing/nvx{delimiter}listener"); + for control in [false, true] { + let endpoint = if control { + invalid.as_str() + } else { + "/missing/control" + }; + let console = if control { + "/missing/console" + } else { + invalid.as_str() + }; + let error = config + .openvmm_arguments( + &runtime, + endpoint, + Path::new("/missing/report"), + Some(console), + ) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + assert!(error.to_string().contains(if control { + "endpoint" + } else { + "console_endpoint" + })); + } + } + + let endpoint = "/missing/control-\u{e9} with spaces"; + let console = "/missing/console-\u{f1} with spaces"; + let arguments = config + .openvmm_arguments( + &runtime, + endpoint, + Path::new("/missing/report"), + Some(console), + ) + .unwrap(); + assert_eq!( + values(&arguments, "--microvm-control-console"), + [OsStr::new(&format!("listen={endpoint}"))] + ); + assert_eq!( + values(&arguments, "--virtio-console"), + [OsStr::new(&format!("listen={console}"))] + ); +} + #[cfg(any(unix, windows))] #[test] fn native_argument_paths_require_unicode() { @@ -1307,6 +1361,65 @@ fn native_argument_paths_require_unicode() { ); } +#[cfg(any(unix, windows))] +#[test] +fn native_argument_exports_require_unicode() { + #[cfg(unix)] + let invalid = { + use std::os::unix::ffi::OsStringExt; + OsString::from_vec(vec![b'e', 0xff]) + }; + #[cfg(windows)] + let invalid = { + use std::os::windows::ffi::OsStringExt; + OsString::from_wide(&[101, 0xd800]) + }; + let mut plan = export("/work"); + plan.children[0].root = missing("exports").join(invalid); + let config = cold(VmConfig { + filesystem: Some(plan), + ..blockless() + }); + config.check().unwrap(); + config.kernel_command_line().unwrap(); + for result in [ + config.check_command_line(), + config + .openvmm_arguments( + &runtime(), + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .map(|_| ()), + ] { + let error = result.unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + assert!(error.to_string().contains("Unicode")); + } + + let root = missing("export-\u{e9} with spaces"); + let mut plan = export("/work"); + plan.children[0].root = root.clone(); + let config = cold(VmConfig { + filesystem: Some(plan), + ..blockless() + }); + config.check_command_line().unwrap(); + let arguments = config + .openvmm_arguments( + &runtime(), + "/missing/control", + Path::new("/missing/report"), + Some("/missing/console"), + ) + .unwrap(); + assert_eq!( + values(&arguments, "--mount-child"), + [OsStr::new(&format!("0,{},ro", root.display()))] + ); +} + #[test] fn cold_boot_arguments_select_the_configured_guest_without_managed_tokens() { for hypervisor in [Hypervisor::Kvm, Hypervisor::Mshv, Hypervisor::Whp] {