diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a72b160b..4aa4cb234 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 directory on Unix, the recovery of leftovers and the crash cases, `test-support` only (slice H1a-2, part 3 of 4). `remove_empty_dir` moves the directory through the same quarantine directory and checks the link count of the held directory afterwards; `remove_leftover_file` takes a leftover `.h1q-*` directory by its generated name only, authenticates the file in it again and removes it only if accepted. A simulated crash, an entry stranded by a lost permission and a foreign object left by a refused put-back are covered. No new `unsafe`, no new package. Issue #359. PR #1070. + - **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. diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove.rs index 03bfa29e7..75039ba93 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/remove.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove.rs @@ -147,6 +147,40 @@ fn removing_a_hard_linked_name_leaves_the_other_names() { ); } +#[cfg(unix)] +#[test] +fn an_empty_directory_is_removed_and_a_filled_one_or_a_link_is_not() { + let scratch = Scratch::new("dirs"); + let root = scratch.root(); + let aside = scratch.aside(); + fs::create_dir(root.join("empty")).unwrap(); + fs::create_dir(root.join("filled")).unwrap(); + fs::write(root.join("filled").join("inside"), b"inside").unwrap(); + fs::create_dir(aside.join("target")).unwrap(); + link_to_dir(&aside.join("target"), &root.join("link")); + let pinned = PinnedDir::open_root(&root).unwrap(); + assert_eq!( + ( + pinned.remove_empty_dir("empty"), + pinned.remove_empty_dir("filled"), + pinned.remove_empty_dir("link"), + pinned.remove_empty_dir("missing"), + fs::read(root.join("filled").join("inside")).unwrap(), + aside.join("target").is_dir(), + names(&pinned) + ), + ( + removed(), + Ok(RemovalOutcome::NotEmpty), + Err(PinnedError::Redirected), + Err(PinnedError::NotFound), + b"inside".to_vec(), + true, + vec!["filled".to_string(), "link".to_string()] + ) + ); +} + #[cfg(windows)] #[test] fn every_removal_refuses_until_windows_implements_it_and_removes_nothing() { diff --git a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs index cf62aa1cf..fbb0a54a9 100644 --- a/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs +++ b/crates/worldscript-secure-storage/src/pinned/tests/remove_unix.rs @@ -1,4 +1,4 @@ -//! Unix removal of a file under the swap the design exists for: another actor replaces the last +//! Unix removal of a file, a directory and of a leftover quarantine directory 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 @@ -7,6 +7,7 @@ use std::fs; use std::os::fd::AsFd; use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt}; +use std::panic::{catch_unwind, AssertUnwindSafe}; use std::path::{Path, PathBuf}; use rustix::fs::{unlinkat, AtFlags}; @@ -559,3 +560,190 @@ fn an_error_after_the_move_that_cannot_be_rolled_back_is_reported_as_stranded() ) ); } + +#[test] +fn a_directory_swapped_or_filled_after_the_check_is_put_back_and_not_removed() { + let f = fixture("dir-swap"); + fs::create_dir(f.root.join("d")).unwrap(); + fs::create_dir(f.root.join("e")).unwrap(); + let swapped = f.pinned.remove_empty_dir_observed( + "d", + &mut on(At::Checked, |_| { + fs::rename(f.root.join("d"), f.aside.join("original-d")).unwrap(); + fs::create_dir(f.root.join("d")).unwrap(); + fs::write(f.root.join("d").join("live"), b"live").unwrap(); + }), + ); + let filled = f.pinned.remove_empty_dir_observed( + "e", + &mut on(At::Checked, |_| { + fs::write(f.root.join("e").join("late"), b"late").unwrap() + }), + ); + assert_eq!( + ( + swapped, + fs::read(f.root.join("d").join("live")).unwrap(), + f.aside.join("original-d").is_dir(), + filled, + fs::read(f.root.join("e").join("late")).unwrap(), + names(&f.pinned) + ), + ( + Ok(RemovalOutcome::Swapped { restored: true }), + b"live".to_vec(), + true, + Ok(RemovalOutcome::NotEmpty), + b"late".to_vec(), + vec!["a".to_string(), "d".to_string(), "e".to_string()] + ) + ); +} + +#[test] +fn an_empty_directory_swapped_in_the_last_window_is_reported_as_disturbed() { + let f = fixture("window-empty-dir"); + fs::create_dir(f.root.join("d")).unwrap(); + let original = ino(&f.root.join("d")); + let outcome = f.pinned.remove_empty_dir_observed( + "d", + &mut on(At::Identified, |quarantined| { + let quarantined = quarantined.unwrap(); + fs::rename(quarantined, f.aside.join("original-d")).unwrap(); + fs::create_dir(quarantined).unwrap(); + }), + ); + assert_eq!( + ( + outcome, + f.root.join("d").exists(), + ino(&f.aside.join("original-d")) == original, + quarantines(&f.root).len() + ), + (Ok(RemovalOutcome::Disturbed), false, true, 0) + ); +} + +/// A removal that "crashes" (the observer panics) once the object is in the quarantine directory. +fn crash_after_the_move(f: &Fixture) { + let crashed = catch_unwind(AssertUnwindSafe(|| { + let _ = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::MovedAside, |_| panic!("simulated crash")), + ); + })); + assert!(crashed.is_err()); +} + +#[test] +fn a_crash_leaves_the_object_in_quarantine_and_a_leftover_removal_finishes_it_after_authenticating() +{ + let f = fixture("crash"); + crash_after_the_move(&f); + let left = quarantines(&f.root); + let name = left[0].file_name().unwrap().to_owned(); + let before = (names(&f.pinned).len(), held_in_quarantine(&f.root)); + let refused = f + .pinned + .remove_leftover_file(&name, holds(b"something else")); + let finished = f.pinned.remove_leftover_file(&name, holds(b"alpha")); + assert_eq!( + (before, refused, finished, names(&f.pinned)), + ( + (1, Some(b"alpha".to_vec())), + Ok(RemovalOutcome::Refused), + removed(), + Vec::::new() + ) + ); +} + +#[test] +fn a_leftover_is_taken_by_its_generated_name_only_and_an_empty_one_is_removed() { + let f = fixture("leftover-names"); + let generated = format!("{QUARANTINE_PREFIX}{}", "0".repeat(32)); + for name in [generated.as_str(), ".h1q-empty", "not-a-quarantine"] { + fs::create_dir(f.root.join(name)).unwrap(); + fs::set_permissions(f.root.join(name), fs::Permissions::from_mode(0o700)).unwrap(); + } + let taken = |name: &str| f.pinned.remove_leftover_file(name, |_| Ok(true)); + assert_eq!( + ( + taken("a"), + taken("not-a-quarantine"), + taken(".h1q-empty"), + taken(&format!("{QUARANTINE_PREFIX}{}", "G".repeat(32))), + taken(&generated), + names(&f.pinned) + ), + ( + Err(PinnedError::InvalidName), + Err(PinnedError::InvalidName), + Err(PinnedError::InvalidName), + Err(PinnedError::InvalidName), + Ok(RemovalOutcome::Vanished), + vec![ + ".h1q-empty".to_string(), + "a".to_string(), + "not-a-quarantine".to_string() + ] + ) + ); +} + +#[test] +fn a_foreign_object_left_in_quarantine_is_never_removed_by_a_leftover_removal() { + let f = fixture("leftover-foreign"); + 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 first = f + .pinned + .remove_authenticated_file_observed("a", holds(b"alpha"), &mut |stage| { + at_checked(stage)?; + at_moved(stage) + }); + let name = quarantines(&f.root)[0].file_name().unwrap().to_owned(); + // The object in the quarantine directory is the live file, not an object of the sweep. + let refused = f.pinned.remove_leftover_file(&name, holds(b"alpha")); + assert_eq!( + (first, refused, held_in_quarantine(&f.root)), + ( + Ok(RemovalOutcome::Swapped { restored: false }), + Ok(RemovalOutcome::Refused), + Some(b"live".to_vec()) + ) + ); +} + +#[test] +fn a_stranded_entry_is_finished_by_the_leftover_removal_after_authenticating_it() { + assert!( + !rustix::process::geteuid().is_root(), + "the permission checks of this case do not apply to root; run the tests as a normal user" + ); + let f = fixture("stranded-recovery"); + let stranded = f.pinned.remove_authenticated_file_observed( + "a", + holds(b"alpha"), + &mut on(At::MovedAside, lock_the_quarantine), + ); + let left = quarantines(&f.root); + fs::set_permissions(&left[0], fs::Permissions::from_mode(0o700)).unwrap(); + let name = left[0].file_name().unwrap().to_owned(); + let refused = f + .pinned + .remove_leftover_file(&name, holds(b"something else")); + let finished = f.pinned.remove_leftover_file(&name, holds(b"alpha")); + assert_eq!( + (stranded.is_ok(), refused, finished, names(&f.pinned)), + ( + true, + Ok(RemovalOutcome::Refused), + removed(), + Vec::::new() + ) + ); +} diff --git a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs index 88333ca43..58848fbbc 100644 --- a/crates/worldscript-secure-storage/src/pinned/unix/removal.rs +++ b/crates/worldscript-secure-storage/src/pinned/unix/removal.rs @@ -1,10 +1,10 @@ //! 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. +//! one window that no Unix primitive closes. use std::ffi::{OsStr, OsString}; use std::fs::File; +use std::os::unix::ffi::OsStrExt; use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; @@ -25,6 +25,9 @@ use crate::random::{OsRandom, RandomSource}; /// The name an entry has inside the quarantine directory. pub(in crate::pinned) const INSIDE: &str = "x"; +/// How many hexadecimal digits follow the prefix in the name of a quarantine directory. +const SUFFIX_DIGITS: usize = 32; + /// 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 { @@ -53,6 +56,17 @@ enum Step { Unlinked, } +/// Whether `name` is exactly what [`Quarantine::create`] produces. +fn is_generated_name(name: &OsStr) -> bool { + let bytes = name.as_bytes(); + let prefix = QUARANTINE_PREFIX.as_bytes(); + bytes.len() == prefix.len() + SUFFIX_DIGITS + && bytes.starts_with(prefix) + && bytes[prefix.len()..] + .iter() + .all(|digit| matches!(digit, b'0'..=b'9' | b'a'..=b'f')) +} + impl Quarantine { fn create(parent: &DirHandle) -> Result { let mut bits = [0u8; 16]; @@ -84,6 +98,21 @@ impl Quarantine { } } + /// An existing quarantine directory of `parent`, found by its generated name. + fn open_existing(parent: &DirHandle, name: &OsStr) -> Result { + if !is_generated_name(name) { + return Err(PinnedError::InvalidName); + } + let fd = Self::open_owned(parent, name)?; + if Self::mode_of(&fd)? & 0o077 != 0 { + return Err(PinnedError::Unsupported); + } + Ok(Quarantine { + fd, + name: name.to_os_string(), + }) + } + /// 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 { @@ -239,6 +268,65 @@ pub(in crate::pinned) fn remove_file( remove_moved(dir, target, observer) } +pub(in crate::pinned) fn remove_dir( + dir: &PinnedDir, + name: &OsStr, + observer: Observer<'_>, +) -> Result { + let child = dir.open_dir(name)?; + match child.entries(0) { + Ok(_) => {} + Err(PinnedError::TooManyEntries) => return Ok(RemovalOutcome::NotEmpty), + Err(error) => return Err(error), + } + observer(RemovalStage::Checked)?; + let target = Target { + name, + expected: child.identity(), + kind: EntryKind::Directory, + }; + remove_moved(dir, target, observer) +} + +/// Removes the leftover file of the quarantine directory `quarantine` after authenticating it again. +pub(in crate::pinned) fn remove_leftover( + dir: &PinnedDir, + quarantine: &OsStr, + authenticate: impl FnOnce(&mut PinnedFile) -> Result, + observer: Observer<'_>, +) -> Result { + let held = Quarantine::open_existing(&dir.handle, quarantine)?; + let step = leftover_step(&held, dir, authenticate, observer); + held.discard(&dir.handle); + settle(dir, step?, observer) +} + +fn leftover_step( + held: &Quarantine, + dir: &PinnedDir, + authenticate: impl FnOnce(&mut PinnedFile) -> Result, + observer: Observer<'_>, +) -> Result { + let (file, identity) = match held.inspect(EntryKind::File)? { + // An empty quarantine directory is junk and goes with `discard`. + Inside::Absent => return Ok(Step::Changed(RemovalOutcome::Vanished)), + Inside::Foreign => return Ok(Step::Unchanged(RemovalOutcome::Refused)), + Inside::Object(file, identity) => (file, identity), + }; + let mut pinned = PinnedFile { file, identity }; + if !authenticate(&mut pinned)? { + return Ok(Step::Unchanged(RemovalOutcome::Refused)); + } + observer(RemovalStage::Identified { + quarantined: &held.path_inside(&dir.path), + })?; + Ok(unlinked(unlink_verified( + held, + &pinned.file, + EntryKind::File, + )?)) +} + /// Removes the entry of `target` from `dir` if, once moved aside, it is the object it must be. fn remove_moved( dir: &PinnedDir, @@ -338,22 +426,3 @@ fn settle( } } } - -/// Removing a directory follows in the next part. -pub(in crate::pinned) fn remove_dir( - _dir: &PinnedDir, - _name: &OsStr, - _observer: Observer<'_>, -) -> Result { - Err(PinnedError::Unsupported) -} - -/// Finishing a leftover quarantine directory follows in the next part. -pub(in crate::pinned) fn remove_leftover( - _dir: &PinnedDir, - _quarantine: &OsStr, - _authenticate: impl FnOnce(&mut PinnedFile) -> Result, - _observer: Observer<'_>, -) -> Result { - Err(PinnedError::Unsupported) -} diff --git a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md index c20255dc8..a04939052 100644 --- a/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md +++ b/docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md @@ -1780,3 +1780,30 @@ 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. + +#### Part 3: removal of a directory on Unix, leftovers and crashes + +Status words: `H1A_2_UNIX_DIRECTORY_AND_LEFTOVERS=IMPLEMENTED_TEST_SUPPORT_ONLY`, `H1_PROVED=NO`. `remove_empty_dir` opens the child +directory (no link followed), refuses one that has entries (`NotEmpty`) and moves it through the same quarantine directory: +identified by the open directory, then `unlinkat(AT_REMOVEDIR)`; a directory that fills after the check goes back +(`NotEmpty`). The same window exists and is detected the same way: the link count of the held directory must have dropped +(`Disturbed` otherwise; for a directory it falls to zero). An empty foreign directory that wins the race in the last window +is removed instead of the authenticated one, and the removal says so; it holds no data. + +**Leftovers and crashes.** A crash after the move, or an entry that could not be put back (`Swapped { restored: false }`, +`Stranded`), leaves a `.h1q-*` directory holding the entry as `x`. A sweep never removes it by itself. +`remove_leftover_file(name, authenticate)` takes it by its **generated name only** (`.h1q-` and exactly 32 lowercase +hexadecimal digits; any other name, including an unrelated private directory, is `InvalidName` and untouched), requires a +directory of the effective user without access for group or others, authenticates the file in it again and, only if +accepted, unlinks it with the same link-count check. A file that does not authenticate (a foreign object left by a refused +put-back) stays; an empty generated quarantine directory is removed. + +**Proof.** A directory swapped in the first window and one filled after the check are put back and not removed; an empty +directory that wins the last window gives `Disturbed` with the original directory intact; a simulated crash (the observer +panics once the entry is in the quarantine directory) leaves the entry there under no name of the store, a leftover removal +with a failing authentication refuses and leaves it, and with the right one finishes it; names without the generated grammar +(`.h1q-empty`, non-hexadecimal digits, other directories, plain files) are refused; a foreign object left in a quarantine +directory by a refused put-back is never removed by it; an entry stranded by a quarantine directory that lost its search +permission is finished after the permission is back and the file authenticates again. + +**Not claimed.** Windows creates no quarantine directory (part 4); the Disturbed window is bounded and detected, not closed.