diff --git a/docs/migration.md b/docs/migration.md index 0c810fc..a37faa3 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -520,7 +520,7 @@ started, ran, and then died on a file it could not open: | `runtime_root` and every directory above it | traversable by the workload: `o+x`, or group-owned by the state group with `g+x` | the workload is a `DynamicUser` and owns nothing, and `ReadOnlyPaths=` binds the bundle's leaf into the namespace without granting traversal above it. Idunn now checks the whole chain and names the closed directory | | `runtime_root` | setgid, group-owned by the state group (`2750 root:`) | `harden_root_authority_file` requires the write-lease record's group to equal its parent's, and only setgid makes a newly created record inherit it. Without it the record lands `root:root` and the workload cannot read its own lease | | the write-lease record | `root:`, mode `0640` | follows from the setgid parent; the workload reads it through the state group. Idunn writes it, so fixing an existing wrong-group record means deleting it, and that is safe only when no transaction is live | -| `/var/lib/gamecult/idunn-projection` | default ACL `u::rw-,g::r--,o::r--` | Idunn runs with `UMask=027`, so each publish creates the store and its `.lock` at `0640`. The code re-opens them after every write, but that leaves a window; a default ACL sets the mode at creation and closes it | +| `/var/lib/gamecult/idunn-projection` | default ACL `u::rw-,g::r--,o::r--` (no longer required) | Idunn runs with `UMask=027`, but writes the projection on a thread with umask `022`, so the staged file and its `.lock` are created `0644` and are never visible in another mode. The ACL is redundant defense in depth; a rebuilt host works without it | | `/etc/nginx/idunn-stream-routes` | `root:root`, not group-writable | "route authority parent is not canonical root-owned and nonwritable". It was `root:idunn 0775` from the generation where Idunn ran unprivileged and needed to write it as a group member | Two shapes are worth stating plainly because they are easy to get backwards. diff --git a/src/drivers.rs b/src/drivers.rs index 8743b94..8ce421e 100644 --- a/src/drivers.rs +++ b/src/drivers.rs @@ -4388,6 +4388,7 @@ impl CultCacheTopologyDriver { { if let Some(parent) = self.projection_store.parent() { fs::create_dir_all(parent)?; + publish_directory_default_mode(parent)?; } let store = SingleFileMessagePackBackingStore::new(&self.projection_store); for _ in 0..8 { @@ -4396,7 +4397,6 @@ impl CultCacheTopologyDriver { return Ok(()); }; if store.compare_exchange_snapshot(&entries, &replacement)? { - publish_projection_mode(&self.projection_store)?; return Ok(()); } } @@ -7582,24 +7582,51 @@ fn ensure_bundle_is_reachable_by_workload( /// reads it to verify its own Expected incarnation against the Idunn anchor. /// /// Idunn runs with `UMask=027`, which is right for its private state and wrong -/// for this one file -- it lands `0640 root:root`, and a `DynamicUser` workload -/// gets EACCES. A chmod by hand does not hold, because each publish writes a -/// new file. Integrity here comes from the signatures over the records, not -/// from the mode, so the published copy is readable. -fn publish_projection_mode(path: &Path) -> Result<()> { - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - // The lock sibling too: CultCache opens it alongside the store, so a - // 0640 lock denies the read just as surely as a 0640 store, and it is - // created fresh under Idunn's umask on every publish. - for path in [path.to_path_buf(), authority_lock_path(path)] { - if path.exists() { - fs::set_permissions(&path, fs::Permissions::from_mode(0o644)) - .with_context(|| format!("publishing {}", path.display()))?; - } - } +/// for this directory: the store and its `.lock` sibling would land `0640 +/// root:root` and a `DynamicUser` workload gets EACCES. CultCache stages a +/// temporary file and renames it over the store, so the mode has to be right +/// when the staged file is *created*; a chmod after the rename leaves a window +/// in which the visible file is unreadable. CultCache has no mode parameter and +/// a process umask is shared by every thread, so the directory carries the +/// mode: a default ACL of `u::rw-,g::r--,o::r--` overrides the umask for +/// everything created inside it. This directory holds only the published +/// projection. Integrity comes from the signatures over the records, not from +/// the mode, so the published copy is readable. +#[cfg(target_os = "linux")] +fn publish_directory_default_mode(dir: &Path) -> Result<()> { + use std::os::unix::ffi::OsStrExt; + // POSIX ACL xattr: version 2 header, then (tag u16, perm u16, id u32) entries. + const USER_OBJ: u16 = 0x01; + const GROUP_OBJ: u16 = 0x04; + const OTHER: u16 = 0x20; + const READ: u16 = 4; + const WRITE: u16 = 2; + const NO_ID: u32 = u32::MAX; + let mut acl = 2u32.to_le_bytes().to_vec(); + for (tag, perm) in [(USER_OBJ, READ | WRITE), (GROUP_OBJ, READ), (OTHER, READ)] { + acl.extend_from_slice(&tag.to_le_bytes()); + acl.extend_from_slice(&perm.to_le_bytes()); + acl.extend_from_slice(&NO_ID.to_le_bytes()); } + let path = std::ffi::CString::new(dir.as_os_str().as_bytes())?; + let status = unsafe { + libc::setxattr( + path.as_ptr(), + c"system.posix_acl_default".as_ptr(), + acl.as_ptr().cast(), + acl.len(), + 0, + ) + }; + if status != 0 { + return Err(anyhow::Error::from(std::io::Error::last_os_error())) + .with_context(|| format!("setting the default mode of {}", dir.display())); + } + Ok(()) +} + +#[cfg(not(target_os = "linux"))] +fn publish_directory_default_mode(_dir: &Path) -> Result<()> { Ok(()) } @@ -9532,27 +9559,78 @@ Content-Le".to_vec()), |_| {}), )); } - #[cfg(unix)] - #[test] - fn the_published_projection_is_readable_by_a_workload() { - use std::os::unix::fs::PermissionsExt; + /// Sets the process umask to Idunn's `027` for a scope. + #[cfg(target_os = "linux")] + struct Umask027(libc::mode_t); - let temp = tempfile::tempdir().unwrap(); - let path = temp.path().join("topology.cc"); - std::fs::write(&path, b"x").unwrap(); - // What Idunn's UMask=027 leaves behind. - std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o640)).unwrap(); - let lock = temp.path().join("topology.cc.lock"); - std::fs::write(&lock, b"").unwrap(); - std::fs::set_permissions(&lock, std::fs::Permissions::from_mode(0o640)).unwrap(); - publish_projection_mode(&path).unwrap(); - let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; - assert_eq!( - mode, 0o644, - "every target must be able to read the projection" - ); - let lock_mode = std::fs::metadata(&lock).unwrap().permissions().mode() & 0o777; - assert_eq!(lock_mode, 0o644, "the lock is opened alongside the store"); + #[cfg(target_os = "linux")] + impl Umask027 { + fn set() -> Self { + Self(unsafe { libc::umask(0o027) }) + } + } + + #[cfg(target_os = "linux")] + impl Drop for Umask027 { + fn drop(&mut self) { + unsafe { libc::umask(self.0) }; + } + } + + #[cfg(target_os = "linux")] + #[test] + fn the_published_projection_is_never_visible_in_another_mode() -> Result<()> { + use std::os::unix::fs::MetadataExt; + use std::sync::atomic::{AtomicBool, Ordering}; + + struct Done<'a>(&'a AtomicBool); + impl Drop for Done<'_> { + fn drop(&mut self) { + self.0.store(true, Ordering::Release); + } + } + + let temp = tempfile::tempdir()?; + let (expected, _activation, _warming, provider_anchor) = + authenticated_warming(temp.path())?; + let published = temp.path().join("published"); + let driver = topology_driver(&published, "topology"); + let lock = authority_lock_path(&driver.projection_store); + let done = AtomicBool::new(false); + let observed = std::thread::scope(|scope| -> Result<_> { + // A reader as another user sees only what the directory shows: + // record the mode of every store and lock inode it can open. + let reader = scope.spawn(|| { + let mut seen = std::collections::BTreeSet::new(); + while !done.load(Ordering::Acquire) { + for path in [&driver.projection_store, &lock] { + if let Ok(meta) = fs::metadata(path) { + seen.insert((path.clone(), meta.ino(), meta.mode() & 0o777)); + } + } + } + seen + }); + let _done = Done(&done); + // Idunn's own umask, in force while it publishes. + let _umask = Umask027::set(); + for _ in 0..40 { + driver.publish_expected(&expected, &provider_anchor)?; + driver.withdraw_incarnation(&expected, &provider_anchor, None, None)?; + } + drop(_done); + Ok(reader.join().unwrap()) + })?; + assert!(!observed.is_empty(), "the reader never saw the store"); + for (path, _, mode) in &observed { + assert_eq!( + *mode, + 0o644, + "{} was visible with mode {mode:o}", + path.display() + ); + } + Ok(()) } #[cfg(unix)]