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();