host: write private files through the platform layer (SL6.0d)

doc/acceptance-case-matrix.md

@@ -506,7 +506,7 @@ this matrix. The README's own code is no doctest.
 | `nvx-rs/src/host/transport.rs - host::transport (line 22)` | kept |  |
 | `nvx-rs/src/lib.rs - (line 58)` | kept |  |
 | `nvx-rs/src/protocol/nvxs/mod.rs - protocol::nvxs (line 68)` | kept |  |
-| `nvx-rs/src/host/snapshot/capture.rs - host::snapshot::capture::finish_capture (line 50)` | kept |  |
+| `nvx-rs/src/host/snapshot/capture.rs - host::snapshot::capture::finish_capture (line 48)` | kept |  |
 | `nvx-rs/src/host/store/mod.rs - host::store (line 104)` | kept | The sandbox store leaves (#1121), whose PR retires it |
 | `nvx-rs/README.md`, its Example: an instance through `LaunchSpec` and the sandbox store | rewritten | Nothing compiles it. `nvx-rs/examples/host-workflows.rs` compiles the documented host workflows, and the library's guide (SL6.10a) replaces the example |
 | `nvx-rs/examples/external-launch.rs` | kept | Compiled with the crate's targets, and run by hand on a host |

nvx-rs/src/host/cli/snapshots.rs

@@ -30,7 +30,7 @@ use crate::host::config::NicConfig;
 use crate::host::sandbox::options::SandboxOperation;
 use crate::host::snapshot::{OPENVMM_DIR, SNAPSHOT_ARTIFACTS, Snapshot};
 use crate::host::vm::VmDir;
-use crate::host::{filesystem, paths, platform, store};
+use crate::host::{filesystem, paths, platform};
 use crate::{Error, ErrorCode};
 
 /// The parser of the snapshot command.
@@ -411,11 +411,11 @@ impl LockedSnapshot<'_> {
             ));
         }
         Snapshot::open(staged.dir())?;
-        store::files::sync_dir(staged.dir())?;
+        platform::sync_dir(staged.dir())?;
         platform::rename_no_replace(staged.dir(), &self.dir)
             .map_err(|error| io_error(ErrorCode::Io, &error, &self.dir))?;
-        store::files::sync_dir(self.store.entries.dir())?;
-        store::files::sync_dir(self.store.staging.dir())?;
+        platform::sync_dir(self.store.entries.dir())?;
+        platform::sync_dir(self.store.staging.dir())?;
         self.open()
     }
 }

nvx-rs/src/host/cli/snapshots/capture.rs

@@ -333,7 +333,7 @@ fn capture(
 /// Reads the geometry through the existing block-device helper on this VM's filesystem.
 fn disk_geometry(dir: &VmDir) -> Result<BlockGeometry> {
     let path = dir.path().join("geometry.raw");
-    let file = crate::host::store::files::create_new(&path)
+    let file = crate::host::platform::create_new(&path)
         .map_err(|error| io_error(ErrorCode::Io, &error, &path))?;
     file.set_len(512)
         .map_err(|error| io_error(ErrorCode::Io, &error, &path))?;

nvx-rs/src/host/cli/snapshots/capture/tests.rs

@@ -739,23 +739,19 @@ fn capture_helper() {
     })).unwrap()).unwrap();
     vm::create_private_dir(Path::new(OPENVMM_DIR)).unwrap();
     let manifest: Vec<u8> = serde_json::from_value(recipe["manifest"].clone()).unwrap();
-    crate::host::store::files::write_private(
-        &Path::new(OPENVMM_DIR).join(MANIFEST_NAME),
-        &manifest,
-    )
-    .unwrap();
+    crate::host::platform::write_private(&Path::new(OPENVMM_DIR).join(MANIFEST_NAME), &manifest)
+        .unwrap();
     if refused {
         thread::sleep(Duration::from_secs(120));
         return;
     }
     for (entry, field) in [("memory.bin", "memory"), ("state.bin", "state")] {
-        let file =
-            crate::host::store::files::create_new(&Path::new(OPENVMM_DIR).join(entry)).unwrap();
+        let file = crate::host::platform::create_new(&Path::new(OPENVMM_DIR).join(entry)).unwrap();
         platform::make_sparse(&file).unwrap();
         file.set_len(recipe[field].as_u64().unwrap()).unwrap();
     }
     let failed = recipe["behavior"] == "fail";
