diff --git a/.github/workflows/run-nvx-microvm-tests.yml b/.github/workflows/run-nvx-microvm-tests.yml index 8f9b7acfe..45e843859 100644 --- a/.github/workflows/run-nvx-microvm-tests.yml +++ b/.github/workflows/run-nvx-microvm-tests.yml @@ -453,6 +453,7 @@ jobs: scratch="build/ubuntu-shares-${{ inputs.backend }}.ext4" workspace="build/ubuntu-shares-workspace-${{ inputs.backend }}" toolcache="build/ubuntu-shares-toolcache-${{ inputs.backend }}" + files="build/ubuntu-shares-files-${{ inputs.backend }}" outside="$(dirname "${workspace}")/nvx-outside-marker" logs="build/test-results/ubuntu-${{ inputs.backend }}" # Keep the control socket path short. @@ -466,19 +467,29 @@ jobs: } trap cleanup EXIT prepare_shares() { - rm -rf "${workspace}" "${toolcache}" + rm -rf "${workspace}" "${toolcache}" "${files}" mkdir -p "${workspace}/nvx-denied" "${workspace}/nvx-links" \ - "${toolcache}/nvx-denied" + "${toolcache}/nvx-denied" "${files}/config" "${files}/results" printf 'host-to-guest\n' >"${workspace}/nvx-host-marker" printf 'host-to-guest\n' >"${toolcache}/nvx-host-marker" printf 'secret\n' >"${workspace}/nvx-denied/secret" printf 'secret\n' >"${toolcache}/nvx-denied/secret" printf 'host-outside\n' >"${outside}" + # Each shared file sits beside a file that no share exposes. Every + # guest identity may write both shared files, so only the read-only + # share can refuse a write. + printf 'host-to-guest\n' >"${files}/config/settings.json" + printf 'secret\n' >"${files}/config/secret.json" + printf 'host-to-guest\n' >"${files}/results/output.txt" + printf 'other\n' >"${files}/results/other.txt" + chmod 0666 "${files}/config/settings.json" "${files}/results/output.txt" chmod 0777 "${workspace}" "${workspace}/nvx-links" cp --reflink=auto build/ubuntu-smoke-scratch.ext4 "${scratch}" } # The guest writes to the read-write share, and neither it nor a link - # in the read-write share changes the read-only share. + # in the read-write share changes the read-only share. It rewrites + # only the read-write file, and the files beside the shared files + # stay unchanged. check_shares() { test "$(cat "${workspace}/nvx-guest-marker")" = guest-to-host test -d "${workspace}/nvx-guest-directory" @@ -488,14 +499,24 @@ jobs: test "$(cat "${toolcache}/nvx-denied/secret")" = secret test "$(cat "${workspace}/nvx-denied/secret")" = secret test "$(cat "${outside}")" = host-outside + test "$(ls -A "${files}/config" | sort | tr '\n' ' ')" = "secret.json settings.json " + test "$(ls -A "${files}/results" | sort | tr '\n' ' ')" = "other.txt output.txt " + test "$(cat "${files}/config/settings.json")" = host-to-guest + test "$(cat "${files}/config/secret.json")" = secret + test "$(cat "${files}/results/output.txt")" = guest-to-host + test "$(cat "${files}/results/other.txt")" = other } share_options=( --mount "/workspace,${workspace},rw" --mount-deny nvx-denied --mount "/opt/hostedtoolcache,${toolcache},ro" --mount-deny nvx-denied + --mount "/config/settings.json,${files}/config/settings.json,ro" + --mount "/results/output.txt,${files}/results/output.txt,rw" ) smoke_arguments=( --entrypoint /sbin/nvx-sandbox-smoke --arg /workspace --arg rw --arg /opt/hostedtoolcache --arg ro + --arg /config/settings.json --arg file-ro + --arg /results/output.txt --arg file-rw ) prepare_shares python3 scripts/nvx.py sandbox \ @@ -507,7 +528,7 @@ jobs: --workload-user 65534:65534 \ --memory-mib 256 check_shares - # A managed sandbox persists both shares and reattaches them on start. + # A managed sandbox persists every share and reattaches them on start. prepare_shares python3 scripts/nvx.py sandbox provision \ --state-dir "${state}" \ @@ -523,21 +544,28 @@ jobs: with open(sys.argv[1], encoding="utf-8") as stream: config = json.load(stream) shares = [ - (mount["guest_target"], mount["access"], mount["denied_paths"]) + ( + mount["guest_target"], + mount["access"], + mount["denied_paths"], + mount["kind"], + ) for mount in config["mounts"] ] expected = [ - ("/workspace", "rw", ["nvx-denied"]), - ("/opt/hostedtoolcache", "ro", ["nvx-denied"]), + ("/workspace", "rw", ["nvx-denied"], "directory"), + ("/opt/hostedtoolcache", "ro", ["nvx-denied"], "directory"), + ("/config/settings.json", "ro", [], "file"), + ("/results/output.txt", "rw", [], "file"), ] - if config["format"] != 4 or "mount" in config or shares != expected: - sys.exit(f"managed sandbox did not persist both shares: {config}") + if config["format"] != 6 or "mount" in config or shares != expected: + sys.exit(f"managed sandbox did not persist every share: {config}") EOF python3 scripts/nvx.py sandbox start --state-dir "${state}" python3 scripts/nvx.py sandbox exec --state-dir "${state}" \ "${smoke_arguments[@]}" python3 scripts/nvx.py sandbox stop --state-dir "${state}" - # A clean ext4 journal shows that stop unmounted both shares, the + # A clean ext4 journal shows that stop unmounted every share, the # overlay, and scratch in order. python3 - "${state}/outcome.json" "${scratch}" <<'EOF' import json @@ -556,7 +584,7 @@ jobs: EOF check_shares python3 scripts/nvx.py sandbox deprovision --state-dir "${state}" - rm -rf "${workspace}" "${toolcache}" "${scratch}" "${outside}" + rm -rf "${workspace}" "${toolcache}" "${files}" "${scratch}" "${outside}" - name: Run Ubuntu sandbox share access policy on Linux if: runner.os != 'Windows' && !inputs.debug-kernel @@ -901,32 +929,46 @@ jobs: $scratch = Join-Path $PWD "build\ubuntu-shares-${{ inputs.backend }}.ext4" $workspace = Join-Path $PWD "build\ubuntu-shares-workspace-${{ inputs.backend }}" $toolcache = Join-Path $PWD "build\ubuntu-shares-toolcache-${{ inputs.backend }}" + $files = Join-Path $PWD "build\ubuntu-shares-files-${{ inputs.backend }}" $state = Join-Path $PWD "build\ubuntu-shares-state-${{ inputs.backend }}" $outside = Join-Path (Split-Path $workspace) "nvx-outside-marker" $logs = "build\test-results\ubuntu-${{ inputs.backend }}" + $settings = Join-Path $files "config\settings.json" + $output = Join-Path $files "results\output.txt" $shareOptions = @( "--mount", "/workspace,$workspace,rw", "--mount-deny", "nvx-denied", - "--mount", "/opt/hostedtoolcache,$toolcache,ro", "--mount-deny", "nvx-denied" + "--mount", "/opt/hostedtoolcache,$toolcache,ro", "--mount-deny", "nvx-denied", + "--mount", "/config/settings.json,$settings,ro", + "--mount", "/results/output.txt,$output,rw" ) $smokeArguments = @( "--entrypoint", "/sbin/nvx-sandbox-smoke", "--arg", "/workspace", "--arg", "rw", - "--arg", "/opt/hostedtoolcache", "--arg", "ro" + "--arg", "/opt/hostedtoolcache", "--arg", "ro", + "--arg", "/config/settings.json", "--arg", "file-ro", + "--arg", "/results/output.txt", "--arg", "file-rw" ) function Initialize-Shares { - foreach ($path in $workspace, $toolcache, $state) { + foreach ($path in $workspace, $toolcache, $files, $state) { if (Test-Path $path) { Remove-Item -Recurse -Force $path } } - New-Item -ItemType Directory -Force (Join-Path $workspace "nvx-denied"), (Join-Path $workspace "nvx-links"), (Join-Path $toolcache "nvx-denied") | Out-Null + New-Item -ItemType Directory -Force (Join-Path $workspace "nvx-denied"), (Join-Path $workspace "nvx-links"), (Join-Path $toolcache "nvx-denied"), (Join-Path $files "config"), (Join-Path $files "results") | Out-Null foreach ($share in $workspace, $toolcache) { [IO.File]::WriteAllText((Join-Path $share "nvx-host-marker"), "host-to-guest`n") [IO.File]::WriteAllText((Join-Path $share "nvx-denied\secret"), "secret`n") } [IO.File]::WriteAllText($outside, "host-outside`n") + # Each shared file sits beside a file that no share exposes. + [IO.File]::WriteAllText($settings, "host-to-guest`n") + [IO.File]::WriteAllText((Join-Path $files "config\secret.json"), "secret`n") + [IO.File]::WriteAllText($output, "host-to-guest`n") + [IO.File]::WriteAllText((Join-Path $files "results\other.txt"), "other`n") Copy-Item -Force build\ubuntu-smoke-scratch.ext4 $scratch } # The guest writes to the read-write share, and neither it nor a link - # in the read-write share changes the read-only share. + # in the read-write share changes the read-only share. It rewrites + # only the read-write file, and the files beside the shared files + # stay unchanged. function Test-Shares { if ([IO.File]::ReadAllText((Join-Path $workspace "nvx-guest-marker")) -ne "guest-to-host`n") { throw "guest write did not reach the read-write share" @@ -944,16 +986,26 @@ jobs: } if ([IO.File]::ReadAllText((Join-Path $toolcache "nvx-host-marker")) -ne "host-to-guest`n") { throw "guest changed the read-only share" } if ([IO.File]::ReadAllText($outside) -ne "host-outside`n") { throw "a guest link reached host data outside the shares" } + foreach ($directory in "config", "results") { + $entries = (Get-ChildItem -Force -Name (Join-Path $files $directory) | Sort-Object) -join " " + $expected = @{ config = "secret.json settings.json"; results = "other.txt output.txt" }[$directory] + if ($entries -ne $expected) { throw "guest changed the directory of a shared file: $entries" } + } + if ([IO.File]::ReadAllText($settings) -ne "host-to-guest`n") { throw "guest changed the read-only file" } + if ([IO.File]::ReadAllText($output) -ne "guest-to-host`n") { throw "guest write did not reach the read-write file" } + if ([IO.File]::ReadAllText((Join-Path $files "config\secret.json")) -ne "secret`n") { throw "guest changed a file beside a shared file" } + if ([IO.File]::ReadAllText((Join-Path $files "results\other.txt")) -ne "other`n") { throw "guest changed a file beside a shared file" } } Initialize-Shares Invoke-Nvx sandbox --hypervisor "${{ inputs.backend }}" --layer $layer --scratch $scratch @shareOptions @smokeArguments --workload-user 65534:65534 --memory-mib 256 Test-Shares - # A managed sandbox persists both shares and reattaches them on start. + # A managed sandbox persists every share and reattaches them on start. Initialize-Shares Invoke-Nvx sandbox provision --state-dir $state --hypervisor "${{ inputs.backend }}" --layer $layer --scratch $scratch @shareOptions --workload-user 65534:65534 $config = Get-Content -Raw (Join-Path $state "config.json") | ConvertFrom-Json - if ($config.format -ne 4 -or $config.mounts.Count -ne 2 -or $config.mounts[0].guest_target -ne "/workspace" -or $config.mounts[1].access -ne "ro") { - throw "managed sandbox did not persist both shares: $($config | ConvertTo-Json -Compress -Depth 5)" + $shares = ($config.mounts | ForEach-Object { "$($_.guest_target),$($_.access),$($_.kind)" }) -join " " + if ($config.format -ne 6 -or $shares -ne "/workspace,rw,directory /opt/hostedtoolcache,ro,directory /config/settings.json,ro,file /results/output.txt,rw,file") { + throw "managed sandbox did not persist every share: $($config | ConvertTo-Json -Compress -Depth 5)" } try { Invoke-Nvx sandbox start --state-dir $state @@ -971,7 +1023,7 @@ jobs: if ($outcome.operation -ne "managed" -or $outcome.category -ne "success" -or $outcome.status_code -ne 0) { throw "managed sandbox outcome is invalid: $($outcome | ConvertTo-Json -Compress)" } - # A clean ext4 journal shows that stop unmounted both shares, the + # A clean ext4 journal shows that stop unmounted every share, the # overlay, and scratch in order. $image = [IO.File]::OpenRead($scratch) try { @@ -986,7 +1038,7 @@ jobs: } Test-Shares Invoke-Nvx sandbox deprovision --state-dir $state - Remove-Item -Recurse -Force $workspace, $toolcache, $scratch, $outside + Remove-Item -Recurse -Force $workspace, $toolcache, $files, $scratch, $outside - name: Run Ubuntu sandbox share access policy on Windows if: runner.os == 'Windows' && !inputs.debug-kernel diff --git a/aci_edge_sandboxes/README.md b/aci_edge_sandboxes/README.md index ea57ef3af..78d6d26c9 100644 --- a/aci_edge_sandboxes/README.md +++ b/aci_edge_sandboxes/README.md @@ -622,29 +622,33 @@ translation, for example to turn a host working directory into a - 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 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. + directory or file per child, mounted at `/run/nvx/hostfs/root`, a guest + directory that only the guest's root can enter. The children are the + outermost mapped directories and the mapped files outside them, 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 host enforces each mapping's access. A child 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. OpenVMM exports a mapped + file alone: the guest reaches neither its siblings nor its directory, which + it cannot list, inspect, or change, so a read-write file can be rewritten + in place but not replaced, renamed, or removed. OpenVMM follows the file's + name, so the guest sees the file that a host program, such as an editor, + saves over it, and the file's accesses fail with `EACCES` while anything + but a regular file holds the name. Start refuses to run a sandbox whose + mapped file a directory replaced, or whose mapped directory a file + replaced, and OpenVMM refuses such a child too, so a file grant never + becomes a directory grant. Mapping a whole volume (`/` or `C:\`) is + refused; map directories inside it. - 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. + or singles out, such as 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 @@ -653,16 +657,18 @@ translation, for example to turn a host working directory into 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 +- A sandbox maps at most 4096 paths through at most 256 children. A policy + whose mapped directories, and mapped files outside them, would need more + children is refused rather than served through the directories that hold + its files, which would grant more than the files; map directories that + contain the files instead. OpenVMM hides at most 128 denied paths, and + singles out at most 128 read-write paths inside read-only ones, in each + exported directory. +- 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`. + these limits with `policy_validation`, as it does one whose children would + export one object twice, such as two mapped host hard links to one file. - 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 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 8448409ee..de7c61846 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 @@ -20,11 +20,11 @@ //! 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. +//! numbered `--mount-child` directories, and files with the `file` flag, 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 children, 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: @@ -347,11 +347,19 @@ fn parse(arguments: &[String]) -> Result { { let invalid = || format!("invalid --mount-child {child}"); let (name, rest) = child.split_once(',').ok_or_else(invalid)?; + // Like OpenVMM, a child exposes a directory or, with the `file` flag, a regular + // file, and refuses a host path of the other kind. + let (rest, file) = match rest.strip_suffix(",file") { + Some(rest) => (rest, true), + None => (rest, false), + }; let (host, mode) = rest.rsplit_once(',').ok_or_else(invalid)?; - if name != index.to_string() - || !Path::new(host).is_dir() - || !["ro", "rw"].contains(&mode) - { + let kind_matches = if file { + Path::new(host).is_file() + } else { + Path::new(host).is_dir() + }; + if name != index.to_string() || !kind_matches || !["ro", "rw"].contains(&mode) { return Err(invalid()); } children.push(PathBuf::from(host)); @@ -369,7 +377,12 @@ fn parse(arguments: &[String]) -> Result { 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)) { + // No policy path names a file child, which contains nothing. + if !path.exists() + || !children + .iter() + .any(|root| root.is_dir() && path.starts_with(root)) + { return Err(format!("invalid {option} {}", path.display())); } } diff --git a/aci_edge_sandboxes/src/openvmm/filesystem.rs b/aci_edge_sandboxes/src/openvmm/filesystem.rs index 3d24ddc64..ba2dd5b7d 100644 --- a/aci_edge_sandboxes/src/openvmm/filesystem.rs +++ b/aci_edge_sandboxes/src/openvmm/filesystem.rs @@ -1,19 +1,22 @@ //! Host paths exposed to the guest through OpenVMM's aggregate virtio-fs export. //! //! 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. +//! read-only root that lists one child per exported host directory or file, 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. +//! The children are the outermost mapped directories and the mapped files outside them. OpenVMM +//! exports each such file alone, without its directory, so mapped paths may lie anywhere, on any +//! volume, except that a whole volume cannot be mapped. 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. +//! +//! A policy that would need more children than OpenVMM offers is refused rather than served +//! through the directories of its files, which would grant more than the files. Records of +//! earlier crates may still hold such a directory as a hidden child, whose root is no mapping +//! itself and which exposes only its mapped 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. @@ -60,15 +63,20 @@ pub(crate) struct HostMapping { pub(crate) binds: Vec, } -/// One host directory of the aggregate export. +/// One host directory or file 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. + /// Canonical host directory, or the canonical host file that the child exposes alone. pub(crate) root: PathBuf, + /// Whether the root is a regular file, which OpenVMM exposes without its directory. Records + /// without file children leave it out, and earlier crates refuse records with them. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub(crate) file: bool, /// Whether the guest may modify it, which a read-write mapping inside it requires. pub(crate) writable: bool, - /// Whether OpenVMM hides everything but the allowed paths, because the root is no mapping. + /// Whether OpenVMM hides everything but the allowed paths, because the root is no mapping, + /// which only records of earlier crates hold. pub(crate) hidden: bool, /// Canonical host paths inside the child that OpenVMM hides. pub(crate) denied: Vec, @@ -180,10 +188,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. 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. +/// that is both read-only and read-write is mapped read-only. A whole volume cannot be mapped, +/// and a policy that would need more children than OpenVMM offers is refused. 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(); @@ -209,7 +217,9 @@ pub(crate) fn plan(policy: &FilesystemPolicy) -> Result> { path.display() )); } - if let Some(reason) = volume_root_refusal(&canonical, metadata.is_dir()) { + if metadata.is_dir() + && let Some(reason) = volume_root_refusal(&canonical) + { return unsupported(format!("{field} entry {} {reason}", path.display())); } let Some(target) = guest_path(&canonical) else { @@ -275,9 +285,10 @@ pub(crate) fn plan(policy: &FilesystemPolicy) -> Result> { let roots = export_roots(&mapped)?; if roots.len() > MAX_CHILDREN { + // Exporting directories for some of the files would grant more than the files. return unsupported(format!( - "the mapped paths lie in {} separate host directories, but OpenVMM exports at most \ - {MAX_CHILDREN}", + "the mapped paths need {} exported directories and files, but OpenVMM exports at \ + most {MAX_CHILDREN}; map directories that contain the files instead", roots.len() )); } @@ -300,17 +311,21 @@ pub(crate) fn plan(policy: &FilesystemPolicy) -> Result> { .iter() .filter(|entry| entry.canonical.starts_with(&root)) .collect(); - let hidden = !inside.iter().any(|entry| entry.canonical == root); - let visible = outermost(&inside); + // Every root is a mapping itself, so the guest reaches all of it. + let Some(own) = inside.iter().find(|entry| entry.canonical == root) else { + return Err(Error::backend_error(format!( + "{} is exported but not mapped", + root.display() + ))); + }; 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) { + // Unless the read-write mappings cover the whole child, OpenVMM limits writes to them. + let mut write: Vec = if writable && own.read_only { outermost(&read_write) .iter() .map(|entry| entry.canonical.clone()) @@ -319,45 +334,32 @@ pub(crate) fn plan(policy: &FilesystemPolicy) -> Result> { 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))) - }) + .filter(|path| path.starts_with(&root)) .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)?; + policy_bytes += check_policy_paths(&root, &child_denied, "hide")?; + policy_bytes += check_policy_paths(&root, &write, "make writable")?; let denied_identities = child_denied .iter() .map(|path| movable(path).then(|| identity(path)).transpose()) .collect::>>()?; children.push(Child { root, + file: !own.directory, writable, - hidden, + hidden: false, denied: child_denied, denied_identities, - allowed, + allowed: Vec::new(), write, }); } if policy_bytes > MAX_AGGREGATE_POLICY_BYTES { return unsupported(format!( - "the paths that OpenVMM is to hide, expose, or make writable exceed its \ + "the paths that OpenVMM is to hide or make writable exceed its \ {MAX_AGGREGATE_POLICY_BYTES}-byte limit" )); } @@ -402,45 +404,22 @@ pub(crate) fn plan(policy: &FilesystemPolicy) -> Result> { 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", - ) - } +/// Explains why a mapped host directory, given in canonical form, cannot be exported, or returns +/// `None` if it can: a whole volume. It inspects only the path, not the host. +fn volume_root_refusal(canonical: &Path) -> Option<&'static str> { + (!canonical + .components() + .any(|component| matches!(component, Component::Normal(_)))) + .then_some( + "is a whole volume, which the openvmm backend does not map; map the directories inside \ + it", + ) } -/// 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. +/// Returns the paths to export, in order: the outermost mapped directories and the mapped files +/// outside them, each exported alone. Distinct paths must not name one directory or file. 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()); - } - } + let mut candidates: Vec = mapped.iter().map(|entry| entry.canonical.clone()).collect(); candidates.sort(); candidates.dedup(); let roots: Vec = candidates @@ -452,13 +431,14 @@ fn export_roots(mapped: &[Mapped]) -> Result> { }) .cloned() .collect(); - // OpenVMM refuses one directory exported twice, which bind mounts make possible. + // OpenVMM refuses one object exported twice, which bind mounts and hard links 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", + "{} and {} are the same host directory or file; map it once", other.display(), root.display() ))); @@ -535,12 +515,14 @@ fn outermost<'a>(entries: &[&'a Mapped]) -> Vec<&'a Mapped> { .collect() } -/// Checks that every mapped and denied path inside a read-write mapping still names the object -/// it named at provision. +/// Checks that every child's root is still the kind of object that provision exported, and that +/// every mapped and denied path inside a read-write mapping still names the object it named at +/// provision. /// -/// A workload with a read-write mapping could otherwise rename a denied or read-only object and -/// put a decoy at its path, and the next start would protect the decoy instead. The VM is -/// stopped while this runs, so no workload can race with it. +/// A file that a directory replaced would otherwise grant the whole directory, and a workload +/// with a read-write mapping could rename a denied or read-only object and put a decoy at its +/// path, so the next start would protect the decoy instead. The VM is stopped while this runs, so +/// no workload can race with it, and OpenVMM checks each child's kind again when it opens it. pub(crate) fn verify(mapping: &HostMapping) -> Result<()> { let changed = |path: &Path| { Err(Error::backend_error(format!( @@ -549,6 +531,18 @@ pub(crate) fn verify(mapping: &HostMapping) -> Result<()> { path.display() ))) }; + for child in &mapping.children { + let kind_matches = fs::symlink_metadata(&child.root).is_ok_and(|metadata| { + if child.file { + metadata.is_file() + } else { + metadata.is_dir() + } + }); + if !kind_matches { + return changed(&child.root); + } + } for bind in &mapping.binds { let Some(expected) = bind.identity else { continue; @@ -607,15 +601,10 @@ fn check_root(root: &Path) -> 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 { +/// Returns their combined length. +fn check_policy_paths(root: &Path, paths: &[PathBuf], action: &str) -> Result { let unsupported = |message: String| Err(Error::policy_validation(message)); - let count = paths.len() + usize::from(with_root); + let count = paths.len(); if count > MAX_POLICY_PATHS { return unsupported(format!( "OpenVMM can {action} at most {MAX_POLICY_PATHS} paths in one exported directory, \ @@ -731,13 +720,18 @@ pub(crate) fn guest_table(mapping: &HostMapping) -> Vec { .collect() } -/// OpenVMM options that export the mapping's directories with their access policies. +/// OpenVMM options that export the mapping's directories and files with their access policies. +/// A file child carries the `file` flag, so OpenVMM refuses a directory that replaced the file. pub(crate) fn openvmm_arguments(mapping: &HostMapping) -> Vec { 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" }; + let kind = if child.file { ",file" } else { "" }; arguments.push("--mount-child".to_owned()); - arguments.push(format!("{index},{},{mode}", display_path(&child.root))); + arguments.push(format!( + "{index},{},{mode}{kind}", + display_path(&child.root) + )); } let mut add = |option: &str, path: &Path| { arguments.push(option.to_owned()); @@ -977,51 +971,161 @@ mod tests { } #[test] - fn mapped_files_are_exposed_through_their_hidden_parents() { + fn mapped_files_are_exported_alone() { 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. + // Not exported at all: no child holds it. 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); + // Each file is a child of its own, without its directory, beside the directories. + let children: Vec<(PathBuf, bool, bool, bool)> = mapping + .children + .iter() + .map(|child| (child.root.clone(), child.file, child.writable, child.hidden)) + .collect(); assert_eq!( - child.allowed, - ["config.json", "out", "run.sh", "src"].map(|name| work.join(name)) + children, + [ + (work.join("config.json"), true, false, false), + (work.join("out"), false, true, false), + (work.join("run.sh"), true, true, false), + (work.join("src"), false, false, false), + ] + ); + assert!( + mapping + .children + .iter() + .all(|child| child.allowed.is_empty() && child.write.is_empty()) + ); + assert_eq!( + mapping.children[3].denied, + [work.join("src").join("secret")] ); - // 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), + ("0".to_owned(), true), + ("1".to_owned(), false), + ("2".to_owned(), false), + ("3".to_owned(), true), ] ); + // OpenVMM exposes each file as the child itself, so nothing hides its directory, and the + // `file` flag makes it refuse a directory in the file's place. let arguments = openvmm_arguments(&mapping); + assert_eq!( + option_values(&arguments, "--mount-child")[0], + format!("0,{},ro,file", display_path(&work.join("config.json"))) + ); assert_eq!( option_values(&arguments, "--mount-deny"), - [ - display_path(&work), - display_path(&work.join("src").join("secret")) - ] + [display_path(&work.join("src").join("secret"))] + ); + assert!(option_values(&arguments, "--mount-allow").is_empty()); + assert!(option_values(&arguments, "--mount-write").is_empty()); + + // A record without file children leaves the field out, and one with them names it. + let record = serde_json::to_value(&mapping).unwrap(); + assert_eq!(record["children"][0]["file"], true); + assert!(record["children"][1].get("file").is_none()); + let restored: HostMapping = serde_json::from_value(record).unwrap(); + assert_eq!(restored, mapping); + + // A file inside a mapped directory belongs to that directory's child. + let nested = FilesystemPolicy { + readonly_paths: vec![base.join("work/config.json")], + readwrite_paths: vec![base.join("work")], + ..FilesystemPolicy::default() + }; + if !cfg!(windows) { + let mapping = plan(&nested).unwrap().unwrap(); + assert_eq!(mapping.children.len(), 1); + assert!(!mapping.children[0].file); + assert_eq!( + sources(&mapping), + [("0".to_owned(), false), ("0/config.json".to_owned(), true)] + ); + } + + // One file reached through two names is one object, which is mapped once. + let link = base.join("work/config-link.json"); + fs::hard_link(base.join("work/config.json"), &link).unwrap(); + let aliased = FilesystemPolicy { + readonly_paths: vec![base.join("work/config.json"), link], + ..FilesystemPolicy::default() + }; + let error = plan(&aliased).unwrap_err(); + assert!( + error.message().contains("same host directory or file"), + "{error}" ); - assert_eq!(option_values(&arguments, "--mount-allow").len(), 4); - assert_eq!(option_values(&arguments, "--mount-write").len(), 2); + } + + #[test] + fn policies_beyond_the_children_of_an_aggregate_are_refused() { + let directory = root(); + let base = directory.path(); + let files = |parent: &str, count: usize| -> Vec { + (0..count) + .map(|index| { + let path = base.join(parent).join(format!("file-{index:03}")); + fs::write(&path, b"x").unwrap(); + path + }) + .collect() + }; + // Up to 256 files are children of their own, however many share a directory. + let policy = FilesystemPolicy { + readonly_paths: files("tools", 256), + ..FilesystemPolicy::default() + }; + let mapping = plan(&policy).unwrap().unwrap(); + assert_eq!(mapping.children.len(), 256); + assert!( + mapping + .children + .iter() + .all(|child| child.file && !child.hidden && child.allowed.is_empty()) + ); + + // More are refused rather than exported through their directory, which would grant + // more than the files. + let mut readonly_paths = files("tools", 256); + readonly_paths.extend(files("build-output", 1)); + let error = plan(&FilesystemPolicy { + readonly_paths, + ..FilesystemPolicy::default() + }) + .unwrap_err(); + assert_eq!(error.code(), ErrorCode::PolicyValidation); + assert!(error.message().contains("at most 256"), "{error}"); + assert!( + error + .message() + .contains("map directories that contain the files instead"), + "{error}" + ); + + // A policy that grants the directory instead needs one child. + let mut readonly_paths = vec![base.join("tools")]; + readonly_paths.extend(files("build-output", 1)); + let mapping = plan(&FilesystemPolicy { + readonly_paths, + ..FilesystemPolicy::default() + }) + .unwrap() + .unwrap(); + assert_eq!(mapping.children.len(), 2); } #[test] @@ -1168,44 +1272,20 @@ mod tests { } #[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"], - ) + fn whole_volumes_are_refused() { + // The paths are only inspected, so nothing is created at a volume root. A file directly + // in a volume's root is exported alone, so only a whole volume is refused. + let (volumes, directories): (&[&str], &[&str]) = if cfg!(windows) { + (&[r"C:\", r"D:\"], &[r"C:\work", r"D:\a\b"]) } else { - ( - &["/"], - &["/work", "/srv/a/b"], - &["/notes.txt", "/build.log"], - &["/work/notes.txt", "/srv/a/b/build.log"], - ) + (&["/"], &["/work", "/srv/a/b"]) }; for volume in volumes { - let reason = volume_root_refusal(Path::new(volume), true).unwrap(); + let reason = volume_root_refusal(Path::new(volume)).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); + assert_eq!(volume_root_refusal(Path::new(directory)), None); } } @@ -1252,7 +1332,7 @@ mod tests { fs::create_dir_all(base.join(name)).unwrap(); } fs::write(base.join("my files/notes one.txt"), b"notes").unwrap(); - // Names may contain spaces. + // Names may contain spaces, in a file that a child exposes alone and in a policy path. let spaced = FilesystemPolicy { readonly_paths: vec![base.join("my files/notes one.txt")], readwrite_paths: vec![base.join("work/out")], @@ -1261,10 +1341,10 @@ mod tests { let mapping = plan(&spaced).unwrap().unwrap(); let arguments = openvmm_arguments(&mapping); assert!( - option_values(&arguments, "--mount-allow")[0].ends_with("notes one.txt"), + option_values(&arguments, "--mount-child")[0].ends_with("notes one.txt,ro,file"), "{arguments:?}" ); - assert!(option_values(&arguments, "--mount-deny")[1].ends_with("My Secrets")); + assert!(option_values(&arguments, "--mount-deny")[0].ends_with("My Secrets")); // But they must not begin or end with one. let leading = FilesystemPolicy { readwrite_paths: vec![base.join("work/out")], @@ -1317,20 +1397,24 @@ mod tests { }) .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}"); + // One exported directory hides at most 128 denied paths, and singles out at most 128 + // read-write paths inside a read-only one. + for policy in [ + FilesystemPolicy { + readonly_paths: vec![base.join("tools")], + denied_paths: files(129), + ..FilesystemPolicy::default() + }, + FilesystemPolicy { + readonly_paths: vec![base.join("tools")], + readwrite_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] @@ -1377,6 +1461,32 @@ mod tests { fs::remove_dir_all(base.join("tools")).unwrap(); fs::create_dir(base.join("tools")).unwrap(); verify(&mapping).unwrap(); + + // Every child keeps its kind: a directory in place of a file child would grant all of it, + // and a directory child must remain a directory. + let settings = base.join("settings.json"); + fs::write(&settings, b"{}").unwrap(); + let with_file = FilesystemPolicy { + readonly_paths: vec![settings.clone(), base.join("tools")], + ..FilesystemPolicy::default() + }; + let mapping = plan(&with_file).unwrap().unwrap(); + verify(&mapping).unwrap(); + fs::remove_file(&settings).unwrap(); + fs::create_dir(&settings).unwrap(); + assert_eq!( + verify(&mapping).unwrap_err().code(), + ErrorCode::BackendError + ); + fs::remove_dir(&settings).unwrap(); + fs::write(&settings, b"{}").unwrap(); + verify(&mapping).unwrap(); + fs::remove_dir(base.join("tools")).unwrap(); + fs::write(base.join("tools"), b"").unwrap(); + assert_eq!( + verify(&mapping).unwrap_err().code(), + ErrorCode::BackendError + ); } /// A directory on another volume than the temporary directory, if the host has one: the one diff --git a/aci_edge_sandboxes/src/openvmm/launch.rs b/aci_edge_sandboxes/src/openvmm/launch.rs index 9d5e86ad0..a4025a6ef 100644 --- a/aci_edge_sandboxes/src/openvmm/launch.rs +++ b/aci_edge_sandboxes/src/openvmm/launch.rs @@ -237,6 +237,7 @@ mod tests { record.filesystem = Some(filesystem::HostMapping { children: vec![filesystem::Child { root: "/host/work".into(), + file: false, writable: false, hidden: false, denied: vec!["/host/work/secret".into()], @@ -313,6 +314,7 @@ mod tests { record.filesystem = Some(filesystem::HostMapping { children: vec![filesystem::Child { root: "/host/work".into(), + file: false, writable: false, hidden: false, denied_identities: vec![None; denied.len()], diff --git a/aci_edge_sandboxes/tests/openvmm_e2e.rs b/aci_edge_sandboxes/tests/openvmm_e2e.rs index ddf8c23cd..13e9d4d46 100644 --- a/aci_edge_sandboxes/tests/openvmm_e2e.rs +++ b/aci_edge_sandboxes/tests/openvmm_e2e.rs @@ -357,6 +357,92 @@ fn mapped_files_hide_their_siblings_and_names_may_hold_spaces() { } } +#[test] +#[ignore = "requires a hypervisor and NVX guest artifacts; run scripts/nvx.py test-aci-edge-sandboxes"] +fn single_files_are_granted_without_their_directories() { + // The layout of microsoft/nvx#282: a read-only settings file and a read-write output file, + // each beside a file that the policy does not grant. + let host = tempfile::tempdir().unwrap(); + let base = host.path(); + for (name, contents) in [ + ("config/settings.json", "settings"), + ("config/secret.json", "secret"), + ("results/output.txt", "output"), + ("results/other.txt", "other"), + ] { + let path = base.join(name); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, contents).unwrap(); + } + let request = ProvisionRequest::new().with_filesystem(FilesystemPolicy { + readonly_paths: vec![base.join("config/settings.json")], + readwrite_paths: vec![base.join("results/output.txt")], + denied_paths: Vec::new(), + }); + let (nvx, backend) = client("grants"); + let sandbox = started(&nvx, &backend, &request); + let sandbox_id = id(&sandbox); + let guest = |name: &str| guest_path(&base.join(name)).unwrap(); + let output = run( + &nvx, + sandbox_id, + script( + "cat \"$1\" && ! echo changed > \"$1\" && ! rm -f \"$1\" && echo updated > \"$2\" \ + && cat \"$2\" && ls -a \"$3\" && ls -a \"$4\" && [ ! -e \"$5\" ] && [ ! -e \"$6\" ]", + &[ + guest("config/settings.json"), + guest("results/output.txt"), + guest("config"), + guest("results"), + guest("config/secret.json"), + guest("results/other.txt"), + ], + ), + ); + assert!(output.outcome.success(), "{output:?}"); + // The guest's directories hold only the granted files. + assert_eq!( + output.stdout, b"settingsupdated\n.\n..\nsettings.json\n.\n..\noutput.txt\n", + "{output:?}" + ); + assert_eq!( + fs::read_to_string(base.join("config/settings.json")).unwrap(), + "settings" + ); + assert_eq!( + fs::read_to_string(base.join("results/output.txt")).unwrap(), + "updated\n" + ); + // Nothing beside the granted files changed, and nothing was created there. + for (directory, entries) in [ + ( + "config", + [("secret.json", "secret"), ("settings.json", "settings")], + ), + ( + "results", + [("other.txt", "other"), ("output.txt", "updated\n")], + ), + ] { + let mut found: Vec<(String, String)> = fs::read_dir(base.join(directory)) + .unwrap() + .map(|entry| { + let entry = entry.unwrap(); + ( + entry.file_name().into_string().unwrap(), + fs::read_to_string(entry.path()).unwrap(), + ) + }) + .collect(); + found.sort(); + let expected: Vec<(String, String)> = entries + .iter() + .map(|(name, contents)| ((*name).to_owned(), (*contents).to_owned())) + .collect(); + assert_eq!(found, expected, "{directory}"); + } +} + #[test] #[ignore = "requires a hypervisor and NVX guest artifacts; run scripts/nvx.py test-aci-edge-sandboxes"] fn many_mappings_are_mounted() { diff --git a/aci_edge_sandboxes/tests/openvmm_fake.rs b/aci_edge_sandboxes/tests/openvmm_fake.rs index f138e648a..96fa40f6f 100644 --- a/aci_edge_sandboxes/tests/openvmm_fake.rs +++ b/aci_edge_sandboxes/tests/openvmm_fake.rs @@ -1486,6 +1486,87 @@ fn filesystem_and_network_policies_reach_openvmm() { nvx.deprovision(&sandbox_id).unwrap(); } +#[test] +fn mapped_files_are_exported_without_their_directories() { + let fixture = Fixture::new(); + let nvx = fixture.nvx(); + let base = fixture.directory.path(); + // The layout of microsoft/nvx#282: a read-only and a read-write file, each beside a file + // that the policy does not map. + for (name, contents) in [ + ("config/settings.json", "settings"), + ("config/secret.json", "secret"), + ("results/output.txt", "output"), + ("results/other.txt", "other"), + ] { + let path = base.join(name); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, contents).unwrap(); + } + let request = ProvisionRequest::new().with_filesystem(FilesystemPolicy { + readonly_paths: vec![base.join("config/settings.json")], + readwrite_paths: vec![base.join("results/output.txt")], + 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 value = |name: &str| { + arguments + .windows(2) + .filter(|pair| pair[0] == name) + .map(|pair| pair[1].clone()) + .collect::>() + }; + // Each file is a child of its own, which OpenVMM must find to be a regular file, and nothing + // hides, exposes, or narrows its directory. + let children = value("--mount-child"); + assert_eq!(children.len(), 2, "{arguments:?}"); + for (child, (name, mode)) in children + .iter() + .zip([("config/settings.json", "ro"), ("results/output.txt", "rw")]) + { + let (_, rest) = child.split_once(',').unwrap(); + let rest = rest.strip_suffix(",file").expect(child); + let (host, child_mode) = rest.rsplit_once(',').unwrap(); + assert!(Path::new(host).ends_with(name), "{child}"); + assert_eq!(child_mode, mode, "{child}"); + } + for option in ["--mount-deny", "--mount-allow", "--mount-write"] { + assert!(value(option).is_empty(), "{option}: {arguments:?}"); + } + let record = state_json(&fixture, &sandbox_id, "sandbox.json"); + let children = record["filesystem"]["children"].as_array().unwrap(); + assert!( + children.iter().all(|child| child["file"] == true), + "{record}" + ); + 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!(entries, [("0", true), ("1", false)]); + nvx.stop(&sandbox_id).unwrap(); + + // A directory that replaces a granted file would grant all of it, so start refuses it. + let output = base.join("results/output.txt"); + fs::remove_file(&output).unwrap(); + fs::create_dir(&output).unwrap(); + let error = nvx.start(&sandbox_id).unwrap_err(); + assert_eq!(error.code(), ErrorCode::BackendError, "{error}"); + fs::remove_dir(&output).unwrap(); + fs::write(&output, "output").unwrap(); + nvx.start(&sandbox_id).unwrap(); + nvx.stop(&sandbox_id).unwrap(); + nvx.deprovision(&sandbox_id).unwrap(); +} + #[test] fn mapped_files_are_not_working_directories() { let fixture = Fixture::new(); diff --git a/doc/ci.md b/doc/ci.md index 5828fe5aa..0f94bd7c1 100644 --- a/doc/ci.md +++ b/doc/ci.md @@ -266,14 +266,23 @@ managed sandbox then repeats 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, 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 +attaches a read-write `/workspace` share, a read-only `/opt/hostedtoolcache` +share, a read-only `/config/settings.json` file, and a read-write +`/results/output.txt` file to one sandbox at the same time, as the children +of one aggregate virtio-fs device; each directory has its own denied +subdirectory, and each file lies beside a file that no share exposes. 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 -one-shot run and for a managed sandbox whose format-4 configuration must -persist both shares and whose `stop` must leave a cleanly unmounted scratch +cannot write into the read-only share. It requires each file to be a +`virtiofs` bind mount with its mode, alone in its guest directory, the +read-only file to refuse writes with `EROFS`, and the workload to rewrite the +read-write file but neither remove nor rename either one. The host requires +the guest's writes in the workspace and the read-write file, an unchanged tool +cache, read-only file, and denied files, and unchanged files beside the shared +files, both for a +one-shot run and for a managed sandbox whose format-6 configuration must +persist every share and its kind and whose `stop` must leave a cleanly +unmounted scratch filesystem. Each backend then attaches a read-write `/workspace` share whose [access policy](run.md#access-policy) denies `nvx-denied`, allows `nvx-denied/nvx-allowed`, and makes only `nvx-writable` writable, with every diff --git a/doc/design/machine-and-device-abi.md b/doc/design/machine-and-device-abi.md index e300041c6..9f8c84da5 100644 --- a/doc/design/machine-and-device-abi.md +++ b/doc/design/machine-and-device-abi.md @@ -377,15 +377,26 @@ lifetimes. It requires FUSE 7.31 or newer and caps writes at 1 MiB. Without `--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 +`--mount-child NAME,HOST_PATH[,ro|rw][,file]` 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 +directory or, with the `file` flag, regular file 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. The flag, not the host +object, sets a child's kind, and OpenVMM refuses a host path of the other kind. +A file child serves only its file, as the child +itself: its volume is the file's parent directory, narrowed to the file's name, +which the guest never reaches, so the guest cannot list or change anything +beside the file, a lookup below the file fails with `ENOTDIR`, and every +request fails with `EACCES` while anything but a regular file holds the name. +A file child takes no policy paths, because its mode applies to the whole file, +and restore requires each child to be the same kind. `--mount` exports only a +directory, because the guest mounts the device's root. 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 +no child can reach the files of another under a different policy, while a file +child may lie beside a directory child. 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 @@ -444,15 +455,18 @@ 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 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 +aggregate, the same target and the same children, names, kinds, 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 +device-private schema version 8, or 9 when a child is a file, which earlier +releases reject instead of restoring the file child as a directory. 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 733187612..b2170dff1 100644 --- a/doc/design/sandbox-filesystem-and-agent-architecture.md +++ b/doc/design/sandbox-filesystem-and-agent-architecture.md @@ -113,37 +113,46 @@ workload. ### Live host shares 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. +cache, or a single host file, such as a settings file, to the workload without +copying it into an image: edits are visible in both directions without staging +or copy-back. -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 +The microVM has one virtio-fs slot, tag `microvm`. One `--mount` of a directory +attaches a HostFs server to it. Several, or a shared file, 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 +policies. A file child serves only its file, never the file's directory or +siblings, because the guest mounts the device's root, which is a directory. +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 another or reach its files under a different policy, and it -rejects shares whose kernel command-line tokens exceed the sandbox's budget. +starts, NVX rejects policy paths outside their share or for a shared file, and +guest targets or host paths that equal or contain one another, because one +share could 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 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 +read-write when any child is. NVX then adds one +`nvx_share=NAME,TARGET,MODE[,file]` token per child, where `file` marks a +shared file. 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 +a share outside that root. The last component of a shared file's target is a +regular file, which the agent creates when the container root lacks it, and of +any other target a directory. 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 +over, such as `/etc/machine-id`, 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 @@ -152,8 +161,9 @@ 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. -[Live host-directory shares](../run.md#live-host-directory-shares) describes +shares, including whether each is a file, when it is provisioned, and +reattaches them on every start. +[Live host shares](../run.md#live-host-shares) describes the options and their rules. ## Image preparation and distribution (Proposed) diff --git a/doc/openvmm-upstream-roadmap.md b/doc/openvmm-upstream-roadmap.md index d5b6ad66d..fa4765c4f 100644 --- a/doc/openvmm-upstream-roadmap.md +++ b/doc/openvmm-upstream-roadmap.md @@ -5,7 +5,7 @@ - OpenVMM fork base: `e1cdbd916d3aacb216309c8e7779b832546759a1`. - Mapped fork tip: `2728f33ea9a44d2d0fb956d9f01768ae2100fb62`, position 166. The graph, PR summary, and commit mapping below cover positions 001-166. -- NVX's pinned OpenVMM revision: `7bf0ee28b826374a2d386067df0456b81bcae818`, 139 commits past the +- NVX's pinned OpenVMM revision: `2130f24b5a51561cc6224fe5a0aff93cb121ac32`, 142 commits past the mapped tip on the same first-parent chain. These commits are not yet assigned to PRs; see [Commits after the mapped tip](#commits-after-the-mapped-tip). - Upstream main: `1fd455b19e7c69c72a2d375d0ed1cb35d901c688` (2026-10-01), 18 commits past @@ -199,7 +199,7 @@ e1cdbd916 -> PR18/PR23 split [162] -> PR18 fixup [163] -> PR25 fixup [164] -> PR27 [165-166] -> 2728f33ea (mapped tip) - -> unassigned [167-305] -> 7bf0ee28b (NVX pin) + -> unassigned [167-308] -> 2130f24b5 (NVX pin) ``` For upstream review, PR01-PR08 and PR14 can be rebased independently onto current upstream main. @@ -215,7 +215,7 @@ can be submitted independently if preferred. ## Commits after the mapped tip -The NVX pin extends the chain past position 166 without merges. The 139 later commits are not +The NVX pin extends the chain past position 166 without merges. The 142 later commits are not yet classified or assigned to PRs. This table groups them by topic in chain order; ranges are inclusive, as in the [PR summary](#pr-summary). @@ -234,6 +234,7 @@ inclusive, as in the [PR summary](#pr-summary). | 298 | `f12dcf5e8` | 1 | RAM-backed microVM overlays without scratch | | 299-301 | `3796991d5..e5bf79776` | 3 | Egress rules that select a protocol or a destination-port range, and IPv6 in the portable network profile | | 302-305 | `3072d7a91..7bf0ee28b` | 4 | Several host directories on one virtio-fs slot, their snapshot, and their path policy | +| 306-308 | `be5297817..2130f24b5` | 3 | Single host files as aggregate virtio-fs children, their saved kinds, and the `file` flag that requests one | Some of these commits change mapped work rather than add to it. Positions 255-259 remove the legacy microVM clock paths and change files that PR10 and PR17 also change. Position 261 makes diff --git a/doc/run.md b/doc/run.md index 7ec56f71b..d6c95f73e 100644 --- a/doc/run.md +++ b/doc/run.md @@ -449,12 +449,28 @@ python3 scripts/nvx.py run \ --mount "/opt/hostedtoolcache,/opt/hostedtoolcache,ro" ``` -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 +A `--mount` may also share one regular host file. The guest then reaches that +file at its target, but neither its directory nor its siblings, for example a +read-only settings file next to a read-write results file: + +```bash +python3 scripts/nvx.py run \ + --mount "/config/settings.json,/srv/app/config/settings.json,ro" \ + --mount "/results/output.txt,/srv/app/results/output.txt,rw" +``` + +With several mappings, or a shared file, NVX attaches the slot as an +aggregate: OpenVMM's `--mount-aggregate /run/nvx/shares` with one +`--mount-child NAME,HOST_PATH,MODE[,file]` per mapping, in order, where +OpenVMM's `file` flag marks a shared file, so OpenVMM refuses a directory that +replaced the file rather than export all of it. The initramfs mounts +the aggregate at `/run/nvx/shares`, whose root lists one entry per mapping and +only root can enter, and binds each entry at its target with its mode and +`nosuid,nodev`, as an `nvx_share=NAME,TARGET,MODE[,file]` token on the kernel +command line describes. The `file` suffix marks a shared file, which the +initramfs binds at a regular file that it creates at the target, along with +the target's missing directories; an existing target must already be a +regular file. 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 @@ -463,16 +479,26 @@ 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 +fails with `EROFS` first. OpenVMM serves a shared file alone: the guest cannot +list, inspect, or change the file's directory or reach its siblings, a lookup +below the file fails with `ENOTDIR`, and a read-write file can be rewritten in +place but not replaced, renamed, or removed. OpenVMM follows the file's name, +so the guest sees the file that host software, such as an editor, saves over +it, and requests fail with `EACCES` while anything but a regular file holds the +name. Guest targets that equal or contain one another or `/run/nvx/shares`, +and host paths that equal or contain one another, such as a shared file inside +a shared directory or two hard links to one file, 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. +order, with the same canonical host paths, kinds, targets, modes, and +filesystem identities, so a restore refuses a shared file that was replaced +since the capture. A repeatable `--mount-deny HOST_PATH` hides an existing file or directory -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 +inside a mapped root. With several mappings, or a shared file, each +`--mount-deny`, `--mount-allow`, and `--mount-write` path must be absolute, and +it applies to the mapping whose root contains it. A shared file takes none of +them, because its mode applies to the whole file. Denied names are omitted from directory listings and remain inaccessible through `..`, a symlink/junction, or another mount of the same virtio-fs device. Names may contain spaces, but not begin or end with one. @@ -494,16 +520,16 @@ Treat links in a writable share as untrusted when host software later reads the directory. By default, OpenVMM performs every guest operation on the mapping as its own user. On a Linux host, `--mount-owner caller` instead performs each one as the -guest caller's UID and GID and squashes guest root to the owner of the host -directory, so files that guest root creates are owned by that owner rather -than by root or OpenVMM; see [File ownership](#file-ownership). A snapshot -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`, but -not with several; after resume, mount it explicitly inside the guest because -the initramfs hook has already completed: +guest caller's UID and GID and squashes guest root to the owner of the shared +host directory or file, so files that guest root creates are owned by that +owner rather than by root or OpenVMM; see [File ownership](#file-ownership). A +snapshot 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` of a +directory, but not with several or with a file; after resume, mount it +explicitly inside the guest because the initramfs hook has already completed: ```sh mkdir -p /mnt/host @@ -567,6 +593,12 @@ keep twice `MEMORY_MAX` within the guest's free memory. With `--arg TARGET --arg ro|rw`, it also checks a live share at `TARGET` as described below, including symbolic links in an `rw` share; the share needs a host-created, world-writable `nvx-links` directory for them. +With `--arg TARGET --arg file-ro|file-rw`, it checks a shared file at +`TARGET`: a `virtiofs` bind with the given mode, alone in its directory, +holding `host-to-guest`, which the workload can neither remove nor rename. It +requires a write to a `file-ro` file to fail with `EROFS`, so the host must let +every identity write the file, and it writes `guest-to-host` to a `file-rw` +file. Repeat the pair to check several shares; with an `rw` and an `ro` share, it also verifies that a link in the `rw` share cannot write into the `ro` share. With `--mount-owner caller`, `--arg caller` performs the `rw` checks and also @@ -606,16 +638,17 @@ The outer agent retains the initramfs root; the capability-stripped child enters only the assembled root with `chroot`, because Linux cannot `pivot_root` away from an initramfs `rootfs`. -### Live host-directory shares +### Live host shares `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 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: +the microVM's virtio-fs device, and several shares, or a shared file, 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 \ @@ -627,18 +660,25 @@ python3 scripts/nvx.py sandbox \ --entrypoint /bin/sh ``` +A `--mount` whose host path is a regular file shares only that file, as +[virtio-fs host mapping](#virtio-fs-host-mapping) describes: the workload sees +it at its target, but neither its directory nor its siblings. A shared file +takes no `--mount-deny`, `--mount-allow`, or `--mount-write` path, because its +mode applies to the whole file. + 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 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 +paths 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 -the other directory's ancestors, so a share inside a bind mount of the other -share's directory is rejected before OpenVMM starts. On Linux, OpenVMM also +mode, so a shared file may lie beside a shared directory, but not inside it. +NVX and OpenVMM compare the host paths by resolved path and by file +identity, so one directory or file reached through two paths, such as a bind +mount or a hard link, is rejected too. NVX also compares each share's identity +with those of the other share's ancestors, so a share inside a bind mount of +the other share's directory is rejected before OpenVMM starts. On Linux, +OpenVMM also compares the filesystem sources that each directory reaches, through the mount that contains it and every mount below it, so it also rejects a bind mount or nested mount that exposes part of one share inside the other. That @@ -650,19 +690,22 @@ 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 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 +namespace. With several shares, or a shared file, 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[,file]` tokens that NVX adds to the kernel command +line describe. It binds a shared file at a regular file, which it creates when +the container root has nothing at the target. 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. +path; `/` and `/etc`, and the `/etc/machine-id`, `/proc`, `/sys`, `/dev`, and +`/.nvx-agent` trees, are reserved for the container runtime, which replaces +`/etc/machine-id` with the workload's machine ID. The guest refuses a target whose path crosses a symbolic link in a container -layer, a repeated child, and overlapping targets, and any validation or mount +layer, a target that a container layer holds as another kind of file than the +share's, 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. @@ -670,13 +713,16 @@ An `rw` share supports the symbolic links that package managers and language toolchains create; see [virtio-fs host mapping](#virtio-fs-host-mapping) for their semantics. The 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 several shares uses format -4, which earlier NVX releases reject rather than start without the other -shares. +sandbox stores each share's absolute host path, kind, mode, denied, allowed, +and writable paths and the ownership mode in its configuration, and reattaches +every share on each `start`, which refuses a host path that no longer holds a +directory or regular file as recorded. 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. +A configuration with a shared file uses format 6, which earlier NVX releases +reject rather than start without the file. #### Access policy diff --git a/doc/usage.md b/doc/usage.md index dc267362b..f2917bf2e 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -501,11 +501,11 @@ 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 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). | +| `--mount GUEST_TARGET,HOST_PATH[,ro\|rw]` | none | Expose a host directory, or a single regular host file without its directory, at the absolute guest target. Repeat to expose more directories or files, 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; a shared file always uses the aggregate. Targets and host paths 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, kinds, targets, modes, and ownership mode; a dormant-slot restore may attach one new directory 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, or a shared file, the path must be absolute; a shared file takes none. | +| `--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, or a shared file, the path must be absolute; a shared file takes none. 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, or a shared file, the path must be absolute; a shared file takes none. 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 or file. `caller` performs each one as the guest caller's UID and GID and squashes guest root to the owner of the shared directory or file; 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`. | | `--network-egress {allow,deny}` | `allow` | Set the default guest egress policy. | @@ -609,11 +609,11 @@ 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 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 GUEST_TARGET,HOST_PATH[,ro\|rw]` | none | Live-share a host directory, or a single regular host file without its 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; a shared file always uses the aggregate. Targets and host paths 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. `/` and `/etc`, and the `/etc/machine-id`, `/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, which must share a directory. 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). | +| `--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 shared host directory or file, 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`. | | `--network-egress {allow,deny}` | `allow` | Set the default guest egress policy for `run` or `provision`. | diff --git a/guest/common/nvx-hostmount b/guest/common/nvx-hostmount index dcf794ceb..837ec640e 100644 --- a/guest/common/nvx-hostmount +++ b/guest/common/nvx-hostmount @@ -2,10 +2,12 @@ # 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. +# aggregate of host directories and files, its root lists one child per +# directory or file, which only root can enter, and each +# nvx_share=CHILD,TARGET,MODE[,file] token binds one child at TARGET with +# MODE, without set-user-ID programs or device nodes; the file suffix marks a +# child that is a host file, which is bound at a file. With --unmount, the +# binds and then the share are unmounted. fail() { echo "virtfs: $*" >&2 @@ -36,11 +38,19 @@ set +f [ -n "$vmode" ] || vmode=ro [ "$aggregate" = true ] || shares= -# Splits the share specification $1 into share_child, share_target, and -# share_mode. +# Splits the share specification $1 into share_child, share_target, +# share_mode, and share_kind, which is file for a child that is a host file +# and directory otherwise. parse_share() { share_child=${1%%,*} share_rest=${1#*,} + share_kind=directory + case "$share_rest" in + *,file) + share_kind="file" + share_rest=${share_rest%,file} + ;; + esac share_target=${share_rest%,*} share_mode=${share_rest##*,} case "$share_child" in @@ -52,6 +62,26 @@ parse_share() { fail "invalid live share $1" } +# Reports whether $1 is a mount point. BusyBox's mountpoint inspects only +# directories, so a bound file is found in the mount table instead. +is_mount_point() { + if [ -d "$1" ]; then + mountpoint -q "$1" + else + awk -v target="$1" '$2 == target { found = 1 } END { exit !found }' \ + /proc/self/mounts + fi +} + +# Makes $1 a regular file to bind a file child at, creating it and its +# missing directories unless it already is one. A link at $1 is refused +# before anything is created, so that creating the file cannot write +# through the link. +make_file_mount_point() { + [ ! -L "$1" ] && mkdir -p "${1%/*}/" && + { [ -e "$1" ] || : >"$1"; } && [ -f "$1" ] +} + if [ "${1:-}" = --unmount ]; then status=0 reversed= @@ -62,7 +92,7 @@ if [ "${1:-}" = --unmount ]; then for share in $reversed; do set +f parse_share "$share" - if mountpoint -q "$share_target" && ! umount "$share_target"; then + if is_mount_point "$share_target" && ! umount "$share_target"; then echo "virtfs: failed to unmount live share $share_child at $share_target" >&2 status=1 fi @@ -85,9 +115,16 @@ 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" || + if [ "$share_kind" = file ]; then + [ -f "$vdir/$share_child" ] || fail "live share $share_child is not an exported file" + make_file_mount_point "$share_target" || + fail "failed to create the file mount point of live share $share_child at $share_target" + else + [ -d "$vdir/$share_child" ] || fail "live share $share_child is not exported" + mkdir -p "$share_target" || + fail "failed to create the mount point of live share $share_child at $share_target" + fi + if ! 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 diff --git a/guest/common/nvx-init-agent b/guest/common/nvx-init-agent index 02da27623..56f4a8527 100644 --- a/guest/common/nvx-init-agent +++ b/guest/common/nvx-init-agent @@ -162,7 +162,7 @@ validate_share_target() { # The container runtime mounts or binds these paths after the share, so a # share there would be shadowed or would expose host data to runtime writes. case "$1" in - /proc | /proc/* | /sys | /sys/* | /dev | /dev/* | /.nvx-agent | /.nvx-agent/* | /etc) + /proc | /proc/* | /sys | /sys/* | /dev | /dev/* | /.nvx-agent | /.nvx-agent/* | /etc | /etc/machine-id | /etc/machine-id/*) fatal "live share target is reserved: $1" ;; esac @@ -171,6 +171,8 @@ validate_share_target() { # 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. +# With $2 set to file, the last component is a regular file, for a share that +# binds a host file, and every other component a directory. make_share_mount_point() { share_path=$rootfs share_remainder=${1#/} @@ -184,6 +186,14 @@ make_share_mount_point() { share_path=$share_path/$share_component if [ -L "$share_path" ]; then fatal "live share target crosses a symbolic link: $1" + elif [ -z "$share_remainder" ] && [ "${2:-directory}" = file ]; then + if [ -e "$share_path" ]; then + [ -f "$share_path" ] || + fatal "live share target is not a regular file: $1" + else + (umask 022 && : >"$share_path") || + fatal "failed to create live share target $1" + fi elif [ -e "$share_path" ]; then [ -d "$share_path" ] || fatal "live share target is not a directory: $1" @@ -242,11 +252,19 @@ mount_live_share() { 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. +# Splits the aggregate child specification $1, CHILD,TARGET,MODE[,file], into +# child_name, child_target, child_mode, and child_kind, which is file for a +# child that is a host file and directory otherwise. parse_share_child() { child_name=${1%%,*} child_rest=${1#*,} + child_kind=directory + case "$child_rest" in + *,file) + child_kind="file" + child_rest=${child_rest%,file} + ;; + esac child_target=${child_rest%,*} child_mode=${child_rest##*,} if [ "$child_rest" = "$1" ] || [ "$child_target" = "$child_rest" ]; then @@ -263,9 +281,10 @@ parse_share_child() { # 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. +# device nodes. The child is a directory, or a host file when $5 is file, and +# the kernel refuses to bind either at a mount point of the other kind. bind_live_share() { - make_share_mount_point "$3" + make_share_mount_point "$3" "$5" mount -o bind "$1/$2" "$share_path" || fatal "failed to bind live share $2 at $3" record_live_share "$share_path" @@ -277,9 +296,9 @@ bind_live_share() { mount_live_shares() { # 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. + # aggregate of host directories and files, which + # nvx_share=CHILD,TARGET,MODE[,file] tokens place. Parse every token + # before mounting anything, so that a malformed bootstrap mounts nothing. share_dir= share_tag= share_mode= @@ -347,7 +366,7 @@ mount_live_shares() { esac share_seen="$share_seen $child_name" claim_share_target "$child_target" - make_share_mount_point "$child_target" + make_share_mount_point "$child_target" "$child_kind" set -f done set +f @@ -360,7 +379,8 @@ mount_live_shares() { for share_spec in $share_children; do set +f parse_share_child "$share_spec" - bind_live_share "$share_dir" "$child_name" "$child_target" "$child_mode" + bind_live_share "$share_dir" "$child_name" "$child_target" "$child_mode" \ + "$child_kind" set -f done set +f diff --git a/guest/common/nvx-sandbox-smoke b/guest/common/nvx-sandbox-smoke index 777cd7aa3..795e86ce0 100644 --- a/guest/common/nvx-sandbox-smoke +++ b/guest/common/nvx-sandbox-smoke @@ -287,6 +287,13 @@ check_share() { echo "NVX-UBUNTU-SANDBOX-SHARE-OK target=$share mode=$share_mode" return fi + case "$share_mode" in + file-ro | file-rw) + check_file_share "$share" "${share_mode#file-}" + echo "NVX-UBUNTU-SANDBOX-SHARE-OK target=$share mode=$share_mode" + return + ;; + esac [ "$(cat "$share/nvx-host-marker")" = host-to-guest ] [ ! -e "$share/nvx-denied" ] case "$share_mode" in @@ -315,6 +322,32 @@ check_share() { echo "NVX-UBUNTU-SANDBOX-SHARE-OK target=$share mode=$share_mode" } +# A share of one host file, bound with mode $2, ro or rw, at a regular file +# that the agent created in a directory of the container root. The workload +# reaches the file and nothing beside it, cannot remove or rename it, and gets +# EROFS from a read-only file; it rewrites a read-write one, and the host +# checks the file and its siblings. +check_file_share() { + share=$1 + require_mount "$share" virtiofs "$2" nosuid nodev + if [ ! -f "$share" ] || [ -L "$share" ]; then + fail "$share is not a regular file" + fi + [ "$(ls -A "${share%/*}")" = "${share##*/}" ] || + fail "${share%/*} holds more than the shared file" + [ "$(cat "$share")" = host-to-guest ] || + fail "$share does not hold the host's contents" + if [ "$2" = ro ]; then + expect_erofs "$share" + else + printf 'guest-to-host\n' >"$share" || + fail "the workload cannot rewrite $share" + fi + if rm -f "$share" 2>/dev/null || mv "$share" "$share.moved" 2>/dev/null; then + fail "the workload removed or renamed $share" + fi +} + # With --mount-owner caller, the workload owns what it creates, so it can # populate the directories that it creates without host-granted access. check_share_ownership() { @@ -503,7 +536,8 @@ check_devices check_cgroup check_image echo "NVX-UBUNTU-SANDBOX-PROFILE-OK uid=65534 gid=65534" -# Each share is a TARGET MODE pair; a final TARGET alone is read-only. +# Each share is a TARGET MODE pair; a final TARGET alone is read-only. A MODE +# of file-ro or file-rw checks a shared file. # `limits MEMORY_MAX PIDS_MAX` checks the workload's resource limits. rw_share= ro_share= diff --git a/openvmm b/openvmm index 7bf0ee28b..2130f24b5 160000 --- a/openvmm +++ b/openvmm @@ -1 +1 @@ -Subproject commit 7bf0ee28b826374a2d386067df0456b81bcae818 +Subproject commit 2130f24b5a51561cc6224fe5a0aff93cb121ac32 diff --git a/scripts/nvx.py b/scripts/nvx.py index 10b799efc..c3b1e64ba 100644 --- a/scripts/nvx.py +++ b/scripts/nvx.py @@ -10,6 +10,7 @@ import subprocess import sys from collections.abc import Sequence +from dataclasses import replace from pathlib import Path from typing import cast @@ -95,9 +96,11 @@ SandboxLaunch, SandboxLayer, SandboxMount, + aggregate_child_argument, aggregate_child_name, aggregate_command_line_fragment, aggregate_share_tokens, + mount_kind, parse_workload_identity, require_mount_owner_supported, ) @@ -146,7 +149,8 @@ def __call__( def _sandbox_mounts(args: argparse.Namespace) -> tuple[SandboxMount, ...]: """Attribute each access-policy path to its share and parse the --mount - options. + options, each of which shares a directory or, when its host path is a + regular file, that file. With one --mount, every --mount-deny, --mount-allow, and --mount-write names a path in it. With several, each names a path in the --mount that @@ -168,16 +172,17 @@ def _sandbox_mounts(args: argparse.Namespace) -> tuple[SandboxMount, ...]: paths[index].append(path) attributed[kind] = paths owner = args.mount_owner or "vmm" - return tuple( - SandboxMount.parse( + parsed: list[SandboxMount] = [] + for index, value in enumerate(mounts): + mount = SandboxMount.parse( value, tuple(attributed["denied"][index]), owner, allowed_paths=tuple(attributed["allowed"][index]), writable_paths=tuple(attributed["writable"][index]), ) - for index, value in enumerate(mounts) - ) + parsed.append(replace(mount, kind=mount_kind(mount.host_path))) + return tuple(parsed) def _run_share(value: str) -> tuple[str, str, str]: @@ -204,9 +209,10 @@ def _validate_run_share_targets(targets: list[str]) -> None: 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 ','" + f"invalid --mount target {target!r}: with several shares, or a " + "shared file, 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 ( @@ -560,7 +566,11 @@ def command_run(args: argparse.Namespace) -> None: if args.cpu_profile is not None: command.extend(["--cpu-profile", args.cpu_profile]) shares = [_run_share(value) for value in args.mount] - if len(shares) == 1: + # A file is shared as the child of an aggregate, even alone, because the + # guest mounts the device's root, which is a directory. + kinds = [mount_kind(Path(host_path)) for _, host_path, _ in shares] + aggregate = len(shares) > 1 or "file" in kinds + if shares and not aggregate: command.extend(["--mount", args.mount[0]]) elif shares: _validate_run_share_targets([target for target, _, _ in shares]) @@ -569,17 +579,22 @@ def command_run(args: argparse.Namespace) -> None: # 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}"]) + command.extend( + [ + "--mount-child", + aggregate_child_argument(name, host_path, access, kinds[index]), + ] + ) 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 + # OpenVMM attributes each path of an aggregate to the child whose # host directory contains it. - if len(shares) > 1 and not Path(path).is_absolute(): + if aggregate and not Path(path).is_absolute(): raise ScriptError( - f"with several --mount shares, {option} {path} must be an " - "absolute host path" + f"with several --mount shares, or a shared file, {option} " + f"{path} must be an absolute host path" ) command.extend([option, str(path)]) if args.mount_owner is not None: @@ -596,13 +611,16 @@ def command_run(args: argparse.Namespace) -> None: 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: + if aggregate and args.restore_snapshot is None: cmdline = " ".join( token for token in ( args.cmdline, *aggregate_share_tokens( - [(target, access) for target, _, access in shares] + [ + (target, access, kind) + for (target, _, access), kind in zip(shares, kinds, strict=True) + ] ), ) if token @@ -1150,8 +1168,9 @@ 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 for more shares, each with its " - "own guest target and mode, as children of one virtio-fs device" + "live-share a host directory, or a single regular host file without " + "its 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=[]) @@ -1179,7 +1198,8 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: choices=MOUNT_OWNERS, help=( "host identity of the share's file operations: vmm (default) or " - "caller, with guest root squashed to the directory owner (Linux only)" + "caller, with guest root squashed to the owner of the shared " + "directory or file (Linux only)" ), ) run.add_argument("--net", metavar="IPV4/PREFIX") @@ -1311,9 +1331,10 @@ def sandbox_identity(value: str) -> tuple[int, int]: default=[], metavar="GUEST_TARGET,HOST_PATH[,ro|rw]", help=( - "live-share a host directory inside the container rootfs; repeat " - "to attach more shares, each with its own target and mode, as " - "children of one virtio-fs device" + "live-share a host directory, or a single regular host file without " + "its directory, inside the container rootfs; repeat to attach more " + "shares, each with its own target and mode, as children of one " + "virtio-fs device" ), ) sandbox.add_argument( @@ -1352,7 +1373,7 @@ def sandbox_identity(value: str) -> tuple[int, int]: help=( "host identity of the share's file operations: vmm (default) or " "caller, the workload identity with guest root squashed to the " - "directory owner (Linux only)" + "owner of the shared directory or file (Linux only)" ), ) sandbox.add_argument("--net", metavar="IPV4/PREFIX") 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 3b74cfb6d..b47981b70 100644 --- a/scripts/nvx_tools/microvm_test_scripts/filesystem-shares-snapshot.sh +++ b/scripts/nvx_tools/microvm_test_scripts/filesystem-shares-snapshot.sh @@ -9,12 +9,19 @@ fail() { 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 +grep -q '^microvm /results/output.txt virtiofs rw' /proc/mounts || fail 88 exec 3<>/workspace/journal printf NVX-BEFORE >&3 +# A handle on a file child survives the snapshot like one in a directory. +: >/results/output.txt +exec 4<>/results/output.txt +printf NVX-FILE-BEFORE >&4 echo NVX-FILESYSTEM-SHARES-BEFORE nvx-snapshot printf NVX-AFTER >&3 exec 3>&- +printf NVX-FILE-AFTER >&4 +exec 4>&- [ "$(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 @@ -24,5 +31,10 @@ 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 +[ "$(cat /results/output.txt)" = NVX-FILE-BEFORENVX-FILE-AFTER ] || fail 89 +[ "$(cat /config/settings.json)" = NVX-SETTINGS ] || fail 90 +if (printf 'append\n' >>/config/settings.json) 2>/dev/null; then + fail 91 +fi 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 4dbbcec67..c6d54f353 100644 --- a/scripts/nvx_tools/microvm_test_scripts/filesystem-shares.sh +++ b/scripts/nvx_tools/microvm_test_scripts/filesystem-shares.sh @@ -27,7 +27,7 @@ grep -q '^microvm /run/nvx/shares virtiofs rw' /proc/mounts || fail 43 grep -q '^microvm /workspace virtiofs rw' /proc/mounts || fail 44 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 +[ "$(ls /run/nvx/shares | tr '\n' ' ')" = "0 1 2 3 4 " ] || 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 @@ -118,10 +118,51 @@ case "$error" in esac [ ! -e /workspace/linked ] || fail 85 [ ! -e /srv/data/public/linked ] || fail 86 -if mkdir /run/nvx/shares/3 2>/dev/null; then +if mkdir /run/nvx/shares/5 2>/dev/null; then fail 87 fi [ "$(cat /opt/hostedtoolcache/seed)" = NVX-TOOLCACHE ] || fail 88 +# Each file child is the file alone: the aggregate lists it as a regular +# file, which the guest binds at a file, and nothing else of its host +# directory reaches the guest. +[ -f /run/nvx/shares/3 ] && [ -f /run/nvx/shares/4 ] || fail 89 +grep -q '^microvm /config/settings.json virtiofs ro' /proc/mounts || fail 90 +grep -q '^microvm /results/output.txt virtiofs rw' /proc/mounts || fail 91 +[ "$(cat /config/settings.json)" = NVX-SETTINGS ] || fail 92 +[ "$(cat /results/output.txt)" = NVX-OUTPUT ] || fail 93 +[ "$(ls /config | tr '\n' ' ')" = "settings.json " ] || fail 94 +if error=$(ls /run/nvx/shares/3/secret.json 2>&1); then + fail 95 +fi +case "$error" in + *'Not a directory'*) ;; + *) + echo "NVX-FILESYSTEM-SHARES-ERROR $error" + fail 96 + ;; +esac +# The read-only file rejects writes, also through the aggregate as root, and +# the read-write file accepts them, truncation included. +if error=$( (printf 'append\n' >>/config/settings.json) 2>&1); then + fail 97 +fi +expect_read_only "$error" 98 +if error=$( (printf 'append\n' >>/run/nvx/shares/3) 2>&1); then + fail 99 +fi +expect_read_only "$error" 100 +printf 'NVX-GUEST-OUTPUT\n' >/results/output.txt || fail 101 +[ "$(cat /run/nvx/shares/4)" = NVX-GUEST-OUTPUT ] || fail 102 +# The guest can neither remove nor replace a file child. +if error=$(rm -f /run/nvx/shares/4 2>&1); then + fail 103 +fi +expect_read_only "$error" 104 +if mv /results/output.txt /results/renamed 2>/dev/null; then + fail 105 +fi +[ "$(cat /results/output.txt)" = NVX-GUEST-OUTPUT ] || fail 106 + 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 927ef6ae3..34930d352 100644 --- a/scripts/nvx_tools/microvm_tests.py +++ b/scripts/nvx_tools/microvm_tests.py @@ -5444,24 +5444,37 @@ 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. +# The guest binds each child of the aggregate at its target with its mode, and +# each file child at a file. _SHARE_TOKENS = ( "nvx_share=0,/workspace,rw nvx_share=1,/opt/hostedtoolcache,ro " - "nvx_share=2,/srv/data,rw" + "nvx_share=2,/srv/data,rw nvx_share=3,/config/settings.json,ro,file " + "nvx_share=4,/results/output.txt,rw,file" ) +def _shared_files(files: Path) -> tuple[Path, Path]: + """Return the read-only settings file and the read-write output file that + the filesystem-shares scenario shares from `files`, each beside a file + that it does not share.""" + return files / "config" / "settings.json", files / "results" / "output.txt" + + def _aggregate_arguments( workspace: Path, toolcache: Path, data: Path, - names: tuple[str, str, str] = ("0", "1", "2"), + files: Path, + names: tuple[str, str, str, str, str] = ("0", "1", "2", "3", "4"), ) -> 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 + read-only tool cache, the data directory, and the settings and output + files below `files` 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. The settings file is read-only and the output + file read-write, and neither shares its directory.""" + workspace_name, toolcache_name, data_name, settings_name, output_name = names + settings, output = _shared_files(files) return ( "--mount-aggregate", "/run/nvx/shares", @@ -5471,6 +5484,10 @@ def _aggregate_arguments( f"{toolcache_name},{toolcache},ro", "--mount-child", f"{data_name},{data},rw", + "--mount-child", + f"{settings_name},{settings},ro,file", + "--mount-child", + f"{output_name},{output},rw,file", "--mount-deny", str(data), "--mount-allow", @@ -5491,6 +5508,7 @@ def _shares_command( workspace: Path, toolcache: Path, data: Path, + files: Path, ) -> list[str]: command = workload_boot_command( executable, @@ -5500,7 +5518,7 @@ def _shares_command( memory_mib, f"quiet loglevel=0 {_SHARE_TOKENS}", ) - command.extend(_aggregate_arguments(workspace, toolcache, data)) + command.extend(_aggregate_arguments(workspace, toolcache, data, files)) return command @@ -5514,17 +5532,22 @@ def run_filesystem_shares( timeout: float, output_dir: Path, ) -> None: - """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.""" + """Attach a read-write workspace, a read-only tool cache, a data directory + with a hidden root, and a read-only and a read-write file, each without + its directory, 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" + files = root / "files" + settings, output = _shared_files(files) (workspace / "secrets").mkdir(parents=True) (toolcache / "tools").mkdir(parents=True) (toolcache / "credentials").mkdir() (data / "public").mkdir(parents=True) + settings.parent.mkdir(parents=True) + output.parent.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") @@ -5533,10 +5556,23 @@ def run_filesystem_shares( (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") + settings.write_bytes(b"NVX-SETTINGS") + (settings.parent / "secret.json").write_bytes(b"NVX-CONFIG-SECRET") + output.write_bytes(b"NVX-OUTPUT\n") + (output.parent / "other.txt").write_bytes(b"NVX-OTHER") toolcache_contents = _tree_contents(toolcache) + files_contents = _tree_contents(files) command = _shares_command( - executable, backend, kernel, initrd, memory_mib, workspace, toolcache, data + executable, + backend, + kernel, + initrd, + memory_mib, + workspace, + toolcache, + data, + files, ) command.extend( ( @@ -5570,6 +5606,12 @@ def run_filesystem_shares( raise RuntimeError( "guest changed the data share beyond its one writable path" ) + files_contents["results/output.txt"] = b"NVX-GUEST-OUTPUT\n" + if _tree_contents(files) != files_contents: + raise RuntimeError( + "guest changed the shared files' directories, the read-only " + "file, or the read-write file other than as it wrote" + ) nested = workspace / "nested" nested.mkdir() @@ -5598,6 +5640,48 @@ def run_filesystem_shares( ), b"must not overlap", ), + # A file overlaps a directory child that contains it. + ( + "file-inside-child", + ( + "--mount-aggregate", + "/run/nvx/shares", + "--mount-child", + f"0,{settings.parent},rw", + "--mount-child", + f"1,{settings},ro,file", + ), + b"must not overlap", + ), + # The `file` flag, not the host object, decides a child's kind, so + # a directory never stands in for a file, nor a file for a + # directory. + ( + "directory-as-file", + ( + "--mount-aggregate", + "/run/nvx/shares", + "--mount-child", + f"0,{settings.parent},ro,file", + ), + b"not a regular file", + ), + ( + "file-as-directory", + ( + "--mount-aggregate", + "/run/nvx/shares", + "--mount-child", + f"0,{settings},ro", + ), + b"not a plain directory", + ), + # The guest mounts the root of a lone share, which is a directory. + ( + "file-mount", + ("--mount", f"/config/settings.json,{settings},ro"), + b"not a plain directory", + ), ): invalid = workload_boot_command( executable, backend, kernel, initrd, memory_mib, "quiet loglevel=0" @@ -5613,6 +5697,8 @@ def run_filesystem_shares( b"cannot be used with", ), ("relative-deny", ("--mount-deny", "secrets"), b"absolute host path"), + # A file child's mode applies to the whole file. + ("file-policy", ("--mount-deny", str(settings)), b"exposes the file"), ): invalid = _shares_command( executable, @@ -5623,6 +5709,7 @@ def run_filesystem_shares( workspace, toolcache, data, + files, ) invalid.extend(arguments) _expect_boot_failure( @@ -5642,6 +5729,7 @@ def run_filesystem_shares( workspace, toolcache, data, + files, ), "--snapshot-destination", str(snapshot), @@ -5665,7 +5753,7 @@ def run_filesystem_shares( raise RuntimeError("aggregate snapshot source crossed the capture boundary") fingerprint = _snapshot_fingerprint(snapshot) - aggregate = _aggregate_arguments(workspace, toolcache, data) + aggregate = _aggregate_arguments(workspace, toolcache, data, files) for name, arguments, expected in ( ( "missing-share", @@ -5681,13 +5769,15 @@ def run_filesystem_shares( ), ( "swapped-shares", - _aggregate_arguments(toolcache, workspace, data), + _aggregate_arguments(toolcache, workspace, data, files), 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")), + _aggregate_arguments( + workspace, toolcache, data, files, ("0", "1", "data", "3", "4") + ), b"do not match the snapshot contract", ), ( @@ -5720,8 +5810,27 @@ def run_filesystem_shares( 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 output.read_bytes() != b"NVX-FILE-BEFORENVX-FILE-AFTER": + raise RuntimeError("read-write file did not resume after restore") if _tree_contents(toolcache) != toolcache_contents: raise RuntimeError("restored guest modified the read-only share") + if settings.read_bytes() != b"NVX-SETTINGS": + raise RuntimeError("restored guest modified the read-only file") + + # A snapshot pins the object that a file child names, so a file that + # the host puts in its place, such as by saving over it, is another. + replacement = settings.with_name("settings.json.new") + replacement.write_bytes(b"NVX-SETTINGS") + replacement.replace(settings) + replaced = snapshot_restore_command(executable, backend, snapshot) + replaced.extend(aggregate) + _expect_process_failure( + replaced, + output_dir / "filesystem-shares-replaced-file.log", + timeout, + b"root identity does not match", + (FILESYSTEM_SHARES_AFTER_MARKER,), + ) def _policy_arguments(share: Path) -> tuple[str, ...]: diff --git a/scripts/nvx_tools/sandbox.py b/scripts/nvx_tools/sandbox.py index f410457be..7826ca10c 100644 --- a/scripts/nvx_tools/sandbox.py +++ b/scripts/nvx_tools/sandbox.py @@ -27,11 +27,14 @@ _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 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. +# A share binds a host directory, or a regular host file, which only a child +# of an aggregate can expose, without anything else of its directory. +MOUNT_KINDS = ("directory", "file") +# OpenVMM attaches one virtio-fs device, tag `microvm`. One directory share +# uses it directly. Several shares, or a file, 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 @@ -39,9 +42,9 @@ MOUNT_OWNERS = ("vmm", "caller") # The container runtime owns these guest paths: it mounts procfs, sysfs, and a # private /dev over them, stages its tools under /.nvx-agent, and bind-mounts the -# workload machine ID into /etc. A live share must not shadow or be shadowed by -# them. -RESERVED_MOUNT_TARGETS = ("/proc", "/sys", "/dev", "/.nvx-agent") +# workload machine ID at /etc/machine-id, which it first replaces with a regular +# file. A live share must not shadow or be shadowed by them. +RESERVED_MOUNT_TARGETS = ("/proc", "/sys", "/dev", "/.nvx-agent", "/etc/machine-id") RESERVED_EXACT_MOUNT_TARGETS = ("/etc",) # OpenVMM accepts at most this many denied, allowed, and writable paths each. MAX_MOUNT_POLICY_PATHS = 128 @@ -138,7 +141,9 @@ class SandboxMount: `denied_paths` hide paths in the share, `allowed_paths` expose paths inside denied paths again, and `writable_paths`, when present, are the - only paths of a read-write share that the workload can modify. + only paths of a read-write share that the workload can modify. A share of + `kind` file binds one regular host file, whose mode applies to the whole + file, so it has none of them. """ guest_target: str @@ -148,6 +153,7 @@ class SandboxMount: owner: str = "vmm" allowed_paths: tuple[str, ...] = () writable_paths: tuple[str, ...] = () + kind: str = "directory" @classmethod def parse( @@ -158,6 +164,7 @@ def parse( *, allowed_paths: tuple[str, ...] = (), writable_paths: tuple[str, ...] = (), + kind: str = "directory", ) -> SandboxMount: fields = value.split(",") if len(fields) not in (2, 3): @@ -174,6 +181,7 @@ def parse( owner=owner, allowed_paths=allowed_paths, writable_paths=writable_paths, + kind=kind, ) def __post_init__(self) -> None: @@ -186,6 +194,11 @@ def __post_init__(self) -> None: raise ScriptError( f"unsupported sandbox mount owner {self.owner!r}; choose vmm or caller" ) + if self.kind not in MOUNT_KINDS: + raise ScriptError( + f"unsupported sandbox mount kind {self.kind!r}; choose directory " + "or file" + ) raw_path = os.fspath(self.host_path) if any(character in raw_path for character in ",\0"): raise ScriptError( @@ -195,6 +208,12 @@ def __post_init__(self) -> None: raise ScriptError( f"sandbox mount host path contains a parent component: {raw_path}" ) + if self.kind == "file" and any(paths for _, paths in self.policy_paths()): + raise ScriptError( + "--mount-deny, --mount-allow, and --mount-write name paths inside " + f"a shared directory, but --mount {self.guest_target} shares a " + "file, whose mode applies to the whole file" + ) for kind, paths in self.policy_paths(): if len(paths) > MAX_MOUNT_POLICY_PATHS: raise ScriptError( @@ -227,7 +246,14 @@ def policy_paths(self) -> tuple[tuple[str, tuple[str, ...]], ...]: def validated(self) -> SandboxMount: require_mount_owner_supported(self.owner) - if self.host_path.is_symlink() or not self.host_path.is_dir(): + # The kind was fixed when the share was requested, so a host path that + # became a directory in place of a file is not shared in its place. + if self.kind == "file": + if self.host_path.is_symlink() or not self.host_path.is_file(): + raise ScriptError( + f"sandbox mount host path is not a regular file: {self.host_path}" + ) + elif self.host_path.is_symlink() or not self.host_path.is_dir(): raise ScriptError( f"sandbox mount host path is not a plain directory: {self.host_path}" ) @@ -236,7 +262,7 @@ def validated(self) -> SandboxMount: if status.st_uid == 0 or status.st_gid == 0: raise ScriptError( "--mount-owner caller squashes guest root to the owner of the " - f"shared directory, so {self.host_path} must not be owned by " + f"shared {self.kind}, so {self.host_path} must not be owned by " "UID 0 or GID 0" ) return self @@ -256,6 +282,7 @@ def absolute(self) -> SandboxMount: owner=self.owner, allowed_paths=self.allowed_paths, writable_paths=self.writable_paths, + kind=self.kind, ) def _absolute_paths(self, paths: tuple[str, ...]) -> tuple[str, ...]: @@ -282,6 +309,19 @@ def _targets_overlap(left: str, right: str) -> bool: return left == right or left.startswith(f"{right}/") or right.startswith(f"{left}/") +def mount_kind(host_path: Path) -> str: + """Return the kind of share that `host_path` requests: a file for a + regular file, and otherwise a directory, which validation then requires.""" + return "file" if host_path.is_file() else "directory" + + +def mounts_use_aggregate(mounts: Sequence[SandboxMount]) -> bool: + """Return whether `mounts` share the virtio-fs device as the children of + an aggregate: several shares do, and so does a file, because the guest + mounts the device's root, which is a directory.""" + return len(mounts) > 1 or any(mount.kind == "file" for mount in mounts) + + def validate_mounts(mounts: tuple[SandboxMount, ...]) -> None: """Validate the live shares that one sandbox attaches, in child order.""" for index, mount in enumerate(mounts): @@ -295,30 +335,32 @@ def validate_mounts(mounts: tuple[SandboxMount, ...]) -> None: raise ScriptError("every sandbox --mount must use the same --mount-owner") -def _directory_identity(path: Path) -> tuple[int, int]: +def _path_identity(path: Path) -> tuple[int, int]: status = path.stat() return status.st_dev, status.st_ino def validate_mount_host_paths(mounts: tuple[SandboxMount, ...]) -> None: - """Reject shares whose host directories overlap, or deny another's paths.""" + """Reject shares whose host paths overlap, or deny another's paths. A + shared file overlaps only a shared directory that contains it, or another + share of the same file.""" if len(mounts) < 2: return roots = [mount.host_path.resolve() for mount in mounts] # Resolving follows symbolic links but not bind mounts, so also compare the # file identities that OpenVMM pins for each share: a root with the identity - # of another root, or of one of its ancestors, is that directory or lies - # inside a bind mount of it. On Linux, OpenVMM also compares the mount - # sources that each share reaches, which catches a bind mount that exposes - # part of one share inside the other. + # of another root, or of one of its ancestors, is that directory or file, + # such as through a hard link, or lies inside a bind mount of it. On Linux, + # OpenVMM also compares the mount sources that each share reaches, which + # catches a bind mount that exposes part of one share inside the other. lineages = [ - [_directory_identity(path) for path in (root, *root.parents)] for root in roots + [_path_identity(path) for path in (root, *root.parents)] for root in roots ] for index, root in enumerate(roots): for other_index, other in enumerate(roots[:index]): if root == other or other in root.parents or root in other.parents: raise ScriptError( - f"sandbox mount host directories {other} and {root} overlap; " + f"sandbox mount host paths {other} and {root} overlap; " "a share could reach files of another under its access mode" ) if ( @@ -326,8 +368,9 @@ def validate_mount_host_paths(mounts: tuple[SandboxMount, ...]) -> None: or lineages[other_index][0] in lineages[index] ): raise ScriptError( - f"sandbox mount host directories {other} and {root} are the " - "same directory, or one is inside the other through a bind mount" + f"sandbox mount host paths {other} and {root} name the same " + "directory or file, or one is inside the other through a bind " + "mount" ) for mount, root in zip(mounts, roots, strict=True): for kind, paths in mount.absolute_policy_paths(): @@ -357,11 +400,20 @@ def aggregate_child_name(index: int, target: str) -> str: return f"{index}-{digest}" +def aggregate_child_argument(name: str, host_path: str, access: str, kind: str) -> str: + """Return the `--mount-child` value that attaches `host_path` as the child + `name`. A file child carries OpenVMM's `file` flag, so OpenVMM refuses a + directory that replaced the file instead of exporting all of it.""" + return f"{name},{host_path},{access}" + (",file" if kind == "file" else "") + + def mounts_openvmm_arguments(mounts: tuple[SandboxMount, ...]) -> list[str]: - """Return the OpenVMM arguments that attach `mounts`: one share directly, - and several as the children of an aggregate, in order.""" + """Return the OpenVMM arguments that attach `mounts`: one directory share + directly, and several shares, or a file, as the children of an aggregate, + in order.""" arguments: list[str] = [] - if len(mounts) == 1: + aggregate = mounts_use_aggregate(mounts) + if mounts and not aggregate: mount = mounts[0] arguments.extend( ( @@ -376,14 +428,16 @@ def mounts_openvmm_arguments(mounts: tuple[SandboxMount, ...]) -> list[str]: arguments.extend( ( "--mount-child", - f"{name},{os.fspath(mount.host_path)},{mount.access}", + aggregate_child_argument( + name, os.fspath(mount.host_path), mount.access, mount.kind + ), ) ) for mount in mounts: # OpenVMM resolves a relative policy path in the only share, and # attributes absolute policy paths to the child that contains them. policy_paths = ( - mount.policy_paths() if len(mounts) == 1 else mount.absolute_policy_paths() + mount.absolute_policy_paths() if aggregate else mount.policy_paths() ) for kind, paths in policy_paths: for path in paths: @@ -393,15 +447,15 @@ def mounts_openvmm_arguments(mounts: tuple[SandboxMount, ...]) -> list[str]: return arguments -def aggregate_share_tokens(shares: Sequence[tuple[str, str]]) -> list[str]: +def aggregate_share_tokens(shares: Sequence[tuple[str, 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 [] + each child of an aggregate, given each share's guest target, access, and + kind. A file child's token ends in `,file`, so the guest binds it at a + file.""" return [ f"nvx_share={aggregate_child_name(index, target)},{target},{access}" - for index, (target, access) in enumerate(shares) + + (",file" if kind == "file" else "") + for index, (target, access, kind) in enumerate(shares) ] @@ -417,10 +471,10 @@ def aggregate_command_line_fragment(accesses: Sequence[str]) -> str: def mounts_command_line_fragment(mounts: tuple[SandboxMount, ...]) -> str: """Return the bootstrap tokens that OpenVMM appends for `mounts`.""" - if len(mounts) == 1: - return mounts[0].command_line_fragment() if not mounts: return "" + if not mounts_use_aggregate(mounts): + return mounts[0].command_line_fragment() return aggregate_command_line_fragment([mount.access for mount in mounts]) @@ -536,11 +590,15 @@ 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] + if mounts_use_aggregate(self.mounts): + tokens.extend( + aggregate_share_tokens( + [ + (mount.guest_target, mount.access, mount.kind) + 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. diff --git a/scripts/nvx_tools/sandbox_lifecycle.py b/scripts/nvx_tools/sandbox_lifecycle.py index f2594f08a..ddb5a69ad 100644 --- a/scripts/nvx_tools/sandbox_lifecycle.py +++ b/scripts/nvx_tools/sandbox_lifecycle.py @@ -52,12 +52,23 @@ # configuration with either lists every share under `mounts` in a format that # they reject. POLICY_CONFIG_FORMAT = 5 +# Format-5 readers know only directory shares, so a configuration with a file +# share lists every share under `mounts`, with its kind, in a format that they +# reject rather than share a directory that later appears at the file's path. +FILE_MOUNT_CONFIG_FORMAT = 6 CONFIG_FORMATS = ( CONFIG_FORMAT, MOUNT_CONFIG_FORMAT, OWNER_CONFIG_FORMAT, MULTI_MOUNT_CONFIG_FORMAT, POLICY_CONFIG_FORMAT, + FILE_MOUNT_CONFIG_FORMAT, +) +# The formats that list every share under `mounts`. +MOUNTS_CONFIG_FORMATS = ( + MULTI_MOUNT_CONFIG_FORMAT, + POLICY_CONFIG_FORMAT, + FILE_MOUNT_CONFIG_FORMAT, ) OUTCOME_SCHEMA_VERSION = 1 @@ -228,9 +239,13 @@ def _serialize_launch( "cmdline": cmdline, } config_format = _config_format(launch.mounts) - if config_format in (MULTI_MOUNT_CONFIG_FORMAT, POLICY_CONFIG_FORMAT): + if config_format in MOUNTS_CONFIG_FORMATS: config["mounts"] = [ - _serialize_mount(mount, policy=config_format == POLICY_CONFIG_FORMAT) + _serialize_mount( + mount, + policy=config_format != MULTI_MOUNT_CONFIG_FORMAT, + kind=config_format == FILE_MOUNT_CONFIG_FORMAT, + ) for mount in launch.mounts ] else: @@ -239,6 +254,8 @@ def _serialize_launch( def _config_format(mounts: tuple[SandboxMount, ...]) -> int: + if any(mount.kind == "file" for mount in mounts): + return FILE_MOUNT_CONFIG_FORMAT if any(mount.allowed_paths or mount.writable_paths for mount in mounts): return POLICY_CONFIG_FORMAT if len(mounts) > 1: @@ -248,7 +265,9 @@ def _config_format(mounts: tuple[SandboxMount, ...]) -> int: return MOUNT_CONFIG_FORMAT if mounts else CONFIG_FORMAT -def _serialize_mount(mount: SandboxMount, *, policy: bool = False) -> dict[str, Any]: +def _serialize_mount( + mount: SandboxMount, *, policy: bool = False, kind: bool = False +) -> dict[str, Any]: absolute = mount.absolute() serialized: dict[str, Any] = { "guest_target": absolute.guest_target, @@ -260,6 +279,8 @@ def _serialize_mount(mount: SandboxMount, *, policy: bool = False) -> dict[str, if policy: serialized["allowed_paths"] = list(absolute.allowed_paths) serialized["writable_paths"] = list(absolute.writable_paths) + if kind: + serialized["kind"] = absolute.kind return serialized @@ -270,7 +291,9 @@ def _deserialize_paths(mount: dict[str, Any], key: str) -> tuple[str, ...]: return tuple(str(path) for path in cast(list[object], paths)) -def _deserialize_mount(value: object, *, policy: bool = False) -> SandboxMount: +def _deserialize_mount( + value: object, *, policy: bool = False, kind: bool = False +) -> SandboxMount: if not isinstance(value, dict): raise TypeError("sandbox mount configuration must be an object") mount = cast(dict[str, Any], value) @@ -283,17 +306,22 @@ def _deserialize_mount(value: object, *, policy: bool = False) -> SandboxMount: owner=str(mount.get("owner", "vmm")), allowed_paths=_deserialize_paths(mount, "allowed_paths") if policy else (), writable_paths=_deserialize_paths(mount, "writable_paths") if policy else (), + kind=str(mount["kind"]) if kind else "directory", ) def _deserialize_mounts(config: dict[str, Any]) -> tuple[SandboxMount, ...]: - if config.get("format") in (MULTI_MOUNT_CONFIG_FORMAT, POLICY_CONFIG_FORMAT): + config_format = config.get("format") + if config_format in MOUNTS_CONFIG_FORMATS: mounts = config["mounts"] if not isinstance(mounts, list): raise TypeError("sandbox mounts configuration must be a list") - policy = config.get("format") == POLICY_CONFIG_FORMAT return tuple( - _deserialize_mount(mount, policy=policy) + _deserialize_mount( + mount, + policy=config_format != MULTI_MOUNT_CONFIG_FORMAT, + kind=config_format == FILE_MOUNT_CONFIG_FORMAT, + ) for mount in cast(list[object], mounts) ) mount = config.get("mount") diff --git a/scripts/test_microvm_tests.py b/scripts/test_microvm_tests.py index 43c83d05c..b0ecc4f52 100644 --- a/scripts/test_microvm_tests.py +++ b/scripts/test_microvm_tests.py @@ -1989,17 +1989,24 @@ def _children(command: list[str]) -> list[str]: @staticmethod def _host(child: str) -> Path: - return Path(child.split(",", 1)[1].rsplit(",", 1)[0]) + value = child.removesuffix(",file") + return Path(value.split(",", 1)[1].rsplit(",", 1)[0]) def _run( self, *, guest_writes_toolcache: bool = False, guest_links_children: bool = False, + guest_writes_settings: bool = False, ) -> list[list[str]]: launched: list[list[str]] = [] children = self._children host = self._host + captured: dict[str, tuple[int, int]] = {} + + def identity(path: Path) -> tuple[int, int]: + status = path.stat() + return status.st_dev, status.st_ino def guest( command: list[str], script: str, marker: bytes, **_kwargs: object @@ -2008,15 +2015,22 @@ def guest( self.assertEqual(marker, microvm_tests.FILESYSTEM_SHARES_MARKER) 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)) + self.assertIn("ls /run/nvx/shares/3/secret.json", script) + workspace, toolcache, data, settings, output = ( + 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") + output.write_bytes(b"NVX-GUEST-OUTPUT\n") if guest_writes_toolcache: (toolcache / "mutation").write_bytes(b"") if guest_links_children: (workspace / "linked").write_bytes(b"NVX-TOOLCACHE") + if guest_writes_settings: + with settings.open("ab") as file: + file.write(b"append\n") class FakeProcess: def __init__(self, command: list[str], _log_path: Path) -> None: @@ -2038,6 +2052,11 @@ def send_bytes(self, _data: bytes) -> None: def wait(self, _timeout: float) -> openvmm_process.OpenvmmProcessResult: command = self.command requested = children(command) + denied = [ + command[index + 1] + for index, value in enumerate(command) + if value == "--mount-deny" + ] if "--snapshot-destination" in command: snapshot = Path( command[command.index("--snapshot-destination") + 1] @@ -2046,6 +2065,8 @@ def wait(self, _timeout: float) -> openvmm_process.OpenvmmProcessResult: for name in ("manifest.bin", "state.bin", "memory.bin"): (snapshot / name).write_bytes(name.encode()) (host(requested[0]) / "journal").write_bytes(b"NVX-BEFORE") + host(requested[4]).write_bytes(b"NVX-FILE-BEFORE") + captured["settings"] = identity(host(requested[3])) return openvmm_process.OpenvmmProcessResult( 0, b"NVX-FILESYSTEM-SHARES-BEFORE\n" ) @@ -2056,26 +2077,47 @@ def wait(self, _timeout: float) -> openvmm_process.OpenvmmProcessResult: b"requires the --mount-aggregate and --mount-child options" ) elif ( - len(requested) != 3 + len(requested) != 5 or host(requested[0]).name != "workspace" - or names != ["0", "1", "2"] + or names != ["0", "1", "2", "3", "4"] ): error = b"do not match the snapshot contract" + elif identity(host(requested[3])) != captured["settings"]: + error = ( + b"restore-time filesystem root identity does not match " + b"the snapshot attachment" + ) else: with (host(requested[0]) / "journal").open("ab") as journal: journal.write(b"NVX-AFTER") + with host(requested[4]).open("ab") as output: + output.write(b"NVX-FILE-AFTER") return openvmm_process.OpenvmmProcessResult( 0, b"NVX-FILESYSTEM-SHARES-AFTER\n" ) return openvmm_process.OpenvmmProcessResult(1, error + b"\n") if command.count("--mount") == 2: error = b"the argument '--mount' cannot be used multiple times" - elif "--mount" in command: + elif "--mount" in command and "--mount-aggregate" in command: error = b"'--mount-aggregate' cannot be used with '--mount'" - elif "secrets" in command: + elif "--mount" in command: + error = b"microVM filesystem root is not a plain directory" + elif any( + child.endswith(",file") and host(child).is_dir() + for child in requested + ): + error = b"microVM filesystem root is not a regular file" + elif any( + not child.endswith(",file") and host(child).is_file() + for child in requested + ): + error = b"microVM filesystem root is not a plain directory" + elif "secrets" in denied: error = b"--mount-deny requires an absolute host path" + elif any(Path(path).name == "settings.json" for path in denied): + error = b"--mount-child 3 exposes the file settings.json" else: - error = b"host directories must not overlap" + error = b"host paths must not overlap" return openvmm_process.OpenvmmProcessResult(2, error + b"\n") with ( @@ -2104,14 +2146,23 @@ def test_scenario_attaches_every_share_and_restores_them_in_order(self): boot = launched[0] self.assertNotIn("--mount", boot) self.assertEqual(boot[boot.index("--mount-aggregate") + 1], "/run/nvx/shares") - workspace, toolcache, data = self._children(boot) + workspace, toolcache, data, settings, output = 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")) + # The files are children of their own, without their directories, + # which OpenVMM must find to be regular files. + self.assertTrue( + settings.startswith("3,") and settings.endswith("settings.json,ro,file") + ) + self.assertTrue( + output.startswith("4,") and output.endswith("output.txt,rw,file") + ) 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", + "nvx_share=2,/srv/data,rw nvx_share=3,/config/settings.json,ro,file " + "nvx_share=4,/results/output.txt,rw,file", cmdline, ) @@ -2130,32 +2181,56 @@ def policy(command: list[str], option: str) -> list[str]: 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) + # Nine invalid sets fail before boot, then a capture with every share, + # four invalid restores, the restore with every share in order, and a + # restore after the host replaced the read-only file. + self.assertEqual(len(launched), 1 + 9 + 1 + 5 + 1) self.assertEqual(launched[1].count("--mount"), 2) self.assertEqual(len(self._children(launched[2])), 2) - capture = launched[5] + # A file inside a directory child, a directory with the `file` flag, a + # file without it, a file as a lone share, and a file child named by a + # policy path. + inside = self._children(launched[3]) + self.assertEqual(self._host(inside[1]), self._host(settings)) + self.assertEqual(self._host(inside[0]), self._host(settings).parent) + self.assertEqual( + self._children(launched[4]), + [f"0,{self._host(settings).parent},ro,file"], + ) + self.assertEqual(self._children(launched[5]), [f"0,{self._host(settings)},ro"]) + self.assertTrue( + launched[6][launched[6].index("--mount") + 1].endswith("settings.json,ro") + ) + self.assertEqual( + policy(launched[9], "--mount-deny")[-1], self._host(settings).name + ) + capture = launched[10] self.assertIn("--snapshot-destination", capture) - self.assertEqual(self._children(capture), [workspace, toolcache, data]) - self.assertEqual(self._children(launched[6]), [workspace, toolcache]) + every = [workspace, toolcache, data, settings, output] + self.assertEqual(self._children(capture), every) + self.assertEqual(self._children(launched[11]), [workspace, toolcache]) self.assertEqual( - [self._host(child).name for child in self._children(launched[7])], - ["toolcache", "workspace", "data"], + [self._host(child).name for child in self._children(launched[12])], + ["toolcache", "workspace", "data", "settings.json", "output.txt"], ) # 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._children(launched[13]), + [workspace, toolcache, data.replace("2,", "data,", 1), settings, output], ) - self.assertIn("--mount", launched[9]) - self.assertEqual(self._children(launched[10]), [workspace, toolcache, data]) + self.assertIn("--mount", launched[14]) + self.assertEqual(self._children(launched[15]), every) + self.assertEqual(self._children(launched[16]), every) 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_write_to_the_read_only_file(self): + with self.assertRaisesRegex(RuntimeError, "changed the shared files"): + self._run(guest_writes_settings=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) diff --git a/scripts/test_nvx_tools.py b/scripts/test_nvx_tools.py index 34f453159..4bca13fb3 100644 --- a/scripts/test_nvx_tools.py +++ b/scripts/test_nvx_tools.py @@ -9168,7 +9168,8 @@ def test_hostmount_accepts_only_child_names_inside_the_aggregate(self): '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' + 'printf "%s|%s|%s|%s\\n" "$share_child" "$share_target" "$share_mode" ' + '"$share_kind"\n' ) def parse(share: str) -> subprocess.CompletedProcess[str]: @@ -9182,9 +9183,12 @@ def parse(share: str) -> subprocess.CompletedProcess[str]: 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"), + (f"{name},/workspace,rw", f"{name}|/workspace|rw|directory\n"), + ("0,/opt/tools,ro", "0|/opt/tools|ro|directory\n"), + ("a.b_C-9,/x,ro", "a.b_C-9|/x|ro|directory\n"), + # A file child is bound at a file. + ("1,/config/settings.json,ro,file", "1|/config/settings.json|ro|file\n"), + ("1,/results/output.txt,rw,file", "1|/results/output.txt|rw|file\n"), ): with self.subTest(share=share): result = parse(share) @@ -9197,12 +9201,113 @@ def parse(share: str) -> subprocess.CompletedProcess[str]: "a/b,/workspace,rw", "a:b,/workspace,rw", "a*,/workspace,rw", + "0,/workspace,file", + "0,/workspace,rw,dir", ): with self.subTest(share=share): result = parse(share) self.assertEqual(result.returncode, 1, result.stdout) self.assertIn("invalid live share", result.stderr) + def test_hostmount_makes_file_mount_points_without_following_links(self): + hostmount = self.AGENT.with_name("nvx-hostmount").read_text(encoding="utf-8") + script = ( + f"{_shell_function(hostmount, 'make_file_mount_point')}" + 'make_file_mount_point "$1"\n' + ) + + def make(target: Path) -> int: + return subprocess.run( + [self.shell, "-s", "--", target.as_posix()], + input=script, + text=True, + capture_output=True, + check=False, + ).returncode + + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + # A missing file is created with its directories, and an existing + # one is kept as it is. + created = root / "config" / "settings.json" + self.assertEqual(make(created), 0) + self.assertEqual(created.read_bytes(), b"") + existing = root / "existing.json" + existing.write_bytes(b"{}") + self.assertEqual(make(existing), 0) + self.assertEqual(existing.read_bytes(), b"{}") + (root / "directory").mkdir() + self.assertEqual(make(root / "directory"), 1) + if os.name == "nt": + return + # A link is refused before anything is created through it, whether + # or not its target exists. + dangling = root / "dangling.json" + dangling.symlink_to(root / "elsewhere.json") + self.assertEqual(make(dangling), 1) + self.assertFalse((root / "elsewhere.json").exists()) + linked = root / "linked.json" + linked.symlink_to(existing) + self.assertEqual(make(linked), 1) + self.assertEqual(existing.read_bytes(), b"{}") + + def test_agent_binds_a_file_child_at_a_file(self): + rootfs = self.rootfs.as_posix() + (self.rootfs / "opt").mkdir() + (self.rootfs / "opt" / "existing.json").write_text("{}", encoding="utf-8") + result, log = self._run( + "nvx_sandbox=1 " + + self._aggregate( + "0,/workspace,rw", + "1,/etc/app/settings.json,ro,file", + "2,/opt/existing.json,rw,file", + ) + ) + self.assertEqual(result.returncode, 0, result.stderr) + # Each file child gets a file mount point, created inside the container + # rootfs or reused there, and is bound with its own mode. + settings = self.rootfs / "etc" / "app" / "settings.json" + self.assertTrue(settings.is_file()) + self.assertEqual(settings.read_text(encoding="utf-8"), "") + self.assertEqual( + (self.rootfs / "opt" / "existing.json").read_text(encoding="utf-8"), "{}" + ) + self.assertTrue((self.rootfs / "workspace").is_dir()) + self.assertEqual( + log.splitlines(), + [ + 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}/etc/app/settings.json", + f"-o remount,bind,ro,nosuid,nodev {rootfs}/etc/app/settings.json", + f"-o bind {self.runtime}/shares/2 {rootfs}/opt/existing.json", + f"-o remount,bind,rw,nosuid,nodev {rootfs}/opt/existing.json", + ], + ) + self.assertIn( + f"mountpoints={rootfs}/opt/existing.json {rootfs}/etc/app/settings.json " + f"{rootfs}/workspace {self.runtime}/shares\n", + result.stdout, + ) + + # A directory is no file mount point, a file no directory one, and + # the runtime binds the machine ID itself. + (self.rootfs / "dir").mkdir() + (self.rootfs / "file").write_text("x", encoding="utf-8") + for child, message in ( + ("0,/dir,ro,file", "not a regular file"), + ("0,/file/settings.json,ro,file", "not a directory"), + ("0,/file,ro", "not a directory"), + ("0,/etc/machine-id,ro,file", "reserved"), + ("0,/etc/machine-id/child,ro", "reserved"), + ): + with self.subTest(child=child): + result, log = self._run(self._aggregate(child)) + self.assertEqual(result.returncode, 125, result.stdout) + self.assertIn(message, result.stderr) + self.assertEqual(log, "") + 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"), @@ -11763,7 +11868,7 @@ def launch(*mounts: sandbox.SandboxMount) -> sandbox.SandboxLaunch: # reached through two paths, or a share inside a bind mount of the # other share's directory, is rejected by its file identities. sandbox.validate_mount_host_paths((workspace, toolcache)) - with patch.object(sandbox, "_directory_identity", return_value=(1, 1)): + with patch.object(sandbox, "_path_identity", return_value=(1, 1)): with self.assertRaisesRegex(common.ScriptError, "same directory"): sandbox.validate_mount_host_paths((workspace, toolcache)) alias = root / "alias" @@ -11778,7 +11883,7 @@ def identity(path: Path) -> tuple[int, int]: cache = sandbox.SandboxMount.parse( f"/opt/hostedtoolcache,{alias / 'cache'}" ) - with patch.object(sandbox, "_directory_identity", side_effect=identity): + with patch.object(sandbox, "_path_identity", side_effect=identity): sandbox.validate_mount_host_paths((workspace, toolcache)) for mounts in ((workspace, cache), (cache, workspace)): with self.assertRaisesRegex(common.ScriptError, "inside the other"): @@ -11862,6 +11967,152 @@ def require(path: Path, _description: str) -> Path: self.assertEqual(command[command.index("--mount-deny") + 1], "secrets") self.assertNotIn("--mount-owner", command) + def test_sandbox_command_shares_a_file(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + layer = root / "distro.erofs" + scratch = root / "scratch.ext4" + settings = root / "settings.json" + layer.write_bytes(b"distro") + scratch.write_bytes(b"scratch") + settings.write_bytes(b"{}") + + def sandbox_args(*mount: str) -> argparse.Namespace: + return nvx.parse_args( + [ + "sandbox", + "--layer", + f"distro,{layer},11111111-1111-1111-1111-111111111111", + "--scratch", + str(scratch), + *mount, + "--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( + sandbox_args("--mount", f"/etc/app/settings.json,{settings},ro") + ) + command = format_command.call_args.args[0] + name = sandbox.aggregate_child_name(0, "/etc/app/settings.json") + # The regular file is a child of its own, without its directory, + # which OpenVMM must find to be a regular file. + self.assertNotIn("--mount", command) + self.assertEqual( + command[command.index("--mount-child") + 1], + f"{name},{settings},ro,file", + ) + self.assertIn( + f"nvx_share={name},/etc/app/settings.json,ro,file", + command[command.index("--cmdline") + 1], + ) + with self.assertRaisesRegex(common.ScriptError, "shares a file"): + nvx.command_sandbox( + sandbox_args( + "--mount", + f"/etc/app/settings.json,{settings},rw", + "--mount-write", + str(settings), + ) + ) + + def test_run_attaches_a_file_as_an_aggregate_child(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + settings = root / "settings.json" + settings.write_bytes(b"{}") + work = root / "work" + work.mkdir() + with ( + patch.object(nvx, "require_file", return_value=Path("artifact")), + patch.object( + nvx, "_format_command", return_value="formatted" + ) as format_command, + ): + nvx.command_run( + nvx.parse_args( + [ + "run", + "--mount", + f"/config/settings.json,{settings},ro", + "--dry-run", + ] + ) + ) + command = format_command.call_args.args[0] + name = sandbox.aggregate_child_name(0, "/config/settings.json") + # Even alone, a file is the child of an aggregate. + self.assertNotIn("--mount", command) + self.assertEqual( + command[command.index("--mount-aggregate") + 1], "/run/nvx/shares" + ) + self.assertEqual( + command[command.index("--mount-child") + 1], + f"{name},{settings},ro,file", + ) + self.assertEqual( + command[command.index("--cmdline") + 1], + f"nvx_share={name},/config/settings.json,ro,file", + ) + + # A directory and a file share the aggregate, and only the + # file's token is marked. + nvx.command_run( + nvx.parse_args( + [ + "run", + "--mount", + f"/work,{work},rw", + "--mount", + f"/config/settings.json,{settings},ro", + "--dry-run", + ] + ) + ) + command = format_command.call_args.args[0] + work_name, settings_name = ( + sandbox.aggregate_child_name(index, target) + for index, target in enumerate(("/work", "/config/settings.json")) + ) + self.assertEqual( + command[command.index("--cmdline") + 1], + f"nvx_share={work_name},/work,rw " + f"nvx_share={settings_name},/config/settings.json,ro,file", + ) + + for arguments, message in ( + (["--mount-deny", "secrets"], "absolute host path"), + (["--cmdline", "x" * 1024], "1024-byte"), + ): + with self.subTest(arguments=arguments): + invalid = nvx.parse_args( + [ + "run", + "--mount", + f"/config/settings.json,{settings},ro", + *arguments, + "--dry-run", + ] + ) + with self.assertRaisesRegex(common.ScriptError, message): + nvx.command_run(invalid) + invalid = nvx.parse_args( + ["run", f"--mount=/a b,{settings}", "--dry-run"] + ) + with self.assertRaisesRegex( + common.ScriptError, "invalid --mount target" + ): + nvx.command_run(invalid) + def test_sandbox_command_forwards_mount_owner(self): with tempfile.TemporaryDirectory() as temporary: root = Path(temporary) @@ -12669,6 +12920,233 @@ def require(path: Path, _description: str) -> Path: ): sandbox_lifecycle._deserialize_launch(config) + def test_launch_contract_attaches_a_file_as_an_aggregate_child(self): + distro = sandbox.SandboxLayer.parse( + "distro,distro.erofs,11111111-1111-1111-1111-111111111111" + ) + settings = sandbox.SandboxMount.parse( + "/etc/app/settings.json,settings.json,ro", kind="file" + ) + launch = sandbox.SandboxLaunch( + layers=(distro,), scratch=Path("scratch.ext4"), mounts=(settings,) + ) + name = sandbox.aggregate_child_name(0, "/etc/app/settings.json") + # Even alone, a file is the child of an aggregate, because the guest + # mounts the device's root, which is a directory. + self.assertTrue(sandbox.mounts_use_aggregate(launch.mounts)) + arguments = launch.openvmm_arguments() + self.assertNotIn("--mount", arguments) + self.assertEqual( + arguments[arguments.index("--mount-aggregate") :], + [ + "--mount-aggregate", + "/run/nvx/shares", + "--mount-child", + f"{name},{os.fspath(Path('settings.json'))},ro,file", + ], + ) + self.assertEqual( + sandbox.mounts_command_line_fragment(launch.mounts), + " virtfs_dir=/run/nvx/shares virtfs_tag=microvm virtfs_mode=ro" + " virtfs_aggregate=1", + ) + self.assertTrue( + launch.kernel_command_line().endswith( + f" nvx_share={name},/etc/app/settings.json,ro,file" + ) + ) + + # Files and directories share one aggregate, and only a file's token + # is marked. + workspace = sandbox.SandboxMount.parse("/workspace,work,rw", ("secrets",)) + output = sandbox.SandboxMount.parse( + "/results/output.txt,output.txt,rw", kind="file" + ) + mixed = sandbox.SandboxLaunch( + layers=(distro,), + scratch=Path("scratch.ext4"), + mounts=(workspace, settings, output), + ) + names = [ + sandbox.aggregate_child_name(index, mount.guest_target) + for index, mount in enumerate(mixed.mounts) + ] + self.assertTrue( + mixed.kernel_command_line().endswith( + f" nvx_share={names[0]},/workspace,rw" + f" nvx_share={names[1]},/etc/app/settings.json,ro,file" + f" nvx_share={names[2]},/results/output.txt,rw,file" + ) + ) + arguments = mixed.openvmm_arguments() + self.assertEqual( + arguments[arguments.index("--mount-deny") + 1], + os.fspath(Path.cwd() / "work" / "secrets"), + ) + + # A file's mode applies to the whole file, so no policy path names + # one, and the runtime owns the machine ID's path. + for option in ("denied_paths", "allowed_paths", "writable_paths"): + policy: dict[str, Any] = {option: ("x",)} + with self.subTest(option=option): + with self.assertRaisesRegex(common.ScriptError, "shares a file"): + sandbox.SandboxMount( + guest_target="/results/output.txt", + host_path=Path("output.txt"), + access="rw", + kind="file", + **policy, + ) + with self.assertRaisesRegex( + common.ScriptError, "unsupported sandbox mount kind" + ): + sandbox.SandboxMount.parse("/a,a", kind="socket") + # The runtime replaces /etc/machine-id with a file, so nothing at or + # below it may be shared. + for target in ("/etc/machine-id", "/etc/machine-id/child"): + with self.subTest(target=target): + with self.assertRaisesRegex(common.ScriptError, "reserved"): + sandbox.SandboxMount.parse(f"{target},id", kind="file") + # Only /etc itself is reserved, not what lies below it. + sandbox.SandboxMount.parse("/etc/app,app") + + def test_launch_validation_requires_the_kind_of_each_share(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + config = root / "config" + config.mkdir() + settings = config / "settings.json" + settings.write_bytes(b"{}") + self.assertEqual(sandbox.mount_kind(settings), "file") + self.assertEqual(sandbox.mount_kind(config), "directory") + self.assertEqual(sandbox.mount_kind(root / "missing"), "directory") + share = sandbox.SandboxMount( + guest_target="/etc/app/settings.json", host_path=settings, kind="file" + ) + self.assertIs(share.validated(), share) + # A directory at the file's path is not shared in its place. + settings.unlink() + settings.mkdir() + with self.assertRaisesRegex(common.ScriptError, "not a regular file"): + share.validated() + settings.rmdir() + with self.assertRaisesRegex(common.ScriptError, "not a regular file"): + share.validated() + + # A file overlaps a shared directory that contains it, and another + # share of the same file, such as through a hard link, but not a + # file beside it. + settings.write_bytes(b"{}") + other = config / "other.json" + other.write_bytes(b"{}") + link = root / "link.json" + os.link(settings, link) + + def file_share(target: str, path: Path) -> sandbox.SandboxMount: + return sandbox.SandboxMount( + guest_target=target, host_path=path, kind="file" + ) + + sandbox.validate_mount_host_paths( + (file_share("/a.json", settings), file_share("/b.json", other)) + ) + for mounts, message in ( + ( + ( + sandbox.SandboxMount(guest_target="/config", host_path=config), + file_share("/a.json", settings), + ), + "overlap", + ), + ( + (file_share("/a.json", settings), file_share("/b.json", link)), + "same directory or file", + ), + ): + with self.subTest(message=message): + with self.assertRaisesRegex(common.ScriptError, message): + sandbox.validate_mount_host_paths(mounts) + + def test_managed_configuration_format_records_file_shares(self): + def entry(target: str, path: str, kind: str) -> dict[str, object]: + return { + "guest_target": target, + "host_path": path, + "access": "ro", + "denied_paths": [], + "owner": "vmm", + "allowed_paths": [], + "writable_paths": [], + "kind": kind, + } + + config: dict[str, object] = { + "format": sandbox_lifecycle.FILE_MOUNT_CONFIG_FORMAT, + "layers": [ + { + "role": "distro", + "path": "distro.erofs", + "uuid": "11111111-1111-1111-1111-111111111111", + } + ], + "scratch": "scratch.ext4", + "hostname": "nvx-sandbox", + "workload_uid": 65534, + "workload_gid": 65534, + "memory_max": None, + "pids_max": None, + "mounts": [entry("/etc/app/settings.json", "settings.json", "file")], + } + + def require(path: Path, _description: str) -> Path: + return path + + def unchanged(mount: sandbox.SandboxMount) -> sandbox.SandboxMount: + return mount + + with ( + patch.object(sandbox, "require_file", side_effect=require), + patch.object(sandbox.SandboxMount, "validated", unchanged), + ): + launch = sandbox_lifecycle._deserialize_launch(config) + self.assertEqual([mount.kind for mount in launch.mounts], ["file"]) + serialized = sandbox_lifecycle._serialize_launch( + launch, + hypervisor="kvm", + memory_mib=256, + net=None, + network_profile=None, + network_egress=None, + network_ingress=None, + network_egress_allow=(), + network_egress_deny=(), + host_loopback=None, + network_proxy=None, + host_loopback_forward=(), + cmdline="", + ) + self.assertEqual( + serialized["format"], sandbox_lifecycle.FILE_MOUNT_CONFIG_FORMAT + ) + self.assertEqual(serialized["mounts"][0]["kind"], "file") + # Only format 6 holds a file share, and every format-6 share + # records its kind. + config["mounts"] = [entry("/workspace", "work", "directory")] + with self.assertRaisesRegex(common.ScriptError, "does not match"): + sandbox_lifecycle._deserialize_launch(config) + config["mounts"] = [{**entry("/a.json", "a.json", "file"), "kind": None}] + with self.assertRaisesRegex(common.ScriptError, "kind"): + sandbox_lifecycle._deserialize_launch(config) + unmarked = entry("/a.json", "a.json", "file") + del unmarked["kind"] + config["mounts"] = [unmarked] + with self.assertRaisesRegex(common.ScriptError, "malformed"): + sandbox_lifecycle._deserialize_launch(config) + config["format"] = sandbox_lifecycle.POLICY_CONFIG_FORMAT + config["mounts"] = [entry("/a.json", "a.json", "file")] + with self.assertRaisesRegex(common.ScriptError, "does not match"): + sandbox_lifecycle._deserialize_launch(config) + def test_managed_configuration_format_binds_several_mounts(self): def entry(target: str, path: str, owner: str = "vmm") -> dict[str, object]: return { @@ -12709,7 +13187,7 @@ def identity(path: Path) -> tuple[int, int]: with ( patch.object(sandbox, "require_file", side_effect=require), patch.object(sandbox.SandboxMount, "validated", unchanged), - patch.object(sandbox, "_directory_identity", side_effect=identity), + patch.object(sandbox, "_path_identity", side_effect=identity), ): launch = sandbox_lifecycle._deserialize_launch(config) self.assertEqual(