From 03b7ccb22dc70e2d8f5431c4d33db0292976d3fb Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 12 Nov 2026 08:12:50 -0800 Subject: [PATCH] host: keep the export's pins in the CLI and paths only in ExportPlan ExportPlan, the public description of a VM's export, combined the host paths with the file identities that only the CLI's store needs: each child's denied_identities, each bind's identity, and FileIdentity, with check's rules that a recorded plan pins each object that a workload could move. The paths are what nvx lowers; the identities are the CLI's recheck of its own records before each start. The CLI now owns PlannedExport { description: ExportPlan, pins: Pins }, crate-private and cli-gated beside the planner and the verifier that SL6.5a2 made private: - The planner returns a PlannedExport: the description, and the pins of each child's denied paths and of each bind, in their orders, which it computes as before. The binds keep their pins as they are sorted. - PlannedExport::check runs ExportPlan::check, then the pin rules that moved out of it: a slot for each child and each of its denied paths, one for each bind, and the identity of each denied or mapped object strictly inside a part that the guest may write, with the same messages. PlannedExport::verify is the old ExportPlan::verify, which reads the identities from the pins. - The sandbox's config.json keeps the description under export, paths only, where CI's share checks read it, and records the pins beside it under export_pins, as it records each image beside its binding. Config::new requires the field and refuses an export without pins or pins without an export as malformed; Config::check checks and verifies the pair before vm_config passes the description to the VM. plan_mounts, provision's ProvisionConfig and check_images carry the PlannedExport; nvx-rs run, which launches its plan at once and never again, keeps the description and checks no pins, as before. nvx's public API loses FileIdentity, ExportChild::denied_identities, Bind::identity, and ExportPlan::openvmm_arguments and kernel_token, which only VmConfig's lowering calls and which become pub(crate); lowering is unchanged. ExportPlan::check stays structural and needs no identity data. host::filesystem documents what the caller keeps stable: the launch race, the writable-part recheck and durable denials. A configuration that an earlier build wrote has no export_pins, so a start refuses it as "sandbox configuration has no export pins; deprovision the sandbox and provision it again", as for every field that the record gained before; nothing is migrated, and the format stays 10. Tests: the planner's and verifier's unit tests read the pins from the PlannedExport, and narrowed_writes_pin_only_what_lies_in_writable_paths also checks the new slot rules; ExportPlan's own tests and fixtures, in host::config, host::vm, the restore tests and tests/cli/openvmm.rs, build plans without identities. A new state test records an export with its pins, reads both back, and refuses a moved, a gone and an uninspectable pinned object at Config::check. The CLI's lifecycle tests expect export_pins in each record, and malformed_configuration refuses a record without it, an export without pins, and pins without an export. Part of #1125 (SL6.5b). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/acceptance-case-matrix.md | 2 +- doc/run.md | 16 +- nvx-rs/README.md | 26 +- nvx-rs/public-api/linux.txt | 7 - nvx-rs/public-api/windows.txt | 7 - nvx-rs/src/host/cli/run.rs | 4 +- nvx-rs/src/host/cli/run/guest_tokens/tests.rs | 3 +- nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs | 15 +- .../host/cli/sandbox/lifecycle/start/tests.rs | 11 +- .../sandbox/lifecycle/start/tests/restore.rs | 2 - nvx-rs/src/host/cli/sandbox/mod.rs | 12 +- nvx-rs/src/host/cli/sandbox/state.rs | 48 ++- nvx-rs/src/host/cli/sandbox/state/tests.rs | 90 ++++++ nvx-rs/src/host/config/tests.rs | 4 - nvx-rs/src/host/filesystem/mod.rs | 141 +++++--- nvx-rs/src/host/filesystem/policy.rs | 2 + nvx-rs/src/host/filesystem/tests.rs | 144 +++++---- nvx-rs/src/host/filesystem/verify.rs | 305 ++++++++++-------- nvx-rs/src/host/sandbox/mount/aggregate.rs | 14 +- nvx-rs/src/host/sandbox/mount/tests.rs | 11 +- nvx-rs/src/host/sandbox/options/tests.rs | 4 +- nvx-rs/src/host/sandbox/tests.rs | 20 +- nvx-rs/src/host/vm/tests.rs | 4 - nvx-rs/tests/cli/lifecycle.rs | 54 +++- nvx-rs/tests/cli/openvmm.rs | 29 +- 25 files changed, 620 insertions(+), 355 deletions(-) diff --git a/doc/acceptance-case-matrix.md b/doc/acceptance-case-matrix.md index e6a64ff73..88f6102aa 100644 --- a/doc/acceptance-case-matrix.md +++ b/doc/acceptance-case-matrix.md @@ -242,7 +242,7 @@ The adapter's README said what each proved. | `a_run_whose_clone_does_not_start_runs_nothing` | retired | Template selections and the template's `run` leave with the old clone path (#1121) | | `a_run_whose_clone_s_vm_exits_during_its_execution_ends_with_the_clone_s_failure` | retired | The template's `run`, clone records and leases leave with the old clone path (#1121) | | `a_selection_that_names_no_template_leases_nothing` | retired | Template selections and leases leave with the old clone path (#1121) | -| `a_start_refuses_an_export_whose_mapped_objects_changed_before_any_vmm_starts` | rewritten | `nvx-rs/src/host/filesystem/tests.rs::an_exported_object_that_a_workload_could_move_is_checked_again_before_a_start`; `nvx-rs/src/host/filesystem/tests.rs::swapped_objects_fail_verification` | +| `a_start_refuses_an_export_whose_mapped_objects_changed_before_any_vmm_starts` | rewritten | `nvx-rs/src/host/filesystem/tests.rs::an_exported_object_that_a_workload_could_move_is_checked_again_before_a_start`; `nvx-rs/src/host/filesystem/tests.rs::swapped_objects_fail_verification`; `nvx-rs/src/host/cli/sandbox/state/tests.rs::records_keep_the_export_with_its_pins_and_starts_refuse_changed_pins`, whose record keeps the pins beside the export, and whose start refuses a changed pin before the VM's configuration takes the export (SL6.5b) | | `a_start_refuses_what_the_template_cannot_serve_before_any_vmm_starts` | rewritten | `nvx-rs/src/host/snapshot/tests.rs::snapshot_accepts_refuses_an_image_longer_than_its_placeholder`; `nvx-rs/src/host/snapshot/tests.rs::snapshot_accepts_refuses_each_scratch_geometry_mismatch`; `nvx-rs/src/host/snapshot/tests.rs::snapshot_accepts_refuses_each_memory_mismatch_without_rounding_or_overflow` | | `a_template_without_a_nic_serves_clones_without_a_network` | rewritten | `nvx-rs/src/host/snapshot/tests.rs::snapshot_accepts_refuses_both_nic_presence_mismatches_and_each_named_address_mismatch` | | `a_tombstone_leaves_the_running_clone_of_its_generation_to_its_lease` | retired | Tombstones, generations and leases leave with the old clone path (#1121) | diff --git a/doc/run.md b/doc/run.md index 563eb97c2..f6d2b6d4a 100644 --- a/doc/run.md +++ b/doc/run.md @@ -1300,13 +1300,15 @@ inside a writable part of a share. On Windows, it refuses a path that only a verbatim `\\?\` path names, such as that of a directory whose name ends in a period or a space, which the path without the prefix would not name. The shares' children follow their resolved host paths. A managed sandbox stores -the plan in its typed configuration, in format 10. Each `start` checks it again, -as `run` does once it has verified the distro layer: it refuses a host path -that no longer holds a directory or regular file as recorded, a recorded path -whose file identity changed, and shares whose host paths came to overlap, such -as through a hard link or a bind mount. NVX no longer reads earlier -configuration formats, including those that recorded the shares as requested; -deprovision such a sandbox and provision it again. +the plan, with those identities beside it, in its typed configuration, in +format 10. Each `start` checks it again, as `run` does once it has verified the +distro layer: it refuses a host path that no longer holds a directory or +regular file as recorded, a recorded path whose file identity changed, and +shares whose host paths came to overlap, such as through a hard link or a bind +mount. NVX no longer reads earlier configuration formats, including those that +recorded the shares as requested, nor a configuration that earlier builds wrote +without those identities beside the plan; deprovision such a sandbox and +provision it again. A share must not reach the sandbox's own files, which the workload could otherwise read or change: `provision` refuses a share whose resolved host path diff --git a/nvx-rs/README.md b/nvx-rs/README.md index cb8f04ee5..a6506e192 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -937,16 +937,26 @@ action that needs more than 256 rules. The `ExportPlan` of host paths through OpenVMM's aggregate virtio-fs device, for a root's `ReservedTargets`, the blockless root's or the container root's: its -children, its ordered binds, which give the host-mapping table, and the -identities that each start checks again; `ExportPlan::check`, which checks a -recorded plan without reading the host, and the plan's OpenVMM options, boot -token, and mapping table; and `guest_path` (also at `nvx::`), the guest path at -which the host-path convention places a resolved host path. +children and its ordered binds, which give the host-mapping table, paths only; +`ExportPlan::check`, which checks a recorded plan without reading the host, and +the plan's mapping table, `ExportPlan::guest_table`; and `guest_path` (also at +`nvx::`), the guest path at which the host-path convention places a resolved +host path. A `VmConfig` lowers the plan into OpenVMM's options and its +`nvx_maps` boot token internally. + +OpenVMM resolves the plan's paths only when it starts, so the caller keeps them +naming the objects that it means, as the module documents: across the launch +race, from its own checks of the host until OpenVMM opens the paths; through a +writable-part recheck before each later launch, since a workload that may write +a part of a child can move a mapped or denied object there and put a decoy at +its path; and for each durable denial, which holds only while its path names +the object that it hides. The planner, which turns the mapped host paths at their guest targets, their -denied, allowed, and write paths, and the owner into a plan, and the checks of a +denied, allowed, and write paths, and the owner into a plan, with the pins of +the objects that a workload could move, and the checks of those pins and of the plan's host objects before each start are the CLI's path rules, private to the -crate (feature `cli`). +crate (feature `cli`). The CLI records the pins beside the plan. ### `nvx::host::nvxs` @@ -1930,7 +1940,7 @@ provision it again: | File | Contents | | --- | --- | -| `config.json` | The provisioned sandbox, in format 10 only: `image` holds its format, path and identity; `upper` is tagged as scratch with its path or RAM with `max_memory_percent`. `distro_binding` binds the image, and `scratch_binding` binds scratch or is `null` for RAM. The export is typed, `null` when absent. `network`, `null` without `--net`, holds `--net`, `--network-egress`, `--network-ingress`, `--host-loopback`, `--network-proxy`, each `null` when not given, and the `--host-loopback-forward` options, as given, with the egress rules in OpenVMM's form, from a policy file's compile if one was given. A sandbox that `snapshot restore` provisioned also records its snapshot's name under `source_snapshot`, the absolute path of the snapshot's store under `source_store`, and the program and arguments of the workload that each restore runs under `workload`; one that boots cold records none of them. An atomic pretty-JSON write uses mode 0600, and start reads the same strict typed fields without a legacy reader. | +| `config.json` | The provisioned sandbox, in format 10 only: `image` holds its format, path and identity; `upper` is tagged as scratch with its path or RAM with `max_memory_percent`. `distro_binding` binds the image, and `scratch_binding` binds scratch or is `null` for RAM. The export is typed, `null` when absent, and holds paths only; `export_pins`, `null` without an export, holds the identities of the export's denied and mapped objects that a workload could move, which each start checks before it launches the export, and a configuration without it is refused with the advice to deprovision the sandbox and provision it again. `network`, `null` without `--net`, holds `--net`, `--network-egress`, `--network-ingress`, `--host-loopback`, `--network-proxy`, each `null` when not given, and the `--host-loopback-forward` options, as given, with the egress rules in OpenVMM's form, from a policy file's compile if one was given. A sandbox that `snapshot restore` provisioned also records its snapshot's name under `source_snapshot`, the absolute path of the snapshot's store under `source_store`, and the program and arguments of the workload that each restore runs under `workload`; one that boots cold records none of them. An atomic pretty-JSON write uses mode 0600, and start reads the same strict typed fields without a legacy reader. | | `sandbox.lock` | The store's lock, which serializes the lifecycle transitions | | `exec.lock` | The exec lock, which an `exec` holds from before it finds the running VM until its workload ends, so that one `exec` at a time runs in the sandbox; it stays from the first `exec` until `deprovision` | | `launch.json` | The store's journal of a start in progress, or of one that was interrupted | diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index e4d1a8cc1..1b2f72310 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -943,14 +943,12 @@ mod nvx::host::filesystem struct nvx::host::filesystem::Bind field nvx::host::filesystem::Bind::child method nvx::host::filesystem::Bind::guest_source -field nvx::host::filesystem::Bind::identity field nvx::host::filesystem::Bind::read_only field nvx::host::filesystem::Bind::source field nvx::host::filesystem::Bind::target struct nvx::host::filesystem::ExportChild field nvx::host::filesystem::ExportChild::allowed field nvx::host::filesystem::ExportChild::denied -field nvx::host::filesystem::ExportChild::denied_identities field nvx::host::filesystem::ExportChild::file field nvx::host::filesystem::ExportChild::root field nvx::host::filesystem::ExportChild::writable @@ -960,12 +958,7 @@ field nvx::host::filesystem::ExportPlan::binds method nvx::host::filesystem::ExportPlan::check field nvx::host::filesystem::ExportPlan::children method nvx::host::filesystem::ExportPlan::guest_table -method nvx::host::filesystem::ExportPlan::kernel_token -method nvx::host::filesystem::ExportPlan::openvmm_arguments field nvx::host::filesystem::ExportPlan::owner -struct nvx::host::filesystem::FileIdentity -field nvx::host::filesystem::FileIdentity::device -field nvx::host::filesystem::FileIdentity::index const nvx::host::filesystem::GUEST_EXPORT const nvx::host::filesystem::MAX_CHILDREN enum nvx::host::filesystem::Owner diff --git a/nvx-rs/public-api/windows.txt b/nvx-rs/public-api/windows.txt index ae266b3ac..93a33df47 100644 --- a/nvx-rs/public-api/windows.txt +++ b/nvx-rs/public-api/windows.txt @@ -401,14 +401,12 @@ mod nvx::host::filesystem struct nvx::host::filesystem::Bind field nvx::host::filesystem::Bind::child method nvx::host::filesystem::Bind::guest_source -field nvx::host::filesystem::Bind::identity field nvx::host::filesystem::Bind::read_only field nvx::host::filesystem::Bind::source field nvx::host::filesystem::Bind::target struct nvx::host::filesystem::ExportChild field nvx::host::filesystem::ExportChild::allowed field nvx::host::filesystem::ExportChild::denied -field nvx::host::filesystem::ExportChild::denied_identities field nvx::host::filesystem::ExportChild::file field nvx::host::filesystem::ExportChild::root field nvx::host::filesystem::ExportChild::writable @@ -418,12 +416,7 @@ field nvx::host::filesystem::ExportPlan::binds method nvx::host::filesystem::ExportPlan::check field nvx::host::filesystem::ExportPlan::children method nvx::host::filesystem::ExportPlan::guest_table -method nvx::host::filesystem::ExportPlan::kernel_token -method nvx::host::filesystem::ExportPlan::openvmm_arguments field nvx::host::filesystem::ExportPlan::owner -struct nvx::host::filesystem::FileIdentity -field nvx::host::filesystem::FileIdentity::device -field nvx::host::filesystem::FileIdentity::index const nvx::host::filesystem::GUEST_EXPORT const nvx::host::filesystem::MAX_CHILDREN enum nvx::host::filesystem::Owner diff --git a/nvx-rs/src/host/cli/run.rs b/nvx-rs/src/host/cli/run.rs index 7d9bad90e..d334b97c3 100644 --- a/nvx-rs/src/host/cli/run.rs +++ b/nvx-rs/src/host/cli/run.rs @@ -616,6 +616,7 @@ fn cold_config(options: &Options, rules: &NativeRules) -> Result { read_only: *access == MountAccess::ReadOnly, }) .collect(); + // A run launches its export right after it plans it, and never again, so it checks no pins. let filesystem = filesystem::plan(&ExportRequest { mappings: &mappings, denied: PathList { @@ -637,7 +638,8 @@ fn cold_config(options: &Options, rules: &NativeRules) -> Result { }, reserved: ReservedTargets::BLOCKLESS_ROOT, }) - .map_err(error_with_cause)?; + .map_err(error_with_cause)? + .map(|planned| planned.description); cold_vm_config(options, rules, filesystem) } diff --git a/nvx-rs/src/host/cli/run/guest_tokens/tests.rs b/nvx-rs/src/host/cli/run/guest_tokens/tests.rs index 0f41932e6..540efbaf1 100644 --- a/nvx-rs/src/host/cli/run/guest_tokens/tests.rs +++ b/nvx-rs/src/host/cli/run/guest_tokens/tests.rs @@ -74,7 +74,8 @@ fn planned_boot_shares_keep_table_sources_targets_access_and_file_kind() { reserved: ReservedTargets::BLOCKLESS_ROOT, }) .unwrap() - .unwrap(); + .unwrap() + .description; let table = plan.guest_table(); assert_eq!(table[1].source, "0/single.txt"); let line = render("", Some(&plan)).unwrap(); diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs index 1b11bc969..29e0f1b16 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs @@ -1,9 +1,10 @@ //! Persistent sandboxes on the CLI's store and caller-owned [`crate::host::vm`] interface. //! -//! Provision records the typed image and upper, image bindings, network and export. Start journals the -//! prepared capability and outputs before spawn, records the verified process, and commits the -//! matching saved VM only after authenticated readiness and mappings. Every lifecycle command -//! takes the CLI store's recoverable lock; indeterminate launches retain their evidence. +//! Provision records the typed image and upper, image bindings, network, and export with its pins. +//! Start journals the prepared capability and outputs before spawn, records the verified process, +//! and commits the matching saved VM only after authenticated readiness and mappings. Every +//! lifecycle command takes the CLI store's recoverable lock; indeterminate launches retain their +//! evidence. //! //! A restored sandbox, whose configuration names a named snapshot, restores it at each start in //! place of a cold boot, under the snapshot's lock, through the blocking restore driver, which @@ -38,7 +39,7 @@ use crate::host::cli::sandbox::{Artifacts, error_with_cause}; use crate::host::cli::session::Session; use crate::host::cli::store::StateFile; use crate::host::cli::{Exit, Result}; -use crate::host::filesystem::ExportPlan; +use crate::host::filesystem::PlannedExport; use crate::host::process::{Opened, Process}; use crate::host::runtime::{IdentityPart, LaunchOptions, Runtime, RuntimeIdentity}; use crate::host::sandbox::layer::SandboxLayer; @@ -74,8 +75,8 @@ pub(crate) struct ProvisionConfig<'a> { pub(crate) hostname: &'a str, /// `--workload-user`. pub(crate) workload_identity: (u32, u32), - /// The export plan of the shares, if any. - pub(crate) export: Option<&'a ExportPlan>, + /// The export plan of the shares, with its pins, if any. + pub(crate) export: Option<&'a PlannedExport>, /// The network options, or `None` without `--net`. pub(crate) network: Option<&'a NetworkRecord>, } diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests.rs index 6a7ca9d85..90c6caedb 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests.rs @@ -4,7 +4,7 @@ use std::fs; use std::path::PathBuf; use super::*; -use crate::host::filesystem::ExportPlan; +use crate::host::filesystem::PlannedExport; use crate::host::paths::resolve; use crate::host::sandbox::distro::DistroBinding; use crate::host::sandbox::layer::{LayerRole, SandboxLayer}; @@ -111,11 +111,12 @@ fn starts_check_the_kernel_command_line_that_they_launch() { let export = |shares: &[String]| plan_mounts(&sandbox_mounts(shares, &[], None).unwrap()); let read_only = export(std::slice::from_ref(&settings)).unwrap(); let writable = export(&[settings, output]).unwrap(); + let tokens = |export: &PlannedExport| export_tokens(&export.description); assert_eq!( - export_tokens(read_only.as_ref().unwrap()), + tokens(read_only.as_ref().unwrap()), " virtfs_dir=/run/nvx/hostfs/root virtfs_tag=microvm virtfs_mode=ro virtfs_aggregate=1" ); - let configure = |cmdline: &str, scratch: Option<&Path>, export: Option<&ExportPlan>| { + let configure = |cmdline: &str, scratch: Option<&Path>, export: Option<&PlannedExport>| { let config = ProvisionConfig { hypervisor: "kvm", memory_mib: 256, @@ -130,7 +131,7 @@ fn starts_check_the_kernel_command_line_that_they_launch() { let vm = config.vm_config().unwrap(); (config, vm) }; - let refusal = |cmdline: &str, scratch: Option<&Path>, export: Option<&ExportPlan>| { + let refusal = |cmdline: &str, scratch: Option<&Path>, export: Option<&PlannedExport>| { let (config, vm) = configure(cmdline, scratch, export); let error = check_command_line(&config, &vm).unwrap_err(); assert_eq!(error.code(), Some(ErrorCode::Unsupported), "{error:?}"); @@ -139,7 +140,7 @@ fn starts_check_the_kernel_command_line_that_they_launch() { for upper in [None, Some(scratch.as_path())] { for export in [None, read_only.as_ref(), writable.as_ref()] { let (_, vm) = configure("", upper, export); - let fragment = export.map(export_tokens).unwrap_or_default(); + let fragment = export.map(tokens).unwrap_or_default(); let room = COMMAND_LINE_BUDGET - vm.kernel_command_line().unwrap().len() - fragment.len() - 2; let (config, vm) = configure(&"x".repeat(room), upper, export); diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs index f974b9867..d4f5c4cee 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs @@ -268,7 +268,6 @@ fn restored_config(fixture: &Fixture, gpt: bool, ram: bool) -> VmConfig { file: false, writable: false, denied: Vec::new(), - denied_identities: Vec::new(), allowed: Vec::new(), write: Vec::new(), }], @@ -277,7 +276,6 @@ fn restored_config(fixture: &Fixture, gpt: bool, ram: bool) -> VmConfig { source: String::new(), target: "/work".to_owned(), read_only: true, - identity: None, }], owner: Owner::Vmm, }); diff --git a/nvx-rs/src/host/cli/sandbox/mod.rs b/nvx-rs/src/host/cli/sandbox/mod.rs index 52dffcc65..bd7a52e68 100644 --- a/nvx-rs/src/host/cli/sandbox/mod.rs +++ b/nvx-rs/src/host/cli/sandbox/mod.rs @@ -28,7 +28,7 @@ use super::session::ExecRequest; use super::{Exit, Result, io_error, output}; use crate::ErrorCode; use crate::host::artifacts::{Guest, KERNEL_NAME, require_artifact}; -use crate::host::filesystem::{ExportPlan, ReservedTargets}; +use crate::host::filesystem::{ExportPlan, PlannedExport, ReservedTargets}; use crate::host::paths; use crate::host::runtime::RuntimeIdentity; use crate::host::sandbox::DEFAULT_ENTRYPOINT; @@ -306,7 +306,7 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re validate_mounts(&mounts)?; check_images(&layer, scratch.as_deref(), None)?; // The shares are planned once, from their resolved host paths, and `provision` records - // the plan, whose identities each `start` checks again. + // the plan with its pins, which each `start` checks again. let export = plan_mounts(&mounts)?; if let Some(export) = &export { export.check(ReservedTargets::CONTAINER_ROOT)?; @@ -352,7 +352,7 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re Some(("state directory", Path::new(state_dir))), layer.path(), scratch.as_deref(), - export.as_ref(), + export.as_ref().map(|export| &export.description), located.as_ref(), )?; lifecycle::provision( @@ -426,11 +426,11 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re } /// Returns the options of `options` that `provision` records beside the images of its launch, with -/// `export`, the export plan of its shares, and `network`, the record of its network options, -/// which a run records and reads back too. +/// `export`, the export plan of its shares with its pins, and `network`, the record of its network +/// options, which a run records and reads back too. fn recorded<'a>( options: &'a Options, - export: Option<&'a ExportPlan>, + export: Option<&'a PlannedExport>, network: Option<&'a NetworkRecord>, ) -> ProvisionConfig<'a> { ProvisionConfig { diff --git a/nvx-rs/src/host/cli/sandbox/state.rs b/nvx-rs/src/host/cli/sandbox/state.rs index 15729d333..7856c3668 100644 --- a/nvx-rs/src/host/cli/sandbox/state.rs +++ b/nvx-rs/src/host/cli/sandbox/state.rs @@ -1,6 +1,6 @@ //! The CLI's typed `config.json`, in format 10 only: one EROFS or GPT image by path, //! a scratch upper by path or a RAM upper with its explicit cap, the image bindings, -//! and the workload's view, network and export. +//! and the workload's view, network and export, with the export's pins. //! //! Provision checks the same [`VmConfig`] that start reconstructs. Older records, those of format //! 9 among them, are refused, not migrated, with the advice to deprovision the sandbox and @@ -11,6 +11,10 @@ //! [`check_images`] and [`check_workload`], and each start checks them again in the record, with //! [`Config::check`]. //! +//! The export is the [`ExportPlan`] of the shares, its paths only, which each start passes to the +//! VM's configuration once it has checked the [`Pins`] that the record holds beside it under +//! `export_pins`: the identities of the objects in it that a workload could move. +//! //! The network is the [`NetworkRecord`] of the sandbox's network options, from which each start //! builds the network's policy again, as provision built it. //! @@ -40,7 +44,7 @@ use crate::host::config::{ Boot, ConsoleAttachment, LayeredRoot, NetworkConfig, RootConfig, UpperConfig, VmConfig, reject_container_path, reject_disk_path, }; -use crate::host::filesystem::{ExportPlan, ReservedTargets}; +use crate::host::filesystem::{ExportPlan, Pins, PlannedExport, ReservedTargets}; use crate::host::paths; use crate::host::sandbox::distro::{DISTRO_POLICY_REVISION, DistroBinding, DistroFormat}; use crate::host::sandbox::layer::{LayerRole, SandboxLayer}; @@ -156,14 +160,14 @@ pub(crate) fn check_workload( /// regular file whose path, and the path to which it resolves, has no comma, semicolon, or /// unbalanced bracket, and does not end in an extension with which OpenVMM would not attach it as /// a raw image. It also refuses `export`, if any, once its directories and files overlap, by path -/// or through a hard link or a bind mount, or once [`ExportPlan::verify`] refuses it. +/// or through a hard link or a bind mount, or once [`PlannedExport::verify`] refuses it. /// /// A missing image fails with [`ErrorCode::Unavailable`], as `distro EROFS layer not found: /// PATH`, `distro GPT image not found: PATH`, or `ext4 scratch image not found: PATH`. pub(crate) fn check_images( distro: &SandboxLayer, scratch: Option<&Path>, - export: Option<&ExportPlan>, + export: Option<&PlannedExport>, ) -> Result { let image = match distro.format() { DistroFormat::Erofs => "distro EROFS layer", @@ -191,7 +195,7 @@ pub(crate) fn check_images( } } if let Some(export) = export { - check_distinct_export_roots(export, &HostQueries::native())?; + check_distinct_export_roots(&export.description, &HostQueries::native())?; export.verify()?; } Ok(()) @@ -416,7 +420,8 @@ pub(crate) fn serialize_launch( cmdline: config.cmdline.to_owned(), distro_binding: distro.binding.clone(), scratch_binding: scratch.map(|scratch| scratch.binding.clone()), - export: config.export.cloned(), + export: config.export.map(|export| export.description.clone()), + export_pins: config.export.map(|export| export.pins.clone()), source_snapshot: source.map(|source| source.snapshot.clone()), source_store: source .map(|source| path_string(&source.store)) @@ -469,6 +474,9 @@ pub(crate) struct Config { cmdline: String, #[serde(deserialize_with = "Option::deserialize")] export: Option, + /// The pins of the export, which a record holds exactly when it holds an export. + #[serde(deserialize_with = "Option::deserialize")] + export_pins: Option, distro_binding: DistroBinding, #[serde(deserialize_with = "Option::deserialize")] scratch_binding: Option, @@ -509,7 +517,11 @@ impl Config { /// Returns the fields of `object`, a configuration that [`read_config`] read, which fails as /// `sandbox configuration is malformed` without a field that it requires, with a value of - /// another type, or with a network that its record cannot build again. + /// another type, with a network that its record cannot build again, or with an export without + /// its pins or pins without an export. A configuration without the field of a binding, of the + /// network, of the export, or of the export's pins, such as one that `nvx-rs` wrote before it + /// recorded the pins beside the export, fails as `sandbox configuration has no ...`, with the + /// advice to deprovision the sandbox and provision it again. /// /// Guest RAM that is a number, but not an integer from 1 to [`MAX_MEMORY_MIB`], the range of /// `--memory-mib`, fails as `sandbox configuration records N MiB of guest RAM`, which names @@ -523,6 +535,7 @@ impl Config { ("scratch_binding", "scratch binding"), ("network", "network policy"), ("export", "export plan"), + ("export_pins", "export pins"), ] { if !object.contains_key(name) { return Err(Exit::error( @@ -555,6 +568,9 @@ impl Config { if let Some(network) = &config.network { network.config()?; } + if config.export.is_some() != config.export_pins.is_some() { + return Err(malformed()); + } Ok(config) } @@ -571,8 +587,9 @@ impl Config { /// Checks the recorded image, upper, workload and export at each start, as the plan of the /// sandbox profile checked them: a RAM upper's cap from 1 to 90 percent, or the configuration /// is malformed; the image's identity, a UUID in its canonical text; the workload, as - /// [`check_workload`] checks it with the default entrypoint; the export plan, for the - /// container root; and the images and the export plan, as [`check_images`] checks them. + /// [`check_workload`] checks it with the default entrypoint; the export plan and its pins, for + /// the container root; and the images and the export plan with its pins, as [`check_images`] + /// checks them, before [`vm_config`](Self::vm_config) passes the plan to the VM. pub(crate) fn check(&self) -> Result { if let Upper::Ram { max_memory_percent } = &self.upper && !RAM_UPPER_CAPS.contains(max_memory_percent) @@ -589,10 +606,11 @@ impl Config { return Err(Exit::error(ErrorCode::Unsupported, message)); } check_workload(DEFAULT_ENTRYPOINT, &[], &self.hostname, identity)?; - if let Some(export) = &self.export { + let export = self.planned_export(); + if let Some(export) = &export { export.check(ReservedTargets::CONTAINER_ROOT)?; } - check_images(&distro, self.scratch().as_deref(), self.export.as_ref()) + check_images(&distro, self.scratch().as_deref(), export.as_ref()) } /// Returns the recorded image, as the distro layer that `start` checks. @@ -619,6 +637,14 @@ impl Config { self.export.as_ref() } + /// Returns the export plan of the shares with its pins, if any. + fn planned_export(&self) -> Option { + Some(PlannedExport { + description: self.export.clone()?, + pins: self.export_pins.clone()?, + }) + } + /// The binding that start checks against the recorded image. pub(crate) fn distro_binding(&self) -> &DistroBinding { &self.distro_binding diff --git a/nvx-rs/src/host/cli/sandbox/state/tests.rs b/nvx-rs/src/host/cli/sandbox/state/tests.rs index 4ffeb45ea..69f10f9cb 100644 --- a/nvx-rs/src/host/cli/sandbox/state/tests.rs +++ b/nvx-rs/src/host/cli/sandbox/state/tests.rs @@ -2,8 +2,98 @@ use serde_json::json; use super::*; use crate::host::sandbox::layer::LayerRole; +use crate::host::sandbox::mount::{MountKind, MountOwner, MountPolicy, SandboxMount, plan_mounts}; use crate::host::sandbox::{DEFAULT_HOSTNAME, DEFAULT_WORKLOAD_IDENTITY}; +/// A configuration records the export's description, its paths alone, under `export`, and its +/// pins beside it under `export_pins`, and reads both back. Each start's check, which comes before +/// the VM's configuration takes the description, refuses a pin that names another object now, as +/// when a workload moved the denied object and put a decoy at its path, or one that the host can +/// no longer inspect. +#[test] +fn records_keep_the_export_with_its_pins_and_starts_refuse_changed_pins() { + let directory = tempfile::tempdir().unwrap(); + let root = paths::resolve(directory.path()).unwrap(); + let image = root.join("distro.erofs"); + std::fs::write(&image, b"distro layer\n").unwrap(); + // The guest may write all of the share, so a workload could move the denied `secret`. + let work = root.join("work"); + std::fs::create_dir_all(work.join("secret")).unwrap(); + let share = SandboxMount::parse( + &format!("/workspace,{},rw", work.display()), + MountPolicy { + denied: vec!["secret".to_owned()], + ..MountPolicy::default() + }, + MountOwner::Vmm, + MountKind::Directory, + ) + .unwrap(); + let export = plan_mounts(&[share]).unwrap().unwrap(); + assert!(export.pins.denied[0][0].is_some()); + let layer = SandboxLayer::new( + LayerRole::Distro, + image.to_str().unwrap(), + "11111111-1111-1111-1111-111111111111", + ); + let distro = RecordedDistro::new(&layer, DistroBinding::verify(&layer).unwrap()).unwrap(); + let options = ProvisionConfig { + hypervisor: "whp", + memory_mib: 128, + cmdline: "", + ram_upper_percent: None, + hostname: DEFAULT_HOSTNAME, + workload_identity: DEFAULT_WORKLOAD_IDENTITY, + export: Some(&export), + network: None, + }; + let text = serialize_launch(&distro, None, &options, None).unwrap(); + let object: Object = serde_json::from_str(&text).unwrap(); + assert_eq!( + object["export"], + serde_json::to_value(&export.description).unwrap() + ); + assert_eq!( + object["export_pins"], + serde_json::to_value(&export.pins).unwrap() + ); + let config = Config::new(object.clone()).unwrap(); + assert_eq!(config.planned_export().as_ref(), Some(&export)); + config.check().unwrap(); + assert_eq!( + config.vm_config().unwrap().filesystem.as_ref(), + Some(&export.description) + ); + + let refused = |config: &Config, code: ErrorCode, message: &str| { + let error = config.check().unwrap_err(); + assert_eq!(error.code(), Some(code), "{error:?}"); + assert!( + error.message().unwrap_or_default().contains(message), + "{error:?}" + ); + }; + let changed = "no longer names the host object it named at provision"; + std::fs::rename(work.join("secret"), work.join("moved")).unwrap(); + std::fs::create_dir(work.join("secret")).unwrap(); + refused(&config, ErrorCode::Other, changed); + std::fs::remove_dir(work.join("secret")).unwrap(); + refused(&config, ErrorCode::Other, changed); + std::fs::rename(work.join("moved"), work.join("secret")).unwrap(); + config.check().unwrap(); + // A record whose pinned path the host cannot look up, here by a name too long for its file + // system, shows nothing of the object, and fails as the host's own failure. + let mut unnamed = object; + let long = work.join("n".repeat(300)); + unnamed["export"]["children"][0]["denied"] = json!([long]); + let unnamed = Config::new(unnamed).unwrap(); + refused( + &unnamed, + ErrorCode::Io, + &format!("cannot inspect {}", long.display()), + ); +} + #[test] fn restored_sandbox_records_round_trip_only_their_own_source() { let directory = tempfile::tempdir().unwrap(); diff --git a/nvx-rs/src/host/config/tests.rs b/nvx-rs/src/host/config/tests.rs index 51b8d2eb0..745fca1a5 100644 --- a/nvx-rs/src/host/config/tests.rs +++ b/nvx-rs/src/host/config/tests.rs @@ -89,7 +89,6 @@ fn export(target: &str) -> ExportPlan { file: false, writable: false, denied: Vec::new(), - denied_identities: Vec::new(), allowed: Vec::new(), write: Vec::new(), }], @@ -98,7 +97,6 @@ fn export(target: &str) -> ExportPlan { source: String::new(), target: target.to_owned(), read_only: true, - identity: None, }], owner: Owner::Vmm, } @@ -1043,7 +1041,6 @@ fn mapping_table_is_the_exact_export_projection_counted_by_the_boot_token() { source: String::new(), target: "/zother".to_owned(), read_only: false, - identity: None, }); plan.binds.push(Bind { source: "nested/file".to_owned(), @@ -2078,7 +2075,6 @@ fn configuration_command_lines_keep_the_same_reserve_on_every_host() { .join(format!("{index:03}-{}", "x".repeat(120))) }) .collect(); - plan.children[0].denied_identities = vec![None; 128]; cold(VmConfig { filesystem: Some(plan), ..blockless() diff --git a/nvx-rs/src/host/filesystem/mod.rs b/nvx-rs/src/host/filesystem/mod.rs index 9f0ee1372..4a4d26538 100644 --- a/nvx-rs/src/host/filesystem/mod.rs +++ b/nvx-rs/src/host/filesystem/mod.rs @@ -1,8 +1,9 @@ //! Host paths exposed to the guest through OpenVMM's aggregate virtio-fs export, for the blockless //! root and for the container root of a layered sandbox alike: the [`ExportPlan`] that a caller -//! records, checks, and lowers into OpenVMM's options and the guest's mapping table. The planner -//! that makes a plan of host paths, with its path rules, and the checks of a plan's host objects -//! before each start belong to NVX's CLI, private to the crate (feature `cli`). +//! records and checks, and that a VM's configuration lowers into OpenVMM's options and the guest's +//! mapping table. The planner that makes a plan of host paths, with its path rules, the pins of +//! the objects that a workload could move, and the checks of a plan's host objects before each +//! start belong to NVX's CLI, private to the crate (feature `cli`). //! //! OpenVMM offers one virtio-fs device, which a plan attaches as an aggregate: a synthetic, //! read-only root that lists one child per exported host directory or file, named by its index, @@ -14,7 +15,7 @@ //! A plan names the mapped host paths, each read-only or read-write at its guest target; the host //! paths that OpenVMM hides inside them, and the paths inside those that stay visible; the paths to //! which the writes of a read-write directory are narrowed; and the host identity of the file -//! operations. +//! operations. It names paths only, never the objects at them. //! //! - A caller gives each target, such as the guest path that MXC's convention for Linux guests //! derives from a resolved host path ([`fn@guest_path`]): a Windows path `C:\work\src` appears @@ -31,11 +32,32 @@ //! cannot keep a path read-only inside a writable part, so a read-only path there is refused on //! every host, while a read-write path inside a read-only directory is not. OpenVMM also hides the //! denied paths, except for the allowed paths inside them. -//! -//! The plan records the identities of the mapped and denied paths that a workload could move, -//! those inside a writable part, and the CLI checks them again before each start. //! 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. +//! +//! # What the caller keeps stable +//! +//! `nvx` reads nothing on the host when it checks or lowers a plan, and OpenVMM resolves the +//! plan's paths only when it starts, so the caller keeps them naming the objects that it means: +//! +//! - **The launch race.** From its own last check of the host until OpenVMM has opened the paths, +//! the caller keeps each path naming the object that it checked: an object that is renamed in +//! between, with another put at its path, leaves OpenVMM to export, hide, or show the other. +//! NVX's CLI checks a recorded plan just before it starts the VM, while no workload of the plan +//! runs. OpenVMM checks each child's kind again when it opens it. +//! - **The writable-part recheck.** A workload that may write a part of a child, all of a writable +//! child without write paths or one of its write paths, can rename a mapped or denied object +//! strictly inside that part and put a decoy at its path, which a later launch of the same plan +//! would bind or hide instead. Before each launch of a plan with which a workload has run, the +//! caller checks that each such path still names the object that it named when the caller made +//! the plan. NVX's CLI pins the identities of those objects when it plans an export, and checks +//! them before each start; it pins no other object, since host edits often replace files. +//! - **Durable denial.** A denial holds only while its path names the object that it hides: once +//! the object moves, a later launch hides whatever lies at the path then, and shows the object +//! wherever the export reaches it, and a denied path that names no object hides nothing that a +//! workload creates at it later. The caller keeps each denied object at its path for as long as +//! its denial is to hold, out of the reach of workloads or rechecked as above. NVX's CLI refuses +//! a denied path inside a mapped path that does not exist. mod guest_path; mod host_path; @@ -172,7 +194,9 @@ impl Owner { } /// The host paths of one sandbox, as OpenVMM's aggregate export exports them: the children of the -/// export, the bind mounts of the guest, and the host identity of the file operations. +/// export, the bind mounts of the guest, and the host identity of the file operations. It names +/// paths only, which the caller keeps stable, as [`host::filesystem`](crate::host::filesystem) +/// describes. /// /// Callers record the plan, so its serde form is part of their state: a caller versions its /// records, and a record of another version is not read. @@ -203,9 +227,6 @@ pub struct ExportChild { /// Canonical host paths inside the child that OpenVMM hides; the root itself, when it shows /// only its allowed paths. pub denied: Vec, - /// Identities, at provision, of the denied paths that a workload could move: those inside a - /// writable part. - pub denied_identities: Vec>, /// Canonical paths inside denied paths that stay visible. pub allowed: Vec, /// Canonical paths to which OpenVMM limits the guest's writes; empty if the guest may modify @@ -214,13 +235,40 @@ pub struct ExportChild { } /// Device and file index of a host object, which survive renames. +#[cfg(feature = "cli")] #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] -pub struct FileIdentity { +pub(crate) struct FileIdentity { /// Device, or the volume serial number on Windows. - pub device: u64, + pub(crate) device: u64, /// Inode, or the file index on Windows. - pub index: u64, + pub(crate) index: u64, +} + +/// An export that the CLI's planner made: the description of its paths, which the CLI passes to a +/// VM's configuration, and the pins of the objects in it that a workload could move, which the CLI +/// records beside the description and checks before each start. +#[cfg(feature = "cli")] +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct PlannedExport { + /// The paths of the export. + pub(crate) description: ExportPlan, + /// The identities of the objects in it that a workload could move. + pub(crate) pins: Pins, +} + +/// The identities, when the CLI planned an export, of the denied and mapped objects that a +/// workload could move: those strictly inside a part of a child that the guest may write. +#[cfg(feature = "cli")] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub(crate) struct Pins { + /// For each child of the export, in order, the identity of each of its denied paths, in order, + /// that a workload could move, and `None` for each other. + pub(crate) denied: Vec>>, + /// For each bind of the export, in order, the identity of the object that it maps, if a + /// workload could move it. + pub(crate) binds: Vec>, } /// One mapped host path. @@ -235,10 +283,6 @@ pub struct Bind { pub target: String, /// Whether the guest mounts it read-only. pub read_only: bool, - /// Identity of the host object at provision, when it lies inside a writable part and a - /// workload could therefore move it. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub identity: Option, } impl Bind { @@ -307,7 +351,8 @@ pub(crate) struct ExportRequest<'a> { pub(crate) reserved: ReservedTargets<'a>, } -/// Plans the export of `request`, or returns `None` if it maps nothing. +/// Plans the export of `request`, with the pins of the objects in it that a workload could move, +/// or returns `None` if it maps nothing. /// /// Mapped paths must exist, and resolve to paths that are valid UTF-8, so two spellings of one /// object (links, letter case, or short names) name one object. A mapping's target, which the @@ -331,10 +376,13 @@ pub(crate) struct ExportRequest<'a> { /// path between them, and a write path must not be hidden. The caller owner needs a Linux host, /// on which no exported directory or file may belong to UID 0 or GID 0. /// +/// The pins hold the identity of each denied and mapped object strictly inside a part of a child +/// that the guest may write, which [`PlannedExport::verify`] checks before each start. +/// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) when the request breaks /// these rules or exceeds a limit of OpenVMM or the guest agent. #[cfg(feature = "cli")] -pub(crate) fn plan(request: &ExportRequest<'_>) -> Result> { +pub(crate) fn plan(request: &ExportRequest<'_>) -> Result> { let unsupported = |message: String| Err(Error::unsupported(message)); if request.owner == Owner::Caller && cfg!(windows) { return unsupported(CALLER_OWNER_ON_WINDOWS.to_owned()); @@ -428,6 +476,7 @@ pub(crate) fn plan(request: &ExportRequest<'_>) -> Result> { .any(|(root, part)| path.starts_with(root) && part.moves(root, path)) }; let mut children = Vec::with_capacity(roots.len()); + let mut denied_pins = Vec::with_capacity(roots.len()); let mut policy_bytes = 0; for (root, part) in roots.iter().zip(&parts) { // Every root is a mapping itself, so the guest reaches all of it. @@ -454,12 +503,12 @@ pub(crate) fn plan(request: &ExportRequest<'_>) -> Result> { .iter() .map(|path| movable(path).then(|| identity(path)).transpose()) .collect::>>()?; + denied_pins.push(denied_identities); children.push(ExportChild { root: root.clone(), file: !own.directory, writable: part.writable, denied: child_denied, - denied_identities, allowed: child_allowed, write: part.write.clone(), }); @@ -493,37 +542,47 @@ pub(crate) fn plan(request: &ExportRequest<'_>) -> Result> { entry.given.display() )); } - binds.push(Bind { - child, - source, - target: entry.target.clone(), - read_only: entry.read_only, - identity: movable(&entry.canonical) + binds.push(( + Bind { + child, + source, + target: entry.target.clone(), + read_only: entry.read_only, + }, + movable(&entry.canonical) .then(|| identity(&entry.canonical)) .transpose()?, - }); + )); } - binds.sort_by(|left, right| { + // The pins follow the binds into their order. + binds.sort_by(|(left, _), (right, _)| { depth(&left.target) .cmp(&depth(&right.target)) .then_with(|| left.target.cmp(&right.target)) }); + let (binds, bind_pins) = binds.into_iter().unzip(); if request.owner == Owner::Caller { check_caller_roots(&children, unsupported_inspection)?; } - let plan = ExportPlan { - children, - binds, - owner: request.owner, + let planned = PlannedExport { + description: ExportPlan { + children, + binds, + owner: request.owner, + }, + pins: Pins { + denied: denied_pins, + binds: bind_pins, + }, }; - // Callers read a record of the plan back through `check`, which accepts every plan that this - // function makes. + // The CLI reads a record of the plan and its pins back through `check`, which accepts every + // plan that this function makes. debug_assert!( - plan.check(request.reserved).is_ok(), + planned.check(request.reserved).is_ok(), "the planner made a plan that its check refuses: {:?}", - plan.check(request.reserved) + planned.check(request.reserved) ); - Ok(Some(plan)) + Ok(Some(planned)) } /// Resolves one requested mapping: its host path must exist as a directory or regular file that @@ -704,8 +763,8 @@ fn unsupported_inspection(path: &Path, error: std::io::Error) -> Error { Error::unsupported(format!("cannot inspect {}", path.display())).with_source(error) } -/// Returns the failure of [`ExportPlan::verify`] for `path`, which no longer names the host object -/// that the plan found there. +/// Returns the failure of [`PlannedExport::verify`] for `path`, which no longer names the host +/// object that the plan found there. #[cfg(feature = "cli")] fn changed_object(path: &Path) -> Error { Error::other(format!( @@ -715,7 +774,7 @@ fn changed_object(path: &Path) -> Error { )) } -/// Returns the failure of [`ExportPlan::verify`] that cannot inspect `path`, an object that the +/// Returns the failure of [`PlannedExport::verify`] that cannot inspect `path`, an object that the /// plan found, with `error`: a path that no longer reaches the object, as the host sees it, whose /// object went, a component of which is no longer a directory or is a link that [`loops`], or /// that the host may no longer search or open, names it no longer, [`changed_object`], and any diff --git a/nvx-rs/src/host/filesystem/policy.rs b/nvx-rs/src/host/filesystem/policy.rs index 8d7940b12..87c4899fd 100644 --- a/nvx-rs/src/host/filesystem/policy.rs +++ b/nvx-rs/src/host/filesystem/policy.rs @@ -444,6 +444,7 @@ pub(super) fn nearest_rule<'p>( } /// Whether a child is writable, and the paths to which OpenVMM limits its writes. +#[cfg(feature = "cli")] pub(super) struct Writable { /// Whether the guest may modify the child. pub(super) writable: bool, @@ -452,6 +453,7 @@ pub(super) struct Writable { pub(super) write: Vec, } +#[cfg(feature = "cli")] impl Writable { /// Returns whether a workload could move `path`, which lies in the child at `root`: whether it /// lies strictly inside a writable part, in a directory that the workload may change. diff --git a/nvx-rs/src/host/filesystem/tests.rs b/nvx-rs/src/host/filesystem/tests.rs index 7caab2148..c6fa98dcf 100644 --- a/nvx-rs/src/host/filesystem/tests.rs +++ b/nvx-rs/src/host/filesystem/tests.rs @@ -16,9 +16,10 @@ struct FilesystemPolicy { } /// Plans `policy` into the blockless root, each path at the guest path at which MXC's convention -/// for Linux guests places it, [`convention`]. +/// for Linux guests places it, [`convention`], with the pins of the objects that a workload could +/// move. #[cfg(feature = "cli")] -fn plan(policy: &FilesystemPolicy) -> Result> { +fn planned(policy: &FilesystemPolicy) -> Result> { let entries: Vec<(&str, &Path, String, bool)> = [ ("filesystem.readonlyPaths", &policy.readonly_paths, true), ("filesystem.readwritePaths", &policy.readwrite_paths, false), @@ -48,6 +49,12 @@ fn plan(policy: &FilesystemPolicy) -> Result> { }) } +/// Plans `policy` as [`planned`] does, and returns the description of the export alone. +#[cfg(feature = "cli")] +fn plan(policy: &FilesystemPolicy) -> Result> { + Ok(planned(policy)?.map(|planned| planned.description)) +} + /// Returns the guest path at which MXC's convention for Linux guests places `path`: the one that /// [`guest_path`] derives from the resolved host path, so that every spelling of one object /// (links, letter case, or short names) takes one target. A path that does not resolve takes an @@ -630,7 +637,10 @@ fn denied_paths_belong_to_the_directory_that_contains_them() { base.join("elsewhere"), ], }; - let mapping = plan(&policy).unwrap().unwrap(); + let PlannedExport { + description: mapping, + pins, + } = planned(&policy).unwrap().unwrap(); let denied: Vec> = mapping .children .iter() @@ -645,13 +655,7 @@ fn denied_paths_belong_to_the_directory_that_contains_them() { ] ); // Nothing a workload could move is pinned without a read-write parent. - assert!( - mapping - .children - .iter() - .flat_map(|child| &child.denied_identities) - .all(Option::is_none) - ); + assert!(pins.denied.iter().flatten().all(Option::is_none)); assert_eq!( option_values(&mapping.openvmm_arguments(), "--mount-deny").len(), 2 @@ -908,13 +912,8 @@ fn swapped_objects_fail_verification() { denied_paths: vec![base.join("work/src/secret")], ..FilesystemPolicy::default() }; - let mapping = plan(&policy).unwrap().unwrap(); - assert!( - mapping.children[0] - .denied_identities - .iter() - .all(Option::is_some) - ); + let mapping = planned(&policy).unwrap().unwrap(); + assert!(mapping.pins.denied[0].iter().all(Option::is_some)); mapping.verify().unwrap(); fs::rename(base.join("work/src/secret"), base.join("work/src/moved")).unwrap(); fs::create_dir(base.join("work/src/secret")).unwrap(); @@ -932,7 +931,7 @@ fn swapped_objects_fail_verification() { readonly_paths: vec![base.join("tools")], ..FilesystemPolicy::default() }; - let mapping = plan(&read_only).unwrap().unwrap(); + let mapping = planned(&read_only).unwrap().unwrap(); fs::remove_dir_all(base.join("tools")).unwrap(); fs::create_dir(base.join("tools")).unwrap(); mapping.verify().unwrap(); @@ -945,7 +944,7 @@ fn swapped_objects_fail_verification() { readonly_paths: vec![settings.clone(), base.join("tools")], ..FilesystemPolicy::default() }; - let mapping = plan(&with_file).unwrap().unwrap(); + let mapping = planned(&with_file).unwrap().unwrap(); mapping.verify().unwrap(); fs::remove_file(&settings).unwrap(); fs::create_dir(&settings).unwrap(); @@ -970,18 +969,16 @@ fn objects_that_cannot_be_inspected_fail_verification_by_what_the_failure_shows( denied_paths: vec![base.join("work/src/secret")], ..FilesystemPolicy::default() }; - let mapping = plan(&policy).unwrap().unwrap(); + let mapping = planned(&policy).unwrap().unwrap(); mapping.verify().unwrap(); // A pinned object at a name that the host's file system cannot hold. let mut unnamed = mapping.clone(); - let pinned = unnamed - .binds - .iter() - .position(|bind| bind.identity.is_some()) - .unwrap(); - unnamed.binds[pinned].source = "n".repeat(300); - let bind = &unnamed.binds[pinned]; - let path = unnamed.children[bind.child].root.join(&bind.source); + let pinned = unnamed.pins.binds.iter().position(Option::is_some).unwrap(); + unnamed.description.binds[pinned].source = "n".repeat(300); + let bind = &unnamed.description.binds[pinned]; + let path = unnamed.description.children[bind.child] + .root + .join(&bind.source); let error = unnamed.verify().unwrap_err(); assert_eq!(error.kind(), ErrorKind::Io, "{error}"); assert_eq!( @@ -1062,8 +1059,9 @@ fn read_only_binds_that_the_host_cannot_enforce_fail_verification() { let directory = root(); let base = directory.path(); fs::create_dir_all(base.join("work/out/cache")).unwrap(); - let refused = |mut mapping: ExportPlan, source: &str| { + let refused = |mut mapping: PlannedExport, source: &str| { let bind = mapping + .description .binds .iter_mut() .find(|bind| bind.source == source) @@ -1077,7 +1075,7 @@ fn read_only_binds_that_the_host_cannot_enforce_fail_verification() { ); }; // A read-only path inside a read-write root, all of which the guest may write. - let writable_root = plan(&FilesystemPolicy { + let writable_root = planned(&FilesystemPolicy { readwrite_paths: vec![base.join("work"), base.join("work/src")], ..FilesystemPolicy::default() }) @@ -1087,7 +1085,7 @@ fn read_only_binds_that_the_host_cannot_enforce_fail_verification() { refused(writable_root, "src"); // A read-only path inside a writable path of a read-only root, whose other read-only // paths stay read-only on the host. - let writable_path = plan(&FilesystemPolicy { + let writable_path = planned(&FilesystemPolicy { readonly_paths: vec![base.join("work"), base.join("work/src")], readwrite_paths: vec![base.join("work/out"), base.join("work/out/cache")], ..FilesystemPolicy::default() @@ -1164,7 +1162,7 @@ fn explicit_targets_place_mappings_at_any_free_guest_path() { at(&work, "/mirror", false), ]; let request = plain_request(&mappings, ReservedTargets::CONTAINER_ROOT); - let mapping = super::plan(&request).unwrap().unwrap(); + let mapping = super::plan(&request).unwrap().unwrap().description; let roots: Vec<&Path> = mapping .children .iter() @@ -1388,7 +1386,8 @@ fn policy_paths_hide_show_and_narrow_writes() { paths(&["src/secret/public"]), paths(&["out"]), ); - let mapping = request(&whole, &denied, &allowed, &write).unwrap().unwrap(); + let planned = request(&whole, &denied, &allowed, &write).unwrap().unwrap(); + let mapping = &planned.description; let child = &mapping.children[0]; let resolved = canonical(&work); assert!(child.writable); @@ -1399,7 +1398,7 @@ fn policy_paths_hide_show_and_narrow_writes() { ); assert_eq!(child.write, [resolved.join("out")]); // Writes are narrowed to `out`, so nothing outside it can move. - assert_eq!(child.denied_identities, [None]); + assert_eq!(planned.pins.denied[0], [None]); let arguments = mapping.openvmm_arguments(); let text = |path: PathBuf| vec![display_path(&path)]; assert_eq!( @@ -1415,12 +1414,13 @@ fn policy_paths_hide_show_and_narrow_writes() { text(resolved.join("out")) ); assert!(!arguments.contains(&"--mount-owner".to_owned())); - mapping.check(ReservedTargets::CONTAINER_ROOT).unwrap(); + planned.check(ReservedTargets::CONTAINER_ROOT).unwrap(); // A denied root shows only its allowed paths. let mapping = request(&whole, std::slice::from_ref(&work), &paths(&["src"]), &[]) .unwrap() - .unwrap(); + .unwrap() + .description; assert_eq!(mapping.children[0].denied, std::slice::from_ref(&resolved)); assert_eq!(mapping.children[0].allowed, [resolved.join("src")]); let error = request(&whole, std::slice::from_ref(&work), &[], &[]).unwrap_err(); @@ -1545,27 +1545,25 @@ fn narrowed_writes_pin_only_what_lies_in_writable_paths() { ..plain_request(&mappings, ReservedTargets::CONTAINER_ROOT) }; let mapping = super::plan(&request).unwrap().unwrap(); - let child = &mapping.children[0]; + let child = &mapping.description.children[0]; assert_eq!(child.write, [canonical(&work).join("out")]); - let pinned: Vec = child - .denied_identities - .iter() - .map(Option::is_some) - .collect(); + let pinned: Vec = mapping.pins.denied[0].iter().map(Option::is_some).collect(); assert_eq!(pinned, [true, false]); let pinned: Vec<(&str, bool)> = mapping + .description .binds .iter() - .map(|bind| (bind.target.as_str(), bind.identity.is_some())) + .zip(&mapping.pins.binds) + .map(|(bind, pin)| (bind.target.as_str(), pin.is_some())) .collect(); assert_eq!( pinned, [("/cache", true), ("/src", false), ("/workspace", false)] ); - // A record keeps every pin of the plan, and narrows writes only to paths that keep its - // read-write mappings writable. + // A record keeps a slot for each denied path and bind, and every pin of the plan, and + // narrows writes only to paths that keep its read-write mappings writable. mapping.check(ReservedTargets::CONTAINER_ROOT).unwrap(); - let refused = |change: fn(&mut ExportPlan), expected: &str| { + let refused = |change: fn(&mut PlannedExport), expected: &str| { let mut changed = mapping.clone(); change(&mut changed); let error = changed.check(ReservedTargets::CONTAINER_ROOT).unwrap_err(); @@ -1573,20 +1571,39 @@ fn narrowed_writes_pin_only_what_lies_in_writable_paths() { assert!(error.message().contains(expected), "{error}"); }; refused( - |changed| changed.children[0].denied_identities[0] = None, + |changed| changed.pins.denied[0][0] = None, "does not pin the denied path", ); refused( - |changed| changed.binds[0].identity = None, + |changed| changed.pins.binds[0] = None, "does not pin the object that it maps at /cache", ); refused( - |changed| changed.children[0].write = vec![changed.children[0].root.join("other")], + |changed| { + changed.pins.denied[0].pop(); + }, + "child 0 has 2 denied paths but 1 identities", + ); + refused( + |changed| { + changed.pins.binds.pop(); + }, + "it pins 1 children and 2 binds, but it has 1 children and 3 binds", + ); + refused( + |changed| changed.pins.denied.push(Vec::new()), + "it pins 2 children and 3 binds, but it has 1 children and 3 binds", + ); + refused( + |changed| { + let child = &mut changed.description.children[0]; + child.write = vec![child.root.join("other")]; + }, "which it maps read-write, stays read-only", ); // Without write paths, the guest could move `src/secret` too. refused( - |changed| changed.children[0].write.clear(), + |changed| changed.description.children[0].write.clear(), "does not pin the denied path", ); mapping.verify().unwrap(); @@ -1616,18 +1633,19 @@ fn the_caller_owner_reaches_openvmm_on_linux_hosts() { let owned = fs::metadata(&work).unwrap(); if owned.uid() != 0 && owned.gid() != 0 { - let mapping = super::plan(&request).unwrap().unwrap(); + let planned = super::plan(&request).unwrap().unwrap(); + let mapping = &planned.description; assert_eq!(mapping.owner, Owner::Caller); let arguments = mapping.openvmm_arguments(); assert_eq!( arguments[arguments.len() - 2..], ["--mount-owner", "caller"] ); - mapping.verify().unwrap(); - let record = serde_json::to_value(&mapping).unwrap(); + planned.verify().unwrap(); + let record = serde_json::to_value(mapping).unwrap(); assert_eq!(record["owner"], "caller"); assert_eq!( - serde_json::from_value::(record).unwrap(), + &serde_json::from_value::(record).unwrap(), mapping ); } @@ -1651,7 +1669,8 @@ fn the_caller_owner_reaches_openvmm_on_linux_hosts() { // The default owner is left out of the record. let vmm = super::plan(&plain_request(&mappings, ReservedTargets::CONTAINER_ROOT)) .unwrap() - .unwrap(); + .unwrap() + .description; assert!(serde_json::to_value(&vmm).unwrap().get("owner").is_none()); } @@ -1710,7 +1729,8 @@ fn read_write_mappings_to_which_writes_are_narrowed_stay_visible() { &[], ) .unwrap() - .unwrap(); + .unwrap() + .description; let child = &mapping.children[0]; assert!(child.writable && child.write.is_empty()); assert_eq!(child.denied, [canonical(&private)]); @@ -1742,7 +1762,10 @@ fn denied_mapped_directories_inside_denied_ones_show_their_allowed_paths() { let both = [at(&tree, "/tree", false), at(&private, "/private", false)]; // Each mapped directory is denied itself and shows the allowed path inside it; OpenVMM // needs only the outer denial, which hides the inner one already. - let mapping = request(&both, &[private.join("public")]).unwrap().unwrap(); + let mapping = request(&both, &[private.join("public")]) + .unwrap() + .unwrap() + .description; let child = &mapping.children[0]; assert_eq!(child.denied, [canonical(&tree)]); assert_eq!(child.allowed, [canonical(&private).join("public")]); @@ -1754,7 +1777,8 @@ fn denied_mapped_directories_inside_denied_ones_show_their_allowed_paths() { &[private.join("public")], ) .unwrap() - .unwrap(); + .unwrap() + .description; let child = &mapping.children[0]; assert_eq!(child.root, canonical(&private)); assert_eq!(child.denied, [canonical(&private)]); @@ -1785,7 +1809,6 @@ fn recorded_plans_hold_no_more_policy_than_one_aggregate_accepts() { file: false, writable: true, denied: paths("denied"), - denied_identities: vec![None; 4], allowed: (0..4) .map(|path| hidden.join(format!("allowed-{path}"))) .collect(), @@ -1800,7 +1823,6 @@ fn recorded_plans_hold_no_more_policy_than_one_aggregate_accepts() { source: String::new(), target: format!("/c{index}"), read_only: false, - identity: None, }) .collect(), owner: Owner::Vmm, @@ -1846,7 +1868,7 @@ fn mapped_paths_resolve_to_utf8_paths() { /// Plans the export of `mappings` into the blockless root. #[cfg(feature = "cli")] -fn plan_blockless(mappings: &[Mapping<'_>]) -> Result> { +fn plan_blockless(mappings: &[Mapping<'_>]) -> Result> { super::plan(&plain_request(mappings, ReservedTargets::BLOCKLESS_ROOT)) } diff --git a/nvx-rs/src/host/filesystem/verify.rs b/nvx-rs/src/host/filesystem/verify.rs index fb7890826..1fb4a9e14 100644 --- a/nvx-rs/src/host/filesystem/verify.rs +++ b/nvx-rs/src/host/filesystem/verify.rs @@ -7,7 +7,9 @@ use std::path::PathBuf; use super::guest_path::depth; use super::host_path::{display_path, relative_text}; use super::limits::policy_limits; -use super::policy::{Writable, nearest_rule, policy_rule_problem}; +#[cfg(feature = "cli")] +use super::policy::Writable; +use super::policy::{nearest_rule, policy_rule_problem}; use super::{ Bind, ExportChild, ExportPlan, GUEST_EXPORT, MAX_AGGREGATE_POLICY_BYTES, MAX_CHILDREN, MAX_GUEST_PATH, Owner, ReservedTargets, outermost, resolved_path_problem, target_problem, @@ -15,7 +17,8 @@ use super::{ }; #[cfg(feature = "cli")] use super::{ - CALLER_OWNER_ON_WINDOWS, FileIdentity, changed_object, check_caller_roots, uninspected, + CALLER_OWNER_ON_WINDOWS, FileIdentity, PlannedExport, changed_object, check_caller_roots, + uninspected, }; use crate::error::{Error, Result}; #[cfg(feature = "cli")] @@ -27,17 +30,16 @@ impl ExportPlan { /// that reserves `reserved`: at least one and at most [`MAX_CHILDREN`] children, none inside /// another, each an absolute host path below a volume's root, with its host paths in the form /// in which `plan` resolves them, whose policy paths lie inside it within OpenVMM's limits and - /// keep its rules, with an identity slot for each denied path, and writable exactly when a bind - /// maps a path of it read-write; and at least one and at most [`MAX_HOST_MAPPINGS`] binds, - /// each of a known child, from a relative source that holds the host's names joined with `/`, - /// that a guest path can hold, and that the child does not hide, unless it shows paths inside - /// it again, to a free guest target, parents before children, with a bind of every child's - /// root. A target inside another's must show what the other shows there. A child's write - /// paths must be the outermost paths of its read-write binds, unless one maps the whole child, - /// when they may narrow its writes only to paths that keep its other read-write binds - /// writable; and each denied or mapped object that a workload could move, strictly inside a - /// part of the child that the guest may write, must carry its identity. It reads nothing from - /// the host, which the CLI checks before each start. + /// keep its rules, and writable exactly when a bind maps a path of it read-write; and at least + /// one and at most [`MAX_HOST_MAPPINGS`] binds, each of a known child, from a relative source + /// that holds the host's names joined with `/`, that a guest path can hold, and that the child + /// does not hide, unless it shows paths inside it again, to a free guest target, parents before + /// children, with a bind of every child's root. A target inside another's must show what the + /// other shows there. A child's write paths must be the outermost paths of its read-write + /// binds, unless one maps the whole child, when they may narrow its writes only to paths that + /// keep its other read-write binds writable. It reads nothing from the host: the caller keeps + /// the paths naming the objects that it means, as [`host::filesystem`](crate::host::filesystem) + /// describes. /// /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported). pub fn check(&self, reserved: ReservedTargets<'_>) -> Result<()> { @@ -77,13 +79,6 @@ impl ExportPlan { "child {index} has the host path {path:?}, which {problem}" )); } - if child.denied_identities.len() != child.denied.len() { - return invalid(format!( - "child {index} has {} denied paths but {} identities", - child.denied.len(), - child.denied_identities.len() - )); - } if let Some(path) = [&child.denied, &child.allowed, &child.write] .into_iter() .flatten() @@ -291,114 +286,6 @@ impl ExportPlan { )); } } - // A workload that may write a part of the child could move an object there and put a - // decoy at its path, so the plan pins each denied and mapped object in such a part, as - // `plan` does, for `verify` to check it at the next start. - let part = Writable { - writable: child.writable, - write: child.write.clone(), - }; - if let Some((path, _)) = child - .denied - .iter() - .zip(&child.denied_identities) - .find(|(path, identity)| identity.is_none() && part.moves(&child.root, path)) - { - return invalid(format!( - "child {index} does not pin the denied path {}, which a workload could move", - path.display() - )); - } - if let Some(bind) = self.binds.iter().find(|bind| { - bind.child == index - && bind.identity.is_none() - && part.moves(&child.root, &bound(bind)) - }) { - return invalid(format!( - "it does not pin the object that it maps at {}, which a workload could move", - bind.target - )); - } - } - Ok(()) - } - - /// Checks that the host keeps every read-only mapping read-only, that every child's root is - /// still the kind of object that [`crate::host::filesystem::plan`] exported, that every mapped and denied path inside a - /// writable part still names the object it named then, and that the host still supports the - /// owner. - /// - /// [`crate::host::filesystem::plan`] refuses a read-only path inside a part that the guest may write, which only the - /// guest's bind mount would keep read-only; a recorded plan that holds one fails with - /// [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported). A file that a directory replaced - /// would otherwise grant the whole directory, and a workload that may write a directory could - /// rename a denied or mapped object inside it and put a decoy at its path, so the next start - /// would hide or bind the decoy instead; these fail with - /// [`ErrorKind::Other`](crate::ErrorKind::Other), as does a path that no longer reaches its - /// object, as the host sees it: one whose object went, one a component of which is no longer a - /// directory or is a link that loops, as a link that names itself does, and one that the host - /// may no longer search or open. With the caller owner, the - /// host must be a Linux host, and no exported directory or file may belong to UID 0 or GID 0; - /// these fail with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported). Any other - /// failure to inspect an object, which shows nothing of the object, fails with - /// [`ErrorKind::Io`](crate::ErrorKind::Io), with its cause. Run it while the VM is stopped, so - /// that no workload can race with it; OpenVMM checks each child's kind again when it opens it. - #[cfg(feature = "cli")] - pub(crate) fn verify(&self) -> Result<()> { - for bind in self.binds.iter().filter(|bind| bind.read_only) { - let (child, path) = self.bound(bind)?; - // Without writable paths, the guest may write all of a writable child. - if child.writable - && (child.write.is_empty() - || child.write.iter().any(|write| path.starts_with(write))) - { - return Err(Error::unsupported(format!( - "the sandbox maps {} read-only inside a part of {} that the guest may write, \ - where the host cannot keep it read-only; deprovision the sandbox, and \ - provision it again without read-only paths inside read-write directories", - path.display(), - child.root.display() - ))); - } - } - for child in &self.children { - let kind_matches = match fs::symlink_metadata(&child.root) { - Ok(metadata) if child.file => metadata.is_file(), - Ok(metadata) => metadata.is_dir(), - Err(error) => return Err(uninspected(&child.root, error)), - }; - if !kind_matches { - return Err(changed_object(&child.root)); - } - } - for bind in &self.binds { - let Some(expected) = bind.identity else { - continue; - }; - let (_, path) = self.bound(bind)?; - match platform::file_identity(&path) { - Ok((device, index)) if (FileIdentity { device, index }) == expected => {} - Ok(_) => return Err(changed_object(&path)), - Err(error) => return Err(uninspected(&path, error)), - } - } - for child in &self.children { - for (path, expected) in child.denied.iter().zip(&child.denied_identities) { - let Some(expected) = expected else { - continue; - }; - match platform::file_identity(path) { - Ok((device, index)) if (FileIdentity { device, index }) == *expected => {} - Ok(_) => return Err(changed_object(path)), - Err(error) => return Err(uninspected(path, error)), - } - } - } - if self.owner == Owner::Caller { - if cfg!(windows) { - return Err(Error::unsupported(CALLER_OWNER_ON_WINDOWS)); - } - check_caller_roots(&self.children, uninspected)?; } Ok(()) } @@ -421,7 +308,7 @@ impl ExportPlan { /// Kernel command-line token that tells the guest agent how many bind mounts to expect over /// the control channel. - pub fn kernel_token(&self) -> String { + pub(crate) fn kernel_token(&self) -> String { format!("{MAPS_TOKEN}{}", self.binds.len()) } @@ -440,7 +327,7 @@ impl ExportPlan { /// OpenVMM options that export the plan's directories and files at [`GUEST_EXPORT`], with /// their access policies and the owner. A file child carries the `file` flag, so OpenVMM /// refuses a directory that replaced the file. - pub fn openvmm_arguments(&self) -> Vec { + pub(crate) fn openvmm_arguments(&self) -> Vec { let mut arguments = vec!["--mount-aggregate".to_owned(), GUEST_EXPORT.to_owned()]; for (index, child) in self.children.iter().enumerate() { let mode = if child.writable { "rw" } else { "ro" }; @@ -487,3 +374,161 @@ impl ExportChild { &self.write } } + +#[cfg(feature = "cli")] +impl PlannedExport { + /// Checks that an export read from the CLI's record is one that `plan` could make for a root + /// that reserves `reserved`: its description passes [`ExportPlan::check`], and its pins hold a + /// slot for each child, with one for each of its denied paths, and one for each bind, and pin + /// each denied or mapped object that a workload could move, strictly inside a part of its + /// child that the guest may write. It reads nothing from the host, which + /// [`verify`](Self::verify) checks before each start. + /// + /// Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported). + pub(crate) fn check(&self, reserved: ReservedTargets<'_>) -> Result<()> { + let Self { description, pins } = self; + description.check(reserved)?; + let invalid = |reason: String| { + Err(Error::unsupported(format!( + "the recorded export plan is invalid: {reason}" + ))) + }; + if pins.denied.len() != description.children.len() + || pins.binds.len() != description.binds.len() + { + return invalid(format!( + "it pins {} children and {} binds, but it has {} children and {} binds", + pins.denied.len(), + pins.binds.len(), + description.children.len(), + description.binds.len() + )); + } + for (index, (child, denied)) in description.children.iter().zip(&pins.denied).enumerate() { + if denied.len() != child.denied.len() { + return invalid(format!( + "child {index} has {} denied paths but {} identities", + child.denied.len(), + denied.len() + )); + } + // A workload that may write a part of the child could move an object there and put a + // decoy at its path, so the plan pins each denied and mapped object in such a part, as + // `plan` does, for `verify` to check it at the next start. + let part = Writable { + writable: child.writable, + write: child.write.clone(), + }; + if let Some((path, _)) = child + .denied + .iter() + .zip(denied) + .find(|(path, identity)| identity.is_none() && part.moves(&child.root, path)) + { + return invalid(format!( + "child {index} does not pin the denied path {}, which a workload could move", + path.display() + )); + } + if let Some((bind, _)) = + description + .binds + .iter() + .zip(&pins.binds) + .find(|(bind, identity)| { + bind.child == index + && identity.is_none() + && description + .bound(bind) + .is_ok_and(|(_, path)| part.moves(&child.root, &path)) + }) + { + return invalid(format!( + "it does not pin the object that it maps at {}, which a workload could move", + bind.target + )); + } + } + Ok(()) + } + + /// Checks that the host keeps every read-only mapping read-only, that every child's root is + /// still the kind of object that `plan` exported, that every pinned object is still the one + /// that its path named then, and that the host still supports the owner, before the CLI + /// passes the description to a VM's configuration. + /// + /// `plan` refuses a read-only path inside a part that the guest may write, which only the + /// guest's bind mount would keep read-only; a recorded plan that holds one fails with + /// [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported). A file that a directory replaced + /// would otherwise grant the whole directory, and a workload that may write a directory could + /// rename a denied or mapped object inside it and put a decoy at its path, so the next start + /// would hide or bind the decoy instead; these fail with + /// [`ErrorKind::Other`](crate::ErrorKind::Other), as does a path that no longer reaches its + /// object, as the host sees it: one whose object went, one a component of which is no longer a + /// directory or is a link that loops, as a link that names itself does, and one that the host + /// may no longer search or open. With the caller owner, the host must be a Linux host, and no + /// exported directory or file may belong to UID 0 or GID 0; these fail with + /// [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported). Any other failure to inspect an + /// object, which shows nothing of the object, fails with [`ErrorKind::Io`](crate::ErrorKind::Io), + /// with its cause. Run it after [`check`](Self::check), while the VM is stopped, so that no + /// workload can race with it; OpenVMM checks each child's kind again when it opens it. + pub(crate) fn verify(&self) -> Result<()> { + let Self { description, pins } = self; + for bind in description.binds.iter().filter(|bind| bind.read_only) { + let (child, path) = description.bound(bind)?; + // Without writable paths, the guest may write all of a writable child. + if child.writable + && (child.write.is_empty() + || child.write.iter().any(|write| path.starts_with(write))) + { + return Err(Error::unsupported(format!( + "the sandbox maps {} read-only inside a part of {} that the guest may write, \ + where the host cannot keep it read-only; deprovision the sandbox, and \ + provision it again without read-only paths inside read-write directories", + path.display(), + child.root.display() + ))); + } + } + for child in &description.children { + let kind_matches = match fs::symlink_metadata(&child.root) { + Ok(metadata) if child.file => metadata.is_file(), + Ok(metadata) => metadata.is_dir(), + Err(error) => return Err(uninspected(&child.root, error)), + }; + if !kind_matches { + return Err(changed_object(&child.root)); + } + } + for (bind, expected) in description.binds.iter().zip(&pins.binds) { + let Some(expected) = expected else { + continue; + }; + let (_, path) = description.bound(bind)?; + match platform::file_identity(&path) { + Ok((device, index)) if (FileIdentity { device, index }) == *expected => {} + Ok(_) => return Err(changed_object(&path)), + Err(error) => return Err(uninspected(&path, error)), + } + } + for (child, identities) in description.children.iter().zip(&pins.denied) { + for (path, expected) in child.denied.iter().zip(identities) { + let Some(expected) = expected else { + continue; + }; + match platform::file_identity(path) { + Ok((device, index)) if (FileIdentity { device, index }) == *expected => {} + Ok(_) => return Err(changed_object(path)), + Err(error) => return Err(uninspected(path, error)), + } + } + } + if description.owner == Owner::Caller { + if cfg!(windows) { + return Err(Error::unsupported(CALLER_OWNER_ON_WINDOWS)); + } + check_caller_roots(&description.children, uninspected)?; + } + Ok(()) + } +} diff --git a/nvx-rs/src/host/sandbox/mount/aggregate.rs b/nvx-rs/src/host/sandbox/mount/aggregate.rs index 9f1d584ba..6982fc390 100644 --- a/nvx-rs/src/host/sandbox/mount/aggregate.rs +++ b/nvx-rs/src/host/sandbox/mount/aggregate.rs @@ -9,7 +9,7 @@ use super::{ }; use crate::error::Result; use crate::host::filesystem::{ - self, ExportPlan, ExportRequest, Mapping, Owner, PathList, ReservedTargets, Target, + self, ExportRequest, Mapping, Owner, PathList, PlannedExport, ReservedTargets, Target, }; use crate::host::paths; use crate::host::sandbox::refused; @@ -24,12 +24,13 @@ use crate::host::sandbox::refused; /// requires to be one, becomes the export's, and the container root reserves the trees of its /// runtime. Each share keeps the kind with which it was requested: a host path that the planner /// finds to be of the other kind is refused, as [`SandboxMount::validated`] refuses it. Returns -/// `None` without shares. +/// the export with the pins of the objects in it that a workload could move, or `None` without +/// shares. /// /// Fails as those checks and [`filesystem::plan`] do, and with /// [`ErrorKind::Io`](crate::ErrorKind::Io) if a relative host path meets an unreadable current /// directory, or Windows cannot qualify a drive-relative path. -pub(crate) fn plan_mounts(mounts: &[SandboxMount]) -> Result> { +pub(crate) fn plan_mounts(mounts: &[SandboxMount]) -> Result> { validate_mounts(mounts)?; for mount in mounts { mount.validated()?; @@ -43,7 +44,7 @@ pub(crate) fn plan_mounts(mounts: &[SandboxMount]) -> Result> pub(crate) fn plan_mounts_with( mounts: &[SandboxMount], cwd: &dyn Fn() -> io::Result, -) -> Result> { +) -> Result> { let Some(first) = mounts.first() else { return Ok(None); }; @@ -89,7 +90,10 @@ pub(crate) fn plan_mounts_with( // since the share was requested is refused rather than shared whole, and so is a host path // that now leads into another share's directory, where the share would have no root of its // own. - if let Some(plan) = &plan { + if let Some(PlannedExport { + description: plan, .. + }) = &plan + { for mount in mounts { let Some(bind) = plan .binds diff --git a/nvx-rs/src/host/sandbox/mount/tests.rs b/nvx-rs/src/host/sandbox/mount/tests.rs index 3d409baf7..d45eb4bf7 100644 --- a/nvx-rs/src/host/sandbox/mount/tests.rs +++ b/nvx-rs/src/host/sandbox/mount/tests.rs @@ -546,7 +546,8 @@ fn shares_are_planned_through_the_export_planner() { ] }; assert_eq!(plan_mounts(&[]).unwrap(), None); - let plan = plan_mounts(&mounts(MountOwner::Vmm)).unwrap().unwrap(); + let planned = plan_mounts(&mounts(MountOwner::Vmm)).unwrap().unwrap(); + let plan = &planned.description; let work = resolved(&base.join("work")); // The children follow their resolved host paths, and each share maps its root at its target, // parents first, with its policy paths resolved. @@ -568,7 +569,7 @@ fn shares_are_planned_through_the_export_planner() { assert_eq!(workspace.allowed, [work.join("secrets").join("public")]); assert_eq!(workspace.write, [work.join("out")]); // Writes are narrowed to `out`, so nothing that the plan names can move. - assert_eq!(workspace.denied_identities, [None]); + assert_eq!(planned.pins.denied[2], [None]); let binds: Vec<(usize, &str, &str, bool)> = plan .binds .iter() @@ -590,8 +591,8 @@ fn shares_are_planned_through_the_export_planner() { ] ); assert_eq!(plan.owner, Owner::Vmm); - plan.verify().unwrap(); - plan.check(ReservedTargets::CONTAINER_ROOT).unwrap(); + planned.verify().unwrap(); + planned.check(ReservedTargets::CONTAINER_ROOT).unwrap(); // The planner refuses what OpenVMM would: here, a writable path that a denied path hides. let error = plan_mounts(&[share( @@ -612,7 +613,7 @@ fn shares_are_planned_through_the_export_planner() { let owned = std::fs::metadata(directory.path()).unwrap(); if owned.uid() != 0 && owned.gid() != 0 { let plan = plan_mounts(&mounts(MountOwner::Caller)).unwrap().unwrap(); - assert_eq!(plan.owner, Owner::Caller); + assert_eq!(plan.description.owner, Owner::Caller); } } } diff --git a/nvx-rs/src/host/sandbox/options/tests.rs b/nvx-rs/src/host/sandbox/options/tests.rs index 6a155985a..08c9b809e 100644 --- a/nvx-rs/src/host/sandbox/options/tests.rs +++ b/nvx-rs/src/host/sandbox/options/tests.rs @@ -992,7 +992,9 @@ impl Shares { .check()?; let mounts = sandbox_mounts(&mounts, &policy_paths, owner)?; validate_mounts(&mounts)?; - let export = plan_mounts(&mounts)?.expect("shares have an export plan"); + let export = plan_mounts(&mounts)? + .expect("shares have an export plan") + .description; Ok((export.openvmm_arguments(), export.kernel_token())) } } diff --git a/nvx-rs/src/host/sandbox/tests.rs b/nvx-rs/src/host/sandbox/tests.rs index 9d911d9e7..4f3d0772a 100644 --- a/nvx-rs/src/host/sandbox/tests.rs +++ b/nvx-rs/src/host/sandbox/tests.rs @@ -7,7 +7,7 @@ use std::path::PathBuf; use super::mount::{MountAccess, MountKind, MountOwner, MountPolicy, SandboxMount}; use super::*; use crate::ErrorKind; -use crate::host::filesystem::ExportPlan; +use crate::host::filesystem::PlannedExport; use crate::host::paths; mod specification; @@ -223,21 +223,22 @@ impl Fixture { path.to_str().unwrap().to_owned() } - /// Returns the export plan of `mounts`, which [`mount::plan_mounts`] checks against the host. - fn planned(&self, mounts: Vec) -> Result { + /// Returns the export plan of `mounts`, with its pins, which [`mount::plan_mounts`] checks + /// against the host. + fn planned(&self, mounts: Vec) -> Result { Ok(mount::plan_mounts(&mounts)?.expect("shares have an export plan")) } } /// Checks `export` against the host again, as a start of a provisioned sandbox does: its -/// directories and files must still not overlap, and it must pass [`ExportPlan::verify`]. -fn validated(export: &ExportPlan) -> Result<()> { +/// directories and files must still not overlap, and it must pass [`PlannedExport::verify`]. +fn validated(export: &PlannedExport) -> Result<()> { validated_with(export, &mount::HostQueries::native()) } /// Implements [`validated`] with the given host queries. -fn validated_with(export: &ExportPlan, host: &mount::HostQueries<'_>) -> Result<()> { - mount::check_distinct_export_roots(export, host)?; +fn validated_with(export: &PlannedExport, host: &mount::HostQueries<'_>) -> Result<()> { + mount::check_distinct_export_roots(&export.description, host)?; export.verify() } @@ -336,7 +337,7 @@ fn launch_validation_rejects_overlapping_or_foreign_host_paths() { ]) .unwrap(); validated(&export).unwrap(); - assert_eq!(export.children.len(), 2); + assert_eq!(export.description.children.len(), 2); // One share's policy paths must lie in it too. let error = fixture .planned(vec![parse( @@ -462,6 +463,7 @@ fn launch_validation_rejects_shares_that_came_to_overlap() { .unwrap(); validated(&export).unwrap(); let roots: Vec = export + .description .children .iter() .map(|child| child.root.clone()) @@ -518,7 +520,7 @@ fn launch_validation_requires_plain_mount_directory() { .to_str() .and_then(|text| text.strip_prefix(r"\\?\")) .map_or(canonical.clone(), PathBuf::from); - assert_eq!(export.children[0].root, canonical); + assert_eq!(export.description.children[0].root, canonical); } #[test] diff --git a/nvx-rs/src/host/vm/tests.rs b/nvx-rs/src/host/vm/tests.rs index 242b80238..cef6dbb99 100644 --- a/nvx-rs/src/host/vm/tests.rs +++ b/nvx-rs/src/host/vm/tests.rs @@ -456,7 +456,6 @@ fn capture_creation_refuses_layered_roots_and_exports_without_creating_outputs() file: false, writable: false, denied: Vec::new(), - denied_identities: Vec::new(), allowed: Vec::new(), write: Vec::new(), }], @@ -465,7 +464,6 @@ fn capture_creation_refuses_layered_roots_and_exports_without_creating_outputs() source: String::new(), target: "/mnt/shared".to_owned(), read_only: true, - identity: None, }], owner: Owner::Vmm, }), @@ -682,7 +680,6 @@ fn prepared_record_keeps_checked_root_view_export_resources_and_transaction() { file: false, writable: false, denied: Vec::new(), - denied_identities: Vec::new(), allowed: Vec::new(), write: Vec::new(), }], @@ -691,7 +688,6 @@ fn prepared_record_keeps_checked_root_view_export_resources_and_transaction() { source: String::new(), target: "/work".to_owned(), read_only: true, - identity: None, }], owner: Owner::Vmm, }); diff --git a/nvx-rs/tests/cli/lifecycle.rs b/nvx-rs/tests/cli/lifecycle.rs index c052ea250..bbd48193b 100644 --- a/nvx-rs/tests/cli/lifecycle.rs +++ b/nvx-rs/tests/cli/lifecycle.rs @@ -74,6 +74,7 @@ pub(crate) fn minimal_config() -> Value { json!({ "cmdline": "", "export": null, + "export_pins": null, "format": 10, "hostname": "nvx-sandbox", "hypervisor": HYPERVISOR, @@ -86,13 +87,31 @@ pub(crate) fn minimal_config() -> Value { }) } -/// Returns the configuration of a sandbox with shares, whose export plan is `export`. +/// Returns the configuration of a sandbox with shares, whose export plan is `export`, and whose +/// pins pin nothing, as for shares that each bind their child's root, which no workload can move, +/// and that deny no path inside a part that the guest may write. pub(crate) fn shared_config(export: Value) -> Value { let mut config = minimal_config(); + config["export_pins"] = unpinned(&export); config["export"] = export; config } +/// Returns the pins of `export` that pin nothing: a `null` for each denied path of each child, and +/// for each bind. +pub(crate) fn unpinned(export: &Value) -> Value { + let nulls = |items: &Value| vec![Value::Null; items.as_array().unwrap().len()]; + json!({ + "binds": nulls(&export["binds"]), + "denied": export["children"] + .as_array() + .unwrap() + .iter() + .map(|child| nulls(&child["denied"])) + .collect::>(), + }) +} + /// Returns the bind of an export plan that binds its child `child` at `target`. pub(crate) fn bind(child: usize, target: &str, read_only: bool) -> Value { json!({"child": child, "readOnly": read_only, "source": "", "target": target}) @@ -104,7 +123,6 @@ pub(crate) fn child(root: &str, writable: bool) -> Value { json!({ "allowed": [], "denied": [], - "deniedIdentities": [], "root": root, "writable": writable, "write": [], @@ -362,7 +380,6 @@ fn configurations() -> Vec { "children": [{ "allowed": ["{root/share/hidden/shown}"], "denied": ["{root/share/hidden}"], - "deniedIdentities": [null], "root": "{root/share}", "writable": true, "write": ["{root/share/writable}"], @@ -386,7 +403,6 @@ fn configurations() -> Vec { "children": [{ "allowed": [], "denied": [], - "deniedIdentities": [], "file": true, "root": "{root/single.txt}", "writable": false, @@ -1281,8 +1297,9 @@ fn start_without_openvmm(root: &Root) { /// A start refuses a configuration of a format that it does not read, one that is not an object, /// one that is not strict JSON, one without the fields of its format or with fields of other -/// types or ranges, one whose format is not that of its shares, and one whose upper is not as its -/// format records it, before it launches OpenVMM. +/// types or ranges, one whose format is not that of its shares, one whose upper is not as its +/// format records it, and one that holds an export without its pins or pins without an export, +/// before it launches OpenVMM. fn malformed_configuration(root: &Root) { root.nvx_rs(&provision("{root/state}", &[])) .assert_silent_success(); @@ -1290,6 +1307,19 @@ fn malformed_configuration(root: &Root) { let config = json(&root.below("state/config.json")); let mut without_export = config.clone(); without_export.as_object_mut().unwrap().remove("export"); + // It records the export's pins beside the export, which a configuration that `nvx-rs` wrote + // before it pinned exported objects there lacks, and an export without its pins, or pins + // without an export, are malformed. + let mut without_export_pins = config.clone(); + without_export_pins + .as_object_mut() + .unwrap() + .remove("export_pins"); + let mut export_without_pins = config.clone(); + export_without_pins["export"] = + json!({"binds": [bind(0, "/data", true)], "children": [child("/share", false)]}); + let mut pins_without_export = config.clone(); + pins_without_export["export_pins"] = json!({"binds": [], "denied": []}); let mut memory_text = config.clone(); memory_text["memory_mib"] = json!("256"); let mut uid_beyond_32_bits = config.clone(); @@ -1336,6 +1366,10 @@ fn malformed_configuration(root: &Root) { network_with_a_broken_rule["network"]["egress_allow"] = json!(["192.0.2.0/33"]); let unsupported = &["unsupported format", "{root/state/config.json}"][..]; let missing_export = &["sandbox configuration has no export plan"][..]; + let missing_export_pins = &[ + "sandbox configuration has no export pins", + "deprovision the sandbox and provision it again", + ][..]; let malformed = &["sandbox configuration is malformed"][..]; for (text, code, fragments) in [ ( @@ -1395,6 +1429,13 @@ fn malformed_configuration(root: &Root) { "format-unsupported", missing_export, ), + ( + without_export_pins.to_string(), + "format-unsupported", + missing_export_pins, + ), + (export_without_pins.to_string(), "io", malformed), + (pins_without_export.to_string(), "io", malformed), ( format_1_with_export.to_string(), "format-unsupported", @@ -1814,6 +1855,7 @@ fn start_refuses_recorded_shares_of_its_own_files() { shared["file"] = json!(Path::new(&host).is_file()); let mut config = json(&root.below("control/state/config.json")); config["export"] = json!({"binds": [bind(0, "/data", true)], "children": [shared]}); + config["export_pins"] = unpinned(&config["export"]); root.write("control/state/config.json", config.to_string()); let before = root.snapshot(); root.nvx_rs(&sandbox::<&str>("start", "{root/control/state}", &[])) diff --git a/nvx-rs/tests/cli/openvmm.rs b/nvx-rs/tests/cli/openvmm.rs index 9cd387fce..c38db9c97 100644 --- a/nvx-rs/tests/cli/openvmm.rs +++ b/nvx-rs/tests/cli/openvmm.rs @@ -43,16 +43,13 @@ fn mount_value(value: &str) -> (String, String, bool) { /// alone, as `nvx::host::filesystem` documents an export, independently of the CLI's planner: each /// share is a child of its own at its resolved host path, in the order of those paths, with the /// resolved policy paths inside it, sorted, and writable if its share is, whole or only below its -/// write paths; each denied path that a workload could move, strictly inside a part that the guest -/// may write, carries an identity, which `ExportPlan::check` requires there but no command line -/// carries, so a zero one serves; and each share binds its child's root at its target, the -/// shallower targets first. +/// write paths; and each share binds its child's root at its target, the shallower targets first. fn fixture_export( shares: &[(String, PathBuf, bool)], policies: &[Vec; 3], owner: nvx::host::filesystem::Owner, ) -> Option { - use nvx::host::filesystem::{Bind, ExportChild, ExportPlan, FileIdentity}; + use nvx::host::filesystem::{Bind, ExportChild, ExportPlan}; if shares.is_empty() { return None; @@ -94,28 +91,9 @@ fn fixture_export( } else { Vec::new() }; - let denied = inside(&denied, &root); - let moves = |path: &Path| { - writable - && path != root.as_path() - && (write.is_empty() - || write - .iter() - .any(|part| path != part.as_path() && path.starts_with(part))) - }; - let denied_identities = denied - .iter() - .map(|path| { - moves(path).then_some(FileIdentity { - device: 0, - index: 0, - }) - }) - .collect(); ExportChild { allowed: inside(&allowed, &root), - denied, - denied_identities, + denied: inside(&denied, &root), write, root, file, @@ -135,7 +113,6 @@ fn fixture_export( source: String::new(), target: target.clone(), read_only: *read_only, - identity: None, } }) .collect();