-    crate::host::store::files::write_private(Path::new("outcome.json"), &serde_json::to_vec(&json!({
+    crate::host::platform::write_private(Path::new("outcome.json"), &serde_json::to_vec(&json!({
         "outcome": { "operation": "capture", "category": if failed { "failure" } else { "success" },
             "status_code": if failed { 1 } else { 0 } },
     })).unwrap()).unwrap();

nvx-rs/src/host/cli/snapshots/tests.rs

@@ -130,7 +130,7 @@ fn invalid_or_linked_directories_never_publish_or_open() {
     let store = Store::create(&root.path().join("store")).unwrap();
     let held = store.lock(&name("first"), Deadline::after(0.0)).unwrap();
     let pending = staged(&held);
-    crate::host::store::files::write_private(&pending.dir().join(KEY_FILE), b"bad key").unwrap();
+    crate::host::platform::write_private(&pending.dir().join(KEY_FILE), b"bad key").unwrap();
     assert!(held.publish(pending.clone()).is_err());
     assert!(!held.dir.exists());
     let valid = staged(&held);

nvx-rs/src/host/cli/store/files.rs

@@ -6,7 +6,6 @@ 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.
@@ -15,7 +14,7 @@ const MAX_RECORD_BYTES: u64 = 1024 * 1024;
 pub(super) fn read(state: &LockedState, name: StateFile) -> Result<Option<Vec<u8>>> {
     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) {
+    let file = match platform::open_regular(&path) {
         Ok(file) => file,
         Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None),
         Err(error) => return Err(failed(error)),
@@ -26,7 +25,7 @@ pub(super) fn read(state: &LockedState, name: StateFile) -> Result<Option<Vec<u8
             path.display()
         )));
     }
-    regular::read_at_most(file, MAX_RECORD_BYTES, "a sandbox record")
+    platform::read_at_most(file, MAX_RECORD_BYTES, "a sandbox record")
         .map(Some)
         .map_err(failed)
 }
@@ -37,8 +36,8 @@ pub(super) fn replace(state: &LockedState, name: StateFile, bytes: &[u8]) -> Res
     }
     // 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())
+    platform::write_private(&state.path(name), bytes)?;
+    platform::sync_dir(state.dir())
 }
 
 pub(super) fn remove(state: &LockedState, name: StateFile) -> Result<()> {
@@ -47,5 +46,5 @@ pub(super) fn remove(state: &LockedState, name: StateFile) -> Result<()> {
     fs::remove_file(&path).map_err(|error| {
         Error::io(format!("cannot remove {}", path.display())).with_source(error)
     })?;
-    files::sync_dir(state.dir())
+    platform::sync_dir(state.dir())
 }

nvx-rs/src/host/platform/mod.rs

@@ -11,16 +11,17 @@
 //! Its crate-private reads of regular files open a file without following a link, or on Windows
 //! any other reparse point, refuse one that is not a regular file before reading anything from
 //! it, and read it within a bound.
+//!
+//! Its crate-private writes create files that only the current user may read, replace a file
+//! atomically, and make the changes to a directory's entries durable.
 
 use std::fmt;
 use std::fs::{self, File, OpenOptions};
-use std::io::{self, Read};
+use std::io::{self, Read, Write};
 use std::path::Path;
 use std::time::Duration;
 
-#[cfg(windows)]
-use crate::error::Error;
-use crate::error::Result;
+use crate::error::{Error, Result};
 
 pub(crate) mod inherit;
 #[cfg(target_os = "linux")]
@@ -137,6 +138,92 @@ pub(crate) fn read_at_most(file: File, limit: u64, what: &str) -> io::Result<Vec
     Ok(bytes)
 }
 
+/// Mode of a private file on Unix: readable and writable by the current user only.
+#[cfg(unix)]
+pub(crate) 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(crate) fn private_options() -> OpenOptions {
+    let mut options = OpenOptions::new();
+    options.write(true);
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::OpenOptionsExt;
+
+        options
+            .mode(PRIVATE_FILE_MODE)
+            .custom_flags(nix::fcntl::OFlag::O_NOFOLLOW.bits());
+    }
+    options
+}
+
+/// Creates the file at `path`, which must not exist, readable only by the current user.
+///
+/// On Unix, the umask may clear bits of the mode that the file is created with, even the
+/// current user's permission to read it, so the mode is set again through the new file.
+pub(crate) fn create_new(path: &Path) -> io::Result<File> {
+    let file = private_options().create_new(true).open(path)?;
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+
+        file.set_permissions(fs::Permissions::from_mode(PRIVATE_FILE_MODE))?;
+    }
+    Ok(file)
+}
+
+/// Atomically replaces `path` with `contents`, readable only by the current user.
+///
+/// The contents go to a fresh temporary file beside `path`, which is flushed to disk and then
+/// renamed over it, so a reader sees the old contents or the new ones, and a crash leaves at most
+/// the temporary file.
+pub(crate) fn write_private(path: &Path, contents: &[u8]) -> Result<()> {
+    let mut suffix = [0u8; 8];
+    getrandom::fill(&mut suffix)
+        .map_err(|error| Error::other("cannot name a temporary file").with_source(error))?;
+    let name = path
+        .file_name()
+        .map(|name| name.to_string_lossy().into_owned())
+        .unwrap_or_default();
+    let temporary = path.with_file_name(format!(".{name}.{}.tmp", u64::from_le_bytes(suffix)));
+    let written = (|| -> io::Result<()> {
+        let mut file = create_new(&temporary)?;
+        file.write_all(contents)?;
+        file.sync_all()?;
+        drop(file);
+        fs::rename(&temporary, path)
+    })();
+    if written.is_err() {
+        let _ = fs::remove_file(&temporary);
+    }
+    written
+        .map_err(|error| Error::io(format!("cannot write {}", path.display())).with_source(error))
+}
+
+/// 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.
+///
+/// On Unix, it syncs the directory. Windows has no portable way to flush a directory, so there a
+/// rename or a removal is as durable as the filesystem makes it, as OpenVMM's publication of a
+/// snapshot assumes.
+pub(crate) fn sync_dir(dir: &Path) -> Result<()> {
+    #[cfg(unix)]
+    {
+        File::open(dir)
+            .and_then(|directory| directory.sync_all())
+            .map_err(|error| {
+                Error::io(format!("cannot sync the directory {}", dir.display())).with_source(error)
+            })
+    }
+    #[cfg(not(unix))]
+    {
+        let _ = dir;
+        Ok(())
+    }
+}
+
 /// Refuses a VMM working directory that exceeds Windows' process-creation limit, without
 /// accessing the filesystem. Other hosts have no such limit.
 pub(crate) fn check_working_dir(path: &Path) -> Result<()> {

nvx-rs/src/host/platform/tests.rs

@@ -49,6 +49,71 @@ fn an_open_of_a_regular_file_refuses_a_link_and_a_directory() {
     }
 }
 
