From a7afdf01d8ae7af4e94476e1ed18d5a772d7dbd5 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 01:52:41 -0800 Subject: [PATCH 1/4] host: open one strict snapshot directory Validate the private snapshot entries, strict OpenVMM manifest, artifact lengths and matching canonical key. Require the informational description without reading its contents, and leave later immutability to the caller. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 195a07bf-2a62-48c4-96c5-f60b233d27d9 --- nvx-rs/README.md | 18 +- nvx-rs/src/host/snapshot.rs | 183 +++++++++++++- nvx-rs/src/host/snapshot/key.rs | 12 + nvx-rs/src/host/snapshot/tests.rs | 408 +++++++++++++++++++++++++++++- nvx-rs/src/host/vm/tests.rs | 3 +- 5 files changed, 614 insertions(+), 10 deletions(-) diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 3019c6ebc..c432b148f 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -125,7 +125,7 @@ changes only when the module's purpose does. | [`nvx::exec`](#nvxexec) | The execution engine: the streams, credit, and outcome of an execution's two sides, without I/O, for any framing | | [`nvx::host::runtime`](#nvxhostruntime) | `Runtime`: what every launch shares, the runtime's files and their identity, the hypervisor and its probe, and the launch options | | [`nvx::host::vm`](#nvxhostvm) | A private VM directory, cold preparation, verified detached launch, confirmed stop, and caller-saved reattachment | -| [`nvx::host::snapshot`](#nvxhostsnapshot) | Snapshot compatibility keys, strict manifests, restore-time eligibility, and the opaque input of a future VM restore | +| [`nvx::host::snapshot`](#nvxhostsnapshot) | Strict private snapshot directories, verified keys and manifests, time eligibility, and the input of a future VM restore | | [`nvx::host::config`](#nvxhostconfig) | `VmConfig` and `CaptureConfig`: pure configuration, cold-boot kernel/OpenVMM arguments, portable length checks, and capture identity | | [`nvx::host::restore`](#nvxhostrestore) | A caller-driven restore to Ready without I/O or a clock, and its peer-verified restore-ready listener | | [`nvx::host::artifacts`](#nvxhostartifacts) | Locating the OpenVMM executable and the guest artifacts that a VM runs, and their runtime identity | @@ -579,9 +579,19 @@ The key's canonical encoding and identity use the snapshot names and omit and no reader or migration of the prior encoding is supported. `CaptureClass` still records the legacy capture inputs, including its guest build. -`Snapshot` is an opaque input of a future restore. `vm::create` currently -refuses `Some(snapshot)` with `ErrorKind::Unsupported`; only cold boots are -supported. +`Snapshot::open` checks exactly `key.bin`, `snapshot.json` and +`openvmm/{manifest.bin,memory.bin,state.bin}`. Directories are private, files +are regular and private to their owner, artifact lengths equal the strict +manifest's, and the canonical key is consistent with that manifest. +`KEY_FILE`, `DESCRIPTION_FILE` and `OPENVMM_DIR` name the root entries. +The description must exist, but its contents are never read, parsed or +trusted. `Snapshot::key` and `manifest` expose the verified metadata. + +The caller keeps the directory and every entry private and immutable while +using the snapshot; opening checks them once and retains no file handles +or lock. `Snapshot` remains an opaque input of a future restore: +`vm::create` still refuses `Some(snapshot)` with `ErrorKind::Unsupported`; +only cold boots are supported. ### `nvx::host::config` diff --git a/nvx-rs/src/host/snapshot.rs b/nvx-rs/src/host/snapshot.rs index 599a0fb53..22ff1b877 100644 --- a/nvx-rs/src/host/snapshot.rs +++ b/nvx-rs/src/host/snapshot.rs @@ -1,4 +1,8 @@ -//! Snapshot metadata and the opaque input of a future VM restore. +//! Strict snapshot directories, compatibility keys, manifests and restore-time eligibility. +//! +//! [`Snapshot::open`] checks the one snapshot format without reading its informational +//! description. The caller keeps the directory and its entries private and immutable while +//! using the opened snapshot. //! //! [`SnapshotKey`] derives a compatibility key from a capture's class and manifest. //! [`SnapshotManifest::read`] reads the manifest that OpenVMM wrote, and @@ -31,15 +35,186 @@ pub(super) mod manifest; pub(crate) mod tests; mod wire; +use std::ffi::OsString; +use std::fs; +use std::io; +use std::path::{Path, PathBuf}; + +use crate::host::paths::{absolute, normalized}; +use crate::host::store::regular; +use crate::host::{filesystem, platform}; +use crate::{Error, Result}; + pub use self::eligibility::{Downtime, DowntimeSource, Eligibility, HostTime, MAX_DOWNTIME_NS}; pub use self::key::{CaptureClass, ClassIdentity, Invariants, KeyIdentity, SnapshotKey}; pub use self::manifest::{ BlockGeometry, HostClock, MANIFEST_NAME, MAX_MANIFEST_LEN, SnapshotManifest, TimeContract, }; -/// An opaque snapshot input. Creating a VM from a snapshot is not supported yet. +/// The canonical snapshot key. +pub const KEY_FILE: &str = "key.bin"; +/// The informational description, whose contents [`Snapshot::open`] never reads. +pub const DESCRIPTION_FILE: &str = "snapshot.json"; +/// The directory of OpenVMM's manifest, RAM and saved state. +pub const OPENVMM_DIR: &str = "openvmm"; + +/// A checked snapshot directory and its verified metadata, for a future VM restore. +/// +/// The caller must keep the directory and its entries private and immutable while using this +/// value. Opening checks them once; it neither watches them nor holds their files open. #[derive(Debug)] pub struct Snapshot { - /// Reserved for snapshot metadata. - pub(crate) _private: (), + #[cfg_attr( + not(test), + expect(dead_code, reason = "SL2.15 uses the snapshot directory (#1103)") + )] + pub(crate) dir: PathBuf, + key: SnapshotKey, + manifest: SnapshotManifest, +} + +impl Snapshot { + /// Opens the one strict snapshot format without changing any entry. + /// + /// `dir` must be a private directory containing exactly [`KEY_FILE`], [`DESCRIPTION_FILE`] + /// and [`OPENVMM_DIR`]. The latter must be a private directory containing exactly + /// [`MANIFEST_NAME`], `memory.bin` and `state.bin`. Every file must be regular, not a link or + /// reparse point, and private to its owner. The manifest is read by + /// [`SnapshotManifest::read`], its artifact lengths must match, and the bounded canonical + /// key must be consistent with it. The description is required but its contents are never + /// read, parsed or trusted. + /// + /// The caller must keep the directory and its entries private and immutable while using + /// the returned snapshot. This check retains no open file handles or lock. + /// + /// Fails with [`ErrorKind::Io`](crate::ErrorKind::Io) for an unreadable or invalid directory, + /// entry, length or key relationship, and otherwise as [`SnapshotManifest::read`] and + /// [`SnapshotKey::from_bytes`] do. + pub fn open(dir: &Path) -> Result { + let dir = absolute(&normalized(dir)).map_err(|error| entry_error(dir, error))?; + check_directory(&dir, &[KEY_FILE, DESCRIPTION_FILE, OPENVMM_DIR])?; + let openvmm = dir.join(OPENVMM_DIR); + check_directory(&openvmm, &[MANIFEST_NAME, "memory.bin", "state.bin"])?; + for path in [ + dir.join(KEY_FILE), + dir.join(DESCRIPTION_FILE), + openvmm.join(MANIFEST_NAME), + ] { + check_file(&path)?; + } + let manifest = SnapshotManifest::read(&openvmm)?; + for (name, expected) in [ + ("memory.bin", manifest.memory_len()), + ("state.bin", manifest.state_len()), + ] { + let path = openvmm.join(name); + let found = check_file(&path)?.len(); + if found != expected { + return Err(Error::io(format!( + "the snapshot artifact {} is {found} bytes long, not the {expected} bytes \ + that its manifest records", + path.display() + ))); + } + } + let path = dir.join(KEY_FILE); + let bytes = regular::open_regular(&path) + .and_then(|file| { + regular::read_at_most(file, SnapshotKey::MAX_LEN as u64, "a snapshot key") + }) + .map_err(|error| entry_error(&path, error))?; + let key = SnapshotKey::from_bytes(&bytes)?; + key.check_manifest(&manifest)?; + Ok(Self { dir, key, manifest }) + } + + /// Returns the verified snapshot key. + pub fn key(&self) -> &SnapshotKey { + &self.key + } + + /// Returns the strictly parsed OpenVMM manifest. + pub fn manifest(&self) -> &SnapshotManifest { + &self.manifest + } +} + +fn entry_error(path: &Path, error: io::Error) -> Error { + Error::io(format!("cannot examine snapshot entry {}", path.display())).with_source(error) +} + +fn check_directory(path: &Path, expected: &[&str]) -> Result<()> { + if let Some(problem) = + platform::check_snapshot_dir(path).map_err(|error| entry_error(path, error))? + { + return Err(Error::io(format!( + "the snapshot directory {} {problem}", + path.display() + ))); + } + let mut found = fs::read_dir(path) + .and_then(|entries| { + entries + .map(|entry| Ok(entry?.file_name())) + .collect::>>() + }) + .map_err(|error| entry_error(path, error))?; + let mut expected: Vec = expected.iter().map(|name| OsString::from(*name)).collect(); + found.sort(); + expected.sort(); + if found != expected { + return Err(Error::io(format!( + "the snapshot directory {} holds {found:?}, not exactly {expected:?}", + path.display() + ))); + } + Ok(()) +} + +fn check_file(path: &Path) -> Result { + #[cfg(windows)] + let file = { + use std::os::windows::fs::OpenOptionsExt; + use windows_sys::Win32::Storage::FileSystem::{ + FILE_FLAG_OPEN_REPARSE_POINT, FILE_READ_ATTRIBUTES, READ_CONTROL, + }; + + fs::OpenOptions::new() + .access_mode(READ_CONTROL | FILE_READ_ATTRIBUTES) + .custom_flags(FILE_FLAG_OPEN_REPARSE_POINT) + .open(path) + .map_err(|error| entry_error(path, error))? + }; + #[cfg(windows)] + let metadata = file.metadata().map_err(|error| entry_error(path, error))?; + #[cfg(not(windows))] + let metadata = fs::symlink_metadata(path).map_err(|error| entry_error(path, error))?; + if !metadata.is_file() || filesystem::is_reparse_point(&metadata) { + return Err(Error::io(format!( + "the snapshot entry {} is not a regular file, or is a link or reparse point", + path.display() + ))); + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + + if metadata.uid() != nix::unistd::Uid::effective().as_raw() || metadata.mode() & 0o077 != 0 + { + return Err(Error::io(format!( + "the snapshot file {} is not private to the current user", + path.display() + ))); + } + } + #[cfg(windows)] + if let Some(problem) = + platform::private_file_problem(&file).map_err(|error| entry_error(path, error))? + { + return Err(Error::io(format!( + "the snapshot file {} {problem}", + path.display() + ))); + } + Ok(metadata) } diff --git a/nvx-rs/src/host/snapshot/key.rs b/nvx-rs/src/host/snapshot/key.rs index 9f0c77169..5fdddc8f8 100644 --- a/nvx-rs/src/host/snapshot/key.rs +++ b/nvx-rs/src/host/snapshot/key.rs @@ -634,6 +634,18 @@ impl SnapshotKey { self.contracts == Contracts::CURRENT } + pub(super) fn check_manifest(&self, manifest: &SnapshotManifest) -> Result<()> { + if self.machine != manifest.machine + || !self.speaks_current_contracts() + || self.filesystem_tag != MOUNT_TAG + { + return Err(Error::io( + "the snapshot key is not consistent with its manifest and this format's contracts", + )); + } + Ok(()) + } + /// Returns the backend. pub fn hypervisor(&self) -> Hypervisor { self.machine.hypervisor diff --git a/nvx-rs/src/host/snapshot/tests.rs b/nvx-rs/src/host/snapshot/tests.rs index 3772e3ac5..c73fe2710 100644 --- a/nvx-rs/src/host/snapshot/tests.rs +++ b/nvx-rs/src/host/snapshot/tests.rs @@ -4,9 +4,16 @@ //! that the tests check, and [`ManifestSpec`] holds each field of a manifest, so that a test //! changes one field and encodes the others as they were. +use std::fs; +use std::path::{Path, PathBuf}; + use sha2::{Digest, Sha256}; -use super::{BlockGeometry, CaptureClass, Invariants}; +use super::{ + BlockGeometry, CaptureClass, DESCRIPTION_FILE, Invariants, KEY_FILE, MANIFEST_NAME, + OPENVMM_DIR, Snapshot, SnapshotKey, SnapshotManifest, +}; +use crate::ErrorKind; use crate::host::network::{Access, NetworkPolicy}; use crate::host::sandbox::SandboxNetwork; use crate::host::store::{ArtifactIdentity, RuntimeIdentity}; @@ -986,3 +993,402 @@ pub(in crate::host) fn network_class() -> CaptureClass { ..class() } } + +pub(in crate::host) struct SnapshotFixture { + root: tempfile::TempDir, + pub(in crate::host) dir: PathBuf, + spec: ManifestSpec, + key: SnapshotKey, +} + +impl SnapshotFixture { + pub(in crate::host) fn new() -> Self { + use crate::host::platform; + use crate::host::store::files::{create_new, write_private}; + + let root = tempfile::tempdir().unwrap(); + let dir = root.path().join("snapshot"); + assert_eq!(platform::create_owner_only_dir(&dir).unwrap(), None); + let openvmm = dir.join(OPENVMM_DIR); + assert_eq!(platform::create_owner_only_dir(&openvmm).unwrap(), None); + let spec = ManifestSpec::template(); + let manifest = SnapshotManifest::parse(&spec.encode()).unwrap(); + let key = SnapshotKey::new(&class(), &manifest).unwrap(); + write_private(&dir.join(KEY_FILE), &key.to_bytes()).unwrap(); + write_private(&dir.join(DESCRIPTION_FILE), b"not JSON").unwrap(); + write_private(&openvmm.join(MANIFEST_NAME), &spec.encode()).unwrap(); + for (name, length) in [ + ("memory.bin", spec.memory_size), + ("state.bin", spec.state_size), + ] { + let file = create_new(&openvmm.join(name)).unwrap(); + platform::make_sparse(&file).unwrap(); + file.set_len(length).unwrap(); + } + Self { + root, + dir, + spec, + key, + } + } + + fn files(&self) -> [PathBuf; 5] { + let openvmm = self.dir.join(OPENVMM_DIR); + [ + self.dir.join(KEY_FILE), + self.dir.join(DESCRIPTION_FILE), + openvmm.join(MANIFEST_NAME), + openvmm.join("memory.bin"), + openvmm.join("state.bin"), + ] + } +} + +fn refused(path: &Path) { + let error = Snapshot::open(path).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io, "{error}"); +} + +#[test] +fn snapshot_open_checks_the_complete_directory_and_preserves_its_metadata() { + let fixture = SnapshotFixture::new(); + let snapshot = Snapshot::open(&fixture.dir).unwrap(); + assert_eq!(snapshot.dir, fixture.dir); + assert_eq!(snapshot.key(), &fixture.key); + assert_eq!( + snapshot.manifest(), + &SnapshotManifest::parse(&fixture.spec.encode()).unwrap() + ); + for suffix in ["", std::path::MAIN_SEPARATOR_STR, "."] { + let path = if suffix == "." { + fixture.dir.join(".") + } else { + let mut path = fixture.dir.as_os_str().to_os_string(); + path.push(suffix); + PathBuf::from(path) + }; + assert_eq!(Snapshot::open(&path).unwrap().key(), &fixture.key); + } +} + +#[test] +fn snapshot_open_refuses_every_missing_entry() { + for index in 0..6 { + let fixture = SnapshotFixture::new(); + let missing = if index == 5 { + fixture.dir.join(OPENVMM_DIR) + } else { + fixture.files()[index].clone() + }; + fs::rename(&missing, fixture.root.path().join("missing")).unwrap(); + refused(&fixture.dir); + } + let fixture = SnapshotFixture::new(); + refused(&fixture.root.path().join("absent")); +} + +#[test] +fn snapshot_open_refuses_extra_entries_and_alternate_names() { + use crate::host::store::files::write_private; + + for nested in [false, true] { + let fixture = SnapshotFixture::new(); + let dir = if nested { + fixture.dir.join(OPENVMM_DIR) + } else { + fixture.dir.clone() + }; + write_private(&dir.join("extra"), b"extra").unwrap(); + refused(&fixture.dir); + } + let fixture = SnapshotFixture::new(); + fs::rename(fixture.dir.join(KEY_FILE), fixture.dir.join("KEY.BIN")).unwrap(); + refused(&fixture.dir); +} + +#[test] +fn snapshot_open_refuses_every_directory_in_place_of_a_regular_file() { + for index in 0..5 { + let fixture = SnapshotFixture::new(); + let path = &fixture.files()[index]; + fs::remove_file(path).unwrap(); + fs::create_dir(path).unwrap(); + refused(&fixture.dir); + } +} + +#[test] +fn snapshot_open_refuses_files_in_place_of_its_directories() { + use crate::host::store::files::write_private; + + let fixture = SnapshotFixture::new(); + let file = fixture.root.path().join("file"); + write_private(&file, b"file").unwrap(); + refused(&file); + fs::rename( + fixture.dir.join(OPENVMM_DIR), + fixture.root.path().join("saved"), + ) + .unwrap(); + write_private(&fixture.dir.join(OPENVMM_DIR), b"file").unwrap(); + refused(&fixture.dir); +} + +#[test] +fn snapshot_open_refuses_both_artifact_length_mismatches() { + for name in ["memory.bin", "state.bin"] { + for difference in [-1i64, 1] { + let fixture = SnapshotFixture::new(); + let path = fixture.dir.join(OPENVMM_DIR).join(name); + let file = fs::OpenOptions::new().write(true).open(path).unwrap(); + let length = file.metadata().unwrap().len(); + file.set_len(length.checked_add_signed(difference).unwrap()) + .unwrap(); + refused(&fixture.dir); + } + } +} + +#[test] +fn snapshot_open_uses_the_strict_manifest_and_bounded_key_decoders() { + use crate::host::store::files::write_private; + + for bytes in [vec![0x80], vec![0; super::MAX_MANIFEST_LEN as usize + 1]] { + let fixture = SnapshotFixture::new(); + write_private(&fixture.dir.join(OPENVMM_DIR).join(MANIFEST_NAME), &bytes).unwrap(); + assert!(Snapshot::open(&fixture.dir).is_err()); + } + let fixture = SnapshotFixture::new(); + let mut trailing = fixture.key.to_bytes(); + trailing.push(0); + let mut version = fixture.key.to_bytes(); + version[0] += 1; + for bytes in [ + vec![0], + trailing, + version, + vec![0; SnapshotKey::MAX_LEN + 1], + ] { + write_private(&fixture.dir.join(KEY_FILE), &bytes).unwrap(); + assert!(Snapshot::open(&fixture.dir).is_err()); + } +} + +#[test] +fn snapshot_open_refuses_a_key_from_another_manifest_machine() { + use crate::host::store::files::write_private; + + let mutations: [fn(&mut ManifestSpec); 4] = [ + |spec| spec.cpu().digest[0] ^= 1, + |spec| spec.contract().command_line.push_str(" extra"), + |spec| spec.contract().devices[0].state_unit.push_str("-other"), + |spec| spec.time().tsc_hz += 1, + ]; + for mutate in mutations { + let fixture = SnapshotFixture::new(); + let mut spec = fixture.spec.clone(); + mutate(&mut spec); + let manifest = SnapshotManifest::parse(&spec.encode()).unwrap(); + let key = SnapshotKey::new(&class(), &manifest).unwrap(); + write_private(&fixture.dir.join(KEY_FILE), &key.to_bytes()).unwrap(); + refused(&fixture.dir); + } +} + +#[test] +fn snapshot_description_contents_do_not_affect_opening() { + use crate::host::store::files::write_private; + + let fixture = SnapshotFixture::new(); + for contents in [b"".as_slice(), b"{broken JSON", &[0xff, 0, 0x80]] { + write_private(&fixture.dir.join(DESCRIPTION_FILE), contents).unwrap(); + assert_eq!(Snapshot::open(&fixture.dir).unwrap().key(), &fixture.key); + } +} + +#[cfg(target_os = "linux")] +#[test] +fn snapshot_open_refuses_every_shared_file_and_both_shared_directories() { + use std::os::unix::fs::PermissionsExt; + + for index in 0..5 { + for mode in [0o640, 0o602, 0o610] { + let fixture = SnapshotFixture::new(); + let path = &fixture.files()[index]; + fs::set_permissions(path, fs::Permissions::from_mode(mode)).unwrap(); + refused(&fixture.dir); + assert_eq!( + fs::metadata(path).unwrap().permissions().mode() & 0o777, + mode + ); + } + } + for nested in [false, true] { + let fixture = SnapshotFixture::new(); + let path = if nested { + fixture.dir.join(OPENVMM_DIR) + } else { + fixture.dir.clone() + }; + fs::set_permissions(&path, fs::Permissions::from_mode(0o750)).unwrap(); + refused(&fixture.dir); + assert_eq!( + fs::metadata(path).unwrap().permissions().mode() & 0o777, + 0o750 + ); + } +} + +#[cfg(target_os = "linux")] +#[test] +#[ignore = "requires root to change only the owned snapshot fixture's entry owners"] +fn snapshot_open_refuses_entries_owned_by_another_user() { + use nix::unistd::{Uid, chown}; + + assert!(Uid::effective().is_root()); + for index in 0..7 { + let fixture = SnapshotFixture::new(); + let path = match index { + 5 => fixture.dir.clone(), + 6 => fixture.dir.join(OPENVMM_DIR), + _ => fixture.files()[index].clone(), + }; + chown(&path, Some(Uid::from_raw(1)), None).unwrap(); + refused(&fixture.dir); + } +} + +#[cfg(target_os = "linux")] +#[test] +fn snapshot_open_refuses_file_links_fifos_and_sockets_without_reading_them() { + use std::os::unix::fs::symlink; + use std::os::unix::net::UnixListener; + + for index in 0..5 { + let fixture = SnapshotFixture::new(); + let path = &fixture.files()[index]; + let saved = fixture.root.path().join("saved"); + fs::rename(path, &saved).unwrap(); + symlink(&saved, path).unwrap(); + refused(&fixture.dir); + fs::remove_file(path).unwrap(); + nix::unistd::mkfifo(path, nix::sys::stat::Mode::S_IRUSR).unwrap(); + refused(&fixture.dir); + fs::remove_file(path).unwrap(); + let _socket = UnixListener::bind(path).unwrap(); + refused(&fixture.dir); + } +} + +#[cfg(target_os = "linux")] +#[test] +fn snapshot_open_refuses_directory_links_even_with_trailing_separators() { + use std::os::unix::fs::symlink; + + let fixture = SnapshotFixture::new(); + let link = fixture.root.path().join("link"); + symlink(&fixture.dir, &link).unwrap(); + for suffix in ["", "/", "/.", "/./"] { + let mut path = link.as_os_str().to_os_string(); + path.push(suffix); + refused(Path::new(&path)); + } + let saved = fixture.root.path().join("saved"); + let openvmm = fixture.dir.join(OPENVMM_DIR); + fs::rename(&openvmm, &saved).unwrap(); + symlink(&saved, &openvmm).unwrap(); + refused(&fixture.dir); +} + +#[cfg(target_os = "linux")] +#[test] +fn snapshot_description_needs_no_read_permission() { + use std::os::unix::fs::PermissionsExt; + + let fixture = SnapshotFixture::new(); + let path = fixture.dir.join(DESCRIPTION_FILE); + fs::set_permissions(&path, fs::Permissions::from_mode(0o200)).unwrap(); + if !nix::unistd::Uid::effective().is_root() { + assert_eq!( + fs::read(&path).unwrap_err().kind(), + std::io::ErrorKind::PermissionDenied + ); + } + assert_eq!(Snapshot::open(&fixture.dir).unwrap().key(), &fixture.key); + assert_eq!( + fs::metadata(path).unwrap().permissions().mode() & 0o777, + 0o200 + ); +} + +#[cfg(windows)] +#[test] +fn snapshot_open_refuses_every_shared_file_and_both_shared_directory_acls() { + use crate::host::platform::tests::{access, set_access, user_sid}; + + let sid = user_sid(); + let shared = format!("D:P(A;;FA;;;{sid})(A;;FR;;;WD)"); + for index in 0..5 { + let fixture = SnapshotFixture::new(); + let path = &fixture.files()[index]; + set_access(path, &shared).unwrap(); + let before = access(path).unwrap(); + refused(&fixture.dir); + assert_eq!(access(path).unwrap(), before); + } + for nested in [false, true] { + let fixture = SnapshotFixture::new(); + let path = if nested { + fixture.dir.join(OPENVMM_DIR) + } else { + fixture.dir.clone() + }; + set_access(&path, &shared).unwrap(); + let before = access(&path).unwrap(); + refused(&fixture.dir); + assert_eq!(access(&path).unwrap(), before); + } +} + +#[cfg(windows)] +#[test] +fn snapshot_open_refuses_directory_junctions_and_every_reparse_file_entry() { + use crate::host::tests::junction; + + let fixture = SnapshotFixture::new(); + let link = fixture.root.path().join("link"); + junction(&link, &fixture.dir); + for path in [link.clone(), link.join("."), link.join("")] { + refused(&path); + } + let saved = fixture.root.path().join("saved"); + let openvmm = fixture.dir.join(OPENVMM_DIR); + fs::rename(&openvmm, &saved).unwrap(); + junction(&openvmm, &saved); + refused(&fixture.dir); + for index in 0..5 { + let fixture = SnapshotFixture::new(); + let path = &fixture.files()[index]; + fs::remove_file(path).unwrap(); + junction(path, fixture.root.path()); + refused(&fixture.dir); + fs::remove_dir(path).unwrap(); + } +} + +#[cfg(windows)] +#[test] +fn snapshot_description_needs_no_data_read_access() { + use crate::host::platform::tests::{set_access, user_sid}; + + let fixture = SnapshotFixture::new(); + let path = fixture.dir.join(DESCRIPTION_FILE); + let sid = user_sid(); + set_access(&path, &format!("D:P(D;;0x1;;;{sid})(A;;FA;;;{sid})")).unwrap(); + assert_eq!( + fs::read(&path).unwrap_err().kind(), + std::io::ErrorKind::PermissionDenied + ); + assert_eq!(Snapshot::open(&fixture.dir).unwrap().key(), &fixture.key); +} diff --git a/nvx-rs/src/host/vm/tests.rs b/nvx-rs/src/host/vm/tests.rs index b637a7532..be5e3c799 100644 --- a/nvx-rs/src/host/vm/tests.rs +++ b/nvx-rs/src/host/vm/tests.rs @@ -494,7 +494,8 @@ fn a_missing_boot_and_a_snapshot_are_refused_before_any_output_is_created() { let runtime = cold_runtime(); let mut config = cold_config(); config.boot = None; - let snapshot = Snapshot { _private: () }; + let fixture = crate::host::snapshot::tests::SnapshotFixture::new(); + let snapshot = Snapshot::open(&fixture.dir).unwrap(); for (from, config, expected) in [ (None, &config, "Boot"), (Some(&snapshot), &cold_config(), "Snapshot"), From f54c0db4cb87747499d38ce78bf91969dbfd46ea Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 02:00:38 -0800 Subject: [PATCH 2/4] tests: register the snapshot directory metadata hook Keep the future restore directory field in production and record its test-dependent dead-code expectation in the existing layout allowlist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 195a07bf-2a62-48c4-96c5-f60b233d27d9 --- scripts/test-layout-allowlist.json | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/test-layout-allowlist.json b/scripts/test-layout-allowlist.json index 5b7e6aa10..adf7c96c6 100644 --- a/scripts/test-layout-allowlist.json +++ b/scripts/test-layout-allowlist.json @@ -17,6 +17,7 @@ "nvx-rs/src/host/restore.rs": 3, "nvx-rs/src/host/sandbox/plan.rs": 5, "nvx-rs/src/host/session.rs": 1, + "nvx-rs/src/host/snapshot.rs": 1, "nvx-rs/src/host/vm.rs": 5, "nvx-rs/src/protocol/app.rs": 6 } From 7c6067a983d8462d65b46b36689329cf1271f419 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 02:03:36 -0800 Subject: [PATCH 3/4] doc: record the strict snapshot value API Regenerate the Linux and Windows public API snapshots for the three filename constants and the checked snapshot methods. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 195a07bf-2a62-48c4-96c5-f60b233d27d9 --- nvx-rs/public-api/linux.txt | 6 ++++++ nvx-rs/public-api/windows.txt | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index 95fa34dc1..8201b7dd2 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -3004,6 +3004,7 @@ method nvx::host::snapshot::CaptureClass::to_bytes struct nvx::host::snapshot::ClassIdentity method nvx::host::snapshot::ClassIdentity::as_bytes method nvx::host::snapshot::ClassIdentity::from_bytes +const nvx::host::snapshot::DESCRIPTION_FILE struct nvx::host::snapshot::Downtime field nvx::host::snapshot::Downtime::nanos field nvx::host::snapshot::Downtime::source @@ -3041,13 +3042,18 @@ method nvx::host::snapshot::Invariants::iter method nvx::host::snapshot::Invariants::len method nvx::host::snapshot::Invariants::new method nvx::host::snapshot::Invariants::with +const nvx::host::snapshot::KEY_FILE struct nvx::host::snapshot::KeyIdentity method nvx::host::snapshot::KeyIdentity::as_bytes method nvx::host::snapshot::KeyIdentity::from_bytes const nvx::host::snapshot::MANIFEST_NAME const nvx::host::snapshot::MAX_DOWNTIME_NS const nvx::host::snapshot::MAX_MANIFEST_LEN +const nvx::host::snapshot::OPENVMM_DIR struct nvx::host::snapshot::Snapshot +method nvx::host::snapshot::Snapshot::key +method nvx::host::snapshot::Snapshot::manifest +method nvx::host::snapshot::Snapshot::open struct nvx::host::snapshot::SnapshotKey const nvx::host::snapshot::SnapshotKey::MAX_LEN const nvx::host::snapshot::SnapshotKey::VERSION diff --git a/nvx-rs/public-api/windows.txt b/nvx-rs/public-api/windows.txt index 7faf4dff9..9bce65bf2 100644 --- a/nvx-rs/public-api/windows.txt +++ b/nvx-rs/public-api/windows.txt @@ -1590,6 +1590,7 @@ method nvx::host::snapshot::CaptureClass::to_bytes struct nvx::host::snapshot::ClassIdentity method nvx::host::snapshot::ClassIdentity::as_bytes method nvx::host::snapshot::ClassIdentity::from_bytes +const nvx::host::snapshot::DESCRIPTION_FILE struct nvx::host::snapshot::Downtime field nvx::host::snapshot::Downtime::nanos field nvx::host::snapshot::Downtime::source @@ -1627,13 +1628,18 @@ method nvx::host::snapshot::Invariants::iter method nvx::host::snapshot::Invariants::len method nvx::host::snapshot::Invariants::new method nvx::host::snapshot::Invariants::with +const nvx::host::snapshot::KEY_FILE struct nvx::host::snapshot::KeyIdentity method nvx::host::snapshot::KeyIdentity::as_bytes method nvx::host::snapshot::KeyIdentity::from_bytes const nvx::host::snapshot::MANIFEST_NAME const nvx::host::snapshot::MAX_DOWNTIME_NS const nvx::host::snapshot::MAX_MANIFEST_LEN +const nvx::host::snapshot::OPENVMM_DIR struct nvx::host::snapshot::Snapshot +method nvx::host::snapshot::Snapshot::key +method nvx::host::snapshot::Snapshot::manifest +method nvx::host::snapshot::Snapshot::open struct nvx::host::snapshot::SnapshotKey const nvx::host::snapshot::SnapshotKey::MAX_LEN const nvx::host::snapshot::SnapshotKey::VERSION From e5096eb6f6de3d49b8fcba1773295266dec9079f Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 7 Nov 2026 02:42:11 -0800 Subject: [PATCH 4/4] host: document snapshot checks and cover key consistency guards Document the private snapshot metadata checks and independently refuse valid keys with another contract version or filesystem tag through the public directory reader. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 195a07bf-2a62-48c4-96c5-f60b233d27d9 --- nvx-rs/src/host/snapshot.rs | 6 ++++++ nvx-rs/src/host/snapshot/key.rs | 1 + nvx-rs/src/host/snapshot/key/tests.rs | 23 +++++++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/nvx-rs/src/host/snapshot.rs b/nvx-rs/src/host/snapshot.rs index 22ff1b877..7fcb88f6e 100644 --- a/nvx-rs/src/host/snapshot.rs +++ b/nvx-rs/src/host/snapshot.rs @@ -64,12 +64,15 @@ pub const OPENVMM_DIR: &str = "openvmm"; /// value. Opening checks them once; it neither watches them nor holds their files open. #[derive(Debug)] pub struct Snapshot { + /// The absolute directory path, with its components preserved rather than its links resolved. #[cfg_attr( not(test), expect(dead_code, reason = "SL2.15 uses the snapshot directory (#1103)") )] pub(crate) dir: PathBuf, + /// The bounded canonical key checked against the manifest. key: SnapshotKey, + /// The strictly parsed metadata of OpenVMM's snapshot. manifest: SnapshotManifest, } @@ -139,10 +142,12 @@ impl Snapshot { } } +/// Names the failing snapshot entry while retaining its I/O error. fn entry_error(path: &Path, error: io::Error) -> Error { Error::io(format!("cannot examine snapshot entry {}", path.display())).with_source(error) } +/// Checks one private directory and its exact fixed entry-name inventory. fn check_directory(path: &Path, expected: &[&str]) -> Result<()> { if let Some(problem) = platform::check_snapshot_dir(path).map_err(|error| entry_error(path, error))? @@ -171,6 +176,7 @@ fn check_directory(path: &Path, expected: &[&str]) -> Result<()> { Ok(()) } +/// Checks a regular private file from metadata alone, without reading its contents. fn check_file(path: &Path) -> Result { #[cfg(windows)] let file = { diff --git a/nvx-rs/src/host/snapshot/key.rs b/nvx-rs/src/host/snapshot/key.rs index 5fdddc8f8..8ab84cc1d 100644 --- a/nvx-rs/src/host/snapshot/key.rs +++ b/nvx-rs/src/host/snapshot/key.rs @@ -634,6 +634,7 @@ impl SnapshotKey { self.contracts == Contracts::CURRENT } + /// Checks the manifest's complete machine, the current key contracts and the filesystem tag. pub(super) fn check_manifest(&self, manifest: &SnapshotManifest) -> Result<()> { if self.machine != manifest.machine || !self.speaks_current_contracts() diff --git a/nvx-rs/src/host/snapshot/key/tests.rs b/nvx-rs/src/host/snapshot/key/tests.rs index c7fce6252..0ee47de27 100644 --- a/nvx-rs/src/host/snapshot/key/tests.rs +++ b/nvx-rs/src/host/snapshot/key/tests.rs @@ -709,6 +709,29 @@ fn key_refusal(bytes: &[u8], code: ErrorCode) -> String { error.message().to_owned() } +#[cfg(any(target_os = "linux", windows))] +#[test] +fn snapshot_open_refuses_other_key_contracts_and_filesystem_tags() { + use crate::host::snapshot::{KEY_FILE, Snapshot}; + use crate::host::store::files::write_private; + + let changes: [fn(&mut SnapshotKey); 2] = [ + |key| key.contracts.snapshot += 1, + |key| key.filesystem_tag.push_str("-other"), + ]; + for change in changes { + let fixture = support::SnapshotFixture::new(); + let mut key = Snapshot::open(&fixture.dir).unwrap().key().clone(); + change(&mut key); + let bytes = key.to_bytes(); + assert!(SnapshotKey::from_bytes(&bytes).is_ok()); + write_private(&fixture.dir.join(KEY_FILE), &bytes).unwrap(); + let error = Snapshot::open(&fixture.dir).unwrap_err(); + assert_eq!(error.code(), ErrorCode::Io, "{error}"); + assert!(error.message().contains("not consistent"), "{error}"); + } +} + #[test] fn other_encodings_of_a_key_are_refused() { let key = network_key();