diff --git a/nvx-rs/src/host/store/files.rs b/nvx-rs/src/host/store/files.rs index 63fde8e50..d20664270 100644 --- a/nvx-rs/src/host/store/files.rs +++ b/nvx-rs/src/host/store/files.rs @@ -1,12 +1,17 @@ //! The private files of the store: their creation, atomic replacement, reading, and removal, and //! the syncs of their directories. //! +//! The sandbox's store opens, creates, replaces, and reads its lock, its records, its capability, +//! and its log only as [`StoreFile`]s, each of which holds a file's path with what messages call +//! it, so that no read or lock of one of them skips the checks that refuse a file that others may +//! open. +//! //! The template store, [`template::Store`](crate::host::template::Store), keeps its own records //! with the same lock files, atomic replacements, removals, and syncs. use std::fs::{self, File, OpenOptions}; use std::io::{self, Read, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use crate::error::{Error, Result}; use crate::host::{filesystem, platform}; @@ -27,7 +32,7 @@ const PRIVATE_FILE_MODE: u32 = 0o600; /// Returns options that create a file readable only by the current user (mode 0600 on Unix) /// and that open no link that its path names on Unix. -pub(super) fn private_options() -> OpenOptions { +fn private_options() -> OpenOptions { let mut options = OpenOptions::new(); options.write(true); #[cfg(unix)] @@ -159,48 +164,6 @@ pub(crate) fn write_private(path: &Path, contents: &[u8]) -> Result<()> { written.map_err(io_error(format!("cannot write {}", path.display()))) } -/// Reads the file at `path`, the store's `description`, or returns `None` if it does not exist. -/// -/// The open does not wait, as it would on Unix for a FIFO without a writer, and a file that is -/// not a regular file is refused before anything is read from it, as a FIFO whose writer keeps -/// it open would keep a read waiting. A file that is not private to the current user is refused, -/// since others could have written it: on Windows, a file's own access control list decides who -/// may open it, whatever the store's directory allows. -pub(super) fn read_private(path: &Path, description: &str) -> Result>> { - let failed = || io_error(format!("cannot read the {description} {}", path.display())); - let mut options = OpenOptions::new(); - options.read(true); - #[cfg(unix)] - { - use std::os::unix::fs::OpenOptionsExt; - - // Reads of a regular file do not depend on the flag. - options.custom_flags(nix::fcntl::OFlag::O_NONBLOCK.bits()); - } - let mut file = match options.open(path) { - Ok(file) => file, - Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), - // A socket refuses an open. - #[cfg(unix)] - Err(error) if error.raw_os_error() == Some(nix::errno::Errno::ENXIO as i32) => { - return Err(failed()(not_regular())); - } - Err(error) => return Err(failed()(error)), - }; - if !file.metadata().map_err(failed())?.file_type().is_file() { - return Err(failed()(not_regular())); - } - if let Some(problem) = platform::private_file_problem(&file).map_err(failed())? { - return Err(Error::io(format!( - "the {description} {} {problem}", - path.display() - ))); - } - let mut contents = Vec::new(); - file.read_to_end(&mut contents).map_err(failed())?; - Ok(Some(contents)) -} - /// Makes the changes to the entries of the directory `dir` so far durable, such as files that /// were renamed into it or removed from it: a crash that keeps a later change keeps them too, even /// one on another filesystem. @@ -234,3 +197,130 @@ pub(crate) fn remove_if_present(path: &Path) -> Result<()> { _ => Ok(()), } } + +/// One file of a sandbox's store: its path, and what messages call it. +/// +/// [`SandboxStore`](super::SandboxStore) builds one for each file that it creates and reads, its +/// lock, its launch journal, its runtime record, its capability, and its log, and opens, creates, +/// replaces, and reads those files only through it, so that each of them is created readable only +/// by the current user, replaced atomically, read only if it is a regular file that others may not +/// open, and locked only if others may not open it, and that every message about it calls it by +/// its description. +#[derive(Debug)] +pub(super) struct StoreFile { + /// The file's path. + path: PathBuf, + /// What messages call the file, such as `sandbox runtime record`. + description: &'static str, +} + +impl StoreFile { + /// Returns the file at `path`, which messages call `description`. + pub(super) fn new(path: PathBuf, description: &'static str) -> Self { + Self { path, description } + } + + /// Returns the file's path. + pub(super) fn path(&self) -> &Path { + &self.path + } + + /// Returns what messages call the file. + pub(super) fn description(&self) -> &'static str { + self.description + } + + /// Creates the file, which must not exist, readable only by the current user, as + /// [`create_new`] does. + pub(super) fn create_new(&self) -> io::Result { + create_new(&self.path) + } + + /// Opens the file as a lock, creating it readable only by the current user if it does not + /// exist, as [`open_lock`] does, and refuses it if it is not private to the current user, since + /// others could hold its lock. + /// + /// `failed` maps the failures to open the file and to check it, as the caller maps those of + /// the lock that it then takes. + pub(super) fn open_lock(&self, failed: impl Fn(io::Error) -> Error) -> Result { + let file = open_lock(&self.path).map_err(&failed)?; + if let Some(problem) = platform::private_file_problem(&file).map_err(failed)? { + return Err(self.not_private(&problem)); + } + Ok(file) + } + + /// Atomically replaces the file with `contents`, readable only by the current user, as + /// [`write_private`] does. + pub(super) fn replace(&self, contents: &[u8]) -> Result<()> { + write_private(&self.path, contents) + } + + /// Reads the file, or returns `None` if it does not exist. + /// + /// The open does not wait, as it would on Unix for a FIFO without a writer, and a file that is + /// not a regular file is refused before anything is read from it, as a FIFO whose writer keeps + /// it open would keep a read waiting. A file that is not private to the current user is + /// refused, since others could have written it: on Windows, a file's own access control list + /// decides who may open it, whatever the store's directory allows. + pub(super) fn read(&self) -> Result>> { + let mut options = OpenOptions::new(); + options.read(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + + // Reads of a regular file do not depend on the flag. + options.custom_flags(nix::fcntl::OFlag::O_NONBLOCK.bits()); + } + let mut file = match options.open(&self.path) { + Ok(file) => file, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + // A socket refuses an open. + #[cfg(unix)] + Err(error) if error.raw_os_error() == Some(nix::errno::Errno::ENXIO as i32) => { + return Err(self.read_failed()(not_regular())); + } + Err(error) => return Err(self.read_failed()(error)), + }; + if !file + .metadata() + .map_err(self.read_failed())? + .file_type() + .is_file() + { + return Err(self.read_failed()(not_regular())); + } + if let Some(problem) = platform::private_file_problem(&file).map_err(self.read_failed())? { + return Err(self.not_private(&problem)); + } + let mut contents = Vec::new(); + file.read_to_end(&mut contents) + .map_err(self.read_failed())?; + Ok(Some(contents)) + } + + /// Removes the file if it exists, as [`remove_if_present`] does. + pub(super) fn remove_if_present(&self) -> Result<()> { + remove_if_present(&self.path) + } + + /// Builds an error mapper that keeps the I/O source of a failure to read the file. + pub(super) fn read_failed(&self) -> impl FnOnce(io::Error) -> Error { + io_error(format!( + "cannot read the {} {}", + self.description, + self.path.display() + )) + } + + /// Returns the error of the file, which is not private to the current user for the reason + /// that `problem` gives. + fn not_private(&self, problem: &str) -> Error { + Error::io(format!( + "the {} {} {problem}", + self.description, + self.path.display() + )) + } +} diff --git a/nvx-rs/src/host/store/mod.rs b/nvx-rs/src/host/store/mod.rs index 45833869c..89ec87ac1 100644 --- a/nvx-rs/src/host/store/mod.rs +++ b/nvx-rs/src/host/store/mod.rs @@ -249,12 +249,13 @@ use std::time::{Duration, Instant}; pub use self::caller::CallerFiles; use self::caller::{is_report_staging, is_store_temporary}; -use self::files::{io_error, remove_if_present}; +use self::files::{StoreFile, io_error, remove_if_present}; pub use self::identity::{ ArtifactIdentity, IdentityCheck, IdentityPart, MAX_IDENTITY_LEN, RuntimeIdentity, }; use self::lease::{Lease, clone_socket_name, is_clone_socket, open_payload}; pub use self::lifecycle::{Lingering, RemovalError}; +use self::record::Record; pub use self::record::{ LaunchId, LaunchJournal, LaunchState, LogClaim, MAX_ENDPOINT_LEN, RuntimeRecord, }; @@ -520,6 +521,36 @@ impl SandboxStore { self.dir.join(name) } + /// Returns the store's file `name`, one of [`FILE_NAMES`], which messages call `description`. + fn file(&self, name: &str, description: &'static str) -> StoreFile { + StoreFile::new(self.path(name), description) + } + + /// Returns the sandbox's lock file, [`LOCK_NAME`]. + fn lock_file(&self) -> StoreFile { + self.file(LOCK_NAME, "sandbox lock") + } + + /// Returns the launch journal, [`JOURNAL_NAME`]. + fn journal_file(&self) -> StoreFile { + self.file(JOURNAL_NAME, LaunchJournal::DESCRIPTION) + } + + /// Returns the runtime record, [`RUNTIME_NAME`]. + fn runtime_file(&self) -> StoreFile { + self.file(RUNTIME_NAME, RuntimeRecord::DESCRIPTION) + } + + /// Returns the file that holds the launch capability, [`CAPABILITY_NAME`]. + fn capability_file(&self) -> StoreFile { + self.file(CAPABILITY_NAME, "sandbox control capability") + } + + /// Returns the VMM's log, [`LOG_NAME`]. + fn log_file(&self) -> StoreFile { + self.file(LOG_NAME, "log") + } + /// Returns the path of the VMM's log. pub fn log_path(&self) -> PathBuf { self.path(LOG_NAME) @@ -582,29 +613,26 @@ impl SandboxStore { /// Reads the runtime record, which names the sandbox's VM from the commit of its launch until /// its runtime is cleared. pub fn runtime(&self) -> Result> { - record::read(&self.path(RUNTIME_NAME)) + record::read(&self.runtime_file()) } /// Reads the journal of a launch in progress, or of one that was interrupted. pub fn journal(&self) -> Result> { - record::read(&self.path(JOURNAL_NAME)) + record::read(&self.journal_file()) } /// Reads the launch capability of the sandbox's VM. pub fn read_capability(&self) -> Result<[u8; CAPABILITY_LEN]> { - let path = self.capability_path(); - let description = "sandbox control capability"; - let Some(bytes) = files::read_private(&path, description)? else { - return Err(io_error(format!( - "cannot read the {description} {}", - path.display() - ))(io::Error::from(io::ErrorKind::NotFound))); + let file = self.capability_file(); + let Some(bytes) = file.read()? else { + return Err(file.read_failed()(io::Error::from(io::ErrorKind::NotFound))); }; match <[u8; CAPABILITY_LEN]>::try_from(bytes.as_slice()) { Ok(capability) if capability != [0; CAPABILITY_LEN] => Ok(capability), _ => Err(Error::io(format!( - "the sandbox control capability {} is invalid", - path.display() + "the {} {} is invalid", + file.description(), + file.path().display() ))), } } @@ -643,15 +671,10 @@ impl SandboxStore { /// then: the open then fails with an I/O error whose cause is the denied access, while a /// removed lock file that the lock's own handle keeps is taken for a replaced one. fn take_lock(&self, deadline: Option) -> Result { - let path = self.path(LOCK_NAME); + let lock = self.lock_file(); + let path = lock.path(); let failed = |error| store_error(format!("cannot lock {}", path.display()))(error); - let file = files::open_lock(&path).map_err(failed)?; - if let Some(problem) = platform::private_file_problem(&file).map_err(failed)? { - return Err(Error::io(format!( - "the sandbox lock {} {problem}", - path.display() - ))); - } + let file = lock.open_lock(failed)?; match deadline { None => file.lock().map_err(failed)?, Some(deadline) => loop { @@ -669,7 +692,7 @@ impl SandboxStore { }, } let locked = platform::opened_file_id(&file).map_err(failed)?; - match platform::file_id(&path) { + match platform::file_id(path) { Ok(current) if current == locked => Ok(Taken::Locked(file)), Ok(_) => Ok(Taken::Replaced), Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(Taken::Replaced), @@ -945,10 +968,11 @@ impl LockedStore { let endpoint = platform::control_endpoint(listening) .map_err(|error| Error::io("cannot choose a control endpoint").with_source(error))?; let (log, claim) = self.create_log()?; - files::write_private(&self.capability_path(), capability)?; + let capability_file = self.capability_file(); + capability_file.replace(capability)?; let journal = LaunchJournal::prepared(launch.clone(), endpoint, runtime.clone(), claim); - if let Err(error) = record::write(&self.path(JOURNAL_NAME), &journal) { - let _ = remove_if_present(&self.capability_path()); + if let Err(error) = record::write(&self.journal_file(), &journal) { + let _ = capability_file.remove_if_present(); return Err(error); } // A lease reserves one launch, which has begun. @@ -962,10 +986,16 @@ impl LockedStore { /// Replaces the log of the previous launch with a fresh file, and claims it. fn create_log(&self) -> Result<(File, LogClaim)> { - let path = self.log_path(); - let failed = |error| io_error(format!("cannot create the log {}", path.display()))(error); - remove_if_present(&path)?; - let log = files::create_new(&path).map_err(failed)?; + let file = self.log_file(); + let failed = |error| { + io_error(format!( + "cannot create the {} {}", + file.description(), + file.path().display() + ))(error) + }; + file.remove_if_present()?; + let log = file.create_new().map_err(failed)?; platform::claim_log(&log).map_err(failed)?; let (device, index) = platform::opened_file_identity(&log).map_err(failed)?; let claim = LogClaim { @@ -992,7 +1022,7 @@ impl LockedStore { match self.runtime()? { None => self.end_launch(&journal), Some(runtime) if runtime.launch() == journal.launch() => { - remove_if_present(&self.path(JOURNAL_NAME))?; + self.journal_file().remove_if_present()?; Ok(Recovery::Clean) } Some(runtime) => Err(Error::io(format!( @@ -1012,14 +1042,13 @@ impl LockedStore { None => self.end_unidentified(journal), }; if !matches!(recovery, Recovery::Indeterminate(_)) { - for name in [CAPABILITY_NAME, SOCKET_NAME] { - remove_if_present(&self.path(name))?; - } + self.capability_file().remove_if_present()?; + remove_if_present(&self.socket_path())?; self.remove_clone_socket(journal.endpoint())?; self.remove_report_staging()?; // The journal goes last, so that an interrupted removal leaves it for a later // recovery to finish. - remove_if_present(&self.path(JOURNAL_NAME))?; + self.journal_file().remove_if_present()?; } Ok(recovery) } @@ -1143,14 +1172,13 @@ impl LockedStore { /// does, given `record`, the runtime record that the caller read under this lock, if any, whose /// control endpoint is the VM's. pub(crate) fn clear_runtime_of(&mut self, record: Option<&RuntimeRecord>) -> Result<()> { - for name in [CAPABILITY_NAME, SOCKET_NAME] { - remove_if_present(&self.path(name))?; - } + self.capability_file().remove_if_present()?; + remove_if_present(&self.socket_path())?; if let Some(record) = record { self.remove_clone_socket(record.endpoint())?; } self.remove_report_staging()?; - remove_if_present(&self.path(RUNTIME_NAME)) + self.runtime_file().remove_if_present() } /// Removes the control socket at `endpoint`, the control endpoint that a launch of the @@ -1276,7 +1304,7 @@ impl Launch<'_> { .process() .map_err(|error| Error::io("cannot identify the VMM process").with_source(error))?; let journal = self.journal.spawned(process.reference().clone()); - record::write(&self.locked.path(JOURNAL_NAME), &journal)?; + record::write(&self.locked.journal_file(), &journal)?; self.journal = journal; Ok(process) } @@ -1295,8 +1323,8 @@ impl Launch<'_> { )); }; let runtime = RuntimeRecord::committed(&self.journal, process.clone()); - record::write(&self.locked.path(RUNTIME_NAME), &runtime)?; - let _ = remove_if_present(&self.locked.path(JOURNAL_NAME)); + record::write(&self.locked.runtime_file(), &runtime)?; + let _ = self.locked.journal_file().remove_if_present(); Ok(runtime) } diff --git a/nvx-rs/src/host/store/record.rs b/nvx-rs/src/host/store/record.rs index 748c3e3b7..fe86eac4f 100644 --- a/nvx-rs/src/host/store/record.rs +++ b/nvx-rs/src/host/store/record.rs @@ -2,13 +2,12 @@ //! the identities that they hold, and their reading and writing in format [`FORMAT`]. use std::fmt; -use std::path::Path; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use super::FORMAT; -use super::files::{read_private, write_private}; +use super::files::StoreFile; use super::identity::{IdentityCheck, RuntimeIdentity}; use crate::error::{Error, ErrorCode, Result}; use crate::host::filesystem::FileIdentity; @@ -285,7 +284,7 @@ impl LaunchJournal { /// A record of the store, in format [`FORMAT`]. pub(super) trait Record: Serialize + DeserializeOwned { - /// What messages call the record. + /// What messages call the record, and the [`StoreFile`] that holds it. const DESCRIPTION: &'static str; /// Explains why the record, which parsed, is invalid, if it is. @@ -314,17 +313,17 @@ impl Record for LaunchJournal { } } -/// Reads the record at `path`, or returns `None` if the file does not exist. +/// Reads the record that `file` holds, or returns `None` if the file does not exist. /// /// A record of another format fails before its fields are read, with a message that names its /// format, and the code [`ErrorCode::FormatUnsupported`]: the store reads format [`FORMAT`] only, /// and migrates no other. A record file that is not private to the current user fails as well, /// since others could have written it, and so does one that is not a regular file, without waiting -/// on it. Every failure to read or check the record is an [`ErrorKind::Io`](crate::ErrorKind::Io) -/// error. -pub(super) fn read(path: &Path) -> Result> { - let description = T::DESCRIPTION; - let Some(text) = read_private(path, description)? else { +/// on it, as [`StoreFile::read`] reads it. Every failure to read or check the record is an +/// [`ErrorKind::Io`](crate::ErrorKind::Io) error. +pub(super) fn read(file: &StoreFile) -> Result> { + let (path, description) = (file.path(), file.description()); + let Some(text) = file.read()? else { return Ok(None); }; let malformed = || format!("the {description} {} is malformed", path.display()); @@ -355,11 +354,11 @@ pub(super) fn read(path: &Path) -> Result> { } } -/// Atomically replaces the record at `path` with `record`, as pretty JSON and a newline. -pub(super) fn write(path: &Path, record: &T) -> Result<()> { +/// Atomically replaces the record that `file` holds with `record`, as pretty JSON and a newline. +pub(super) fn write(file: &StoreFile, record: &T) -> Result<()> { let mut text = serde_json::to_vec_pretty(record).map_err(|error| { - Error::other(format!("cannot encode the {}", T::DESCRIPTION)).with_source(error) + Error::other(format!("cannot encode the {}", file.description())).with_source(error) })?; text.push(b'\n'); - write_private(path, &text) + file.replace(&text) } diff --git a/nvx-rs/src/host/store/tests.rs b/nvx-rs/src/host/store/tests.rs index 12ddc19ea..7b810d298 100644 --- a/nvx-rs/src/host/store/tests.rs +++ b/nvx-rs/src/host/store/tests.rs @@ -495,6 +495,77 @@ fn records_and_capabilities_that_are_not_regular_files_fail_at_once() { } } +#[test] +fn failures_to_read_the_records_and_the_capability_name_them() { + let (_directory, store) = store(); + let read = |name: &str| match name { + RUNTIME_NAME => store.runtime().map(drop), + JOURNAL_NAME => store.journal().map(drop), + _ => store.read_capability().map(drop), + }; + let files = [ + (RUNTIME_NAME, "sandbox runtime record"), + (JOURNAL_NAME, "sandbox launch journal"), + (CAPABILITY_NAME, "sandbox control capability"), + ]; + // A missing record is none, but a missing capability fails, as its launch needs it. + assert!(store.runtime().unwrap().is_none()); + assert!(store.journal().unwrap().is_none()); + let path = store.path(CAPABILITY_NAME); + let error = store.read_capability().unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert_eq!( + error.message(), + format!( + "cannot read the sandbox control capability {}", + path.display() + ) + ); + let cause = std::error::Error::source(&error) + .and_then(|source| source.downcast_ref::()) + .map(io::Error::kind); + assert_eq!(cause, Some(io::ErrorKind::NotFound), "{error}"); + // A capability of another length, or an all-zero one, is invalid. + for invalid in [&CAPABILITY[1..], &[0; CAPABILITY_LEN][..]] { + fs::write(&path, invalid).unwrap(); + let error = store.read_capability().unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert_eq!( + error.message(), + format!( + "the sandbox control capability {} is invalid", + path.display() + ) + ); + } + fs::remove_file(&path).unwrap(); + // A record that does not parse is malformed. + for (name, description) in &files[..2] { + let path = store.path(name); + fs::write(&path, b"{").unwrap(); + let error = read(name).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert_eq!( + error.message(), + format!("the {description} {} is malformed", path.display()) + ); + fs::remove_file(&path).unwrap(); + } + // A directory in the place of a record or of the capability cannot be read as one: Unix opens + // it, and refuses it as no regular file, and Windows refuses to open it. + for (name, description) in files { + let path = store.path(name); + fs::create_dir(&path).unwrap(); + let error = read(name).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert_eq!( + error.message(), + format!("cannot read the {description} {}", path.display()) + ); + fs::remove_dir(&path).unwrap(); + } +} + #[cfg(target_os = "linux")] #[test] fn the_store_is_private_to_the_current_user() { @@ -652,17 +723,32 @@ fn the_store_is_private_to_the_current_user() { "{name}" ); } + let refused = |description: &str, name: &str| { + format!( + "the {description} {} allows other users", + store.path(name).display() + ) + }; let error = store.journal().unwrap_err(); - assert!(error.message().ends_with("allows other users"), "{error}"); + assert_eq!( + error.message(), + refused("sandbox launch journal", JOURNAL_NAME) + ); let error = locked.recover().unwrap_err(); - assert!(error.message().ends_with("allows other users"), "{error}"); + assert_eq!( + error.message(), + refused("sandbox launch journal", JOURNAL_NAME) + ); let error = store.read_capability().unwrap_err(); - assert!(error.message().ends_with("allows other users"), "{error}"); + assert_eq!( + error.message(), + refused("sandbox control capability", CAPABILITY_NAME) + ); // So is a lock file whose own list allows others, who could hold its lock. drop(locked); platform::set_access(&store.path(LOCK_NAME), "D:P(A;;FA;;;WD)").unwrap(); let error = store.lock(None).unwrap_err(); - assert!(error.message().ends_with("allows other users"), "{error}"); + assert_eq!(error.message(), refused("sandbox lock", LOCK_NAME)); } #[test] @@ -896,8 +982,8 @@ fn a_journal_that_its_runtime_record_supersedes_is_removed() { drop(launch); // The launch wrote its runtime record and crashed before it removed its journal. let committed = RuntimeRecord::committed(&journal, journal.process().unwrap().clone()); - record::write(&store.path(RUNTIME_NAME), &committed).unwrap(); - record::write(&store.path(JOURNAL_NAME), &journal).unwrap(); + record::write(&store.runtime_file(), &committed).unwrap(); + record::write(&store.journal_file(), &journal).unwrap(); assert_eq!(locked.recover().unwrap(), Recovery::Clean); assert!(store.journal().unwrap().is_none()); assert_eq!(store.runtime().unwrap(), Some(committed)); @@ -908,7 +994,7 @@ fn a_journal_that_its_runtime_record_supersedes_is_removed() { runtime(), *journal.log(), ); - record::write(&store.path(JOURNAL_NAME), &other).unwrap(); + record::write(&store.journal_file(), &other).unwrap(); let error = locked.recover().unwrap_err(); assert_eq!(error.kind(), ErrorKind::Io); assert!(error.message().contains("names launch"), "{error}");