+#[test]
+fn a_private_write_replaces_its_file_whole_and_a_failed_one_leaves_no_temporary_file() {
+    let directory = tempfile::tempdir().unwrap();
+    let path = directory.path().join("record.json");
+    std::fs::write(&path, b"old").unwrap();
+    write_private(&path, b"new").unwrap();
+    assert_eq!(std::fs::read(&path).unwrap(), b"new");
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+
+        let mode = std::fs::metadata(&path).unwrap().permissions().mode();
+        assert_eq!(mode & 0o7777, PRIVATE_FILE_MODE);
+    }
+    // A directory takes the place of the file, so the rename fails.
+    let occupied = directory.path().join("occupied");
+    std::fs::create_dir(&occupied).unwrap();
+    let error = write_private(&occupied, b"new").unwrap_err();
+    assert_eq!(
+        error.to_string(),
+        format!("cannot write {}", occupied.display())
+    );
+    let mut names: Vec<_> = std::fs::read_dir(directory.path())
+        .unwrap()
+        .map(|entry| entry.unwrap().file_name())
+        .collect();
+    names.sort();
+    assert_eq!(names, ["occupied", "record.json"]);
+}
+
+#[test]
+fn a_new_private_file_never_takes_the_place_of_an_existing_entry() {
+    let directory = tempfile::tempdir().unwrap();
+    let path = directory.path().join("record.json");
+    std::fs::write(&path, b"old").unwrap();
+    let error = create_new(&path).unwrap_err();
+    assert_eq!(error.kind(), io::ErrorKind::AlreadyExists, "{error}");
+    assert_eq!(std::fs::read(&path).unwrap(), b"old");
+    // Nor does it follow a link to a file that does not exist yet.
+    #[cfg(unix)]
+    {
+        let target = directory.path().join("target");
+        let link = directory.path().join("link");
+        std::os::unix::fs::symlink(&target, &link).unwrap();
+        create_new(&link).unwrap_err();
+        assert!(!target.exists());
+    }
+}
+
+#[test]
+fn a_directory_sync_fails_for_a_missing_directory_only_on_unix() {
+    let directory = tempfile::tempdir().unwrap();
+    sync_dir(directory.path()).unwrap();
+    let missing = directory.path().join("missing");
+    let synced = sync_dir(&missing);
+    #[cfg(unix)]
+    assert_eq!(
+        synced.unwrap_err().to_string(),
+        format!("cannot sync the directory {}", missing.display())
+    );
+    // Windows has no portable way to flush a directory.
+    #[cfg(not(unix))]
+    synced.unwrap();
+}
+
 #[cfg(any(target_os = "linux", windows))]
 #[test]
 fn a_transport_without_a_native_handle_needs_no_clone_implementation() {

nvx-rs/src/host/snapshot/capture.rs

@@ -11,10 +11,8 @@ use super::{
     CaptureContext, DESCRIPTION_FILE, KEY_FILE, MANIFEST_NAME, OPENVMM_DIR, SNAPSHOT_ARTIFACTS,
     SnapshotKey, SnapshotManifest, check_openvmm,
 };
-use crate::host::platform;
+use crate::host::platform::{self, open_regular, read_at_most, sync_dir, write_private};
 use crate::host::process::Exited;
-use crate::host::store::files::{sync_dir, write_private};
-use crate::host::store::regular::{open_regular, read_at_most};
 use crate::host::vm_directory::{OUTCOME, VmDir};
 use crate::{Error, ErrorKind, Result};
 

nvx-rs/src/host/snapshot/key/tests.rs

@@ -748,8 +748,8 @@ fn key_refusal(bytes: &[u8], code: ErrorCode) -> String {
 #[cfg(any(target_os = "linux", windows))]
 #[test]
 fn snapshot_open_refuses_other_key_contracts_and_filesystem_tags() {
+    use crate::host::platform::write_private;
     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,