diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 69f274959..31b418906 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -111,6 +111,7 @@ changes only when the module's purpose does. | [`nvx::protocol::cmdline`](#nvxprotocolcmdline) | The kernel command line as Linux and the guest's init read it, with the virtio-mmio devices and the guest's DNS server that it names | | [`nvx::protocol::export`](#nvxprotocolexport) | The boot tokens of the host export, the virtio-fs device from which a guest maps host paths, and the guest's parser of them | | [`nvx::protocol::nvxs`](#nvxprotocolnvxs) | The NVXS session, independent of any application protocol: its records and the state machines of its host and guest roles | +| [`nvx::protocol::restore_ready`](#nvxprotocolrestore_ready) | The exact parser of OpenVMM's restore-ready event, including its terminating newline | | [`nvx::protocol::root`](#nvxprotocolroot) | The boot tokens that tell the guest which image its layered root assembles and where the root keeps its upper | | [`nvx::protocol::share`](#nvxprotocolshare) | The guest targets that a sandbox's live shares may not take, which the host and the guest both refuse | | [`nvx::protocol::status`](#nvxprotocolstatus) | The exit statuses with which a clone's guest ends its VM once it fails fast, and what each means | @@ -124,6 +125,7 @@ changes only when the module's purpose does. | [`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) | The directory of a VM, private to the current user, with the fixed names of the files and endpoints of its launches | | [`nvx::host::config`](#nvxhostconfig) | `VmConfig`, a VM's configuration as one value, and `CaptureConfig`, what a capture adds and the identity of its snapshots, checked without reading the host | +| [`nvx::host::restore`](#nvxhostrestore) | The restore-ready listener, with native ownership transfer for a caller's own I/O | | [`nvx::host::artifacts`](#nvxhostartifacts) | Locating the OpenVMM executable and the guest artifacts that a VM runs, and their runtime identity | | [`nvx::host::launch`](#nvxhostlaunch) | `LaunchSpec` and `VmmLaunch`: the OpenVMM command line of a managed microVM, its checks, and the inputs that it shares with an adapter's launch | | [`nvx::host::network`](#nvxhostnetwork) | The one lowering of a network posture into OpenVMM options, for the CLI and external orchestrators | @@ -234,6 +236,12 @@ number, and the guest's receive window and credits, without I/O. An application message may span DATA records, and the application's own framing reassembles it. +### `nvx::protocol::restore_ready` + +`parse_ready_event` accepts only `OPENVMM_RESTORE_READY_V1\n`, byte for byte, +including the newline. Truncated events, another version, CRLF endings, and +trailing bytes fail with `ProtocolError`. + ### `nvx::protocol::root` The boot tokens of a layered root, which the host renders and guests parse by @@ -524,6 +532,36 @@ boot's command line. No path, launch option, view, network policy, or anything of the VM's root is part of it, nor the VM's own processors and memory, for which the capture's count. +### `nvx::host::restore` + +`ReadyListener` owns a restore-ready endpoint: a mode-0600 Unix socket in the +private VM directory on Linux, or a fresh first-instance named pipe with an +owner-only protected access control list on Windows. The library's receiver +checks the connecting client's process ID against the held VM process. On +Linux it also checks the socket's original peer through `SO_PEERPIDFD`, so +a reaped client's queued event cannot impersonate a VM that reuses its ID. +This requires Linux 6.5 or a backport of that option; if the host cannot +provide it, receive fails with `ErrorKind::Unavailable`, without a numeric-PID +fallback. An exited held VM is refused too. + +`into_std()` transfers the same nonblocking `UnixListener` on Linux, or the +same overlapped first pipe instance as an `OwnedHandle` on Windows, without +reopening or duplicating it. Transfer precedes acceptance; transferring after +the receiver accepted a client fails with `ErrorKind::Unsupported`. A caller +that drives the native object owns its I/O. On Linux it checks the original +socket peer through `SO_PEERPIDFD`, refuses an exited peer, and matches its +credentials against the held VM process: opening a pidfd from the cached +numeric PID is not authentication. On Windows it checks the named-pipe client +process ID. After matching the peer, it checks `Process::has_exited` and +refuses an exited VM. Listener construction remains internal until restore +launch preparation supplies it. + +The Linux PID-reuse regression runs a separate helper in private PID and mount +namespaces with Bash as its reaper. If the host refuses unprivileged user +namespaces, it uses passwordless `sudo` only for that isolated helper, not the +test suite or build. If neither route works, the test fails; it never changes +host namespace policy or skips the identity assertions. + ### `nvx::host::artifacts` `Artifacts` (also `nvx::Artifacts`): the OpenVMM executable, guest kernel, and diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index 204252140..c917686dd 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -2132,6 +2132,9 @@ field nvx::host::report::Report::category field nvx::host::report::Report::status_code field nvx::host::report::Report::teardown_complete fn nvx::host::report::read +mod nvx::host::restore +struct nvx::host::restore::ReadyListener +method nvx::host::restore::ReadyListener::into_std mod nvx::host::runtime struct nvx::host::runtime::ArtifactIdentity method nvx::host::runtime::ArtifactIdentity::as_str @@ -3555,6 +3558,8 @@ field nvx::protocol::nvxs::SessionError::Window::outstanding field nvx::protocol::nvxs::SessionError::Window::window fn nvx::protocol::nvxs::encode_record fn nvx::protocol::parse_host_mapping_count +mod nvx::protocol::restore_ready +fn nvx::protocol::restore_ready::parse_ready_event mod nvx::protocol::root const nvx::protocol::root::DISTRO_MMIO_BASE enum nvx::protocol::root::ImageFormat diff --git a/nvx-rs/public-api/windows.txt b/nvx-rs/public-api/windows.txt index 6d9434f1e..5396206e5 100644 --- a/nvx-rs/public-api/windows.txt +++ b/nvx-rs/public-api/windows.txt @@ -1058,6 +1058,9 @@ field nvx::host::report::Report::category field nvx::host::report::Report::status_code field nvx::host::report::Report::teardown_complete fn nvx::host::report::read +mod nvx::host::restore +struct nvx::host::restore::ReadyListener +method nvx::host::restore::ReadyListener::into_std mod nvx::host::runtime struct nvx::host::runtime::ArtifactIdentity method nvx::host::runtime::ArtifactIdentity::as_str @@ -2480,6 +2483,8 @@ field nvx::protocol::nvxs::SessionError::Window::outstanding field nvx::protocol::nvxs::SessionError::Window::window fn nvx::protocol::nvxs::encode_record fn nvx::protocol::parse_host_mapping_count +mod nvx::protocol::restore_ready +fn nvx::protocol::restore_ready::parse_ready_event mod nvx::protocol::root const nvx::protocol::root::DISTRO_MMIO_BASE enum nvx::protocol::root::ImageFormat diff --git a/nvx-rs/src/host/mod.rs b/nvx-rs/src/host/mod.rs index 365e62d05..d18162cd4 100644 --- a/nvx-rs/src/host/mod.rs +++ b/nvx-rs/src/host/mod.rs @@ -52,6 +52,8 @@ //! opened, which it inherits through the [`supervisor`]'s allowlist. //! - [`vm`] holds a caller-owned VM directory, private to the current user, the fixed names of //! its files and endpoints, and its control credential, which the library never writes. +//! - [`restore`] listens for OpenVMM's restore-ready event and verifies its client against the +//! held VM process. //! - `cli` (feature `cli`) is the `nvx-rs` command-line interface, which runs microVMs and //! sandboxes, looks up and downloads artifacts, and compiles egress policies. //! @@ -85,6 +87,7 @@ pub(crate) mod paths; pub mod platform; pub mod process; pub mod report; +pub mod restore; pub mod runtime; pub mod sandbox; pub mod seal; diff --git a/nvx-rs/src/host/platform/linux/endpoint.rs b/nvx-rs/src/host/platform/linux/endpoint.rs index f0d027671..83850a45d 100644 --- a/nvx-rs/src/host/platform/linux/endpoint.rs +++ b/nvx-rs/src/host/platform/linux/endpoint.rs @@ -3,13 +3,16 @@ use std::fs::{self, OpenOptions, Permissions}; use std::io::{self, Read}; -use std::os::fd::AsRawFd; +use std::os::fd::{AsFd, AsRawFd}; use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; use std::os::unix::net::{UnixListener, UnixStream}; use std::path::Path; use std::thread; use std::time::{Duration, Instant}; +use nix::poll::{PollFd, PollFlags, PollTimeout, poll}; +use nix::sys::socket::{getsockopt, sockopt::PeerPidfd}; + use crate::host::platform::{Transport, TransportHandle, time_left}; use crate::host::sys; @@ -60,6 +63,17 @@ pub(crate) fn listen_ready(socket_path: &Path) -> io::Result<(String, ReadyListe } impl ReadyListener { + /// Transfers the listening socket only while no receive has accepted a client. + pub(crate) fn into_std(self) -> io::Result { + if self.connection.is_some() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "the restore-ready listener already accepted a client", + )); + } + Ok(self.socket) + } + /// Waits at most `timeout` for what the VMM, the process `pid`, sends to the endpoint, and /// appends it to `received`, which then holds at most `limit` bytes. Returns whether more may /// arrive: `false` once the VMM closed its end of the connection. @@ -72,6 +86,30 @@ impl ReadyListener { received: &mut Vec, limit: usize, timeout: Duration, + ) -> io::Result { + self.receive_checked(pid, received, limit, timeout, false) + } + + /// Receives only from the original socket peer while it still runs, never from a cached + /// numeric PID that a later process reused. Requires Linux's `SO_PEERPIDFD`. + pub(crate) fn receive_verified( + &mut self, + pid: u32, + received: &mut Vec, + limit: usize, + timeout: Duration, + ) -> io::Result { + self.receive_checked(pid, received, limit, timeout, true) + } + + /// Shares the byte/deadline receive path, verifying the original peer only when requested. + fn receive_checked( + &mut self, + pid: u32, + received: &mut Vec, + limit: usize, + timeout: Duration, + verify_original_peer: bool, ) -> io::Result { let deadline = Instant::now() + timeout; if self.connection.is_none() { @@ -100,6 +138,9 @@ impl ReadyListener { ), )); } + if verify_original_peer { + check_original_peer(&connection)?; + } connection.set_nonblocking(true)?; self.connection = Some(connection); } @@ -136,6 +177,46 @@ impl ReadyListener { } } +/// The kernel returns a descriptor of the socket's original peer, not a new lookup of its PID. +fn check_original_peer(connection: &UnixStream) -> io::Result<()> { + let peer = match getsockopt(connection, PeerPidfd) { + Err( + sys::Errno::ENOPROTOOPT + | sys::Errno::EOPNOTSUPP + | sys::Errno::ENOSYS + | sys::Errno::EPERM + | sys::Errno::EACCES, + ) => { + return Err(io::Error::new( + io::ErrorKind::Unsupported, + "restore-ready peer authentication requires SO_PEERPIDFD", + )); + } + // Before Linux 6.16, a reaped peer yields EINVAL rather than a readable pidfd. + Err(sys::Errno::EINVAL | sys::Errno::ESRCH | sys::Errno::ENODATA) => { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "the original restore-ready socket peer exited", + )); + } + result => result?, + }; + let mut descriptors = [PollFd::new(peer.as_fd(), PollFlags::POLLIN)]; + if poll(&mut descriptors, PollTimeout::ZERO)? == 0 { + return Ok(()); + } + let events = descriptors[0].revents().unwrap_or(PollFlags::empty()); + if events.intersects(PollFlags::POLLIN | PollFlags::POLLHUP) { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "the original restore-ready socket peer exited", + )); + } + Err(io::Error::other(format!( + "the restore-ready peer pidfd reported unexpected events {events:?}" + ))) +} + /// Gives the restore-ready socket at `socket_path`, which the caller bound, the mode 0600, whatever /// the umask: its owner, the user of the clone's VMM, may connect to it, which takes the /// permission to write it, and no one else may. diff --git a/nvx-rs/src/host/platform/windows/endpoint.rs b/nvx-rs/src/host/platform/windows/endpoint.rs index 736721baa..9bafc42a9 100644 --- a/nvx-rs/src/host/platform/windows/endpoint.rs +++ b/nvx-rs/src/host/platform/windows/endpoint.rs @@ -63,36 +63,55 @@ pub(crate) fn listen_ready(_socket_path: &Path) -> io::Result<(String, ReadyList getrandom::fill(&mut bytes).map_err(io::Error::other)?; let suffix: String = bytes.iter().map(|byte| format!("{byte:02x}")).collect(); let endpoint = format!("//./pipe/nvx-restore-ready-{suffix}"); - let name = wide(endpoint.replace('/', "\\")); - let pipe = - with_owner_only_attributes(&CurrentUser::get()?, FILE_ALL_ACCESS, 0, |attributes| { - // SAFETY: the name is NUL-terminated, and the attributes hold a valid descriptor. - let handle = unsafe { - CreateNamedPipeW( - name.as_ptr(), - PIPE_ACCESS_INBOUND | FILE_FLAG_FIRST_PIPE_INSTANCE | FILE_FLAG_OVERLAPPED, - PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, - 1, - 0, - READY_BUFFER_LEN, - 0, - attributes, - ) - }; - owned(handle).ok_or_else(io::Error::last_os_error) - })??; - // SAFETY: creates an unnamed manual-reset event with default security. - let event = owned(unsafe { CreateEventW(ptr::null(), 1, 0, ptr::null()) }) - .ok_or_else(io::Error::last_os_error)?; - let listener = ReadyListener { - pipe, - event, - connected: false, - }; + let listener = ReadyListener::bind(&endpoint)?; Ok((endpoint, listener)) } impl ReadyListener { + /// Binds the first instance of the fresh endpoint that the VM directory selected. + pub(crate) fn bind(endpoint: &str) -> io::Result { + let name = wide(endpoint.replace('/', "\\")); + let pipe = + with_owner_only_attributes(&CurrentUser::get()?, FILE_ALL_ACCESS, 0, |attributes| { + // SAFETY: the name is NUL-terminated, and the attributes hold a valid descriptor. + let handle = unsafe { + CreateNamedPipeW( + name.as_ptr(), + PIPE_ACCESS_INBOUND | FILE_FLAG_FIRST_PIPE_INSTANCE | FILE_FLAG_OVERLAPPED, + PIPE_TYPE_BYTE + | PIPE_READMODE_BYTE + | PIPE_WAIT + | PIPE_REJECT_REMOTE_CLIENTS, + 1, + 0, + READY_BUFFER_LEN, + 0, + attributes, + ) + }; + owned(handle).ok_or_else(io::Error::last_os_error) + })??; + // SAFETY: creates an unnamed manual-reset event with default security. + let event = owned(unsafe { CreateEventW(ptr::null(), 1, 0, ptr::null()) }) + .ok_or_else(io::Error::last_os_error)?; + Ok(Self { + pipe, + event, + connected: false, + }) + } + + /// Transfers the overlapped first pipe instance only before acceptance. + pub(crate) fn into_std(self) -> io::Result { + if self.connected { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "the restore-ready listener already accepted a client", + )); + } + Ok(self.pipe) + } + /// Waits at most `timeout` for what the VMM, the process `pid`, sends to the endpoint, and /// appends it to `received`, which then holds at most `limit` bytes. Returns whether more may /// arrive: `false` once the VMM closed its end of the pipe. diff --git a/nvx-rs/src/host/restore.rs b/nvx-rs/src/host/restore.rs new file mode 100644 index 000000000..73c898ede --- /dev/null +++ b/nvx-rs/src/host/restore.rs @@ -0,0 +1,181 @@ +//! A restore-ready endpoint whose client must be the held VM process. +//! +//! A launch binds before spawning the VM and receives through its verified [`Process`]. +//! Parse the completed bytes with +//! [`parse_ready_event`](crate::protocol::restore_ready::parse_ready_event). +//! External I/O users take the native listener with `ReadyListener::into_std` on Linux or +//! Windows and check its client's process identity before reading the event. + +use std::io; +#[cfg(not(windows))] +use std::path::Path; +use std::time::Duration; + +use super::platform; +use super::process::{Process, ProcessRef}; +use super::vm::VmDir; +use crate::{Error, Result}; + +/// The listener of one restore, bound before its VM starts. +/// +/// The first receive fixes the expected process identity. Every receive borrows its held +/// [`Process`], never merely a PID, and refuses an exited process. Dropping the listener closes +/// its handles; on Linux, the socket's directory entry remains for the caller to remove. +#[derive(Debug)] +pub struct ReadyListener { + /// The endpoint passed to OpenVMM's `--restore-ready-path`. + endpoint: String, + /// The existing platform listener and its accepted connection, if any. + listener: platform::ReadyListener, + /// The identity supplied by the first receive, which later receives may not change. + process: Option, +} + +impl ReadyListener { + /// Binds `restore-ready.sock` in the private VM directory on Linux, or a fresh, first-instance + /// owner-only pipe on Windows. The Linux socket has mode 0600, whatever the umask. + /// + /// Replaces a previous Linux endpoint. Fails with [`ErrorKind::Io`](crate::ErrorKind::Io) + /// if binding fails or the directory is no longer private, and with + /// [`ErrorKind::Unavailable`](crate::ErrorKind::Unavailable) on unsupported hosts. + #[cfg_attr( + not(all(test, any(target_os = "linux", windows))), + expect(dead_code, reason = "SL2.16 binds a restore's endpoint (#1103)") + )] + pub(crate) fn bind(directory: &VmDir) -> Result { + let directory = VmDir::open(directory.path())?; + let endpoint = directory.restore_ready_endpoint()?; + #[cfg(not(windows))] + let (_, listener) = platform::listen_ready(Path::new(&endpoint)).map_err(endpoint_error)?; + #[cfg(windows)] + let listener = platform::ReadyListener::bind(&endpoint).map_err(endpoint_error)?; + Ok(Self { + endpoint, + listener, + process: None, + }) + } + + /// Returns the endpoint for OpenVMM's `--restore-ready-path`. + #[cfg_attr( + not(all(test, any(target_os = "linux", windows))), + expect(dead_code, reason = "SL2.16 renders the restore endpoint (#1103)") + )] + pub(crate) fn endpoint(&self) -> &str { + &self.endpoint + } + + /// Appends at most `limit` total bytes to `received`, waiting at most `timeout`. + /// + /// Returns `false` at end of stream, or `true` when more may arrive, including after a + /// timeout without data. The first client must match `process`; another client, or a later + /// receive with a different process identity, fails with + /// [`ErrorKind::Protocol`](crate::ErrorKind::Protocol). An exited held process fails with + /// [`ErrorKind::Exited`](crate::ErrorKind::Exited), even if its PID has been reused. + /// Linux authenticates the original socket peer through `SO_PEERPIDFD` (Linux 6.5 or a + /// backport), failing with [`ErrorKind::Unavailable`](crate::ErrorKind::Unavailable) + /// if the host cannot provide it, rather than falling back to a numeric PID. + #[cfg_attr( + not(all(test, any(target_os = "linux", windows))), + expect( + dead_code, + reason = "SL4.3's blocking driver receives the restore-ready event" + ) + )] + pub(crate) fn receive( + &mut self, + process: &Process, + received: &mut Vec, + limit: usize, + timeout: Duration, + ) -> Result { + if self + .process + .as_ref() + .is_some_and(|expected| expected != process.reference()) + { + return Err(Error::protocol( + "the restore-ready process identity changed", + )); + } + check_running(process)?; + self.process = Some(process.reference().clone()); + #[cfg(target_os = "linux")] + let open = + self.listener + .receive_verified(process.reference().pid, received, limit, timeout); + #[cfg(not(target_os = "linux"))] + let open = self + .listener + .receive(process.reference().pid, received, limit, timeout); + // A matching numeric PID is not enough if the held process exited during acceptance. + check_running(process)?; + open.map_err(listener_error) + } + + /// Transfers the same nonblocking Unix listener, without reopening or duplicating it. + /// + /// Call before a receive accepts a client; otherwise fails with + /// [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported). The caller must verify each + /// accepted stream's original peer through `SO_PEERPIDFD`, refuse an exited peer, and match + /// its credentials against the held VM process before reading its event. Opening a pidfd + /// from the credentials' numeric PID does not authenticate an already-reaped peer. + /// After matching the peer, check [`Process::has_exited`] and refuse an exited VM. + #[cfg(target_os = "linux")] + pub fn into_std(self) -> Result { + self.listener.into_std().map_err(transfer_error) + } + + /// Transfers the same overlapped first pipe instance, without reopening or duplicating it. + /// + /// Call before a receive accepts a client; otherwise fails with + /// [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported). The caller completes + /// `ConnectNamedPipe`, checks its client process ID against the held VM process before + /// reading its event, and owns completion or cancellation of its overlapped operations. + /// After matching the PID, check [`Process::has_exited`] and refuse an exited process. + #[cfg(windows)] + pub fn into_std(self) -> Result { + self.listener.into_std().map_err(transfer_error) + } +} + +/// Refuses a stale process handle, so a reused PID cannot pass a peer check. +fn check_running(process: &Process) -> Result<()> { + if process + .has_exited() + .map_err(|error| Error::io("cannot check the restore-ready process").with_source(error))? + { + return Err(Error::exited("the restore-ready VM process exited")); + } + Ok(()) +} + +/// Keeps the existing clone endpoint's peer mismatch distinct from host I/O failures. +fn listener_error(error: io::Error) -> Error { + if error.kind() == io::ErrorKind::PermissionDenied { + Error::protocol("another process connected to the restore-ready endpoint") + .with_source(error) + } else { + endpoint_error(error) + } +} + +/// Reports endpoint I/O separately from an unsupported host. +fn endpoint_error(error: io::Error) -> Error { + let message = "cannot use the restore-ready endpoint"; + if error.kind() == io::ErrorKind::Unsupported { + Error::unavailable(message).with_source(error) + } else { + Error::io(message).with_source(error) + } +} + +/// Reports an accepted listener that can no longer be handed over as a fresh native listener. +#[cfg(any(target_os = "linux", windows))] +fn transfer_error(error: io::Error) -> Error { + Error::unsupported("cannot transfer a restore-ready listener after acceptance") + .with_source(error) +} + +#[cfg(test)] +mod tests; diff --git a/nvx-rs/src/host/restore/tests.rs b/nvx-rs/src/host/restore/tests.rs new file mode 100644 index 000000000..782dc3567 --- /dev/null +++ b/nvx-rs/src/host/restore/tests.rs @@ -0,0 +1,563 @@ +#![cfg(any(target_os = "linux", windows))] +#![cfg_attr(windows, allow(unsafe_code))] + +use std::io::{BufRead, BufReader, Write}; +use std::process::{Child, Command, Stdio}; +use std::time::Instant; + +use super::*; +use crate::ErrorKind; +use crate::host::platform::inherit::spawn_command; +use crate::host::process::BootId; +use crate::host::vm::create_private_dir; +use crate::protocol::restore_ready::parse_ready_event; + +fn vm_directory() -> (tempfile::TempDir, VmDir) { + let temporary = tempfile::tempdir().unwrap(); + let path = temporary.path().join("vm"); + create_private_dir(&path).unwrap(); + let directory = VmDir::open(&path).unwrap(); + (temporary, directory) +} + +fn held_process(pid: u32) -> Process { + Process::open(&ProcessRef { + pid, + start_time: platform::process_start_time(pid).unwrap().unwrap(), + boot: BootId::current().unwrap(), + }) + .unwrap() + .unwrap() +} + +/// A separate process, held before it may connect, and alive until the test ends it. +struct Client { + child: Child, + process: Process, +} + +impl Client { + fn start(endpoint: &str, data: &[u8], close: bool) -> Self { + #[cfg(target_os = "linux")] + let mut command = { + let mut command = Command::new("python3"); + command.args([ + "-c", + "import os,socket,sys\n\ + sys.stdin.buffer.read(1)\n\ + peer=socket.socket(socket.AF_UNIX)\n\ + peer.connect(os.environ['NVX_TEST_READY_ENDPOINT'])\n\ + peer.sendall(os.environ['NVX_TEST_READY_DATA'].encode('ascii'))\n\ + if os.environ['NVX_TEST_READY_CLOSE']=='1': peer.close()\n\ + print('sent',flush=True)\n\ + sys.stdin.buffer.read(1)\n\ + peer.close()\n", + ]); + command + }; + #[cfg(windows)] + let mut command = { + let mut command = Command::new("powershell"); + command.args([ + "-NoProfile", + "-NonInteractive", + "-Command", + "$ErrorActionPreference='Stop'; \ + $inputStream=[Console]::OpenStandardInput(); $null=$inputStream.ReadByte(); \ + $name=$env:NVX_TEST_READY_ENDPOINT.Substring('//./pipe/'.Length); \ + $peer=[IO.Pipes.NamedPipeClientStream]::new('.', $name, [IO.Pipes.PipeDirection]::Out); \ + $peer.Connect(10000); \ + $data=[Text.Encoding]::ASCII.GetBytes([string]$env:NVX_TEST_READY_DATA); \ + $peer.Write($data,0,$data.Length); $peer.Flush(); \ + if ($env:NVX_TEST_READY_CLOSE -eq '1') { $peer.Dispose() }; \ + [Console]::Out.WriteLine('sent'); [Console]::Out.Flush(); \ + $null=$inputStream.ReadByte(); $peer.Dispose()", + ]); + command + }; + command + .env("NVX_TEST_READY_ENDPOINT", endpoint) + .env("NVX_TEST_READY_DATA", std::str::from_utf8(data).unwrap()) + .env("NVX_TEST_READY_CLOSE", if close { "1" } else { "0" }) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()); + let child = spawn_command(&mut command).unwrap(); + let process = held_process(child.id()); + let mut client = Self { child, process }; + client + .child + .stdin + .as_mut() + .unwrap() + .write_all(b"g") + .unwrap(); + let mut line = String::new(); + BufReader::new(client.child.stdout.as_mut().unwrap()) + .read_line(&mut line) + .unwrap(); + assert_eq!(line.trim(), "sent"); + client + } + + fn stop(&mut self) { + if self.child.try_wait().unwrap().is_none() { + self.child.kill().unwrap(); + } + self.child.wait().unwrap(); + } +} + +impl Drop for Client { + fn drop(&mut self) { + self.stop(); + } +} + +#[test] +fn the_held_vm_process_is_accepted_even_after_its_client_closed() { + let (_temporary, directory) = vm_directory(); + for data in [&b""[..], b"OPENVMM_RESTORE_READY_V1\n"] { + let mut listener = ReadyListener::bind(&directory).unwrap(); + let client = Client::start(listener.endpoint(), data, true); + let mut received = Vec::new(); + let deadline = Instant::now() + Duration::from_secs(10); + while listener + .receive( + &client.process, + &mut received, + 64, + Duration::from_millis(50), + ) + .unwrap() + { + assert!(Instant::now() < deadline, "the endpoint did not close"); + } + assert_eq!(received, data); + assert_eq!( + listener.into_std().unwrap_err().kind(), + ErrorKind::Unsupported + ); + } +} + +#[test] +fn a_different_real_process_cannot_supply_the_event() { + let (_temporary, directory) = vm_directory(); + let mut listener = ReadyListener::bind(&directory).unwrap(); + let expected = held_process(std::process::id()); + let client = Client::start(listener.endpoint(), b"OPENVMM_RESTORE_READY_V1\n", false); + assert_ne!(client.process.reference().pid, expected.reference().pid); + let mut received = Vec::new(); + let error = listener + .receive(&expected, &mut received, 64, Duration::from_secs(2)) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Protocol); + assert!(received.is_empty()); +} + +#[test] +fn an_exited_held_process_cannot_supply_even_an_already_queued_event() { + let (_temporary, directory) = vm_directory(); + let mut listener = ReadyListener::bind(&directory).unwrap(); + let mut client = Client::start(listener.endpoint(), b"OPENVMM_RESTORE_READY_V1\n", true); + client.stop(); + assert!(client.process.has_exited().unwrap()); + let mut received = Vec::new(); + let error = listener + .receive(&client.process, &mut received, 64, Duration::from_secs(2)) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Exited); + assert!(received.is_empty()); +} + +#[cfg(target_os = "linux")] +#[test] +fn a_queued_peer_cannot_be_the_vm_that_reuses_its_pid() { + use std::fs; + use std::os::unix::fs::MetadataExt; + use std::thread; + + let namespace = fs::read_link("/proc/self/ns/pid").unwrap(); + let mount = fs::read_link("/proc/self/ns/mnt").unwrap(); + let outer = "NVX_TEST_READY_OUTER_PID_NAMESPACE"; + let outer_mount = "NVX_TEST_READY_OUTER_MOUNT_NAMESPACE"; + let verified = "NVX-READY-PID-REUSE-VERIFIED"; + if std::env::var_os(outer).is_none() { + let mut diagnostics = String::new(); + for privileged in [false, true] { + let route = if privileged { "sudo" } else { "userns" }; + let mut command = Command::new(if privileged { "sudo" } else { "unshare" }); + if privileged { + command.args([ + "-n", + "--preserve-env=NVX_TEST_READY_OUTER_PID_NAMESPACE,NVX_TEST_READY_OUTER_MOUNT_NAMESPACE,TMPDIR,TEMP,TMP", + "/usr/bin/unshare", + ]); + } else { + command.args(["--user", "--map-root-user"]); + } + command + .args([ + "--mount", + "--propagation", + "private", + "--pid", + "--fork", + "--mount-proc", + "/bin/bash", + "-c", + "\"$@\"; result=$?; exit \"$result\"", + "--", + ]) + .arg(std::env::current_exe().unwrap()) + .args([ + "--exact", + "host::restore::tests::a_queued_peer_cannot_be_the_vm_that_reuses_its_pid", + "--nocapture", + "--test-threads=1", + ]) + .env(outer, &namespace) + .env(outer_mount, &mount) + .env("LC_ALL", "C") + .env_remove("BASH_ENV") + .env_remove("ENV") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let output = spawn_command(&mut command) + .and_then(Child::wait_with_output) + .unwrap_or_else(|error| panic!("{diagnostics}\n{route} helper: {error}")); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + diagnostics.push_str(&format!("{route}: {}\n{stdout}\n{stderr}\n", output.status)); + if output.status.success() { + assert!(stdout.contains(verified), "{diagnostics}"); + println!("{route}: {stdout}"); + return; + } + assert!( + !privileged + && output.status.code() == Some(1) + && stderr.contains("unshare:") + && (stderr.contains("Operation not permitted") + || stderr.contains("Permission denied")), + "{diagnostics}" + ); + } + unreachable!("both namespace setup failures must fail the test"); + } + // Both namespaces must be private before any PID allocation, reaping, or signal in the test. + assert_ne!(namespace.as_os_str(), std::env::var_os(outer).unwrap()); + assert_ne!(mount.as_os_str(), std::env::var_os(outer_mount).unwrap()); + assert_eq!(nix::unistd::getppid().as_raw(), 1); + assert_eq!(fs::read_to_string("/proc/1/comm").unwrap().trim(), "bash"); + let (_temporary, directory) = vm_directory(); + let mut listener = ReadyListener::bind(&directory).unwrap(); + let mut peer = Client::start(listener.endpoint(), b"OPENVMM_RESTORE_READY_V1\n", true); + let recycled = peer.child.id(); + let uid = fs::metadata(format!("/proc/{recycled}")).unwrap().uid(); + assert_eq!(uid, nix::unistd::geteuid().as_raw()); + peer.stop(); + assert!(peer.process.has_exited().unwrap()); + thread::sleep(Duration::from_millis(20)); + fs::write("/proc/sys/kernel/ns_last_pid", (recycled - 1).to_string()).unwrap(); + let mut command = Command::new("python3"); + command + .args([ + "-c", + "import sys\nprint('running',flush=True)\nsys.stdin.buffer.read(1)\n", + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()); + let child = spawn_command(&mut command).unwrap(); + assert_eq!(child.id(), recycled); + let process = held_process(child.id()); + let mut vm = Client { child, process }; + assert_eq!( + fs::metadata(format!("/proc/{recycled}")).unwrap().uid(), + uid + ); + let mut line = String::new(); + BufReader::new(vm.child.stdout.as_mut().unwrap()) + .read_line(&mut line) + .unwrap(); + assert_eq!(line.trim(), "running"); + assert!(!vm.process.has_exited().unwrap()); + let mut received = Vec::new(); + let error = listener + .receive(&vm.process, &mut received, 64, Duration::from_secs(2)) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Protocol); + assert!(received.is_empty()); + println!( + "{verified} pid={recycled} uid={uid} reaper=bash pidns={} mountns={}", + namespace.display(), + mount.display() + ); +} + +#[cfg(target_os = "linux")] +#[test] +fn the_receiver_fails_closed_without_original_peer_pidfds() { + let child = "NVX_TEST_READY_PIDFD_DENIAL"; + if std::env::var_os(child).is_none() { + for errno in [nix::libc::ENOPROTOOPT, nix::libc::EPERM, nix::libc::EINVAL] { + let mut command = Command::new(std::env::current_exe().unwrap()); + command + .args([ + "--exact", + "host::restore::tests::the_receiver_fails_closed_without_original_peer_pidfds", + "--nocapture", + ]) + .env(child, errno.to_string()) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + crate::host::sys::tests::refuse_peer_pidfd_on_spawn(&mut command, errno); + let output = spawn_command(&mut command) + .unwrap() + .wait_with_output() + .unwrap(); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + return; + } + let (_temporary, directory) = vm_directory(); + let mut listener = ReadyListener::bind(&directory).unwrap(); + let client = Client::start(listener.endpoint(), b"OPENVMM_RESTORE_READY_V1\n", true); + let mut received = Vec::new(); + let error = listener + .receive(&client.process, &mut received, 64, Duration::from_secs(2)) + .unwrap_err(); + let expected = if std::env::var(child).unwrap() == nix::libc::EINVAL.to_string() { + ErrorKind::Protocol + } else { + ErrorKind::Unavailable + }; + assert_eq!(error.kind(), expected); + assert!(received.is_empty()); +} + +#[test] +fn the_expected_process_cannot_change_after_acceptance() { + let (_temporary, directory) = vm_directory(); + let mut listener = ReadyListener::bind(&directory).unwrap(); + let client = Client::start(listener.endpoint(), b"OPENVMM_RESTORE_READY_V1\n", false); + let mut received = Vec::new(); + listener + .receive(&client.process, &mut received, 64, Duration::from_secs(2)) + .unwrap(); + parse_ready_event(&received).unwrap(); + let error = listener + .receive( + &held_process(std::process::id()), + &mut received, + 64, + Duration::from_secs(2), + ) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Protocol); +} + +#[test] +fn a_native_listener_transfer_keeps_the_endpoint_usable_after_an_empty_receive() { + let (_temporary, directory) = vm_directory(); + let mut listener = ReadyListener::bind(&directory).unwrap(); + let endpoint = listener.endpoint().to_owned(); + assert!( + listener + .receive( + &held_process(std::process::id()), + &mut Vec::new(), + 64, + Duration::from_millis(1), + ) + .unwrap() + ); + let native = listener.into_std().unwrap(); + let client = Client::start(&endpoint, b"OPENVMM_RESTORE_READY_V1\n", false); + #[cfg(target_os = "linux")] + let data = { + use std::io::Read; + + assert_eq!( + native.local_addr().unwrap().as_pathname(), + Some(Path::new(&endpoint)) + ); + native.set_nonblocking(false).unwrap(); + let (mut peer, _) = native.accept().unwrap(); + assert_eq!( + crate::host::sys::peer_pid(&peer).unwrap(), + client.process.reference().pid + ); + peer.set_read_timeout(Some(Duration::from_secs(2))).unwrap(); + let mut data = vec![0; b"OPENVMM_RESTORE_READY_V1\n".len()]; + peer.read_exact(&mut data).unwrap(); + data + }; + #[cfg(windows)] + let data = read_native_pipe(native, &client.process); + parse_ready_event(&data).unwrap(); +} + +#[cfg(target_os = "linux")] +#[test] +fn the_socket_is_owner_only_even_when_the_umask_clears_owner_write() { + use std::fs; + use std::os::unix::fs::PermissionsExt; + use std::os::unix::net::UnixStream; + use std::thread; + + let (_temporary, directory) = vm_directory(); + let listener = thread::scope(|scope| { + scope + .spawn(|| { + nix::sched::unshare(nix::sched::CloneFlags::CLONE_FS).unwrap(); + nix::sys::stat::umask(nix::sys::stat::Mode::from_bits_truncate(0o277)); + ReadyListener::bind(&directory).unwrap() + }) + .join() + .unwrap() + }); + let path = directory.path().join("restore-ready.sock"); + assert_eq!( + fs::symlink_metadata(path).unwrap().permissions().mode() & 0o7777, + 0o600 + ); + UnixStream::connect(listener.endpoint()).unwrap(); +} + +#[cfg(windows)] +#[test] +fn the_pipe_is_a_fresh_owner_only_protected_first_instance() { + use std::os::windows::io::AsRawHandle; + use std::ptr; + + use windows_sys::Win32::Foundation::{ERROR_ACCESS_DENIED, ERROR_PIPE_BUSY, LocalFree}; + use windows_sys::Win32::Security::Authorization::{ + ConvertSecurityDescriptorToStringSecurityDescriptorW, GetSecurityInfo, SDDL_REVISION_1, + SE_FILE_OBJECT, + }; + use windows_sys::Win32::Security::{DACL_SECURITY_INFORMATION, OWNER_SECURITY_INFORMATION}; + + let (_temporary, directory) = vm_directory(); + let listener = ReadyListener::bind(&directory).unwrap(); + let endpoint = listener.endpoint().to_owned(); + let second = ReadyListener::bind(&directory).unwrap(); + assert_ne!(endpoint, second.endpoint()); + let native = listener.into_std().unwrap(); + let error = platform::ReadyListener::bind(&endpoint).unwrap_err(); + // The single-instance limit may reject the competing bind before the first-instance flag. + assert!(matches!( + error.raw_os_error().map(|code| code as u32), + Some(ERROR_ACCESS_DENIED | ERROR_PIPE_BUSY) + )); + let mut descriptor = ptr::null_mut(); + let information = OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION; + // SAFETY: the live pipe handle and descriptor output pointer are valid. + let error = unsafe { + GetSecurityInfo( + native.as_raw_handle(), + SE_FILE_OBJECT, + information, + ptr::null_mut(), + ptr::null_mut(), + ptr::null_mut(), + ptr::null_mut(), + &mut descriptor, + ) + }; + assert_eq!(error, 0); + let mut text = ptr::null_mut(); + let mut length = 0; + // SAFETY: the descriptor belongs to this test, and the conversion's output pointers are valid. + assert_ne!( + unsafe { + ConvertSecurityDescriptorToStringSecurityDescriptorW( + descriptor, + SDDL_REVISION_1, + information, + &mut text, + &mut length, + ) + }, + 0 + ); + // SAFETY: the conversion returned `length` UTF-16 units, including its terminating NUL. + let access = + String::from_utf16(unsafe { std::slice::from_raw_parts(text, length as usize - 1) }) + .unwrap(); + // SAFETY: each function allocated the corresponding buffer with LocalAlloc. + unsafe { + LocalFree(text.cast()); + LocalFree(descriptor); + } + let sid = platform::tests::user_sid(); + assert_eq!(access, format!("O:{sid}D:P(A;;FA;;;{sid})")); +} + +#[cfg(windows)] +fn read_native_pipe(pipe: std::os::windows::io::OwnedHandle, process: &Process) -> Vec { + use std::os::windows::io::AsRawHandle; + use std::ptr; + + use windows_sys::Win32::Foundation::{ERROR_IO_PENDING, ERROR_PIPE_CONNECTED}; + use windows_sys::Win32::Storage::FileSystem::ReadFile; + use windows_sys::Win32::System::IO::{GetOverlappedResult, OVERLAPPED}; + use windows_sys::Win32::System::Pipes::{ConnectNamedPipe, GetNamedPipeClientProcessId}; + + let raw = pipe.as_raw_handle(); + let mut operation: OVERLAPPED = unsafe { std::mem::zeroed() }; + let mut transferred = 0; + // SAFETY: the operation and live pipe outlast the wait below. + if unsafe { ConnectNamedPipe(raw, &mut operation) } == 0 { + let error = io::Error::last_os_error().raw_os_error().unwrap() as u32; + if error != ERROR_PIPE_CONNECTED { + assert_eq!(error, ERROR_IO_PENDING); + // SAFETY: waits for the pending operation before its storage is reused. + assert_ne!( + unsafe { GetOverlappedResult(raw, &operation, &mut transferred, 1) }, + 0 + ); + } + } + let mut client = 0; + // SAFETY: the handle is a connected pipe server and the output pointer is valid. + assert_ne!(unsafe { GetNamedPipeClientProcessId(raw, &mut client) }, 0); + assert_eq!(client, process.reference().pid); + assert!(!process.has_exited().unwrap()); + let mut data = vec![0; b"OPENVMM_RESTORE_READY_V1\n".len()]; + let mut used = 0; + while used < data.len() { + operation = unsafe { std::mem::zeroed() }; + // SAFETY: the buffer, operation and pipe remain alive until GetOverlappedResult returns. + if unsafe { + ReadFile( + raw, + data[used..].as_mut_ptr(), + (data.len() - used) as u32, + ptr::null_mut(), + &mut operation, + ) + } == 0 + { + assert_eq!( + io::Error::last_os_error().raw_os_error(), + Some(ERROR_IO_PENDING as i32) + ); + } + // SAFETY: completes the read before its operation or buffer is reused. + assert_ne!( + unsafe { GetOverlappedResult(raw, &operation, &mut transferred, 1) }, + 0 + ); + assert_ne!(transferred, 0); + used += transferred as usize; + } + data +} diff --git a/nvx-rs/src/host/sys/process/tests.rs b/nvx-rs/src/host/sys/process/tests.rs index 3ed4f786a..a9d5b763c 100644 --- a/nvx-rs/src/host/sys/process/tests.rs +++ b/nvx-rs/src/host/sys/process/tests.rs @@ -1,20 +1,20 @@ use super::*; -/// Makes `command` refuse the `umask` system call with the error `errno` in its new process, as -/// a seccomp policy can, through a seccomp filter that allows every other call, before the hooks -/// that a later [`detach`] adds run. The filter needs `no_new_privs`, which the process sets -/// first. -pub(crate) fn refuse_umask_on_spawn(command: &mut Command, errno: i32) { - let statement = |code: u32, if_equal: u8, otherwise: u8, k: u32| libc::sock_filter { +fn statement(code: u32, if_equal: u8, otherwise: u8, k: u32) -> libc::sock_filter { + libc::sock_filter { // Every BPF code fits 16 bits. code: code as u16, jt: if_equal, jf: otherwise, k, - }; + } +} + +/// Makes `command` refuse only `umask` in its new process, before a later [`detach`] hook. +pub(crate) fn refuse_umask_on_spawn(command: &mut Command, errno: i32) { // The filter loads the call's number, the first word of the kernel's `seccomp_data`, and // returns `errno` for `umask`, or allows the call. - let mut filter = [ + let filter = [ statement(libc::BPF_LD | libc::BPF_W | libc::BPF_ABS, 0, 0, 0), // A system call number fits 32 bits. statement( @@ -32,6 +32,34 @@ pub(crate) fn refuse_umask_on_spawn(command: &mut Command, errno: i32) { ), statement(libc::BPF_RET | libc::BPF_K, 0, 0, libc::SECCOMP_RET_ALLOW), ]; + filter_on_spawn(command, filter); +} + +/// Makes only `getsockopt(SOL_SOCKET, SO_PEERPIDFD)` fail in the new process. +pub(crate) fn refuse_peer_pidfd_on_spawn(command: &mut Command, errno: i32) { + let arguments = std::mem::offset_of!(libc::seccomp_data, args) as u32; + let word = if cfg!(target_endian = "big") { 4 } else { 0 }; + let load = libc::BPF_LD | libc::BPF_W | libc::BPF_ABS; + let equal = libc::BPF_JMP | libc::BPF_JEQ | libc::BPF_K; + let filter = [ + statement(load, 0, 0, 0), + statement(equal, 0, 5, libc::SYS_getsockopt as u32), + statement(load, 0, 0, arguments + 8 + word), + statement(equal, 0, 3, libc::SOL_SOCKET as u32), + statement(load, 0, 0, arguments + 16 + word), + statement(equal, 0, 1, libc::SO_PEERPIDFD as u32), + statement( + libc::BPF_RET | libc::BPF_K, + 0, + 0, + libc::SECCOMP_RET_ERRNO | errno as u32, + ), + statement(libc::BPF_RET | libc::BPF_K, 0, 0, libc::SECCOMP_RET_ALLOW), + ]; + filter_on_spawn(command, filter); +} + +fn filter_on_spawn(command: &mut Command, mut filter: [libc::sock_filter; N]) { // SAFETY: the hook makes only async-signal-safe system calls and allocates nothing, as code // that runs between fork and exec must: the filter was built before the fork, and the kernel // copies it before `prctl` returns. diff --git a/nvx-rs/src/host/sys/tests.rs b/nvx-rs/src/host/sys/tests.rs index f3097ac75..521253653 100644 --- a/nvx-rs/src/host/sys/tests.rs +++ b/nvx-rs/src/host/sys/tests.rs @@ -1,4 +1,5 @@ #[cfg(target_os = "linux")] pub(crate) use super::process::tests::{ - clear_close_on_exec, mark_listed_descriptors_on_spawn, refuse_umask_on_spawn, + clear_close_on_exec, mark_listed_descriptors_on_spawn, refuse_peer_pidfd_on_spawn, + refuse_umask_on_spawn, }; diff --git a/nvx-rs/src/host/vm.rs b/nvx-rs/src/host/vm.rs index 09893b92b..daabf3921 100644 --- a/nvx-rs/src/host/vm.rs +++ b/nvx-rs/src/host/vm.rs @@ -114,10 +114,6 @@ impl VmDir { } /// Returns a restore-ready endpoint without listening on it. - #[cfg_attr( - not(test), - expect(dead_code, reason = "SL2.17's ReadyListener calls it (#1099)") - )] pub(crate) fn restore_ready_endpoint(&self) -> Result { self.endpoint(RESTORE_READY, RESTORE_READY_PIPE) } diff --git a/nvx-rs/src/protocol/mod.rs b/nvx-rs/src/protocol/mod.rs index e38062744..9d715f3ae 100644 --- a/nvx-rs/src/protocol/mod.rs +++ b/nvx-rs/src/protocol/mod.rs @@ -47,6 +47,7 @@ mod features; mod maps; pub mod nvxs; mod outer; +pub mod restore_ready; pub mod root; pub mod share; pub mod status; diff --git a/nvx-rs/src/protocol/restore_ready.rs b/nvx-rs/src/protocol/restore_ready.rs new file mode 100644 index 000000000..13b652f87 --- /dev/null +++ b/nvx-rs/src/protocol/restore_ready.rs @@ -0,0 +1,15 @@ +//! OpenVMM's one restore-ready event, including its terminating newline. + +use super::{ProtocolError, violation}; + +/// Accepts exactly `OPENVMM_RESTORE_READY_V1\n`, with no missing or trailing bytes. +pub fn parse_ready_event(bytes: &[u8]) -> Result<(), ProtocolError> { + if bytes == b"OPENVMM_RESTORE_READY_V1\n" { + Ok(()) + } else { + Err(violation("not OpenVMM's restore-ready event")) + } +} + +#[cfg(test)] +mod tests; diff --git a/nvx-rs/src/protocol/restore_ready/tests.rs b/nvx-rs/src/protocol/restore_ready/tests.rs new file mode 100644 index 000000000..382b69899 --- /dev/null +++ b/nvx-rs/src/protocol/restore_ready/tests.rs @@ -0,0 +1,22 @@ +use super::parse_ready_event; + +#[test] +fn only_the_complete_openvmm_event_is_ready() { + let event = b"OPENVMM_RESTORE_READY_V1\n"; + parse_ready_event(event).unwrap(); + for length in 0..event.len() { + assert!(parse_ready_event(&event[..length]).is_err(), "{length}"); + } + for malformed in [ + &b"OPENVMM_RESTORE_READY_V2\n"[..], + b"OPENVMM_RESTORE_READY_V1\r\n", + b"OPENVMM_RESTORE_READY_V1\n\n", + b"OPENVMM_RESTORE_READY_V1\n\0", + b" OPENVMM_RESTORE_READY_V1\n", + b"openvmm_restore_ready_v1\n", + b"OPENVMM_RESTORE_READY_V1\nOPENVMM_RESTORE_READY_V1\n", + b"OPENVMM_RESTORE_READY_V1\xff", + ] { + assert!(parse_ready_event(malformed).is_err(), "{malformed:?}"); + } +} diff --git a/scripts/test-layout-allowlist.json b/scripts/test-layout-allowlist.json index 9577c9ff4..6dd083769 100644 --- a/scripts/test-layout-allowlist.json +++ b/scripts/test-layout-allowlist.json @@ -15,7 +15,8 @@ "nvx-rs/src/host/cli/store/mod.rs": 1, "nvx-rs/src/host/network/address.rs": 2, "nvx-rs/src/host/platform/windows/spawn.rs": 1, + "nvx-rs/src/host/restore.rs": 3, "nvx-rs/src/host/sandbox/plan.rs": 5, - "nvx-rs/src/host/vm.rs": 7 + "nvx-rs/src/host/vm.rs": 6 } } diff --git a/scripts/test_adversarial.py b/scripts/test_adversarial.py index 1b3cb3a2c..674fe88ea 100644 --- a/scripts/test_adversarial.py +++ b/scripts/test_adversarial.py @@ -76,7 +76,11 @@ run_bounded_process, terminate_process_tree, ) -from nvx_tools.benchmark import InteractiveProcess, run_guest_script +from nvx_tools.benchmark import ( + InteractiveProcess, + _wait_for_windows_process, + run_guest_script, +) from nvx_tools.build_constants import ( AlpineBuildConstants, InitramfsBuildConstants, @@ -853,6 +857,7 @@ def spawn_unrelated( def test_contained_guest_runner_timeout_kills_descendants(self) -> None: with tempfile.TemporaryDirectory() as temporary: child_pid_path = Path(temporary) / "child.pid" + ready_path = Path(temporary) / "ready" command = [ sys.executable, "-u", @@ -860,13 +865,61 @@ def test_contained_guest_runner_timeout_kills_descendants(self) -> None: ( "import pathlib,subprocess,sys,time; " "child=subprocess.Popen([sys.executable,'-c'," - "'import time; time.sleep(60)']); " - "pathlib.Path(sys.argv[1]).write_text(str(child.pid)); " - "print('ALPINE-MICROVM-BOOT-OK',flush=True); time.sleep(60)" + "'import os,pathlib,sys,time; " + "path=pathlib.Path(sys.argv[1]); " + 'path.with_suffix(".tmp").write_text(str(os.getpid())); ' + 'path.with_suffix(".tmp").replace(path); ' + "time.sleep(60)',sys.argv[1]]); " + "print('ALPINE-MICROVM-BOOT-OK',flush=True); " + "pathlib.Path(sys.argv[2]).touch(); time.sleep(60)" ), str(child_pid_path), + str(ready_path), ] - with self.assertRaisesRegex(RuntimeError, "did not finish"): + + def start_ready( + command: Sequence[str], + environment: dict[str, str], + *, + contain_process_tree: bool, + separate_stderr: bool, + ) -> InteractiveProcess: + interaction = InteractiveProcess( + command, + environment, + contain_process_tree=contain_process_tree, + separate_stderr=separate_stderr, + ) + ready = False + try: + deadline = time.monotonic() + 5.0 + while not ready_path.exists() or not child_pid_path.exists(): + self.assertIsNone( + interaction.process.poll(), + "fixture exited before readiness", + ) + self.assertLess( + time.monotonic(), deadline, "fixture did not become ready" + ) + time.sleep(0.01) + child_pid = int(child_pid_path.read_text(encoding="utf-8")) + self.assertGreater(child_pid, 0) + self.assertTrue(_process_running(child_pid)) + ready = True + return interaction + finally: + if not ready: + interaction.close() + + with ( + patch( + "nvx_tools.benchmark.InteractiveProcess", side_effect=start_ready + ), + self.assertRaisesRegex( + (TimeoutError, RuntimeError), + r"^guest workload did not finish within 0\.5s(?:\n|$)", + ), + ): run_guest_script( command, "exit\n", @@ -3209,6 +3262,12 @@ def test_close_is_bounded_when_a_surviving_child_holds_executor_stdout(self): check=False, capture_output=True, ) + self.assertTrue( + _wait_for_windows_process( + int(pid), max(0.0, deadline - time.monotonic()) + ), + f"surviving child {pid} did not exit after taskkill", + ) else: os.kill(int(pid), signal.SIGKILL)