diff --git a/doc/usage.md b/doc/usage.md index a0e64ec5d..12eb06dd9 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -110,6 +110,7 @@ each code for callers of the `nvx` library. | `template-ineligible` | 1 | The restore window of a template's generation, which the time contract of its capture sets, excludes the host's time, so no clone may restore it now. | | `template-tombstoned` | 1 | A tombstone quarantines the template's generation, which takes no new clones. | | `exited` | 1 | OpenVMM exited, as when a start fails, whose message names the category and status of OpenVMM's outcome report when it has one. | +| `still-running` | 1 | A stop could not confirm the VM process's exit, so the caller keeps its verified process. | | `exec-refused` | 1 | The guest agent refused the workload, as for a working directory that it cannot use. | | `endpoint-closed` | 1 | OpenVMM closed the control endpoint, as it does when the VM shuts down. | | `protocol` | 1 | The control session failed otherwise, as when the guest breaks the protocol or the session is reset. | diff --git a/nvx-rs/README.md b/nvx-rs/README.md index cd9c6f7ca..c432b148f 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -124,8 +124,8 @@ changes only when the module's purpose does. | [`nvx::Hypervisor`](#nvxhypervisor) | KVM, MSHV, or WHP, with its serde form behind the `serde` feature | | [`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, 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::vm`](#nvxhostvm) | A private VM directory, cold preparation, verified detached launch, confirmed stop, and caller-saved reattachment | +| [`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 | @@ -500,6 +500,17 @@ or reading runtime files. `Reattached` holds both the process and authenticated connection. Failures retain their original kinds and sources and leave the saved values unchanged. +`stop(process, outputs, graceful, grace, deadline)` waits through the verified +process, first after the caller's graceful request, then after forced termination +if needed. It returns `Stopped` only after confirmed exit. `self_exited` records +confirmation before a forced attempt, and `forced` records that attempt, not +proof from kill success. An unconfirmed exit is `StillRunning`, and the caller +keeps the process. The callback receives the bounded grace deadline and must +respect it; its boolean result is never exit proof. + +`Stopped::report` retains the outcome-report read's result separately, so report +failure never hides confirmed exit. + `vm::create(&runtime, &config, None, &dir)` creates an inspectable cold `Prepared` launch without starting a process or binding a listener. It requires `config.boot`, refuses a snapshot input, rechecks the directory's privacy, and @@ -568,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` @@ -976,6 +997,10 @@ On Windows, the duplicate retains the original handle's access rights. Duplication failures have kind `Io` and retain their original I/O source. No watcher thread, future or callback is created by the library. +`Process::wait(deadline)` returns `Some(Exited)` only after verified exit, or +`None` when the deadline passes without confirmation. `Exited` cannot be made +by a caller. + `Process::bytes_read` reports how many bytes the process has read through the operating system's read calls, whether the host's cache served them or not: `rchar` of `/proc//io` on Linux, which it reads only while the process @@ -1422,6 +1447,11 @@ The authenticated control session, which carries NVX's control protocol, `NVXC`, one frame per DATA record of a `HostConnection`; Linux and Windows primitives; and OpenVMM's outcome report. +`report::read` returns `Ok(None)` for a missing report, and an `Io` error for an +unreadable or malformed report, retaining the underlying error. It keeps today's +summary of the outcome category, status code and teardown steps, not free-form +fields. + A `Transport` to an OpenVMM control endpoint reads and writes with deadlines, and tells, through `Transport::unread`, what the endpoint has not read yet of what it wrote, where the platform tells: on Linux, what `SIOCOUTQ` reports of @@ -3055,6 +3085,7 @@ optional cause. | `Unavailable` | A missing or unusable artifact, hypervisor, or release, a guest image without a required control feature, or an unsupported host | | `Unsupported` | A request that NVX refuses: a value that it does not accept, such as a malformed or out-of-range one, options that it cannot combine, or a request that it cannot represent or enforce, such as a policy beyond a limit of OpenVMM or the guest agent | | `Exited` | The VM's OpenVMM process exited | +| `StillRunning` | A stop could not confirm the VM process's exit, so the caller keeps its verified process | | `Io` | A host I/O failure, or state that NVX recorded on the host and cannot read | | `NotFound` | State that an operation names and that does not exist on the host, such as the store of a sandbox that was never created or that a removal deleted | | `Protocol` | A control session that failed, or a request that the guest agent refused | @@ -3087,6 +3118,7 @@ usage errors, which no library error has. | `unavailable` | `Unavailable` | A dependency is missing or unusable, other than as a finer code says: install metadata, the hypervisor, a release, a distro layer or scratch image that the request names, or a host that NVX does not support | | `unsupported` | `Unsupported` | NVX refuses the request: options that it cannot combine, a value that it does not accept, or a policy that it cannot represent or enforce. The message names the option or field | | `exited` | `Exited` | OpenVMM exited, as when a start fails, whose message names the category and status of OpenVMM's outcome report when it has one | +| `still-running` | `StillRunning` | A stop could not confirm the VM process's exit, so the caller keeps its verified process | | `io` | `Io` | A host I/O operation failed, or state that NVX recorded on the host is unreadable or malformed | | `not-found` | `NotFound` | Other state that the operation names does not exist on the host | | `protocol` | `Protocol` | The control session failed otherwise: the peer violated the protocol, reset the session, or failed authentication | diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index 6a422c570..f134e992e 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -58,6 +58,7 @@ variant nvx::ErrorCode::SandboxNotRunning variant nvx::ErrorCode::SandboxProvisioned variant nvx::ErrorCode::SandboxRunning variant nvx::ErrorCode::ScratchClaimed +variant nvx::ErrorCode::StillRunning variant nvx::ErrorCode::TemplateIneligible variant nvx::ErrorCode::TemplateTombstoned variant nvx::ErrorCode::TimedOut @@ -73,6 +74,7 @@ variant nvx::ErrorKind::NotFound variant nvx::ErrorKind::Other variant nvx::ErrorKind::Protocol variant nvx::ErrorKind::RuntimeMismatch +variant nvx::ErrorKind::StillRunning variant nvx::ErrorKind::TimedOut variant nvx::ErrorKind::Unavailable variant nvx::ErrorKind::Unsupported @@ -2445,6 +2447,7 @@ method nvx::host::process::BootId::as_bytes method nvx::host::process::BootId::current method nvx::host::process::BootId::from_bytes type nvx::host::process::ExitHandle +struct nvx::host::process::Exited struct nvx::host::process::Process method nvx::host::process::Process::bytes_read method nvx::host::process::Process::exit_handle @@ -3032,6 +3035,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 @@ -3069,13 +3073,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 @@ -3768,6 +3777,10 @@ field nvx::host::vm::SavedVm::endpoint field nvx::host::vm::SavedVm::process field nvx::host::vm::SavedVm::runtime field nvx::host::vm::SavedVm::vm_dir +struct nvx::host::vm::Stopped +field nvx::host::vm::Stopped::forced +field nvx::host::vm::Stopped::report +field nvx::host::vm::Stopped::self_exited struct nvx::host::vm::VmDir method nvx::host::vm::VmDir::open method nvx::host::vm::VmDir::path @@ -3780,6 +3793,7 @@ fn nvx::host::vm::create fn nvx::host::vm::create_private_dir fn nvx::host::vm::reattach fn nvx::host::vm::spawn +fn nvx::host::vm::stop fn nvx::host::vm::verify mod nvx::net struct nvx::net::Cidr diff --git a/nvx-rs/public-api/windows.txt b/nvx-rs/public-api/windows.txt index 9aa7bf0f8..375201d56 100644 --- a/nvx-rs/public-api/windows.txt +++ b/nvx-rs/public-api/windows.txt @@ -58,6 +58,7 @@ variant nvx::ErrorCode::SandboxNotRunning variant nvx::ErrorCode::SandboxProvisioned variant nvx::ErrorCode::SandboxRunning variant nvx::ErrorCode::ScratchClaimed +variant nvx::ErrorCode::StillRunning variant nvx::ErrorCode::TemplateIneligible variant nvx::ErrorCode::TemplateTombstoned variant nvx::ErrorCode::TimedOut @@ -73,6 +74,7 @@ variant nvx::ErrorKind::NotFound variant nvx::ErrorKind::Other variant nvx::ErrorKind::Protocol variant nvx::ErrorKind::RuntimeMismatch +variant nvx::ErrorKind::StillRunning variant nvx::ErrorKind::TimedOut variant nvx::ErrorKind::Unavailable variant nvx::ErrorKind::Unsupported @@ -1031,6 +1033,7 @@ method nvx::host::process::BootId::as_bytes method nvx::host::process::BootId::current method nvx::host::process::BootId::from_bytes type nvx::host::process::ExitHandle +struct nvx::host::process::Exited struct nvx::host::process::Process method nvx::host::process::Process::bytes_read method nvx::host::process::Process::exit_handle @@ -1618,6 +1621,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 @@ -1655,13 +1659,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 @@ -2353,6 +2362,10 @@ field nvx::host::vm::SavedVm::endpoint field nvx::host::vm::SavedVm::process field nvx::host::vm::SavedVm::runtime field nvx::host::vm::SavedVm::vm_dir +struct nvx::host::vm::Stopped +field nvx::host::vm::Stopped::forced +field nvx::host::vm::Stopped::report +field nvx::host::vm::Stopped::self_exited struct nvx::host::vm::VmDir method nvx::host::vm::VmDir::open method nvx::host::vm::VmDir::path @@ -2365,6 +2378,7 @@ fn nvx::host::vm::create fn nvx::host::vm::create_private_dir fn nvx::host::vm::reattach fn nvx::host::vm::spawn +fn nvx::host::vm::stop fn nvx::host::vm::verify mod nvx::net struct nvx::net::Cidr diff --git a/nvx-rs/src/error.rs b/nvx-rs/src/error.rs index 989538bab..e98ae8830 100644 --- a/nvx-rs/src/error.rs +++ b/nvx-rs/src/error.rs @@ -19,6 +19,8 @@ pub enum ErrorKind { Unsupported, /// The OpenVMM process of a VM exited. Exited, + /// The exit of a VM's process could not be confirmed; it may still run. + StillRunning, /// A host I/O operation failed, or state that NVX recorded on the host is unreadable or /// malformed. Io, @@ -44,6 +46,7 @@ impl ErrorKind { Self::Unavailable => "unavailable", Self::Unsupported => "unsupported", Self::Exited => "exited", + Self::StillRunning => "still-running", Self::Io => "io", Self::NotFound => "not-found", Self::Protocol => "protocol", @@ -88,6 +91,8 @@ pub enum ErrorCode { Unsupported, /// `exited`: the OpenVMM process of a VM exited, as when a start fails. Exited, + /// `still-running`: the exit of a VM's process could not be confirmed. + StillRunning, /// `io`: a host I/O operation failed, or state that NVX recorded on the host is unreadable or /// malformed. Io, @@ -170,6 +175,7 @@ impl ErrorCode { Self::Unavailable, Self::Unsupported, Self::Exited, + Self::StillRunning, Self::Io, Self::NotFound, Self::Protocol, @@ -205,6 +211,7 @@ impl ErrorCode { Self::Unavailable => ErrorKind::Unavailable.as_str(), Self::Unsupported => ErrorKind::Unsupported.as_str(), Self::Exited => ErrorKind::Exited.as_str(), + Self::StillRunning => ErrorKind::StillRunning.as_str(), Self::Io => ErrorKind::Io.as_str(), Self::NotFound => ErrorKind::NotFound.as_str(), Self::Protocol => ErrorKind::Protocol.as_str(), @@ -248,6 +255,7 @@ impl ErrorCode { | Self::ArgumentMissing | Self::ArgumentInvalid => ErrorKind::Unsupported, Self::Exited | Self::SandboxExited => ErrorKind::Exited, + Self::StillRunning => ErrorKind::StillRunning, Self::Io | Self::FormatUnsupported => ErrorKind::Io, Self::NotFound | Self::SandboxNotProvisioned => ErrorKind::NotFound, Self::Protocol | Self::ExecRefused | Self::EndpointClosed => ErrorKind::Protocol, @@ -278,6 +286,7 @@ impl From for ErrorCode { ErrorKind::Unavailable => Self::Unavailable, ErrorKind::Unsupported => Self::Unsupported, ErrorKind::Exited => Self::Exited, + ErrorKind::StillRunning => Self::StillRunning, ErrorKind::Io => Self::Io, ErrorKind::NotFound => Self::NotFound, ErrorKind::Protocol => Self::Protocol, diff --git a/nvx-rs/src/error/tests.rs b/nvx-rs/src/error/tests.rs index eb87d7dc0..a893e72b7 100644 --- a/nvx-rs/src/error/tests.rs +++ b/nvx-rs/src/error/tests.rs @@ -52,6 +52,7 @@ fn every_kind_has_a_code_of_its_own_spelling() { ErrorKind::Unavailable, ErrorKind::Unsupported, ErrorKind::Exited, + ErrorKind::StillRunning, ErrorKind::Io, ErrorKind::NotFound, ErrorKind::Protocol, diff --git a/nvx-rs/src/host/cli/store/files.rs b/nvx-rs/src/host/cli/store/files.rs new file mode 100644 index 000000000..9050cf281 --- /dev/null +++ b/nvx-rs/src/host/cli/store/files.rs @@ -0,0 +1,51 @@ +//! Private, regular, non-link files, atomically replaced and synced under the state lock. + +use std::fs; +use std::io; + +use super::StateFile; +use super::locked::LockedState; +use crate::host::platform; +use crate::host::store::{files, regular}; +use crate::{Error, Result}; + +/// Bounds a fixed record's paths and identities without reading an unbounded corrupt file. +const MAX_RECORD_BYTES: u64 = 1024 * 1024; + +pub(super) fn read(state: &LockedState, name: StateFile) -> Result>> { + let path = state.path(name); + let failed = |error| Error::io(format!("cannot read {}", path.display())).with_source(error); + let file = match regular::open_regular(&path) { + Ok(file) => file, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(failed(error)), + }; + if let Some(problem) = platform::private_file_problem(&file).map_err(failed)? { + return Err(Error::io(format!( + "the sandbox file {} {problem}", + path.display() + ))); + } + regular::read_at_most(file, MAX_RECORD_BYTES, "a sandbox record") + .map(Some) + .map_err(failed) +} + +pub(super) fn replace(state: &LockedState, name: StateFile, bytes: &[u8]) -> Result<()> { + if bytes.len() as u64 > MAX_RECORD_BYTES { + return Err(Error::io("the sandbox record exceeds its size bound")); + } + // Refuse an existing unsafe entry rather than replacing it or following its target. + read(state, name)?; + files::write_private(&state.path(name), bytes)?; + files::sync_dir(state.dir()) +} + +pub(super) fn remove(state: &LockedState, name: StateFile) -> Result<()> { + read(state, name)?; + let path = state.path(name); + fs::remove_file(&path).map_err(|error| { + Error::io(format!("cannot remove {}", path.display())).with_source(error) + })?; + files::sync_dir(state.dir()) +} diff --git a/nvx-rs/src/host/cli/store/launch.rs b/nvx-rs/src/host/cli/store/launch.rs new file mode 100644 index 000000000..d0a5c04e6 --- /dev/null +++ b/nvx-rs/src/host/cli/store/launch.rs @@ -0,0 +1,80 @@ +//! Locked launch transitions, without spawning, recovery, or process termination. + +use super::StateFile; +use super::files; +use super::locked::LockedState; +use super::record::{LaunchJournal, RuntimeRecord}; +use crate::host::process::ProcessRef; +use crate::host::runtime::RuntimeIdentity; +use crate::host::vm::{Prepared, SavedVm}; +use crate::{Error, Result}; + +/// A launch whose lock stays borrowed until commit or until its journal is left behind. +#[derive(Debug)] +pub(crate) struct Launch<'a> { + locked: &'a mut LockedState, + journal: LaunchJournal, +} + +impl LockedState { + /// Durably publishes the capability and complete prepared journal before the caller spawns. + pub(crate) fn begin_launch( + &mut self, + prepared: &Prepared, + runtime: &RuntimeIdentity, + ) -> Result> { + if self.journal()?.is_some() || self.runtime()?.is_some() { + return Err(Error::other( + "the sandbox already has a runtime record or an interrupted launch", + )); + } + let journal = LaunchJournal::prepared(self, prepared, runtime)?; + files::replace(self, StateFile::Capability, &journal.capability)?; + self.write_record(StateFile::Journal, &journal)?; + Ok(Launch { + locked: self, + journal, + }) + } +} + +impl Launch<'_> { + /// Records only the caller's spawned process, durably, before changing the in-memory state. + pub(crate) fn record_process(&mut self, process: &ProcessRef) -> Result<()> { + if self.journal.process.is_some() { + return Err(Error::other("the launch has recorded its process already")); + } + let journal = LaunchJournal { + process: Some(process.clone()), + ..self.journal.clone() + }; + self.locked.write_record(StateFile::Journal, &journal)?; + self.journal = journal; + Ok(()) + } + + /// Publishes the exact saved VM, only after its matching process was journaled. + /// + /// The runtime record is the commit. A failed journal removal keeps that committed record + /// and reports the failure; it never rolls back the commit or signals the VM. + pub(crate) fn commit(self, saved: &SavedVm) -> Result<()> { + if self.journal.process.as_ref() != Some(&saved.process) + || self.journal.vm_dir != saved.vm_dir.path() + || self.journal.endpoint != saved.endpoint + || self.journal.capability != saved.capability + || self.journal.runtime != saved.runtime + { + return Err(Error::other( + "the saved VM does not match the launch and its journaled process", + )); + } + self.locked + .write_record(StateFile::Runtime, &RuntimeRecord::from(saved))?; + files::remove(self.locked, StateFile::Journal).map_err(|error| { + Error::io(format!( + "the VM runtime was committed but its launch journal could not be removed: {error}" + )) + .with_source(error) + }) + } +} diff --git a/nvx-rs/src/host/cli/store/mod.rs b/nvx-rs/src/host/cli/store/mod.rs index 6f0c6479f..16bcb2432 100644 --- a/nvx-rs/src/host/cli/store/mod.rs +++ b/nvx-rs/src/host/cli/store/mod.rs @@ -26,6 +26,10 @@ //! and the Administrators group, as one under the user's profile does. An existing directory that //! others may access is refused rather than restricted, since they could have placed files in it, //! or still hold them open. The store creates its files readable only by the current user. +//! +//! A locked launch writes its prepared capability and outputs before spawn, records the spawned +//! process durably, and publishes the caller's exact saved VM only at commit. Dropping an +//! uncommitted launch keeps its journal; these transitions do not recover or signal a process. #![cfg_attr( not(test), expect( @@ -34,8 +38,11 @@ ) )] +mod files; +mod launch; mod locked; mod names; +mod record; use std::io; use std::path::{Path, PathBuf}; diff --git a/nvx-rs/src/host/cli/store/record.rs b/nvx-rs/src/host/cli/store/record.rs new file mode 100644 index 000000000..d84718801 --- /dev/null +++ b/nvx-rs/src/host/cli/store/record.rs @@ -0,0 +1,159 @@ +//! The fixed launch journal and the caller-saved VM's runtime record. + +use std::fmt; +use std::path::PathBuf; + +use serde::de::DeserializeOwned; +use serde::{Deserialize, Serialize}; + +use super::StateFile; +use super::files; +use super::locked::LockedState; +use crate::host::process::ProcessRef; +use crate::host::runtime::RuntimeIdentity; +use crate::host::vm::{Prepared, SavedVm, VmDir, VmOutputs}; +use crate::protocol::CAPABILITY_LEN; +use crate::{Error, Result}; + +#[derive(Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub(crate) struct LaunchJournal { + pub(super) vm_dir: PathBuf, + pub(super) capability: [u8; CAPABILITY_LEN], + pub(super) log: PathBuf, + pub(super) outcome: PathBuf, + pub(super) endpoint: String, + pub(super) console: Option, + pub(super) runtime: RuntimeIdentity, + pub(super) process: Option, +} + +impl fmt::Debug for LaunchJournal { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("LaunchJournal") + .field("vm_dir", &self.vm_dir) + .field("log", &self.log) + .field("outcome", &self.outcome) + .field("endpoint", &self.endpoint) + .field("console", &self.console) + .field("runtime", &self.runtime) + .field("process", &self.process) + .finish_non_exhaustive() + } +} + +impl LaunchJournal { + pub(super) fn prepared( + state: &LockedState, + prepared: &Prepared, + runtime: &RuntimeIdentity, + ) -> Result { + runtime.check()?; + let vm_dir = VmDir::open(state.dir())?; + let VmOutputs { + log, + outcome, + endpoint, + console, + } = prepared.outputs(); + if *log != vm_dir.path().join(StateFile::Log.name()) + || *outcome != vm_dir.path().join(StateFile::Outcome.name()) + { + return Err(Error::other( + "the prepared launch belongs to another sandbox directory", + )); + } + Ok(Self { + vm_dir: vm_dir.path().to_path_buf(), + capability: *prepared.capability().as_bytes(), + log: log.clone(), + outcome: outcome.clone(), + endpoint: endpoint.clone(), + console: console.clone(), + runtime: runtime.clone(), + process: None, + }) + } + + pub(crate) fn process(&self) -> Option<&ProcessRef> { + self.process.as_ref() + } +} + +#[derive(Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub(super) struct RuntimeRecord { + vm_dir: PathBuf, + endpoint: String, + capability: [u8; CAPABILITY_LEN], + process: ProcessRef, + runtime: RuntimeIdentity, +} + +impl From<&SavedVm> for RuntimeRecord { + fn from(saved: &SavedVm) -> Self { + Self { + vm_dir: saved.vm_dir.path().to_path_buf(), + endpoint: saved.endpoint.clone(), + capability: saved.capability, + process: saved.process.clone(), + runtime: saved.runtime.clone(), + } + } +} + +impl LockedState { + pub(crate) fn journal(&self) -> Result> { + self.read_record(StateFile::Journal) + } + + pub(crate) fn runtime(&self) -> Result> { + let Some(record) = self.read_record::(StateFile::Runtime)? else { + return Ok(None); + }; + let vm_dir = VmDir::open(self.dir())?; + if record.vm_dir != vm_dir.path() { + return Err(Error::io( + "the runtime record belongs to another sandbox directory", + )); + } + Ok(Some(SavedVm { + vm_dir, + endpoint: record.endpoint, + capability: record.capability, + process: record.process, + runtime: record.runtime, + })) + } + + pub(crate) fn capability(&self) -> Result> { + files::read(self, StateFile::Capability)? + .map(|bytes| { + bytes.try_into().map_err(|_| { + Error::io(format!( + "the sandbox capability must hold {CAPABILITY_LEN} bytes" + )) + }) + }) + .transpose() + } + + fn read_record(&self, name: StateFile) -> Result> { + files::read(self, name)? + .map(|bytes| { + serde_json::from_slice(&bytes).map_err(|error| { + Error::io(format!("cannot parse {}", self.path(name).display())) + .with_source(error) + }) + }) + .transpose() + } + + pub(super) fn write_record(&self, name: StateFile, record: &impl Serialize) -> Result<()> { + let bytes = serde_json::to_vec(record).map_err(|error| { + Error::io(format!("cannot serialize {}", self.path(name).display())).with_source(error) + })?; + files::replace(self, name, &bytes) + } +} diff --git a/nvx-rs/src/host/cli/store/tests.rs b/nvx-rs/src/host/cli/store/tests.rs index bb36e8418..f28a33df9 100644 --- a/nvx-rs/src/host/cli/store/tests.rs +++ b/nvx-rs/src/host/cli/store/tests.rs @@ -8,6 +8,8 @@ use super::*; use crate::host::cli::deadline::Deadline; use crate::host::store::{JOURNAL_NAME, RUNTIME_NAME, SandboxStore}; +mod launch; + /// How long a test waits for a lock that it released to be free. /// /// A process that a concurrent test spawns holds a copy of each descriptor of the test process diff --git a/nvx-rs/src/host/cli/store/tests/launch.rs b/nvx-rs/src/host/cli/store/tests/launch.rs new file mode 100644 index 000000000..aff660e14 --- /dev/null +++ b/nvx-rs/src/host/cli/store/tests/launch.rs @@ -0,0 +1,382 @@ +use std::error::Error as _; + +use super::*; +use crate::ErrorKind; +use crate::activation::{MachineIdMode, View}; +use crate::host::cli::store::{files, record}; +use crate::host::config::{Boot, ConsoleAttachment, RootConfig, VmConfig}; +use crate::host::process::{BootId, ProcessRef}; +use crate::host::runtime::{ArtifactIdentity, LaunchOptions, Runtime, RuntimeIdentity}; +use crate::host::vm::{self, Prepared, SavedVm, VmDir}; +use crate::hypervisor::Hypervisor; +use crate::protocol::CAPABILITY_LEN; + +fn prepared(state: &StateDir, console: ConsoleAttachment) -> (Prepared, RuntimeIdentity) { + let identity = |text| ArtifactIdentity::new(text).unwrap(); + let runtime = Runtime { + openvmm: "missing-openvmm".into(), + kernel: "missing-kernel".into(), + initramfs: "missing-initramfs".into(), + identity: RuntimeIdentity { + openvmm: identity("vmm-revision"), + kernel: identity("kernel-revision"), + initramfs: identity("initramfs-revision"), + boot_contract: u64::MAX, + control_contract: "saved-control-revision".to_owned(), + control_session: u64::MAX - 1, + }, + hypervisor: if cfg!(windows) { + Hypervisor::Whp + } else { + Hypervisor::Mshv + }, + cpu_profile: None, + windows_breakaway: false, + options: LaunchOptions::default(), + }; + let config = VmConfig { + processors: 1, + memory_mib: 256, + root: RootConfig::Blockless, + view: View { + hostname: b"journal-test".to_vec(), + machine_id: MachineIdMode::Image, + }, + network: None, + filesystem: None, + console, + boot: Some(Boot { + command_line: "nvx_entrypoint=/bin/sh".to_owned(), + capture: None, + }), + }; + let vm_dir = VmDir::open(state.dir()).unwrap(); + ( + vm::create(&runtime, &config, None, &vm_dir).unwrap(), + runtime.identity, + ) +} + +fn saved(state: &StateDir, prepared: &Prepared, runtime: RuntimeIdentity) -> SavedVm { + SavedVm { + vm_dir: VmDir::open(state.dir()).unwrap(), + endpoint: prepared.outputs().endpoint.clone(), + capability: *prepared.capability().as_bytes(), + process: ProcessRef { + pid: u32::MAX, + start_time: u64::MAX, + boot: BootId::from_bytes([0xa5; 16]), + }, + runtime, + } +} + +fn assert_saved(actual: &SavedVm, expected: &SavedVm) { + assert_eq!(actual.vm_dir, expected.vm_dir); + assert_eq!(actual.endpoint, expected.endpoint); + assert_eq!(actual.capability, expected.capability); + assert_eq!(actual.process, expected.process); + assert_eq!(actual.runtime, expected.runtime); +} + +fn read_file(locked: &locked::LockedState, name: StateFile) -> crate::Result { + match name { + StateFile::Journal => locked.journal().map(|record| record.is_some()), + StateFile::Runtime => locked.runtime().map(|record| record.is_some()), + StateFile::Capability => locked.capability().map(|record| record.is_some()), + _ => unreachable!(), + } +} + +#[test] +fn the_pre_spawn_barrier_and_commit_preserve_every_prepared_and_saved_value() { + for console in [ConsoleAttachment::Listener, ConsoleAttachment::Disconnected] { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, console); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + let spawn_hook = || { + let bytes = fs::read(state.path(StateFile::Journal)).unwrap(); + let journal: record::LaunchJournal = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(journal.vm_dir, saved.vm_dir.path()); + assert_eq!(journal.capability, saved.capability); + assert_eq!(journal.log, prepared.outputs().log); + assert_eq!(journal.outcome, prepared.outputs().outcome); + assert_eq!(journal.endpoint, prepared.outputs().endpoint); + assert_eq!(journal.console, prepared.outputs().console); + assert_eq!(journal.runtime, runtime); + assert_eq!(journal.process(), None); + assert!(!format!("{journal:?}").contains(&format!("{:?}", saved.capability))); + assert_eq!( + fs::read(state.path(StateFile::Capability)).unwrap(), + saved.capability + ); + for name in [StateFile::Journal, StateFile::Capability] { + let path = state.path(name); + let metadata = fs::symlink_metadata(&path).unwrap(); + assert!(metadata.is_file()); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!(metadata.permissions().mode() & 0o7777, 0o600); + } + let file = fs::File::open(path).unwrap(); + assert_eq!(platform::private_file_problem(&file).unwrap(), None); + } + assert!(!state.path(StateFile::Runtime).exists()); + }; + spawn_hook(); + launch.record_process(&saved.process).unwrap(); + let journal: record::LaunchJournal = + serde_json::from_slice(&fs::read(state.path(StateFile::Journal)).unwrap()).unwrap(); + assert_eq!(journal.process(), Some(&saved.process)); + assert!(!state.path(StateFile::Runtime).exists()); + launch.commit(&saved).unwrap(); + assert!(!state.path(StateFile::Journal).exists()); + assert_saved(&locked.runtime().unwrap().unwrap(), &saved); + assert_eq!(locked.capability().unwrap(), Some(saved.capability)); + drop(locked); + drop(prepared); + let reopened = StateDir::open(state.dir()) + .unwrap() + .lock(Deadline::after(10.0)) + .unwrap(); + assert_saved(&reopened.runtime().unwrap().unwrap(), &saved); + for name in [StateFile::Runtime, StateFile::Capability] { + let path = state.path(name); + let metadata = fs::symlink_metadata(&path).unwrap(); + assert!(metadata.is_file()); + assert!(!crate::host::filesystem::is_reparse_point(&metadata)); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!(metadata.permissions().mode() & 0o7777, 0o600); + } + let file = fs::File::open(path).unwrap(); + assert_eq!(platform::private_file_problem(&file).unwrap(), None); + } + } +} + +#[test] +fn an_interruption_keeps_the_journal_and_capability_without_publishing_runtime() { + for spawned in [false, true] { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Listener); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + { + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + if spawned { + launch.record_process(&saved.process).unwrap(); + } + } + drop(locked); + let mut reopened = state.lock(Deadline::after(10.0)).unwrap(); + let journal = reopened.journal().unwrap().unwrap(); + assert_eq!(journal.process(), spawned.then_some(&saved.process)); + assert_eq!(journal.capability, saved.capability); + assert_eq!(reopened.capability().unwrap(), Some(saved.capability)); + assert!(reopened.runtime().unwrap().is_none()); + let before = fs::read(state.path(StateFile::Journal)).unwrap(); + assert!(reopened.begin_launch(&prepared, &runtime).is_err()); + assert_eq!(fs::read(state.path(StateFile::Journal)).unwrap(), before); + } +} + +#[test] +fn a_commit_needs_the_same_saved_values_and_an_already_journaled_process() { + for mismatch in 0..6 { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let mut saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + if mismatch != 0 { + launch.record_process(&saved.process).unwrap(); + assert!(launch.record_process(&saved.process).is_err()); + } + let original = fs::read(state.path(StateFile::Journal)).unwrap(); + match mismatch { + 0 => {} + 1 => saved.process.start_time -= 1, + 2 => saved.endpoint.push('x'), + 3 => saved.capability[0] ^= 1, + 4 => saved.runtime.control_session -= 1, + 5 => { + let other = state.dir().join("other-vm"); + vm::create_private_dir(&other).unwrap(); + saved.vm_dir = VmDir::open(&other).unwrap(); + } + _ => unreachable!(), + } + assert!(launch.commit(&saved).is_err()); + assert!(!state.path(StateFile::Runtime).exists()); + assert_eq!(fs::read(state.path(StateFile::Journal)).unwrap(), original); + } +} + +#[test] +fn preparation_for_another_directory_and_failed_record_writes_do_not_publish_runtime() { + let (_directory, state) = state(); + let (_other_directory, other) = super::state(); + let (other_prepared, runtime) = prepared(&other, ConsoleAttachment::Disconnected); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + assert!(locked.begin_launch(&other_prepared, &runtime).is_err()); + assert!(!state.path(StateFile::Journal).exists()); + assert!(!state.path(StateFile::Capability).exists()); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let saved = saved(&state, &prepared, runtime.clone()); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + let journal = state.path(StateFile::Journal); + let original = journal.with_extension("original"); + fs::rename(&journal, &original).unwrap(); + fs::create_dir(&journal).unwrap(); + let error = launch.record_process(&saved.process).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!(error.source().is_some()); + fs::remove_dir(&journal).unwrap(); + fs::rename(&original, &journal).unwrap(); + assert!(launch.commit(&saved).is_err()); + assert!(!state.path(StateFile::Runtime).exists()); + assert!(locked.journal().unwrap().unwrap().process().is_none()); +} + +#[test] +fn a_failed_runtime_publication_keeps_the_spawned_journal() { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + launch.record_process(&saved.process).unwrap(); + let before = fs::read(state.path(StateFile::Journal)).unwrap(); + fs::create_dir(state.path(StateFile::Runtime)).unwrap(); + let error = launch.commit(&saved).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!(error.source().is_some()); + assert_eq!(fs::read(state.path(StateFile::Journal)).unwrap(), before); + assert_eq!( + fs::read_dir(state.path(StateFile::Runtime)) + .unwrap() + .count(), + 0 + ); +} + +#[test] +fn a_failed_journal_removal_reports_that_runtime_already_committed() { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + launch.record_process(&saved.process).unwrap(); + fs::remove_file(state.path(StateFile::Journal)).unwrap(); + fs::create_dir(state.path(StateFile::Journal)).unwrap(); + let error = launch.commit(&saved).unwrap_err(); + assert!(error.message().contains("runtime was committed"), "{error}"); + assert!(error.source().unwrap().source().is_some()); + assert_saved(&locked.runtime().unwrap().unwrap(), &saved); + assert!(state.path(StateFile::Journal).is_dir()); +} + +#[test] +fn malformed_records_and_capabilities_keep_their_parse_errors() { + let (_directory, state) = state(); + let locked = state.lock(Deadline::after(10.0)).unwrap(); + for name in [StateFile::Journal, StateFile::Runtime] { + files::replace(&locked, name, b"{malformed").unwrap(); + let error = read_file(&locked, name).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!( + error + .source() + .unwrap() + .downcast_ref::() + .is_some(), + "{error}" + ); + } + files::replace(&locked, StateFile::Capability, &[0x5a; CAPABILITY_LEN - 1]).unwrap(); + assert_eq!(locked.capability().unwrap_err().kind(), ErrorKind::Io); +} + +#[test] +fn records_and_capabilities_refuse_non_regular_entries_and_links_before_read_or_replace() { + for name in [ + StateFile::Journal, + StateFile::Runtime, + StateFile::Capability, + ] { + let (directory, state) = state(); + let locked = state.lock(Deadline::after(10.0)).unwrap(); + let path = state.path(name); + fs::create_dir(&path).unwrap(); + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + assert_eq!(fs::read_dir(&path).unwrap().count(), 0); + fs::remove_dir(&path).unwrap(); + let target = directory.path().join("target"); + fs::write(&target, b"preserved target").unwrap(); + #[cfg(unix)] + let linked = std::os::unix::fs::symlink(&target, &path); + #[cfg(windows)] + let linked = std::os::windows::fs::symlink_file(&target, &path); + match linked { + Ok(()) => { + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + assert_eq!(fs::read(&target).unwrap(), b"preserved target"); + fs::remove_file(&path).unwrap(); + } + #[cfg(windows)] + Err(error) if error.raw_os_error() == Some(1314) => { + eprintln!("skipped the symbolic link without Developer Mode or privilege: {error}"); + } + Err(error) => panic!("cannot create the test symbolic link: {error}"), + } + #[cfg(target_os = "linux")] + { + use nix::sys::stat::Mode; + nix::unistd::mkfifo(&path, Mode::S_IRUSR | Mode::S_IWUSR).unwrap(); + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + } + #[cfg(windows)] + { + let target = directory.path().join("junction-target"); + fs::create_dir(&target).unwrap(); + junction(&path, &target); + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + assert_eq!(fs::read_dir(target).unwrap().count(), 0); + } + } +} + +#[cfg(windows)] +#[test] +fn records_and_capabilities_that_other_users_may_access_are_refused_without_replacement() { + let (_directory, state) = state(); + let locked = state.lock(Deadline::after(10.0)).unwrap(); + for name in [ + StateFile::Journal, + StateFile::Runtime, + StateFile::Capability, + ] { + files::replace(&locked, name, b"preserved shared contents").unwrap(); + let path = state.path(name); + allow_everyone(&path, ""); + let before = fs::read(&path).unwrap(); + for error in [ + read_file(&locked, name).unwrap_err(), + files::replace(&locked, name, b"replacement").unwrap_err(), + ] { + assert_eq!(error.kind(), ErrorKind::Io); + assert!(error.message().contains("allows other users"), "{error}"); + } + assert_eq!(fs::read(path).unwrap(), before); + } +} diff --git a/nvx-rs/src/host/instance/locked.rs b/nvx-rs/src/host/instance/locked.rs index fca2a638b..2cbee7f48 100644 --- a/nvx-rs/src/host/instance/locked.rs +++ b/nvx-rs/src/host/instance/locked.rs @@ -216,7 +216,8 @@ impl LockedInstance { }; Ok(Stopped { forced, - report: report::read(&self.instance.report_path()), + // This legacy wrapper keeps its best-effort report policy. + report: report::read(&self.instance.report_path()).ok().flatten(), }) } diff --git a/nvx-rs/src/host/platform/windows/tests.rs b/nvx-rs/src/host/platform/windows/tests.rs index 44dd7359a..7da84f781 100644 --- a/nvx-rs/src/host/platform/windows/tests.rs +++ b/nvx-rs/src/host/platform/windows/tests.rs @@ -16,7 +16,8 @@ use windows_sys::Win32::Foundation::{ use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; use windows_sys::Win32::System::JobObjects::CreateJobObjectW; use windows_sys::Win32::System::Threading::{ - CreateEventW, GetCurrentProcess, INFINITE, OpenProcess, PROCESS_DUP_HANDLE, TerminateProcess, + CreateEventW, GetCurrentProcess, INFINITE, OpenProcess, PROCESS_DUP_HANDLE, + PROCESS_QUERY_LIMITED_INFORMATION, PROCESS_SYNCHRONIZE, TerminateProcess, }; pub(crate) use super::endpoint::tests::{accept_control, control_server}; @@ -38,6 +39,16 @@ pub(crate) fn wait_exit_handle(process: OwnedHandle, deadline: Instant) -> io::R .map(|exited| exited.expect("Windows process handles always provide an exit event")) } +impl ProcessHandle { + pub(crate) fn wait_only(pid: u32) -> Self { + Self { + process: open_handle(pid, PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_SYNCHRONIZE) + .unwrap(), + terminate: false, + } + } +} + thread_local! { /// What a spawn of this thread runs in its window, once its handles are inheritable and /// before it creates its process. @@ -1084,7 +1095,7 @@ fn a_launch_whose_vmm_exited_ends_while_a_process_spawned_in_its_window_holds_it let vmm = launch.record_process(&launched).unwrap(); launched.detach(); assert!( - vmm.wait(Instant::now() + WINDOW_TIMEOUT).unwrap(), + vmm.wait(Instant::now() + WINDOW_TIMEOUT).unwrap().is_some(), "the stand-in did not exit" ); // The VMM has exited, so the launch ends, whatever the other process holds. diff --git a/nvx-rs/src/host/process.rs b/nvx-rs/src/host/process.rs index 32047f8d6..100abe93d 100644 --- a/nvx-rs/src/host/process.rs +++ b/nvx-rs/src/host/process.rs @@ -188,6 +188,12 @@ pub struct Process { handle: platform::ProcessHandle, } +/// Proof that a verified process exited, produced only by [`Process::wait`]. +/// +/// It proves exit, not the process's exit status, and cannot be constructed by a caller. +#[derive(Debug)] +pub struct Exited(()); + impl Process { /// Finds the process that `reference` names, and returns `None` if it has exited. /// @@ -241,13 +247,17 @@ impl Process { }) } - /// Waits until the process exits or `deadline` passes, and returns whether it exited. + /// Waits until the process exits or `deadline` passes, returning exit proof or `None`. /// /// A process exits with its last thread. The wait sleeps until the exit event of the /// process, or with [`WaitMethod::Polling`], checks its identity at a short interval. An /// error means that the wait cannot determine whether the process exited. - pub fn wait(&self, deadline: Instant) -> io::Result { - self.wait_bool(deadline) + pub fn wait(&self, deadline: Instant) -> io::Result> { + Ok(if self.wait_bool(deadline)? { + Some(Exited(())) + } else { + None + }) } /// Keeps the boolean wait result for the legacy lifecycle callers. @@ -262,7 +272,7 @@ impl Process { /// Returns whether the process has exited, without waiting. pub fn has_exited(&self) -> io::Result { - self.wait(Instant::now()) + self.wait(Instant::now()).map(|exited| exited.is_some()) } /// Ends the process at once, unless it has exited or is exiting: with `SIGKILL` on Linux, and diff --git a/nvx-rs/src/host/process/tests.rs b/nvx-rs/src/host/process/tests.rs index ebc305ffa..a90c83102 100644 --- a/nvx-rs/src/host/process/tests.rs +++ b/nvx-rs/src/host/process/tests.rs @@ -275,7 +275,7 @@ fn hosts_without_pidfds_poll_and_never_signal() { let still_running = process.has_exited(); child.kill().unwrap(); child.wait().unwrap(); - assert!(!running.unwrap()); + assert!(running.unwrap().is_none()); assert_eq!(refused.unwrap_err().kind(), io::ErrorKind::Unsupported); assert!(!still_running.unwrap()); // Once the child is reaped, its ID names no process, or another one. @@ -283,6 +283,7 @@ fn hosts_without_pidfds_poll_and_never_signal() { process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); } diff --git a/nvx-rs/src/host/process/tests/linux.rs b/nvx-rs/src/host/process/tests/linux.rs index 4c9177b53..a2a0cf72f 100644 --- a/nvx-rs/src/host/process/tests/linux.rs +++ b/nvx-rs/src/host/process/tests/linux.rs @@ -152,13 +152,14 @@ fn a_process_runs_until_its_last_thread_exits() { .unwrap() .expect("a process whose main thread exited still runs"); assert!( - !process + process .wait(Instant::now() + Duration::from_millis(100)) .unwrap() + .is_none() ); child.0.kill().unwrap(); child.0.wait().unwrap(); - assert!(process.wait(Instant::now()).unwrap()); + assert!(process.wait(Instant::now()).unwrap().is_some()); } #[test] diff --git a/nvx-rs/src/host/report.rs b/nvx-rs/src/host/report.rs index 48f86fec4..f07bf2ee4 100644 --- a/nvx-rs/src/host/report.rs +++ b/nvx-rs/src/host/report.rs @@ -9,6 +9,8 @@ use std::path::Path; use serde_json::Value; +use crate::{Error, Result}; + /// Summary of an outcome report. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct Report { @@ -20,9 +22,24 @@ pub struct Report { pub teardown_complete: Option, } -/// Reads the report at `path`, or returns `None` if it is missing or not JSON. -pub fn read(path: &Path) -> Option { - let report: Value = serde_json::from_slice(&fs::read(path).ok()?).ok()?; +/// Reads the report at `path`, or returns `Ok(None)` if it is missing. +/// +/// An unreadable or malformed report fails with [`ErrorKind::Io`](crate::ErrorKind::Io), +/// retaining the underlying filesystem or JSON error. +pub fn read(path: &Path) -> Result> { + let bytes = match fs::read(path) { + Ok(bytes) => bytes, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(error) => { + return Err( + Error::io(format!("cannot read outcome report {}", path.display())) + .with_source(error), + ); + } + }; + let report: Value = serde_json::from_slice(&bytes).map_err(|error| { + Error::io(format!("cannot parse outcome report {}", path.display())).with_source(error) + })?; let mut summary = Report::default(); if let Some(outcome) = report.get("outcome") { summary.category = outcome @@ -35,7 +52,7 @@ pub fn read(path: &Path) -> Option { let complete = teardown.values().all(|step| step.as_bool() == Some(true)); summary.teardown_complete = Some(complete); } - Some(summary) + Ok(Some(summary)) } #[cfg(test)] diff --git a/nvx-rs/src/host/report/tests.rs b/nvx-rs/src/host/report/tests.rs index 127fd5b2e..961377aa4 100644 --- a/nvx-rs/src/host/report/tests.rs +++ b/nvx-rs/src/host/report/tests.rs @@ -11,7 +11,7 @@ fn reports_are_summarized_without_free_form_fields() { ) .unwrap(); assert_eq!( - read(&path).unwrap(), + read(&path).unwrap().unwrap(), Report { category: Some("success".to_owned()), status_code: Some(0), @@ -20,13 +20,60 @@ fn reports_are_summarized_without_free_form_fields() { ); fs::write(&path, r#"{"teardown":{"workers":true,"memory":false}}"#).unwrap(); assert_eq!( - read(&path).unwrap(), + read(&path).unwrap().unwrap(), Report { teardown_complete: Some(false), ..Report::default() } ); - assert!(read(&directory.path().join("missing.json")).is_none()); +} + +#[test] +fn missing_reports_are_distinct_from_malformed_or_unreadable_reports() { + let directory = tempfile::tempdir().unwrap(); + assert!( + read(&directory.path().join("missing.json")) + .unwrap() + .is_none() + ); + let path = directory.path().join("outcome.json"); fs::write(&path, b"{ truncated").unwrap(); - assert!(read(&path).is_none()); + let malformed = read(&path).unwrap_err(); + assert_eq!(malformed.kind(), crate::ErrorKind::Io); + assert!( + std::error::Error::source(&malformed) + .unwrap() + .is::() + ); + let unreadable = read(directory.path()).unwrap_err(); + assert_eq!(unreadable.kind(), crate::ErrorKind::Io); + assert!( + std::error::Error::source(&unreadable) + .unwrap() + .is::() + ); +} + +#[cfg(windows)] +#[test] +fn a_report_opened_without_read_sharing_returns_its_io_error() { + use std::os::windows::fs::OpenOptionsExt; + + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("outcome.json"); + fs::write(&path, b"{}").unwrap(); + let held = fs::OpenOptions::new() + .read(true) + .share_mode(0) + .open(&path) + .unwrap(); + let error = read(&path).unwrap_err(); + assert_eq!(error.kind(), crate::ErrorKind::Io); + assert!( + std::error::Error::source(&error) + .unwrap() + .is::() + ); + drop(held); + assert_eq!(read(&path).unwrap(), Some(Report::default())); } diff --git a/nvx-rs/src/host/snapshot.rs b/nvx-rs/src/host/snapshot.rs index 599a0fb53..7fcb88f6e 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,192 @@ 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: (), + /// 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, +} + +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 + } +} + +/// 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))? + { + 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(()) +} + +/// Checks a regular private file from metadata alone, without reading its contents. +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..8ab84cc1d 100644 --- a/nvx-rs/src/host/snapshot/key.rs +++ b/nvx-rs/src/host/snapshot/key.rs @@ -634,6 +634,19 @@ 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() + || 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/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(); 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/template/clone/records.rs b/nvx-rs/src/host/template/clone/records.rs index ce9752576..dc5fbc696 100644 --- a/nvx-rs/src/host/template/clone/records.rs +++ b/nvx-rs/src/host/template/clone/records.rs @@ -296,7 +296,7 @@ impl Recorder { /// report only once the clone's VM ended, or its teardown did. pub(super) fn observed(&self) -> Option<&Report> { self.observed - .get_or_init(|| report::read(&self.report)) + .get_or_init(|| report::read(&self.report).ok().flatten()) .as_ref() } @@ -1119,7 +1119,7 @@ impl CloneRecords { // The launch removed the report of an earlier VM of the sandbox before it wrote the first // of the clone's records, which a progress past the plan shows. let report = (progress > Progress::Planned) - .then(|| report::read(&sandbox.outcome_path())) + .then(|| report::read(&sandbox.outcome_path()).ok().flatten()) .flatten(); Ok(Some(Self { lease, diff --git a/nvx-rs/src/host/template/clone/tests/launch.rs b/nvx-rs/src/host/template/clone/tests/launch.rs index c58043a4c..7ec87eec6 100644 --- a/nvx-rs/src/host/template/clone/tests/launch.rs +++ b/nvx-rs/src/host/template/clone/tests/launch.rs @@ -365,14 +365,14 @@ impl VmmDir { .unwrap() .expect("the VMM runs until the test ends it"); self.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } /// Waits until the VMM, which exits on its own, as its [plan](Self::plan) asks, has exited. pub(super) fn wait_exit(&self) { let reference = self.reference.as_ref().expect("the test knows the VMM"); if let Some(process) = Process::open(reference).unwrap() { - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } } @@ -413,7 +413,9 @@ impl Drop for VmmDir { let _ = fs::write(self.path().join(EXIT_MARKER), b""); if let Some(reference) = &self.reference && let Ok(Some(process)) = Process::open(reference) - && !process.wait(Instant::now() + TIMEOUT).unwrap_or(false) + && !process + .wait(Instant::now() + TIMEOUT) + .is_ok_and(|exited| exited.is_some()) { let _ = process.kill(); let _ = process.wait(Instant::now() + TIMEOUT); @@ -1567,7 +1569,7 @@ fn a_clone_whose_launch_failed_before_it_began_launches_no_more_until_its_releas // clone's: an inspection takes none, and the launch removes it before it records anything. let report = acquired.locked.outcome_path(); fs::write(&report, guest_exit(195)).unwrap(); - assert!(crate::host::report::read(&report).is_some()); + assert!(crate::host::report::read(&report).unwrap().is_some()); let records = CloneRecords::read(&acquired.locked).unwrap().unwrap(); assert_eq!( (records.progress, records.report), @@ -2323,7 +2325,7 @@ fn a_launch_whose_caller_crashes_once_its_vmm_started_ends_the_vmm_as_reattach_r } => assert_eq!(terminated, reference), other => panic!("unexpected reattach: {other:?}"), } - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(!exists(&crashed.socket())); crashed.releases(&mut locked); } @@ -2356,7 +2358,10 @@ fn a_restore_whose_caller_crashes_before_its_commit_ends_its_vmm_as_reattach_rec } => assert_eq!(terminated, reference, "{step}"), other => panic!("unexpected reattach at {step}: {other:?}"), } - assert!(process.wait(Instant::now() + TIMEOUT).unwrap(), "{step}"); + assert!( + process.wait(Instant::now() + TIMEOUT).unwrap().is_some(), + "{step}" + ); assert!(!exists(&crashed.socket()), "{step}"); crashed.releases(&mut locked); } diff --git a/nvx-rs/src/host/template/clone/tests/restore.rs b/nvx-rs/src/host/template/clone/tests/restore.rs index 0a33cb1dd..61282c561 100644 --- a/nvx-rs/src/host/template/clone/tests/restore.rs +++ b/nvx-rs/src/host/template/clone/tests/restore.rs @@ -422,7 +422,7 @@ fn vmm_with(plan: &str) -> VmmDir { fn assert_vmm_ended(vmm: &VmmDir) { let reference = vmm.reference.as_ref().unwrap(); if let Some(process) = Process::open(reference).unwrap() { - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } } diff --git a/nvx-rs/src/host/template/store/leases/tests.rs b/nvx-rs/src/host/template/store/leases/tests.rs index 1205e4ba3..eaeed151c 100644 --- a/nvx-rs/src/host/template/store/leases/tests.rs +++ b/nvx-rs/src/host/template/store/leases/tests.rs @@ -2460,7 +2460,9 @@ impl Drop for VmmDir { let _ = fs::write(self.path().join(EXIT_MARKER), b""); if let Some(reference) = &self.reference && let Ok(Some(process)) = Process::open(reference) - && !process.wait(Instant::now() + TIMEOUT).unwrap_or(false) + && !process + .wait(Instant::now() + TIMEOUT) + .is_ok_and(|exited| exited.is_some()) { let _ = process.kill(); let _ = process.wait(Instant::now() + TIMEOUT); @@ -2639,7 +2641,7 @@ fn a_clone_that_crashes_after_its_spawn_keeps_its_lease_until_its_unrecorded_vmm crashed.refuses_release(&mut locked); // Once the VMM exits, the claim that it released proves it. crashed.vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); match reattach_until_determinate(&mut locked) { Reattached::Exited { record: None, @@ -2669,7 +2671,7 @@ fn a_clone_that_crashes_after_recording_its_vmm_keeps_its_lease_until_reattach_e } => assert_eq!(terminated, reference), other => panic!("unexpected reattach: {other:?}"), } - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); crashed.releases(&mut locked); } diff --git a/nvx-rs/src/host/vm.rs b/nvx-rs/src/host/vm.rs index 1429a7a26..72e724aa6 100644 --- a/nvx-rs/src/host/vm.rs +++ b/nvx-rs/src/host/vm.rs @@ -16,6 +16,7 @@ use std::fmt; use std::fs::{self, File, OpenOptions}; use std::io; use std::path::{Path, PathBuf}; +use std::time::Instant; use crate::host::config::{ConsoleAttachment, RootConfig, UpperConfig, VmConfig}; use crate::host::filesystem; @@ -25,6 +26,7 @@ use crate::host::platform; use crate::host::platform::{check_store_dir as check_dir, create_store_dir as create_dir}; #[cfg(windows)] use crate::host::platform::{check_vm_dir as check_dir, create_vm_dir as create_dir}; +use crate::host::report::{self, Report}; use crate::host::runtime::{Runtime, RuntimeIdentity}; use crate::host::snapshot::Snapshot; use crate::host::spawn::{self, VmmCommand}; @@ -522,6 +524,78 @@ pub fn spawn(prepared: Prepared) -> Result { spawn_verified(prepared, spawn::Launched::process) } +/// A confirmed VM exit, with the outcome-report read kept separate from exit proof. +#[derive(Debug)] +pub struct Stopped { + /// Exit was confirmed before any forced termination was attempted. + pub self_exited: bool, + /// Forced termination through the verified process was attempted. + pub forced: bool, + /// The outcome summary, missing report, or report-read error after confirmed exit. + pub report: Result>, +} + +/// Stops a VM, returning only after its verified process has exited. +/// +/// Unless exit is already confirmed, calls `graceful` with `grace`, bounded by `deadline`, +/// then waits until that grace deadline. The callback must respect the supplied deadline; +/// its boolean request result is not exit proof. If exit is still unconfirmed, attempts +/// forced termination and waits until `deadline`. A failed termination is harmless only +/// when that final wait confirms exit. +/// +/// An unconfirmed exit fails with [`ErrorKind::StillRunning`](crate::ErrorKind::StillRunning), +/// retaining host errors. The caller keeps the process in that case. +/// Report-read errors have kind [`ErrorKind::Io`](crate::ErrorKind::Io) +/// inside [`Stopped::report`], never hiding confirmed exit. +pub fn stop( + process: &Process, + outputs: &VmOutputs, + graceful: impl FnOnce(Instant) -> bool, + grace: Instant, + deadline: Instant, +) -> Result { + let failure = |kill_error: Option<&io::Error>| { + let mut message = format!( + "cannot confirm exit of VM process {}", + process.reference().pid + ); + if let Some(error) = kill_error { + message.push_str(&format!("; forced termination failed: {error}")); + } + Error::new(crate::ErrorKind::StillRunning, message) + }; + let wait = |until| { + process + .wait(until) + .map_err(|error| failure(None).with_source(error)) + }; + let self_exited = if wait(Instant::now())?.is_some() { + true + } else { + let grace = grace.min(deadline); + graceful(grace); + wait(grace)?.is_some() + }; + if !self_exited { + let kill_error = process.kill().err(); + let exited = process + .wait(deadline) + .map_err(|error| failure(kill_error.as_ref()).with_source(error))?; + if exited.is_none() { + let error = failure(kill_error.as_ref()); + return Err(match kill_error { + Some(source) => error.with_source(source), + None => error, + }); + } + } + Ok(Stopped { + self_exited, + forced: !self_exited, + report: report::read(&outputs.outcome), + }) +} + /// Keeps the child owned until its process identity is verified, or its exit is confirmed. fn spawn_verified( prepared: Prepared, diff --git a/nvx-rs/src/host/vm/tests.rs b/nvx-rs/src/host/vm/tests.rs index 05af49bd0..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"), @@ -1098,6 +1099,7 @@ fn spawn_caller_helper() { .process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); panic!("cannot record the helper's child: {error}"); } @@ -1181,6 +1183,7 @@ fn an_unrelated_process_started_in_the_library_spawn_window_gets_no_vm_claim() { .process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); assert!( platform::tests::claim_released_within(&log, identity, Duration::from_secs(10)), @@ -1282,10 +1285,193 @@ fn current_cold_spawn_boots_a_guest_accepts_shell_input_and_exits_cleanly() { ) .unwrap(); read_until(&mut *console, b"NVX-SPAWN-COLD-OK"); - assert!(launched.0.process.wait(deadline).unwrap()); + assert!(launched.0.process.wait(deadline).unwrap().is_some()); assert!(reopened.has_exited().unwrap()); - let outcome = crate::host::report::read(&launched.0.outputs.outcome).unwrap(); + let outcome = crate::host::report::read(&launched.0.outputs.outcome) + .unwrap() + .unwrap(); assert_eq!(outcome.category.as_deref(), Some("success")); assert_eq!(outcome.status_code, Some(0)); println!("NVX-LIBRARY-COLD-SPAWN-EXIT-OK: {}", reference.pid); } + +#[cfg(any(target_os = "linux", windows))] +struct StopChild { + child: std::process::Child, + process: Process, +} + +#[cfg(any(target_os = "linux", windows))] +impl StopChild { + fn start() -> Self { + use std::io::{BufRead, BufReader}; + + #[cfg(windows)] + let mut command = { + let mut command = Command::new( + PathBuf::from(std::env::var_os("SystemRoot").unwrap()).join("System32\\cmd.exe"), + ); + command.args(["/d", "/q", "/c", "echo ready & set /p wait="]); + command + }; + #[cfg(target_os = "linux")] + let mut command = { + let mut command = Command::new("sh"); + command.args(["-c", "printf 'ready\\n'; read -r wait"]); + command + }; + command + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + let mut child = crate::host::tests::spawn_locked(&mut command).unwrap(); + let mut ready = String::new(); + BufReader::new(child.stdout.take().unwrap()) + .read_line(&mut ready) + .unwrap(); + assert_eq!(ready.trim_end(), "ready"); + let reference = crate::host::process::ProcessRef { + pid: child.id(), + start_time: platform::process_start_time(child.id()).unwrap().unwrap(), + boot: crate::host::process::BootId::current().unwrap(), + }; + let process = Process::open(&reference).unwrap().unwrap(); + Self { child, process } + } +} + +#[cfg(any(target_os = "linux", windows))] +impl Drop for StopChild { + fn drop(&mut self) { + if self.child.try_wait().unwrap().is_none() { + self.child.kill().unwrap(); + } + self.child.wait().unwrap(); + } +} + +#[cfg(any(target_os = "linux", windows))] +#[test] +fn stop_preserves_report_results_after_a_real_self_exit() { + for report in ["missing", "valid", "malformed", "unreadable"] { + let (_directory, vm) = vm_dir(); + let outputs = vm.outputs(false).unwrap(); + match report { + "missing" => {} + "valid" => fs::write( + &outputs.outcome, + br#"{"outcome":{"category":"success","status_code":0},"teardown":{"workers":true}}"#, + ).unwrap(), + "malformed" => fs::write(&outputs.outcome, b"{ truncated").unwrap(), + "unreadable" => fs::create_dir(&outputs.outcome).unwrap(), + _ => unreachable!(), + } + let mut child = StopChild::start(); + let deadline = Instant::now() + Duration::from_secs(10); + let stopped = stop( + &child.process, + &outputs, + |supplied| { + assert_eq!(supplied, deadline); + drop(child.child.stdin.take()); + false + }, + deadline, + deadline, + ) + .unwrap(); + assert!(stopped.self_exited); + assert!(!stopped.forced); + assert!(child.process.wait(Instant::now()).unwrap().is_some()); + assert!(child.child.try_wait().unwrap().is_some()); + let already_exited = stop( + &child.process, + &outputs, + |_| panic!("an exited process needs no graceful request"), + Instant::now(), + Instant::now(), + ) + .unwrap(); + assert!(already_exited.self_exited); + assert!(!already_exited.forced); + match report { + "missing" => assert!(stopped.report.unwrap().is_none()), + "valid" => assert_eq!( + stopped.report.unwrap(), + Some(Report { + category: Some("success".to_owned()), + status_code: Some(0), + teardown_complete: Some(true), + }) + ), + _ => { + let error = stopped.report.unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!(std::error::Error::source(&error).is_some()); + } + } + } +} + +#[cfg(any(target_os = "linux", windows))] +#[test] +fn stop_forces_only_its_verified_process_after_grace() { + let (_directory, vm) = vm_dir(); + let outputs = vm.outputs(false).unwrap(); + let mut child = StopChild::start(); + let mut other = StopChild::start(); + let grace = Instant::now() + Duration::from_millis(80); + let deadline = grace + Duration::from_secs(10); + let stopped = stop( + &child.process, + &outputs, + |supplied| { + assert_eq!(supplied, grace); + true + }, + grace, + deadline, + ) + .unwrap(); + assert!(!stopped.self_exited); + assert!(stopped.forced); + assert!(stopped.report.unwrap().is_none()); + assert!(child.process.wait(Instant::now()).unwrap().is_some()); + assert!(child.child.try_wait().unwrap().is_some()); + assert!(other.process.wait(Instant::now()).unwrap().is_none()); + assert!(other.child.try_wait().unwrap().is_none()); +} + +#[cfg(any(target_os = "linux", windows))] +#[test] +fn stop_returns_still_running_without_exit_confirmation_at_its_deadline() { + let (_directory, vm) = vm_dir(); + let outputs = vm.outputs(false).unwrap(); + fs::write(&outputs.outcome, b"{ truncated").unwrap(); + let mut child = StopChild::start(); + #[cfg(target_os = "linux")] + let handle = platform::ProcessHandle::without_pidfd(); + #[cfg(windows)] + let handle = platform::ProcessHandle::wait_only(child.child.id()); + let process = Process::held(child.process.reference().clone(), handle); + let grace = Instant::now(); + let deadline = grace + Duration::from_millis(80); + let error = stop(&process, &outputs, |_| false, grace, deadline).unwrap_err(); + assert!(Instant::now() >= deadline); + assert_eq!(error.kind(), ErrorKind::StillRunning); + assert_eq!(error.code().as_str(), "still-running"); + let cause = std::error::Error::source(&error) + .unwrap() + .downcast_ref::() + .unwrap(); + assert_eq!( + cause.kind(), + if cfg!(windows) { + io::ErrorKind::PermissionDenied + } else { + io::ErrorKind::Unsupported + } + ); + assert!(process.wait(Instant::now()).unwrap().is_none()); + assert!(child.child.try_wait().unwrap().is_none()); +} diff --git a/nvx-rs/src/host/vm/tests/reattach.rs b/nvx-rs/src/host/vm/tests/reattach.rs index 88a95d005..0611675b6 100644 --- a/nvx-rs/src/host/vm/tests/reattach.rs +++ b/nvx-rs/src/host/vm/tests/reattach.rs @@ -244,7 +244,7 @@ fn reattaching_returns_the_held_owned_child_and_authenticated_connection() { b"response" ); owner.wait(); - assert!(attached.process.wait(deadline).unwrap()); + assert!(attached.process.wait(deadline).unwrap().is_some()); } #[test] diff --git a/nvx-rs/tests/checked_handoff.rs b/nvx-rs/tests/checked_handoff.rs index b07a9e340..8f1c7b07b 100644 --- a/nvx-rs/tests/checked_handoff.rs +++ b/nvx-rs/tests/checked_handoff.rs @@ -703,6 +703,7 @@ fn a_prepared_launch_hands_the_vmm_its_checked_images_alone() { .unwrap() .wait(Instant::now() + TIMEOUT) .unwrap() + .is_some() ); launched.kill_and_wait(); let scratch = fs::read_to_string(staged.scratch()).unwrap(); @@ -788,6 +789,7 @@ fn a_vmm_keeps_the_scratch_images_claim_once_its_launcher_is_gone() { .unwrap() .wait(Instant::now() + TIMEOUT) .unwrap() + .is_some() ); launched.kill_and_wait(); drop(check_written().unwrap()); diff --git a/nvx-rs/tests/cli.rs b/nvx-rs/tests/cli.rs index f3b64de5b..01191d000 100644 --- a/nvx-rs/tests/cli.rs +++ b/nvx-rs/tests/cli.rs @@ -1007,9 +1007,10 @@ fn a_stop_under_another_identity_never_ends_a_process_that_its_record_does_not_n ); assert_eq!(runtime_record(&root, "state"), None); assert!( - !process + process .wait(Instant::now() + Duration::from_millis(200)) - .unwrap(), + .unwrap() + .is_none(), "{field}" ); process.kill().unwrap(); @@ -1017,6 +1018,7 @@ fn a_stop_under_another_identity_never_ends_a_process_that_its_record_does_not_n process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); } assert_eq!(transition(&root, "deprovision", "state", &[]), succeeded()); @@ -1153,6 +1155,7 @@ fn stop_reports_its_timeout_and_leaves_the_vm_running() { process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); assert_eq!( transition(&root, "deprovision", "unanswered", &[]), @@ -1173,6 +1176,7 @@ fn stop_reports_its_timeout_and_leaves_the_vm_running() { process .wait(Instant::now() + Duration::from_secs(30)) .unwrap() + .is_some() ); assert_eq!( transition(&root, "stop", "slow", &[]), @@ -1194,6 +1198,7 @@ fn a_sandbox_whose_openvmm_exited_reports_it_once_and_starts_again() { process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); }; crash(); diff --git a/nvx-rs/tests/cli/download.rs b/nvx-rs/tests/cli/download.rs index be959f532..f9d758cea 100644 --- a/nvx-rs/tests/cli/download.rs +++ b/nvx-rs/tests/cli/download.rs @@ -171,6 +171,22 @@ fn provision_and_deprovision_need_no_artifact_root() { let executable = elsewhere .path() .join(Path::new(NVX_RS).file_name().unwrap()); + #[cfg(unix)] + { + // Concurrent forks of this test process must inherit no writable executable. + let copied = Command::new("cp") + .arg("-p") + .arg(NVX_RS) + .arg(&executable) + .output() + .unwrap(); + assert!( + copied.status.success(), + "cannot copy the executable: {}", + String::from_utf8_lossy(&copied.stderr) + ); + } + #[cfg(windows)] fs::copy(NVX_RS, &executable).unwrap(); let nvx_rs = |arguments: &[String]| { let output = Command::new(&executable) diff --git a/nvx-rs/tests/cli/lifecycle.rs b/nvx-rs/tests/cli/lifecycle.rs index 7078aba9e..d82a0f548 100644 --- a/nvx-rs/tests/cli/lifecycle.rs +++ b/nvx-rs/tests/cli/lifecycle.rs @@ -604,7 +604,8 @@ fn check_lifecycle(case: &Configuration) { assert!( process .wait(Instant::now() + Duration::from_secs(10)) - .unwrap(), + .unwrap() + .is_some(), "{context}: OpenVMM runs on" ); assert_eq!(entries(&state), STOPPED, "{context}"); @@ -694,7 +695,8 @@ fn lifecycle_of_a_sandbox_whose_upper_is_in_ram() { assert!( process .wait(Instant::now() + Duration::from_secs(10)) - .unwrap(), + .unwrap() + .is_some(), "{context}: OpenVMM runs on" ); assert_eq!(entries(&state), STOPPED, "{context}"); @@ -737,7 +739,8 @@ fn stops_keep_an_outcome_report_that_the_caller_reads_whatever_its_umask() { assert!( process .wait(Instant::now() + Duration::from_secs(10)) - .unwrap(), + .unwrap() + .is_some(), "{context}: OpenVMM runs on" ); assert_eq!(entries(&state), STOPPED, "{context}"); @@ -1437,9 +1440,10 @@ fn recycled_process_id(root: &Root) { assert_refused(root, &outcome, "sandbox-exited", &["stale", "not running"]); assert!(!path.exists()); assert!( - !openvmm + openvmm .wait(Instant::now() + Duration::from_millis(200)) - .unwrap(), + .unwrap() + .is_none(), "the stale record's process was signaled" ); openvmm.kill().unwrap(); @@ -1447,6 +1451,7 @@ fn recycled_process_id(root: &Root) { openvmm .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); let outcome = operate(root, "stop", "state", &[]); assert_refused(root, &outcome, "sandbox-not-running", &["not running"]); diff --git a/nvx-rs/tests/fake_openvmm.rs b/nvx-rs/tests/fake_openvmm.rs index c667dbe84..b112f951b 100644 --- a/nvx-rs/tests/fake_openvmm.rs +++ b/nvx-rs/tests/fake_openvmm.rs @@ -478,7 +478,7 @@ fn outcome_reports_of_another_runtime_are_never_read() { .unwrap(); if exits_first { process.kill().unwrap(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } let stopped = locked.terminate(mismatched, TIMEOUT).unwrap(); assert_eq!( @@ -528,11 +528,12 @@ fn records_that_name_another_process_never_end_it() { drop(locked); assert!(records_nothing(&instance)); assert!( - !process + process .wait(Instant::now() + Duration::from_millis(200)) .unwrap() + .is_none() ); process.kill().unwrap(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } } diff --git a/nvx-rs/tests/store.rs b/nvx-rs/tests/store.rs index 39d55d2e4..9602cce68 100644 --- a/nvx-rs/tests/store.rs +++ b/nvx-rs/tests/store.rs @@ -183,7 +183,9 @@ fn kill(pid: u32, start_time: u64) -> io::Result { return Ok(true); }; process.kill()?; - process.wait(Instant::now() + RECOVERY_TIMEOUT) + process + .wait(Instant::now() + RECOVERY_TIMEOUT) + .map(|exited| exited.is_some()) } /// A VMM that a crashed orchestrator left behind, which is killed when the test ends if it still @@ -533,7 +535,12 @@ fn a_committed_launch_names_its_vm_for_another_build() { .expect("the recorded VMM runs"); assert!(!reattached.has_exited().unwrap()); drop(vmm); - assert!(reattached.wait(Instant::now() + RECOVERY_TIMEOUT).unwrap()); + assert!( + reattached + .wait(Instant::now() + RECOVERY_TIMEOUT) + .unwrap() + .is_some() + ); other.lock(None).unwrap().clear_runtime().unwrap(); assert!(other.runtime().unwrap().is_none()); } diff --git a/nvx-rs/tests/supervisor.rs b/nvx-rs/tests/supervisor.rs index be2fb01a0..45b2058ab 100644 --- a/nvx-rs/tests/supervisor.rs +++ b/nvx-rs/tests/supervisor.rs @@ -450,7 +450,9 @@ impl Drop for VmmDirectory { let _ = File::create(self.path().join(EXIT_MARKER)); if let Some(reference) = &self.reference && let Ok(Some(process)) = Process::open(reference) - && !process.wait(Instant::now() + TIMEOUT).unwrap_or(false) + && !process + .wait(Instant::now() + TIMEOUT) + .is_ok_and(|exited| exited.is_some()) { let _ = process.kill(); let _ = process.wait(Instant::now() + TIMEOUT); @@ -613,7 +615,13 @@ fn long_stores_launch_with_a_short_working_directory() { let mut vmm = VmmDirectory::new(); let mut locked = store.lock(None).unwrap(); let running = launch_vmm(&mut locked, &mut vmm); - assert!(!running.process().wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!( + running + .process() + .wait(Instant::now() + SHORT_WAIT) + .unwrap() + .is_none() + ); let stopped = running .stop(&mut locked, Instant::now() + TIMEOUT, None, |_, _| { vmm.end(); @@ -671,15 +679,15 @@ fn a_child_that_exits_wakes_its_waiter() { assert_eq!(process.reference().boot, BootId::current().unwrap()); assert_eq!(process.wait_method(), WaitMethod::Event); assert!(!process.has_exited().unwrap()); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); // The reference finds the same process through a handle of its own. let found = open_running(process.reference()); assert_eq!(found.reference(), process.reference()); let started = Instant::now(); vmm.end(); - assert!(process.wait(started + TIMEOUT).unwrap()); - assert!(found.wait(started + TIMEOUT).unwrap()); + assert!(process.wait(started + TIMEOUT).unwrap().is_some()); + assert!(found.wait(started + TIMEOUT).unwrap().is_some()); assert!(started.elapsed() < TIMEOUT); assert_ended(&process); // Ending a process that exited does nothing. @@ -698,12 +706,12 @@ fn a_process_that_is_not_a_child_wakes_its_waiters() { launcher.finish(); let process = open_running(&reference); assert_eq!(process.wait_method(), WaitMethod::Event); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); let started = Instant::now(); let waiters: Vec = thread::scope(|scope| { let waiters: Vec<_> = (0..2) - .map(|_| scope.spawn(|| process.wait(started + TIMEOUT).unwrap())) + .map(|_| scope.spawn(|| process.wait(started + TIMEOUT).unwrap().is_some())) .collect(); vmm.end(); waiters @@ -740,10 +748,10 @@ fn a_restarted_launcher_finds_its_vmm_again_and_ends_it() { vmm.record(&reference); let process = open_running(&reference); assert_eq!(process.wait_method(), WaitMethod::Event); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); let started = Instant::now(); process.kill().unwrap(); - assert!(process.wait(started + TIMEOUT).unwrap()); + assert!(process.wait(started + TIMEOUT).unwrap().is_some()); assert_ended(&process); } @@ -783,7 +791,7 @@ fn a_reused_process_id_is_never_signaled() { assert!(!process.has_exited().unwrap()); open_running(&reference); vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(launched.kill_and_wait()); } @@ -814,7 +822,7 @@ fn references_that_name_no_process_are_never_opened() { let launched = spawn_vmm(vmm.path(), false); let process = launched.process().unwrap(); vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(launched.kill_and_wait()); assert!(Process::open(process.reference()).unwrap().is_none()); assert!(process.has_exited().unwrap()); @@ -877,7 +885,7 @@ fn an_unrecorded_vmm_that_serves_no_endpoint_is_never_signaled() { assert!(store.journal().unwrap().is_some()); // Once the VMM exits, the claim that it released proves it. vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); match reattach_until_determinate(&mut locked) { Reattached::Exited { record: None, @@ -1264,7 +1272,7 @@ fn check_vmm(reader: PipeReader, writer: &PipeWriter, others: &Mutex> } } vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(launched.kill_and_wait()); } @@ -1493,7 +1501,7 @@ fn a_vmm_in_its_orchestrators_job_ends_with_it() { // The VMM did not break away, so closing the job ends it with its orchestrator. drop(job); orchestrator.wait(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); let mut locked = store.lock(Some(Instant::now() + TIMEOUT)).unwrap(); match supervisor::reattach(&mut locked, &runtime()).unwrap() { Reattached::Exited { @@ -1517,10 +1525,10 @@ fn a_job_that_ends_its_vmm_wakes_the_waiters() { vmm.record(&reference); // The VMM did not break away, so the job holds it too, and closing the job ends it. let process = open_running(&reference); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); let started = Instant::now(); drop(job); - assert!(process.wait(started + TIMEOUT).unwrap()); + assert!(process.wait(started + TIMEOUT).unwrap().is_some()); assert_ended(&process); launcher.wait(); } @@ -1587,7 +1595,7 @@ fn a_process_that_this_user_may_not_terminate_is_waited_for_but_never_signaled() // Its creator ends it, which wakes the waiter, and ending a process that exited does nothing. child.0.kill().unwrap(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); process.kill().unwrap(); assert_ended(&process); } 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 } diff --git a/tests/runtime-adapter/src/bin/runtime-adapter-host.rs b/tests/runtime-adapter/src/bin/runtime-adapter-host.rs index 449ceb1f0..844c9a4f0 100644 --- a/tests/runtime-adapter/src/bin/runtime-adapter-host.rs +++ b/tests/runtime-adapter/src/bin/runtime-adapter-host.rs @@ -2053,6 +2053,7 @@ fn launch_failed( let exited = starting .process() .wait(Instant::now() + STOP_TIMEOUT) + .map(|exited| exited.is_some()) .map_err(|error| format!("cannot wait for the VM to exit: {error}"))?; starting.abandon().map_err(|error| describe(&error))?; println!("stopped={}", if exited { "graceful" } else { "forced" }); @@ -2078,6 +2079,7 @@ fn table_failed( Ok(_) => starting .process() .wait(Instant::now() + STOP_TIMEOUT) + .map(|exited| exited.is_some()) .map_err(|error| format!("cannot wait for the VM to exit: {error}"))?, Err(_) => false, }; @@ -3625,12 +3627,14 @@ fn hold(locked: &mut LockedStore, runtime: &Runtime) -> Result Result<(), String> { - let outcome = report::read(&store.outcome_path()).ok_or_else(|| { - format!( - "the VM left no outcome report {}", - store.outcome_path().display() - ) - })?; + let outcome = report::read(&store.outcome_path()) + .map_err(|error| error.to_string())? + .ok_or_else(|| { + format!( + "the VM left no outcome report {}", + store.outcome_path().display() + ) + })?; print_outcome(&outcome); if ( outcome.category.as_deref(), diff --git a/tests/runtime-adapter/tests/common/mod.rs b/tests/runtime-adapter/tests/common/mod.rs index 3c3540923..dc5e0f914 100644 --- a/tests/runtime-adapter/tests/common/mod.rs +++ b/tests/runtime-adapter/tests/common/mod.rs @@ -154,11 +154,14 @@ impl Sandbox { pub(crate) fn new(name: &str) -> Self { let pid = std::process::id(); let root = (0..) - .map(|attempt| env::temp_dir().join(format!("runtime-adapter-{name}-{pid}-{attempt}"))) + .map(|attempt| env::temp_dir().join(format!("ra-{pid}-{attempt}"))) .find(|root| match fs::create_dir(root) { Ok(()) => true, Err(error) if error.kind() == io::ErrorKind::AlreadyExists => false, - Err(error) => panic!("cannot create {}: {error}", root.display()), + Err(error) => panic!( + "cannot create the {name} sandbox {}: {error}", + root.display() + ), }) .expect("a directory name is free"); Self { diff --git a/tests/runtime-adapter/tests/vm_clone.rs b/tests/runtime-adapter/tests/vm_clone.rs index 654fad486..cbc20e665 100644 --- a/tests/runtime-adapter/tests/vm_clone.rs +++ b/tests/runtime-adapter/tests/vm_clone.rs @@ -2321,7 +2321,7 @@ impl CloneControl for Early { .send(&self.early, Some(deadline)) .map_err(io::Error::other)?; let answer = session.receive(Some(deadline)); - self.exited = self.vmm.wait(deadline)?; + self.exited = self.vmm.wait(deadline)?.is_some(); Err(match answer { Ok(message) => io::Error::other(format!("the clone answered {message:?}")), Err(error) => io::Error::other(error), @@ -3910,7 +3910,9 @@ fn a_host_program_of_another_runtime_ends_the_clone_through_its_verified_identit assert!(!ended.contains_key("epoch"), "{ended:?}"); assert!(sandbox.stderr().contains("differs in initramfs")); assert!( - vmm.wait(Instant::now() + Duration::from_secs(10)).unwrap(), + vmm.wait(Instant::now() + Duration::from_secs(10)) + .unwrap() + .is_some(), "the clone's VMM still runs" ); assert!(sandbox.record().is_none()); @@ -3948,7 +3950,9 @@ fn a_clone_whose_vm_exits_once_a_stop_found_it_fails_before_its_lease_is_release assert_eq!(sandbox.paused(&mut stopping, &pause), "found"); vmm.kill().unwrap(); assert!( - vmm.wait(Instant::now() + Duration::from_secs(10)).unwrap(), + vmm.wait(Instant::now() + Duration::from_secs(10)) + .unwrap() + .is_some(), "the clone's VMM still runs" ); fs::remove_file(&pause).unwrap(); @@ -5313,7 +5317,9 @@ fn a_run_whose_clone_s_vm_exits_during_its_execution_ends_with_the_clone_s_failu let vmm = sandbox.vmm().expect("the clone's VMM runs"); vmm.kill().unwrap(); assert!( - vmm.wait(Instant::now() + Duration::from_secs(10)).unwrap(), + vmm.wait(Instant::now() + Duration::from_secs(10)) + .unwrap() + .is_some(), "the clone's VMM still runs" );