From 9f3fb167f0c73e73a2df3fc5ab1a38eae1b7d935 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Sat, 10 Oct 2026 00:38:41 -0300 Subject: [PATCH 1/4] openvmm: pin aggregate virtio-fs shares on one microVM slot 7bf0ee28b (fix/aggregate-microvm-mounts-281 in nanvix/openvmm) stacks on e5bf79776, the IPv6 egress of nanvix/openvmm#126, which dev already pins and nanvix/openvmm main has merged. It gives a microVM a single virtio-fs slot, fs:microvm0, and retires the second one, fs:microvm1 with tag microvm1 at 0xd0008000 and IRQ 13. --mount may appear once. --mount-aggregate GUEST_TARGET with a repeatable --mount-child NAME,HOST_PATH[,ro|rw] instead shares several host directories through that slot, as named children of a synthetic read-only root. Each child has its own mode and its own --mount-deny, --mount-allow, and --mount-write paths, which are absolute and belong to the child that contains them. A rename between children fails with EXDEV, and so does a hard link, unless its destination is read-only, which fails with EROFS first, as on Linux. An aggregate share is snapshot-capable, a restore requires the same children, names, and order, and a snapshot that recorded a second slot no longer restores. --mount-deny may also name the root of a share or child when --mount-allow names paths inside it, which leaves that root traverse-only, and policy path names may contain inner spaces. run and sandbox still pass a second share as a repeated --mount; the following commits move them and aci_edge_sandboxes to the aggregate form. Part of #281. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- openvmm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openvmm b/openvmm index e5bf7977..7bf0ee28 160000 --- a/openvmm +++ b/openvmm @@ -1 +1 @@ -Subproject commit e5bf79776bb32f8fa29a69e7b3c9bf668477a13a +Subproject commit 7bf0ee28b826374a2d386067df0456b81bcae818 From 55c31c4a59a60c827b8692fa9935386059f62104 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Fri, 9 Oct 2026 18:45:30 -0300 Subject: [PATCH 2/4] aci_edge_sandboxes: map host paths from any directory, each with its own access The OpenVMM backend exported one virtio-fs share, the deepest directory that contained every mapped path, and its guest agent bind-mounted each mapped path from nvx_map= kernel command-line tokens. Paths on different volumes, or under nothing narrower than a volume's root, were refused, as is the layout of #281: C:\projects\app and C:\build-output read-write beside C:\tools read-only. The export was writable whenever any mapping was, so read-only mappings and unmapped siblings were protected only by the guest kernel, and the 1024-byte kernel command line held about a dozen mappings. The pinned OpenVMM shares several host directories through its one virtio-fs device as the children of an aggregate. The backend now always exports one: its children are the outermost mapped directories and the parents of the outermost mapped files, named by their index. A child is read-write only if it holds a read-write mapping, and --mount-write then limits writes to its read-write mappings, so the host enforces each mapping's access, except that of a read-only path inside a read-write mapping. A child that is a file's parent hides its root behind its mapped paths, so unmapped siblings are not exported. Denied paths are attributed to the child that contains them, and those already hidden are dropped. Mapping a whole volume, or a file directly in a volume's root, whose parent would export that root, is refused. Provision checks OpenVMM's limits: 256 children; 128 hidden, exposed, and writable paths in each, so more than 128 files of one directory must be mapped as that directory; their path syntax, which now admits spaces inside names; and a command line that fits a Windows process with 1,024 characters to spare. The mapping table no longer travels on the kernel command line, which carries only nvx_maps=COUNT. Start sends it in APP_MAPS requests that each fit one 64 KiB control record, after FEATURES and before the sandbox counts as started, and aborts the start if the guest cannot mount an entry. The guest agent accepts the entries in order and only up to the announced count, checks a whole request before it mounts anything, and refuses EXEC as mappings-incomplete until every entry is mounted, so a workload never runs without its host paths. It ignores nvx_map= tokens, stops advertising HOST_MAPPINGS (bit 1), and advertises HOST_MAPPING_TABLE (bit 6), which the backend now requires instead, so images and crates of either generation refuse each other. Sandbox records move to STATE_FORMAT 3, and records of format 2 fail with the unsupported-format error. Unit tests cover the planner, the batching of APP_MAPS, and the size check; the fake OpenVMM applies the table and enforces the new contract; the agent's C tests decode tables, refuse workloads before a table is complete, refuse extra and malformed entries, and ignore stale tokens; and new end-to-end tests map the layout of #281, files beside hidden siblings, names with spaces, 50 mappings, and a second volume where one exists. Part of #281. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- aci_edge_sandboxes/README.md | 90 +- .../bin/aci-edge-sandboxes-fake-openvmm.rs | 275 +++-- aci_edge_sandboxes/src/openvmm/filesystem.rs | 966 ++++++++++++++---- aci_edge_sandboxes/src/openvmm/launch.rs | 146 ++- aci_edge_sandboxes/src/openvmm/mod.rs | 47 +- aci_edge_sandboxes/src/openvmm/protocol.rs | 264 ++++- aci_edge_sandboxes/src/openvmm/session.rs | 133 ++- aci_edge_sandboxes/src/openvmm/state.rs | 46 +- aci_edge_sandboxes/tests/openvmm_e2e.rs | 219 ++++ aci_edge_sandboxes/tests/openvmm_fake.rs | 250 ++++- ...ndbox-filesystem-and-agent-architecture.md | 50 +- guest/common/nvx-managed-agent.c | 463 +++++---- scripts/test_managed_agent.py | 324 ++++++ scripts/test_nvx_tools.py | 25 +- 14 files changed, 2710 insertions(+), 588 deletions(-) diff --git a/aci_edge_sandboxes/README.md b/aci_edge_sandboxes/README.md index 167ed600..ea57ef3a 100644 --- a/aci_edge_sandboxes/README.md +++ b/aci_edge_sandboxes/README.md @@ -300,8 +300,9 @@ hypervisor at every cold boot and does not pin. On Linux, supply a matching ### Native host paths and network The native backend accepts the same `filesystem` and `network` policies as the direct -backend: it maps host paths to the same guest paths under the same rules and expands network -rules the same way; see [Host paths](#host-paths) and [Network rules](#network-rules). It also +backend: it maps host paths to the same guest paths and expands network rules the same way; +see [Host paths](#host-paths) and [Network rules](#network-rules). Unlike the direct backend, +it exports one directory for all mapped paths, under the rules below. It also refuses mappings that would show workloads its state root, described below. Its workloads run as the guest's root. The library plans the policies at provision: it resolves the mapped and denied paths, chooses the export, pins the objects inside read-write mappings, and expands the @@ -498,9 +499,10 @@ Windows and `$XDG_STATE_HOME/nvx/sandboxes` (default caller except its standard input and log, so a caller whose own output is a pipe, such as an MXC phase process, still reaches end-of-file when it exits. - - Start returns once the guest agent answers on the control console and - advertises the control features this backend needs. A guest that lacks any - of them is terminated, and start fails with `backend_unavailable`. + - Start returns once the guest agent answers on the control console, + advertises the control features this backend needs, and has mounted every + mapped host path. A guest that lacks any of the features is terminated, + and start fails with `backend_unavailable`. - **Exec** connects to the control console, authenticates, and streams the workload's output live. The agent runs the workload through `setpriv` as the workload identity, with no capabilities and `no_new_privs`, in a cgroup of @@ -618,34 +620,60 @@ paths for one object. `openvmm::resolve_guest_path` performs the same translation, for example to turn a host working directory into a `process.cwd`; `openvmm::guest_path` translates a path as written. -- OpenVMM offers one virtio-fs export. The backend exports the deepest - directory that contains every mapped path to `/run/nvx/hostfs/root`, a guest - directory that only the guest's root can enter. The guest agent then +- OpenVMM offers one virtio-fs device, which the backend attaches as an + aggregate: a synthetic, read-only root that lists one exported host + directory per child, mounted at `/run/nvx/hostfs/root`, a guest directory + that only the guest's root can enter. The exported directories are the + outermost mapped directories and the parents of the outermost mapped files, + so mapped paths may lie anywhere, on any volume. The guest agent then bind-mounts each mapped path at its guest path, read-only or read-write, so - workloads see only the mapped paths. The export is writable when any path is - read-write, and read-only mounts are then enforced by the guest kernel. - Exporting a whole volume (`/` or `C:\`) is refused, so mapped paths must - share a directory below it. -- Denied paths inside the export are hidden by OpenVMM itself: they are absent - from listings and inaccessible through any name, except a host hard link to a - file inside a denied directory, which OpenVMM cannot tell from any other file. - OpenVMM requires hidden paths below the export to contain no whitespace, - colons, or backslashes and no links, and accepts at most 128 of them. -- Mapped paths must exist, be directories or regular files, and share one - volume. A path listed both read-only and read-write is mapped read-only. A - denied path must not contain a mapped path, and a denied path that does not - exist yet must not lie inside a mapped path, because nothing could hide it - once a workload creates it. On Windows hosts, which open files - case-insensitively, a read-only file inside a read-write directory is - refused. Mapping over the guest's own system directories (`/usr`, `/etc`, - and so on) is refused. Violations are reported as `policy_validation`. + workloads see only the mapped paths. +- The host enforces each mapping's access. An exported directory is + read-write only if it holds a read-write mapping, and OpenVMM then limits + writes to its read-write mappings, so only a read-only path inside a + read-write mapping is read-only through the guest's bind mount alone. The + parent of mapped files exposes only its mapped paths: OpenVMM hides + everything else in it, so unmapped siblings are not exported at all. + Mapping a whole volume (`/` or `C:\`) is refused; map directories inside it. + So is mapping a file directly in a volume's root, such as `C:\notes.txt` or + `/notes.txt`, because its parent would be the volume's root; move the file + into a directory, and map the file or the directory. +- Denied paths inside an exported directory are hidden by OpenVMM itself: they + are absent from listings and inaccessible through any name, except a host + hard link to a file inside a denied directory, which OpenVMM cannot tell + from any other file. Below its exported directory, a path that OpenVMM hides + or singles out, such as a mapped file or a read-write path inside a + read-only one, must not contain colons, backslashes, links, or whitespace + other than spaces, and no name in it may begin or end with a space. +- Mapped paths must exist and be directories or regular files. A path listed + both read-only and read-write is mapped read-only. A denied path must not + contain a mapped path, and a denied path that does not exist yet must not + lie inside a mapped path, because nothing could hide it once a workload + creates it. On Windows hosts, which open files case-insensitively, a + read-only file inside a read-write directory is refused. Mapping over the + guest's own system directories (`/usr`, `/etc`, and so on) is refused. + Violations are reported as `policy_validation`. +- OpenVMM exposes at most 128 paths in each exported directory, so mapping + more than 128 individual files from one directory fails at provision with + `policy_validation`; map the directory instead. Likewise, OpenVMM hides at + most 128 denied paths, and singles out at most 128 read-write paths inside + read-only ones, in each exported directory. +- A sandbox maps at most 4096 paths from at most 256 exported directories. + OpenVMM's command line must fit the 32,767 characters of a Windows process + command line, with 1,024 held in reserve, on every host, which typically + leaves room for several hundred paths. Provision rejects a policy beyond + these limits with `policy_validation`. - Provision records the identity of every mapped and denied object inside a read-write mapping, and start refuses to run if one changed, so a workload cannot rename a denied or read-only object and leave a decoy at its path for the next start. Objects outside read-write mappings are not pinned, so host edits that replace them do not block a restart. -- The bind mounts travel on the kernel command line, which leaves room for - roughly a dozen typical paths. +- Start hands the guest agent the mapping table over the control channel, in + requests that each fit one 64 KiB control record; the kernel command line + carries only the number of entries. The agent refuses workloads until it has + mounted every entry, and start fails if it cannot mount one. Guest images + that predate the mapping table do not advertise its control feature, so + start refuses them. - On Windows hosts, mapped files appear owned by root with mode `0777`, so the workload identity can read and, for read-write paths, write them. On Linux hosts virtio-fs shows host owners and modes unchanged; with @@ -653,8 +681,8 @@ translation, for example to turn a host working directory into a workloads under the calling user's IDs, and the guest creates an account for them. -The guest kernel, not only the workload, separates the mapped paths from the -rest of the export, so place mapped paths under one directory when possible. +Only the guest kernel keeps a read-only path inside a read-write mapping +read-only, so prefer read-only paths that no read-write mapping contains. ### Network rules @@ -754,8 +782,8 @@ python3 scripts/nvx.py test-aci-edge-sandboxes --backend kvm # real VM (reposi control console. It lets the real OpenVMM backend run end to end on hosts without a hypervisor: detached launch, reconnection from a new process, crash recovery, interrupted-start recovery, forced stop, cancellation, - working directories, refusal of guest images that lack required features, - and failure injection. + working directories, host path mapping tables, refusal of guest images that + lack required features, and failure injection. - `cargo run --example lifecycle -- ` runs one command with discovered artifacts. - `nvx.py test-aci-edge-sandboxes` runs the ignored `openvmm_e2e` tests against a real diff --git a/aci_edge_sandboxes/src/bin/aci-edge-sandboxes-fake-openvmm.rs b/aci_edge_sandboxes/src/bin/aci-edge-sandboxes-fake-openvmm.rs index e2e0ba28..8448409e 100644 --- a/aci_edge_sandboxes/src/bin/aci-edge-sandboxes-fake-openvmm.rs +++ b/aci_edge_sandboxes/src/bin/aci-edge-sandboxes-fake-openvmm.rs @@ -19,6 +19,13 @@ //! `/work`, a mapped directory, or a directory made with `mkdir`), or that is a mapped regular //! file, with a diagnostic and the `cwd-failed` category. //! +//! Host paths are mapped as OpenVMM and the guest agent map them: the command line exports +//! numbered `--mount-child` directories through `--mount-aggregate`, the kernel command line +//! announces the number of mappings with `nvx_maps=`, and `MAPS` requests deliver the mapping +//! table in order. The fake checks each entry against the exported directories, refuses +//! workloads until the table is complete, and records it in `fake-openvmm--maps.json` +//! next to the kernel. +//! //! Each workload gets the environment that its exec request selects, in the order in which the //! guest agent builds it. The default environment is the documented guest bootstrap environment: //! `PATH`, `TERM`, and the `HOME`, `USER`, and `LOGNAME` of the workload account. The guest's @@ -31,8 +38,10 @@ //! `fake_boot_delay_ms=MS` delays the control endpoint, `fake_crash_after_ms=MS` makes the VM //! die, `fake_ignore_stop=1` ignores stop requests, `fake_ignore_cancel=1` ignores cancellation, //! `fake_guest_features=MASK` advertises the decimal feature mask `MASK` instead of the current -//! guest's, and `fake_legacy_guest=1` refuses the features request like a guest agent that -//! predates it. The command line is recorded in `fake-openvmm-.json` next to the kernel. +//! guest's, `fake_legacy_guest=1` refuses the features request like a guest agent that +//! predates it, and `fake_refuse_maps=1` fails every `MAPS` request like a guest that cannot +//! mount a mapped path. The command line is recorded in `fake-openvmm-.json` next to the +//! kernel. use std::collections::{BTreeMap, BTreeSet}; use std::fs::{self, OpenOptions}; @@ -58,6 +67,7 @@ const APP_EXEC: u8 = 2; const APP_STOP: u8 = 3; const APP_CANCEL: u8 = 4; const APP_FEATURES: u8 = 5; +const APP_MAPS: u8 = 6; const APP_READY: u8 = 0x81; const APP_STDOUT: u8 = 0x82; const APP_STDERR: u8 = 0x83; @@ -65,9 +75,10 @@ const APP_EXIT: u8 = 0x84; const APP_STOPPED: u8 = 0x85; const APP_ERROR: u8 = 0xff; -/// Control features of the current guest: cancellation, host path mappings, workload accounts, -/// workload containment, per-execution environments, and working directories. -const GUEST_FEATURES: u32 = 0b11_1111; +/// Control features of the current guest: cancellation, workload accounts, workload +/// containment, per-execution environments, working directories, and host path mapping tables. +/// Bit 1 is retired. +const GUEST_FEATURES: u32 = 0b111_1101; const EXEC_EXTENDED: u16 = 1; const EXEC_CWD_PRESENT: u16 = 1 << 0; @@ -75,6 +86,10 @@ const EXEC_ENVIRONMENT_PRESENT: u16 = 1 << 1; const EXEC_INHERIT_DEFAULT_ENV: u16 = 1 << 2; const MAX_ARGUMENT_BYTES: usize = 4096; const MAX_ENVIRONMENT: usize = 256; +const MAX_HOST_MAPPINGS: usize = 4096; +const MAP_READ_ONLY: u16 = 1; +/// The guest directory at which the backend mounts the aggregate export. +const GUEST_EXPORT: &str = "/run/nvx/hostfs/root"; /// `NAME=VALUE` variables in the order in which a workload's `environ` lists them. type Environment = Vec<(String, String)>; @@ -136,22 +151,31 @@ struct Options { report: PathBuf, hypervisor: String, args_dump: PathBuf, + maps_dump: PathBuf, exit_on_start: Option, boot_delay: Duration, crash_after: Option, ignore_stop: bool, ignore_cancel: bool, legacy_guest: bool, + refuse_maps: bool, workload_uid: u32, guest_features: u32, - /// Host paths mapped into the guest. - mapped: Vec, + /// Host directories of the aggregate export, in child order. + children: Vec, + /// Number of host mappings that the kernel command line announces. + maps: usize, } /// A host path mapped into the guest. +#[derive(serde::Serialize)] struct Mapped { + /// Path relative to the guest's mount of the export. + source: String, /// Guest path at which the host path appears. target: String, + /// Whether the guest mounts it read-only. + read_only: bool, /// Whether the host path is a directory rather than a regular file. directory: bool, } @@ -197,9 +221,12 @@ fn main() -> ExitCode { } fn parse(arguments: &[String]) -> Result { - const VALUED: [&str; 20] = [ - "--mount", + const VALUED: [&str; 23] = [ + "--mount-aggregate", + "--mount-child", "--mount-deny", + "--mount-allow", + "--mount-write", "--network-egress-allow", "--network-egress-deny", "--machine", @@ -289,40 +316,64 @@ fn parse(arguments: &[String]) -> Result { let tokens: Vec<&str> = command_line.split_whitespace().collect(); if tokens.iter().any(|token| { token.starts_with("nvx_") - && !token.starts_with("nvx_map=") + && !token.starts_with("nvx_maps=") && *token != "nvx_workload_account=create" }) { return Err("the kernel command line must not select a sandbox".to_owned()); } - let maps = tokens + let maps = match tokens .iter() - .filter(|token| token.starts_with("nvx_map=")) - .count(); - let export = match values.get("--mount").map(Vec::as_slice) { - None if maps == 0 && !values.contains_key("--mount-deny") => None, - Some([mount]) if maps > 0 => { - let mut fields = mount.splitn(3, ','); - let (Some(target), Some(host), Some(mode)) = - (fields.next(), fields.next(), fields.next()) - else { - return Err(format!("invalid --mount {mount}")); - }; - if target != "/run/nvx/hostfs/root" - || !Path::new(host).is_dir() - || !["ro", "rw"].contains(&mode) + .filter_map(|token| token.strip_prefix("nvx_maps=")) + .collect::>() + .as_slice() + { + [] => 0, + [count] => count + .parse::() + .ok() + .filter(|count| (1..=MAX_HOST_MAPPINGS).contains(count)) + .ok_or_else(|| format!("invalid nvx_maps={count}"))?, + _ => return Err("nvx_maps= must be given at most once".to_owned()), + }; + let children = match values.get("--mount-aggregate").map(Vec::as_slice) { + None if maps == 0 => Vec::new(), + Some([GUEST_EXPORT]) if maps > 0 => { + let mut children = Vec::new(); + for (index, child) in values + .get("--mount-child") + .into_iter() + .flatten() + .enumerate() { - return Err(format!("invalid --mount {mount}")); - } - for denied in values.get("--mount-deny").into_iter().flatten() { - let denied = Path::new(denied); - if !denied.exists() || !denied.starts_with(host) { - return Err(format!("invalid --mount-deny {}", denied.display())); + let invalid = || format!("invalid --mount-child {child}"); + let (name, rest) = child.split_once(',').ok_or_else(invalid)?; + let (host, mode) = rest.rsplit_once(',').ok_or_else(invalid)?; + if name != index.to_string() + || !Path::new(host).is_dir() + || !["ro", "rw"].contains(&mode) + { + return Err(invalid()); } + children.push(PathBuf::from(host)); + } + if children.is_empty() { + return Err("--mount-aggregate requires --mount-child".to_owned()); } - Some(PathBuf::from(host)) + children } - _ => return Err("--mount, --mount-deny, and nvx_map tokens must agree".to_owned()), + _ => return Err("--mount-aggregate and nvx_maps= must agree".to_owned()), }; + if children.is_empty() && values.contains_key("--mount-child") { + return Err("--mount-child requires --mount-aggregate".to_owned()); + } + for option in ["--mount-deny", "--mount-allow", "--mount-write"] { + for path in values.get(option).into_iter().flatten() { + let path = Path::new(path); + if !path.exists() || !children.iter().any(|root| path.starts_with(root)) { + return Err(format!("invalid {option} {}", path.display())); + } + } + } if (values.contains_key("--network-egress-allow") || values.contains_key("--network-egress-deny")) && !values.contains_key("--network-egress") @@ -350,25 +401,10 @@ fn parse(arguments: &[String]) -> Result { .and_then(Path::file_name) .map(|name| name.to_string_lossy().into_owned()) .unwrap_or_default(); - // The guest bind-mounts each mapped host path, relative to the export, at its target, so a - // target is a directory exactly when its host path is one. - let mapped = tokens - .iter() - .filter_map(|token| token.strip_prefix("nvx_map=")) - .map(|fields| { - let mut parts = fields.split(',').map(percent_decode); - match (parts.next().flatten(), parts.next().flatten(), &export) { - (Some(source), Some(target), Some(export)) => Ok(Mapped { - directory: export.join(source).is_dir(), - target, - }), - _ => Err(format!("invalid nvx_map={fields}")), - } - }) - .collect::>()?; Ok(Options { endpoint, args_dump: kernel.with_file_name(format!("fake-openvmm-{token}.json")), + maps_dump: kernel.with_file_name(format!("fake-openvmm-{token}-maps.json")), report, hypervisor: hypervisor.to_owned(), exit_on_start: knob("fake_exit_on_start").and_then(|value| value.parse().ok()), @@ -377,31 +413,16 @@ fn parse(arguments: &[String]) -> Result { ignore_stop: knob("fake_ignore_stop") == Some("1"), ignore_cancel: knob("fake_ignore_cancel") == Some("1"), legacy_guest: knob("fake_legacy_guest") == Some("1"), + refuse_maps: knob("fake_refuse_maps") == Some("1"), workload_uid, guest_features: knob("fake_guest_features") .and_then(|value| value.parse().ok()) .unwrap_or(GUEST_FEATURES), - mapped, + children, + maps, }) } -/// Decodes a percent-encoded `nvx_map=` field. -fn percent_decode(field: &str) -> Option { - let mut bytes = Vec::with_capacity(field.len()); - let mut rest = field.as_bytes(); - while let [first, tail @ ..] = rest { - if *first == b'%' { - let digits = std::str::from_utf8(tail.get(..2)?).ok()?; - bytes.push(u8::from_str_radix(digits, 16).ok()?); - rest = &tail[2..]; - } else { - bytes.push(*first); - rest = tail; - } - } - String::from_utf8(bytes).ok() -} - fn read_capability() -> Result<[u8; CAPABILITY_LEN], String> { let mut stdin = io::stdin().lock(); let mut capability = [0u8; CAPABILITY_LEN]; @@ -427,6 +448,8 @@ struct Guest { directories: BTreeSet, /// Mapped regular files, which a workload cannot enter either. files: BTreeSet, + /// Entries of the host mapping table mounted so far. + mapped: Vec, } impl Guest { @@ -448,22 +471,107 @@ impl Guest { .into_iter() .map(|(name, value)| (name.to_owned(), value.to_owned())) .collect(); - let mut directories: BTreeSet = ["/", "/tmp", "/work"].map(str::to_owned).into(); - let mut files = BTreeSet::new(); - for mapped in &options.mapped { - let paths = if mapped.directory { - &mut directories - } else { - &mut files - }; - paths.insert(mapped.target.clone()); - } Self { values: BTreeMap::new(), default_environment, - directories, - files, + directories: ["/", "/tmp", "/work"].map(str::to_owned).into(), + files: BTreeSet::new(), + mapped: Vec::new(), + } + } + + /// Mounts the entries of a `MAPS` request, like the guest agent: in order, and only as many + /// as the kernel command line announced. Returns the error number and category of a refusal. + fn map(&mut self, options: &Options, payload: &[u8]) -> Result<(), (i32, &'static str)> { + const INVALID: (i32, &str) = (22, "invalid-request"); + let header = |offset: usize| { + payload + .get(offset..offset + 4) + .map(|bytes| u32_at(bytes, 0)) + }; + let (Some(first), Some(count)) = (header(0), header(4)) else { + return Err(INVALID); + }; + let (first, count) = (first as usize, count as usize); + if options.maps == 0 + || first != self.mapped.len() + || count == 0 + || count > options.maps - first + { + return Err(INVALID); + } + let field = |offset: usize| { + payload + .get(offset..offset + 2) + .map(|bytes| u16::from_le_bytes([bytes[0], bytes[1]])) + }; + let mut entries = Vec::with_capacity(count); + let mut offset = 8; + for _ in 0..count { + let (Some(flags), Some(source_len), Some(target_len)) = + (field(offset), field(offset + 2), field(offset + 4)) + else { + return Err(INVALID); + }; + offset += 6; + let text = |offset: usize, length: u16| { + payload + .get(offset..offset + usize::from(length)) + .and_then(|bytes| String::from_utf8(bytes.to_vec()).ok()) + }; + let (Some(source), Some(target)) = ( + text(offset, source_len), + text(offset + usize::from(source_len), target_len), + ) else { + return Err(INVALID); + }; + offset += usize::from(source_len) + usize::from(target_len); + if flags & !MAP_READ_ONLY != 0 || !target.starts_with('/') || target.contains('\0') { + return Err(INVALID); + } + entries.push((source, target, flags & MAP_READ_ONLY != 0)); } + if offset != payload.len() { + return Err(INVALID); + } + if options.refuse_maps { + return Err((13, "mapping-failed")); + } + for (source, target, read_only) in entries { + let (child, relative) = source.split_once('/').unwrap_or((&source, "")); + let Some(root) = child + .parse::() + .ok() + .and_then(|child| options.children.get(child)) + else { + return Err(INVALID); + }; + let host = if relative.is_empty() { + root.clone() + } else { + root.join(relative) + }; + let directory = if host.is_dir() { + self.directories.insert(target.clone()); + true + } else if host.is_file() { + self.files.insert(target.clone()); + false + } else { + return Err((2, "mapping-failed")); + }; + self.mapped.push(Mapped { + source, + target, + read_only, + directory, + }); + } + let _ = fs::write( + &options.maps_dump, + serde_json::to_vec(&self.mapped).unwrap_or_default(), + ); + Ok(()) } } @@ -654,11 +762,20 @@ impl Session<'_, S> { &options.guest_features.to_le_bytes(), )?; } + APP_EXEC if guest.mapped.len() < options.maps => { + self.send(APP_ERROR, request_id, 11, b"mappings-incomplete")?; + } APP_EXEC => { if !self.exec(request_id, &payload, guest)? { return Ok(Flow::Continue); } } + APP_MAPS => match guest.map(options, &payload) { + Ok(()) => self.send(APP_READY, request_id, 0, &[])?, + Err((status, category)) => { + self.send(APP_ERROR, request_id, status, category.as_bytes())?; + } + }, // The targeted workload already finished. APP_CANCEL => {} APP_STOP if options.ignore_stop => {} diff --git a/aci_edge_sandboxes/src/openvmm/filesystem.rs b/aci_edge_sandboxes/src/openvmm/filesystem.rs index 0e8f9359..3d24ddc6 100644 --- a/aci_edge_sandboxes/src/openvmm/filesystem.rs +++ b/aci_edge_sandboxes/src/openvmm/filesystem.rs @@ -1,9 +1,19 @@ -//! Host directories exposed to the guest through OpenVMM's single virtio-fs export. +//! Host paths exposed to the guest through OpenVMM's aggregate virtio-fs export. //! -//! OpenVMM offers one virtio-fs export and can hide existing paths inside it. To expose several -//! host paths, the backend exports their deepest common directory to a guest directory that only -//! the guest's root can enter, and the guest agent bind-mounts each mapped path at its target, -//! read-only or read-write. Denied paths inside the export are hidden by OpenVMM itself. +//! OpenVMM offers one virtio-fs device. The backend attaches it as an aggregate: a synthetic, +//! read-only root that lists one child per exported host directory, named by its index, which +//! the guest's init mounts at a directory that only the guest's root can enter. The guest agent +//! then bind-mounts each mapped path at its target, read-only or read-write, from the mapping +//! table that it receives over the control channel. +//! +//! The children are the outermost mapped directories and the parents of the outermost mapped +//! files, so mapped paths may lie anywhere, on any volume, except that neither a whole volume nor +//! a file directly in a volume's root, whose parent would export that root, can be mapped. A +//! child whose root is not mapped itself, the parent of mapped files, hides everything but its +//! mapped paths. A child is read-write only if it holds a read-write mapping, and OpenVMM then +//! limits writes to its read-write mappings, so the host enforces each mapping's access, except +//! that of a read-only path inside a read-write mapping, which only the guest's bind mount keeps +//! read-only. OpenVMM also hides the denied paths. //! //! Guest targets follow MXC's convention for Linux guests: a Windows path `C:\work\src` appears at //! `/mnt/c/work/src`, and a Linux path appears at the same path. @@ -14,6 +24,7 @@ use std::path::{Component, Path, PathBuf, Prefix}; use serde::{Deserialize, Serialize}; use super::platform; +use super::protocol::{HostMap, MAX_HOST_MAPPINGS}; use crate::error::{Error, Result}; use crate::model::FilesystemPolicy; @@ -21,9 +32,17 @@ use crate::model::FilesystemPolicy; /// its parent. pub(crate) const GUEST_EXPORT: &str = "/run/nvx/hostfs/root"; -/// Largest number of denied paths, and their largest combined length, that OpenVMM accepts. -const MAX_DENIED_PATHS: usize = 128; -const MAX_DENIED_BYTES: usize = 16 * 1024; +/// Largest number of host directories that OpenVMM exports through one aggregate. +const MAX_CHILDREN: usize = 256; +/// Largest number of hidden, allowed, or writable paths that OpenVMM accepts in one exported +/// directory, and their largest combined length. +const MAX_POLICY_PATHS: usize = 128; +const MAX_POLICY_BYTES: usize = 16 * 1024; +/// Largest length of one such path, and of all of them together. +const MAX_POLICY_PATH_BYTES: usize = 4096; +const MAX_AGGREGATE_POLICY_BYTES: usize = 128 * 1024; +/// Longest guest path, without the terminating NUL. +const MAX_GUEST_PATH: usize = 4095; /// Guest directories that a mapping must not cover, because the guest's own system lives there. const RESERVED_TARGETS: [&str; 13] = [ @@ -31,22 +50,36 @@ const RESERVED_TARGETS: [&str; 13] = [ "/var", "/init", ]; -/// Exported host directory, hidden paths, and per-path bind mounts of one sandbox. +/// Exported host directories and per-path bind mounts of one sandbox. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub(crate) struct HostMapping { - /// Canonical host directory exported through virtio-fs. + /// Children of the aggregate export; each is named by its index. + pub(crate) children: Vec, + /// Bind mounts the guest agent creates, parents before children. + pub(crate) binds: Vec, +} + +/// One host directory of the aggregate export. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub(crate) struct Child { + /// Canonical host directory. pub(crate) root: PathBuf, - /// Whether the export is writable, which any read-write mapping requires. + /// Whether the guest may modify it, which a read-write mapping inside it requires. pub(crate) writable: bool, - /// Canonical host paths inside the export that OpenVMM hides. + /// Whether OpenVMM hides everything but the allowed paths, because the root is no mapping. + pub(crate) hidden: bool, + /// Canonical host paths inside the child that OpenVMM hides. pub(crate) denied: Vec, /// Identities, at provision, of the denied paths that a workload could move: those inside /// a read-write mapping. - #[serde(default)] pub(crate) denied_identities: Vec>, - /// Bind mounts the guest agent creates, parents before children. - pub(crate) binds: Vec, + /// Canonical mapped paths that stay visible below a hidden root. + pub(crate) allowed: Vec, + /// Canonical paths to which OpenVMM limits the guest's writes; empty if the guest may modify + /// everything that it can reach in a writable child. + pub(crate) write: Vec, } /// Device and file index of a host object, which survive renames. @@ -61,7 +94,9 @@ pub(crate) struct FileIdentity { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub(crate) struct Bind { - /// Path relative to the export root, `/`-separated; empty for the root itself. + /// Index of the child that holds the mapped path. + pub(crate) child: usize, + /// Path relative to the child's root, `/`-separated; empty for the root itself. pub(crate) source: String, /// Absolute guest path. pub(crate) target: String, @@ -73,6 +108,21 @@ pub(crate) struct Bind { pub(crate) identity: Option, } +impl Bind { + /// Path of the mapped object relative to the guest's mount of the export. + pub(crate) fn guest_source(&self) -> String { + guest_source(self.child, &self.source) + } +} + +fn guest_source(child: usize, source: &str) -> String { + if source.is_empty() { + child.to_string() + } else { + format!("{child}/{source}") + } +} + /// Returns the guest path at which the backend exposes the host path `path`, or `None` if it /// has no guest equivalent (relative paths, UNC paths, or paths that are not valid UTF-8). /// @@ -130,9 +180,10 @@ pub fn resolve_guest_path(path: &Path) -> Option { /// /// Mapped paths must exist, and their guest paths derive from their resolved host paths, so two /// spellings of one object (links, letter case, or short names) map to one guest path. A path -/// that is both read-only and read-write is mapped read-only. A denied path must not contain a -/// mapped path, and a denied path that does not exist yet must not lie inside a mapped path, -/// because nothing could hide it once a workload creates it. +/// that is both read-only and read-write is mapped read-only. Neither a whole volume nor a file +/// directly in a volume's root can be mapped. A denied path must not contain a mapped path, and +/// a denied path that does not exist yet must not lie inside a mapped path, because nothing +/// could hide it once a workload creates it. pub(crate) fn plan(policy: &FilesystemPolicy) -> Result> { let unsupported = |message: String| Err(Error::policy_validation(message)); let mut mapped: Vec = Vec::new(); @@ -158,6 +209,9 @@ pub(crate) fn plan(policy: &FilesystemPolicy) -> Result> { path.display() )); } + if let Some(reason) = volume_root_refusal(&canonical, metadata.is_dir()) { + return unsupported(format!("{field} entry {} {reason}", path.display())); + } let Some(target) = guest_path(&canonical) else { return unsupported(format!( "{field} entry {} has no guest path; use an absolute local path", @@ -189,6 +243,11 @@ pub(crate) fn plan(policy: &FilesystemPolicy) -> Result> { if mapped.is_empty() { return Ok(None); } + if mapped.len() > MAX_HOST_MAPPINGS { + return unsupported(format!( + "the openvmm backend maps at most {MAX_HOST_MAPPINGS} paths into one sandbox" + )); + } // A Windows host opens files case-insensitively, while the guest lets a file have several // names, so a read-only file inside a read-write directory stays writable under another // spelling of its name. @@ -214,30 +273,205 @@ pub(crate) fn plan(policy: &FilesystemPolicy) -> Result> { )); } - let root = common_directory(&mapped).ok_or_else(|| { - Error::policy_validation( - "the openvmm backend exports one host directory, and the mapped paths share none; \ - place them on one volume", - ) - })?; - if !root - .components() - .any(|component| matches!(component, Component::Normal(_))) - { + let roots = export_roots(&mapped)?; + if roots.len() > MAX_CHILDREN { return unsupported(format!( - "the mapped paths share only {}, and exporting a whole volume is refused; place \ - them under one directory", - root.display() + "the mapped paths lie in {} separate host directories, but OpenVMM exports at most \ + {MAX_CHILDREN}", + roots.len() )); } - let root_text = display_path(&root); - if root_text.contains(',') { + let denied = denied_paths(policy, &mapped)?; + // Only objects inside a read-write mapping can be swapped by a workload; pinning others + // would make ordinary host edits, which often replace files, block the next start. + let movable = |path: &Path| { + mapped.iter().any(|entry| { + entry.directory + && !entry.read_only + && path != entry.canonical + && path.starts_with(&entry.canonical) + }) + }; + + let mut children = Vec::with_capacity(roots.len()); + let mut policy_bytes = 0; + for root in roots { + let inside: Vec<&Mapped> = mapped + .iter() + .filter(|entry| entry.canonical.starts_with(&root)) + .collect(); + let hidden = !inside.iter().any(|entry| entry.canonical == root); + let visible = outermost(&inside); + let read_write: Vec<&Mapped> = inside + .iter() + .copied() + .filter(|entry| !entry.read_only) + .collect(); + let writable = !read_write.is_empty(); + // Unless the read-write mappings cover everything that the guest reaches in the child, + // OpenVMM limits writes to them. + let mut write: Vec = if writable && visible.iter().any(|entry| entry.read_only) { + outermost(&read_write) + .iter() + .map(|entry| entry.canonical.clone()) + .collect() + } else { + Vec::new() + }; + write.sort(); + let mut allowed: Vec = if hidden { + visible + .iter() + .map(|entry| entry.canonical.clone()) + .collect() + } else { + Vec::new() + }; + allowed.sort(); + // Below a hidden root, only paths inside an allowed path need hiding. + let child_denied: Vec = denied + .iter() + .filter(|path| { + path.starts_with(&root) + && (!hidden || allowed.iter().any(|allowed| path.starts_with(allowed))) + }) + .cloned() + .collect(); + check_root(&root)?; + policy_bytes += check_policy_paths(&root, &child_denied, "hide", hidden)?; + policy_bytes += check_policy_paths(&root, &allowed, "expose", false)?; + policy_bytes += check_policy_paths(&root, &write, "make writable", false)?; + let denied_identities = child_denied + .iter() + .map(|path| movable(path).then(|| identity(path)).transpose()) + .collect::>>()?; + children.push(Child { + root, + writable, + hidden, + denied: child_denied, + denied_identities, + allowed, + write, + }); + } + if policy_bytes > MAX_AGGREGATE_POLICY_BYTES { return unsupported(format!( - "the common directory {root_text} of the mapped paths contains a comma, which \ - OpenVMM does not accept" + "the paths that OpenVMM is to hide, expose, or make writable exceed its \ + {MAX_AGGREGATE_POLICY_BYTES}-byte limit" )); } + let mut binds = Vec::with_capacity(mapped.len()); + for entry in &mapped { + let Some(child) = children + .iter() + .position(|child| entry.canonical.starts_with(&child.root)) + else { + return Err(Error::backend_error(format!( + "{} lies in no exported directory", + entry.given.display() + ))); + }; + let Some(source) = relative_text(&children[child].root, &entry.canonical) else { + return unsupported(format!("{} is not valid UTF-8", entry.given.display())); + }; + if GUEST_EXPORT.len() + 1 + guest_source(child, &source).len() > MAX_GUEST_PATH + || entry.target.len() > MAX_GUEST_PATH + { + return unsupported(format!( + "{} is too long for a guest path", + entry.given.display() + )); + } + binds.push(Bind { + child, + source, + target: entry.target.clone(), + read_only: entry.read_only, + identity: movable(&entry.canonical) + .then(|| identity(&entry.canonical)) + .transpose()?, + }); + } + binds.sort_by(|left, right| { + depth(&left.target) + .cmp(&depth(&right.target)) + .then_with(|| left.target.cmp(&right.target)) + }); + Ok(Some(HostMapping { children, binds })) +} + +/// Explains why a mapped host path, given in canonical form, cannot be exported, or returns +/// `None` if it can: a whole volume, or a file directly in a volume's root, which would export +/// that root as the file's parent. It inspects only the path, not the host. +fn volume_root_refusal(canonical: &Path, directory: bool) -> Option<&'static str> { + let is_volume_root = |path: &Path| { + !path + .components() + .any(|component| matches!(component, Component::Normal(_))) + }; + if directory { + is_volume_root(canonical).then_some( + "is a whole volume, which the openvmm backend does not map; map the directories \ + inside it", + ) + } else { + canonical.parent().is_none_or(is_volume_root).then_some( + "lies directly in a volume's root, which the openvmm backend does not export; move \ + the file into a directory", + ) + } +} + +/// Returns the directories to export: the outermost mapped directories and the parents of the +/// outermost mapped files, in order. Distinct paths must not name one directory. +fn export_roots(mapped: &[Mapped]) -> Result> { + let mut candidates = Vec::with_capacity(mapped.len()); + for entry in mapped { + if entry.directory { + candidates.push(entry.canonical.clone()); + } else { + let Some(parent) = entry.canonical.parent() else { + return Err(Error::policy_validation(format!( + "{} has no parent directory", + entry.given.display() + ))); + }; + candidates.push(parent.to_path_buf()); + } + } + candidates.sort(); + candidates.dedup(); + let roots: Vec = candidates + .iter() + .filter(|candidate| { + !candidates + .iter() + .any(|other| other != *candidate && candidate.starts_with(other)) + }) + .cloned() + .collect(); + // OpenVMM refuses one directory exported twice, which bind mounts make possible. + let mut identities: Vec<(FileIdentity, &Path)> = Vec::with_capacity(roots.len()); + for root in &roots { + let current = identity(root)?; + if let Some((_, other)) = identities.iter().find(|(other, _)| *other == current) { + return Err(Error::policy_validation(format!( + "{} and {} are the same host directory; map it once", + other.display(), + root.display() + ))); + } + identities.push((current, root)); + } + Ok(roots) +} + +/// Resolves the denied paths that OpenVMM needs to hide: those that exist, without the ones +/// that other denied paths contain. +fn denied_paths(policy: &FilesystemPolicy, mapped: &[Mapped]) -> Result> { + let unsupported = |message: String| Err(Error::policy_validation(message)); let mut denied: Vec = Vec::new(); for path in &policy.denied_paths { let (canonical, exists) = canonicalize_lenient(path).map_err(|error| { @@ -257,9 +491,6 @@ pub(crate) fn plan(policy: &FilesystemPolicy) -> Result> { entry.given.display() )); } - if !canonical.starts_with(&root) { - continue; - } if !exists { if let Some(entry) = mapped .iter() @@ -288,47 +519,20 @@ pub(crate) fn plan(policy: &FilesystemPolicy) -> Result> { .cloned() .collect(); denied.retain(|path| !nested.contains(path)); - check_denied_paths(&root, &denied)?; - // Only objects inside a read-write mapping can be swapped by a workload; pinning others - // would make ordinary host edits, which often replace files, block the next start. - let movable = |path: &Path| { - mapped.iter().any(|entry| { - entry.directory - && !entry.read_only - && path != entry.canonical - && path.starts_with(&entry.canonical) - }) - }; - let denied_identities = denied - .iter() - .map(|path| movable(path).then(|| identity(path)).transpose()) - .collect::>>()?; + Ok(denied) +} - let mut binds = Vec::with_capacity(mapped.len()); - for entry in &mapped { - binds.push(Bind { - source: relative_text(&root, &entry.canonical).ok_or_else(|| { - Error::policy_validation(format!("{} is not valid UTF-8", entry.given.display())) - })?, - target: entry.target.clone(), - read_only: entry.read_only, - identity: movable(&entry.canonical) - .then(|| identity(&entry.canonical)) - .transpose()?, - }); - } - binds.sort_by(|left, right| { - depth(&left.target) - .cmp(&depth(&right.target)) - .then_with(|| left.target.cmp(&right.target)) - }); - Ok(Some(HostMapping { - root, - writable: mapped.iter().any(|entry| !entry.read_only), - denied, - denied_identities, - binds, - })) +/// Returns the entries that no other entry contains. +fn outermost<'a>(entries: &[&'a Mapped]) -> Vec<&'a Mapped> { + entries + .iter() + .copied() + .filter(|entry| { + !entries.iter().any(|other| { + other.canonical != entry.canonical && entry.canonical.starts_with(&other.canonical) + }) + }) + .collect() } /// Checks that every mapped and denied path inside a read-write mapping still names the object @@ -349,50 +553,96 @@ pub(crate) fn verify(mapping: &HostMapping) -> Result<()> { let Some(expected) = bind.identity else { continue; }; + let Some(child) = mapping.children.get(bind.child) else { + return Err(Error::backend_error( + "the sandbox state maps a path of an unknown host directory", + )); + }; let path = if bind.source.is_empty() { - mapping.root.clone() + child.root.clone() } else { - mapping.root.join(&bind.source) + child.root.join(&bind.source) }; match platform::file_identity(&path) { Ok((device, index)) if (FileIdentity { device, index }) == expected => {} _ => return changed(&path), } } - for (path, expected) in mapping.denied.iter().zip(&mapping.denied_identities) { - let Some(expected) = expected else { + for child in &mapping.children { + for (path, expected) in child.denied.iter().zip(&child.denied_identities) { + let Some(expected) = expected else { + continue; + }; + match platform::file_identity(path) { + Ok((device, index)) if (FileIdentity { device, index }) == *expected => {} + _ => return changed(path), + } + } + } + Ok(()) +} + +/// Applies OpenVMM's rules for exported directories, which it reaches without links. +fn check_root(root: &Path) -> Result<()> { + let mut current = PathBuf::new(); + for component in root.components() { + current.push(component.as_os_str()); + if !matches!(component, Component::Normal(_)) { continue; - }; - match platform::file_identity(path) { - Ok((device, index)) if (FileIdentity { device, index }) == *expected => {} - _ => return changed(path), + } + let metadata = fs::symlink_metadata(¤t).map_err(|error| { + Error::policy_validation(format!("cannot inspect {}", current.display())) + .with_source(error) + })?; + if metadata.file_type().is_symlink() || is_reparse_point(&metadata) { + return Err(Error::policy_validation(format!( + "OpenVMM cannot export {}, because {} is a link or reparse point", + root.display(), + current.display() + ))); } } Ok(()) } -/// Applies OpenVMM's rules for hidden paths, so a policy that OpenVMM would refuse fails at -/// provision rather than at every start. -fn check_denied_paths(root: &Path, denied: &[PathBuf]) -> Result<()> { +/// Applies OpenVMM's rules for the paths that it is to `action` in the exported directory +/// `root`, so a policy that OpenVMM would refuse fails at provision rather than at every start. +/// `with_root` counts the root itself among them. Returns their combined length. +fn check_policy_paths( + root: &Path, + paths: &[PathBuf], + action: &str, + with_root: bool, +) -> Result { let unsupported = |message: String| Err(Error::policy_validation(message)); - if denied.len() > MAX_DENIED_PATHS { + let count = paths.len() + usize::from(with_root); + if count > MAX_POLICY_PATHS { return unsupported(format!( - "OpenVMM hides at most {MAX_DENIED_PATHS} paths inside the mapped directories" + "OpenVMM can {action} at most {MAX_POLICY_PATHS} paths in one exported directory, \ + but {} needs {count}", + root.display() )); } let mut total = 0; - for path in denied { + for path in paths { let Some(relative) = relative_text(root, path) else { return unsupported(format!("{} is not valid UTF-8", path.display())); }; - if relative - .chars() - .any(|character| character.is_whitespace() || matches!(character, ':' | '\\')) + // A name may contain spaces, but not begin or end with one. + if relative.len() > MAX_POLICY_PATH_BYTES + || relative.chars().any(|character| { + (character.is_whitespace() && character != ' ') || matches!(character, ':' | '\\') + }) + || relative + .split('/') + .any(|name| name.starts_with(' ') || name.ends_with(' ')) { return unsupported(format!( - "OpenVMM cannot hide {}: hidden paths must not contain whitespace, colons, or \ - backslashes below the mapped directories", - path.display() + "OpenVMM cannot {action} {}: below the exported directory {}, a path must not \ + contain colons, backslashes, or whitespace other than spaces, and no name may \ + begin or end with a space", + path.display(), + root.display() )); } total += relative.len(); @@ -405,7 +655,7 @@ fn check_denied_paths(root: &Path, denied: &[PathBuf]) -> Result<()> { })?; if metadata.file_type().is_symlink() || is_reparse_point(&metadata) { return unsupported(format!( - "OpenVMM cannot hide {} through the link {}", + "OpenVMM cannot {action} {}, because {} is a link or reparse point", path.display(), current.display() )); @@ -416,18 +666,20 @@ fn check_denied_paths(root: &Path, denied: &[PathBuf]) -> Result<()> { != platform::file_identity(root).ok().map(|(device, _)| device) { return unsupported(format!( - "OpenVMM cannot hide {}, which lies on another file system than {}", + "OpenVMM cannot {action} {}, which lies on another file system than {}", path.display(), root.display() )); } } - if total > MAX_DENIED_BYTES { + if total > MAX_POLICY_BYTES { return unsupported(format!( - "the hidden paths exceed OpenVMM's {MAX_DENIED_BYTES}-byte limit" + "the paths that OpenVMM is to {action} in {} exceed its {MAX_POLICY_BYTES}-byte \ + limit", + root.display() )); } - Ok(()) + Ok(total) } #[cfg(windows)] @@ -460,33 +712,54 @@ fn relative_text(root: &Path, path: &Path) -> Option { Some(parts.join("/")) } -/// Kernel command-line tokens that tell the guest agent which bind mounts to create. -pub(crate) fn kernel_tokens(mapping: &HostMapping) -> Vec { +/// Kernel command-line token that tells the guest agent how many bind mounts to expect over +/// the control channel. +pub(crate) fn kernel_token(mapping: &HostMapping) -> String { + format!("nvx_maps={}", mapping.binds.len()) +} + +/// The bind mounts in the order and form in which the guest agent receives them. +pub(crate) fn guest_table(mapping: &HostMapping) -> Vec { mapping .binds .iter() - .map(|bind| { - let source = if bind.source.is_empty() { - ".".to_owned() - } else { - encode(&bind.source) - }; - let mode = if bind.read_only { "ro" } else { "rw" }; - format!("nvx_map={source},{},{mode}", encode(&bind.target)) + .map(|bind| HostMap { + source: bind.guest_source(), + target: bind.target.clone(), + read_only: bind.read_only, }) .collect() } -/// OpenVMM options that export the mapping's root and hide its denied paths. +/// OpenVMM options that export the mapping's directories with their access policies. pub(crate) fn openvmm_arguments(mapping: &HostMapping) -> Vec { - let mode = if mapping.writable { "rw" } else { "ro" }; - let mut arguments = vec![ - "--mount".to_owned(), - format!("{GUEST_EXPORT},{},{mode}", display_path(&mapping.root)), - ]; - for path in &mapping.denied { - arguments.push("--mount-deny".to_owned()); + let mut arguments = vec!["--mount-aggregate".to_owned(), GUEST_EXPORT.to_owned()]; + for (index, child) in mapping.children.iter().enumerate() { + let mode = if child.writable { "rw" } else { "ro" }; + arguments.push("--mount-child".to_owned()); + arguments.push(format!("{index},{},{mode}", display_path(&child.root))); + } + let mut add = |option: &str, path: &Path| { + arguments.push(option.to_owned()); arguments.push(display_path(path)); + }; + for child in mapping.children.iter().filter(|child| child.hidden) { + add("--mount-deny", &child.root); + } + for child in &mapping.children { + for path in &child.denied { + add("--mount-deny", path); + } + } + for child in &mapping.children { + for path in &child.allowed { + add("--mount-allow", path); + } + } + for child in &mapping.children { + for path in &child.write { + add("--mount-write", path); + } } arguments } @@ -510,48 +783,6 @@ fn depth(path: &str) -> usize { path.split('/').filter(|part| !part.is_empty()).count() } -/// Percent-encodes everything but unreserved characters and `/`, so a value is one kernel -/// command-line token without commas. -fn encode(value: &str) -> String { - let mut encoded = String::with_capacity(value.len()); - for byte in value.bytes() { - if byte.is_ascii_alphanumeric() || matches!(byte, b'/' | b'.' | b'_' | b'~' | b'-') { - encoded.push(char::from(byte)); - } else { - encoded.push_str(&format!("%{byte:02X}")); - } - } - encoded -} - -/// Deepest directory that contains every mapped path. -fn common_directory(mapped: &[Mapped]) -> Option { - let mut common: Option = None; - for entry in mapped { - let directory = if entry.directory { - entry.canonical.clone() - } else { - entry.canonical.parent()?.to_path_buf() - }; - common = Some(match common { - None => directory, - Some(common) => { - let shared: PathBuf = common - .components() - .zip(directory.components()) - .take_while(|(left, right)| left == right) - .map(|(component, _)| component) - .collect(); - if !shared.has_root() { - return None; - } - shared - } - }); - } - common -} - /// Resolves links and returns a path without the Windows verbatim prefix. fn canonicalize(path: &Path) -> std::io::Result { let canonical = fs::canonicalize(path)?; @@ -620,10 +851,19 @@ mod tests { fn root() -> tempfile::TempDir { let directory = tempfile::tempdir().unwrap(); - for name in ["work/src", "work/out", "work/src/secret", "tools"] { + for name in [ + "work/src/secret", + "work/out", + "work/private", + "projects/app", + "projects/private", + "build-output", + "tools/bin", + ] { fs::create_dir_all(directory.path().join(name)).unwrap(); } fs::write(directory.path().join("work/config.json"), b"{}").unwrap(); + fs::write(directory.path().join("work/run.sh"), b"true").unwrap(); directory } @@ -631,6 +871,22 @@ mod tests { canonicalize(path).unwrap() } + fn sources(mapping: &HostMapping) -> Vec<(String, bool)> { + mapping + .binds + .iter() + .map(|bind| (bind.guest_source(), bind.read_only)) + .collect() + } + + fn option_values(arguments: &[String], option: &str) -> Vec { + arguments + .windows(2) + .filter(|pair| pair[0] == option) + .map(|pair| pair[1].clone()) + .collect() + } + #[test] fn guest_paths_follow_the_mxc_convention() { if cfg!(windows) { @@ -655,67 +911,205 @@ mod tests { assert_eq!(guest_path(Path::new("..")), None); } + /// The layout of microsoft/nvx#281: directories that share no directory but the volume's + /// root, each with its own access. #[test] - fn multiple_paths_share_one_export() { + fn unrelated_directories_are_exported_with_their_own_access() { let directory = root(); let base = directory.path(); let policy = FilesystemPolicy { - readonly_paths: vec![base.join("work/src"), base.join("work/config.json")], - readwrite_paths: vec![base.join("work/out")], - denied_paths: vec![base.join("work/src/secret"), base.join("elsewhere")], + readonly_paths: vec![base.join("tools")], + readwrite_paths: vec![base.join("projects/app"), base.join("build-output")], + ..FilesystemPolicy::default() }; let mapping = plan(&policy).unwrap().unwrap(); - assert_eq!(mapping.root, canonical(&base.join("work"))); - assert!(mapping.writable); - assert_eq!(mapping.denied, [canonical(&base.join("work/src/secret"))]); - let sources: Vec<(&str, bool)> = mapping - .binds + let children: Vec<(PathBuf, bool, bool)> = mapping + .children .iter() - .map(|bind| (bind.source.as_str(), bind.read_only)) + .map(|child| (child.root.clone(), child.writable, child.hidden)) .collect(); assert_eq!( - sources, - [("config.json", true), ("out", false), ("src", true)] + children, + [ + (canonical(&base.join("build-output")), true, false), + (canonical(&base.join("projects/app")), true, false), + (canonical(&base.join("tools")), false, false), + ] ); - let target = guest_path(&canonical(&base.join("work/out"))).unwrap(); - assert!( - mapping.binds.iter().any(|bind| bind.target == target), - "missing canonical guest target {target:?} in {:?}", - mapping.binds + assert!(mapping.children.iter().all(|child| child.write.is_empty() + && child.allowed.is_empty() + && child.denied.is_empty())); + // Parents mount before children, so the shallower targets come first. + assert_eq!( + sources(&mapping), + [ + ("0".to_owned(), false), + ("2".to_owned(), true), + ("1".to_owned(), false) + ] + ); + + let arguments = openvmm_arguments(&mapping); + assert_eq!(arguments[..2], ["--mount-aggregate", GUEST_EXPORT]); + assert_eq!( + option_values(&arguments, "--mount-child"), + [ + format!( + "0,{},rw", + display_path(&canonical(&base.join("build-output"))) + ), + format!( + "1,{},rw", + display_path(&canonical(&base.join("projects/app"))) + ), + format!("2,{},ro", display_path(&canonical(&base.join("tools")))), + ] + ); + assert_eq!(arguments.len(), 8); + assert_eq!(kernel_token(&mapping), "nvx_maps=3"); + let table = guest_table(&mapping); + assert_eq!(table.len(), 3); + assert_eq!( + table[1].target, + guest_path(&canonical(&base.join("tools"))).unwrap() + ); + assert!(table[1].read_only); + } + + #[test] + fn mapped_files_are_exposed_through_their_hidden_parents() { + let directory = root(); + let base = directory.path(); + let policy = FilesystemPolicy { + readonly_paths: vec![base.join("work/config.json"), base.join("work/src")], + readwrite_paths: vec![base.join("work/run.sh"), base.join("work/out")], + denied_paths: vec![ + // Hidden already: the parent hides everything but its mapped paths. + base.join("work/private"), + base.join("work/src/secret"), + ], + }; + let mapping = plan(&policy).unwrap().unwrap(); + assert_eq!(mapping.children.len(), 1); + let child = &mapping.children[0]; + let work = canonical(&base.join("work")); + assert_eq!(child.root, work); + assert!(child.hidden && child.writable); + assert_eq!( + child.allowed, + ["config.json", "out", "run.sh", "src"].map(|name| work.join(name)) + ); + // The read-only mappings stay read-only on the host. + assert_eq!(child.write, ["out", "run.sh"].map(|name| work.join(name))); + assert_eq!(child.denied, [work.join("src").join("secret")]); + assert_eq!( + sources(&mapping), + [ + ("0/config.json".to_owned(), true), + ("0/out".to_owned(), false), + ("0/run.sh".to_owned(), false), + ("0/src".to_owned(), true), + ] ); let arguments = openvmm_arguments(&mapping); - assert_eq!(arguments[0], "--mount"); - assert!(arguments[1].starts_with(&format!("{GUEST_EXPORT},"))); - assert!(arguments[1].ends_with(",rw")); - assert_eq!(arguments[2], "--mount-deny"); - let tokens = kernel_tokens(&mapping); - assert_eq!(tokens.len(), 3); - assert!(tokens.iter().all(|token| token.starts_with("nvx_map="))); - assert!(tokens[0].ends_with(",ro")); + assert_eq!( + option_values(&arguments, "--mount-deny"), + [ + display_path(&work), + display_path(&work.join("src").join("secret")) + ] + ); + assert_eq!(option_values(&arguments, "--mount-allow").len(), 4); + assert_eq!(option_values(&arguments, "--mount-write").len(), 2); } #[test] - fn a_single_directory_is_exported_itself_and_parents_mount_first() { + fn nested_mappings_merge_into_the_outermost_directory() { let directory = root(); let base = directory.path(); + fs::create_dir_all(base.join("work/out/cache")).unwrap(); + let policy = FilesystemPolicy { + readonly_paths: vec![ + base.join("work"), + base.join("work/src"), + base.join("work/out/cache"), + ], + readwrite_paths: vec![base.join("work/out")], + ..FilesystemPolicy::default() + }; + let mapping = plan(&policy).unwrap().unwrap(); + assert_eq!(mapping.children.len(), 1); + let child = &mapping.children[0]; + let work = canonical(&base.join("work")); + assert!(!child.hidden && child.writable); + // The read-only root is read-only on the host; the cache inside the read-write mapping + // is left to the guest's bind mount. + assert_eq!(child.write, [work.join("out")]); + assert_eq!( + sources(&mapping), + [ + ("0".to_owned(), true), + ("0/out".to_owned(), false), + ("0/src".to_owned(), true), + ("0/out/cache".to_owned(), true), + ] + ); + + // A read-write root covers everything, so OpenVMM need not limit writes. let policy = FilesystemPolicy { readonly_paths: vec![base.join("work/src")], readwrite_paths: vec![base.join("work"), base.join("work/src")], ..FilesystemPolicy::default() }; let mapping = plan(&policy).unwrap().unwrap(); - assert_eq!(mapping.root, canonical(&base.join("work"))); - let binds: Vec<(&str, bool)> = mapping - .binds + assert!(mapping.children[0].writable && mapping.children[0].write.is_empty()); + // The read-write duplicate of the read-only path is tightened to read-only. + assert_eq!( + sources(&mapping), + [("0".to_owned(), false), ("0/src".to_owned(), true)] + ); + } + + #[test] + fn denied_paths_belong_to_the_directory_that_contains_them() { + let directory = root(); + let base = directory.path(); + let policy = FilesystemPolicy { + readonly_paths: vec![base.join("work/src"), base.join("projects")], + readwrite_paths: vec![base.join("build-output")], + denied_paths: vec![ + base.join("work/src/secret"), + base.join("projects/private"), + base.join("work/out"), + base.join("elsewhere"), + ], + }; + let mapping = plan(&policy).unwrap().unwrap(); + let denied: Vec> = mapping + .children .iter() - .map(|bind| (bind.source.as_str(), bind.read_only)) + .map(|child| child.denied.clone()) .collect(); - // The read-write duplicate of the read-only path is tightened to read-only. - assert_eq!(binds, [("", false), ("src", true)]); assert_eq!( - kernel_tokens(&mapping)[0], - format!("nvx_map=.,{},rw", encode(&mapping.binds[0].target)) + denied, + [ + Vec::new(), + vec![canonical(&base.join("projects/private"))], + vec![canonical(&base.join("work/src/secret"))], + ] + ); + // Nothing a workload could move is pinned without a read-write parent. + assert!( + mapping + .children + .iter() + .flat_map(|child| &child.denied_identities) + .all(Option::is_none) + ); + assert_eq!( + option_values(&openvmm_arguments(&mapping), "--mount-deny").len(), + 2 ); } @@ -738,6 +1132,16 @@ mod tests { denied_paths: vec![base.join("work/out/not-yet")], ..FilesystemPolicy::default() }, + FilesystemPolicy { + readonly_paths: vec![base.join("work/config.json")], + denied_paths: vec![base.join("work")], + ..FilesystemPolicy::default() + }, + // A whole volume is never exported. + FilesystemPolicy { + readonly_paths: vec![PathBuf::from(if cfg!(windows) { r"C:\" } else { "/" })], + ..FilesystemPolicy::default() + }, ]; for policy in cases { assert_eq!( @@ -763,6 +1167,48 @@ mod tests { assert_eq!(plan(&empty).unwrap(), None); } + #[test] + fn volumes_and_files_directly_in_their_roots_are_refused() { + // The paths are only inspected, so nothing is created at a volume root. + let (volumes, directories, files_in_roots, nested_files): ( + &[&str], + &[&str], + &[&str], + &[&str], + ) = if cfg!(windows) { + ( + &[r"C:\", r"D:\"], + &[r"C:\work", r"D:\a\b"], + &[r"C:\notes.txt", r"D:\build.log"], + &[r"C:\work\notes.txt", r"D:\a\b\build.log"], + ) + } else { + ( + &["/"], + &["/work", "/srv/a/b"], + &["/notes.txt", "/build.log"], + &["/work/notes.txt", "/srv/a/b/build.log"], + ) + }; + for volume in volumes { + let reason = volume_root_refusal(Path::new(volume), true).unwrap(); + assert!(reason.contains("whole volume"), "{volume}: {reason}"); + } + for file in files_in_roots { + let reason = volume_root_refusal(Path::new(file), false).unwrap(); + assert!( + reason.contains("move the file into a directory"), + "{file}: {reason}" + ); + } + for directory in directories { + assert_eq!(volume_root_refusal(Path::new(directory), true), None); + } + for file in nested_files { + assert_eq!(volume_root_refusal(Path::new(file), false), None); + } + } + #[test] fn spellings_of_one_object_share_its_guest_path() { let directory = root(); @@ -780,6 +1226,7 @@ mod tests { let mapping = plan(&policy).unwrap().unwrap(); assert_eq!(mapping.binds.len(), 1); assert!(mapping.binds[0].read_only); + assert!(!mapping.children[0].writable); assert_eq!( mapping.binds[0].target, guest_path(&canonical(&base.join("work").join("src"))).unwrap() @@ -798,30 +1245,53 @@ mod tests { } #[test] - fn denied_paths_follow_openvmm_rules() { + fn policy_paths_follow_openvmm_rules() { let directory = root(); let base = directory.path(); - fs::create_dir_all(base.join("work/out/My Secrets")).unwrap(); + for name in ["work/out/My Secrets", "work/out/ leading", "my files"] { + fs::create_dir_all(base.join(name)).unwrap(); + } + fs::write(base.join("my files/notes one.txt"), b"notes").unwrap(); + // Names may contain spaces. let spaced = FilesystemPolicy { + readonly_paths: vec![base.join("my files/notes one.txt")], readwrite_paths: vec![base.join("work/out")], denied_paths: vec![base.join("work/out/My Secrets")], + }; + let mapping = plan(&spaced).unwrap().unwrap(); + let arguments = openvmm_arguments(&mapping); + assert!( + option_values(&arguments, "--mount-allow")[0].ends_with("notes one.txt"), + "{arguments:?}" + ); + assert!(option_values(&arguments, "--mount-deny")[1].ends_with("My Secrets")); + // But they must not begin or end with one. + let leading = FilesystemPolicy { + readwrite_paths: vec![base.join("work/out")], + denied_paths: vec![base.join("work/out/ leading")], ..FilesystemPolicy::default() }; assert_eq!( - plan(&spaced).unwrap_err().code(), + plan(&leading).unwrap_err().code(), ErrorCode::PolicyValidation ); - if cfg!(windows) { - let volume = FilesystemPolicy { - readonly_paths: vec![base.join("work"), PathBuf::from(r"C:\Windows")], - ..FilesystemPolicy::default() - }; - if canonical(base).starts_with(r"C:\") { + if !cfg!(windows) { + fs::create_dir_all(base.join("work/out/tab\there")).unwrap(); + fs::create_dir_all(base.join("work/out/colon:here")).unwrap(); + for name in ["tab\there", "colon:here"] { + let policy = FilesystemPolicy { + readwrite_paths: vec![base.join("work/out")], + denied_paths: vec![base.join("work/out").join(name)], + ..FilesystemPolicy::default() + }; assert_eq!( - plan(&volume).unwrap_err().code(), - ErrorCode::PolicyValidation + plan(&policy).unwrap_err().code(), + ErrorCode::PolicyValidation, + "{name}" ); } + } + if cfg!(windows) { let nested_file = FilesystemPolicy { readonly_paths: vec![base.join("work/config.json")], readwrite_paths: vec![base.join("work")], @@ -834,6 +1304,35 @@ mod tests { } } + #[test] + fn openvmm_limits_are_checked_per_directory() { + let directory = root(); + let base = directory.path(); + let files = |count: usize| -> Vec { + (0..count) + .map(|index| { + let path = base.join("tools").join(format!("tool-{index:03}")); + fs::write(&path, b"x").unwrap(); + path + }) + .collect() + }; + // A hidden root exposes at most 128 mapped paths. + let policy = FilesystemPolicy { + readonly_paths: files(128), + ..FilesystemPolicy::default() + }; + let mapping = plan(&policy).unwrap().unwrap(); + assert_eq!(mapping.children[0].allowed.len(), 128); + let policy = FilesystemPolicy { + readonly_paths: files(129), + ..FilesystemPolicy::default() + }; + let error = plan(&policy).unwrap_err(); + assert_eq!(error.code(), ErrorCode::PolicyValidation); + assert!(error.message().contains("at most 128"), "{error}"); + } + #[test] fn swapped_objects_fail_verification() { let directory = root(); @@ -845,7 +1344,12 @@ mod tests { denied_paths: vec![base.join("work/src/secret")], }; let mapping = plan(&policy).unwrap().unwrap(); - assert!(mapping.denied_identities.iter().all(Option::is_some)); + assert!( + mapping.children[0] + .denied_identities + .iter() + .all(Option::is_some) + ); verify(&mapping).unwrap(); fs::rename(base.join("work/src/secret"), base.join("work/src/moved")).unwrap(); fs::create_dir(base.join("work/src/secret")).unwrap(); @@ -870,14 +1374,46 @@ mod tests { ..FilesystemPolicy::default() }; let mapping = plan(&read_only).unwrap().unwrap(); - fs::remove_dir(base.join("tools")).unwrap(); + fs::remove_dir_all(base.join("tools")).unwrap(); fs::create_dir(base.join("tools")).unwrap(); verify(&mapping).unwrap(); } + /// A directory on another volume than the temporary directory, if the host has one: the one + /// that `ACI_EDGE_SANDBOXES_TEST_SECOND_VOLUME` names, or the runner's temporary directory + /// on GitHub's Windows runners. Tests create their files below it. Linux's `/dev/shm` does + /// not qualify, because it lies in the guest's reserved `/dev`. + fn second_volume() -> Option { + let candidate = std::env::var_os("ACI_EDGE_SANDBOXES_TEST_SECOND_VOLUME") + .or_else(|| std::env::var_os("RUNNER_TEMP")) + .map(PathBuf::from)?; + let temporary = canonical(&std::env::temp_dir()); + let candidate = canonicalize(&candidate).ok()?; + let volume = |path: &Path| platform::file_identity(path).ok().map(|(device, _)| device); + (candidate.is_dir() && volume(&candidate) != volume(&temporary)).then_some(candidate) + } + #[test] - fn encoding_keeps_tokens_free_of_separators() { - assert_eq!(encode("/mnt/c/My Work,1%"), "/mnt/c/My%20Work%2C1%25"); - assert_eq!(encode("a-b_c.d~e"), "a-b_c.d~e"); + fn directories_on_other_volumes_are_exported_too() { + let Some(volume) = second_volume() else { + eprintln!("skipped: the host has no second volume"); + return; + }; + let directory = root(); + let other = tempfile::tempdir_in(volume).unwrap(); + fs::create_dir(other.path().join("cache")).unwrap(); + let policy = FilesystemPolicy { + readonly_paths: vec![directory.path().join("tools")], + readwrite_paths: vec![other.path().join("cache")], + ..FilesystemPolicy::default() + }; + let mapping = plan(&policy).unwrap().unwrap(); + assert_eq!(mapping.children.len(), 2); + let modes: Vec = mapping + .children + .iter() + .map(|child| child.writable) + .collect(); + assert!(modes.contains(&true) && modes.contains(&false)); } } diff --git a/aci_edge_sandboxes/src/openvmm/launch.rs b/aci_edge_sandboxes/src/openvmm/launch.rs index db3a67ee..9d5e86ad 100644 --- a/aci_edge_sandboxes/src/openvmm/launch.rs +++ b/aci_edge_sandboxes/src/openvmm/launch.rs @@ -5,7 +5,7 @@ //! init then starts the managed agent, which runs workloads as the configured non-root identity. //! The arguments mirror the managed-lifecycle check of `scripts/nvx_tools/microvm_tests.py`. -use std::ffi::OsString; +use std::ffi::{OsStr, OsString}; use std::path::Path; use super::config::OpenVmmConfig; @@ -15,6 +15,14 @@ use crate::error::{Error, Result}; /// Bytes of the x86 kernel command line available to caller tokens, including the NUL. pub(crate) const COMMAND_LINE_BUDGET: usize = 1024; +/// Longest Windows process command line, in UTF-16 code units, including the terminating NUL. +/// +/// The backend holds OpenVMM's command line to it on every host, so a policy behaves alike on +/// all of them. +pub(crate) const PROCESS_COMMAND_LINE_LIMIT: usize = 32_767; +/// 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; /// Returns the start of every sandbox's kernel command line, which the configuration alone /// determines: the caller's extra parameters and the workload hostname. @@ -27,25 +35,76 @@ pub(crate) fn base_command_line(extra: &str, hostname: &str) -> String { } /// Builds the kernel command line: the caller's extra parameters, the workload hostname, and -/// the guest's bind mounts of mapped host paths. +/// the number of host mappings that the guest agent receives over the control channel. pub(crate) fn kernel_command_line(extra: &str, record: &SandboxRecord) -> Result { let mut tokens = vec![base_command_line(extra, &record.hostname)]; if record.create_workload_account { tokens.push("nvx_workload_account=create".to_owned()); } if let Some(mapping) = &record.filesystem { - tokens.extend(filesystem::kernel_tokens(mapping)); + tokens.push(filesystem::kernel_token(mapping)); } let command_line = tokens.join(" "); if command_line.len() + 1 > COMMAND_LINE_BUDGET { return Err(Error::policy_validation(format!( - "the kernel command line exceeds its {COMMAND_LINE_BUDGET}-byte budget; map fewer or \ - shorter filesystem paths" + "the kernel command line exceeds its {COMMAND_LINE_BUDGET}-byte budget; shorten the \ + extra kernel parameters or the hostname" ))); } Ok(command_line) } +/// Fails if OpenVMM's command line for `record` would not fit [`PROCESS_COMMAND_LINE_LIMIT`] +/// with room to spare, as can happen with many filesystem paths. +pub(crate) fn check_process_command_line( + config: &OpenVmmConfig, + record: &SandboxRecord, + endpoint: &str, + report: &Path, +) -> Result<()> { + let arguments = openvmm_arguments(config, record, endpoint, report)?; + let length = process_command_line_units(config.openvmm.as_os_str(), &arguments); + if length + PROCESS_COMMAND_LINE_RESERVE > PROCESS_COMMAND_LINE_LIMIT { + return Err(Error::policy_validation(format!( + "OpenVMM's command line would take {length} of the {PROCESS_COMMAND_LINE_LIMIT} \ + characters that a Windows process command line allows, with \ + {PROCESS_COMMAND_LINE_RESERVE} held in reserve; map, hide, or allow fewer or \ + shorter filesystem paths, or use fewer network rules" + ))); + } + Ok(()) +} + +/// 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. +fn process_command_line_units(program: &OsStr, arguments: &[OsString]) -> usize { + let mut length = 0; + for (index, argument) in std::iter::once(program) + .chain(arguments.iter().map(OsString::as_os_str)) + .enumerate() + { + let text = argument.to_string_lossy(); + let quoted = index == 0 || text.is_empty() || text.contains([' ', '\t']); + // A separating space, or the terminating NUL after the last argument. + 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 +} + /// Builds the complete OpenVMM argument list for starting a sandbox. pub(crate) fn openvmm_arguments( config: &OpenVmmConfig, @@ -173,14 +232,20 @@ mod tests { } #[test] - fn mappings_add_the_export_and_guest_bind_tokens() { + fn mappings_add_the_export_and_the_mapping_count() { let mut record = record(); record.filesystem = Some(filesystem::HostMapping { - root: "/host/work".into(), - writable: false, - denied: vec!["/host/work/secret".into()], - denied_identities: Vec::new(), + children: vec![filesystem::Child { + root: "/host/work".into(), + writable: false, + hidden: false, + denied: vec!["/host/work/secret".into()], + denied_identities: vec![None], + allowed: Vec::new(), + write: Vec::new(), + }], binds: vec![filesystem::Bind { + child: 0, source: String::new(), target: "/host/work".to_owned(), read_only: true, @@ -189,25 +254,76 @@ mod tests { }); assert_eq!( kernel_command_line("", &record).unwrap(), - "hostname=nvx-sandbox nvx_map=.,/host/work,ro" + "hostname=nvx-sandbox nvx_maps=1" ); record.create_workload_account = true; assert_eq!( kernel_command_line("", &record).unwrap(), - "hostname=nvx-sandbox nvx_workload_account=create nvx_map=.,/host/work,ro" + "hostname=nvx-sandbox nvx_workload_account=create nvx_maps=1" ); record.create_workload_account = false; let config = OpenVmmConfig::new("/o", "/k/vmlinux", "/k/initrd", Hypervisor::Kvm, "/state"); let arguments = openvmm_arguments(&config, &record, "/e", Path::new("/r")).unwrap(); let arguments: Vec<&str> = arguments.iter().map(|a| a.to_str().unwrap()).collect(); assert_eq!( - arguments[arguments.len() - 4..], + arguments[arguments.len() - 6..], [ - "--mount", - "/run/nvx/hostfs/root,/host/work,ro", + "--mount-aggregate", + "/run/nvx/hostfs/root", + "--mount-child", + "0,/host/work,ro", "--mount-deny", "/host/work/secret", ] ); } + + #[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 config = OpenVmmConfig::new("/o", "/k/vmlinux", "/k/initrd", Hypervisor::Kvm, "/state"); + let mut record = record(); + let report = Path::new("/state/t/outcome.json"); + check_process_command_line(&config, &record, "/state/t/control.sock", report).unwrap(); + let denied: Vec = (0..128) + .map(|index| format!("/host/work/{index:03}-{}", "d".repeat(240)).into()) + .collect(); + record.filesystem = Some(filesystem::HostMapping { + children: vec![filesystem::Child { + root: "/host/work".into(), + writable: false, + hidden: false, + denied_identities: vec![None; denied.len()], + denied, + allowed: Vec::new(), + write: Vec::new(), + }], + binds: Vec::new(), + }); + let error = check_process_command_line(&config, &record, "/state/t/control.sock", report) + .unwrap_err(); + assert_eq!(error.code(), crate::ErrorCode::PolicyValidation); + } } diff --git a/aci_edge_sandboxes/src/openvmm/mod.rs b/aci_edge_sandboxes/src/openvmm/mod.rs index f11c0165..bad2e459 100644 --- a/aci_edge_sandboxes/src/openvmm/mod.rs +++ b/aci_edge_sandboxes/src/openvmm/mod.rs @@ -11,11 +11,12 @@ //! - **start** launches a detached OpenVMM process with the managed lifecycle, passes it a fresh //! 32-byte capability through standard input, and waits until the guest agent answers on the //! authenticated control console. The agent must also advertise the control features this -//! backend depends on (cancellation, host path mappings, workload accounts, workload -//! containment, per-execution environments, and working directories). A guest image that lacks +//! backend depends on (cancellation, workload accounts, workload containment, per-execution +//! environments, working directories, and host path mapping tables). A guest image that lacks //! one is terminated and start fails with //! [`ErrorCode::BackendUnavailable`](crate::ErrorCode::BackendUnavailable), because such an -//! image would silently ignore the policy or request that needs the feature. +//! image would silently ignore the policy or request that needs the feature. Start then hands +//! the agent the table of mapped host paths, and fails if the agent cannot mount them all. //! - **exec** runs the workload through the control console and streams its output live. //! - **stop** asks the guest to shut down and falls back to terminating OpenVMM after //! [`OpenVmmConfig::stop_timeout`]. @@ -46,9 +47,13 @@ //! [`ErrorCode::PolicyValidation`](crate::ErrorCode::PolicyValidation); [`guest_path`] //! translates host paths. //! -//! Host paths share OpenVMM's single virtio-fs export: the backend exports the deepest directory -//! that contains every mapped path to a guest directory that only the guest's root can enter, and -//! the guest agent bind-mounts each mapped path, read-only or read-write. Egress rules are expanded +//! Host paths share OpenVMM's single virtio-fs device: the backend exports each outermost mapped +//! directory, and the parent of each outermost mapped file, as one child of an aggregate that +//! only the guest's root can enter, and the guest agent bind-mounts each mapped path, read-only +//! or read-write. OpenVMM enforces each mapping's access on the host, except that of a read-only +//! path inside a read-write mapping, and hides denied paths and everything that is not mapped +//! beside mapped files. A whole volume, or a file directly in a volume's root, is not mapped. +//! Egress rules are expanded //! into OpenVMM's IPv4 and IPv6 rules exactly; rules that would need more than 256 OpenVMM rules //! are rejected. Egress denied without allow rules, with ingress denied, //! attaches no network device. Workloads run as the fixed non-root identity of @@ -539,6 +544,17 @@ impl Backend for OpenVmmBackend { }; launch::kernel_command_line(&self.config.kernel_command_line, &record)?; let sandbox_id = SandboxId::generate()?; + // Windows names each start's control pipe afresh, but every name has the same length. + let endpoint = + platform::control_endpoint(&self.store.socket_path(&sandbox_id)).map_err(|error| { + Error::backend_error("cannot choose a control endpoint").with_source(error) + })?; + launch::check_process_command_line( + &self.config, + &record, + &endpoint, + &self.store.outcome_path(&sandbox_id), + )?; self.store.create(&sandbox_id, &record)?; Ok(ProvisionResult { sandbox_id, @@ -646,10 +662,11 @@ impl Backend for OpenVmmBackend { .and_then(|transport| ControlSession::attach(transport, &capability, deadline)) .and_then(|mut session| { session.ping(deadline)?; - session.features(deadline) + let features = session.features(deadline)?; + Ok((session, features)) }); - let features = match ready { - Ok(features) => features, + let (mut session, features) = match ready { + Ok(ready) => ready, Err(error) => { let reason = readiness_failure(&error, record.create_workload_account); return Err(self.abort_start(sandbox_id, &runtime, &reason)); @@ -659,6 +676,13 @@ impl Backend for OpenVmmBackend { if !missing.is_empty() { return Err(self.reject_guest(sandbox_id, &runtime, &missing)); } + // Workloads run only once the guest has mounted every mapped host path. + if let Some(mapping) = &record.filesystem + && let Err(error) = session.map_host_paths(&filesystem::guest_table(mapping), deadline) + { + let reason = format!("the host paths were not mapped: {error}"); + return Err(self.abort_start(sandbox_id, &runtime, &reason)); + } let mut metadata = Metadata::new(); let boot_ms = u64::try_from(started.elapsed().as_millis()).unwrap_or(u64::MAX); metadata.insert("bootMilliseconds".to_owned(), boot_ms.into()); @@ -866,6 +890,11 @@ fn pump( protocol::LAUNCH_FAILED => Ok(ExecOutcome::Failed(ExecFailure::LaunchFailed)), // The guest already wrote a diagnostic that names the directory to stderr. protocol::CWD_FAILED => Ok(ExecOutcome::Failed(ExecFailure::WorkingDirectory)), + // Only a start that was interrupted leaves a guest without its mappings. + protocol::MAPPINGS_INCOMPLETE => Err(Error::backend_error( + "the guest has not mounted every mapped host path, because its start did \ + not complete; stop the sandbox and start it again", + )), _ => Err(Error::backend_error(format!( "the guest agent rejected the workload: {category} (status {status})" ))), diff --git a/aci_edge_sandboxes/src/openvmm/protocol.rs b/aci_edge_sandboxes/src/openvmm/protocol.rs index 8e118333..fb0e2dab 100644 --- a/aci_edge_sandboxes/src/openvmm/protocol.rs +++ b/aci_edge_sandboxes/src/openvmm/protocol.rs @@ -28,6 +28,7 @@ pub(crate) const APP_EXEC: u8 = 2; pub(crate) const APP_STOP: u8 = 3; pub(crate) const APP_CANCEL: u8 = 4; pub(crate) const APP_FEATURES: u8 = 5; +pub(crate) const APP_MAPS: u8 = 6; pub(crate) const APP_READY: u8 = 0x81; pub(crate) const APP_STDOUT: u8 = 0x82; pub(crate) const APP_STDERR: u8 = 0x83; @@ -42,6 +43,12 @@ pub(crate) const LAUNCH_FAILED: &str = "launch-failed"; /// Error category with which a guest agent reports a working directory that the workload cannot /// enter. The status is the guest's error number, and nothing of the workload ran. pub(crate) const CWD_FAILED: &str = "cwd-failed"; +/// Error category with which a guest agent refuses a workload until it has applied every host +/// mapping that the kernel command line announced. +pub(crate) const MAPPINGS_INCOMPLETE: &str = "mappings-incomplete"; +/// Error category with which a guest agent reports a host mapping that it could not apply. The +/// status is the guest's error number. +pub(crate) const MAPPING_FAILED: &str = "mapping-failed"; /// Length of the launch capability that authenticates the host client. pub(crate) const CAPABILITY_LEN: usize = 32; @@ -59,11 +66,18 @@ pub(crate) const MAX_ENVIRONMENT: usize = 256; pub(crate) const MAX_TIMEOUT_MS: u32 = u32::MAX; /// Largest combined stdout and stderr volume the guest agent forwards for one execution. pub(crate) const MAX_OUTPUT_BYTES: usize = 1024 * 1024; +/// Largest number of host mappings that the guest agent accepts. +pub(crate) const MAX_HOST_MAPPINGS: usize = 4096; +/// Largest source or target of one host mapping: Linux's `PATH_MAX` without the terminating NUL. +pub(crate) const MAX_MAP_PATH_BYTES: usize = 4095; const EXEC_EXTENDED: u16 = 1; const EXEC_CWD_PRESENT: u16 = 1 << 0; const EXEC_ENVIRONMENT_PRESENT: u16 = 1 << 1; const EXEC_INHERIT_DEFAULT_ENV: u16 = 1 << 2; +const MAPS_HEADER_LEN: usize = 8; +const MAP_ENTRY_HEADER_LEN: usize = 6; +const MAP_READ_ONLY: u16 = 1 << 0; /// Protocol violation detected while encoding or decoding. #[derive(Debug, Clone, PartialEq, Eq)] @@ -311,6 +325,82 @@ pub(crate) fn encode_exec_payload(workload: &Workload<'_>) -> Result, Pr Ok(payload) } +/// One bind mount of the host mapping table. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct HostMap { + /// Path of the mapped object relative to the guest's mount of the export, `/`-separated. + pub(crate) source: String, + /// Absolute guest path at which the guest agent mounts it. + pub(crate) target: String, + /// Whether the guest agent mounts it read-only. + pub(crate) read_only: bool, +} + +/// Encodes the host mapping table as the payloads of consecutive `APP_MAPS` requests, each at +/// most `limit` bytes long. +/// +/// A payload holds the index of its first entry and its number of entries, so the guest agent +/// applies the entries in order and only up to the number that the kernel command line +/// announced. Each entry holds its flags, the lengths of its source and target, and then both. +/// The guest agent requires 1 to [`MAX_HOST_MAPPINGS`] entries, a relative source and an +/// absolute target of at most [`MAX_MAP_PATH_BYTES`] without NUL bytes. +pub(crate) fn encode_maps_payloads( + maps: &[HostMap], + limit: usize, +) -> Result>, ProtocolError> { + if maps.is_empty() || maps.len() > MAX_HOST_MAPPINGS { + return Err(violation(format!( + "a host mapping table holds 1 to {MAX_HOST_MAPPINGS} entries" + ))); + } + let mut payloads = Vec::new(); + let mut payload = Vec::new(); + let mut count = 0u32; + for (index, map) in maps.iter().enumerate() { + let (source, target) = (map.source.as_bytes(), map.target.as_bytes()); + if source.is_empty() + || source.starts_with(b"/") + || !target.starts_with(b"/") + || source.len() > MAX_MAP_PATH_BYTES + || target.len() > MAX_MAP_PATH_BYTES + || source.contains(&0) + || target.contains(&0) + { + return Err(violation(format!( + "a host mapping needs a relative source and an absolute target of at most \ + {MAX_MAP_PATH_BYTES} bytes without NUL characters" + ))); + } + let entry_len = MAP_ENTRY_HEADER_LEN + source.len() + target.len(); + if MAPS_HEADER_LEN + entry_len > limit { + return Err(violation( + "a host mapping exceeds the control protocol limit", + )); + } + if count > 0 && payload.len() + entry_len > limit { + payload[4..8].copy_from_slice(&count.to_le_bytes()); + payloads.push(std::mem::take(&mut payload)); + count = 0; + } + if count == 0 { + let first = u32::try_from(index).unwrap_or(u32::MAX); + payload.extend_from_slice(&first.to_le_bytes()); + payload.extend_from_slice(&0u32.to_le_bytes()); + } + let flags = if map.read_only { MAP_READ_ONLY } else { 0 }; + let length = |bytes: &[u8]| u16::try_from(bytes.len()).unwrap_or(u16::MAX); + payload.extend_from_slice(&flags.to_le_bytes()); + payload.extend_from_slice(&length(source).to_le_bytes()); + payload.extend_from_slice(&length(target).to_le_bytes()); + payload.extend_from_slice(source); + payload.extend_from_slice(target); + count += 1; + } + payload[4..8].copy_from_slice(&count.to_le_bytes()); + payloads.push(payload); + Ok(payloads) +} + /// Control features of a guest image, advertised in the response to `APP_FEATURES`. /// /// The bits are defined by the guest agent in `guest/common/nvx-managed-agent.c`, which also @@ -325,8 +415,8 @@ impl GuestFeatures { pub(crate) const NONE: Self = Self(0); /// Terminates a workload on `APP_CANCEL` and reports the `cancelled` exit category. pub(crate) const CANCEL: Self = Self(1 << 0); - /// Bind-mounts the `nvx_map=` host paths and keeps the shared export root-only. - pub(crate) const HOST_MAPPINGS: Self = Self(1 << 1); + // Bit 1 announced the bind mounts that `nvx_map=` kernel command-line tokens listed. Guests + // no longer set it, so a host that still sends those tokens refuses them. /// Creates the account that `nvx_workload_account=create` selects. pub(crate) const WORKLOAD_ACCOUNT: Self = Self(1 << 2); /// Runs each workload in a cgroup of its own and kills whatever it leaves behind. @@ -338,23 +428,27 @@ impl GuestFeatures { /// entered with the workload's identity, and refuses the launch with the `cwd-failed` /// category when the workload cannot enter it. pub(crate) const EXEC_CWD: Self = Self(1 << 5); + /// Bind-mounts the host mapping table of `APP_MAPS` requests, as many entries as the + /// `nvx_maps=` kernel command-line token announces, keeps the shared export root-only, and + /// refuses workloads until every entry is mounted. + pub(crate) const HOST_MAPPING_TABLE: Self = Self(1 << 6); /// The features that the openvmm backend depends on. pub(crate) const REQUIRED: Self = Self( Self::CANCEL.0 - | Self::HOST_MAPPINGS.0 | Self::WORKLOAD_ACCOUNT.0 | Self::EXEC_CGROUP.0 | Self::EXEC_ENVIRONMENT.0 - | Self::EXEC_CWD.0, + | Self::EXEC_CWD.0 + | Self::HOST_MAPPING_TABLE.0, ); const NAMES: [(Self, &'static str); 6] = [ (Self::CANCEL, "cancellation"), - (Self::HOST_MAPPINGS, "host path mappings"), (Self::WORKLOAD_ACCOUNT, "workload accounts"), (Self::EXEC_CGROUP, "workload containment"), (Self::EXEC_ENVIRONMENT, "per-execution environments"), (Self::EXEC_CWD, "working directories"), + (Self::HOST_MAPPING_TABLE, "host path mapping tables"), ]; /// Decodes the payload of a features response. @@ -709,15 +803,20 @@ mod tests { assert_eq!( cancel_only.missing(required), [ - "host path mappings", "workload accounts", "workload containment", "per-execution environments", "working directories", + "host path mapping tables", ] ); assert!(GuestFeatures::decode(&[1, 0, 0]).is_err()); + // A guest of the previous release announced its host mappings with bit 1, which no + // longer counts. + let previous = GuestFeatures::decode(&0b11_1111u32.to_le_bytes()).unwrap(); + assert_eq!(previous.missing(required), ["host path mapping tables"]); + // Every required feature has a name, so a refusal can say what is missing. assert_eq!( GuestFeatures::NONE.missing(required).len(), @@ -727,7 +826,7 @@ mod tests { #[test] fn guest_features_use_distinct_bits() { - let mut seen = 0u32; + let mut seen = 1u32 << 1; for (feature, name) in GuestFeatures::NAMES { assert_eq!(feature.0.count_ones(), 1, "{name}"); assert_eq!(seen & feature.0, 0, "{name} reuses a feature bit"); @@ -735,6 +834,126 @@ mod tests { } } + fn map(source: &str, target: &str, read_only: bool) -> HostMap { + HostMap { + source: source.to_owned(), + target: target.to_owned(), + read_only, + } + } + + // Golden vector produced with Python's struct module: " = (0..10) + .map(|index| { + map( + &format!("0/{index:02}"), + &format!("/work/{index:02}"), + index % 2 == 0, + ) + }) + .collect(); + // Each entry takes 6 + 4 + 8 bytes, so a 64-byte payload holds 3 entries. + let payloads = encode_maps_payloads(&maps, 64).unwrap(); + assert_eq!(payloads.len(), 4); + let mut next = 0u32; + for payload in &payloads { + assert!(payload.len() <= 64); + assert_eq!(u32::from_le_bytes(array(&payload[..4])), next); + let count = u32::from_le_bytes(array(&payload[4..8])); + let mut offset = MAPS_HEADER_LEN; + for index in next..next + count { + let field = + |at: usize| usize::from(u16::from_le_bytes(array(&payload[at..at + 2]))); + let flags = field(offset); + let source_len = field(offset + 2); + let target_len = field(offset + 4); + offset += MAP_ENTRY_HEADER_LEN; + let expected = &maps[index as usize]; + assert_eq!(flags == usize::from(MAP_READ_ONLY), expected.read_only); + assert_eq!( + &payload[offset..offset + source_len], + expected.source.as_bytes() + ); + offset += source_len; + assert_eq!( + &payload[offset..offset + target_len], + expected.target.as_bytes() + ); + offset += target_len; + } + assert_eq!(offset, payload.len()); + next += count; + } + assert_eq!(next, 10); + + // The largest table fits the control protocol in a few requests. + let long = format!("/{}", "t".repeat(MAX_MAP_PATH_BYTES - 1)); + let largest: Vec = (0..MAX_HOST_MAPPINGS) + .map(|index| map(&format!("0/{index}"), &long, false)) + .collect(); + let payloads = encode_maps_payloads(&largest, OUTER_MAX_PAYLOAD - APP_HEADER_LEN).unwrap(); + assert!( + payloads + .iter() + .all(|payload| APP_HEADER_LEN + payload.len() <= OUTER_MAX_PAYLOAD) + ); + assert_eq!( + payloads + .iter() + .map(|payload| u32::from_le_bytes(array(&payload[4..8]))) + .sum::() as usize, + MAX_HOST_MAPPINGS + ); + } + + #[test] + fn maps_payloads_enforce_agent_limits() { + let limit = OUTER_MAX_PAYLOAD - APP_HEADER_LEN; + let too_long = format!("/{}", "t".repeat(MAX_MAP_PATH_BYTES)); + for invalid in [ + map("", "/w", false), + map("/0", "/w", false), + map("0", "w", false), + map("0", &too_long, false), + map("0\0", "/w", false), + map("0", "/w\0", false), + ] { + assert!( + encode_maps_payloads(std::slice::from_ref(&invalid), limit).is_err(), + "{invalid:?}" + ); + } + assert!(encode_maps_payloads(&[], limit).is_err()); + let too_many = vec![map("0", "/w", true); MAX_HOST_MAPPINGS + 1]; + assert!(encode_maps_payloads(&too_many, limit).is_err()); + // An entry that cannot fit any request is refused rather than split. + assert!(encode_maps_payloads(&[map("0", "/abcdefghijklmnop", true)], 20).is_err()); + } + fn guest_agent_source() -> Option { let source = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) .parent()? @@ -749,14 +968,17 @@ mod tests { let Some(source) = guest_agent_source() else { return; }; - let mut definitions = vec![format!("#define APP_FEATURES {APP_FEATURES}U")]; + let mut definitions = vec![ + format!("#define APP_FEATURES {APP_FEATURES}U"), + format!("#define APP_MAPS {APP_MAPS}U"), + ]; for (name, feature) in [ ("CANCEL", GuestFeatures::CANCEL), - ("HOST_MAPPINGS", GuestFeatures::HOST_MAPPINGS), ("WORKLOAD_ACCOUNT", GuestFeatures::WORKLOAD_ACCOUNT), ("EXEC_CGROUP", GuestFeatures::EXEC_CGROUP), ("EXEC_ENVIRONMENT", GuestFeatures::EXEC_ENVIRONMENT), ("EXEC_CWD", GuestFeatures::EXEC_CWD), + ("HOST_MAPPING_TABLE", GuestFeatures::HOST_MAPPING_TABLE), ] { let bit = feature.0.trailing_zeros(); definitions.push(format!("#define FEATURE_{name} (1U << {bit})")); @@ -767,6 +989,30 @@ mod tests { "the guest agent does not define `{definition}`" ); } + // The retired bit stays unused, so hosts that need it refuse the image. + assert!(!source.contains("(1U << 1)")); + } + + #[test] + fn host_mapping_table_matches_the_guest_agent_source() { + let Some(source) = guest_agent_source() else { + return; + }; + for expected in [ + format!("#define MAX_HOST_MAPPINGS {MAX_HOST_MAPPINGS}U"), + format!("#define MAX_GUEST_PATH {}U", MAX_MAP_PATH_BYTES + 1), + format!("#define MAPS_HEADER_LEN {MAPS_HEADER_LEN}U"), + format!("#define MAP_ENTRY_HEADER_LEN {MAP_ENTRY_HEADER_LEN}U"), + format!("#define MAP_READ_ONLY {MAP_READ_ONLY}U"), + "#define MAPS_TOKEN \"nvx_maps=\"".to_owned(), + format!("\"{MAPPINGS_INCOMPLETE}\""), + format!("\"{MAPPING_FAILED}\""), + ] { + assert!( + source.contains(&expected), + "the guest agent does not contain `{expected}`" + ); + } } #[test] diff --git a/aci_edge_sandboxes/src/openvmm/session.rs b/aci_edge_sandboxes/src/openvmm/session.rs index 5a0ed4ba..330c4698 100644 --- a/aci_edge_sandboxes/src/openvmm/session.rs +++ b/aci_edge_sandboxes/src/openvmm/session.rs @@ -7,10 +7,11 @@ use std::time::{Duration, Instant}; use super::platform::{self, Transport}; use super::protocol::{ - self, APP_CANCEL, APP_ERROR, APP_EXEC, APP_EXIT, APP_FEATURES, APP_PING, APP_READY, APP_STDERR, - APP_STDOUT, APP_STOP, APP_STOPPED, CAPABILITY_LEN, ExitCategory, GuestFeatures, OUTER_DATA, - OUTER_ERROR, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, OUTER_RESET, OUTER_WAIT, - OuterHeader, ProtocolError, UNSUPPORTED_OPERATION, Workload, + self, APP_CANCEL, APP_ERROR, APP_EXEC, APP_EXIT, APP_FEATURES, APP_HEADER_LEN, APP_MAPS, + APP_PING, APP_READY, APP_STDERR, APP_STDOUT, APP_STOP, APP_STOPPED, CAPABILITY_LEN, + ExitCategory, GuestFeatures, HostMap, OUTER_DATA, OUTER_ERROR, OUTER_HEADER_LEN, + OUTER_HOST_ATTACH, OUTER_MAX_PAYLOAD, OUTER_READY, OUTER_RESET, OUTER_WAIT, OuterHeader, + ProtocolError, UNSUPPORTED_OPERATION, Workload, }; /// Failure of a control session. @@ -204,6 +205,48 @@ impl ControlSession { } } + /// Hands the guest agent the host mapping table, in as many requests as it takes, and waits + /// until it has mounted every entry. + pub(crate) fn map_host_paths( + &mut self, + maps: &[HostMap], + deadline: Instant, + ) -> Result<(), SessionError> { + let payloads = protocol::encode_maps_payloads(maps, OUTER_MAX_PAYLOAD - APP_HEADER_LEN)?; + for payload in payloads { + let request_id = request_id()?; + self.send_app(APP_MAPS, request_id, &payload, Some(deadline))?; + let (kind, response_id, status, payload) = self.read_app(Some(deadline))?; + if response_id != request_id { + return Err(SessionError::Protocol( + "the guest agent returned a mismatched request ID".to_owned(), + )); + } + match kind { + APP_READY if status == 0 && payload.is_empty() => {} + APP_ERROR if payload == protocol::MAPPING_FAILED.as_bytes() => { + return Err(SessionError::Protocol(format!( + "the guest agent could not mount a mapped host path (error {status})" + ))); + } + APP_ERROR => { + let category: String = + String::from_utf8_lossy(&payload).chars().take(64).collect(); + return Err(SessionError::Protocol(format!( + "the guest agent did not mount the host paths: {category} (status \ + {status})" + ))); + } + _ => { + return Err(SessionError::Protocol( + "the guest agent returned an invalid host mapping response".to_owned(), + )); + } + } + } + Ok(()) + } + /// Starts a workload and returns the request ID that identifies its events. pub(crate) fn start_exec( &mut self, @@ -434,11 +477,16 @@ mod tests { Instant::now() + Duration::from_secs(5) } - /// Transport of a guest that answers each application request with `answer(kind, request_id)`. + /// Kind and payload of each application request that a guest received. + type Requests = Arc)>>>; + + /// Transport of a guest that answers each application request with `answer(kind, request_id)` + /// and records the kind and payload of each request. struct Responder { input: VecDeque, next_sequence: u64, answer: fn(u8, u64) -> Vec, + requests: Requests, } impl Transport for Responder { @@ -457,6 +505,10 @@ mod tests { let header = decode_outer_header(data[..OUTER_HEADER_LEN].try_into().unwrap()).unwrap(); if header.record_type == OUTER_DATA { let request = decode_app(&data[OUTER_HEADER_LEN..]).unwrap(); + self.requests + .lock() + .unwrap() + .push((request.kind, request.payload.to_vec())); let reply = (self.answer)(request.kind, request.request_id); self.input .extend(outer(OUTER_DATA, 1, self.next_sequence, &reply)); @@ -468,12 +520,22 @@ mod tests { /// Attaches to a guest that answers each application request with `answer`. fn attached(answer: fn(u8, u64) -> Vec) -> ControlSession { + attached_recording(answer).0 + } + + /// Attaches to a guest that answers each application request with `answer`, and returns the + /// kind and payload of each request that the guest receives. + fn attached_recording(answer: fn(u8, u64) -> Vec) -> (ControlSession, Requests) { + let requests = Arc::new(Mutex::new(Vec::new())); let guest = Responder { input: outer(OUTER_READY, 1, 0, &[]).into(), next_sequence: 1, answer, + requests: Arc::clone(&requests), }; - ControlSession::attach(Box::new(guest), &[1; CAPABILITY_LEN], deadline()).unwrap() + let session = + ControlSession::attach(Box::new(guest), &[1; CAPABILITY_LEN], deadline()).unwrap(); + (session, requests) } /// Splits the host's writes into (record type, sequence, application frame) tuples. @@ -711,6 +773,65 @@ mod tests { } } + #[test] + fn host_mappings_travel_in_records_that_the_guest_acknowledges() { + let (mut session, requests) = attached_recording(|kind, id| { + assert_eq!(kind, APP_MAPS); + encode_app(APP_READY, id, 0, &[]) + }); + let target = format!("/{}", "t".repeat(4000)); + let maps: Vec = (0..40) + .map(|index| HostMap { + source: format!("0/{index}"), + target: target.clone(), + read_only: index % 2 == 0, + }) + .collect(); + session.map_host_paths(&maps, deadline()).unwrap(); + // Entries of about 4 KiB each fill a 64 KiB record 16 at a time. + let requests = requests.lock().unwrap(); + let positions: Vec<(u32, u32)> = requests + .iter() + .map(|(_, payload)| { + ( + u32::from_le_bytes(payload[..4].try_into().unwrap()), + u32::from_le_bytes(payload[4..8].try_into().unwrap()), + ) + }) + .collect(); + assert_eq!(positions, [(0, 16), (16, 16), (32, 8)]); + } + + #[test] + fn refused_host_mappings_fail_the_session() { + let maps = [HostMap { + source: "0".to_owned(), + target: "/work".to_owned(), + read_only: false, + }]; + let mut session = attached(|_, id| encode_app(APP_ERROR, id, 2, b"mapping-failed")); + let error = session.map_host_paths(&maps, deadline()).unwrap_err(); + assert!( + error + .to_string() + .contains("could not mount a mapped host path (error 2)"), + "{error}" + ); + let answers: [fn(u8, u64) -> Vec; 4] = [ + |_, id| encode_app(APP_ERROR, id, 22, b"invalid-request"), + |_, id| encode_app(APP_ERROR, id, 95, b"unsupported-operation"), + |_, id| encode_app(APP_READY, id, 0, b"x"), + |_, id| encode_app(APP_READY, id ^ 1, 0, &[]), + ]; + for answer in answers { + let mut session = attached(answer); + assert!(matches!( + session.map_host_paths(&maps, deadline()), + Err(SessionError::Protocol(_)) + )); + } + } + #[test] fn expired_deadlines_time_out() { let (transport, _) = scripted(&[]); diff --git a/aci_edge_sandboxes/src/openvmm/state.rs b/aci_edge_sandboxes/src/openvmm/state.rs index 9af22f85..e57f68f3 100644 --- a/aci_edge_sandboxes/src/openvmm/state.rs +++ b/aci_edge_sandboxes/src/openvmm/state.rs @@ -29,8 +29,10 @@ use crate::model::NetworkPolicy; /// Version of the state files written by this crate. /// -/// Version 1 described sandboxes assembled from image layers and a scratch disk. -pub(crate) const STATE_FORMAT: u32 = 2; +/// Version 1 described sandboxes assembled from image layers and a scratch disk, and version 2 +/// exported the mapped host paths as one directory whose bind mounts the kernel command line +/// listed. +pub(crate) const STATE_FORMAT: u32 = 3; /// Backend key recorded in every sandbox. pub(crate) const BACKEND_KEY: &str = "openvmm"; /// Linux control socket name. @@ -201,17 +203,23 @@ impl StateStore { /// Loads the configuration of a provisioned sandbox. pub(crate) fn load(&self, sandbox_id: &SandboxId) -> Result { let path = self.dir(sandbox_id).join(RECORD_NAME); - let record: SandboxRecord = match read_json(&path)? { + // Records of other formats have other fields, so their format is checked first. + let record: serde_json::Value = match read_json(&path)? { Some(record) => record, None => return Err(not_provisioned(sandbox_id)), }; - if record.format != STATE_FORMAT || record.backend != BACKEND_KEY { + if record.get("format").and_then(serde_json::Value::as_u64) != Some(STATE_FORMAT.into()) + || record.get("backend").and_then(serde_json::Value::as_str) != Some(BACKEND_KEY) + { return Err(Error::backend_error(format!( "sandbox state {} has an unsupported format", path.display() ))); } - Ok(record) + serde_json::from_value(record).map_err(|error| { + Error::backend_error(format!("sandbox state {} is malformed", path.display())) + .with_source(error) + }) } pub(crate) fn runtime(&self, sandbox_id: &SandboxId) -> Result> { @@ -516,14 +524,30 @@ mod tests { store.create(&id, &record()).unwrap(); fs::write(store.dir(&id).join(RECORD_NAME), b"{ truncated").unwrap(); assert_eq!(store.load(&id).unwrap_err().code(), ErrorCode::BackendError); + let unsupported = |record: &serde_json::Value| { + fs::write( + store.dir(&id).join(RECORD_NAME), + serde_json::to_vec(record).unwrap(), + ) + .unwrap(); + let error = store.load(&id).unwrap_err(); + assert_eq!(error.code(), ErrorCode::BackendError); + assert!(error.message().contains("unsupported format"), "{error}"); + }; let mut legacy = serde_json::to_value(record()).unwrap(); legacy["format"] = 1.into(); - fs::write( - store.dir(&id).join(RECORD_NAME), - serde_json::to_vec(&legacy).unwrap(), - ) - .unwrap(); - assert_eq!(store.load(&id).unwrap_err().code(), ErrorCode::BackendError); + unsupported(&legacy); + // Version 2 exported one directory and listed its bind mounts on the kernel command + // line. + legacy["format"] = 2.into(); + legacy["filesystem"] = serde_json::json!({ + "root": "/host/work", + "writable": false, + "denied": [], + "deniedIdentities": [], + "binds": [{ "source": "", "target": "/host/work", "readOnly": true }], + }); + unsupported(&legacy); } #[test] diff --git a/aci_edge_sandboxes/tests/openvmm_e2e.rs b/aci_edge_sandboxes/tests/openvmm_e2e.rs index 90449abc..ddf8c23c 100644 --- a/aci_edge_sandboxes/tests/openvmm_e2e.rs +++ b/aci_edge_sandboxes/tests/openvmm_e2e.rs @@ -218,6 +218,225 @@ fn host_paths_are_mapped_into_the_guest() { ); } +/// A directory on another volume than the temporary directory, if the host has one: the one +/// that `ACI_EDGE_SANDBOXES_E2E_SECOND_VOLUME` names, or the runner's temporary directory, which +/// lies on another volume on GitHub's Windows runners. Tests create their files below it. +fn second_volume() -> Option { + let candidate = env::var_os("ACI_EDGE_SANDBOXES_E2E_SECOND_VOLUME") + .or_else(|| env::var_os("RUNNER_TEMP")) + .map(PathBuf::from)?; + let candidate = fs::canonicalize(candidate).ok()?; + let temporary = fs::canonicalize(env::temp_dir()).ok()?; + #[cfg(unix)] + let other = { + use std::os::unix::fs::MetadataExt; + fs::metadata(&candidate).ok()?.dev() != fs::metadata(&temporary).ok()?.dev() + }; + #[cfg(not(unix))] + let other = candidate.components().next() != temporary.components().next(); + (other && candidate.is_dir()).then_some(candidate) +} + +/// Runs `script` with `paths` as its positional parameters. +fn script(script: &str, paths: &[String]) -> ExecRequest { + let mut argv = vec!["/bin/sh".to_owned(), "-c".to_owned(), script.to_owned()]; + argv.push("sh".to_owned()); + argv.extend(paths.iter().cloned()); + ExecRequest::argv(argv) +} + +#[test] +#[ignore = "requires a hypervisor and NVX guest artifacts; run scripts/nvx.py test-aci-edge-sandboxes"] +fn unrelated_directories_keep_their_own_access() { + // The layout of microsoft/nvx#281: projects/app and build-output are read-write, tools is + // read-only, and nothing else of the host is exported. + let host = tempfile::tempdir().unwrap(); + let base = host.path(); + for directory in ["projects/app", "projects/private", "build-output", "tools"] { + fs::create_dir_all(base.join(directory)).unwrap(); + } + fs::write(base.join("tools").join("lint.cfg"), "rules").unwrap(); + fs::write(base.join("projects").join("private").join("key"), "secret").unwrap(); + let request = ProvisionRequest::new().with_filesystem(FilesystemPolicy { + readonly_paths: vec![base.join("tools")], + readwrite_paths: vec![base.join("projects/app"), base.join("build-output")], + denied_paths: Vec::new(), + }); + let (nvx, backend) = client("volumes"); + let sandbox = started(&nvx, &backend, &request); + let sandbox_id = id(&sandbox); + let guest = |name: &str| guest_path(&base.join(name)).unwrap(); + let paths = [ + guest("projects/app"), + guest("build-output"), + guest("tools"), + guest("projects"), + ]; + let output = run( + &nvx, + sandbox_id, + script( + "echo app > \"$1/result\" && echo build > \"$2/result\" && cat \"$3/lint.cfg\" \ + && ! touch \"$3/new\" 2>/dev/null && ls \"$4\"", + &paths, + ), + ); + assert!(output.outcome.success(), "{output:?}"); + // The parent of projects/app holds only its mount point. + assert_eq!(output.stdout, b"rulesapp\n", "{output:?}"); + assert_eq!( + fs::read_to_string(base.join("projects/app/result")).unwrap(), + "app\n" + ); + assert_eq!( + fs::read_to_string(base.join("build-output/result")).unwrap(), + "build\n" + ); + assert!(!base.join("tools").join("new").exists()); +} + +#[test] +#[ignore = "requires a hypervisor and NVX guest artifacts; run scripts/nvx.py test-aci-edge-sandboxes"] +fn mapped_files_hide_their_siblings_and_names_may_hold_spaces() { + let host = tempfile::tempdir().unwrap(); + let work = host.path().join("my work"); + fs::create_dir_all(work.join("data").join("My Secrets")).unwrap(); + fs::write(work.join("notes one.txt"), "notes").unwrap(); + fs::write(work.join("log file.txt"), "").unwrap(); + fs::write(work.join("sibling.txt"), "hidden").unwrap(); + fs::write(work.join("data").join("My Secrets").join("key"), "hidden").unwrap(); + fs::write(work.join("data").join("shared.txt"), "shared").unwrap(); + let request = ProvisionRequest::new().with_filesystem(FilesystemPolicy { + readonly_paths: vec![work.join("notes one.txt")], + readwrite_paths: vec![work.join("log file.txt"), work.join("data")], + denied_paths: vec![work.join("data").join("My Secrets")], + }); + let (nvx, backend) = client("files"); + let sandbox = started(&nvx, &backend, &request); + let sandbox_id = id(&sandbox); + let guest = |name: &str| guest_path(&work.join(name)).unwrap(); + let directory = guest_path(&work).unwrap(); + let output = run( + &nvx, + sandbox_id, + script( + "cat \"$1\" && echo logged >> \"$2\" && ! echo x > \"$1\" 2>/dev/null && ls \"$3\" \ + && ls \"$4\"", + &[ + guest("notes one.txt"), + guest("log file.txt"), + directory.clone(), + guest("data"), + ], + ), + ); + assert!(output.outcome.success(), "{output:?}"); + // The guest's directory holds only the mapped paths, and the denied directory is hidden. + assert_eq!( + output.stdout, b"notesdata\nlog file.txt\nnotes one.txt\nshared.txt\n", + "{output:?}" + ); + assert_eq!( + fs::read_to_string(work.join("log file.txt")).unwrap(), + "logged\n" + ); + for hidden in [ + format!("{directory}/sibling.txt"), + format!("{directory}/data/My Secrets/key"), + ] { + let output = run( + &nvx, + sandbox_id, + script("cat \"$1\"", std::slice::from_ref(&hidden)), + ); + assert_ne!( + output.outcome, + ExecOutcome::Exited(0), + "{hidden}: {output:?}" + ); + } +} + +#[test] +#[ignore = "requires a hypervisor and NVX guest artifacts; run scripts/nvx.py test-aci-edge-sandboxes"] +fn many_mappings_are_mounted() { + let host = tempfile::tempdir().unwrap(); + let mut readonly_paths = Vec::new(); + let mut readwrite_paths = Vec::new(); + for index in 0..25 { + let directory = host.path().join(format!("tree-{index:02}")).join("src"); + fs::create_dir_all(&directory).unwrap(); + fs::write(directory.join("file.txt"), format!("{index}")).unwrap(); + let file = host.path().join(format!("file-{index:02}.txt")); + fs::write(&file, format!("{index}")).unwrap(); + if index % 2 == 0 { + readonly_paths.push(directory); + readwrite_paths.push(file); + } else { + readwrite_paths.push(directory); + readonly_paths.push(file); + } + } + let mapped: Vec = readonly_paths + .iter() + .chain(&readwrite_paths) + .map(|path| guest_path(path).unwrap()) + .collect(); + assert_eq!(mapped.len(), 50); + let request = ProvisionRequest::new().with_filesystem(FilesystemPolicy { + readonly_paths, + readwrite_paths, + denied_paths: Vec::new(), + }); + let (nvx, backend) = client("many"); + let sandbox = started(&nvx, &backend, &request); + let output = run( + &nvx, + id(&sandbox), + script( + "count=0; for path in \"$@\"; do [ -e \"$path\" ] || exit 1; \ + count=$((count + 1)); done; echo $count", + &mapped, + ), + ); + assert_eq!(output.stdout, b"50\n", "{output:?}"); +} + +#[test] +#[ignore = "requires a hypervisor and NVX guest artifacts; run scripts/nvx.py test-aci-edge-sandboxes"] +fn directories_on_another_volume_are_mapped() { + let Some(volume) = second_volume() else { + eprintln!("skipped: the host has no second volume"); + return; + }; + let primary = tempfile::tempdir().unwrap(); + let secondary = tempfile::tempdir_in(volume).unwrap(); + fs::write(primary.path().join("input.txt"), "input").unwrap(); + let request = ProvisionRequest::new().with_filesystem(FilesystemPolicy { + readonly_paths: vec![primary.path().to_path_buf()], + readwrite_paths: vec![secondary.path().to_path_buf()], + denied_paths: Vec::new(), + }); + let (nvx, backend) = client("volume"); + let sandbox = started(&nvx, &backend, &request); + let output = run( + &nvx, + id(&sandbox), + script( + "cat \"$1/input.txt\" > \"$2/output.txt\"", + &[ + guest_path(primary.path()).unwrap(), + guest_path(secondary.path()).unwrap(), + ], + ), + ); + assert!(output.outcome.success(), "{output:?}"); + assert_eq!( + fs::read_to_string(secondary.path().join("output.txt")).unwrap(), + "input" + ); +} + /// Runs the shell's `pwd`, which reports `PWD`, then `/bin/pwd`, which resolves the directory. fn pwd(nvx: &AciEdgeSandbox, sandbox_id: &SandboxId, cwd: Option<&str>) -> ExecOutput { let request = ExecRequest::command_line("pwd; /bin/pwd"); diff --git a/aci_edge_sandboxes/tests/openvmm_fake.rs b/aci_edge_sandboxes/tests/openvmm_fake.rs index d99f7b96..f138e648 100644 --- a/aci_edge_sandboxes/tests/openvmm_fake.rs +++ b/aci_edge_sandboxes/tests/openvmm_fake.rs @@ -146,6 +146,15 @@ impl Fixture { serde_json::from_slice(&fs::read(path).unwrap()).unwrap() } + /// The host mapping table that the guest received, in order. + fn mapping_table(&self, sandbox_id: &SandboxId) -> Vec { + let path = self + .directory + .path() + .join(format!("fake-openvmm-{}-maps.json", sandbox_id.token())); + serde_json::from_slice(&fs::read(path).unwrap()).unwrap() + } + fn sandbox_dirs(&self) -> usize { fs::read_dir(&self.state_root) .unwrap() @@ -1059,11 +1068,11 @@ fn guests_without_the_required_features_are_refused() { assert_eq!(error.code(), ErrorCode::BackendUnavailable, "{error}"); for feature in [ "cancellation", - "host path mappings", "workload accounts", "workload containment", "per-execution environments", "working directories", + "host path mapping tables", ] { assert!(error.message().contains(feature), "{error}"); } @@ -1087,7 +1096,7 @@ fn guests_without_the_required_features_are_refused() { // This guest agent predates working directories, so it would start workloads elsewhere. It // provides every other required feature. let without_cwd = fixture.nvx_with(|config| { - config.kernel_command_line = "fake_guest_features=31".to_owned(); + config.kernel_command_line = "fake_guest_features=95".to_owned(); }); let error = without_cwd.start(&sandbox_id).unwrap_err(); assert_eq!(error.code(), ErrorCode::BackendUnavailable, "{error}"); @@ -1098,6 +1107,20 @@ fn guests_without_the_required_features_are_refused() { "{error}" ); + // The previous release's guest agent read its bind mounts from the kernel command line, so + // it would ignore the mapping table. + let previous = fixture.nvx_with(|config| { + config.kernel_command_line = "fake_guest_features=63".to_owned(); + }); + let error = previous.start(&sandbox_id).unwrap_err(); + assert_eq!(error.code(), ErrorCode::BackendUnavailable, "{error}"); + assert!( + error + .message() + .contains("the openvmm backend needs (host path mapping tables)"), + "{error}" + ); + let current = fixture.nvx(); current.start(&sandbox_id).unwrap(); assert!( @@ -1390,10 +1413,18 @@ fn filesystem_and_network_policies_reach_openvmm() { .map(|pair| pair[1].clone()) .collect::>() }; - let mounts = value("--mount"); - assert_eq!(mounts.len(), 1); - assert!(mounts[0].starts_with("/run/nvx/hostfs/root,"), "{mounts:?}"); - assert!(mounts[0].ends_with(",rw"), "{mounts:?}"); + let mounts = value("--mount-child"); + assert_eq!(mounts.len(), 2, "{mounts:?}"); + assert!( + mounts[0].starts_with("0,") && mounts[0].ends_with(",rw"), + "{mounts:?}" + ); + assert!( + mounts[1].starts_with("1,") && mounts[1].ends_with(",ro"), + "{mounts:?}" + ); + assert_eq!(value("--mount-aggregate"), ["/run/nvx/hostfs/root"]); + assert!(value("--mount").is_empty()); assert_eq!(value("--mount-deny").len(), 1); assert_eq!(value("--network-egress"), ["deny"]); assert_eq!( @@ -1413,14 +1444,24 @@ fn filesystem_and_network_policies_reach_openvmm() { "2001:db8::1/128" ] ); + // Only the number of mappings travels on the kernel command line. let command_line = &value("--cmdline")[0]; - let maps: Vec<&str> = command_line - .split(' ') - .filter(|token| token.starts_with("nvx_map=")) + assert!( + command_line.split(' ').any(|token| token == "nvx_maps=2"), + "{command_line}" + ); + assert!(!command_line.contains("nvx_map="), "{command_line}"); + let table = fixture.mapping_table(&sandbox_id); + let entries: Vec<(&str, bool)> = table + .iter() + .map(|entry| { + ( + entry["source"].as_str().unwrap(), + entry["read_only"].as_bool().unwrap(), + ) + }) .collect(); - assert_eq!(maps.len(), 2, "{command_line}"); - assert!(maps.iter().any(|token| token.ends_with(",ro"))); - assert!(maps.iter().any(|token| token.ends_with(",rw"))); + assert_eq!(entries, [("0", false), ("1", true)]); let guest = aci_edge_sandboxes::openvmm::resolve_guest_path(&base.join("work").join("out")).unwrap(); @@ -1429,7 +1470,7 @@ fn filesystem_and_network_policies_reach_openvmm() { .as_array() .unwrap() .iter() - .any(|bind| bind["source"] == "out" && bind["target"] == guest), + .any(|bind| bind["child"] == 0 && bind["source"] == "" && bind["target"] == guest), "the working directory must match the canonical mapped guest path" ); let pwd = nvx @@ -1497,6 +1538,189 @@ fn mapped_files_are_not_working_directories() { nvx.deprovision(&sandbox_id).unwrap(); } +#[test] +fn unrelated_directories_are_exported_side_by_side() { + let fixture = Fixture::new(); + let nvx = fixture.nvx(); + let base = fixture.directory.path(); + for name in ["projects/app", "projects/private", "build-output", "tools"] { + fs::create_dir_all(base.join(name)).unwrap(); + } + fs::write(base.join("tools").join("lint.cfg"), b"rules").unwrap(); + // The layout of microsoft/nvx#281: the paths share no directory that could be exported + // alone without the unmapped projects/private. + let request = ProvisionRequest::new().with_filesystem(FilesystemPolicy { + readonly_paths: vec![base.join("tools")], + readwrite_paths: vec![base.join("projects/app"), base.join("build-output")], + denied_paths: Vec::new(), + }); + let sandbox_id = nvx.provision(&request).unwrap().sandbox_id; + nvx.start(&sandbox_id).unwrap(); + let arguments = fixture.launch_arguments(&sandbox_id); + let children: Vec<&String> = arguments + .windows(2) + .filter(|pair| pair[0] == "--mount-child") + .map(|pair| &pair[1]) + .collect(); + assert_eq!(children.len(), 3, "{arguments:?}"); + let modes: Vec<&str> = children + .iter() + .map(|child| child.rsplit(',').next().unwrap()) + .collect(); + assert_eq!(modes, ["rw", "rw", "ro"]); + // Each mapped directory is a child of its own; their parent stays unexported. + for (child, name) in children + .iter() + .zip(["build-output", "projects/app", "tools"]) + { + let (_, rest) = child.split_once(',').unwrap(); + let (host, _) = rest.rsplit_once(',').unwrap(); + assert!(Path::new(host).ends_with(name), "{child}"); + } + for name in ["projects/app", "build-output", "tools"] { + let guest = aci_edge_sandboxes::openvmm::resolve_guest_path(&base.join(name)).unwrap(); + let pwd = nvx + .exec( + &sandbox_id, + &ExecRequest::command_line("pwd").with_cwd(guest.clone()), + ) + .unwrap() + .wait_with_output() + .unwrap(); + assert_eq!(pwd.stdout, format!("{guest}\n").into_bytes(), "{name}"); + } + nvx.stop(&sandbox_id).unwrap(); + nvx.deprovision(&sandbox_id).unwrap(); +} + +#[test] +fn large_mapping_tables_reach_the_guest_in_several_records() { + let fixture = Fixture::new(); + let nvx = fixture.nvx(); + let work = fixture.directory.path().join("work"); + // Read-only directories inside a read-write one need no OpenVMM options of their own, so + // the table outgrows one 64 KiB control record long before the command line fills up. + let names: Vec = (0..800) + .map(|index| format!("{}-{index:04}", "directory".repeat(6))) + .collect(); + for name in &names { + fs::create_dir_all(work.join(name)).unwrap(); + } + let request = ProvisionRequest::new().with_filesystem(FilesystemPolicy { + readonly_paths: names.iter().map(|name| work.join(name)).collect(), + readwrite_paths: vec![work.clone()], + denied_paths: Vec::new(), + }); + let sandbox_id = nvx.provision(&request).unwrap().sandbox_id; + nvx.start(&sandbox_id).unwrap(); + let command_line = fixture + .launch_arguments(&sandbox_id) + .windows(2) + .find(|pair| pair[0] == "--cmdline") + .map(|pair| pair[1].clone()) + .unwrap(); + assert!(command_line.contains("nvx_maps=801"), "{command_line}"); + let table = fixture.mapping_table(&sandbox_id); + assert_eq!(table.len(), 801); + let bytes: usize = table + .iter() + .map(|entry| { + 6 + entry["source"].as_str().unwrap().len() + entry["target"].as_str().unwrap().len() + }) + .sum(); + assert!(bytes > 64 * 1024, "{bytes}"); + assert_eq!(table[0]["source"], "0"); + assert!( + table[1..] + .iter() + .all(|entry| entry["read_only"] == true && entry["directory"] == true) + ); + let last = aci_edge_sandboxes::openvmm::resolve_guest_path(&work.join(&names[799])).unwrap(); + let pwd = nvx + .exec( + &sandbox_id, + &ExecRequest::command_line("pwd").with_cwd(last.clone()), + ) + .unwrap() + .wait_with_output() + .unwrap(); + assert_eq!(pwd.stdout, format!("{last}\n").into_bytes()); + nvx.stop(&sandbox_id).unwrap(); + nvx.deprovision(&sandbox_id).unwrap(); +} + +#[test] +fn starts_fail_when_the_guest_cannot_mount_the_mappings() { + let fixture = Fixture::new(); + let work = fixture.directory.path().join("work"); + fs::create_dir_all(&work).unwrap(); + let nvx = fixture.nvx_with(|config| { + config.kernel_command_line = "fake_refuse_maps=1".to_owned(); + }); + let request = ProvisionRequest::new().with_filesystem(FilesystemPolicy { + readwrite_paths: vec![work], + ..FilesystemPolicy::default() + }); + let sandbox_id = nvx.provision(&request).unwrap().sandbox_id; + let error = nvx.start(&sandbox_id).unwrap_err(); + assert_eq!(error.code(), ErrorCode::BackendError, "{error}"); + assert!( + error + .message() + .contains("could not mount a mapped host path (error 13)"), + "{error}" + ); + // The VM was terminated and forgotten, so the sandbox is merely provisioned. + assert!( + !fixture + .state_root + .join(sandbox_id.token()) + .join("runtime.json") + .exists() + ); + assert_eq!( + nvx.exec(&sandbox_id, &ExecRequest::command_line("echo")) + .unwrap_err() + .code(), + ErrorCode::NotStarted + ); + nvx.deprovision(&sandbox_id).unwrap(); +} + +#[test] +fn sandboxes_of_the_previous_state_format_are_refused() { + let fixture = Fixture::new(); + let nvx = fixture.nvx(); + let work = fixture.directory.path().join("work"); + fs::create_dir_all(&work).unwrap(); + let request = ProvisionRequest::new().with_filesystem(FilesystemPolicy { + readwrite_paths: vec![work.clone()], + ..FilesystemPolicy::default() + }); + let sandbox_id = nvx.provision(&request).unwrap().sandbox_id; + // Version 2 exported one directory and listed its bind mounts on the kernel command line. + let mut record = state_json(&fixture, &sandbox_id, "sandbox.json"); + record["format"] = 2.into(); + record["filesystem"] = serde_json::json!({ + "root": work, + "writable": true, + "denied": [], + "deniedIdentities": [], + "binds": [{ "source": "", "target": "/work", "readOnly": false }], + }); + fs::write( + fixture + .state_root + .join(sandbox_id.token()) + .join("sandbox.json"), + serde_json::to_vec(&record).unwrap(), + ) + .unwrap(); + let error = nvx.start(&sandbox_id).unwrap_err(); + assert_eq!(error.code(), ErrorCode::BackendError, "{error}"); + assert!(error.message().contains("unsupported format"), "{error}"); +} + #[test] fn concurrent_execs_are_serialized() { let fixture = Fixture::new(); diff --git a/doc/design/sandbox-filesystem-and-agent-architecture.md b/doc/design/sandbox-filesystem-and-agent-architecture.md index 8b7b5fb8..8b04ee6d 100644 --- a/doc/design/sandbox-filesystem-and-agent-architecture.md +++ b/doc/design/sandbox-filesystem-and-agent-architecture.md @@ -375,12 +375,29 @@ standard error and refuses the request with the `cwd-failed` category and the error number as status, so nothing runs in another directory. Without sandbox layers, the agent maps host directories for workloads from -one virtio-fs export. The host exports the deepest directory that contains -every mapped path to `/run/nvx/hostfs/root`; before it accepts control traffic, -the agent makes `/run/nvx/hostfs` root-only and bind-mounts each mapped path -named by an `nvx_map=SOURCE,TARGET,ro|rw` kernel token (percent-encoded paths, -`SOURCE` relative to the export), remounting each bind `nosuid,nodev` and, for -`ro`, read-only. OpenVMM's export deny list hides denied paths. A +one aggregate virtio-fs export. The host exports the outermost mapped +directories and the parents of the outermost mapped files as numbered children +of the export, which the init mounts at `/run/nvx/hostfs/root`, and enforces +each mapping's access itself: a child is read-write only if it holds a +read-write mapping, OpenVMM limits writes to its read-write mappings, and the +parent of mapped files hides everything else. Neither a whole volume nor a file +directly in a volume's root, whose parent would be that root, is mapped. Before +it accepts control +traffic, the agent makes `/run/nvx/hostfs` root-only. The kernel command line +announces the number of mappings with an `nvx_maps=COUNT` token, and the host +then sends the mapping table in `MAPS` requests. Each request carries the index +of its first entry and its number of entries, and each entry its flags (bit 0 +selects read-only), the lengths of its source and target, the source, a path +relative to the export such as `0/src`, and the absolute target. The agent +accepts entries only in order and only up to the announced count, checks a +whole request before it mounts anything, bind-mounts each source at its +target, remounting the bind `nosuid,nodev` and, for a read-only entry, +read-only, and answers `READY`, or `ERROR` with the `mapping-failed` category +and the error number. Until it has mounted every announced entry, it refuses +`EXEC` with the `mappings-incomplete` category, and a failed mount keeps it +refusing. Without an `nvx_maps=` token, it refuses `MAPS` as +`invalid-request`, and it ignores the `nvx_map=` tokens of older hosts. +OpenVMM's deny list hides denied paths. A `nvx_workload_account=create` token lets the managed init create an account for a host-selected non-root identity that the image lacks, which Linux hosts use to run workloads under the host user's IDs. An image account that already uses @@ -392,17 +409,20 @@ more for it, and acknowledges the new epoch, so the next session does not wait for an abandoned workload. A host cannot tell from a successful readiness probe whether the image enforces -the behaviors it depends on: an older guest boots and answers, but ignores -`nvx_map=` tokens and `CANCEL` requests. A `FEATURES` request, which carries no +the behaviors it depends on: an older guest boots and answers, but ignores the +mapping table and `CANCEL` requests. A `FEATURES` request, which carries no payload, therefore asks which control behaviors the image provides. The agent answers `READY` with a four-byte little-endian bit mask: `CANCEL` (bit 0), -`HOST_MAPPINGS` (bit 1), `WORKLOAD_ACCOUNT` (bit 2, provided by the managed -init and reported by the agent, because both ship in one initramfs), -`EXEC_CGROUP` (bit 3), `EXEC_ENVIRONMENT` (bit 4, which applies an explicit -environment to each execution, either replacing or layering over the bootstrap -environment), and `EXEC_CWD` (bit 5, the `cwd-failed` refusal of a working -directory that the workload cannot enter). Without sandbox layers all six are -provided; with them, only `CANCEL`, `WORKLOAD_ACCOUNT`, and `EXEC_ENVIRONMENT`. +`WORKLOAD_ACCOUNT` (bit 2, provided by the managed init and reported by the +agent, because both ship in one initramfs), `EXEC_CGROUP` (bit 3), +`EXEC_ENVIRONMENT` (bit 4, which applies an explicit environment to each +execution, either replacing or layering over the bootstrap environment), +`EXEC_CWD` (bit 5, the `cwd-failed` refusal of a working directory that the +workload cannot enter), and `HOST_MAPPING_TABLE` (bit 6, the `MAPS` requests +above). Bit 1 announced the bind mounts that `nvx_map=` kernel tokens listed in +earlier images; it stays unset, so hosts that still send those tokens refuse +current images. Without sandbox layers all six features are provided; with +them, only `CANCEL`, `WORKLOAD_ACCOUNT`, and `EXEC_ENVIRONMENT`. An agent that predates the request refuses it as `unsupported-operation`, which a host reads as no features, so a host terminates a guest that lacks a feature it needs instead of running workloads without the policy it asked for. During diff --git a/guest/common/nvx-managed-agent.c b/guest/common/nvx-managed-agent.c index 57108796..ac3b1e2b 100644 --- a/guest/common/nvx-managed-agent.c +++ b/guest/common/nvx-managed-agent.c @@ -35,6 +35,7 @@ #define APP_STOP 3U #define APP_CANCEL 4U #define APP_FEATURES 5U +#define APP_MAPS 6U #define APP_READY 0x81U #define APP_STDOUT 0x82U #define APP_STDERR 0x83U @@ -49,14 +50,16 @@ * (aci_edge_sandboxes, src/openvmm/protocol.rs). The init script of the image provides * FEATURE_WORKLOAD_ACCOUNT; the agent speaks for it because both ship in one * initramfs. An agent that predates FEATURES refuses the request as an - * unsupported operation, which the host reads as no features. + * unsupported operation, which the host reads as no features. Bit 1 announced + * bind mounts that nvx_map= kernel command-line tokens listed; it stays unused, + * so hosts that still send those tokens refuse this image. */ #define FEATURE_CANCEL (1U << 0) -#define FEATURE_HOST_MAPPINGS (1U << 1) #define FEATURE_WORKLOAD_ACCOUNT (1U << 2) #define FEATURE_EXEC_CGROUP (1U << 3) #define FEATURE_EXEC_ENVIRONMENT (1U << 4) #define FEATURE_EXEC_CWD (1U << 5) +#define FEATURE_HOST_MAPPING_TABLE (1U << 6) #define MAX_ARGUMENTS 64U #define MAX_ARGUMENT_LEN 4096U @@ -80,9 +83,15 @@ #define CGROUP_ROOT "/sys/fs/cgroup" #endif #define EXEC_CGROUP CGROUP_ROOT "/nvx-exec" +#ifndef HOSTFS_DIR #define HOSTFS_DIR "/run/nvx/hostfs" +#endif #define HOSTFS_ROOT HOSTFS_DIR "/root" -#define MAP_TOKEN "nvx_map=" +#define MAPS_TOKEN "nvx_maps=" +#define MAX_HOST_MAPPINGS 4096U +#define MAPS_HEADER_LEN 8U +#define MAP_ENTRY_HEADER_LEN 6U +#define MAP_READ_ONLY 1U #define MAX_COMMAND_LINE 4096U #define MAX_GUEST_PATH 4096U @@ -129,6 +138,16 @@ struct exec_config { int inherit_default_env; }; +/* + * Host mappings that nvx_maps= announces and APP_MAPS requests deliver. Until + * every announced mapping is mounted, workloads are refused. + */ +struct host_mappings { + uint32_t expected; + uint32_t applied; + int failed; +}; + #define LAUNCH_STAGE_CWD 1 #define LAUNCH_STAGE_SETUP 2 @@ -315,53 +334,6 @@ static int write_outer_record( return payload_len == 0 || write_all(fd, payload, payload_len) == 0 ? 0 : -1; } -static int hex_digit(char value) -{ - if (value >= '0' && value <= '9') { - return value - '0'; - } - if (value >= 'A' && value <= 'F') { - return value - 'A' + 10; - } - if (value >= 'a' && value <= 'f') { - return value - 'a' + 10; - } - return -1; -} - -/* Decodes a percent-encoded field of length `length` into a NUL-terminated string. */ -static int percent_decode(const char *input, size_t length, char *output, size_t capacity) -{ - size_t index; - size_t used = 0; - - for (index = 0; index < length; ++index) { - char value = input[index]; - - if (value == '%') { - int high; - int low; - - if (index + 2 >= length) { - return -1; - } - high = hex_digit(input[index + 1]); - low = hex_digit(input[index + 2]); - if (high < 0 || low < 0) { - return -1; - } - value = (char)((high << 4) | low); - index += 2; - } - if (value == '\0' || used + 1 >= capacity) { - return -1; - } - output[used++] = value; - } - output[used] = '\0'; - return 0; -} - /* Accepts a /-separated path without empty, "." (unless alone), or ".." components. */ static int safe_path(const char *path, int absolute) { @@ -423,20 +395,60 @@ static int make_directories(const char *path, int include_last) } /* - * Bind-mounts each mapped host path at its guest target. The host's export is - * mounted at HOSTFS_ROOT, whose parent only root may enter, so workloads reach - * mapped paths only through these bind mounts. Every token has the form - * nvx_map=SOURCE,TARGET,ro|rw with percent-encoded paths; SOURCE is relative to - * the export, and "." is the export itself. + * Reads the number of host mappings that nvx_maps=COUNT announces into + * `count`, or 0 without the token. Tokens are split in place. Older hosts + * listed their mappings in nvx_map= tokens, which are ignored. */ -static int setup_host_mappings(void) +static int parse_host_mapping_count(char *command_line, uint32_t *count) { - char command_line[MAX_COMMAND_LINE]; char *token; char *cursor; + int found = 0; + + *count = 0; + for (token = strtok_r(command_line, " \t\n", &cursor); token != NULL; + token = strtok_r(NULL, " \t\n", &cursor)) { + const char *digit = token + strlen(MAPS_TOKEN); + uint32_t value = 0; + + if (strncmp(token, MAPS_TOKEN, strlen(MAPS_TOKEN)) != 0) { + continue; + } + if (found || *digit == '\0') { + errno = EINVAL; + return -1; + } + for (; *digit != '\0'; ++digit) { + if (*digit < '0' || *digit > '9' || value > MAX_HOST_MAPPINGS) { + errno = EINVAL; + return -1; + } + value = value * 10U + (uint32_t)(*digit - '0'); + } + if (value == 0 || value > MAX_HOST_MAPPINGS) { + errno = EINVAL; + return -1; + } + *count = value; + found = 1; + } + return 0; +} + +/* + * Reads the number of announced host mappings and, while the host's export is + * mounted at HOSTFS_ROOT, makes its parent a directory that only root may + * enter, so workloads reach mapped paths only through the bind mounts. The + * announced mappings need that export. + */ +static int prepare_host_mappings(struct host_mappings *mappings) +{ + char command_line[MAX_COMMAND_LINE]; + struct stat parent; + struct stat export; ssize_t count; + int mounted; int fd = open("/proc/cmdline", O_RDONLY | O_CLOEXEC); - int prepared = 0; if (fd < 0) { return -1; @@ -447,98 +459,121 @@ static int setup_host_mappings(void) return -1; } command_line[count] = '\0'; + if (parse_host_mapping_count(command_line, &mappings->expected) != 0) { + return -1; + } + mounted = stat(HOSTFS_DIR, &parent) == 0 && stat(HOSTFS_ROOT, &export) == 0 && + parent.st_dev != export.st_dev; + if (mounted && (chown(HOSTFS_DIR, 0, 0) != 0 || chmod(HOSTFS_DIR, 0700) != 0)) { + return -1; + } + if (mappings->expected != 0 && !mounted) { + errno = ENOENT; + return -1; + } + return 0; +} - for (token = strtok_r(command_line, " \n", &cursor); token != NULL; - token = strtok_r(NULL, " \n", &cursor)) { - char source[MAX_GUEST_PATH]; - char target[MAX_GUEST_PATH]; - char host[MAX_GUEST_PATH + sizeof(HOSTFS_ROOT) + 1]; - const char *fields = token + strlen(MAP_TOKEN); - const char *first; - const char *second; - struct stat status; - unsigned long flags = MS_BIND | MS_REMOUNT | MS_NOSUID | MS_NODEV; - - if (strncmp(token, MAP_TOKEN, strlen(MAP_TOKEN)) != 0) { - continue; - } - first = strchr(fields, ','); - second = first == NULL ? NULL : strchr(first + 1, ','); - if (second == NULL || strchr(second + 1, ',') != NULL || - percent_decode(fields, (size_t)(first - fields), source, sizeof(source)) != 0 || - percent_decode(first + 1, (size_t)(second - first - 1), target, sizeof(target)) != - 0 || - !safe_path(source, 0) || !safe_path(target, 1)) { - errno = EINVAL; - return -1; - } - if (strcmp(second + 1, "ro") == 0) { - flags |= MS_RDONLY; - } else if (strcmp(second + 1, "rw") != 0) { - errno = EINVAL; +/* + * Bind-mounts SOURCE, a path relative to the export, at the guest path TARGET, + * read-only or read-write, without set-user-ID programs or device nodes. + */ +static int apply_host_mapping(const char *source, const char *target, int read_only) +{ + char host[MAX_GUEST_PATH + sizeof(HOSTFS_ROOT) + 1]; + struct stat status; + unsigned long flags = MS_BIND | MS_REMOUNT | MS_NOSUID | MS_NODEV; + + if (read_only) { + flags |= MS_RDONLY; + } + snprintf(host, sizeof(host), "%s/%s", HOSTFS_ROOT, source); + if (lstat(host, &status) != 0) { + return -1; + } + if (S_ISDIR(status.st_mode)) { + if (make_directories(target, 1) != 0) { return -1; } - if (!prepared) { - struct stat parent; - struct stat export; + } else if (S_ISREG(status.st_mode)) { + struct stat existing; - /* The export must be a mount of its own, inside a root-only directory. */ - if (stat(HOSTFS_DIR, &parent) != 0 || stat(HOSTFS_ROOT, &export) != 0 || - parent.st_dev == export.st_dev || chown(HOSTFS_DIR, 0, 0) != 0 || - chmod(HOSTFS_DIR, 0700) != 0) { - return -1; - } - prepared = 1; - } - if (strcmp(source, ".") == 0) { - snprintf(host, sizeof(host), "%s", HOSTFS_ROOT); - } else { - snprintf(host, sizeof(host), "%s/%s", HOSTFS_ROOT, source); - } - if (lstat(host, &status) != 0) { + if (make_directories(target, 0) != 0) { return -1; } - if (S_ISDIR(status.st_mode)) { - if (make_directories(target, 1) != 0) { - return -1; - } - } else if (S_ISREG(status.st_mode)) { - struct stat existing; + /* + * An existing target may lie inside an earlier read-only bind, where + * even O_CREAT with write access fails, so only a missing one is made. + */ + if (lstat(target, &existing) != 0) { + int file; - if (make_directories(target, 0) != 0) { + if (errno != ENOENT) { return -1; } - /* - * An existing target may lie inside an earlier read-only bind, where - * even O_CREAT with write access fails, so only a missing one is made. - */ - if (lstat(target, &existing) != 0) { - int file; - - if (errno != ENOENT) { - return -1; - } - file = open(target, O_RDONLY | O_CREAT | O_EXCL | O_CLOEXEC | O_NOFOLLOW, 0644); - if (file < 0) { - return -1; - } - close(file); - } else if (!S_ISREG(existing.st_mode)) { - errno = EINVAL; + file = open(target, O_RDONLY | O_CREAT | O_EXCL | O_CLOEXEC | O_NOFOLLOW, 0644); + if (file < 0) { return -1; } - } else { + close(file); + } else if (!S_ISREG(existing.st_mode)) { errno = EINVAL; return -1; } - if (mount(host, target, NULL, MS_BIND, NULL) != 0 || - mount(NULL, target, NULL, flags, NULL) != 0) { - return -1; - } + } else { + errno = EINVAL; + return -1; + } + if (mount(host, target, NULL, MS_BIND, NULL) != 0 || + mount(NULL, target, NULL, flags, NULL) != 0) { + return -1; } return 0; } +/* + * Decodes the APP_MAPS entry at `*offset` and advances past it: 16-bit flags, + * source length, and target length, then the source, a relative path of the + * export, and the target, an absolute guest path. + */ +static int decode_host_mapping( + const uint8_t *payload, + uint32_t payload_len, + uint32_t *offset, + char *source, + char *target, + int *read_only) +{ + uint16_t flags; + uint16_t source_len; + uint16_t target_len; + + if (payload_len - *offset < MAP_ENTRY_HEADER_LEN) { + return -1; + } + flags = read_u16(payload + *offset); + source_len = read_u16(payload + *offset + 2); + target_len = read_u16(payload + *offset + 4); + *offset += MAP_ENTRY_HEADER_LEN; + if ((flags & ~MAP_READ_ONLY) != 0 || source_len == 0 || target_len == 0 || + source_len >= MAX_GUEST_PATH || target_len >= MAX_GUEST_PATH || + payload_len - *offset < (uint32_t)source_len + target_len) { + return -1; + } + memcpy(source, payload + *offset, source_len); + source[source_len] = '\0'; + *offset += source_len; + memcpy(target, payload + *offset, target_len); + target[target_len] = '\0'; + *offset += target_len; + if (strlen(source) != source_len || strlen(target) != target_len || + strcmp(source, ".") == 0 || !safe_path(source, 0) || !safe_path(target, 1)) { + return -1; + } + *read_only = (flags & MAP_READ_ONLY) != 0; + return 0; +} + static int send_guest_attach(int fd) { const uint8_t zero_instance[16] = {0}; @@ -2090,69 +2125,116 @@ static uint32_t agent_features(const struct agent_config *config) FEATURE_CANCEL | FEATURE_WORKLOAD_ACCOUNT | FEATURE_EXEC_ENVIRONMENT; if (config->direct) { - features |= FEATURE_HOST_MAPPINGS | FEATURE_EXEC_CGROUP | FEATURE_EXEC_CWD; + features |= FEATURE_HOST_MAPPING_TABLE | FEATURE_EXEC_CGROUP | FEATURE_EXEC_CWD; } return features; } -static int handle_data_record( +/* + * Mounts the entries FIRST to FIRST + COUNT - 1 of the host mapping table that + * an APP_MAPS request carries after its 32-bit FIRST and COUNT. Entries arrive + * in order and only up to the number that nvx_maps= announced, before any + * workload runs. The whole request is checked before anything is mounted, and + * a mapping that fails to mount fails every later workload. + */ +static int handle_host_mappings( + struct control_session *session, + struct host_mappings *mappings, + const struct app_request *request) +{ + char source[MAX_GUEST_PATH]; + char target[MAX_GUEST_PATH]; + uint32_t first; + uint32_t count; + uint32_t index; + uint32_t offset = MAPS_HEADER_LEN; + int read_only; + + if (mappings->failed || request->payload_len < MAPS_HEADER_LEN) { + return send_app_error(session, request->request_id, 22, "invalid-request"); + } + first = read_u32(request->payload); + count = read_u32(request->payload + 4); + if (first != mappings->applied || count == 0 || + count > mappings->expected - mappings->applied) { + return send_app_error(session, request->request_id, 22, "invalid-request"); + } + for (index = 0; index < count; ++index) { + if (decode_host_mapping( + request->payload, request->payload_len, &offset, source, target, + &read_only) != 0) { + return send_app_error(session, request->request_id, 22, "invalid-request"); + } + } + if (offset != request->payload_len) { + return send_app_error(session, request->request_id, 22, "invalid-request"); + } + offset = MAPS_HEADER_LEN; + for (index = 0; index < count; ++index) { + decode_host_mapping( + request->payload, request->payload_len, &offset, source, target, &read_only); + if (apply_host_mapping(source, target, read_only) != 0) { + int status = errno; + + mappings->failed = 1; + portb_error("host-mapping", status); + dprintf( + STDERR_FILENO, + "NVX-MANAGED-ERROR: stage=host-mapping status=%d entry=%u\n", + status, + (unsigned)(first + index)); + return send_app_error(session, request->request_id, status, "mapping-failed"); + } + ++mappings->applied; + } + return send_app_frame(session, APP_READY, request->request_id, 0, NULL, 0); +} + +/* Serves one application request of the authenticated host. */ +static int handle_app_request( struct control_session *session, const struct agent_config *config, - const struct outer_record *record) + struct host_mappings *mappings, + const struct app_request *request) { - struct app_request request; char **workload_argv = NULL; uint8_t features[4]; struct exec_config exec_config = {0}; uint32_t timeout_ms = 0; int result; - if (record->sequence != session->host_sequence) { - portb_error("data-sequence", (int)record->sequence); - return -1; - } - if (record->epoch != session->epoch || - memcmp(record->instance_id, session->instance_id, 16) != 0) { - portb_error("data-identity", (int)record->epoch); - return -1; - } - if (parse_app_request(record->payload, record->payload_len, &request) != 0) { - portb_error("data-application", (int)record->payload_len); - return -1; - } - ++session->host_sequence; - if (send_credit(session, record->payload_len) != 0) { - portb_error("data-credit", (int)record->payload_len); - return -1; - } - - switch (request.kind) { + switch (request->kind) { case APP_PING: - if (request.payload_len != 0) { - portb_error("ping-payload", (int)request.payload_len); + if (request->payload_len != 0) { + portb_error("ping-payload", (int)request->payload_len); return send_app_error( - session, request.request_id, 22, "invalid-request"); + session, request->request_id, 22, "invalid-request"); } result = send_app_frame( - session, APP_READY, request.request_id, 0, NULL, 0); + session, APP_READY, request->request_id, 0, NULL, 0); if (result != 0) { portb_error("ping-ready", errno); } return result; case APP_EXEC: + /* Workloads must not run without the host paths that they expect. */ + if (mappings->applied != mappings->expected) { + return send_app_error( + session, request->request_id, EAGAIN, "mappings-incomplete"); + } if (decode_exec_payload( - request.payload, - request.payload_len, + request->payload, + request->payload_len, &timeout_ms, &workload_argv, &exec_config) != 0) { return send_app_error( - session, request.request_id, 22, "invalid-request"); + session, request->request_id, 22, "invalid-request"); } result = run_exec( session, config, - request.request_id, + request->request_id, timeout_ms, workload_argv, &exec_config); @@ -2163,35 +2245,67 @@ static int handle_data_record( /* The targeted workload already finished and reported its outcome. */ return 0; case APP_FEATURES: - if (request.payload_len != 0) { + if (request->payload_len != 0) { return send_app_error( - session, request.request_id, 22, "invalid-request"); + session, request->request_id, 22, "invalid-request"); } write_u32(features, agent_features(config)); return send_app_frame( session, APP_READY, - request.request_id, + request->request_id, 0, features, sizeof(features)); + case APP_MAPS: + return handle_host_mappings(session, mappings, request); case APP_STOP: - if (request.payload_len != 0 || + if (request->payload_len != 0 || send_app_frame( - session, APP_STOPPED, request.request_id, 0, NULL, 0) != 0) { + session, APP_STOPPED, request->request_id, 0, NULL, 0) != 0) { return -1; } tcdrain(session->fd); return AGENT_STOPPED; default: return send_app_error( - session, request.request_id, 95, "unsupported-operation"); + session, request->request_id, 95, "unsupported-operation"); } } +static int handle_data_record( + struct control_session *session, + const struct agent_config *config, + struct host_mappings *mappings, + const struct outer_record *record) +{ + struct app_request request; + + if (record->sequence != session->host_sequence) { + portb_error("data-sequence", (int)record->sequence); + return -1; + } + if (record->epoch != session->epoch || + memcmp(record->instance_id, session->instance_id, 16) != 0) { + portb_error("data-identity", (int)record->epoch); + return -1; + } + if (parse_app_request(record->payload, record->payload_len, &request) != 0) { + portb_error("data-application", (int)record->payload_len); + return -1; + } + ++session->host_sequence; + if (send_credit(session, record->payload_len) != 0) { + portb_error("data-credit", (int)record->payload_len); + return -1; + } + return handle_app_request(session, config, mappings, &request); +} + static int run_agent( struct control_session *session, - const struct agent_config *config) + const struct agent_config *config, + struct host_mappings *mappings) { struct outer_record record; @@ -2210,7 +2324,7 @@ static int run_agent( return -1; } } else if (record.type == OUTER_DATA) { - int result = handle_data_record(session, config, &record); + int result = handle_data_record(session, config, mappings, &record); if (result == AGENT_STOPPED) { free_outer_record(&record); @@ -2249,6 +2363,7 @@ static int finish_agent(const struct agent_config *config, int status) int main(int argc, char **argv) { struct control_session session = {0}; + struct host_mappings mappings = {0}; struct agent_config config; int result; @@ -2275,7 +2390,7 @@ int main(int argc, char **argv) status); return finish_agent(&config, 125); } - if (config.direct && setup_host_mappings() != 0) { + if (config.direct && prepare_host_mappings(&mappings) != 0) { int status = errno; portb_error("host-mappings", status); dprintf( @@ -2295,7 +2410,7 @@ int main(int argc, char **argv) close(session.fd); return finish_agent(&config, 125); } - result = run_agent(&session, &config); + result = run_agent(&session, &config, &mappings); if (result == AGENT_STOPPED) { close(session.fd); return finish_agent(&config, 0); diff --git a/scripts/test_managed_agent.py b/scripts/test_managed_agent.py index 01a45fcf..794ff2ff 100644 --- a/scripts/test_managed_agent.py +++ b/scripts/test_managed_agent.py @@ -7,6 +7,7 @@ import sys import tempfile import unittest +from collections.abc import Sequence from pathlib import Path @@ -467,5 +468,328 @@ def test_parent_barrier_release_preserves_live_child_start(self): self.assertEqual((result.returncode, result.stderr), (0, b"")) +MS_RDONLY = 1 +MS_NOSUID = 2 +MS_NODEV = 4 +MS_REMOUNT = 32 +MS_BIND = 4096 +REMOUNT = MS_BIND | MS_REMOUNT | MS_NOSUID | MS_NODEV +APP_EXEC = 2 +APP_FEATURES = 5 +APP_MAPS = 6 +APP_READY = 0x81 +APP_ERROR = 0xFF + + +@unittest.skipUnless(sys.platform.startswith("linux"), "guest agent requires Linux") +class ManagedAgentHostMappingTests(unittest.TestCase): + """Drives the host mapping table of a direct-mode agent whose mounts are recorded.""" + + @classmethod + def setUpClass(cls): + compiler = shutil.which("cc") + if compiler is None: + raise unittest.SkipTest("guest agent requires the guest build C compiler") + cls.temporary = tempfile.TemporaryDirectory(prefix="nvx-agent-mappings-") + cls.addClassCleanup(cls.temporary.cleanup) + cls.root = Path(cls.temporary.name) + cls.executable = cls.root / "mappings" + harness = cls.root / "mappings.c" + harness.write_text( + """ +#define main nvx_agent_main +#include "nvx-managed-agent.c" +#undef main + +int __wrap_mount(const char *source, const char *target, const char *type, + unsigned long flags, const void *data) { + FILE *log = fopen(getenv("NVX_TEST_MOUNT_LOG"), "a"); + (void)type; + (void)data; + if (log == NULL) return -1; + fprintf(log, "%s|%s|%lu\\n", source == NULL ? "-" : source, target, flags); + fclose(log); + return 0; +} + +/* + * mappings COMMAND_LINE REQUESTS OUTPUT: handles each request of REQUESTS, a + * 1-byte kind and a 4-byte length before each payload, with IDs 1, 2, ..., and + * writes the answers to OUTPUT and the mapping state to stdout. + */ +int main(int argc, char **argv) { + static unsigned char bytes[1 << 20]; + char command_line[MAX_COMMAND_LINE]; + struct control_session session = {0}; + struct host_mappings mappings = {0}; + struct agent_config config = { + .rootfs = "-", .hostname = "test", .uid = "1000", .gid = "1000", + .user = "test", .home = "/", .direct = 1, + }; + uint64_t request_id = 1; + size_t offset = 0; + size_t length; + FILE *file; + + if (argc != 4) return 2; + snprintf(command_line, sizeof(command_line), "%s", argv[1]); + if (parse_host_mapping_count(command_line, &mappings.expected) != 0) return 3; + file = fopen(argv[2], "rb"); + if (file == NULL) return 2; + length = fread(bytes, 1, sizeof(bytes), file); + fclose(file); + session.fd = open(argv[3], O_WRONLY | O_CREAT | O_TRUNC, 0600); + if (session.fd < 0) return 2; + while (offset + 5 <= length) { + struct app_request request = {0}; + request.kind = bytes[offset]; + request.payload_len = read_u32(bytes + offset + 1); + request.payload = bytes + offset + 5; + request.request_id = request_id++; + offset += 5 + request.payload_len; + if (offset > length) return 2; + if (handle_app_request(&session, &config, &mappings, &request) < 0) return 4; + } + printf("%u %u %d\\n", mappings.expected, mappings.applied, mappings.failed); + return 0; +} +""", + encoding="utf-8", + ) + guest = Path(__file__).resolve().parent.parent / "guest" / "common" + subprocess.run( + [ + compiler, + f'-DHOSTFS_DIR="{cls.root / "hostfs"}"', + "-D_GNU_SOURCE", + "-std=c11", + "-I", + str(guest), + str(harness), + "-Wl,--wrap=mount", + "-o", + str(cls.executable), + ], + check=True, + capture_output=True, + timeout=30, + ) + + def setUp(self): + self.case = Path(tempfile.mkdtemp(prefix="case-", dir=self.root)) + export = self.root / "hostfs" / "root" + shutil.rmtree(export, ignore_errors=True) + (export / "0" / "src").mkdir(parents=True) + (export / "0" / "notes.txt").write_text("notes", encoding="utf-8") + (export / "1").mkdir() + self.log = self.case / "mounts.log" + + @staticmethod + def maps( + first: int, + entries: Sequence[tuple[str | bytes, str | bytes, int]], + count: int | None = None, + ) -> bytes: + payload = struct.pack(" tuple[subprocess.CompletedProcess[bytes], list[tuple[int, int, int, bytes]]]: + source = self.case / "requests.bin" + source.write_bytes( + b"".join( + struct.pack(" list[tuple[str, str, int]]: + if not self.log.exists(): + return [] + return [ + (source, target, int(flags)) + for source, target, flags in ( + line.split("|") for line in self.log.read_text().splitlines() + ) + ] + + def test_mappings_mount_in_order_and_unblock_workloads(self): + export = self.root / "hostfs" / "root" + src = str(self.case / "guest" / "src") + notes = str(self.case / "guest" / "src" / "notes.txt") + out = str(self.case / "guest" / "out") + result, answers = self.run_agent( + # A stale token of an older host is ignored. + "quiet nvx_map=0,/stale,rw nvx_maps=3", + [ + (APP_EXEC, b""), + ( + APP_MAPS, + self.maps(0, [("0/src", src, 1), ("0/notes.txt", notes, 0)]), + ), + (APP_EXEC, b""), + (APP_MAPS, self.maps(2, [("1", out, 0)])), + # An empty request is malformed, so it fails only once the mappings are complete. + (APP_EXEC, b""), + ], + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, b"3 3 0\n") + self.assertEqual( + [(kind, status, payload) for kind, _, status, payload in answers], + [ + (APP_ERROR, 11, b"mappings-incomplete"), + (APP_READY, 0, b""), + (APP_ERROR, 11, b"mappings-incomplete"), + (APP_READY, 0, b""), + (APP_ERROR, 22, b"invalid-request"), + ], + ) + self.assertEqual( + self.mounts(), + [ + (str(export / "0" / "src"), src, MS_BIND), + ("-", src, REMOUNT | MS_RDONLY), + (str(export / "0" / "notes.txt"), notes, MS_BIND), + ("-", notes, REMOUNT), + (str(export / "1"), out, MS_BIND), + ("-", out, REMOUNT), + ], + ) + self.assertTrue(Path(src).is_dir()) + self.assertTrue(Path(notes).is_file()) + self.assertTrue(Path(out).is_dir()) + + def test_entries_beyond_the_announcement_or_out_of_order_are_refused(self): + target = str(self.case / "guest" / "src") + entry = [("0/src", target, 0)] + result, answers = self.run_agent( + "nvx_maps=1", + [ + (APP_MAPS, self.maps(1, entry)), + (APP_MAPS, self.maps(0, entry * 2)), + (APP_MAPS, self.maps(0, entry)), + (APP_MAPS, self.maps(1, entry)), + ], + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, b"1 1 0\n") + self.assertEqual( + [(kind, status) for kind, _, status, _ in answers], + [(APP_ERROR, 22), (APP_ERROR, 22), (APP_READY, 0), (APP_ERROR, 22)], + ) + self.assertEqual(len(self.mounts()), 2) + + def test_malformed_tables_mount_nothing(self): + target = str(self.case / "guest" / "x") + good = ("0/src", target, 0) + malformed = [ + self.maps(0, [good, ("0/../1", target, 0)]), + self.maps(0, [good, ("/0/src", target, 0)]), + self.maps(0, [good, ("0/src", "relative", 0)]), + self.maps(0, [good, (".", target, 0)]), + self.maps(0, [good, ("0//src", target, 0)]), + self.maps(0, [good, (b"0/s\0rc", target, 0)]), + self.maps(0, [good, ("0/src", target, 2)]), + self.maps(0, [good, ("", target, 0)]), + self.maps(0, [good]) + b"\0", + self.maps(0, [good], count=2), + self.maps(0, [], count=0), + b"\0\0\0", + ] + result, answers = self.run_agent( + "nvx_maps=2", [(APP_MAPS, payload) for payload in malformed] + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, b"2 0 0\n") + self.assertEqual( + [(kind, status, payload) for kind, _, status, payload in answers], + [(APP_ERROR, 22, b"invalid-request")] * len(malformed), + ) + self.assertEqual(self.mounts(), []) + + def test_without_an_announcement_tables_are_refused(self): + target = str(self.case / "guest" / "src") + result, answers = self.run_agent( + "nvx_map=0,/w,rw", + [(APP_MAPS, self.maps(0, [("0/src", target, 0)])), (APP_EXEC, b"")], + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, b"0 0 0\n") + self.assertEqual( + [(kind, status, payload) for kind, _, status, payload in answers], + [(APP_ERROR, 22, b"invalid-request"), (APP_ERROR, 22, b"invalid-request")], + ) + self.assertEqual(self.mounts(), []) + + def test_a_failed_mount_keeps_refusing_workloads(self): + target = str(self.case / "guest" / "missing") + result, answers = self.run_agent( + "nvx_maps=2", + [ + (APP_MAPS, self.maps(0, [("0/missing", target, 0)])), + (APP_EXEC, b""), + (APP_MAPS, self.maps(0, [("0/src", target, 0)])), + ], + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, b"2 0 1\n") + self.assertEqual( + [(kind, status, payload) for kind, _, status, payload in answers], + [ + (APP_ERROR, 2, b"mapping-failed"), + (APP_ERROR, 11, b"mappings-incomplete"), + (APP_ERROR, 22, b"invalid-request"), + ], + ) + + def test_mapping_counts_are_validated(self): + for command_line in [ + "nvx_maps=0", + "nvx_maps=", + "nvx_maps=4097", + "nvx_maps=99999999999", + "nvx_maps=1x", + "nvx_maps=1 nvx_maps=1", + ]: + result, _ = self.run_agent(command_line, []) + self.assertEqual(result.returncode, 3, command_line) + result, _ = self.run_agent("nvx_maps=4096", []) + self.assertEqual(result.stdout, b"4096 0 0\n") + + def test_features_announce_the_mapping_table_but_not_retired_bit_one(self): + _, answers = self.run_agent("", [(APP_FEATURES, b"")]) + self.assertEqual(len(answers), 1) + kind, _, status, payload = answers[0] + self.assertEqual((kind, status), (APP_READY, 0)) + (features,) = struct.unpack(" Date: Fri, 9 Oct 2026 19:10:27 -0300 Subject: [PATCH 3/4] run, sandbox: attach several shares as children of one virtio-fs device The pinned OpenVMM gives a microVM one virtio-fs slot and refuses a repeated --mount, so run and sandbox could no longer attach the second share that they placed in the retired slot, and they never could attach a third. One --mount still attaches the slot as before. Two or more now attach it as an aggregate: --mount-aggregate /run/nvx/shares, with one --mount-child NAME,HOST_PATH,MODE per share in order, and absolute policy paths that OpenVMM attributes to the child that contains them. A share's name is its index, a hyphen, and the first 32 hexadecimal digits of the SHA-256 digest of its guest target. NVX adds an nvx_share=NAME,TARGET,MODE token per share to the kernel command line, and the two-share cap gives way to that command line's 1024-byte budget, which the shares' tokens and OpenVMM's bootstrap tokens share. run rejects targets that overlap each other or /run/nvx/shares, and relative policy paths among several shares. A restore passes the same aggregate without the tokens, because the restored guest keeps its binds. OpenVMM does not record the guest targets, but its snapshot contract records the children's names, so a restore that requests another target fails before the guest runs instead of resuming binds at the snapshot's targets. With several shares, sandbox accepts a --mount-deny of a share's own directory, which OpenVMM accepts when --mount-allow paths inside it remain, and still requires every other policy path to lie inside its share. The managed configuration still lists the shares, so its format is unchanged. In the guest, nvx-hostmount mounts an aggregate at /run/nvx/shares, whose root only root can enter, and binds each child at its target with its mode and nosuid,nodev; with --unmount, it unmounts the binds in reverse order and then the share. nvx-init-agent checks every child and creates every target before it mounts anything, mounts the aggregate outside the container root, binds each child into it, and unmounts the binds before the aggregate on teardown. Both accept a child name of ASCII letters, digits, '.', '_', and '-', other than '.' and '..'. nvx-virtio-restore-probe reads and writes through an aggregate's first child and unmounts with nvx-hostmount --unmount. Tests cover the argument translation, the children's names, the tokens and their budget, a hidden share root, a restore's names, the managed lifecycle's replay, and the guest's child names, binds, refusals, and teardown order. The run and usage guides, the machine and device ABI, which frees IRQ 13 and the 0xd0008000 window, and the sandbox design describe one slot, the aggregate, and its children's names. A rename between children fails with EXDEV, and so does a hard link, unless its destination is read-only, which fails with EROFS first, as on Linux. Part of #281. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/ci.md | 4 +- doc/design/cold-boot.md | 7 +- doc/design/machine-and-device-abi.md | 79 ++- ...ndbox-filesystem-and-agent-architecture.md | 57 +- doc/run.md | 92 ++- doc/usage.md | 16 +- guest/common/nvx-hostmount | 121 ++-- guest/common/nvx-init-agent | 226 +++++-- guest/common/nvx-virtio-restore-probe | 19 +- scripts/nvx.py | 104 ++- scripts/nvx_tools/sandbox.py | 112 +++- scripts/test_nvx_tools.py | 605 +++++++++++++++--- 12 files changed, 1094 insertions(+), 348 deletions(-) diff --git a/doc/ci.md b/doc/ci.md index 7d3b9f0c..b66cd033 100644 --- a/doc/ci.md +++ b/doc/ci.md @@ -267,8 +267,8 @@ the read-write check through `provision`, `start`, `exec`, and `stop`, and must report a successful outcome with a cleanly unmounted scratch filesystem, which shows that `stop` unmounted the share and overlay first. Each backend then attaches a read-write `/workspace` share and a read-only -`/opt/hostedtoolcache` share to one sandbox at the same time, each with its -own denied subdirectory. The smoke script runs the read-write and read-only +`/opt/hostedtoolcache` share to one sandbox at the same time, as the children +of one aggregate virtio-fs device, each with its own denied subdirectory. The smoke script runs the read-write and read-only checks on the two shares and verifies that a link in the read-write share cannot write into the read-only share; the host requires the guest's writes in the workspace, an unchanged tool cache, and unchanged denied files, both for a diff --git a/doc/design/cold-boot.md b/doc/design/cold-boot.md index 983e0b74..7bef2af8 100644 --- a/doc/design/cold-boot.md +++ b/doc/design/cold-boot.md @@ -96,14 +96,13 @@ tokens. A fresh boot then appends host-owned tokens in this order: `nvx_workload_gid=`) and workload lifecycle (`nvx_lifecycle=`); 3. `nvx_snapshot_tier=` for a capture with sandbox blocks; 4. device-discovery tokens in fixed address order: network, the first - filesystem slot, boot console, sandbox blocks, the control console followed - by `nvx_control_tty=hvc2`, and the second filesystem slot when a second - HostFs export is attached; and + filesystem slot, boot console, sandbox blocks, and the control console + followed by `nvx_control_tty=hvc2`; and 5. network bootstrap tokens, including gateway DNS only when the egress policy permits it, which names the IPv4 gateway or else the IPv6 one, followed by [filesystem bootstrap tokens](machine-and-device-abi.md#filesystem) for - each attached HostFs export, in slot order. + the attached HostFs export or aggregate. The command line carries no clock parameter: the guest reads its TSC and LAPIC rates from the [time ABI](time-abi.md#rates) MSRs, and a diff --git a/doc/design/machine-and-device-abi.md b/doc/design/machine-and-device-abi.md index 849dbd2d..e300041c 100644 --- a/doc/design/machine-and-device-abi.md +++ b/doc/design/machine-and-device-abi.md @@ -103,22 +103,22 @@ restore, the same write acknowledges completion of guest repair. ## Fixed virtio-mmio transport -All nine fixed address slots are reserved, including the dedicated control -console at `0xd0007000..0xd0007fff` on IRQ 3 (shared status at `0x3001c`) and -the second virtio-fs slot at `0xd0008000..0xd0008fff` on IRQ 13 (shared status -at `0x30020`). +All eight fixed address slots are reserved, including the dedicated control +console at `0xd0007000..0xd0007fff` on IRQ 3 (shared status at `0x3001c`). +The window at `0xd0008000..0xd0008fff`, IRQ 13, and the shared-status word at +`0x30020` held a second virtio-fs slot in earlier releases; they are unused, and +a snapshot that recorded that slot no longer restores. Every microVM cold-booted from the command line or the management RPC -instantiates the first virtio-fs slot, so it is discoverable before capture even +instantiates the virtio-fs slot, so it is discoverable before capture even without a host attachment; a restore keeps the slot only when the snapshot -recorded it. The second virtio-fs slot exists only with a second host -attachment, so machines with one share or none keep their device inventory -and command line. Other optional devices are instantiated only when configured. +recorded it. Several shares are children of one aggregate attachment on that +slot. Other optional devices are instantiated only when configured. The control slot is instantiated for a control console, either a live authenticated local endpoint on Linux or Windows or an explicitly disconnected console. Every device uses virtio-mmio, is discovered only through profile-owned command-line tokens, and has packed-ring support masked. -The MP table routes only the 16 ISA interrupts, and no other device uses -IRQ 13 on KVM, MSHV, or WHP. +The MP table routes only the 16 ISA interrupts, and no device uses IRQ 13 on +KVM, MSHV, or WHP. | Device | Stable identity | MMIO range | IRQ | Availability | | --- | --- | ---: | ---: | --- | @@ -130,7 +130,6 @@ IRQ 13 on KVM, MSHV, or WHP. | `custom` virtio-blk | `blk:sandbox:custom` | `0xd0005000..0xd0005fff` | 9 | Optional read-only role | | `scratch` virtio-blk | `blk:sandbox:scratch` | `0xd0006000..0xd0006fff` | 11 | Required writable final role when blocks are present | | Control virtio-console | `console:microvm-control0` | `0xd0007000..0xd0007fff` | 3 | Optional authenticated local endpoint | -| Second virtio-fs | `fs:microvm1` | `0xd0008000..0xd0008fff` | 13 | Optional; requires a HostFs attachment | Explicit placement metadata bypasses the standard sequential MMIO allocator. The worker validates the complete device count, kind, bus, address, IRQ, and @@ -165,7 +164,6 @@ per fixed slot resides in the reserved shared-status page: | `custom` block | `0x30014` | | `scratch` block | `0x30018` | | Control virtio-console | `0x3001c` | -| Second virtio-fs | `0x30020` | OpenVMM publishes config-change and used-buffer bits with a sequentially consistent compare-exchange loop. It pulses the device IRQ only when the old @@ -373,18 +371,28 @@ traffic must establish fresh post-restore flows. ### Filesystem -Each filesystem slot is a no-DAX virtio-fs device with a fixed tag, `microvm` -for the first slot and `microvm1` for the second, one -high-priority queue, one request queue, direct I/O, and zero guest cache +The filesystem slot is a no-DAX virtio-fs device with the fixed tag `microvm`, +one high-priority queue, one request queue, direct I/O, and zero guest cache lifetimes. It requires FUSE 7.31 or newer and caps writes at 1 MiB. Without -`--mount`, the first slot has no HostFs backend or active filesystem policy but remains -guest-discoverable. A second `--mount` attaches a second, independent HostFs -server with its own access mode and denied paths to the second slot. Guest -targets and host roots of the two attachments must not equal or contain one -another, so neither share can hide the other or reach its files under a -different policy. Each attachment adds one `virtfs_dir=`, `virtfs_tag=`, -`virtfs_mode=` token triplet, in slot order. Its explicit profile rejects SectionFs, Aggregate, -alternate tags, extra queues, shared-memory windows, and PCI transport. +`--mount` or `--mount-aggregate`, the slot has no HostFs backend or active +filesystem policy but remains guest-discoverable. `--mount` may appear once and +attaches one HostFs server. `--mount-aggregate GUEST_TARGET` with a repeatable +`--mount-child NAME,HOST_PATH[,ro|rw]` instead attaches an aggregate: a +synthetic root, mode `0500` and owned by root, which lists one child per host +directory under its name and refuses every change, and below each child an +independent HostFs volume with its own access mode and denied, allowed, and +writable paths. Policy paths of an aggregate are absolute and apply to the +child whose root contains them. Child names are unique `[A-Za-z0-9._-]` +names of at most 64 bytes, at most 256 children share at most 128 KiB of +policy paths, and their host roots must not equal or contain one another, so +no child can reach the files of another under a different policy. A rename +between children fails with `EXDEV`. A hard link between children fails with +`EROFS` when its destination is read-only, which Linux also reports first, and +otherwise with `EXDEV`. The attachment adds one +`virtfs_dir=`, `virtfs_tag=`, `virtfs_mode=` token triplet, `rw` when any child +is read-write, and `virtfs_aggregate=1` for an aggregate. Its explicit profile +rejects SectionFs, alternate tags, extra queues, shared-memory windows, PCI +transport, and aggregates whose children change after boot. The host root is pinned by its platform object identity and revalidated when the export is opened, and every operation stays confined to the export: on Linux, each host operation opens its parent directory without following a @@ -395,16 +403,19 @@ rejected. A read-write mapping lets the guest create symbolic links with exact targets, which only the guest resolves; a read-only mapping rejects them. Bounded inode, alias, and handle tables fail an operation rather than create untracked host state. -Read-only mode rejects mutation in the host device before invoking host -filesystem operations; read-write mode exposes only the supported common host -contract. +Read-only mode, of a share or of an aggregate child, rejects mutation in the +host device before invoking host filesystem operations; read-write mode exposes +only the supported common host contract. Denied host paths are canonicalized into a bounded relative set and enforced before HostFs operations. Prefix checks hide complete subtrees, while denied root device/inode identities block hard-link, junction, and bind-mount aliases. The policy is unchanged by a second guest mount. Allowed host paths expose subtrees of denied paths again; the nearest policy path that contains a path decides whether the guest can see it, and denied -and allowed paths alternate, so neither kind is redundant. A hidden directory +and allowed paths alternate, so neither kind is redundant. A denied path may +name the root of a share or child when allowed paths inside it remain, which +leaves the root traverse-only. Policy path names may contain spaces, but not +begin or end with one. A hidden directory on the way to an allowed path is traverse-only: the guest can look it up as a directory and list only the entries that lead to allowed paths, and its object identity is accepted only at its own path. Writable host paths, when present, @@ -432,10 +443,16 @@ and handle allocation, aliases (including those of symbolic links), lookup counts, directory snapshots and cookies, and the identities needed to reopen objects. Restore requires the same path, target, mode, denied, allowed, and writable paths, ownership mode, root -identity, and reopenable objects for every captured attachment, supplied in -slot order; the contract -records the second attachment separately, so a restore can neither drop nor -add it. A caller-owned +identity, and reopenable objects for the captured attachment; for an +aggregate, the same target and the same children, names, modes, host paths, +root identities, and policies, in the same order, and the contract records a +digest of the children's identities, so a restore can neither drop, add, nor +reorder one. The guest binds each child at a target that OpenVMM does not +record, so NVX names each child after its index and a digest of its guest +target: the snapshot then pins each target, and a restore that requests +another target fails before the guest runs. An aggregate's capture also saves +each child's volume and the synthetic root's directory state in +device-private schema version 8. A caller-owned capture records device-private schema version 6, which earlier releases reject instead of restoring the attachment as the VMM. A capture with allowed or writable paths records its complete access policy in device-private schema diff --git a/doc/design/sandbox-filesystem-and-agent-architecture.md b/doc/design/sandbox-filesystem-and-agent-architecture.md index 8b04ee6d..73318761 100644 --- a/doc/design/sandbox-filesystem-and-agent-architecture.md +++ b/doc/design/sandbox-filesystem-and-agent-architecture.md @@ -32,8 +32,9 @@ supplied native library and an image-backed edge guest that NVX does not build. ## Implemented filesystem bootstrap The public `nvx sandbox` command accepts one to three role-bearing EROFS lower -images, a preformatted ext4 scratch image, up to two -[live host shares](#live-host-shares), an absolute entrypoint, and +images, a preformatted ext4 scratch image, any number of +[live host shares](#live-host-shares) that its kernel command line holds, an +absolute entrypoint, and individual argument tokens. It supplies non-secret kernel-command-line configuration for one-shot runs. Managed execution carries bounded arguments and a per-execution environment over the authenticated control channel; @@ -81,7 +82,8 @@ The assembled view is: /run/nvx/scratch/work (same ext4) ------> workdir overlay --> /run/nvx/rootfs virtio-fs tag microvm (optional share) ---> /run/nvx/rootfs/TARGET -virtio-fs tag microvm1 (optional share) ---> /run/nvx/rootfs/TARGET + or an aggregate ---> /run/nvx/shares, each child bound at + /run/nvx/rootfs/TARGET ``` At least one lower role is required by the bootstrap; distro plus runtime is @@ -114,29 +116,40 @@ A live share exposes a host directory, such as a source checkout or a tool cache, to the workload without copying it into an image: edits are visible in both directions without staging or copy-back. -Each `--mount` attaches its own fixed virtio-fs slot, tag `microvm` for the -first share and `microvm1` for the second, served by its own HostFs server, so -the two shares keep independent `ro` or `rw` modes and access policies. On -every request, whichever guest mount or link reaches the share, the host -enforces the share's mode and its denied, allowed, and writable paths, and -accesses host files as the identity that `--mount-owner` selects. Guest -mount flags are therefore not a security boundary, and the access policy adds -no guest configuration. Before OpenVMM starts, NVX rejects a third share and -policy paths outside their share. It also rejects guest targets or host +The microVM has one virtio-fs slot, tag `microvm`. One `--mount` attaches a +HostFs server to it. Several attach an aggregate instead, whose children, named +by their index and a digest of their guest target, are independent HostFs +volumes, so the shares keep independent `ro` or `rw` modes and access +policies. On every request, whichever guest +mount or link reaches the share, the host enforces the share's mode and its +denied, allowed, and writable paths, and accesses host files as the identity +that `--mount-owner` selects. Guest mount flags are therefore not a security +boundary, and the access policy adds no guest configuration. Before OpenVMM +starts, NVX rejects policy paths outside their share and guest targets or host directories that equal or contain one another, because one share could -otherwise hide the other or reach its files under a different policy. +otherwise hide another or reach its files under a different policy, and it +rejects shares whose kernel command-line tokens exceed the sandbox's budget. [Machine and device ABI](machine-and-device-abi.md#filesystem) defines the device contract. -OpenVMM appends one `virtfs_dir=`, `virtfs_tag=`, `virtfs_mode=` triplet per -share, in slot order. The init agent parses every triplet before it mounts -any, so a malformed bootstrap mounts nothing. It creates each target inside -the container root one component at a time and refuses a path that crosses a -symbolic link, so an image layer cannot redirect a share outside that root. -It also refuses a repeated tag, overlapping targets, and targets that the -container entry helper later mounts or binds over, where the runtime would -hide the share or write into it. Each share is mounted with its mode and -`nosuid,nodev`, and the workload's private mount namespace inherits it. Any +OpenVMM appends one `virtfs_dir=`, `virtfs_tag=`, `virtfs_mode=` triplet for +its share, and `virtfs_aggregate=1` for an aggregate, which it mounts +read-write when any child is. NVX then adds one `nvx_share=NAME,TARGET,MODE` +token per child. Because a child's name holds a digest of its target, a +snapshot pins each target, and a restore that requests another target fails +before the guest runs. The init agent parses every token, and creates every +target, before it mounts anything, so a malformed bootstrap mounts nothing. It +creates each target inside the container root one component at a time and +refuses a path that crosses a symbolic link, so an image layer cannot redirect +a share outside that root. It also refuses a repeated child, overlapping +targets, and targets that the container entry helper later mounts or binds +over, where the runtime would hide the share or write into it. A single share +is mounted at its target with its mode and `nosuid,nodev`. An aggregate is +mounted at `/run/nvx/shares`, outside the container root, where its root lists +the children and only root can enter, and each child is bound at its target +with its mode and `nosuid,nodev`. The workload's private mount namespace +inherits the mounts, and teardown unmounts the binds in reverse order before +the aggregate. Any refusal or mount failure aborts the sandbox with status 125 instead of starting the workload without its shares. A managed sandbox records its shares when it is provisioned and reattaches them on every start. diff --git a/doc/run.md b/doc/run.md index 03c2a237..7ec56f71 100644 --- a/doc/run.md +++ b/doc/run.md @@ -424,9 +424,8 @@ backing and are onlined before restore readiness. ## virtio-fs host mapping -The microVM has two mapping slots with the fixed tags `microvm` and -`microvm1`. On a cold boot with `--mount`, the initramfs mounts each mapping -automatically: +The microVM has one virtio-fs slot, with the fixed tag `microvm`. On a cold +boot with `--mount`, the initramfs mounts the mapping automatically: ```bash python3 scripts/nvx.py run --mount "/mnt/host,/absolute/host/share,rw" @@ -440,8 +439,8 @@ python scripts\nvx.py run ` ``` Use `ro` for read-only access. The guest target must be an absolute Linux path. -Host paths containing commas are unsupported. Repeat `--mount` once to attach -a second directory with its own target and access mode, for example a +Host paths containing commas are unsupported. Repeat `--mount` to attach more +directories, each with its own target and access mode, for example a read-write workspace next to a read-only tool cache: ```bash @@ -450,20 +449,36 @@ python3 scripts/nvx.py run \ --mount "/opt/hostedtoolcache,/opt/hostedtoolcache,ro" ``` -The first mapping uses tag `microvm` and the second tag `microvm1`. OpenVMM -enforces each mapping's access mode on the host, so a read-only mapping -rejects writes with `EROFS` even if guest root remounts its tag read-write. -Guest targets that equal or contain one another, host directories that equal -or contain one another, and more than two mappings are rejected before boot. +With several mappings, NVX attaches the slot as an aggregate: OpenVMM's +`--mount-aggregate /run/nvx/shares` with one `--mount-child NAME,HOST_PATH,MODE` +per mapping, in order. The initramfs mounts the aggregate at `/run/nvx/shares`, +whose root lists one directory per mapping and only root can enter, and binds +each directory at its target with its mode and `nosuid,nodev`, as an +`nvx_share=NAME,TARGET,MODE` token on the kernel command line describes. A +mapping's name is its index, a hyphen, and the first 32 hexadecimal digits of +the SHA-256 digest of its target. A snapshot records every name, so it pins +each target: a restore that requests another target fails before the guest +runs, rather than resume binds at the snapshot's targets. +OpenVMM enforces each mapping's access mode on the host, so a read-only mapping +rejects writes with `EROFS` even if guest root remounts its bind read-write or +writes through `/run/nvx/shares`. A rename between mappings fails with +`EXDEV`, and so does a hard link, unless its destination is read-only, which +fails with `EROFS` first. Guest targets that equal or contain one another or +`/run/nvx/shares`, and host directories that equal or contain one another, are +rejected before boot. The number of mappings is bounded only by the 1024-byte +budget that their tokens and `--cmdline` share on the kernel command line. A snapshot captured with mappings requires the same mappings, in the same order, with the same canonical host paths, targets, modes, and filesystem identities. A repeatable `--mount-deny HOST_PATH` hides an existing file or directory -inside a mapped root. With two mappings, each `--mount-deny` must be an +inside a mapped root. With several mappings, each `--mount-deny` must be an absolute path, and it applies to the mapping whose root contains it. Denied names are omitted from directory listings and remain inaccessible through `..`, a symlink/junction, or another mount of the same -virtio-fs device. Unsafe, external, duplicate, overlapping, and nested-mount +virtio-fs device. Names may contain spaces, but not begin or end with one. +`--mount-deny` may name a mapping's root itself when `--mount-allow` paths +inside it remain: the root then lists only the way to them. Unsafe, external, +duplicate, overlapping, and nested-mount rules are rejected before boot. A repeatable `--mount-allow HOST_PATH` exposes a path inside a denied path again, and a repeatable `--mount-write HOST_PATH` makes a path one of the only writable parts of an @@ -486,9 +501,9 @@ captured with a mapping also requires the same `--mount-owner` mode. Capture and restore inspect and reopen the guest's open files as OpenVMM's user, so unless OpenVMM runs as root, they fail while the guest holds a file that only its caller can reach or reopen, such as one open for writing. -A snapshot captured without a mapping may restore with one new `--mount`; after -resume, mount it explicitly inside the guest because the initramfs hook has -already completed: +A snapshot captured without a mapping may restore with one new `--mount`, but +not with several; after resume, mount it explicitly inside the guest because +the initramfs hook has already completed: ```sh mkdir -p /mnt/host @@ -593,14 +608,14 @@ enters only the assembled root with `chroot`, because Linux cannot ### Live host-directory shares -`sandbox run` and `sandbox provision` accept up to two +`sandbox run` and `sandbox provision` accept repeatable `--mount GUEST_TARGET,HOST_PATH[,ro|rw]` (default `ro`) options, each with its own guest target and access mode, plus repeatable `--mount-deny HOST_PATH`, `--mount-allow HOST_PATH`, and `--mount-write HOST_PATH` rules (see -[Access policy](#access-policy)). OpenVMM exports each host directory through -its own microVM virtio-fs device and enforces its access mode and policy on -the host side, so edits are visible in both directions without staging or -copy-back: +[Access policy](#access-policy)). OpenVMM exports one host directory through +the microVM's virtio-fs device, and several as the children of an aggregate on +that device, and enforces each share's access mode and policy on the host side, +so edits are visible in both directions without staging or copy-back: ```bash python3 scripts/nvx.py sandbox \ @@ -614,10 +629,11 @@ python3 scripts/nvx.py sandbox \ A relative `--mount-deny`, `--mount-allow`, or `--mount-write` path is resolved inside its share's host directory. With one share, every such path -applies to it. With two, each applies to the `--mount` before it and must name -a path inside that share's directory. The guest targets and the host directories of the two -shares must not equal or contain one another, because one share could -otherwise hide the other or reach its files under a different access mode. +applies to it. With several, each applies to the `--mount` before it and must +name a path inside that share's directory. The guest targets and the host +directories of the shares must not equal or contain one another, because one +share could otherwise hide another or reach its files under a different access +mode. NVX and OpenVMM compare the host directories by resolved path and by file identity, so one directory reached through two paths, such as a bind mount, is rejected too. NVX also compares each directory's identity with those of @@ -633,15 +649,20 @@ an `ro` share with `EROFS`, whichever mount or link inside the guest reaches it. After it assembles the container overlay and verifies the workload identity, the guest agent creates each target inside the container root and mounts the -shares there in order with `nosuid,nodev` before the workload enters its -private mount namespace. A one-shot workload exit, a managed `stop`, and any -failure after a share is mounted unmount the mounted shares in reverse order -before the overlay is unmounted or the VM powers off. Each target must be an -absolute, canonical path; `/`, `/etc`, and +share there with `nosuid,nodev` before the workload enters its private mount +namespace. With several shares, it mounts the aggregate at `/run/nvx/shares`, +outside the container root, and binds each child at its target with its mode +and `nosuid,nodev`, in order, as the `nvx_share=NAME,TARGET,MODE` tokens that +NVX adds to the kernel command line describe. The number of shares is bounded +only by the sandbox's 1024-byte kernel command-line budget. A one-shot +workload exit, a managed `stop`, and any failure after a share is mounted +unmount the binds in reverse order, then the aggregate, before the overlay is +unmounted or the VM powers off. Each target must be an absolute, canonical +path; `/`, `/etc`, and the `/proc`, `/sys`, `/dev`, and `/.nvx-agent` trees are reserved for the container runtime. The guest refuses a target whose path crosses a symbolic link in a container -layer, a repeated tag, and overlapping targets, and any validation or mount +layer, a repeated child, and overlapping targets, and any validation or mount failure aborts the sandbox with status 125 instead of starting the workload without its shares. @@ -651,8 +672,9 @@ language toolchains create; see existing OpenVMM file-identity policy applies to each share. A managed sandbox stores each share's absolute host path, mode, denied, allowed, and writable paths and the ownership mode in its configuration, and reattaches -every share on each `start`. A configuration with two shares uses format 4, -which earlier NVX releases reject rather than start without the second share. +every share on each `start`. A configuration with several shares uses format +4, which earlier NVX releases reject rather than start without the other +shares. A configuration with allowed or writable paths uses format 5, which earlier NVX releases reject rather than start a share without its access policy. @@ -662,7 +684,9 @@ Each share can narrow what the workload can see and modify inside it. OpenVMM enforces the policy on the host for every request, whichever guest mount, path, or link reaches the share: -- `--mount-deny HOST_PATH` hides a path and everything below it. +- `--mount-deny HOST_PATH` hides a path and everything below it. It may name + the share's root itself when allowed paths inside it remain, so the share + exposes only those paths. - `--mount-allow HOST_PATH` exposes a path inside a denied path, and everything below it, again. The hidden directories on the way to it are traverse-only: the workload can enter and list them, but the listing shows @@ -679,6 +703,8 @@ or link reaches the share: Each path must exist inside the share's host directory, must not cross a symbolic link or junction, and is attributed to a share like `--mount-deny`. +Its names may contain spaces, but not begin or end with one, nor contain +other whitespace, colons, or backslashes. Allowed paths follow the share's write policy: without `--mount-write`, an allowed path in an `rw` share is writable; with it, an allowed path is writable only inside a writable path. For example, diff --git a/doc/usage.md b/doc/usage.md index 69902246..dc267362 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -501,10 +501,10 @@ python3 scripts/nvx.py run | `--memory-capacity-mib MIB` | none | Reserve an immutable, 128 MiB-aligned RAM capacity for a fresh microVM snapshot. | | `--processors {1,2,4,8}` | `1` | Select the microVM processor count. | | `--cpu-profile ID` | `auto` | Select the guest's [CPU profile](#cpu-profiles): `auto` for the built-in profile of the host's CPU, which falls back with a warning to a development profile derived from this host on an Intel or AMD CPU that no built-in profile serves, a built-in profile ID, or `host` for that development profile. A restore uses the snapshot's profile, which `auto` and, for a host profile, `host` also name. | -| `--mount GUEST_TARGET,HOST_PATH[,ro\|rw]` | none | Expose a host directory at the absolute guest target. Repeat once to expose a second directory with its own target and mode; targets and directories must not overlap. An `rw` mapping accepts guest-created symbolic links, which the host never follows. Active snapshot restore requires the same mappings in the same order, with the same canonical paths, targets, modes, and ownership mode; a dormant-slot restore may attach one new mapping that the resumed guest mounts explicitly. | -| `--mount-deny HOST_PATH` | none | Hide one existing file or directory inside a mounted host root; repeat to deny multiple paths. With two mappings, the path must be absolute. | -| `--mount-allow HOST_PATH` | none | Expose one existing file or directory inside a `--mount-deny` path again; the hidden directories on the way list only the entries that lead to it. Repeat to allow multiple paths. With two mappings, the path must be absolute. See [Access policy](run.md#access-policy). | -| `--mount-write HOST_PATH` | none | Make one existing file or directory one of the only writable parts of an `rw` mapping; the guest's writes anywhere else in it fail with `EROFS`. Repeat to declare multiple writable paths. With two mappings, the path must be absolute. Active snapshot restore requires the same denied, allowed, and writable paths. | +| `--mount GUEST_TARGET,HOST_PATH[,ro\|rw]` | none | Expose a host directory at the absolute guest target. Repeat to expose more directories, each with its own target and mode, as the children of an aggregate on the microVM's one virtio-fs device, which the guest binds at their targets; targets and directories must not overlap, and the shares' kernel command-line tokens must fit its 1024-byte budget. An `rw` mapping accepts guest-created symbolic links, which the host never follows. Active snapshot restore requires the same mappings in the same order, with the same canonical paths, targets, modes, and ownership mode; a dormant-slot restore may attach one new mapping that the resumed guest mounts explicitly. | +| `--mount-deny HOST_PATH` | none | Hide one existing file or directory inside a mounted host root; repeat to deny multiple paths. It may name the root itself when `--mount-allow` paths inside it remain. With several mappings, the path must be absolute. | +| `--mount-allow HOST_PATH` | none | Expose one existing file or directory inside a `--mount-deny` path again; the hidden directories on the way list only the entries that lead to it. Repeat to allow multiple paths. With several mappings, the path must be absolute. See [Access policy](run.md#access-policy). | +| `--mount-write HOST_PATH` | none | Make one existing file or directory one of the only writable parts of an `rw` mapping; the guest's writes anywhere else in it fail with `EROFS`. Repeat to declare multiple writable paths. With several mappings, the path must be absolute. Active snapshot restore requires the same denied, allowed, and writable paths. | | `--mount-owner {vmm,caller}` | `vmm` | Select the host identity of the guest's operations on every `--mount` directory. `caller` performs each one as the guest caller's UID and GID and squashes guest root to the directory owner; it requires a Linux host. See [Run](run.md#file-ownership). | | `--net IPV4/PREFIX` | none | Enable virtio-net with the static guest IPv4 address and prefix. The guest also gets the IPv6 address that embeds it in `fd00::/96`, such as `fd00::a00:2/120` for `10.0.0.2/24`. | | `--network-profile {portable}` | none | Select the required cross-platform network behavior contract; must be specified with `--net`. | @@ -609,10 +609,10 @@ launches. | `--environment-file PATH` | omitted | Read the exact managed `exec` environment from a UTF-8 JSON array of `KEY=VALUE` strings, limited to 1 MiB of input. An empty array requests an empty environment. This option is mutually exclusive with `--environment`; omitting both preserves guest defaults. | | `--inherit-default-environment` | off | Layer the managed `exec` environment from `--environment` or `--environment-file` over the guest default environment instead of replacing it; an entry replaces the default variable of the same name. Without either option the workload already receives the defaults, so the flag has no effect. | | `--hypervisor {auto,whp,kvm,mshv}` | `auto` | Select the host hypervisor. | -| `--mount GUEST_TARGET,HOST_PATH[,ro\|rw]` | none | Live-share a host directory at the absolute target inside the container rootfs for `run` or `provision`; defaults to `ro`. Repeat once to attach a second share with its own target and mode, for example a read-write workspace and a read-only tool cache; targets and host directories must not overlap. An `rw` share accepts guest-created symbolic links, which the host never follows. `/`, `/etc`, and the `/proc`, `/sys`, `/dev`, and `/.nvx-agent` trees are reserved. | -| `--mount-deny HOST_PATH` | none | Hide one existing file or directory inside a `--mount` host directory; relative paths are resolved inside it. With two shares, it applies to the `--mount` before it. Repeat to deny multiple paths. | -| `--mount-allow HOST_PATH` | none | Expose one existing file or directory inside a `--mount-deny` path of the same share again, for example a readable `mcp-payloads` directory inside hidden logs. The hidden directories on the way list only the entries that lead to it, and the workload cannot modify them. Relative paths are resolved inside the share; with two shares, it applies to the `--mount` before it. Repeat to allow multiple paths. See [Access policy](run.md#access-policy). | -| `--mount-write HOST_PATH` | none | Make one existing file or directory one of the only writable parts of an `rw` share; the workload's writes anywhere else in the share fail with `EROFS`. Relative paths are resolved inside the share; with two shares, it applies to the `--mount` before it, which must be `rw`. Repeat to declare multiple writable paths. `provision` persists the denied, allowed, and writable paths. | +| `--mount GUEST_TARGET,HOST_PATH[,ro\|rw]` | none | Live-share a host directory at the absolute target inside the container rootfs for `run` or `provision`; defaults to `ro`. Repeat to attach more shares, each with its own target and mode, for example a read-write workspace and a read-only tool cache, as the children of an aggregate that the guest binds at their targets; targets and host directories must not overlap, and the shares' kernel command-line tokens count toward the sandbox's 1024-byte budget. An `rw` share accepts guest-created symbolic links, which the host never follows. `/`, `/etc`, and the `/proc`, `/sys`, `/dev`, and `/.nvx-agent` trees are reserved. | +| `--mount-deny HOST_PATH` | none | Hide one existing file or directory inside a `--mount` host directory; relative paths are resolved inside it. It may name the directory itself when `--mount-allow` paths inside it remain. With several shares, it applies to the `--mount` before it. Repeat to deny multiple paths. | +| `--mount-allow HOST_PATH` | none | Expose one existing file or directory inside a `--mount-deny` path of the same share again, for example a readable `mcp-payloads` directory inside hidden logs. The hidden directories on the way list only the entries that lead to it, and the workload cannot modify them. Relative paths are resolved inside the share; with several shares, it applies to the `--mount` before it. Repeat to allow multiple paths. See [Access policy](run.md#access-policy). | +| `--mount-write HOST_PATH` | none | Make one existing file or directory one of the only writable parts of an `rw` share; the workload's writes anywhere else in the share fail with `EROFS`. Relative paths are resolved inside the share; with several shares, it applies to the `--mount` before it, which must be `rw`. Repeat to declare multiple writable paths. `provision` persists the denied, allowed, and writable paths. | | `--mount-owner {vmm,caller}` | `vmm` | Select the host identity of the shares' file operations for `run` or `provision`. `vmm` performs them as OpenVMM; `caller` performs them as the workload's UID and GID, squashes guest root to the owner of each host directory, and fails them with `EPERM` when OpenVMM cannot assume that identity. `caller` requires a Linux host and is persisted by `provision`. See [File ownership](run.md#file-ownership). | | `--net IPV4/PREFIX` | none | Enable virtio-net with a static guest address. The guest also gets the IPv6 address that embeds it in `fd00::/96`. | | `--network-profile {portable}` | none | Select the required cross-platform network behavior contract; must be specified with `--net`. | diff --git a/guest/common/nvx-hostmount b/guest/common/nvx-hostmount index d2be3680..dcf794ce 100644 --- a/guest/common/nvx-hostmount +++ b/guest/common/nvx-hostmount @@ -1,56 +1,97 @@ #!/bin/sh -mount_share() { - vdir=$1 - vtag=$2 - vmode=$3 - [ -n "$vtag" ] || return 0 - [ -n "$vdir" ] || vdir=/mnt/host - [ -n "$vmode" ] || vmode=ro - mkdir -p "$vdir" - if ! mount -t virtiofs -o "$vmode" "$vtag" "$vdir" 2>/dev/null; then - echo "virtfs: failed to mount tag $vtag at $vdir" >&2 - return 1 - fi - echo "virtfs: mounted live host directory at $vdir (virtiofs,$vmode)" +# Mounts the live host share that OpenVMM's virtfs_dir=, virtfs_tag=, and +# virtfs_mode= tokens describe. When virtfs_aggregate=1 marks it as an +# aggregate of several host directories, its root lists one child per +# directory, which only root can enter, and each nvx_share=CHILD,TARGET,MODE +# token binds one child at TARGET with MODE, without set-user-ID programs or +# device nodes. With --unmount, the binds and then the share are unmounted. + +fail() { + echo "virtfs: $*" >&2 + exit 1 } -# OpenVMM appends one virtfs_dir=, virtfs_tag=, virtfs_mode= triplet per -# attached share, in virtio-fs slot order; a repeated field starts the next. vdir= vtag= vmode= +aggregate=false +shares= cmdline=$(cat /proc/cmdline) # Kernel command-line tokens are deliberately split on spaces. # shellcheck disable=SC2086 set -f for token in $cmdline; do case "$token" in - virtfs_dir=*) - if [ -n "$vdir" ]; then - mount_share "$vdir" "$vtag" "$vmode" || exit 1 - vtag= - vmode= - fi - vdir=${token#*=} - ;; - virtfs_tag=*) - if [ -n "$vtag" ]; then - mount_share "$vdir" "$vtag" "$vmode" || exit 1 - vdir= - vmode= - fi - vtag=${token#*=} - ;; - virtfs_mode=*) - if [ -n "$vmode" ]; then - mount_share "$vdir" "$vtag" "$vmode" || exit 1 - vdir= - vtag= - fi - vmode=${token#*=} - ;; + virtfs_dir=*) vdir=${token#*=} ;; + virtfs_tag=*) vtag=${token#*=} ;; + virtfs_mode=*) vmode=${token#*=} ;; + virtfs_aggregate=1) aggregate=true ;; + nvx_share=*) shares="$shares ${token#*=}" ;; + esac +done +set +f +[ -n "$vtag" ] || exit 0 +[ -n "$vdir" ] || vdir=/mnt/host +[ -n "$vmode" ] || vmode=ro +[ "$aggregate" = true ] || shares= + +# Splits the share specification $1 into share_child, share_target, and +# share_mode. +parse_share() { + share_child=${1%%,*} + share_rest=${1#*,} + share_target=${share_rest%,*} + share_mode=${share_rest##*,} + case "$share_child" in + '' | . | .. | *[!A-Za-z0-9._-]*) fail "invalid live share $1" ;; esac + case "$share_mode" in ro | rw) ;; *) fail "invalid live share mode in $1" ;; esac + case "$share_target" in /?*) ;; *) fail "invalid live share target in $1" ;; esac + [ "$share_rest" != "$1" ] && [ "$share_target" != "$share_rest" ] || + fail "invalid live share $1" +} + +if [ "${1:-}" = --unmount ]; then + status=0 + reversed= + set -f + for share in $shares; do + reversed="$share $reversed" + done + for share in $reversed; do + set +f + parse_share "$share" + if mountpoint -q "$share_target" && ! umount "$share_target"; then + echo "virtfs: failed to unmount live share $share_child at $share_target" >&2 + status=1 + fi + set -f + done + set +f + if mountpoint -q "$vdir" && ! umount "$vdir"; then + echo "virtfs: failed to unmount tag $vtag at $vdir" >&2 + status=1 + fi + exit "$status" +fi + +mkdir -p "$vdir" +if ! mount -t virtiofs -o "$vmode" "$vtag" "$vdir" 2>/dev/null; then + fail "failed to mount tag $vtag at $vdir" +fi +echo "virtfs: mounted live host directory at $vdir (virtiofs,$vmode)" +set -f +for share in $shares; do + set +f + parse_share "$share" + [ -d "$vdir/$share_child" ] || fail "live share $share_child is not exported" + if ! mkdir -p "$share_target" || + ! mount -o bind "$vdir/$share_child" "$share_target" || + ! mount -o "remount,bind,$share_mode,nosuid,nodev" "$share_target"; then + fail "failed to bind live share $share_child at $share_target" + fi + echo "virtfs: bound live share $share_child at $share_target (virtiofs,$share_mode)" + set -f done set +f -mount_share "$vdir" "$vtag" "$vmode" diff --git a/guest/common/nvx-init-agent b/guest/common/nvx-init-agent index 8322bbd9..02da2762 100644 --- a/guest/common/nvx-init-agent +++ b/guest/common/nvx-init-agent @@ -168,43 +168,12 @@ validate_share_target() { esac } -mount_live_share() { - share_tag=$1 - share_target=$2 - share_mode=$3 - case "$share_tag" in - '' | *[!A-Za-z0-9_-]*) fatal "live share tag is invalid" ;; - esac - case "$share_mode" in - ro | rw) ;; - *) fatal "live share mode is unsupported: $share_mode" ;; - esac - validate_share_target "$share_target" - # Each share has its own tag, and no share may hide or shadow another. - case " $share_tags " in - *" $share_tag "*) fatal "live share tag is not unique: $share_tag" ;; - esac - set -f - for mounted_target in $share_targets; do - case "$share_target/" in - "$mounted_target"/*) - set +f - fatal "live share targets overlap: $mounted_target and $share_target" - ;; - esac - case "$mounted_target/" in - "$share_target"/*) - set +f - fatal "live share targets overlap: $mounted_target and $share_target" - ;; - esac - done - set +f - - # Create the mount point one component at a time so that a symbolic link in - # a container layer cannot redirect the share outside the container rootfs. +# Creates the mount point of the live share target $1 inside the container +# rootfs one component at a time, so that a symbolic link in a container layer +# cannot redirect the share outside that rootfs, and sets share_path to it. +make_share_mount_point() { share_path=$rootfs - share_remainder=${share_target#/} + share_remainder=${1#/} while [ -n "$share_remainder" ]; do share_component=${share_remainder%%/*} if [ "$share_component" = "$share_remainder" ]; then @@ -214,64 +183,187 @@ mount_live_share() { fi share_path=$share_path/$share_component if [ -L "$share_path" ]; then - fatal "live share target crosses a symbolic link: $share_target" + fatal "live share target crosses a symbolic link: $1" elif [ -e "$share_path" ]; then [ -d "$share_path" ] || - fatal "live share target is not a directory: $share_target" + fatal "live share target is not a directory: $1" else mkdir -m 0755 "$share_path" || - fatal "failed to create live share target $share_target" + fatal "failed to create live share target $1" fi done +} + +# Records a mounted live share for teardown, most recent first. +record_live_share() { + share_mountpoints="$1${share_mountpoints:+ +$share_mountpoints}" +} + +# Checks that the live share target $1 is valid and that no other share hides +# or shadows it. +claim_share_target() { + validate_share_target "$1" + set -f + for mounted_target in $share_targets; do + case "$1/" in + "$mounted_target"/*) + set +f + fatal "live share targets overlap: $mounted_target and $1" + ;; + esac + case "$mounted_target/" in + "$1"/*) + set +f + fatal "live share targets overlap: $mounted_target and $1" + ;; + esac + done + set +f + share_targets="$share_targets $1" +} + +mount_live_share() { + share_tag=$1 + share_target=$2 + share_mode=$3 + case "$share_tag" in + '' | *[!A-Za-z0-9_-]*) fatal "live share tag is invalid" ;; + esac + case "$share_mode" in + ro | rw) ;; + *) fatal "live share mode is unsupported: $share_mode" ;; + esac + claim_share_target "$share_target" + make_share_mount_point "$share_target" mount -t virtiofs -o "$share_mode,nosuid,nodev" "$share_tag" "$share_path" || fatal "failed to mount live share $share_tag at $share_target" - share_mountpoints="$share_path${share_mountpoints:+ -$share_mountpoints}" - share_tags="$share_tags $share_tag" - share_targets="$share_targets $share_target" + record_live_share "$share_path" echo "NVX-SANDBOX-SHARE: target=$share_target mode=$share_mode tag=$share_tag" } +# Splits the aggregate child specification $1, CHILD,TARGET,MODE, into +# child_name, child_target, and child_mode. +parse_share_child() { + child_name=${1%%,*} + child_rest=${1#*,} + child_target=${child_rest%,*} + child_mode=${child_rest##*,} + if [ "$child_rest" = "$1" ] || [ "$child_target" = "$child_rest" ]; then + fatal "live share child is invalid: $1" + fi + case "$child_name" in + '' | . | .. | *[!A-Za-z0-9._-]*) fatal "live share child is invalid: $1" ;; + esac + case "$child_mode" in + ro | rw) ;; + *) fatal "live share mode is unsupported: $child_mode" ;; + esac +} + +# Binds child $2 of the aggregate share mounted at $1 at the target $3 inside +# the container rootfs, with mode $4 and without set-user-ID programs or +# device nodes. +bind_live_share() { + make_share_mount_point "$3" + mount -o bind "$1/$2" "$share_path" || + fatal "failed to bind live share $2 at $3" + record_live_share "$share_path" + mount -o "remount,bind,$4,nosuid,nodev" "$share_path" || + fatal "failed to restrict live share $2 at $3" + echo "NVX-SANDBOX-SHARE: target=$3 mode=$4 child=$2" +} + mount_live_shares() { - # OpenVMM appends one virtfs_dir=, virtfs_tag=, virtfs_mode= triplet per - # attached share, in virtio-fs slot order. Parse them all before mounting - # any, so that a malformed bootstrap mounts nothing. - share_specs= - share_field=dir + # OpenVMM appends one virtfs_dir=, virtfs_tag=, virtfs_mode= triplet for + # its virtio-fs share, and virtfs_aggregate=1 when that share is an + # aggregate of several host directories, which nvx_share=CHILD,TARGET,MODE + # tokens place. Parse every token before mounting anything, so that a + # malformed bootstrap mounts nothing. + share_dir= + share_tag= + share_mode= + share_fields=0 + share_aggregate=false + share_children= # Kernel command-line tokens are deliberately split on spaces. # shellcheck disable=SC2086 set -f for token in $cmdline; do - case "$share_field:$token" in - dir:virtfs_dir=?*) - share_target=${token#*=} - share_field=tag + case "$token" in + virtfs_dir=?*) + share_dir=${token#*=} + share_fields=$((share_fields + 1)) ;; - tag:virtfs_tag=?*) + virtfs_tag=?*) share_tag=${token#*=} - share_field=mode + share_fields=$((share_fields + 1)) ;; - mode:virtfs_mode=?*) - share_specs="$share_specs$share_tag $share_target ${token#*=} -" - share_field=dir + virtfs_mode=?*) + share_mode=${token#*=} + share_fields=$((share_fields + 1)) ;; - *:virtfs_dir=* | *:virtfs_tag=* | *:virtfs_mode=*) + virtfs_dir=* | virtfs_tag=* | virtfs_mode=*) set +f fatal "live share bootstrap is incomplete" ;; + virtfs_aggregate=1) share_aggregate=true ;; + nvx_share=*) share_children="$share_children ${token#*=}" ;; esac done set +f - [ "$share_field" = dir ] || fatal "live share bootstrap is incomplete" - share_tags= + if [ "$share_fields" -eq 0 ] && [ "$share_aggregate" = false ] && + [ -z "$share_children" ]; then + return 0 + fi + [ "$share_fields" -eq 3 ] && [ -n "$share_dir" ] && [ -n "$share_tag" ] && + [ -n "$share_mode" ] || fatal "live share bootstrap is incomplete" share_targets= - while read -r share_tag share_target share_mode; do - [ -n "$share_tag" ] || continue - mount_live_share "$share_tag" "$share_target" "$share_mode" - done </dev/hvc0 2>&1 || fatal virtiofs-mount-failed fi - seed=$mount_dir/.nvx-virtio-restore-host-seed - result=$mount_dir/.nvx-virtio-restore-guest-result + # An aggregate's root lists its host directories, so the probe uses the + # first, at the target where nvx-hostmount binds it. + io_dir=$mount_dir + if [ "$(cmdline_value virtfs_aggregate || true)" = 1 ]; then + first_share=$(cmdline_value nvx_share || true) + first_share=${first_share#*,} + io_dir=${first_share%,*} + [ -n "$io_dir" ] || fatal virtiofs-share-missing + fi + seed=$io_dir/.nvx-virtio-restore-host-seed + result=$io_dir/.nvx-virtio-restore-guest-result [ -f "$seed" ] || fatal virtiofs-host-seed-missing [ "$(cat "$seed")" = host-to-guest ] || fatal virtiofs-host-seed-invalid printf 'guest-to-host\n' >"$result" || fatal virtiofs-write-failed @@ -196,9 +205,9 @@ emit_marker "NVX-VIRTIO-RESTORE-PROBE phase=pre-io-success device=$device mode=$ if [ "$mode" = deferred ]; then if [ "$device" = virtiofs ]; then - mount_dir=$(cmdline_value virtfs_dir || true) - [ -n "$mount_dir" ] || mount_dir=/mnt/host - umount "$mount_dir" || fatal virtiofs-unmount-failed + # Unmount every bind of an aggregate before the share itself, so the + # device has no file system left when its driver lets go of it. + /sbin/nvx-hostmount --unmount >/dev/hvc0 2>&1 || fatal virtiofs-unmount-failed fi unbind_device fi diff --git a/scripts/nvx.py b/scripts/nvx.py index c25996f2..10b799ef 100644 --- a/scripts/nvx.py +++ b/scripts/nvx.py @@ -88,12 +88,16 @@ verify_source_tree, ) from nvx_tools.sandbox import ( - MAX_MOUNTS, + AGGREGATE_MOUNT_TARGET, MOUNT_OWNERS, MOUNT_POLICY_OPTIONS, + SANDBOX_COMMAND_LINE_MAX_SIZE, SandboxLaunch, SandboxLayer, SandboxMount, + aggregate_child_name, + aggregate_command_line_fragment, + aggregate_share_tokens, parse_workload_identity, require_mount_owner_supported, ) @@ -176,6 +180,46 @@ def _sandbox_mounts(args: argparse.Namespace) -> tuple[SandboxMount, ...]: ) +def _run_share(value: str) -> tuple[str, str, str]: + """Split a run --mount into its guest target, host path, and access.""" + fields = value.split(",") + if len(fields) not in (2, 3): + raise ScriptError("--mount must be GUEST_TARGET,HOST_PATH[,ro|rw]") + target, host_path = fields[:2] + access = fields[2] if len(fields) == 3 else "ro" + if access not in ("ro", "rw"): + raise ScriptError(f"unsupported --mount mode {access!r}; choose ro or rw") + return target, host_path, access + + +def _validate_run_share_targets(targets: list[str]) -> None: + """Reject guest targets that the guest cannot bind from an aggregate: + targets that do not survive the kernel command line, or that hide each + other or the aggregate itself.""" + for index, target in enumerate(targets): + if ( + not target.startswith("/") + or target == "/" + or any(character.isspace() or character in "\0\\=," for character in target) + or any(part in ("", ".", "..") for part in target.split("/")[1:]) + ): + raise ScriptError( + f"invalid --mount target {target!r}: with several shares, each " + "target must be an absolute non-root path without empty, dot, or " + "parent components, whitespace, '\\', '=', or ','" + ) + for other in (AGGREGATE_MOUNT_TARGET, *targets[:index]): + if ( + target == other + or target.startswith(f"{other}/") + or other.startswith(f"{target}/") + ): + raise ScriptError( + f"--mount targets {other} and {target} overlap; a share " + "cannot hide another" + ) + + def _run( args: list[str | os.PathLike[str]], *, cwd: Path = BuildConstants.REPO_ROOT ) -> None: @@ -515,16 +559,28 @@ def command_run(args: argparse.Namespace) -> None: command.extend(["--memory-capacity", f"{args.memory_capacity_mib}M"]) if args.cpu_profile is not None: command.extend(["--cpu-profile", args.cpu_profile]) - if len(args.mount) > MAX_MOUNTS: - raise ScriptError(f"--mount is accepted at most {MAX_MOUNTS} times") - for mount in args.mount: - if mount.count(",") not in (1, 2): - raise ScriptError("--mount must be GUEST_TARGET,HOST_PATH[,ro|rw]") - command.extend(["--mount", mount]) + shares = [_run_share(value) for value in args.mount] + if len(shares) == 1: + command.extend(["--mount", args.mount[0]]) + elif shares: + _validate_run_share_targets([target for target, _, _ in shares]) + command.extend(["--mount-aggregate", AGGREGATE_MOUNT_TARGET]) + # A restore names the children as the boot did, so OpenVMM refuses a + # restore whose targets differ from those that the guest bound. + for index, (target, host_path, access) in enumerate(shares): + name = aggregate_child_name(index, target) + command.extend(["--mount-child", f"{name},{host_path},{access}"]) if args.mount_owner is not None and not args.mount: raise ScriptError("--mount-owner requires --mount") for option in MOUNT_POLICY_OPTIONS.values(): for path in getattr(args, _mount_policy_destination(option)): + # OpenVMM attributes each path of several shares to the child whose + # host directory contains it. + if len(shares) > 1 and not Path(path).is_absolute(): + raise ScriptError( + f"with several --mount shares, {option} {path} must be an " + "absolute host path" + ) command.extend([option, str(path)]) if args.mount_owner is not None: require_mount_owner_supported(args.mount_owner) @@ -537,8 +593,30 @@ def command_run(args: argparse.Namespace) -> None: ) if args.outcome_report is not None: command.extend(["--microvm-report", str(args.outcome_report)]) - if args.cmdline: - command.extend(["--cmdline", args.cmdline]) + cmdline = args.cmdline + # A restored guest keeps the binds that its boot made, at the targets that + # its children's names pin. + if len(shares) > 1 and args.restore_snapshot is None: + cmdline = " ".join( + token + for token in ( + args.cmdline, + *aggregate_share_tokens( + [(target, access) for target, _, access in shares] + ), + ) + if token + ) + fragment = aggregate_command_line_fragment([access for _, _, access in shares]) + if len(cmdline.encode()) + len(fragment.encode()) + 1 > ( + SANDBOX_COMMAND_LINE_MAX_SIZE + ): + raise ScriptError( + "the --mount shares and --cmdline exceed the 1024-byte budget of " + "the kernel command line; attach fewer shares or use shorter targets" + ) + if cmdline: + command.extend(["--cmdline", cmdline]) print(f">> {_format_command(command)}") if not args.dry_run: raise SystemExit( @@ -1072,8 +1150,8 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: default=[], metavar="GUEST_TARGET,HOST_PATH[,ro|rw]", help=( - "live-share a host directory; repeat once for a second share with " - "its own guest target and mode" + "live-share a host directory; repeat for more shares, each with its " + "own guest target and mode, as children of one virtio-fs device" ), ) run.add_argument("--mount-deny", action="append", type=Path, default=[]) @@ -1234,8 +1312,8 @@ def sandbox_identity(value: str) -> tuple[int, int]: metavar="GUEST_TARGET,HOST_PATH[,ro|rw]", help=( "live-share a host directory inside the container rootfs; repeat " - f"to attach up to {MAX_MOUNTS} shares, each with its own target and " - "mode" + "to attach more shares, each with its own target and mode, as " + "children of one virtio-fs device" ), ) sandbox.add_argument( diff --git a/scripts/nvx_tools/sandbox.py b/scripts/nvx_tools/sandbox.py index c986b6bf..f410457b 100644 --- a/scripts/nvx_tools/sandbox.py +++ b/scripts/nvx_tools/sandbox.py @@ -2,8 +2,10 @@ from __future__ import annotations +import hashlib import os import re +from collections.abc import Sequence from dataclasses import dataclass from pathlib import Path from uuid import UUID @@ -25,10 +27,13 @@ _HOSTNAME = re.compile(r"(?=^.{1,63}$)(?!-)[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?$") DEFAULT_WORKLOAD_IDENTITY = (65534, 65534) MOUNT_ACCESS_MODES = ("ro", "rw") -# OpenVMM attaches each live share to the next fixed virtio-fs slot, which has -# its own guest-visible tag. -MOUNT_TAGS = ("microvm", "microvm1") -MAX_MOUNTS = len(MOUNT_TAGS) +# OpenVMM attaches one virtio-fs device, tag `microvm`. One share uses it +# directly. Several share it as the children of an aggregate, named by +# aggregate_child_name, which the guest mounts at AGGREGATE_MOUNT_TARGET, a +# directory that only the guest's root can enter, before it binds each child at +# its target. +MOUNT_TAG = "microvm" +AGGREGATE_MOUNT_TARGET = "/run/nvx/shares" # `vmm` performs every share operation as OpenVMM; `caller` performs each as the # guest caller's identity, with guest root squashed to the share owner. MOUNT_OWNERS = ("vmm", "caller") @@ -48,8 +53,10 @@ "allowed": "--mount-allow", "writable": "--mount-write", } -# OpenVMM appends exactly these virtio-fs bootstrap tokens for each live share. +# OpenVMM appends exactly these virtio-fs bootstrap tokens for its share, and +# the suffix when the share is an aggregate. _MOUNT_COMMAND_LINE_FRAGMENT = " virtfs_dir={} virtfs_tag={} virtfs_mode={}" +_AGGREGATE_COMMAND_LINE_SUFFIX = " virtfs_aggregate=1" def parse_workload_identity(value: str) -> tuple[int, int]: @@ -265,8 +272,10 @@ def absolute_policy_paths(self) -> tuple[tuple[str, tuple[str, ...]], ...]: (kind, self._absolute_paths(paths)) for kind, paths in self.policy_paths() ) - def command_line_fragment(self, tag: str = MOUNT_TAGS[0]) -> str: - return _MOUNT_COMMAND_LINE_FRAGMENT.format(self.guest_target, tag, self.access) + def command_line_fragment(self) -> str: + return _MOUNT_COMMAND_LINE_FRAGMENT.format( + self.guest_target, MOUNT_TAG, self.access + ) def _targets_overlap(left: str, right: str) -> bool: @@ -274,9 +283,7 @@ def _targets_overlap(left: str, right: str) -> bool: def validate_mounts(mounts: tuple[SandboxMount, ...]) -> None: - """Validate the live shares that one sandbox attaches, in slot order.""" - if len(mounts) > MAX_MOUNTS: - raise ScriptError(f"a sandbox permits at most {MAX_MOUNTS} --mount options") + """Validate the live shares that one sandbox attaches, in child order.""" for index, mount in enumerate(mounts): for other in mounts[:index]: if _targets_overlap(other.guest_target, mount.guest_target): @@ -326,27 +333,55 @@ def validate_mount_host_paths(mounts: tuple[SandboxMount, ...]) -> None: for kind, paths in mount.absolute_policy_paths(): option = MOUNT_POLICY_OPTIONS[kind] for path in paths: - if root not in Path(path).resolve().parents: - raise ScriptError( - f"{option} {path} is not inside the host directory of " - f"--mount {mount.guest_target}; with several shares, each " - f"{option} follows the --mount whose directory it names" - ) + resolved = Path(path).resolve() + # A denied root hides the share behind its allowed paths, which + # OpenVMM requires; it accepts no other policy path at a root. + if root in resolved.parents or (kind == "denied" and resolved == root): + continue + raise ScriptError( + f"{option} {path} is not inside the host directory of " + f"--mount {mount.guest_target}; with several shares, each " + f"{option} follows the --mount whose directory it names" + ) + + +def aggregate_child_name(index: int, target: str) -> str: + """Return the name of the aggregate child at `index` that the guest binds + at `target`. + + The name holds a digest of the target. A snapshot records every child's + name, so it pins each target, and OpenVMM refuses a restore that requests + other targets before the guest runs, rather than keep the snapshot's binds. + """ + digest = hashlib.sha256(target.encode()).hexdigest()[:32] + return f"{index}-{digest}" def mounts_openvmm_arguments(mounts: tuple[SandboxMount, ...]) -> list[str]: - """Return the OpenVMM arguments that attach `mounts` in slot order.""" + """Return the OpenVMM arguments that attach `mounts`: one share directly, + and several as the children of an aggregate, in order.""" arguments: list[str] = [] - for mount in mounts: + if len(mounts) == 1: + mount = mounts[0] arguments.extend( ( "--mount", f"{mount.guest_target},{os.fspath(mount.host_path)},{mount.access}", ) ) + elif mounts: + arguments.extend(("--mount-aggregate", AGGREGATE_MOUNT_TARGET)) + for index, mount in enumerate(mounts): + name = aggregate_child_name(index, mount.guest_target) + arguments.extend( + ( + "--mount-child", + f"{name},{os.fspath(mount.host_path)},{mount.access}", + ) + ) for mount in mounts: # OpenVMM resolves a relative policy path in the only share, and - # attributes absolute policy paths to the share that contains them. + # attributes absolute policy paths to the child that contains them. policy_paths = ( mount.policy_paths() if len(mounts) == 1 else mount.absolute_policy_paths() ) @@ -358,12 +393,35 @@ def mounts_openvmm_arguments(mounts: tuple[SandboxMount, ...]) -> list[str]: return arguments +def aggregate_share_tokens(shares: Sequence[tuple[str, str]]) -> list[str]: + """Return the kernel command-line tokens that tell the guest where to bind + each child of an aggregate, given each share's guest target and access, or + none for a single share.""" + if len(shares) < 2: + return [] + return [ + f"nvx_share={aggregate_child_name(index, target)},{target},{access}" + for index, (target, access) in enumerate(shares) + ] + + +def aggregate_command_line_fragment(accesses: Sequence[str]) -> str: + """Return the bootstrap tokens that OpenVMM appends for an aggregate of + shares with `accesses`, which it mounts read-write if any child is.""" + access = "rw" if "rw" in accesses else "ro" + return ( + _MOUNT_COMMAND_LINE_FRAGMENT.format(AGGREGATE_MOUNT_TARGET, MOUNT_TAG, access) + + _AGGREGATE_COMMAND_LINE_SUFFIX + ) + + def mounts_command_line_fragment(mounts: tuple[SandboxMount, ...]) -> str: """Return the bootstrap tokens that OpenVMM appends for `mounts`.""" - return "".join( - mount.command_line_fragment(tag) - for mount, tag in zip(mounts, MOUNT_TAGS[: len(mounts)], strict=True) - ) + if len(mounts) == 1: + return mounts[0].command_line_fragment() + if not mounts: + return "" + return aggregate_command_line_fragment([mount.access for mount in mounts]) @dataclass(frozen=True) @@ -478,6 +536,11 @@ def kernel_command_line(self, user_command_line: str = "") -> str: tokens.append(f"nvx_memory_max={self.memory_max}") if self.pids_max is not None: tokens.append(f"nvx_pids_max={self.pids_max + 1}") + tokens.extend( + aggregate_share_tokens( + [(mount.guest_target, mount.access) for mount in self.mounts] + ) + ) command_line = " ".join(token for token in tokens if token) # OpenVMM appends the live-share bootstrap tokens after this command line, # so they consume the same x86 budget. @@ -487,7 +550,8 @@ def kernel_command_line(self, user_command_line: str = "") -> str: > SANDBOX_COMMAND_LINE_MAX_SIZE ): raise ScriptError( - "sandbox kernel command line exceeds its 1024-byte x86 budget" + "sandbox kernel command line exceeds its 1024-byte x86 budget; " + "attach fewer --mount shares or use shorter targets and arguments" ) return command_line diff --git a/scripts/test_nvx_tools.py b/scripts/test_nvx_tools.py index 55ddf7a1..a338d5e5 100644 --- a/scripts/test_nvx_tools.py +++ b/scripts/test_nvx_tools.py @@ -8832,21 +8832,40 @@ def setUp(self): self.root = Path(temporary.name) self.rootfs = self.root / "rootfs" self.rootfs.mkdir() + self.runtime = (self.root / "run").as_posix() + + SHARE_FUNCTIONS = ( + "validate_share_target", + "make_share_mount_point", + "record_live_share", + "claim_share_target", + "mount_live_share", + "parse_share_child", + "bind_live_share", + "mount_live_shares", + ) + + def _aggregate(self, *children: str, mode: str = "rw") -> str: + """Returns the bootstrap of an aggregate whose children the + CHILD,TARGET,MODE specifications `children` place.""" + tokens = [ + f"virtfs_dir={self.runtime}/shares", + "virtfs_tag=microvm", + f"virtfs_mode={mode}", + "virtfs_aggregate=1", + *(f"nvx_share={child}" for child in children), + ] + return " ".join(tokens) def _run(self, cmdline: str) -> tuple[subprocess.CompletedProcess[str], str]: mount_log = self.root / "mount.log" mount_log.unlink(missing_ok=True) functions = "".join( - _shell_function(self.source, name) - for name in ( - "validate_share_target", - "mount_live_share", - "mount_live_shares", - ) + _shell_function(self.source, name) for name in self.SHARE_FUNCTIONS ) script = ( "set -eu\n" - "rootfs=$1\ncmdline=$2\nmount_log=$3\nshare_mountpoints=\n" + "rootfs=$1\ncmdline=$2\nmount_log=$3\nruntime=$4\nshare_mountpoints=\n" 'fatal() { echo "FATAL: $*" >&2; exit 125; }\n' 'mount() { printf "%s\\n" "$*" >>"$mount_log"; }\n' f"{functions}" @@ -8861,6 +8880,7 @@ def _run(self, cmdline: str) -> tuple[subprocess.CompletedProcess[str], str]: self.rootfs.as_posix(), cmdline, mount_log.as_posix(), + self.runtime, ], input=script, text=True, @@ -9063,77 +9083,135 @@ def test_agent_mounts_share_inside_container_rootfs(self): result.stdout, ) - def test_agent_mounts_every_share_in_slot_order(self): + def test_agent_binds_every_child_of_an_aggregate(self): rootfs = self.rootfs.as_posix() result, log = self._run( - "nvx_sandbox=1 virtfs_dir=/workspace virtfs_tag=microvm virtfs_mode=rw " - "virtfs_dir=/opt/hostedtoolcache virtfs_tag=microvm1 virtfs_mode=ro" + "nvx_sandbox=1 " + + self._aggregate("0,/workspace,rw", "1,/opt/hostedtoolcache,ro") ) self.assertEqual(result.returncode, 0, result.stderr) + # The aggregate stays outside the container rootfs; each child is bound + # inside it with its own mode. self.assertEqual( log.splitlines(), [ - f"-t virtiofs -o rw,nosuid,nodev microvm {rootfs}/workspace", - f"-t virtiofs -o ro,nosuid,nodev microvm1 {rootfs}/opt/hostedtoolcache", + f"-t virtiofs -o rw,nosuid,nodev microvm {self.runtime}/shares", + f"-o bind {self.runtime}/shares/0 {rootfs}/workspace", + f"-o remount,bind,rw,nosuid,nodev {rootfs}/workspace", + f"-o bind {self.runtime}/shares/1 {rootfs}/opt/hostedtoolcache", + f"-o remount,bind,ro,nosuid,nodev {rootfs}/opt/hostedtoolcache", ], ) self.assertIn( - "NVX-SANDBOX-SHARE: target=/workspace mode=rw tag=microvm\n" - "NVX-SANDBOX-SHARE: target=/opt/hostedtoolcache mode=ro tag=microvm1\n", + "NVX-SANDBOX-SHARE: target=/workspace mode=rw child=0\n" + "NVX-SANDBOX-SHARE: target=/opt/hostedtoolcache mode=ro child=1\n", result.stdout, ) - # Teardown unmounts the most recent share first. + # Teardown unmounts the most recent bind first, and the aggregate last. self.assertIn( - f"mountpoints={rootfs}/opt/hostedtoolcache {rootfs}/workspace\n", + f"mountpoints={rootfs}/opt/hostedtoolcache {rootfs}/workspace " + f"{self.runtime}/shares\n", result.stdout, ) - def test_agent_rejects_colliding_tags_and_overlapping_targets(self): - for cmdline, message in ( - ( - "virtfs_dir=/workspace virtfs_tag=microvm virtfs_mode=rw " - "virtfs_dir=/opt/tools virtfs_tag=microvm virtfs_mode=ro", - "tag is not unique", - ), - ( - "virtfs_dir=/workspace virtfs_tag=microvm virtfs_mode=rw " - "virtfs_dir=/workspace virtfs_tag=microvm1 virtfs_mode=ro", - "targets overlap", - ), - ( - "virtfs_dir=/workspace virtfs_tag=microvm virtfs_mode=rw " - "virtfs_dir=/workspace/cache virtfs_tag=microvm1 virtfs_mode=ro", - "targets overlap", - ), - ( - "virtfs_dir=/opt/tools/node virtfs_tag=microvm virtfs_mode=ro " - "virtfs_dir=/opt virtfs_tag=microvm1 virtfs_mode=rw", - "targets overlap", - ), + def test_agent_binds_children_by_the_names_that_nvx_gives_them(self): + rootfs = self.rootfs.as_posix() + workspace = sandbox.aggregate_child_name(0, "/workspace") + tools = sandbox.aggregate_child_name(1, "/opt/tools") + result, log = self._run( + self._aggregate( + f"{workspace},/workspace,rw", f"{tools},/opt/tools,ro", "a.b_C-9,/x,ro" + ) + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn( + f"-o bind {self.runtime}/shares/{workspace} {rootfs}/workspace\n", log + ) + self.assertIn( + f"-o bind {self.runtime}/shares/{tools} {rootfs}/opt/tools\n", log + ) + self.assertIn(f"-o bind {self.runtime}/shares/a.b_C-9 {rootfs}/x\n", log) + self.assertIn( + f"NVX-SANDBOX-SHARE: target=/workspace mode=rw child={workspace}\n", + result.stdout, + ) + # A repeated name is refused, however the child is named. + result, log = self._run( + self._aggregate(f"{workspace},/workspace,rw", f"{workspace},/b,ro") + ) + self.assertEqual(result.returncode, 125, result.stdout) + self.assertIn("child is not unique", result.stderr) + self.assertEqual(log, "") + + def test_hostmount_accepts_only_child_names_inside_the_aggregate(self): + hostmount = self.AGENT.with_name("nvx-hostmount").read_text(encoding="utf-8") + script = ( + 'fail() { echo "FAIL: $*" >&2; exit 1; }\n' + f"{_shell_function(hostmount, 'parse_share')}" + 'parse_share "$1"\n' + 'printf "%s|%s|%s\\n" "$share_child" "$share_target" "$share_mode"\n' + ) + + def parse(share: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [self.shell, "-s", "--", share], + input=script, + text=True, + capture_output=True, + check=False, + ) + + name = sandbox.aggregate_child_name(0, "/workspace") + for share, expected in ( + (f"{name},/workspace,rw", f"{name}|/workspace|rw\n"), + ("0,/opt/tools,ro", "0|/opt/tools|ro\n"), + ("a.b_C-9,/x,ro", "a.b_C-9|/x|ro\n"), ): - with self.subTest(cmdline=cmdline): - result, log = self._run(cmdline) + with self.subTest(share=share): + result = parse(share) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, expected) + for share in ( + ",/workspace,rw", + ".,/workspace,rw", + "..,/workspace,rw", + "a/b,/workspace,rw", + "a:b,/workspace,rw", + "a*,/workspace,rw", + ): + with self.subTest(share=share): + result = parse(share) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("invalid live share", result.stderr) + + def test_agent_rejects_repeated_children_and_overlapping_targets(self): + for children, message in ( + (("0,/workspace,rw", "0,/opt/tools,ro"), "child is not unique"), + (("0,/workspace,rw", "1,/workspace,ro"), "targets overlap"), + (("0,/workspace,rw", "1,/workspace/cache,ro"), "targets overlap"), + (("0,/opt/tools/node,ro", "1,/opt,rw"), "targets overlap"), + ): + with self.subTest(children=children): + result, log = self._run(self._aggregate(*children)) self.assertEqual(result.returncode, 125, result.stdout) self.assertIn(message, result.stderr) - # Only the first, valid share was mounted. - self.assertEqual(len(log.splitlines()), 1) + # Every child is checked before anything is mounted. + self.assertEqual(log, "") # A shared prefix that is not a path component is not an overlap. - result, log = self._run( - "virtfs_dir=/work virtfs_tag=microvm virtfs_mode=rw " - "virtfs_dir=/workspace virtfs_tag=microvm1 virtfs_mode=ro" - ) + result, log = self._run(self._aggregate("0,/work,rw", "1,/workspace,ro")) self.assertEqual(result.returncode, 0, result.stderr) - self.assertEqual(len(log.splitlines()), 2) + self.assertEqual(len(log.splitlines()), 5) def test_agent_parses_every_share_before_mounting_any(self): for cmdline in ( "virtfs_dir=/workspace virtfs_tag=microvm virtfs_mode=rw " - "virtfs_dir=/opt/tools virtfs_tag=microvm1", + "virtfs_dir=/opt/tools", + "virtfs_tag=microvm virtfs_mode=ro", "virtfs_dir=/workspace virtfs_tag=microvm virtfs_mode=rw " - "virtfs_tag=microvm1 virtfs_dir=/opt/tools virtfs_mode=ro", - "virtfs_dir=/workspace virtfs_tag=microvm virtfs_mode=rw " - "virtfs_dir= virtfs_tag=microvm1 virtfs_mode=ro", + "virtfs_dir= virtfs_tag=microvm virtfs_mode=ro", + "virtfs_aggregate=1", + "nvx_share=0,/workspace,rw", ): with self.subTest(cmdline=cmdline): result, log = self._run(cmdline) @@ -9145,13 +9223,7 @@ def test_agent_fatal_unmounts_mounted_shares(self): mount_log = self.root / "mount.log" functions = "".join( _shell_function(self.source, name) - for name in ( - "unmount_live_shares", - "fatal", - "validate_share_target", - "mount_live_share", - "mount_live_shares", - ) + for name in ("unmount_live_shares", "fatal", *self.SHARE_FUNCTIONS) ).replace("/sbin/nvx-exit", "nvx_exit") result = subprocess.run( [ @@ -9159,14 +9231,16 @@ def test_agent_fatal_unmounts_mounted_shares(self): "-s", "--", self.rootfs.as_posix(), - "virtfs_dir=/workspace virtfs_tag=microvm virtfs_mode=rw " - "virtfs_dir=/workspace/nested virtfs_tag=microvm1 virtfs_mode=ro", + self._aggregate("0,/workspace,rw", "1,/opt/tools,ro"), mount_log.as_posix(), + self.runtime, ], input=( "set -eu\n" - "rootfs=$1\ncmdline=$2\nmount_log=$3\nshare_mountpoints=\n" - 'mount() { printf "mount %s\\n" "$*" >>"$mount_log"; }\n' + "rootfs=$1\ncmdline=$2\nmount_log=$3\nruntime=$4\n" + "share_mountpoints=\n" + 'mount() { printf "mount %s\\n" "$*" >>"$mount_log"; ' + 'case "$*" in *shares/1*) return 1 ;; esac; }\n' 'umount() { printf "umount %s\\n" "$1" >>"$mount_log"; }\n' 'nvx_exit() { printf "exit %s\\n" "$1" >>"$mount_log"; exit "$1"; }\n' f"{functions}mount_live_shares\n" @@ -9176,13 +9250,17 @@ def test_agent_fatal_unmounts_mounted_shares(self): check=False, ) self.assertEqual(result.returncode, 125, result.stdout) - self.assertIn("targets overlap", result.stderr) + self.assertIn("failed to bind live share 1", result.stderr) rootfs = self.rootfs.as_posix() self.assertEqual( mount_log.read_text(encoding="utf-8").splitlines(), [ - f"mount -t virtiofs -o rw,nosuid,nodev microvm {rootfs}/workspace", + f"mount -t virtiofs -o rw,nosuid,nodev microvm {self.runtime}/shares", + f"mount -o bind {self.runtime}/shares/0 {rootfs}/workspace", + f"mount -o remount,bind,rw,nosuid,nodev {rootfs}/workspace", + f"mount -o bind {self.runtime}/shares/1 {rootfs}/opt/tools", f"umount {rootfs}/workspace", + f"umount {self.runtime}/shares", "exit 125", ], ) @@ -9207,11 +9285,34 @@ def test_agent_fails_closed_for_invalid_share_bootstrap(self): ("virtfs_dir=/.nvx-agent virtfs_tag=microvm virtfs_mode=ro", "reserved"), ("virtfs_dir=/etc virtfs_tag=microvm virtfs_mode=ro", "reserved"), ("virtfs_dir=/file virtfs_tag=microvm virtfs_mode=ro", "not a directory"), + ( + "virtfs_dir=/workspace virtfs_tag=microvm virtfs_mode=rw " + "nvx_share=0,/workspace,rw", + "require an aggregate", + ), + ( + "virtfs_dir=/shares virtfs_tag=microvm virtfs_mode=rw " + "virtfs_aggregate=1 nvx_share=0,/workspace,rw", + "must be mounted at", + ), + (self._aggregate(), "has no children"), + (self._aggregate(",/workspace,rw"), "child is invalid"), + (self._aggregate(".,/workspace,rw"), "child is invalid"), + (self._aggregate("..,/workspace,rw"), "child is invalid"), + (self._aggregate("a/b,/workspace,rw"), "child is invalid"), + (self._aggregate("a:b,/workspace,rw"), "child is invalid"), + (self._aggregate("a*,/workspace,rw"), "child is invalid"), + (self._aggregate("0,/workspace"), "child is invalid"), + (self._aggregate("0,/workspace,rx"), "mode"), + (self._aggregate("0,workspace,rw"), "absolute"), + (self._aggregate("0,/a,b,rw"), "reserved char"), + (self._aggregate("0,/workspace,rw", "1,/proc,ro"), "reserved"), + (self._aggregate("0,/file,ro"), "not a directory"), ): with self.subTest(cmdline=cmdline): result, log = self._run(cmdline) self.assertEqual(result.returncode, 125, result.stdout) - self.assertIn("FATAL: live share", result.stderr) + self.assertIn("live share", result.stderr) self.assertIn(message, result.stderr) self.assertEqual(log, "") @@ -11406,7 +11507,7 @@ def test_launch_contract_attaches_mount_and_reserves_bootstrap_tokens(self): with self.assertRaisesRegex(common.ScriptError, "1024-byte"): launch.kernel_command_line(fitting + "x") - def test_launch_contract_attaches_two_shares_in_slot_order(self): + def test_launch_contract_attaches_several_shares_as_an_aggregate(self): distro = sandbox.SandboxLayer.parse( "distro,distro.erofs,11111111-1111-1111-1111-111111111111" ) @@ -11418,19 +11519,30 @@ def test_launch_contract_attaches_two_shares_in_slot_order(self): sandbox.SandboxMount.parse( "/opt/hostedtoolcache,tools,ro", ("credentials",) ), + sandbox.SandboxMount.parse("/srv/cache,cache,ro"), ), ) arguments = launch.openvmm_arguments() - # With several shares, OpenVMM attributes each denied path to the share - # that contains it, so relative paths are joined to their share. + workspace, toolcache, cache = ( + sandbox.aggregate_child_name(index, mount.guest_target) + for index, mount in enumerate(launch.mounts) + ) + # Several shares are the children of one aggregate, and OpenVMM + # attributes each denied path to the child that contains it, so + # relative paths are joined to their share. + self.assertNotIn("--mount", arguments) self.assertEqual( - arguments[arguments.index("--mount") :], + arguments[arguments.index("--mount-aggregate") :], [ - "--mount", - f"/workspace,{os.fspath(Path('work'))},rw", - "--mount", - f"/opt/hostedtoolcache,{os.fspath(Path('tools'))},ro", + "--mount-aggregate", + "/run/nvx/shares", + "--mount-child", + f"{workspace},{os.fspath(Path('work'))},rw", + "--mount-child", + f"{toolcache},{os.fspath(Path('tools'))},ro", + "--mount-child", + f"{cache},{os.fspath(Path('cache'))},ro", "--mount-deny", os.fspath(Path.cwd() / "work" / "secrets"), "--mount-deny", @@ -11440,17 +11552,39 @@ def test_launch_contract_attaches_two_shares_in_slot_order(self): fragment = sandbox.mounts_command_line_fragment(launch.mounts) self.assertEqual( fragment, - " virtfs_dir=/workspace virtfs_tag=microvm virtfs_mode=rw" - " virtfs_dir=/opt/hostedtoolcache virtfs_tag=microvm1 virtfs_mode=ro", + " virtfs_dir=/run/nvx/shares virtfs_tag=microvm virtfs_mode=rw" + " virtfs_aggregate=1", + ) + # The guest binds each child at its target, in order. + command_line = launch.kernel_command_line() + self.assertNotIn("virtfs_", command_line) + self.assertTrue( + command_line.endswith( + f" nvx_share={workspace},/workspace,rw" + f" nvx_share={toolcache},/opt/hostedtoolcache,ro" + f" nvx_share={cache},/srv/cache,ro" + ), + command_line, + ) + self.assertEqual( + sandbox.mounts_command_line_fragment(launch.mounts[1:]), + " virtfs_dir=/run/nvx/shares virtfs_tag=microvm virtfs_mode=ro" + " virtfs_aggregate=1", ) - self.assertNotIn("virtfs_", launch.kernel_command_line()) + with self.assertRaisesRegex(common.ScriptError, "owned by the sandbox"): + launch.kernel_command_line("nvx_share=0,/etc,rw") - # Every share's bootstrap tokens count toward the x86 budget. + # Every share's tokens count toward the x86 budget. base = sandbox.SandboxLaunch( layers=(distro,), scratch=Path("s.ext4") ).kernel_command_line() + shares = len(command_line) - len(base) fitting = "x" * ( - sandbox.SANDBOX_COMMAND_LINE_MAX_SIZE - len(base) - len(fragment) - 2 + sandbox.SANDBOX_COMMAND_LINE_MAX_SIZE + - len(base) + - shares + - len(fragment) + - 2 ) launch.kernel_command_line(fitting) with self.assertRaisesRegex(common.ScriptError, "1024-byte"): @@ -11466,6 +11600,42 @@ def test_launch_contract_attaches_two_shares_in_slot_order(self): self.assertEqual(arguments.count("--mount-owner"), 1) self.assertEqual(arguments[-2:], ["--mount-owner", "caller"]) + def test_aggregate_child_names_pin_their_targets(self): + name = sandbox.aggregate_child_name(0, "/workspace") + self.assertEqual(name, f"0-{hashlib.sha256(b'/workspace').hexdigest()[:32]}") + # Equal targets give equal names, so a restore names its children as + # the boot did, and another target or position gives another name. + self.assertEqual(sandbox.aggregate_child_name(0, "/workspace"), name) + self.assertNotEqual(sandbox.aggregate_child_name(0, "/workspace2"), name) + self.assertNotEqual(sandbox.aggregate_child_name(1, "/workspace"), name) + # Every name follows OpenVMM's rule for the name of a child. + for index, target in ((0, "/w"), (999, "/" + "x" * 1000)): + self.assertRegex( + sandbox.aggregate_child_name(index, target), + r"\A[0-9]+-[0-9a-f]{32}\Z", + ) + self.assertLessEqual(len(sandbox.aggregate_child_name(999, "/w")), 64) + + def test_launch_bounds_shares_by_the_kernel_command_line(self): + distro = sandbox.SandboxLayer.parse( + "distro,distro.erofs,11111111-1111-1111-1111-111111111111" + ) + + def launch(count: int) -> sandbox.SandboxLaunch: + return sandbox.SandboxLaunch( + layers=(distro,), + scratch=Path("s.ext4"), + mounts=tuple( + sandbox.SandboxMount.parse(f"/share/{index:02},host{index}") + for index in range(count) + ), + ) + + # No fixed number of shares is the limit, only the command line. + launch(10).kernel_command_line() + with self.assertRaisesRegex(common.ScriptError, "fewer --mount shares"): + launch(20).kernel_command_line() + def test_launch_rejects_invalid_share_sets(self): distro = sandbox.SandboxLayer.parse( "distro,distro.erofs,11111111-1111-1111-1111-111111111111" @@ -11475,7 +11645,6 @@ def mount(value: str, owner: str = "vmm") -> sandbox.SandboxMount: return sandbox.SandboxMount.parse(value, (), owner) for mounts, message in ( - ((mount("/a,a"), mount("/b,b"), mount("/c,c")), "at most 2"), ((mount("/workspace,a,rw"), mount("/workspace,b")), "overlap"), ((mount("/workspace,a,rw"), mount("/workspace/cache,b")), "overlap"), ((mount("/opt/tools/node,a"), mount("/opt,b,rw")), "overlap"), @@ -11539,11 +11708,28 @@ def launch(*mounts: sandbox.SandboxMount) -> sandbox.SandboxLaunch: toolcache = sandbox.SandboxMount.parse(f"/opt/hostedtoolcache,{tools}") with self.assertRaisesRegex(common.ScriptError, "not inside"): launch(foreign, toolcache).validated() - with self.assertRaisesRegex(common.ScriptError, "not inside"): - launch( - sandbox.SandboxMount.parse(f"/workspace,{work},rw", (".",)), - toolcache, - ).validated() + # A denied path may name a share's root, which OpenVMM then hides + # behind its allowed paths, but no allowed or writable path may. + launch( + sandbox.SandboxMount.parse( + f"/workspace,{work},rw", + (".",), + allowed_paths=(os.fspath(work / "nested"),), + ), + toolcache, + ).validated() + for allowed_paths, writable_paths in (((".",), ()), ((), (".",))): + with self.subTest(allowed=allowed_paths, writable=writable_paths): + with self.assertRaisesRegex(common.ScriptError, "not inside"): + launch( + sandbox.SandboxMount.parse( + f"/workspace,{work},rw", + ("nested",), + allowed_paths=allowed_paths, + writable_paths=writable_paths, + ), + toolcache, + ).validated() validated = launch( sandbox.SandboxMount.parse(f"/workspace,{work},rw", ("nested",)), @@ -11862,11 +12048,96 @@ def values(option: str) -> list[str]: ], "--mount-write .* is not inside the host directory", ), + # Only a denied path may name a share's root. + ( + [ + "--mount", + f"/workspace,{work},rw", + "--mount-deny", + "logs", + "--mount-allow", + os.fspath(work), + "--mount", + f"/opt/hostedtoolcache,{tools}", + ], + "--mount-allow .* is not inside the host directory", + ), + ( + [ + "--mount", + f"/workspace,{work},rw", + "--mount-write", + os.fspath(work), + "--mount", + f"/opt/hostedtoolcache,{tools}", + ], + "--mount-write .* is not inside the host directory", + ), ): with self.subTest(message=message): with self.assertRaisesRegex(common.ScriptError, message): nvx.command_sandbox(nvx.parse_args([*common_args, *arguments])) + def test_sandbox_command_hides_a_share_root_behind_its_allowed_paths(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + layer = root / "distro.erofs" + scratch = root / "scratch.ext4" + work = root / "work" + tools = root / "tools" + layer.write_bytes(b"distro") + scratch.write_bytes(b"scratch") + (work / "src").mkdir(parents=True) + tools.mkdir() + args = nvx.parse_args( + [ + "sandbox", + "--layer", + f"distro,{layer},11111111-1111-1111-1111-111111111111", + "--scratch", + str(scratch), + "--mount", + f"/workspace,{work},rw", + "--mount-deny", + os.fspath(work), + "--mount-allow", + os.fspath(work / "src"), + "--mount", + f"/opt/hostedtoolcache,{tools}", + "--dry-run", + ] + ) + + def require(path: Path, _description: str) -> Path: + return path + + with ( + patch.object(nvx, "require_file", side_effect=require), + patch.object( + nvx, "_format_command", return_value="formatted" + ) as format_command, + ): + nvx.command_sandbox(args) + + command = format_command.call_args.args[0] + + def values(option: str) -> list[str]: + return [ + command[index + 1] + for index, value in enumerate(command) + if value == option + ] + + # OpenVMM hides the workspace's root behind its one allowed path. + workspace = sandbox.aggregate_child_name(0, "/workspace") + toolcache = sandbox.aggregate_child_name(1, "/opt/hostedtoolcache") + self.assertEqual( + values("--mount-child"), + [f"{workspace},{work},rw", f"{toolcache},{tools},ro"], + ) + self.assertEqual(values("--mount-deny"), [os.fspath(work)]) + self.assertEqual(values("--mount-allow"), [os.fspath(work / "src")]) + def test_sandbox_command_attributes_mount_deny_to_the_preceding_mount(self): with tempfile.TemporaryDirectory() as temporary: root = Path(temporary) @@ -11922,13 +12193,20 @@ def require(path: Path, _description: str) -> Path: nvx.command_sandbox(args) command = format_command.call_args.args[0] + workspace = sandbox.aggregate_child_name(0, "/workspace") + toolcache = sandbox.aggregate_child_name(1, "/opt/hostedtoolcache") + self.assertNotIn("--mount", command) self.assertEqual( [ command[index + 1] for index, value in enumerate(command) - if value == "--mount" + if value in ("--mount-aggregate", "--mount-child") + ], + [ + "/run/nvx/shares", + f"{workspace},{work},rw", + f"{toolcache},{tools},ro", ], - [f"/workspace,{work},rw", f"/opt/hostedtoolcache,{tools},ro"], ) self.assertEqual( [ @@ -11942,6 +12220,12 @@ def require(path: Path, _description: str) -> Path: os.fspath(tools / "logs"), ], ) + cmdline = command[command.index("--cmdline") + 1] + self.assertIn( + f"nvx_share={workspace},/workspace,rw " + f"nvx_share={toolcache},/opt/hostedtoolcache,ro", + cmdline, + ) # With one share, a --mount-deny may come first; with several, it # must follow the share whose directory it hides. @@ -11962,6 +12246,8 @@ def require(path: Path, _description: str) -> Path: ) with self.assertRaisesRegex(common.ScriptError, "must follow the --mount"): nvx.command_sandbox(leading) + # More shares are children of the same aggregate. + (root / "cache").mkdir() three = nvx.parse_args( [ *common_args, @@ -11970,20 +12256,32 @@ def require(path: Path, _description: str) -> Path: "--mount", f"/b,{tools}", "--mount", - f"/c,{root}", + f"/c,{root / 'cache'}", + "--dry-run", ] ) - with self.assertRaisesRegex(common.ScriptError, "at most 2"): + with ( + patch.object(nvx, "require_file", side_effect=require), + patch.object( + nvx, "_format_command", return_value="formatted" + ) as format_command, + ): nvx.command_sandbox(three) + command = format_command.call_args.args[0] + self.assertEqual(command.count("--mount-child"), 3) - def test_run_forwards_every_mount_in_order(self): + def test_run_attaches_several_mounts_as_an_aggregate(self): args = nvx.parse_args( [ "run", "--mount", "/workspace,work,rw", "--mount", - "/opt/hostedtoolcache,tools,ro", + "/opt/hostedtoolcache,tools", + "--mount", + "/srv/cache,cache,ro", + "--cmdline", + "quiet", "--dry-run", ] ) @@ -11995,19 +12293,126 @@ def test_run_forwards_every_mount_in_order(self): ): nvx.command_run(args) command = format_command.call_args.args[0] + workspace, toolcache, cache = ( + sandbox.aggregate_child_name(index, target) + for index, target in enumerate( + ("/workspace", "/opt/hostedtoolcache", "/srv/cache") + ) + ) + booted = [ + "/run/nvx/shares", + f"{workspace},work,rw", + f"{toolcache},tools,ro", + f"{cache},cache,ro", + ] + self.assertNotIn("--mount", command) self.assertEqual( [ command[index + 1] for index, value in enumerate(command) - if value == "--mount" + if value in ("--mount-aggregate", "--mount-child") ], - ["/workspace,work,rw", "/opt/hostedtoolcache,tools,ro"], + booted, ) - three = nvx.parse_args( - ["run", "--mount", "/a,a", "--mount", "/b,b", "--mount", "/c,c"] + self.assertEqual( + command[command.index("--cmdline") + 1], + f"quiet nvx_share={workspace},/workspace,rw " + f"nvx_share={toolcache},/opt/hostedtoolcache,ro " + f"nvx_share={cache},/srv/cache,ro", + ) + + # One share stays a plain --mount without bind tokens. + single = nvx.parse_args( + ["run", "--mount", "/workspace,work,rw", "--dry-run"] + ) + nvx.command_run(single) + command = format_command.call_args.args[0] + self.assertEqual( + command[command.index("--mount") :], ["--mount", "/workspace,work,rw"] + ) + self.assertNotIn("--cmdline", command) + + # A restored guest keeps its binds, so only the devices are named, + # with the children's names of the boot: they pin the targets that + # the guest bound, and another target names another child. + restored = nvx.parse_args( + [ + "run", + "--restore-snapshot", + "snapshot", + "--mount", + "/workspace,work,rw", + "--mount", + "/opt/hostedtoolcache,tools", + "--mount", + "/srv/cache,cache,ro", + "--dry-run", + ] + ) + nvx.command_run(restored) + command = format_command.call_args.args[0] + self.assertEqual( + [ + command[index + 1] + for index, value in enumerate(command) + if value in ("--mount-aggregate", "--mount-child") + ], + booted, + ) + self.assertNotIn("--cmdline", command) + retargeted = nvx.parse_args( + [ + "run", + "--restore-snapshot", + "snapshot", + "--mount", + "/workspace,work,rw", + "--mount", + "/opt/tools,tools", + "--mount", + "/srv/cache,cache,ro", + "--dry-run", + ] + ) + nvx.command_run(retargeted) + command = format_command.call_args.args[0] + children = [ + command[index + 1] + for index, value in enumerate(command) + if value == "--mount-child" + ] + self.assertEqual(children[0::2], booted[1::2]) + self.assertNotEqual(children[1], booted[2]) + self.assertEqual(children[1].split(",", 1)[1], "tools,ro") + + for mounts, message in ( + (["/a,a,rx"], "unsupported --mount mode"), + (["/workspace,a", "/workspace/cache,b"], "overlap"), + (["/run/nvx,a", "/b,b"], "overlap"), + (["/a b,a", "/b,b"], "invalid --mount target"), + (["relative,a", "/b,b"], "invalid --mount target"), + ([f"/share/{index:02},host" for index in range(60)], "1024-byte"), + ): + with self.subTest(mounts=mounts[:2]): + invalid = nvx.parse_args( + ["run", *[f"--mount={mount}" for mount in mounts], "--dry-run"] + ) + with self.assertRaisesRegex(common.ScriptError, message): + nvx.command_run(invalid) + relative = nvx.parse_args( + [ + "run", + "--mount", + "/a,a", + "--mount", + "/b,b", + "--mount-deny", + "secrets", + "--dry-run", + ] ) - with self.assertRaisesRegex(common.ScriptError, "at most 2"): - nvx.command_run(three) + with self.assertRaisesRegex(common.ScriptError, "absolute host path"): + nvx.command_run(relative) def test_managed_lifecycle_persists_and_replays_two_shares(self): with tempfile.TemporaryDirectory() as temporary: @@ -12115,11 +12520,13 @@ def require(path: Path, _description: str) -> Path: [ command[index + 1] for index, value in enumerate(command) - if value in ("--mount", "--mount-deny") + if value in ("--mount-aggregate", "--mount-child", "--mount-deny") ], [ - f"/workspace,{work},rw", - f"/opt/hostedtoolcache,{tools},ro", + "/run/nvx/shares", + f"{sandbox.aggregate_child_name(0, '/workspace')},{work},rw", + f"{sandbox.aggregate_child_name(1, '/opt/hostedtoolcache')}," + f"{tools},ro", os.fspath(work / "secrets"), ], ) From d5888a6072595ec6372674826c3c08c877a66e78 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Fri, 9 Oct 2026 19:11:47 -0300 Subject: [PATCH 4/4] test-microvm: share three directories through the aggregate virtio-fs device The filesystem-shares scenario attached a read-write workspace and a read-only tool cache to the two virtio-fs slots, with a repeated --mount and the microvm1 tag, which the pinned OpenVMM no longer offers. The scenario now attaches the workspace, the tool cache, and a data directory as the children of one aggregate, --mount-aggregate /run/nvx/shares with three --mount-child options, and passes the nvx_share= tokens with which the guest binds each child at its target. The data directory exercises a hidden root: --mount-deny names the directory itself, --mount-allow exposes its public directory and a file whose name holds a space, and --mount-write makes only that file writable. The guest checks that only the 0xd0001000 slot exists, that the aggregate is mounted at /run/nvx/shares with a root-only root that lists the three children, and that each bind carries its share's mode. It checks each share's denied paths and the data directory's hidden root, writes where the policy allows it, and through /run/nvx/shares, where guest root bypasses the binds' flags, OpenVMM still rejects every write to the read-only child with EROFS. A hard link between children fails with EROFS into the data directory's read-only public directory, which Linux's linkat reports before a link across filesystems, and with EXDEV into the writable workspace, and neither link appears. The host requires the guest's writes and nothing else. OpenVMM must reject a repeated --mount, --mount beside --mount-aggregate, overlapping children, and a relative policy path before boot. A capture with every child resumes the guest's open journal on restore, and restores that drop a child, swap two, rename one, or offer a plain --mount fail before the guest runs. Renaming a child, with the same directory, mode, and order, is enough, which the names that run and sandbox derive from the guest targets rely on. The denied-filesystem-paths scenario still requires OpenVMM to refuse, before boot, a --mount-deny of the share's root without --mount-allow paths, now with the message that a root can be hidden only to expose allowed paths inside it. Part of #281. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../filesystem-shares-snapshot.sh | 15 +- .../microvm_test_scripts/filesystem-shares.sh | 134 +++++++++----- scripts/nvx_tools/microvm_tests.py | 172 ++++++++++++++---- scripts/test_microvm_tests.py | 131 +++++++++---- 4 files changed, 323 insertions(+), 129 deletions(-) diff --git a/scripts/nvx_tools/microvm_test_scripts/filesystem-shares-snapshot.sh b/scripts/nvx_tools/microvm_test_scripts/filesystem-shares-snapshot.sh index 93c5a492..3b74cfb6 100644 --- a/scripts/nvx_tools/microvm_test_scripts/filesystem-shares-snapshot.sh +++ b/scripts/nvx_tools/microvm_test_scripts/filesystem-shares-snapshot.sh @@ -6,18 +6,23 @@ fail() { exit "$code" } -grep -q '^microvm /workspace virtiofs rw' /proc/mounts || fail 80 -grep -q '^microvm1 /opt/hostedtoolcache virtiofs ro' /proc/mounts || fail 81 +grep -q '^microvm /run/nvx/shares virtiofs rw' /proc/mounts || fail 80 +grep -q '^microvm /workspace virtiofs rw' /proc/mounts || fail 81 +grep -q '^microvm /opt/hostedtoolcache virtiofs ro' /proc/mounts || fail 82 exec 3<>/workspace/journal printf NVX-BEFORE >&3 echo NVX-FILESYSTEM-SHARES-BEFORE nvx-snapshot printf NVX-AFTER >&3 exec 3>&- -[ "$(cat /workspace/journal)" = NVX-BEFORENVX-AFTER ] || fail 82 -[ "$(cat /opt/hostedtoolcache/seed)" = NVX-TOOLCACHE ] || fail 83 +[ "$(cat /workspace/journal)" = NVX-BEFORENVX-AFTER ] || fail 83 +[ "$(cat /opt/hostedtoolcache/seed)" = NVX-TOOLCACHE ] || fail 84 if touch /opt/hostedtoolcache/mutation 2>/dev/null; then - fail 84 + fail 85 fi +if touch /run/nvx/shares/1/mutation 2>/dev/null; then + fail 86 +fi +[ "$(ls /srv/data | tr '\n' '|')" = "notes one.txt|public|" ] || fail 87 echo NVX-FILESYSTEM-SHARES-AFTER nvx-exit 0 diff --git a/scripts/nvx_tools/microvm_test_scripts/filesystem-shares.sh b/scripts/nvx_tools/microvm_test_scripts/filesystem-shares.sh index 4d987c8e..4dbbcec6 100644 --- a/scripts/nvx_tools/microvm_test_scripts/filesystem-shares.sh +++ b/scripts/nvx_tools/microvm_test_scripts/filesystem-shares.sh @@ -16,72 +16,112 @@ expect_read_only() { esac } -# Both fixed slots are discovered, and each share has its own tag. +# One virtio-fs slot carries every share as a child of one aggregate, which +# the guest mounts where only root can enter and binds at each target. grep -q 'virtio_mmio.device=0x1000@0xd0001000:6' /proc/cmdline || fail 40 -grep -q 'virtio_mmio.device=0x1000@0xd0008000:13' /proc/cmdline || fail 41 -grep -q 'virtfs_dir=/workspace virtfs_tag=microvm virtfs_mode=rw' /proc/cmdline || fail 42 -grep -q 'virtfs_dir=/opt/hostedtoolcache virtfs_tag=microvm1 virtfs_mode=ro' /proc/cmdline || fail 43 +if grep -q '0xd0008000' /proc/cmdline; then + fail 41 +fi +grep -q 'virtfs_dir=/run/nvx/shares virtfs_tag=microvm virtfs_mode=rw virtfs_aggregate=1' /proc/cmdline || fail 42 +grep -q '^microvm /run/nvx/shares virtiofs rw' /proc/mounts || fail 43 grep -q '^microvm /workspace virtiofs rw' /proc/mounts || fail 44 -grep -q '^microvm1 /opt/hostedtoolcache virtiofs ro' /proc/mounts || fail 45 -[ "$(cat /workspace/seed)" = NVX-WORKSPACE ] || fail 46 -[ "$(cat /opt/hostedtoolcache/seed)" = NVX-TOOLCACHE ] || fail 47 -[ "$(cat /opt/hostedtoolcache/tools/node)" = NVX-TOOL ] || fail 48 +grep -q '^microvm /opt/hostedtoolcache virtiofs ro' /proc/mounts || fail 45 +grep -q '^microvm /srv/data virtiofs rw' /proc/mounts || fail 46 +[ "$(ls /run/nvx/shares | tr '\n' ' ')" = "0 1 2 " ] || fail 47 +[ "$(stat -c %a /run/nvx/shares)" = 500 ] || fail 48 +[ "$(cat /workspace/seed)" = NVX-WORKSPACE ] || fail 49 +[ "$(cat /opt/hostedtoolcache/seed)" = NVX-TOOLCACHE ] || fail 50 +[ "$(cat /opt/hostedtoolcache/tools/node)" = NVX-TOOL ] || fail 51 -# Each share hides only its own denied path. -[ ! -e /workspace/secrets ] || fail 49 -[ ! -e /opt/hostedtoolcache/credentials ] || fail 50 +# Each share hides only its own denied paths, and the data share's root +# exposes only its allowed paths, one of them with a space in its name. +[ ! -e /workspace/secrets ] || fail 52 +[ ! -e /opt/hostedtoolcache/credentials ] || fail 53 +[ "$(ls /srv/data | tr '\n' '|')" = "notes one.txt|public|" ] || fail 54 +[ ! -e /srv/data/private.txt ] || fail 55 +[ "$(cat /srv/data/public/readme)" = NVX-PUBLIC ] || fail 56 +[ "$(cat '/srv/data/notes one.txt')" = NVX-NOTES ] || fail 57 # The read-write share accepts writes, and the read-only share rejects them, # also through a link in the read-write share. -printf 'NVX-GUEST-WRITE\n' >/workspace/from-guest || fail 51 -mkdir /workspace/guest-directory || fail 52 +printf 'NVX-GUEST-WRITE\n' >/workspace/from-guest || fail 58 +mkdir /workspace/guest-directory || fail 59 if touch /opt/hostedtoolcache/mutation 2>/dev/null; then - fail 53 -fi -ln -s /opt/hostedtoolcache/seed /workspace/toolcache-seed || fail 54 -if (printf 'overwrite\n' >/workspace/toolcache-seed) 2>/dev/null; then - fail 55 -fi - -# OpenVMM, not the guest mount flags, enforces the read-only mode: with the -# share's tag mounted read-write in the guest, the host still rejects every -# mutation with EROFS. A second mount of a tag would reuse the read-only -# superblock, so replace the boot mount instead. -umount /opt/hostedtoolcache || fail 56 -mkdir -p /mnt/toolcache-rw -mount -t virtiofs -o rw microvm1 /mnt/toolcache-rw || fail 57 -grep -q '^microvm1 /mnt/toolcache-rw virtiofs rw' /proc/mounts || fail 58 -[ "$(cat /mnt/toolcache-rw/seed)" = NVX-TOOLCACHE ] || fail 59 -if error=$(touch /mnt/toolcache-rw/mutation 2>&1); then fail 60 fi -expect_read_only "$error" 61 -if error=$(mkdir /mnt/toolcache-rw/directory 2>&1); then +ln -s /opt/hostedtoolcache/seed /workspace/toolcache-seed || fail 61 +if (printf 'overwrite\n' >/workspace/toolcache-seed) 2>/dev/null; then fail 62 fi -expect_read_only "$error" 63 -if error=$( (printf 'append\n' >>/mnt/toolcache-rw/seed) 2>&1); then + +# Within the read-write data share, OpenVMM permits writes only to the notes. +printf 'NVX-GUEST-NOTE\n' >>'/srv/data/notes one.txt' || fail 63 +if error=$(touch /srv/data/public/mutation 2>&1); then fail 64 fi expect_read_only "$error" 65 -if error=$(rm -f /mnt/toolcache-rw/seed 2>&1); then +# The data share's root refuses to look up any name that it does not expose, +# so a new name cannot be created there either. +if error=$(touch /srv/data/added 2>&1); then fail 66 fi -expect_read_only "$error" 67 -if error=$(mv /mnt/toolcache-rw/seed /mnt/toolcache-rw/moved 2>&1); then - fail 68 +case "$error" in + *'Permission denied'*) ;; + *) + echo "NVX-FILESYSTEM-SHARES-ERROR $error" + fail 67 + ;; +esac + +# OpenVMM, not the guest mount flags, enforces each share's mode: the +# aggregate is mounted read-write, and through it the guest's root still +# cannot change the read-only share or reach hidden paths. +[ "$(cat /run/nvx/shares/1/seed)" = NVX-TOOLCACHE ] || fail 68 +if error=$(touch /run/nvx/shares/1/mutation 2>&1); then + fail 69 +fi +expect_read_only "$error" 70 +if error=$(mkdir /run/nvx/shares/1/directory 2>&1); then + fail 71 +fi +expect_read_only "$error" 72 +if error=$( (printf 'append\n' >>/run/nvx/shares/1/seed) 2>&1); then + fail 73 +fi +expect_read_only "$error" 74 +if error=$(rm -f /run/nvx/shares/1/seed 2>&1); then + fail 75 +fi +expect_read_only "$error" 76 +if error=$(chmod 0777 /run/nvx/shares/1/seed 2>&1); then + fail 77 +fi +expect_read_only "$error" 78 +[ ! -e /run/nvx/shares/0/secrets ] || fail 79 +[ ! -e /run/nvx/shares/2/private.txt ] || fail 80 +# Hard links cannot join children: like Linux's linkat, OpenVMM refuses a link +# into a read-only directory first, and one into a writable directory of +# another child crosses devices. The aggregate's root cannot change either. +if error=$(ln /run/nvx/shares/0/seed /run/nvx/shares/2/public/linked 2>&1); then + fail 81 fi -expect_read_only "$error" 69 -if error=$(ln -s seed /mnt/toolcache-rw/link 2>&1); then - fail 70 +expect_read_only "$error" 82 +if error=$(ln /run/nvx/shares/1/seed /run/nvx/shares/0/linked 2>&1); then + fail 83 fi -expect_read_only "$error" 71 -if error=$(chmod 0777 /mnt/toolcache-rw/seed 2>&1); then - fail 72 +case "$error" in + *'ross-device link'*) ;; + *) + echo "NVX-FILESYSTEM-SHARES-ERROR $error" + fail 84 + ;; +esac +[ ! -e /workspace/linked ] || fail 85 +[ ! -e /srv/data/public/linked ] || fail 86 +if mkdir /run/nvx/shares/3 2>/dev/null; then + fail 87 fi -expect_read_only "$error" 73 -[ "$(cat /mnt/toolcache-rw/seed)" = NVX-TOOLCACHE ] || fail 74 -umount /mnt/toolcache-rw || fail 75 +[ "$(cat /opt/hostedtoolcache/seed)" = NVX-TOOLCACHE ] || fail 88 echo NVX-FILESYSTEM-SHARES-OK nvx-exit 0 diff --git a/scripts/nvx_tools/microvm_tests.py b/scripts/nvx_tools/microvm_tests.py index 605d5ab1..99530386 100644 --- a/scripts/nvx_tools/microvm_tests.py +++ b/scripts/nvx_tools/microvm_tests.py @@ -3671,7 +3671,11 @@ def run_denied_filesystem_paths( for name, denied_paths, expected in ( ("outside", (outside,), b"outside the filesystem export root"), ("duplicate", (secrets, secrets), b"unique and non-overlapping"), - ("root", (root,), b"cannot hide the complete filesystem export"), + ( + "root", + (root,), + b"can hide its root only to expose allowed paths inside it", + ), ): invalid = workload_boot_command( executable, @@ -5418,6 +5422,44 @@ def _tree_contents(root: Path) -> dict[str, bytes | None]: return contents +# The guest binds each child of the aggregate at its target with its mode. +_SHARE_TOKENS = ( + "nvx_share=0,/workspace,rw nvx_share=1,/opt/hostedtoolcache,ro " + "nvx_share=2,/srv/data,rw" +) + + +def _aggregate_arguments( + workspace: Path, + toolcache: Path, + data: Path, + names: tuple[str, str, str] = ("0", "1", "2"), +) -> tuple[str, ...]: + """Return the OpenVMM options that share the read-write workspace, the + read-only tool cache, and the data directory as the children `names` of + one aggregate. The data directory's root hides everything but `public` and + `notes one.txt`, and only the notes are writable.""" + workspace_name, toolcache_name, data_name = names + return ( + "--mount-aggregate", + "/run/nvx/shares", + "--mount-child", + f"{workspace_name},{workspace},rw", + "--mount-child", + f"{toolcache_name},{toolcache},ro", + "--mount-child", + f"{data_name},{data},rw", + "--mount-deny", + str(data), + "--mount-allow", + str(data / "public"), + "--mount-allow", + str(data / "notes one.txt"), + "--mount-write", + str(data / "notes one.txt"), + ) + + def _shares_command( executable: Path, backend: str, @@ -5426,6 +5468,7 @@ def _shares_command( memory_mib: int, workspace: Path, toolcache: Path, + data: Path, ) -> list[str]: command = workload_boot_command( executable, @@ -5433,10 +5476,9 @@ def _shares_command( kernel, initrd, memory_mib, - "quiet loglevel=0", - mount=f"/workspace,{workspace},rw", + f"quiet loglevel=0 {_SHARE_TOKENS}", ) - command.extend(("--mount", f"/opt/hostedtoolcache,{toolcache},ro")) + command.extend(_aggregate_arguments(workspace, toolcache, data)) return command @@ -5450,23 +5492,29 @@ def run_filesystem_shares( timeout: float, output_dir: Path, ) -> None: - """Attach a read-write workspace and a read-only tool cache together.""" + """Attach a read-write workspace, a read-only tool cache, and a data + directory with a hidden root together, as the children of one aggregate.""" with tempfile.TemporaryDirectory(prefix="nvx-filesystem-shares-") as temporary: root = Path(temporary) workspace = root / "workspace" toolcache = root / "toolcache" + data = root / "data" (workspace / "secrets").mkdir(parents=True) (toolcache / "tools").mkdir(parents=True) (toolcache / "credentials").mkdir() + (data / "public").mkdir(parents=True) (workspace / "seed").write_bytes(b"NVX-WORKSPACE") (workspace / "secrets" / "token").write_bytes(b"NVX-SECRET") (toolcache / "seed").write_bytes(b"NVX-TOOLCACHE") (toolcache / "tools" / "node").write_bytes(b"NVX-TOOL") (toolcache / "credentials" / "token").write_bytes(b"NVX-CREDENTIAL") + (data / "public" / "readme").write_bytes(b"NVX-PUBLIC") + (data / "notes one.txt").write_bytes(b"NVX-NOTES\n") + (data / "private.txt").write_bytes(b"NVX-PRIVATE") toolcache_contents = _tree_contents(toolcache) command = _shares_command( - executable, backend, kernel, initrd, memory_mib, workspace, toolcache + executable, backend, kernel, initrd, memory_mib, workspace, toolcache, data ) command.extend( ( @@ -5476,6 +5524,7 @@ def run_filesystem_shares( str(toolcache / "credentials"), ) ) + data_contents = _tree_contents(data) run_guest_script( command, _read_script("filesystem-shares.sh"), @@ -5488,49 +5537,77 @@ def run_filesystem_shares( if not (workspace / "guest-directory").is_dir(): raise RuntimeError("read-write share did not accept a guest directory") assert_guest_symlink(workspace / "toolcache-seed", "/opt/hostedtoolcache/seed") + if (workspace / "linked").exists(): + raise RuntimeError("a hard link joined two children of the aggregate") if _tree_contents(toolcache) != toolcache_contents: raise RuntimeError("guest modified the read-only share") if (workspace / "secrets" / "token").read_bytes() != b"NVX-SECRET": raise RuntimeError("guest modified a denied path") + data_contents["notes one.txt"] = b"NVX-NOTES\nNVX-GUEST-NOTE\n" + if _tree_contents(data) != data_contents: + raise RuntimeError( + "guest changed the data share beyond its one writable path" + ) - third = root / "third" - third.mkdir() nested = workspace / "nested" nested.mkdir() + third = root / "third" + third.mkdir() for name, arguments, expected in ( ( - "three-shares", - ("--mount", f"/third,{third},ro"), - b"at most 2 filesystems", + "repeated-mount", + ( + "--mount", + f"/workspace,{workspace},rw", + "--mount", + f"/opt/hostedtoolcache,{toolcache},ro", + ), + b"cannot be used multiple times", + ), + ( + "overlapping-children", + ( + "--mount-aggregate", + "/run/nvx/shares", + "--mount-child", + f"0,{workspace},rw", + "--mount-child", + f"1,{nested},ro", + ), + b"must not overlap", ), - ("relative-deny", ("--mount-deny", "secrets"), b"absolute host path"), ): - invalid = _shares_command( - executable, backend, kernel, initrd, memory_mib, workspace, toolcache + invalid = workload_boot_command( + executable, backend, kernel, initrd, memory_mib, "quiet loglevel=0" ) invalid.extend(arguments) _expect_boot_failure( invalid, output_dir / f"filesystem-shares-{name}.log", timeout, expected ) - for name, mount, expected in ( - ("nested-target", f"/workspace/cache,{toolcache},ro", b"overlap"), - ("nested-host", f"/opt/hostedtoolcache,{nested},ro", b"must not overlap"), + for name, arguments, expected in ( + ( + "mount-and-aggregate", + ("--mount", f"/third,{third},ro"), + b"cannot be used with", + ), + ("relative-deny", ("--mount-deny", "secrets"), b"absolute host path"), ): - invalid = workload_boot_command( + invalid = _shares_command( executable, backend, kernel, initrd, memory_mib, - "quiet loglevel=0", - mount=f"/workspace,{workspace},rw", + workspace, + toolcache, + data, ) - invalid.extend(("--mount", mount)) + invalid.extend(arguments) _expect_boot_failure( invalid, output_dir / f"filesystem-shares-{name}.log", timeout, expected ) - # Both shares belong to the snapshot contract, in slot order. + # Every child belongs to the snapshot contract, in order. snapshot = root / "snapshot" with OpenvmmProcess( [ @@ -5542,6 +5619,7 @@ def run_filesystem_shares( memory_mib, workspace, toolcache, + data, ), "--snapshot-destination", str(snapshot), @@ -5560,24 +5638,44 @@ def run_filesystem_shares( if source.returncode != 0 or ( source_lines.count(FILESYSTEM_SHARES_BEFORE_MARKER) != 1 ): - raise RuntimeError("two-share snapshot capture failed") + raise RuntimeError("aggregate snapshot capture failed") if FILESYSTEM_SHARES_AFTER_MARKER in source_lines: - raise RuntimeError("two-share snapshot source crossed the capture boundary") + raise RuntimeError("aggregate snapshot source crossed the capture boundary") fingerprint = _snapshot_fingerprint(snapshot) - workspace_mount = ("--mount", f"/workspace,{workspace},rw") - toolcache_mount = ("--mount", f"/opt/hostedtoolcache,{toolcache},ro") - for name, mounts, expected in ( - ("missing-share", (workspace_mount,), b"2 --mount attachments"), + aggregate = _aggregate_arguments(workspace, toolcache, data) + for name, arguments, expected in ( + ( + "missing-share", + ( + "--mount-aggregate", + "/run/nvx/shares", + "--mount-child", + f"0,{workspace},rw", + "--mount-child", + f"1,{toolcache},ro", + ), + b"do not match the snapshot contract", + ), ( "swapped-shares", - (toolcache_mount, workspace_mount), - b"does not match the snapshot contract", + _aggregate_arguments(toolcache, workspace, data), + b"do not match the snapshot contract", + ), + # The children's names pin the targets where the guest bound them. + ( + "renamed-share", + _aggregate_arguments(workspace, toolcache, data, ("0", "1", "data")), + b"do not match the snapshot contract", + ), + ( + "single-mount", + ("--mount", f"/workspace,{workspace},rw"), + b"--mount-aggregate and --mount-child options", ), ): invalid = snapshot_restore_command(executable, backend, snapshot) - for mount in mounts: - invalid.extend(mount) + invalid.extend(arguments) _expect_process_failure( invalid, output_dir / f"filesystem-shares-{name}.log", @@ -5586,7 +5684,7 @@ def run_filesystem_shares( (FILESYSTEM_SHARES_AFTER_MARKER,), ) restore = snapshot_restore_command(executable, backend, snapshot) - restore.extend((*workspace_mount, *toolcache_mount)) + restore.extend(aggregate) with OpenvmmProcess( restore, output_dir / "filesystem-shares-restore.log" ) as process: @@ -5594,10 +5692,10 @@ def run_filesystem_shares( restored = process.wait(timeout) if restored.returncode != 0: raise RuntimeError( - f"two-share snapshot restore exited with {restored.returncode}" + f"aggregate snapshot restore exited with {restored.returncode}" ) if _snapshot_fingerprint(snapshot) != fingerprint: - raise RuntimeError("two-share snapshot restore modified snapshot artifacts") + raise RuntimeError("aggregate snapshot restore modified snapshot artifacts") if (workspace / "journal").read_bytes() != b"NVX-BEFORENVX-AFTER": raise RuntimeError("read-write share did not resume after restore") if _tree_contents(toolcache) != toolcache_contents: @@ -6674,8 +6772,8 @@ def run(args: argparse.Namespace) -> int: ) if "filesystem-shares" in scenarios: print( - "Running concurrent read-write and read-only microVM shares " - f"on OpenVMM/{args.backend}" + "Running read-write, read-only, and hidden-root microVM shares " + f"through one aggregate on OpenVMM/{args.backend}" ) run_filesystem_shares( executable, diff --git a/scripts/test_microvm_tests.py b/scripts/test_microvm_tests.py index 82b171b5..b3516557 100644 --- a/scripts/test_microvm_tests.py +++ b/scripts/test_microvm_tests.py @@ -1979,30 +1979,43 @@ class FilesystemSharesScenarioTests(unittest.TestCase): """The filesystem-shares scenario with OpenVMM and the guest simulated.""" @staticmethod - def _mounts(command: list[str]) -> list[str]: + def _children(command: list[str]) -> list[str]: return [ command[index + 1] for index, value in enumerate(command) - if value == "--mount" + if value == "--mount-child" ] - def _run(self, *, guest_writes_toolcache: bool = False) -> list[list[str]]: + @staticmethod + def _host(child: str) -> Path: + return Path(child.split(",", 1)[1].rsplit(",", 1)[0]) + + def _run( + self, + *, + guest_writes_toolcache: bool = False, + guest_links_children: bool = False, + ) -> list[list[str]]: launched: list[list[str]] = [] - mounts = self._mounts + children = self._children + host = self._host def guest( command: list[str], script: str, marker: bytes, **_kwargs: object ) -> None: launched.append(command) self.assertEqual(marker, microvm_tests.FILESYSTEM_SHARES_MARKER) - self.assertIn("mount -t virtiofs -o rw microvm1", script) - workspace, toolcache = ( - Path(mount.split(",", 2)[1]) for mount in mounts(command) - ) + self.assertIn("touch /run/nvx/shares/1/mutation", script) + self.assertIn("ln /run/nvx/shares/1/seed /run/nvx/shares/0/linked", script) + workspace, toolcache, data = (host(child) for child in children(command)) (workspace / "from-guest").write_bytes(b"NVX-GUEST-WRITE\n") (workspace / "guest-directory").mkdir() + with (data / "notes one.txt").open("ab") as notes: + notes.write(b"NVX-GUEST-NOTE\n") if guest_writes_toolcache: (toolcache / "mutation").write_bytes(b"") + if guest_links_children: + (workspace / "linked").write_bytes(b"NVX-TOOLCACHE") class FakeProcess: def __init__(self, command: list[str], _log_path: Path) -> None: @@ -2023,8 +2036,7 @@ def send_bytes(self, _data: bytes) -> None: def wait(self, _timeout: float) -> openvmm_process.OpenvmmProcessResult: command = self.command - requested = mounts(command) - workspace = Path(requested[0].split(",", 2)[1]) + requested = children(command) if "--snapshot-destination" in command: snapshot = Path( command[command.index("--snapshot-destination") + 1] @@ -2032,28 +2044,35 @@ def wait(self, _timeout: float) -> openvmm_process.OpenvmmProcessResult: snapshot.mkdir() for name in ("manifest.bin", "state.bin", "memory.bin"): (snapshot / name).write_bytes(name.encode()) - (workspace / "journal").write_bytes(b"NVX-BEFORE") + (host(requested[0]) / "journal").write_bytes(b"NVX-BEFORE") return openvmm_process.OpenvmmProcessResult( 0, b"NVX-FILESYSTEM-SHARES-BEFORE\n" ) if "--restore-snapshot" in command: - if len(requested) == 1: - error = b"requires 2 --mount attachments in snapshot order" - elif not requested[0].startswith("/workspace,"): - error = b"target does not match the snapshot contract" + names = [child.split(",", 1)[0] for child in requested] + if "--mount" in command: + error = ( + b"requires the --mount-aggregate and --mount-child options" + ) + elif ( + len(requested) != 3 + or host(requested[0]).name != "workspace" + or names != ["0", "1", "2"] + ): + error = b"do not match the snapshot contract" else: - with (workspace / "journal").open("ab") as journal: + with (host(requested[0]) / "journal").open("ab") as journal: journal.write(b"NVX-AFTER") return openvmm_process.OpenvmmProcessResult( 0, b"NVX-FILESYSTEM-SHARES-AFTER\n" ) return openvmm_process.OpenvmmProcessResult(1, error + b"\n") - if len(requested) == 3: - error = b"microVM permits at most 2 filesystems" - elif "--mount-deny" in command: + if command.count("--mount") == 2: + error = b"the argument '--mount' cannot be used multiple times" + elif "--mount" in command: + error = b"'--mount-aggregate' cannot be used with '--mount'" + elif "secrets" in command: error = b"--mount-deny requires an absolute host path" - elif requested[1].startswith("/workspace/cache,"): - error = b"guest mount targets overlap" else: error = b"host directories must not overlap" return openvmm_process.OpenvmmProcessResult(2, error + b"\n") @@ -2079,35 +2098,67 @@ def wait(self, _timeout: float) -> openvmm_process.OpenvmmProcessResult: ) return launched - def test_scenario_attaches_both_shares_and_restores_them_in_order(self): + def test_scenario_attaches_every_share_and_restores_them_in_order(self): launched = self._run() boot = launched[0] - workspace, toolcache = self._mounts(boot) - self.assertTrue(workspace.startswith("/workspace,")) - self.assertTrue(workspace.endswith(",rw")) - self.assertTrue(toolcache.startswith("/opt/hostedtoolcache,")) - self.assertTrue(toolcache.endswith(",ro")) - denied = [ - Path(boot[index + 1]).name - for index, value in enumerate(boot) - if value == "--mount-deny" - ] - self.assertEqual(denied, ["secrets", "credentials"]) + self.assertNotIn("--mount", boot) + self.assertEqual(boot[boot.index("--mount-aggregate") + 1], "/run/nvx/shares") + workspace, toolcache, data = self._children(boot) + self.assertTrue(workspace.startswith("0,") and workspace.endswith(",rw")) + self.assertTrue(toolcache.startswith("1,") and toolcache.endswith(",ro")) + self.assertTrue(data.startswith("2,") and data.endswith(",rw")) + cmdline = boot[boot.index("--cmdline") + 1] + self.assertIn( + "nvx_share=0,/workspace,rw nvx_share=1,/opt/hostedtoolcache,ro " + "nvx_share=2,/srv/data,rw", + cmdline, + ) - # Four invalid sets fail before boot, then a capture with both shares, - # two invalid restores, and the restore with both shares in order. - self.assertEqual(len(launched), 1 + 4 + 1 + 3) + def policy(command: list[str], option: str) -> list[str]: + return [ + Path(command[index + 1]).name + for index, value in enumerate(command) + if value == option + ] + + # The data directory's root hides everything but two paths, one of + # them writable, and each other share hides its own denied path. + self.assertEqual( + policy(boot, "--mount-deny"), ["data", "secrets", "credentials"] + ) + self.assertEqual(policy(boot, "--mount-allow"), ["public", "notes one.txt"]) + self.assertEqual(policy(boot, "--mount-write"), ["notes one.txt"]) + + # Four invalid sets fail before boot, then a capture with every share, + # four invalid restores, and the restore with every share in order. + self.assertEqual(len(launched), 1 + 4 + 1 + 5) + self.assertEqual(launched[1].count("--mount"), 2) + self.assertEqual(len(self._children(launched[2])), 2) capture = launched[5] self.assertIn("--snapshot-destination", capture) - self.assertEqual(self._mounts(capture), [workspace, toolcache]) - self.assertEqual(self._mounts(launched[6]), [workspace]) - self.assertEqual(self._mounts(launched[7]), [toolcache, workspace]) - self.assertEqual(self._mounts(launched[8]), [workspace, toolcache]) + self.assertEqual(self._children(capture), [workspace, toolcache, data]) + self.assertEqual(self._children(launched[6]), [workspace, toolcache]) + self.assertEqual( + [self._host(child).name for child in self._children(launched[7])], + ["toolcache", "workspace", "data"], + ) + # Renaming a child, with the same host directory, mode, and order, is + # enough to fail. + self.assertEqual( + self._children(launched[8]), + [workspace, toolcache, data.replace("2,", "data,", 1)], + ) + self.assertIn("--mount", launched[9]) + self.assertEqual(self._children(launched[10]), [workspace, toolcache, data]) def test_scenario_rejects_a_write_to_the_read_only_share(self): with self.assertRaisesRegex(RuntimeError, "modified the read-only share"): self._run(guest_writes_toolcache=True) + def test_scenario_rejects_a_hard_link_between_children(self): + with self.assertRaisesRegex(RuntimeError, "hard link joined two children"): + self._run(guest_links_children=True) + def test_scenario_is_a_default_correctness_scenario(self): self.assertIn("filesystem-shares", microvm_tests.MICROVM_TEST_SCENARIOS) for name in ("filesystem-shares.sh", "filesystem-shares-snapshot.sh"):