diff --git a/CHANGELOG.md b/CHANGELOG.md index 22707cfaf..4a72b160b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- **R-15 Gate 4:** removal of a file on Unix, `test-support` only (slice H1a-2, part 2 of 4). `PinnedDir::remove_authenticated_file` moves the entry into a quarantine directory with `RENAME_NOREPLACE`, identifies the open object there and only then unlinks it; anything else is put back, a failure after the move rolls back or reports `Stranded`, and the parent is synced. The one window no Unix primitive closes (a same-user actor in the quarantine directory after the last identification) is forced in a test and reported as `Disturbed`, detected after the fact and not prevented. The rustix `process` feature is added for `geteuid`; no new package, no new `unsafe`. Directories, leftovers and Windows follow. Issue #359. PR #1069. + - **R-15 Gate 4:** the API and the isolation of the bound removal, `test-support` only (slice H1a-2, part 1 of 4). `PinnedDir::remove_authenticated_file`, `remove_empty_dir` and `remove_leftover_file` with `RemovalOutcome`, `RemovalStage` and `Observer` exist only with the test-only feature, have no caller and, in this part, answer `Unsupported` and remove nothing; the platform implementations follow in three stacked parts. A scan of the sources and a compile of a dependent with and without the feature keep the primitives out of every other build. `PinnedDir::sync` (no deletion) is added. The contract states what no Unix primitive can do (unlink "this inode": a same-user actor in the quarantine directory can win one race, which is detected after the fact as `Disturbed`, not prevented) and leaves the decision about that residual to the maintainer before any sweep uses it. No new `unsafe`, no new dependency feature, no new package. Issue #359. PR #1068. - **R-15 Gate 4:** handle-relative pinned I/O (slice H1a-1). A new parallel module `pinned` opens one directory with no-follow semantics, resolves it once to a link-free path, and reaches everything below it through a directory that is held open (`PinnedDir`, `PinnedFile`): bounded listing, bounded reading from the start of the file, identity and hard-link count; a link, junction or reparse point is refused at every component below the root, and a name must be exactly one ordinary component (on Windows also no stream, wildcard or device name). Unix uses `*at` calls on directory descriptors and checks the root's path without following a link; Windows pins a directory with a handle that does not share delete and opens children (`NtCreateFile` relative to the handle), listings (the handle's directory information) and entry kinds relative to it, so a directory converted into a reparse point in place leads nowhere. It changes nothing in the file system, has no caller and does not change `DurableFs`; the removal primitives follow in H1a-2 behind `test-support`. Local `unsafe` for the relative open, the listing and the final path, and one typed handle-query helper in `root_lock` (an `unsafe trait` with two implementations); the `Wdk_*` features of the `windows-sys` already locked, no new package. Issue #359. PR #1067. diff --git a/crates/worldscript-secure-storage/Cargo.toml b/crates/worldscript-secure-storage/Cargo.toml index aeed20b6a..a8cff79d3 100644 --- a/crates/worldscript-secure-storage/Cargo.toml +++ b/crates/worldscript-secure-storage/Cargo.toml @@ -35,10 +35,11 @@ zeroize_derive = "~1.4" # Gate 4 slice 4A (ยง11.1): the cross-process `root_commit_mutex` is an operating-system advisory # lock. rustix's safe `flock` on Unix (1.x, MSRV 1.63, already in the lockfile); on Windows one # `LockFileEx` call through windows-sys 0.52 (MSRV 1.56, already in the lockfile). -# Gate 4 slice H1a (`pinned`): the same rustix for `*at` calls; on Windows the relative `NtCreateFile` open +# Gate 4 slice H1a (`pinned`): the same rustix for `*at` calls (and, with `process`, `geteuid` for the +# quarantine directory of the test-only removal); on Windows the relative `NtCreateFile` open # (the `Wdk_*` features of windows-sys, no new package) and a few more calls. [target.'cfg(unix)'.dependencies] -rustix = { version = "1.1", default-features = false, features = ["fs", "std"] } +rustix = { version = "1.1", default-features = false, features = ["fs", "process", "std"] } [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.52", features = ["Win32_Foundation", "Win32_Storage_FileSystem", "Win32_System_IO", "Wdk_Foundation", "Wdk_Storage_FileSystem"] } diff --git a/crates/worldscript-secure-storage/src/pinned/tests/mod.rs b/crates/worldscript-secure-storage/src/pinned/tests/mod.rs index b44352bcf..950f4ed6c 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/mod.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/mod.rs @@ -8,6 +8,8 @@ use std::sync::atomic::{AtomicU32, Ordering}; mod read; #[cfg(feature = "test-support")] mod remove; +#[cfg(all(feature = "test-support", unix))] +mod remove_unix; #[cfg(windows)] mod windows_redirect; diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove.rs index 1094c9c69..03bfa29e7 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/remove.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove.rs @@ -1,13 +1,155 @@ -//! Removal, in the slice that only brings the API and the isolation: every call refuses, so nothing is -//! removed and nothing claims a safety it does not have yet. +//! Removal on every platform that implements it: only the authenticated object goes, and a refusal +//! changes nothing. Windows has no implementation in this part yet and keeps the refusal test. use std::fs; +#[cfg(unix)] +use super::link_to_dir; use super::Scratch; +#[cfg(unix)] +use crate::durable::DirectoryDurability; +#[cfg(unix)] +use crate::pinned::PinnedFile; use crate::pinned::{PinnedDir, PinnedError, RemovalOutcome}; +/// Accepts a file whose whole content is `expected`. +#[cfg(unix)] +pub(super) fn holds( + expected: &'static [u8], +) -> impl FnOnce(&mut PinnedFile) -> Result { + move |file| Ok(file.read_at_most(64)? == expected) +} + +/// What a completed removal reports on this platform. +#[cfg(unix)] +pub(super) fn removed() -> Result { + let durability = if cfg!(unix) { + DirectoryDurability::Confirmed + } else { + DirectoryDurability::NotConfirmed + }; + Ok(RemovalOutcome::Removed(durability)) +} + +#[cfg(unix)] +pub(super) fn names(pinned: &PinnedDir) -> Vec { + let mut listed: Vec = pinned + .entries(32) + .unwrap() + .into_iter() + .map(|name| name.to_string_lossy().into_owned()) + .collect(); + listed.sort(); + listed +} + +#[cfg(unix)] +#[test] +fn an_authenticated_file_is_removed_and_nothing_else_is() { + let scratch = Scratch::new("remove"); + let root = scratch.root(); + fs::write(root.join("a"), b"alpha").unwrap(); + fs::write(root.join("b"), b"beta").unwrap(); + let pinned = PinnedDir::open_root(&root).unwrap(); + let outcome = pinned.remove_authenticated_file("a", holds(b"alpha")); + assert_eq!( + (outcome, names(&pinned), fs::read(root.join("b")).unwrap()), + (removed(), vec!["b".to_string()], b"beta".to_vec()) + ); +} + +#[cfg(unix)] +#[test] +fn a_refused_authentication_a_failing_one_and_a_wrong_identity_remove_nothing() { + let scratch = Scratch::new("refuse"); + let root = scratch.root(); + fs::write(root.join("a"), b"alpha").unwrap(); + fs::write(root.join("other"), b"alpha").unwrap(); + let pinned = PinnedDir::open_root(&root).unwrap(); + let expected = pinned.open_file("other").unwrap().identity(); + let refused = pinned.remove_authenticated_file("a", holds(b"something else")); + let failed = pinned.remove_authenticated_file("a", |_| Err(PinnedError::Busy)); + // The bytes match, but this is not the object the caller meant. + let wrong = pinned.remove_authenticated_file("a", move |file| Ok(file.identity() == expected)); + assert_eq!( + ( + refused, + failed, + wrong, + fs::read(root.join("a")).unwrap(), + names(&pinned) + ), + ( + Ok(RemovalOutcome::Refused), + Err(PinnedError::Busy), + Ok(RemovalOutcome::Refused), + b"alpha".to_vec(), + vec!["a".to_string(), "other".to_string()] + ) + ); +} + +#[cfg(unix)] +#[test] +fn a_link_a_directory_and_a_missing_name_are_not_removed_as_files() { + let scratch = Scratch::new("not-files"); + let root = scratch.root(); + let aside = scratch.aside(); + fs::create_dir(aside.join("target")).unwrap(); + fs::write(aside.join("target").join("kept"), b"kept").unwrap(); + link_to_dir(&aside.join("target"), &root.join("link")); + fs::create_dir(root.join("dir")).unwrap(); + let pinned = PinnedDir::open_root(&root).unwrap(); + let remove = |name: &str| pinned.remove_authenticated_file(name, |_| Ok(true)); + assert_eq!( + ( + remove("link"), + remove("dir"), + remove("missing"), + fs::read(aside.join("target").join("kept")).unwrap(), + names(&pinned) + ), + ( + Err(PinnedError::Redirected), + Err(PinnedError::NotRegularFile), + Err(PinnedError::NotFound), + b"kept".to_vec(), + vec!["dir".to_string(), "link".to_string()] + ) + ); +} + +#[cfg(unix)] +#[test] +fn removing_a_hard_linked_name_leaves_the_other_names() { + let scratch = Scratch::new("hard-link"); + let root = scratch.root(); + fs::write(root.join("a"), b"alpha").unwrap(); + fs::hard_link(root.join("a"), root.join("a-too")).unwrap(); + let pinned = PinnedDir::open_root(&root).unwrap(); + let links = pinned.open_file("a").unwrap().link_count(); + let outcome = pinned.remove_authenticated_file("a", holds(b"alpha")); + assert_eq!( + ( + links, + outcome, + names(&pinned), + fs::read(root.join("a-too")).unwrap(), + pinned.open_file("a-too").unwrap().link_count() + ), + ( + Ok(2), + removed(), + vec!["a-too".to_string()], + b"alpha".to_vec(), + Ok(1) + ) + ); +} + +#[cfg(windows)] #[test] -fn every_removal_refuses_until_a_platform_implements_it_and_removes_nothing() { +fn every_removal_refuses_until_windows_implements_it_and_removes_nothing() { let scratch = Scratch::new("remove-stub"); let root = scratch.root(); fs::write(root.join("a"), b"alpha").unwrap(); diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs new file mode 100644 index 000000000..baffda71e --- /dev/null +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs @@ -0,0 +1,562 @@ +//! Unix removal of a file under the swap the design exists for: another actor replaces the last +//! component of the name, or the entry inside the quarantine directory, between the steps of the +//! removal. Each case forces the swap through the observer at a chosen stage and pins what happens to +//! every object (by identity where the content could be mistaken). The counter-proofs show that the swap +//! really takes place: the baseline, and the asserted state of the displaced objects. + +use std::fs; +use std::os::fd::AsFd; +use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt}; +use std::path::{Path, PathBuf}; + +use rustix::fs::{unlinkat, AtFlags}; + +use super::remove::{holds, names, removed}; +use super::Scratch; +use crate::durable::DirectoryDurability; +use crate::pinned::unix::removal::{make_usable, INSIDE}; +use crate::pinned::{PinnedDir, PinnedError, RemovalOutcome, RemovalStage, QUARANTINE_PREFIX}; + +/// A pinned directory holding `a` = "alpha", with the paths a swap needs. +struct Fixture { + /// Keeps the directories until the test ends. + _scratch: Scratch, + root: PathBuf, + aside: PathBuf, + pinned: PinnedDir, +} + +fn fixture(label: &str) -> Fixture { + let scratch = Scratch::new(label); + let root = scratch.root(); + let aside = scratch.aside(); + fs::write(root.join("a"), b"alpha").unwrap(); + let pinned = PinnedDir::open_root(&root).unwrap(); + Fixture { + _scratch: scratch, + root, + aside, + pinned, + } +} + +/// The stages of a removal, without their payload. +#[derive(Clone, Copy, PartialEq)] +enum At { + Checked, + MovedAside, + Identified, + BeforeSync, +} + +/// The stage, and the path inside the quarantine directory once there is one. +fn at(stage: RemovalStage<'_>) -> (At, Option<&Path>) { + match stage { + RemovalStage::Checked => (At::Checked, None), + RemovalStage::MovedAside { quarantined } => (At::MovedAside, Some(quarantined)), + RemovalStage::Identified { quarantined } => (At::Identified, Some(quarantined)), + RemovalStage::BeforeSync => (At::BeforeSync, None), + } +} + +/// An observer that runs `action` at the stage `here` and otherwise does nothing. +fn on( + here: At, + mut action: impl FnMut(Option<&Path>), +) -> impl FnMut(RemovalStage<'_>) -> Result<(), PinnedError> { + move |stage| { + let (now, path) = at(stage); + if now == here { + action(path); + } + Ok(()) + } +} + +/// An observer that fails at the stage `here`. +fn failing_at(here: At) -> impl FnMut(RemovalStage<'_>) -> Result<(), PinnedError> { + move |stage| { + if at(stage).0 == here { + return Err(denied()); + } + Ok(()) + } +} + +fn denied() -> PinnedError { + PinnedError::Io(std::io::ErrorKind::PermissionDenied) +} + +/// The last-component swap: the original is renamed out of the way and a live file takes its name. +fn swap_in_a_live_file(root: &Path, aside: &Path) { + fs::rename(root.join("a"), aside.join("original")).unwrap(); + fs::write(root.join("a"), b"live").unwrap(); +} + +/// The inode of the object at `path` (not following a link): who is who, whatever the content says. +fn ino(path: &Path) -> u64 { + fs::symlink_metadata(path).unwrap().ino() +} + +/// The quarantine directories a removal left in `root`. +fn quarantines(root: &Path) -> Vec { + let mut found: Vec = fs::read_dir(root) + .unwrap() + .map(|entry| entry.unwrap().path()) + .filter(|path| { + path.file_name() + .unwrap() + .to_string_lossy() + .starts_with(QUARANTINE_PREFIX) + }) + .collect(); + found.sort(); + found +} + +/// The content of the entry left in the only quarantine directory of `root`. +fn held_in_quarantine(root: &Path) -> Option> { + let all = quarantines(root); + assert!(all.len() <= 1, "{all:?}"); + fs::read(all.first()?.join(INSIDE)).ok() +} + +/// A name-based removal: identify the entry, let `swap` run, then unlink the name. +fn naive_removal(pinned: &PinnedDir, name: &str, swap: impl FnOnce()) -> rustix::io::Result<()> { + let authenticated = pinned.open_file(name).unwrap(); + assert!(pinned.entry_kind(name).is_ok()); + swap(); + drop(authenticated); + unlinkat(pinned.handle.as_fd(), name, AtFlags::empty()) +} + +#[test] +fn the_swap_destroys_the_live_file_of_a_name_based_removal_so_the_hook_forces_the_race() { + let f = fixture("baseline"); + let result = naive_removal(&f.pinned, "a", || swap_in_a_live_file(&f.root, &f.aside)); + assert_eq!( + ( + result, + f.root.join("a").exists(), + fs::read(f.aside.join("original")).unwrap() + ), + (Ok(()), false, b"alpha".to_vec()) + ); +} + +#[test] +fn a_swap_of_the_last_name_after_the_check_destroys_nothing() { + let f = fixture("swap"); + let original = ino(&f.root.join("a")); + let mut live = 0; + let outcome = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::Checked, |_| { + swap_in_a_live_file(&f.root, &f.aside); + live = ino(&f.root.join("a")); + }), + ); + assert_eq!( + ( + outcome, + ino(&f.root.join("a")) == live, + ino(&f.aside.join("original")) == original, + names(&f.pinned) + ), + ( + Ok(RemovalOutcome::Swapped { restored: true }), + true, + true, + vec!["a".to_string()] + ) + ); +} + +/// What a swap puts at the name, and whether it is still there afterwards. +type Swap = (&'static str, fn(&Path, &Path), fn(&Path, &Path) -> bool); + +/// The swaps that the removal must put back untouched. +fn swaps() -> [Swap; 4] { + [ + ( + "directory", + |root, aside| { + fs::rename(root.join("a"), aside.join("original")).unwrap(); + fs::create_dir(root.join("a")).unwrap(); + fs::write(root.join("a").join("inside"), b"inside").unwrap(); + }, + |root, _| fs::read(root.join("a").join("inside")).ok() == Some(b"inside".to_vec()), + ), + ( + "symlink", + |root, aside| { + fs::rename(root.join("a"), aside.join("original")).unwrap(); + fs::write(aside.join("target"), b"target").unwrap(); + std::os::unix::fs::symlink(aside.join("target"), root.join("a")).unwrap(); + }, + |root, aside| fs::read_link(root.join("a")).ok() == Some(aside.join("target")), + ), + ( + "fifo", + |root, aside| { + fs::rename(root.join("a"), aside.join("original")).unwrap(); + let made = std::process::Command::new("mkfifo") + .arg(root.join("a")) + .status() + .unwrap(); + assert!(made.success()); + }, + |root, _| fs::symlink_metadata(root.join("a")).is_ok_and(|m| m.file_type().is_fifo()), + ), + ( + // A file this process cannot open although it could open the authenticated one. + "unopenable", + |root, aside| { + swap_in_a_live_file(root, aside); + fs::set_permissions(root.join("a"), fs::Permissions::from_mode(0o000)).unwrap(); + }, + |root, _| fs::metadata(root.join("a")).unwrap().permissions().mode() & 0o777 == 0, + ), + ] +} + +#[test] +fn whatever_is_swapped_in_is_put_back_untouched() { + let outcomes: Vec<_> = swaps() + .into_iter() + .map(|(label, swap, intact)| { + let f = fixture(label); + let outcome = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::Checked, |_| swap(&f.root, &f.aside)), + ); + ( + label, + outcome, + intact(&f.root, &f.aside), + fs::read(f.aside.join("original")).unwrap(), + quarantines(&f.root).len(), + ) + }) + .collect(); + let put_back = |label| { + let swapped = Ok(RemovalOutcome::Swapped { restored: true }); + (label, swapped, true, b"alpha".to_vec(), 0) + }; + assert_eq!( + outcomes, + [ + put_back("directory"), + put_back("symlink"), + put_back("fifo"), + put_back("unopenable") + ] + ); +} + +#[test] +fn a_name_taken_again_while_the_swapped_file_is_aside_leaves_every_object_intact() { + let f = fixture("retaken"); + let mut at_checked = on(At::Checked, |_| swap_in_a_live_file(&f.root, &f.aside)); + let mut at_moved = on(At::MovedAside, |_| { + fs::write(f.root.join("a"), b"retaken").unwrap() + }); + let outcome = f + .pinned + .remove_authenticated_file_observed("a", holds(b"alpha"), &mut |stage| { + at_checked(stage)?; + at_moved(stage) + }); + assert_eq!( + ( + outcome, + fs::read(f.root.join("a")).unwrap(), + fs::read(f.aside.join("original")).unwrap(), + held_in_quarantine(&f.root) + ), + ( + Ok(RemovalOutcome::Swapped { restored: false }), + b"retaken".to_vec(), + b"alpha".to_vec(), + Some(b"live".to_vec()) + ) + ); +} + +#[test] +fn a_swap_inside_the_quarantine_directory_before_the_identification_is_never_unlinked() { + let f = fixture("inside"); + let outcome = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::MovedAside, |quarantined| { + let quarantined = quarantined.unwrap(); + fs::rename(quarantined, f.aside.join("moved-original")).unwrap(); + fs::write(quarantined, b"foreign").unwrap(); + }), + ); + assert_eq!( + ( + outcome, + fs::read(f.root.join("a")).unwrap(), + fs::read(f.aside.join("moved-original")).unwrap(), + names(&f.pinned) + ), + ( + Ok(RemovalOutcome::Swapped { restored: true }), + b"foreign".to_vec(), + b"alpha".to_vec(), + vec!["a".to_string()] + ) + ); +} + +/// What the actor does to the quarantine name in the last window, and what must hold afterwards. +type Window = (&'static str, fn(&Path, &Path), fn(&Fixture) -> bool); + +#[test] +fn the_last_window_is_forced_and_what_it_does_is_pinned() { + // No Unix primitive unlinks "this inode": after the last identification the unlink removes whatever + // the name is bound to. A foreign file that wins the race is lost and reported as `Disturbed`; a + // directory is never unlinked and goes back; a hard link made in the window leaves the object alive. + let windows: [Window; 3] = [ + ( + "foreign-file", + |quarantined, aside| { + fs::rename(quarantined, aside.join("original")).unwrap(); + fs::write(quarantined, b"foreign").unwrap(); + }, + |f| !f.root.join("a").exists(), + ), + ( + "foreign-directory", + |quarantined, aside| { + fs::rename(quarantined, aside.join("original")).unwrap(); + fs::create_dir(quarantined).unwrap(); + fs::write(quarantined.join("inside"), b"inside").unwrap(); + }, + |f| fs::read(f.root.join("a").join("inside")).ok() == Some(b"inside".to_vec()), + ), + ( + "hard-link", + |quarantined, aside| fs::hard_link(quarantined, aside.join("original")).unwrap(), + |f| !f.root.join("a").exists(), + ), + ]; + let outcomes: Vec<_> = windows + .into_iter() + .map(|(label, action, expected)| { + let f = fixture(label); + let outcome = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::Identified, |quarantined| { + action(quarantined.unwrap(), &f.aside) + }), + ); + ( + label, + outcome.is_ok(), + matches!(outcome, Ok(RemovalOutcome::Disturbed)), + expected(&f), + fs::read(f.aside.join("original")).unwrap(), + quarantines(&f.root).len(), + ) + }) + .collect(); + let alpha = || b"alpha".to_vec(); + assert_eq!( + outcomes, + [ + ("foreign-file", true, true, true, alpha(), 0), + ("foreign-directory", false, false, true, alpha(), 0), + ("hard-link", true, false, true, alpha(), 0) + ] + ); +} + +#[test] +fn a_name_taken_after_the_move_keeps_the_new_file_and_a_vanished_name_is_reported() { + let taken = fixture("taken-after"); + let after = taken.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::MovedAside, |_| { + fs::write(taken.root.join("a"), b"new").unwrap() + }), + ); + let gone = fixture("vanish"); + let vanished = gone.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::Checked, |_| { + fs::remove_file(gone.root.join("a")).unwrap() + }), + ); + assert_eq!( + ( + after, + fs::read(taken.root.join("a")).unwrap(), + vanished, + names(&gone.pinned) + ), + ( + removed(), + b"new".to_vec(), + Ok(RemovalOutcome::Vanished), + Vec::::new() + ) + ); +} + +#[test] +fn renaming_the_root_during_the_removal_does_not_redirect_it() { + let f = fixture("root-renamed"); + let outcome = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::Checked, |_| { + fs::rename(&f.root, f.aside.join("moved-root")).unwrap(); + fs::create_dir(&f.root).unwrap(); + fs::write(f.root.join("a"), b"decoy").unwrap(); + }), + ); + let moved = fs::read_dir(f.aside.join("moved-root")).unwrap().count(); + assert_eq!( + (outcome, fs::read(f.root.join("a")).unwrap(), moved), + (removed(), b"decoy".to_vec(), 0) + ); +} + +#[test] +fn the_quarantine_directory_is_private_usable_under_any_umask_and_gone_afterwards() { + let f = fixture("private"); + let mut seen = None; + let outcome = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::MovedAside, |quarantined| { + let directory = quarantined.unwrap().parent().unwrap(); + seen = Some(fs::metadata(directory).unwrap().permissions().mode() & 0o777); + }), + ); + // A restrictive umask leaves a directory that nothing can be moved into; it is made usable. + let under_umask = format!("{QUARANTINE_PREFIX}{}", "a".repeat(32)); + fs::create_dir(f.root.join(&under_umask)).unwrap(); + fs::set_permissions(f.root.join(&under_umask), fs::Permissions::from_mode(0o500)).unwrap(); + let made = make_usable(&f.pinned.handle, under_umask.as_ref()); + let mode = fs::metadata(f.root.join(&under_umask)) + .unwrap() + .permissions() + .mode(); + assert_eq!( + ( + outcome, + seen, + made, + mode & 0o777, + quarantines(&f.root).len() + ), + (removed(), Some(0o700), Ok(()), 0o700, 1) + ); +} + +#[test] +fn a_failure_at_any_stage_leaves_the_directory_as_it_was_found() { + let outcomes: Vec<_> = [At::Checked, At::MovedAside, At::Identified] + .into_iter() + .map(|here| { + let f = fixture("rollback"); + let outcome = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut failing_at(here), + ); + ( + outcome, + fs::read(f.root.join("a")).unwrap(), + quarantines(&f.root).len(), + ) + }) + .collect(); + assert_eq!(outcomes, vec![(Err(denied()), b"alpha".to_vec(), 0); 3]); +} + +#[test] +fn a_failed_sync_is_reported_for_a_removal_and_for_a_put_back_and_is_repeated_by_itself() { + let removal = fixture("sync-removal"); + let first = removal.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut failing_at(At::BeforeSync), + ); + // A rerun finds nothing to do and does not sync again; the directory sync is repeated by itself. + let rerun = removal + .pinned + .remove_authenticated_file("a", holds(b"alpha")); + let put_back = fixture("sync-put-back"); + let mut at_checked = on(At::Checked, |_| { + swap_in_a_live_file(&put_back.root, &put_back.aside) + }); + let swapped = + put_back + .pinned + .remove_authenticated_file_observed("a", holds(b"alpha"), &mut |stage| { + at_checked(stage)?; + failing_at(At::BeforeSync)(stage) + }); + assert_eq!( + ( + first, + rerun, + removal.pinned.sync(), + swapped, + fs::read(put_back.root.join("a")).unwrap() + ), + ( + Err(denied()), + Err(PinnedError::NotFound), + Ok(DirectoryDurability::Confirmed), + Err(denied()), + b"live".to_vec() + ) + ); +} + +/// The quarantine directory loses its search permission once the entry is inside: the inspection fails +/// and the entry cannot be put back either. +fn lock_the_quarantine(quarantined: Option<&Path>) { + let directory = quarantined.unwrap().parent().unwrap(); + fs::set_permissions(directory, fs::Permissions::from_mode(0o000)).unwrap(); +} + +#[test] +fn an_error_after_the_move_that_cannot_be_rolled_back_is_reported_as_stranded() { + if rustix::process::geteuid().is_root() { + // Root is not kept out by permissions; the case is about the permission failure itself. + eprintln!("skipped: permissions do not apply to root"); + return; + } + let f = fixture("stranded"); + let original = ino(&f.root.join("a")); + let outcome = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::MovedAside, lock_the_quarantine), + ); + let left = quarantines(&f.root); + // Opened up again, the entry is where the outcome said it is, and is the authenticated object. + fs::set_permissions(&left[0], fs::Permissions::from_mode(0o700)).unwrap(); + let same_object = ino(&left[0].join(INSIDE)) == original; + assert_eq!( + (outcome, f.root.join("a").exists(), left.len(), same_object), + ( + Ok(RemovalOutcome::Stranded { cause: denied() }), + false, + 1, + true + ) + ); +} diff --git a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs index 67be724e9..88333ca43 100644 --- a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs +++ b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs @@ -1,20 +1,345 @@ -//! Unix removal, test-support only: not implemented in this slice. Every call refuses with -//! `Unsupported`; the successor slices bring the quarantine-based removal of files, then of directories -//! and leftovers. See the parent `removal` module for the contract. +//! Unix removal, test-support only: move the entry into a quarantine directory, identify the open +//! object there, and only then unlink it. See the parent `removal` module for the reasoning and for the +//! one window that no Unix primitive closes. This part removes files; directories and the leftover +//! recovery follow and refuse with `Unsupported` until then. -use std::ffi::OsStr; +use std::ffi::{OsStr, OsString}; +use std::fs::File; +use std::os::unix::fs::MetadataExt; +use std::path::{Path, PathBuf}; -use crate::pinned::{Observer, PinnedDir, PinnedError, PinnedFile, RemovalOutcome}; +use rustix::fs::{ + fchmod, fstat, mkdirat, openat, renameat_with, unlinkat, AtFlags, Mode, RenameFlags, +}; +use rustix::io::Errno; +use rustix::process::geteuid; + +use super::{dir_flags, file_flags, io_error, kind_of, stat_child, widen, DirHandle}; +use crate::durable::{DirectoryDurability, EntryKind}; +use crate::pinned::{ + ObjectIdentity, Observer, PinnedDir, PinnedError, PinnedFile, RemovalOutcome, RemovalStage, + QUARANTINE_PREFIX, +}; +use crate::random::{OsRandom, RandomSource}; + +/// The name an entry has inside the quarantine directory. +pub(in crate::pinned) const INSIDE: &str = "x"; + +/// A quarantine directory inside the pinned parent, held by descriptor. Whatever it holds is outside +/// every name grammar of the store and is never removed by a sweep. +struct Quarantine { + fd: File, + name: OsString, +} + +/// What is bound to the quarantine name right now. +enum Inside { + Absent, + /// Not the kind that was asked for (a link, a directory instead of a file, a FIFO), or not openable + /// by this process although the authenticated object was. + Foreign, + /// Opened, with the identity of the open object. + Object(File, ObjectIdentity), +} + +/// How a removal ended before the parent directory was synced. +enum Step { + /// Nothing in the directory changed. + Unchanged(RemovalOutcome), + /// The directory changed (an entry was moved and put back, or a quarantine directory came and + /// went); the outcome is final. + Changed(RemovalOutcome), + /// The object lost its name. + Unlinked, +} + +impl Quarantine { + fn create(parent: &DirHandle) -> Result { + let mut bits = [0u8; 16]; + OsRandom + .fill(&mut bits) + .map_err(|_| PinnedError::Unsupported)?; + let suffix: String = bits.iter().map(|byte| format!("{byte:02x}")).collect(); + let name = OsString::from(format!("{QUARANTINE_PREFIX}{suffix}")); + mkdirat(parent, &name, Mode::RWXU).map_err(io_error)?; + match Self::made_usable(parent, &name) { + Ok(fd) => Ok(Quarantine { fd, name }), + Err(error) => { + let _ = unlinkat(parent, &name, AtFlags::REMOVEDIR); + Err(error) + } + } + } + + /// The directory just created, with the owner's access set explicitly (a restrictive umask would + /// otherwise leave a directory that nothing can be moved into) and checked. + fn made_usable(parent: &DirHandle, name: &OsStr) -> Result { + let fd = Self::open_owned(parent, name)?; + fchmod(&fd, Mode::RWXU).map_err(io_error)?; + let mode = Self::mode_of(&fd)?; + if mode & 0o077 == 0 && mode & 0o700 == 0o700 { + Ok(fd) + } else { + Err(PinnedError::Unsupported) + } + } + + /// The directory `name`, if it is a directory of this process's user (one of another user that was + /// swapped in is refused; the same user is not kept out, which the removal does not rely on). + fn open_owned(parent: &DirHandle, name: &OsStr) -> Result { + let fd = File::from(openat(parent, name, dir_flags(), Mode::empty()).map_err(io_error)?); + let stat = fstat(&fd).map_err(io_error)?; + if kind_of(&stat) == EntryKind::Directory && stat.st_uid == geteuid().as_raw() { + Ok(fd) + } else { + Err(PinnedError::Unsupported) + } + } + + fn mode_of(fd: &File) -> Result { + widen(fstat(fd).map_err(io_error)?.st_mode) + } + + /// Moves what `name` of `parent` is bound to right now into this directory; `false` if the name + /// leads nowhere. + fn adopt(&self, parent: &DirHandle, name: &OsStr) -> Result { + match renameat_with(parent, name, &self.fd, INSIDE, RenameFlags::NOREPLACE) { + Ok(()) => Ok(true), + Err(Errno::NOENT) => Ok(false), + // No RENAME_NOREPLACE here: nothing was moved, so nothing can be identified safely. + Err(Errno::INVAL | Errno::NOSYS | Errno::NOTSUP) => Err(PinnedError::Unsupported), + Err(errno) => Err(io_error(errno)), + } + } + + /// Looks at the entry before it is opened (opening a device node runs its driver), then opens it + /// without following a link and reports the identity of the open object. + fn inspect(&self, kind: EntryKind) -> Result { + let inside = OsStr::new(INSIDE); + match stat_child(&self.fd, inside)? { + None => return Ok(Inside::Absent), + Some(stat) if kind_of(&stat) != kind => return Ok(Inside::Foreign), + Some(_) => {} + } + let flags = if kind == EntryKind::Directory { + dir_flags() + } else { + file_flags() + }; + let object = match openat(&self.fd, inside, flags, Mode::empty()) { + Ok(fd) => File::from(fd), + Err(Errno::NOENT) => return Ok(Inside::Absent), + // Swapped between the look and the open, or not openable: not the object that was opened + // before, which this process could open. + Err(Errno::LOOP | Errno::NOTDIR | Errno::ISDIR | Errno::NXIO | Errno::ACCESS) => { + return Ok(Inside::Foreign) + } + Err(errno) => return Err(io_error(errno)), + }; + let metadata = object.metadata().map_err(PinnedError::from)?; + let right_kind = if kind == EntryKind::Directory { + metadata.is_dir() + } else { + metadata.is_file() + }; + if !right_kind { + return Ok(Inside::Foreign); + } + let identity = ObjectIdentity::new(metadata.dev(), u128::from(metadata.ino())); + Ok(Inside::Object(object, identity)) + } + + /// Moves the entry back under `name`; an error if that was not possible (the name is taken, or + /// anything else prevents it), in which case the entry stays here. + fn restore(&self, parent: &DirHandle, name: &OsStr) -> Result<(), PinnedError> { + renameat_with(&self.fd, INSIDE, parent, name, RenameFlags::NOREPLACE).map_err(io_error) + } + + fn path_inside(&self, parent_path: &Path) -> PathBuf { + parent_path.join(&self.name).join(INSIDE) + } + + /// Removes this directory if it is empty. A directory that still holds an entry is left. + fn discard(&self, parent: &DirHandle) { + let _ = unlinkat(parent, &self.name, AtFlags::REMOVEDIR); + } +} + +/// Makes the quarantine directory `name` of `parent` usable the way [`Quarantine::create`] does: the +/// owner's access is set explicitly, whatever the umask did to the mode it was created with. +#[cfg(test)] +pub(in crate::pinned) fn make_usable(parent: &DirHandle, name: &OsStr) -> Result<(), PinnedError> { + Quarantine::made_usable(parent, name).map(|_| ()) +} + +/// Unlinks the entry that was identified in the quarantine directory and asks the open object whether +/// it was the one that lost a name: its link count must have dropped. That is a check after the fact +/// and a heuristic, not a proof of identity (see the residual in the parent module): an actor that also +/// changes the link count of the authenticated object in the same window is not noticed. +fn unlink_verified( + quarantine: &Quarantine, + object: &File, + kind: EntryKind, +) -> Result { + let before = object.metadata().map_err(PinnedError::from)?.nlink(); + let flags = if kind == EntryKind::Directory { + AtFlags::REMOVEDIR + } else { + AtFlags::empty() + }; + match unlinkat(&quarantine.fd, INSIDE, flags) { + Ok(()) => {} + Err(Errno::NOTEMPTY | Errno::EXIST) if kind == EntryKind::Directory => { + return Ok(RemovalOutcome::NotEmpty) + } + Err(Errno::NOENT) => return Ok(RemovalOutcome::Vanished), + Err(errno) => return Err(io_error(errno)), + } + // Nothing can be said about a removal that cannot be checked afterwards. + let Ok(after) = object.metadata().map(|metadata| metadata.nlink()) else { + return Ok(RemovalOutcome::Disturbed); + }; + if after < before { + Ok(RemovalOutcome::Removed(DirectoryDurability::Confirmed)) + } else { + Ok(RemovalOutcome::Disturbed) + } +} + +/// The entry of a removal: its name, the object it must turn out to be and the kind it must have. +struct Target<'a> { + name: &'a OsStr, + expected: ObjectIdentity, + kind: EntryKind, +} + +/// One removal in progress: the directory, the entry and the quarantine directory it moves through. +struct Removal<'a> { + dir: &'a PinnedDir, + target: Target<'a>, + quarantine: Quarantine, +} pub(in crate::pinned) fn remove_file( - _dir: &PinnedDir, - _name: &OsStr, - _authenticate: impl FnOnce(&mut PinnedFile) -> Result, - _observer: Observer<'_>, + dir: &PinnedDir, + name: &OsStr, + authenticate: impl FnOnce(&mut PinnedFile) -> Result, + observer: Observer<'_>, ) -> Result { - Err(PinnedError::Unsupported) + let mut file = dir.open_file(name)?; + if !authenticate(&mut file)? { + return Ok(RemovalOutcome::Refused); + } + observer(RemovalStage::Checked)?; + let target = Target { + name, + expected: file.identity(), + kind: EntryKind::File, + }; + remove_moved(dir, target, observer) +} + +/// Removes the entry of `target` from `dir` if, once moved aside, it is the object it must be. +fn remove_moved( + dir: &PinnedDir, + target: Target<'_>, + observer: Observer<'_>, +) -> Result { + let removal = Removal { + dir, + target, + quarantine: Quarantine::create(&dir.handle)?, + }; + let step = removal.detach(observer); + removal.quarantine.discard(&dir.handle); + settle(dir, step?, observer) +} + +impl Removal<'_> { + /// Moves the entry into the quarantine directory and judges what arrived there. Whatever fails after + /// the move puts the entry back first; if that is impossible the removal says so + /// ([`RemovalOutcome::Stranded`]) instead of reporting an error that hides where the entry is. + fn detach(&self, observer: Observer<'_>) -> Result { + if !self.quarantine.adopt(&self.dir.handle, self.target.name)? { + return Ok(Step::Unchanged(RemovalOutcome::Vanished)); + } + match self.judge(observer) { + Err(cause) => match self.restore() { + Ok(()) => Err(cause), + Err(_) => Ok(Step::Unchanged(RemovalOutcome::Stranded { cause })), + }, + step => step, + } + } + + fn judge(&self, observer: Observer<'_>) -> Result { + let inside = self.quarantine.path_inside(&self.dir.path); + observer(RemovalStage::MovedAside { + quarantined: &inside, + })?; + match self.quarantine.inspect(self.target.kind)? { + Inside::Absent => Ok(Step::Changed(RemovalOutcome::Vanished)), + Inside::Object(object, found) if found == self.target.expected => { + self.conclude(&object, observer) + } + Inside::Object(..) | Inside::Foreign => { + let restored = self.restore().is_ok(); + Ok(Step::Changed(RemovalOutcome::Swapped { restored })) + } + } + } + + /// The last window: the entry in the quarantine directory is the authenticated object. + fn conclude(&self, object: &File, observer: Observer<'_>) -> Result { + let inside = self.quarantine.path_inside(&self.dir.path); + observer(RemovalStage::Identified { + quarantined: &inside, + })?; + match unlink_verified(&self.quarantine, object, self.target.kind)? { + // A directory that filled after the check goes back where it was. + RemovalOutcome::NotEmpty => { + self.restore()?; + Ok(Step::Changed(RemovalOutcome::NotEmpty)) + } + outcome => Ok(unlinked(outcome)), + } + } + + fn restore(&self) -> Result<(), PinnedError> { + self.quarantine.restore(&self.dir.handle, self.target.name) + } +} + +fn unlinked(outcome: RemovalOutcome) -> Step { + match outcome { + RemovalOutcome::Removed(_) => Step::Unlinked, + other => Step::Changed(other), + } +} + +/// Syncs the parent directory if anything in it changed. A sync that fails is an error although the +/// entry is gone or back: a crash may undo that, and the caller repeats [`PinnedDir::sync`] (a rerun of +/// the removal finds nothing to do and would not sync). +fn settle( + dir: &PinnedDir, + step: Step, + observer: Observer<'_>, +) -> Result { + match step { + Step::Unchanged(outcome) => Ok(outcome), + Step::Changed(outcome) => { + observer(RemovalStage::BeforeSync)?; + dir.sync()?; + Ok(outcome) + } + Step::Unlinked => { + observer(RemovalStage::BeforeSync)?; + Ok(RemovalOutcome::Removed(dir.sync()?)) + } + } } +/// Removing a directory follows in the next part. pub(in crate::pinned) fn remove_dir( _dir: &PinnedDir, _name: &OsStr, @@ -23,6 +348,7 @@ pub(in crate::pinned) fn remove_dir( Err(PinnedError::Unsupported) } +/// Finishing a leftover quarantine directory follows in the next part. pub(in crate::pinned) fn remove_leftover( _dir: &PinnedDir, _quarantine: &OsStr, diff --git a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md index f83acd72a..c20255dc8 100644 --- a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md +++ b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md @@ -1736,3 +1736,47 @@ production sweep is a decision of the maintainer, to be taken before H1b**. **Not claimed.** No removal is implemented in this part; no caller exists and the sweep is not ported (H1b); H1c and I1c are not started. + +#### Part 2: removal of a file on Unix + +Status words: `H1A_2_UNIX_FILE_REMOVAL=IMPLEMENTED_TEST_SUPPORT_ONLY`, `H1_PROVED=NO`. The authenticated file is held open with +identity `(dev, ino)`. A quarantine directory (`.h1q-` and 32 hexadecimal digits from the operating system's random source, +a directory of the effective user, mode 0700 set explicitly with `fchmod` so that a restrictive umask cannot leave a +directory nothing can be moved into) is created in the pinned parent and held by descriptor, and +`renameat2(RENAME_NOREPLACE)` (macOS `renameatx_np(RENAME_EXCL)`) moves whatever the name is bound to at that instant into +it; nothing is overwritten. Only there is the entry identified: it is looked at (a device node is not opened), opened without +following a link, and the identity of the open object is compared. If it is the authenticated object it is unlinked from the +quarantine directory; anything else (a live file, a link, a directory, a file this process cannot open) is moved back +(`Swapped { restored: true }`) or, if the name was taken meanwhile, kept in the quarantine directory (`restored: false`), and +never unlinked. A file system without `RENAME_NOREPLACE` answers `Unsupported` before anything is touched (not exercised on +the hosted runners). **Every failure after the move is handled at one point:** the entry is put back and the error is +returned, or, if it cannot be put back, the outcome is `Stranded { cause }` (the entry is in the quarantine directory as `x`, +nothing was removed) and not an error that hides where the entry is. The parent is synced whenever the directory changed; a +sync that fails is an error although the entry is gone or back, and the caller repeats `PinnedDir::sync` (a rerun of the +removal finds nothing to do and would not sync). + +**The window, stated.** The quarantine directory is not exclusive: another process of the same user can list the parent, find +it and enter it. The design holds when it is manipulated at every point before the last identification (tests below). After the +last identification no Unix primitive unlinks "this inode": the unlink removes the name's current binding, so an actor that +wins that one race can have its own file removed instead. This is **detected after the fact, not prevented**: the link count of +the open authenticated object must have dropped, otherwise the outcome is `Disturbed` (also when the count cannot be read +afterwards), which a caller never counts as a success. The check is a heuristic: an actor that also changes the link count of the +authenticated object in the same window is not noticed (Codex, PR #1068). A directory in that window is not unlinked by a file +removal (it goes back). Whether this is acceptable for a sweep is the maintainer's decision before H1b. + +**Proof (lib unit tests, so that the macOS evidence job runs them too).** The baseline runs the naive check-then-`unlinkat` +against the same swap and shows the live file destroyed, so the observer forces the race. A swap of the last name; a directory, +a symlink, a FIFO and a file this process cannot open swapped in (each put back, identified by inode, none left in quarantine); +a name taken again while the entry is aside (kept in quarantine, both live objects intact); a swap inside the quarantine +directory before the identification; the last window forced with a foreign file (`Disturbed`, the original intact), a +directory (goes back) and a hard link (object alive under the other name); a name taken after the move, a vanished name, +renaming the root during the removal; the quarantine directory is mode 0700 and gone afterwards, and a directory made with +mode 0500 is made usable; an injected failure at each stage leaves the directory as it was found; a failed sync is reported +for a removal and for a put-back and repeated by `sync`; a quarantine directory that loses its search permission after the +move gives `Stranded` with the entry still the authenticated inode. Mutation-checked: skipping the identity comparison fails +five of these cases, skipping the link-count verification fails the forced last window, and removing the rollback of a +failed inspection fails the `Stranded` case. + +**Not claimed.** Directories and the leftover recovery are part 3 (they answer `Unsupported` here); Windows is part 4; the +Disturbed window is bounded and detected, not closed; a device node swapped in is refused before it is opened and cannot be +created by an unprivileged actor, which is argued, not tested.