diff --git a/src/shadow-core/src/transaction.rs b/src/shadow-core/src/transaction.rs index fdf7bf6..69b0530 100644 --- a/src/shadow-core/src/transaction.rs +++ b/src/shadow-core/src/transaction.rs @@ -22,6 +22,7 @@ //! lock file behind, which every later run has to wait out. use std::fmt::Display; +use std::io::Write as _; use std::path::{Path, PathBuf}; use std::str::FromStr; @@ -54,6 +55,9 @@ pub struct LockedFile { lock: Option, entries: Vec, layout: Layout, + /// The file's contents as read, so a commit that would write the same + /// bytes can write nothing at all. + original: Vec, /// Restores the signal mask when the transaction ends, whichever way. _signals: SignalBlocker, } @@ -75,11 +79,13 @@ impl LockedFile { let lock = FileLock::acquire(path)?; // On any failure from here on, `lock` drops and the file is untouched. let (entries, layout) = records::read_with_layout::(path)?; + let original = std::fs::read(path).unwrap_or_default(); Ok(Self { path: path.to_owned(), lock: Some(lock), entries, layout, + original, _signals: signals, }) } @@ -107,11 +113,13 @@ impl LockedFile { } else { (Vec::new(), Layout::default()) }; + let original = std::fs::read(path).unwrap_or_default(); Ok(Self { path: path.to_owned(), lock: Some(lock), entries, layout, + original, _signals: signals, }) } @@ -155,25 +163,8 @@ impl LockedFile { /// Returns `ShadowError::Validation` if an entry holds a value that would /// corrupt the record -- in which case nothing is written -- and /// `ShadowError::IoPath` if the write fails. - pub fn commit(mut self) -> Result<(), ShadowError> { - let entries = &self.entries; - let layout = &self.layout; - let result = atomic::atomic_write(&self.path, |mut file| { - // `write_with_layout` takes `&mut W`, and `atomic_write` hands over - // a `&mut dyn Write`; borrowing it again makes `W` the fat pointer, - // which is sized, rather than the unsized `dyn Write`. - records::write_with_layout(entries, layout, &mut file, |entry, w| { - entry.validate_fields()?; - writeln!(w, "{entry}")?; - Ok(()) - }) - }); - - // Release before returning either way: an error path that held the - // lock until the process exited would block every concurrent tool for - // as long as the caller took to report it. - drop(self.lock.take()); - result + pub fn commit(self) -> Result<(), ShadowError> { + self.commit_or_remove_if(false) } /// Like [`LockedFile::commit`], but remove the file when nothing is left @@ -189,28 +180,70 @@ impl LockedFile { /// state anyone wants to reach by accident; those use `commit`. /// /// The file is only removed when it carried no comments or other preserved - /// lines either. A file that is all comments still says something. + /// lines either. A file that is all comments still says something. A file + /// that was already empty is left alone, since nothing changed. /// /// # Errors /// /// As [`LockedFile::commit`], plus `ShadowError::IoPath` if the file /// cannot be removed. - pub fn commit_or_remove(mut self) -> Result<(), ShadowError> { - if !self.entries.is_empty() || !self.layout.is_empty() { - return self.commit(); - } - let result = std::fs::remove_file(&self.path).or_else(|e| { - // Already gone is the state we wanted. - if e.kind() == std::io::ErrorKind::NotFound { - Ok(()) - } else { - Err(ShadowError::IoPath(e, self.path.clone())) - } - }); + pub fn commit_or_remove(self) -> Result<(), ShadowError> { + self.commit_or_remove_if(true) + } + + /// Render the entries, validating each one. + /// + /// Into a buffer rather than straight to the file, so a value that would + /// corrupt a record is caught while the file is still untouched. + fn render(&self) -> Result, ShadowError> { + let mut out = Vec::with_capacity(self.original.len()); + records::write_with_layout(&self.entries, &self.layout, &mut out, |entry, w| { + entry.validate_fields()?; + writeln!(w, "{entry}")?; + Ok(()) + })?; + Ok(out) + } + + /// Write the file, optionally unlinking it when nothing is left. + fn commit_or_remove_if(mut self, remove_when_empty: bool) -> Result<(), ShadowError> { + let result = self.write(remove_when_empty); + // Release either way: an error path that held the lock until the + // process exited would block every concurrent tool for as long as the + // caller took to report it. drop(self.lock.take()); result } + fn write(&mut self, remove_when_empty: bool) -> Result<(), ShadowError> { + let rendered = self.render()?; + + // Nothing changed: do not rewrite the file. A rewrite is not free -- + // it replaces the inode, moves the mtime, and for an unchanged empty + // file it would fail outright, since the atomic writer refuses to + // produce a zero-length file. Tools that only *might* change a file + // used to guard every write with their own "did anything change" flag. + if rendered == self.original { + return Ok(()); + } + + if remove_when_empty && self.entries.is_empty() && self.layout.is_empty() { + return std::fs::remove_file(&self.path).or_else(|e| { + // Already gone is the state we wanted. + if e.kind() == std::io::ErrorKind::NotFound { + Ok(()) + } else { + Err(ShadowError::IoPath(e, self.path.clone())) + } + }); + } + + atomic::atomic_write(&self.path, |file| { + file.write_all(&rendered)?; + Ok(()) + }) + } + /// Release the lock and discard the changes. /// /// The same as dropping the value; useful where the intent is worth @@ -227,6 +260,81 @@ impl Drop for LockedFile { } } +// --------------------------------------------------------------------------- +// Committing several files together +// --------------------------------------------------------------------------- + +/// A locked file that can be validated and committed without the caller +/// knowing which record type it holds. +/// +/// [`commit_all`] needs a heterogeneous list -- `/etc/group` and +/// `/etc/gshadow` hold different types and have to be written together -- so +/// the operations it needs are behind a trait object. +pub trait Commit { + /// Everything that can be checked before any file is touched. + /// + /// # Errors + /// + /// Returns `ShadowError::Validation` naming the offending field. + fn validate(&self) -> Result<(), ShadowError>; + + /// Write this file and release its lock. + /// + /// # Errors + /// + /// Returns `ShadowError::IoPath` if the write fails. + fn commit_boxed(self: Box) -> Result<(), ShadowError>; + + /// The file this will write. + fn path(&self) -> &Path; +} + +impl Commit for LockedFile { + fn validate(&self) -> Result<(), ShadowError> { + self.render().map(|_| ()) + } + + fn commit_boxed(self: Box) -> Result<(), ShadowError> { + (*self).commit_or_remove_if(false) + } + + fn path(&self) -> &Path { + &self.path + } +} + +/// Commit several locked files, or none of them. +/// +/// `/etc/group` and `/etc/gshadow` have to agree: a group present in one and +/// absent from the other is a broken system, and every tool that touches a +/// group touches both. Committing them one at a time leaves a window in which +/// they disagree, and a failure in the second makes that permanent. +/// +/// **Every file is validated before any is written.** That closes the failure +/// this actually hits: a value that would corrupt a record is rejected while +/// nothing has been touched, instead of after the first file is already on +/// disk. A genuine I/O error partway through the writes can still leave the +/// set half applied; there is no journal, and a rollback that can itself fail +/// would not be an improvement. The window is reduced to the writes +/// themselves, each of which is a rename onto an already-fsynced file. +/// +/// The locks are all held until the last write finishes, so no other process +/// sees the intermediate state. +/// +/// # Errors +/// +/// Returns the first `ShadowError` from validation -- with nothing written -- +/// or from a write, naming the file it failed on. +pub fn commit_all(files: Vec>) -> Result<(), ShadowError> { + for file in &files { + file.validate()?; + } + for file in files { + file.commit_boxed()?; + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -344,4 +452,126 @@ mod tests { let (_d, path) = temp_passwd("alice:x:1000:1000::/home/alice:/bin/sh\nnot-a-record\n"); assert!(LockedFile::::open(&path).is_err()); } + + // ----------------------------------------------------------------------- + // Committing several files together + // ----------------------------------------------------------------------- + + use crate::group::GroupEntry; + + fn temp_group(dir: &Path, content: &str) -> PathBuf { + let path = dir.join("group"); + std::fs::write(&path, content).expect("write"); + path + } + + const GROUP: &str = "staff:x:1000:alice\nadmin:x:1001:\n"; + + #[test] + fn test_commit_all_writes_every_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let passwd_path = dir.path().join("passwd"); + std::fs::write(&passwd_path, TWO).expect("write"); + let group_path = temp_group(dir.path(), GROUP); + + let mut passwd = LockedFile::::open(&passwd_path).expect("open passwd"); + let mut group = LockedFile::::open(&group_path).expect("open group"); + passwd.find_mut("alice").expect("alice").shell = "/bin/bash".into(); + group.find_mut("staff").expect("staff").members = vec!["alice".into(), "bob".into()]; + + commit_all(vec![Box::new(passwd), Box::new(group)]).expect("commit_all"); + + assert!( + std::fs::read_to_string(&passwd_path) + .expect("read") + .contains("/bin/bash") + ); + assert!( + std::fs::read_to_string(&group_path) + .expect("read") + .contains("staff:x:1000:alice,bob") + ); + } + + /// The failure this exists to prevent: a value that would corrupt one file + /// must stop the set before any of it is written, not after the first file + /// is already on disk. + #[test] + fn test_a_bad_value_in_the_second_file_writes_neither() { + let dir = tempfile::tempdir().expect("tempdir"); + let passwd_path = dir.path().join("passwd"); + std::fs::write(&passwd_path, TWO).expect("write"); + let group_path = temp_group(dir.path(), GROUP); + + let mut passwd = LockedFile::::open(&passwd_path).expect("open passwd"); + let mut group = LockedFile::::open(&group_path).expect("open group"); + passwd.find_mut("alice").expect("alice").shell = "/bin/bash".into(); + // A separator in a group name would shift every following field. + group.find_mut("admin").expect("admin").name = "ad:min".into(); + + assert!(commit_all(vec![Box::new(passwd), Box::new(group)]).is_err()); + assert_eq!( + std::fs::read_to_string(&passwd_path).expect("read"), + TWO, + "the first file was written even though the second was invalid" + ); + assert_eq!(std::fs::read_to_string(&group_path).expect("read"), GROUP); + } + + /// And the locks are released, so the next transaction can start. + #[test] + fn test_commit_all_releases_every_lock() { + let dir = tempfile::tempdir().expect("tempdir"); + let passwd_path = dir.path().join("passwd"); + std::fs::write(&passwd_path, TWO).expect("write"); + let group_path = temp_group(dir.path(), GROUP); + + let passwd = LockedFile::::open(&passwd_path).expect("open passwd"); + let group = LockedFile::::open(&group_path).expect("open group"); + commit_all(vec![Box::new(passwd), Box::new(group)]).expect("commit_all"); + + LockedFile::::open(&passwd_path).expect("passwd lock is free"); + LockedFile::::open(&group_path).expect("group lock is free"); + } + + /// A commit that would write the same bytes writes nothing: the inode is + /// not replaced and the mtime does not move. Tools that only *might* + /// change a file used to carry their own "did anything change" flag, and + /// an unchanged empty file would fail outright, since the atomic writer + /// refuses zero length. + #[test] + fn test_an_unchanged_commit_does_not_rewrite_the_file() { + use std::os::unix::fs::MetadataExt; + + let (_d, path) = temp_passwd(TWO); + let before = std::fs::metadata(&path).expect("stat").ino(); + + LockedFile::::open(&path) + .expect("open") + .commit() + .expect("commit"); + + assert_eq!( + std::fs::metadata(&path).expect("stat").ino(), + before, + "the file was rewritten even though nothing changed" + ); + assert_eq!(std::fs::read_to_string(&path).expect("read"), TWO); + } + + /// The same holds for an empty file, which is where it matters: writing it + /// would fail, and the callers that touch `/etc/gshadow` only sometimes + /// change it. + #[test] + fn test_committing_an_unchanged_empty_file_succeeds() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("gshadow"); + std::fs::write(&path, "").expect("write"); + + LockedFile::::open(&path) + .expect("open") + .commit() + .expect("an unchanged empty file must not be an error"); + assert_eq!(std::fs::read_to_string(&path).expect("read"), ""); + } } diff --git a/src/uu/groupmod/src/groupmod.rs b/src/uu/groupmod/src/groupmod.rs index 97b9d5c..f64a904 100644 --- a/src/uu/groupmod/src/groupmod.rs +++ b/src/uu/groupmod/src/groupmod.rs @@ -14,14 +14,13 @@ use std::path::Path; use clap::{Arg, ArgAction, Command}; use uucore::error::{UError, UResult}; -use shadow_core::atomic; use shadow_core::audit; -use shadow_core::group::{self}; -use shadow_core::gshadow::{self}; -use shadow_core::lock::FileLock; +use shadow_core::group::GroupEntry; +use shadow_core::gshadow::GshadowEntry; use shadow_core::nscd; -use shadow_core::passwd; +use shadow_core::passwd::PasswdEntry; use shadow_core::sysroot::SysRoot; +use shadow_core::transaction::{self, Commit, LockedFile}; mod options { pub const GROUP: &str = "GROUP"; @@ -149,36 +148,42 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }) .transpose()?; - // Block signals for the duration of the critical section so a SIGINT - // between lock acquisition and atomic_write cannot leave stale lock files. - let _signals = shadow_core::hardening::SignalBlocker::block_critical() - .map_err(|e| GroupmodError::CantUpdate(format!("cannot block signals: {e}")))?; - - // Lock order across the tools is passwd < group < gshadow < shadow, so - // take the passwd lock first when -g will need it (see below), never after - // group. This keeps the ordering acyclic with useradd/usermod. + // Lock order across the tools is passwd < group < gshadow < shadow, so the + // files are opened in that order and never in another. This keeps the + // ordering acyclic with useradd and usermod. Each transaction blocks + // signals for its lifetime and releases its lock on every path out. let group_path = root.group_path(); let passwd_path = root.passwd_path(); - // Only the -g path touches passwd, and only if it exists (a --prefix tree - // may not carry one). Guarded like the gshadow write below. - let update_passwd = parsed_gid.is_some() && passwd_path.exists(); - let passwd_lock = if update_passwd { - Some(FileLock::acquire(&passwd_path).map_err(|e| { - GroupmodError::CantUpdate(format!("cannot lock {}: {e}", passwd_path.display())) - })?) + let gshadow_path = root.gshadow_path(); + + let cant_update = |path: &std::path::Path| { + let display = path.display().to_string(); + move |e: shadow_core::error::ShadowError| { + GroupmodError::CantUpdate(format!("cannot open {display}: {e}")) + } + }; + + // Only the -g path touches passwd, and only if it exists: a --prefix tree + // may not carry one. + let mut passwd = if parsed_gid.is_some() && passwd_path.exists() { + Some(LockedFile::::open(&passwd_path).map_err(cant_update(&passwd_path))?) } else { None }; - let group_lock = FileLock::acquire(&group_path).map_err(|e| { - GroupmodError::CantUpdate(format!("cannot lock {}: {e}", group_path.display())) - })?; + let mut groups = + LockedFile::::open(&group_path).map_err(cant_update(&group_path))?; - let (mut entries, group_layout) = group::read_group_with_layout(&group_path).map_err(|e| { - GroupmodError::CantUpdate(format!("cannot read {}: {e}", group_path.display())) - })?; + // gshadow is only touched by a rename or a password change. + let touches_gshadow = new_name.is_some() || new_password.is_some(); + let mut gshadow = if gshadow_path.exists() && touches_gshadow { + Some(LockedFile::::open(&gshadow_path).map_err(cant_update(&gshadow_path))?) + } else { + None + }; // Find the target group. + let entries = groups.entries_mut(); let idx = entries .iter() .position(|g| g.name == *group_name) @@ -195,7 +200,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .iter() .any(|g| g.gid == gid && g.name != *group_name) { - drop(group_lock); return Err(GroupmodError::GidInUse(format!("GID '{gid}' already exists")).into()); } entries[idx].gid = gid; @@ -207,7 +211,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .iter() .any(|g| g.name == *name && g.name != *group_name) { - drop(group_lock); return Err(GroupmodError::NameInUse(format!("group '{name}' already exists")).into()); } entries[idx].name.clone_from(name); @@ -235,98 +238,50 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } } - let modified_gid = entries[idx].gid; + // Without a gshadow file the password belongs in the group file, which is + // where a system with no gshadow keeps it; otherwise -p was a silent no-op. + if gshadow.is_none() + && let Some(pw) = new_password + { + entries[idx].passwd.clone_from(pw); + } - // Write /etc/group. - atomic::atomic_write(&group_path, |f| { - group::write_group_with_layout(&entries, &group_layout, f) - }) - .map_err(|e| { - GroupmodError::CantUpdate(format!("cannot write {}: {e}", group_path.display())) - })?; + let modified_gid = entries[idx].gid; // groupmod(8): "Users who use the group as their primary group are updated - // to keep the group as their primary group." Do it while we still hold the - // passwd lock we took above, then release group then passwd. + // to keep the group as their primary group." if let Some(new_gid_val) = parsed_gid && new_gid_val != old_gid - && update_passwd + && let Some(passwd) = passwd.as_mut() { - let (mut pw_entries, passwd_layout) = passwd::read_passwd_with_layout(&passwd_path) - .map_err(|e| { - GroupmodError::CantUpdate(format!("cannot read {}: {e}", passwd_path.display())) - })?; - let mut changed = false; - for e in &mut pw_entries { + for e in passwd.entries_mut() { if e.gid == old_gid { e.gid = new_gid_val; - changed = true; } } - if changed { - atomic::atomic_write(&passwd_path, |f| { - passwd::write_passwd_with_layout(&pw_entries, &passwd_layout, f) - }) - .map_err(|e| { - GroupmodError::CantUpdate(format!("cannot write {}: {e}", passwd_path.display())) - })?; - } } - drop(group_lock); - drop(passwd_lock); - - // Update /etc/gshadow. - let gshadow_path = root.gshadow_path(); - // Without a gshadow file the password belongs in the group file, which is - // where a system with no gshadow keeps it; otherwise -p was a silent no-op. - if !gshadow_path.exists() - && let Some(pw) = new_password - { - let (mut regroup, layout) = group::read_group_with_layout(&group_path).map_err(|e| { - GroupmodError::CantUpdate(format!("cannot read {}: {e}", group_path.display())) - })?; - if let Some(g) = regroup - .iter_mut() - .find(|g| g.name == *new_name.unwrap_or(group_name)) - { - g.passwd.clone_from(pw); + if let Some(gs) = gshadow.as_mut().and_then(|f| f.find_mut(group_name)) { + if let Some(name) = new_name { + gs.name.clone_from(name); } - atomic::atomic_write(&group_path, |f| { - group::write_group_with_layout(®roup, &layout, f) - }) - .map_err(|e| { - GroupmodError::CantUpdate(format!("cannot write {}: {e}", group_path.display())) - })?; - } - if gshadow_path.exists() && (new_name.is_some() || new_password.is_some()) { - let gs_lock = FileLock::acquire(&gshadow_path).map_err(|e| { - GroupmodError::CantUpdate(format!("cannot lock {}: {e}", gshadow_path.display())) - })?; - - let (mut gs_entries, gshadow_layout) = gshadow::read_gshadow_with_layout(&gshadow_path) - .map_err(|e| { - GroupmodError::CantUpdate(format!("cannot read {}: {e}", gshadow_path.display())) - })?; - - if let Some(gs) = gs_entries.iter_mut().find(|g| g.name == *group_name) { - if let Some(name) = new_name { - gs.name.clone_from(name); - } - if let Some(pw) = new_password { - gs.passwd.clone_from(pw); - } + if let Some(pw) = new_password { + gs.passwd.clone_from(pw); } + } - atomic::atomic_write(&gshadow_path, |f| { - gshadow::write_gshadow_with_layout(&gs_entries, &gshadow_layout, f) - }) - .map_err(|e| { - GroupmodError::CantUpdate(format!("cannot write {}: {e}", gshadow_path.display())) - })?; - - drop(gs_lock); + // Every file is validated before any is written, so a value that would + // corrupt one of them cannot leave the set half applied. + let mut files: Vec> = Vec::new(); + if let Some(passwd) = passwd { + files.push(Box::new(passwd)); + } + files.push(Box::new(groups)); + if let Some(gshadow) = gshadow { + files.push(Box::new(gshadow)); } + transaction::commit_all(files) + .map_err(|e| GroupmodError::CantUpdate(format!("cannot write: {e}")))?; nscd::invalidate_cache("group"); diff --git a/src/uu/grpck/src/grpck.rs b/src/uu/grpck/src/grpck.rs index 0576fe3..63d2018 100644 --- a/src/uu/grpck/src/grpck.rs +++ b/src/uu/grpck/src/grpck.rs @@ -22,12 +22,11 @@ use std::path::{Path, PathBuf}; use clap::{Arg, ArgAction, Command}; use uucore::error::{UError, UResult}; -use shadow_core::atomic; -use shadow_core::group::{self, GroupEntry}; +use shadow_core::group::GroupEntry; use shadow_core::gshadow::{self, GshadowEntry}; -use shadow_core::lock::FileLock; use shadow_core::nscd; use shadow_core::sysroot::SysRoot; +use shadow_core::transaction::{self, Commit, LockedFile}; mod options { pub const READ_ONLY: &str = "read-only"; @@ -297,54 +296,32 @@ fn check_group_gshadow_consistency( /// sort would require a significantly different parser that tracks raw /// lines alongside parsed entries. This matches GNU `grpck -s` behavior. fn sort_and_write(group_path: &Path, gshadow_path: &Path) -> UResult<()> { - let group_lock = FileLock::acquire(group_path) - .map_err(|e| GrpckError::CantLock(format!("cannot lock {}: {e}", group_path.display())))?; - - // Re-read under the lock: the entries checked above were read before it. - // The layout keeps comments, blank lines and NIS compat lines, each - // anchored to the entry it preceded, so a comment follows its group. - let (mut sorted_groups, group_layout) = - group::read_group_with_layout(group_path).map_err(|e| { - GrpckError::CantUpdate(format!("cannot read {}: {e}", group_path.display())) - })?; - let original = sorted_groups.clone(); - sorted_groups.sort_by_key(|g| g.gid); - - if sorted_groups == original { - drop(group_lock); - return Ok(()); - } - - atomic::atomic_write(group_path, |f| { - group::write_group_with_layout(&sorted_groups, &group_layout, f) - }) - .map_err(|e| GrpckError::CantUpdate(format!("cannot update {}: {e}", group_path.display())))?; - - // Sort gshadow to match the new group order. + // The transaction re-reads under the lock: the entries checked above were + // read before it. The layout keeps comments, blank lines and NIS compat + // lines, each anchored to the entry it preceded, so a comment follows its + // group. A commit that would write the same bytes writes nothing. + let mut group_file = LockedFile::::open(group_path) + .map_err(|e| GrpckError::CantLock(format!("cannot open {}: {e}", group_path.display())))?; + group_file.entries_mut().sort_by_key(|g| g.gid); + let sorted_groups = group_file.entries().to_vec(); + + // group and gshadow are written together: a sort that reordered one and + // not the other is exactly the "members differ" state grpck reports. + let mut files: Vec> = vec![Box::new(group_file)]; if gshadow_path.exists() { - let gs_lock = FileLock::acquire(gshadow_path).map_err(|e| { - GrpckError::CantLock(format!("cannot lock {}: {e}", gshadow_path.display())) + let mut gshadow_file = LockedFile::::open(gshadow_path).map_err(|e| { + GrpckError::CantLock(format!("cannot open {}: {e}", gshadow_path.display())) })?; - - let (gshadow_entries, gshadow_layout) = gshadow::read_gshadow_with_layout(gshadow_path) - .map_err(|e| { - GrpckError::CantUpdate(format!("cannot read {}: {e}", gshadow_path.display())) - })?; - - if !gshadow_entries.is_empty() { - let sorted_gshadow = sort_gshadow_by_group(&sorted_groups, &gshadow_entries); - atomic::atomic_write(gshadow_path, |f| { - gshadow::write_gshadow_with_layout(&sorted_gshadow, &gshadow_layout, f) - }) - .map_err(|e| { - GrpckError::CantUpdate(format!("cannot update {}: {e}", gshadow_path.display())) - })?; + if !gshadow_file.entries().is_empty() { + let sorted = sort_gshadow_by_group(&sorted_groups, gshadow_file.entries()); + *gshadow_file.entries_mut() = sorted; } - - drop(gs_lock); + files.push(Box::new(gshadow_file)); } - drop(group_lock); + transaction::commit_all(files) + .map_err(|e| GrpckError::CantUpdate(format!("cannot update: {e}")))?; + nscd::invalidate_cache("group"); Ok(()) diff --git a/src/uu/pwck/src/pwck.rs b/src/uu/pwck/src/pwck.rs index 90b885f..1577281 100644 --- a/src/uu/pwck/src/pwck.rs +++ b/src/uu/pwck/src/pwck.rs @@ -18,11 +18,11 @@ use std::path::{Path, PathBuf}; use clap::{Arg, ArgAction, Command}; use shadow_core::group::{self, GroupEntry}; -use shadow_core::lock::FileLock; -use shadow_core::passwd::{self, PasswdEntry}; +use shadow_core::nscd; +use shadow_core::passwd::PasswdEntry; use shadow_core::shadow::{self, ShadowEntry}; use shadow_core::sysroot::SysRoot; -use shadow_core::{atomic, nscd}; +use shadow_core::transaction::{self, Commit, LockedFile}; use uucore::error::{UError, UResult}; @@ -260,55 +260,44 @@ fn sort_and_write(passwd_path: &Path, shadow_path: &Path, read_only: bool) -> UR return Ok(()); } - let passwd_lock = FileLock::acquire(passwd_path) - .map_err(|e| PwckError::CantLock(format!("cannot lock {}: {e}", passwd_path.display())))?; + // The transaction re-reads under the lock: the entries checked above were + // read before it, so sorting those would overwrite anything that changed + // in between. The layout keeps comments, blank lines and NIS compat lines, + // each anchored to the entry it preceded, so a comment follows its + // account. A commit that would write the same bytes writes nothing. + let mut passwd_file = LockedFile::::open(passwd_path) + .map_err(|e| PwckError::CantLock(format!("cannot open {}: {e}", passwd_path.display())))?; + passwd_file.entries_mut().sort_by_key(|e| e.uid); + let sorted_passwd = passwd_file.entries().to_vec(); - // Re-read under the lock: the entries checked above were read before it, - // so sorting those would overwrite anything that changed in between. The - // layout keeps comments, blank lines and NIS compat lines, each anchored - // to the entry it preceded, so a comment follows its account. - let (mut sorted_passwd, passwd_layout) = - passwd::read_passwd_with_layout(passwd_path).map_err(|e| { - PwckError::CantUpdate(format!("cannot read {}: {e}", passwd_path.display())) - })?; - let original = sorted_passwd.clone(); - sorted_passwd.sort_by_key(|e| e.uid); - - if sorted_passwd == original { - drop(passwd_lock); - return Ok(()); - } - - atomic::atomic_write(passwd_path, |f| { - passwd::write_passwd_with_layout(&sorted_passwd, &passwd_layout, f) - }) - // pwck(8) exit 6 is "can not sort"; this is the write that performs it. - .map_err(|e| PwckError::CantSort(format!("cannot sort {}: {e}", passwd_path.display())))?; + let mut files: Vec> = vec![Box::new(passwd_file)]; if shadow_path.exists() { - let shadow_lock = FileLock::acquire(shadow_path).map_err(|e| { - PwckError::CantLock(format!("cannot lock {}: {e}", shadow_path.display())) + let mut shadow_file = LockedFile::::open(shadow_path).map_err(|e| { + PwckError::CantLock(format!("cannot open {}: {e}", shadow_path.display())) })?; - - let (shadow_entries, shadow_layout) = shadow::read_shadow_with_layout(shadow_path) - .map_err(|e| { - PwckError::CantUpdate(format!("cannot read {}: {e}", shadow_path.display())) - })?; - - if !shadow_entries.is_empty() { - let sorted_shadow = sort_shadow_by_passwd(&sorted_passwd, &shadow_entries); - atomic::atomic_write(shadow_path, |f| { - shadow::write_shadow_with_layout(&sorted_shadow, &shadow_layout, f) - }) - .map_err(|e| { - PwckError::CantUpdate(format!("cannot update {}: {e}", shadow_path.display())) - })?; + if !shadow_file.entries().is_empty() { + let sorted = sort_shadow_by_passwd(&sorted_passwd, shadow_file.entries()); + *shadow_file.entries_mut() = sorted; } - - drop(shadow_lock); + files.push(Box::new(shadow_file)); } - drop(passwd_lock); + // pwck(8) keeps two codes here: 6 is "can not sort" and 5 is "can not + // update the files". The error names the file it failed on, so the two + // stay distinct even though both files are written by one call. + transaction::commit_all(files).map_err(|e| { + let on_shadow = matches!( + &e, + shadow_core::error::ShadowError::IoPath(_, p) if p == shadow_path + ); + if on_shadow { + PwckError::CantUpdate(format!("cannot update {}: {e}", shadow_path.display())) + } else { + PwckError::CantSort(format!("cannot sort {}: {e}", passwd_path.display())) + } + })?; + nscd::invalidate_cache("passwd"); Ok(()) diff --git a/src/uu/useradd/src/useradd.rs b/src/uu/useradd/src/useradd.rs index ffae786..5a653ca 100644 --- a/src/uu/useradd/src/useradd.rs +++ b/src/uu/useradd/src/useradd.rs @@ -19,17 +19,16 @@ use std::path::Path; use clap::{Arg, ArgAction, Command}; -use shadow_core::atomic; use shadow_core::audit; -use shadow_core::group::{self, GroupEntry}; -use shadow_core::gshadow::{self, GshadowEntry}; -use shadow_core::lock::FileLock; +use shadow_core::group::GroupEntry; +use shadow_core::gshadow::GshadowEntry; use shadow_core::login_defs::{self, LoginDefs}; use shadow_core::nscd; -use shadow_core::passwd::{self, PasswdEntry}; -use shadow_core::shadow::{self, ShadowEntry}; +use shadow_core::passwd::PasswdEntry; +use shadow_core::shadow::ShadowEntry; use shadow_core::skel; use shadow_core::sysroot::SysRoot; +use shadow_core::transaction::{self, Commit, LockedFile}; use shadow_core::uid_alloc; use shadow_core::validate; @@ -556,23 +555,32 @@ fn do_useradd(opts: &UseraddOptions) -> UResult<()> { let signals = shadow_core::hardening::SignalBlocker::block_critical() .map_err(|e| UseraddError::CannotUpdatePasswd(format!("cannot block signals: {e}")))?; - // Acquire locks BEFORE reading so concurrent useradd cannot - // silently overwrite entries added between our read and write. + // Each transaction locks before reading, so a concurrent useradd cannot + // silently overwrite an entry added between our read and our write. They + // are opened in the project's lock order: passwd, then group, then + // gshadow. Every early return below drops them, releasing the locks with + // the files untouched. let passwd_path = opts.root.passwd_path(); - let passwd_lock = FileLock::acquire(&passwd_path) - .map_err(|e| UseraddError::CannotUpdatePasswd(format!("cannot lock passwd: {e}")))?; - let group_path = opts.root.group_path(); - let group_lock = FileLock::acquire(&group_path) - .map_err(|e| UseraddError::CannotUpdateGroup(format!("cannot lock group: {e}")))?; + let gshadow_path = opts.root.gshadow_path(); - // Step 3: Read passwd under lock and check username not already in use. - let (passwd_entries, passwd_layout) = passwd::read_passwd_with_layout(&passwd_path) - .map_err(|e| UseraddError::CannotUpdatePasswd(format!("{e}")))?; + let mut passwd_file = LockedFile::::open(&passwd_path) + .map_err(|e| UseraddError::CannotUpdatePasswd(format!("cannot open passwd: {e}")))?; + let mut group_file = LockedFile::::open(&group_path) + .map_err(|e| UseraddError::CannotUpdateGroup(format!("cannot open group: {e}")))?; + // A fresh --prefix tree may carry no gshadow; useradd does not create one. + let mut gshadow_file = if gshadow_path.exists() { + Some( + LockedFile::::open(&gshadow_path).map_err(|e| { + UseraddError::CannotUpdateGroup(format!("cannot open gshadow: {e}")) + })?, + ) + } else { + None + }; - if passwd_entries.iter().any(|e| e.name == opts.login) { - drop(group_lock); - drop(passwd_lock); + // Step 3: Check the username is not already in use. + if passwd_file.find(&opts.login).is_some() { return Err( UseraddError::UsernameInUse(format!("user '{}' already exists", opts.login)).into(), ); @@ -587,34 +595,19 @@ fn do_useradd(opts: &UseraddOptions) -> UResult<()> { // Step 5: Determine UID. A prefixed run manages another system's files, so // the local name service is not consulted for it. let scope = uid_alloc::Scope::for_prefix(opts.root.is_prefixed()); - let uid = determine_uid(opts, &passwd_entries, &defs, scope)?; - - // Step 6: Read group entries under lock (needed for GID resolution and - // user group creation). - let (mut group_entries, group_layout) = group::read_group_with_layout(&group_path) - .map_err(|e| UseraddError::CannotUpdateGroup(format!("{e}")))?; + let uid = determine_uid(opts, passwd_file.entries(), &defs, scope)?; // Step 7: Determine primary GID. - let (gid, new_group) = determine_gid(opts, uid, &group_entries, &defs, scope)?; - - // Step 8: Read gshadow entries. - let gshadow_path = opts.root.gshadow_path(); - let (mut gshadow_entries, gshadow_layout) = if gshadow_path.exists() { - gshadow::read_gshadow_with_layout(&gshadow_path) - .map_err(|e| UseraddError::CannotUpdateGroup(format!("{e}")))? - } else { - (Vec::new(), gshadow::Layout::default()) - }; + let (gid, new_group) = determine_gid(opts, uid, group_file.entries(), &defs, scope)?; // Step 9: Validate supplementary groups exist. for grp_name in &opts.groups { // -G takes the same forms as -g: a group name or a GID. - let known = group_entries + let known = group_file + .entries() .iter() .any(|g| g.name == *grp_name || grp_name.parse::().is_ok_and(|id| g.gid == id)); if !known { - drop(group_lock); - drop(passwd_lock); return Err( UseraddError::GroupNotExist(format!("group '{grp_name}' does not exist")).into(), ); @@ -635,29 +628,26 @@ fn do_useradd(opts: &UseraddOptions) -> UResult<()> { }); // ------------------------------------------------------------------- - // Begin mutations. From here, partial state is left on failure - // (matching GNU behavior). Locks are held throughout. + // Begin mutations. Nothing is written until every file has been changed + // in memory and validated, so a value that would corrupt one of them + // cannot leave the account half created. // ------------------------------------------------------------------- - // Step 11: Create user group if needed (group lock already held). + // Step 11: Create the user group if needed. if let Some(ref new_grp) = new_group { - write_new_group(&group_path, &mut group_entries, &group_layout, new_grp)?; - if gshadow_path.exists() { - // Acquire gshadow lock — group.lock does NOT protect gshadow. - let _gs_lock = FileLock::acquire(&gshadow_path).map_err(|e| { - UseraddError::CannotUpdateGroup(format!("cannot lock gshadow: {e}")) - })?; - write_new_gshadow( - &gshadow_path, - &mut gshadow_entries, - &gshadow_layout, - new_grp, - )?; + group_file.entries_mut().push(new_grp.clone()); + if let Some(gshadow_file) = gshadow_file.as_mut() { + gshadow_file.entries_mut().push(GshadowEntry { + name: new_grp.name.clone(), + passwd: "!".to_string(), + admins: Vec::new(), + members: Vec::new(), + }); } } - // Step 12: Write /etc/passwd entry (lock already held). - let passwd_entry = PasswdEntry { + // Step 12: Add the /etc/passwd entry. + passwd_file.entries_mut().push(PasswdEntry { name: opts.login.clone(), passwd: "x".to_string(), uid, @@ -665,8 +655,13 @@ fn do_useradd(opts: &UseraddOptions) -> UResult<()> { gecos: opts.comment.clone(), home: home_dir.clone(), shell: opts.shell.clone(), - }; - write_passwd_entry(&passwd_path, &passwd_entries, &passwd_layout, &passwd_entry)?; + }); + + let mut files: Vec> = vec![Box::new(passwd_file), Box::new(group_file)]; + if let Some(gshadow_file) = gshadow_file { + files.push(Box::new(gshadow_file)); + } + transaction::commit_all(files).map_err(|e| UseraddError::CannotUpdatePasswd(format!("{e}")))?; // Step 13: Write /etc/shadow entry (passwd+group locks still held). let shadow_path = opts.root.shadow_path(); @@ -700,11 +695,10 @@ fn do_useradd(opts: &UseraddOptions) -> UResult<()> { }; write_shadow_entry(&shadow_path, &shadow_entry)?; - // Release locks and signal blocker now that passwd, group, and shadow writes are complete. - // Subsequent steps (subid, supplementary groups, home creation) are individually - // crash-safe and may be long-running, so signals should be interruptible. - drop(group_lock); - drop(passwd_lock); + // The transactions above released their locks when they committed. + // Subsequent steps (subid, supplementary groups, home creation) are + // individually crash-safe and may be long-running, so signals become + // interruptible again here. drop(signals); // Step 14: Allocate subordinate UID/GID ranges for rootless containers. @@ -885,90 +879,17 @@ fn resolve_group(gid_arg: &str, group_entries: &[GroupEntry]) -> Result, - layout: &group::Layout, - new_group: &GroupEntry, -) -> UResult<()> { - group_entries.push(new_group.clone()); - - atomic::atomic_write(group_path, |f| { - group::write_group_with_layout(group_entries, layout, f) - }) - .map_err(|e| UseraddError::CannotUpdateGroup(format!("{e}")))?; - - Ok(()) -} - -/// Append a new gshadow entry to `/etc/gshadow`. -/// -/// Caller must hold the gshadow file lock (or the group file lock -/// if gshadow is protected by the same lock scheme). -fn write_new_gshadow( - gshadow_path: &Path, - gshadow_entries: &mut Vec, - layout: &gshadow::Layout, - new_group: &GroupEntry, -) -> UResult<()> { - gshadow_entries.push(GshadowEntry { - name: new_group.name.clone(), - passwd: "!".to_string(), - admins: Vec::new(), - members: Vec::new(), - }); - - atomic::atomic_write(gshadow_path, |f| { - gshadow::write_gshadow_with_layout(gshadow_entries, layout, f) - }) - .map_err(|e| UseraddError::CannotUpdateGroup(format!("{e}")))?; - - Ok(()) -} - -/// Append a new passwd entry to `/etc/passwd`. -/// -/// Caller must hold the passwd file lock. -fn write_passwd_entry( - passwd_path: &Path, - existing: &[PasswdEntry], - layout: &passwd::Layout, - new_entry: &PasswdEntry, -) -> UResult<()> { - let mut entries: Vec = existing.to_vec(); - entries.push(new_entry.clone()); - - atomic::atomic_write(passwd_path, |f| { - passwd::write_passwd_with_layout(&entries, layout, f) - }) - .map_err(|e| UseraddError::CannotUpdatePasswd(format!("{e}")))?; - - Ok(()) -} - -/// Append a new shadow entry to `/etc/shadow` with proper locking. +/// A shadow file that does not exist yet is created: a fresh `--prefix` tree +/// carries none. fn write_shadow_entry(shadow_path: &Path, new_entry: &ShadowEntry) -> UResult<()> { - let _lock = FileLock::acquire(shadow_path) + let mut shadow = LockedFile::::open_or_empty(shadow_path) + .map_err(|e| UseraddError::CannotUpdatePasswd(format!("{e}")))?; + shadow.entries_mut().push(new_entry.clone()); + shadow + .commit() .map_err(|e| UseraddError::CannotUpdatePasswd(format!("{e}")))?; - - // Read existing entries; if the file does not exist, start fresh. - let (mut entries, layout) = if shadow_path.exists() { - shadow::read_shadow_with_layout(shadow_path) - .map_err(|e| UseraddError::CannotUpdatePasswd(format!("{e}")))? - } else { - (Vec::new(), shadow::Layout::default()) - }; - - entries.push(new_entry.clone()); - - atomic::atomic_write(shadow_path, |f| { - shadow::write_shadow_with_layout(&entries, &layout, f) - }) - .map_err(|e| UseraddError::CannotUpdatePasswd(format!("{e}")))?; - Ok(()) } @@ -990,13 +911,10 @@ fn add_to_supplementary_groups( group_path: &Path, gshadow_path: &Path, ) -> UResult<()> { - let _lock = FileLock::acquire(group_path) + let mut groups = LockedFile::::open(group_path) .map_err(|e| UseraddError::CannotUpdateGroup(format!("{e}")))?; - let (mut entries, layout) = group::read_group_with_layout(group_path) - .map_err(|e| UseraddError::CannotUpdateGroup(format!("{e}")))?; - - for entry in &mut entries { + for entry in groups.entries_mut() { if group_requested(&opts.groups, &entry.name, entry.gid) && !entry.members.contains(&opts.login) { @@ -1004,32 +922,23 @@ fn add_to_supplementary_groups( } } - atomic::atomic_write(group_path, |f| { - group::write_group_with_layout(&entries, &layout, f) - }) - .map_err(|e| UseraddError::CannotUpdateGroup(format!("{e}")))?; - - // Also update gshadow if it exists. + // group and gshadow carry the same membership lists, so they are written + // together: a member in one and not the other is what grpck reports as + // "members differ". + let mut files: Vec> = vec![Box::new(groups)]; if gshadow_path.exists() { - let _gs_lock = FileLock::acquire(gshadow_path) - .map_err(|e| UseraddError::CannotUpdateGroup(format!("{e}")))?; - - let (mut gs_entries, gs_layout) = gshadow::read_gshadow_with_layout(gshadow_path) + let mut gshadow = LockedFile::::open(gshadow_path) .map_err(|e| UseraddError::CannotUpdateGroup(format!("{e}")))?; - - for entry in &mut gs_entries { + for entry in gshadow.entries_mut() { if gs_group_requested(&opts.groups, &entry.name) && !entry.members.contains(&opts.login) { entry.members.push(opts.login.clone()); } } - - atomic::atomic_write(gshadow_path, |f| { - gshadow::write_gshadow_with_layout(&gs_entries, &gs_layout, f) - }) - .map_err(|e| UseraddError::CannotUpdateGroup(format!("{e}")))?; + files.push(Box::new(gshadow)); } + transaction::commit_all(files).map_err(|e| UseraddError::CannotUpdateGroup(format!("{e}")))?; Ok(()) } @@ -1042,51 +951,36 @@ fn add_to_supplementary_groups( /// Skips the write if the user already has an entry in the file. /// Uses file locking and atomic writes for crash safety. fn append_subid_entry(path: &Path, name: &str, count: u64) -> UResult<()> { - use shadow_core::subid::{self, SubIdEntry}; + use shadow_core::subid::SubIdEntry; - let lock = FileLock::acquire(path).map_err(|e| { - UseraddError::CannotUpdatePasswd(format!("cannot lock {}: {e}", path.display())) + let mut file = LockedFile::::open(path).map_err(|e| { + uucore::show_error!("warning: cannot open {}: {e}", path.display()); + UseraddError::CannotUpdatePasswd(format!("cannot open {}: {e}", path.display())) })?; - let (mut entries, layout) = match subid::read_subid_with_layout(path) { - Ok(e) => e, - Err(e) => { - uucore::show_error!("warning: cannot read {}: {e}", path.display()); - return Err(UseraddError::CannotUpdatePasswd(format!( - "cannot read {}: {e}", - path.display() - )) - .into()); - } - }; - // Don't add a duplicate entry. - if entries.iter().any(|e| e.name == name) { - drop(lock); + if file.entries().iter().any(|e| e.name == name) { return Ok(()); } // Find next available range by starting after the highest existing end. // Clamp to at least 100_000 even if existing entries are below that threshold. - let start = entries + let start = file + .entries() .iter() .map(|e| e.start.saturating_add(e.count)) .max() .unwrap_or(100_000) .max(100_000); - entries.push(SubIdEntry { + file.entries_mut().push(SubIdEntry { name: name.to_string(), start, count, }); - atomic::atomic_write(path, |f| { - subid::write_subid_with_layout(&entries, &layout, f) - }) - .map_err(|e| UseraddError::CannotUpdatePasswd(format!("{e}")))?; - - drop(lock); + file.commit() + .map_err(|e| UseraddError::CannotUpdatePasswd(format!("{e}")))?; Ok(()) } @@ -1884,6 +1778,8 @@ mod tests { (dir, root) } + use shadow_core::{group, gshadow, passwd, shadow}; + /// Skip tests that require root privileges. fn skip_unless_root() -> bool { !rustix::process::geteuid().is_root() @@ -1899,14 +1795,12 @@ mod tests { let defs = LoginDefs::load(&root.login_defs_path()).expect("defs"); - let (passwd_entries, passwd_layout) = - passwd::read_passwd_with_layout(&root.passwd_path()).expect("passwd"); - let _group_entries = group::read_group_file(&root.group_path()).expect("group"); + let mut passwd_file = LockedFile::::open(&root.passwd_path()).expect("passwd"); // Allocate UID. let (uid_min, uid_max) = uid_alloc::uid_range(&defs, false); let uid = uid_alloc::next_uid( - &passwd_entries, + passwd_file.entries(), uid_min, uid_max, uid_alloc::Scope::FilesOnly, @@ -1915,7 +1809,7 @@ mod tests { assert_eq!(uid, 1000); // Create passwd entry. - let new_entry = PasswdEntry { + passwd_file.entries_mut().push(PasswdEntry { name: "testuser".into(), passwd: "x".into(), uid, @@ -1923,15 +1817,8 @@ mod tests { gecos: "Test User".into(), home: "/home/testuser".into(), shell: "/bin/bash".into(), - }; - - write_passwd_entry( - &root.passwd_path(), - &passwd_entries, - &passwd_layout, - &new_entry, - ) - .expect("write passwd"); + }); + passwd_file.commit().expect("write passwd"); // Verify. let updated = passwd::read_passwd_file(&root.passwd_path()).expect("re-read"); @@ -1949,33 +1836,25 @@ mod tests { let (_dir, root) = setup_test_root(); - let (mut group_entries, group_layout) = - group::read_group_with_layout(&root.group_path()).expect("group"); - let (mut gshadow_entries, gshadow_layout) = - gshadow::read_gshadow_with_layout(&root.gshadow_path()).expect("gshadow"); + let mut group_file = LockedFile::::open(&root.group_path()).expect("group"); + let mut gshadow_file = + LockedFile::::open(&root.gshadow_path()).expect("gshadow"); - // Create user group. - let new_group = GroupEntry { + // Create the user group in both files, committed together. + group_file.entries_mut().push(GroupEntry { name: "newuser".into(), passwd: "x".into(), gid: 1000, members: Vec::new(), - }; - - write_new_group( - &root.group_path(), - &mut group_entries, - &group_layout, - &new_group, - ) - .expect("write group"); - write_new_gshadow( - &root.gshadow_path(), - &mut gshadow_entries, - &gshadow_layout, - &new_group, - ) - .expect("write gshadow"); + }); + gshadow_file.entries_mut().push(GshadowEntry { + name: "newuser".into(), + passwd: "!".into(), + admins: Vec::new(), + members: Vec::new(), + }); + transaction::commit_all(vec![Box::new(group_file), Box::new(gshadow_file)]) + .expect("write group and gshadow"); // Verify group. let updated_groups = group::read_group_file(&root.group_path()).expect("re-read"); @@ -2026,21 +1905,14 @@ mod tests { let (_dir, root) = setup_test_root(); // Add a "wheel" group. - let (mut group_entries, group_layout) = - group::read_group_with_layout(&root.group_path()).expect("group"); - let wheel = GroupEntry { + let mut group_file = LockedFile::::open(&root.group_path()).expect("group"); + group_file.entries_mut().push(GroupEntry { name: "wheel".into(), passwd: "x".into(), gid: 10, members: Vec::new(), - }; - write_new_group( - &root.group_path(), - &mut group_entries, - &group_layout, - &wheel, - ) - .expect("add wheel"); + }); + group_file.commit().expect("add wheel"); // Now add "testuser" to "wheel" and "users". let opts = UseraddOptions { diff --git a/src/uu/userdel/src/userdel.rs b/src/uu/userdel/src/userdel.rs index 2a3435b..a4aa330 100644 --- a/src/uu/userdel/src/userdel.rs +++ b/src/uu/userdel/src/userdel.rs @@ -15,13 +15,13 @@ use clap::{Arg, ArgAction, Command}; use uucore::error::{UError, UResult}; use shadow_core::audit; -use shadow_core::group::{self}; -use shadow_core::gshadow::{self}; -use shadow_core::lock::FileLock; +use shadow_core::group::GroupEntry; +use shadow_core::gshadow::GshadowEntry; +use shadow_core::nscd; use shadow_core::passwd::{self, PasswdEntry}; use shadow_core::shadow::ShadowEntry; use shadow_core::sysroot::SysRoot; -use shadow_core::{atomic, nscd}; +use shadow_core::transaction::{self, Commit, LockedFile, Record}; mod options { pub const FORCE: &str = "force"; @@ -351,123 +351,66 @@ fn safe_remove_home( // Helpers // --------------------------------------------------------------------------- -trait HasName { - fn name(&self) -> &str; -} - -impl HasName for PasswdEntry { - fn name(&self) -> &str { - &self.name - } -} - -impl HasName for ShadowEntry { - fn name(&self) -> &str { - &self.name - } -} - -/// Remove an entry by name from a file (passwd or shadow format). +/// Remove an entry by name from a record file. +/// +/// This used to be a hand-rolled line filter with its own idea of which lines +/// were comments, next to the parser that already knows. The transaction locks +/// first, parses properly, and puts the preserved lines back where they were. fn remove_entry_from_file(path: &Path, login: &str, file_label: &str) -> Result<(), String> where - T: std::str::FromStr + std::fmt::Display + HasName, - T::Err: std::fmt::Display, + T: Record, { - let lock = FileLock::acquire(path).map_err(|e| format!("cannot lock {file_label}: {e}"))?; - - let content = std::fs::read_to_string(path) - .map_err(|e| format!("cannot read {}: {e}", path.display()))?; + let mut file = + LockedFile::::open(path).map_err(|e| format!("cannot open {file_label}: {e}"))?; - let mut found = false; - let mut kept_lines = Vec::new(); - - for line in content.lines() { - let trimmed = line.trim_start(); - if trimmed.is_empty() || trimmed.starts_with('#') { - kept_lines.push(line.to_string()); - continue; - } - - if let Ok(entry) = line.parse::() - && entry.name() == login - { - found = true; - continue; // skip this entry - } - kept_lines.push(line.to_string()); - } - - if !found { - drop(lock); + let before = file.entries().len(); + file.entries_mut().retain(|e| e.name() != login); + if file.entries().len() == before { + // Dropping releases the lock with the file untouched. return Err(format!("user '{login}' does not exist in {file_label}")); } - atomic::atomic_write(path, |f| { - for line in &kept_lines { - writeln!(f, "{line}")?; - } - Ok(()) - }) - .map_err(|e| format!("cannot write {}: {e}", path.display()))?; - - drop(lock); - Ok(()) + file.commit() + .map_err(|e| format!("cannot write {}: {e}", path.display())) } /// Remove a username from all group membership lists in /etc/group. fn remove_from_group_members(path: &Path, login: &str) -> Result<(), String> { - let lock = FileLock::acquire(path).map_err(|e| format!("cannot lock group file: {e}"))?; - - let (mut entries, layout) = group::read_group_with_layout(path) - .map_err(|e| format!("cannot read {}: {e}", path.display()))?; + let mut file = + LockedFile::::open(path).map_err(|e| format!("cannot open group file: {e}"))?; let mut changed = false; - for entry in &mut entries { + for entry in file.entries_mut() { let before = entry.members.len(); entry.members.retain(|m| m != login); - if entry.members.len() != before { - changed = true; - } + changed |= entry.members.len() != before; } - if changed { - atomic::atomic_write(path, |f| { - group::write_group_with_layout(&entries, &layout, f) - }) - .map_err(|e| format!("cannot write {}: {e}", path.display()))?; + if !changed { + return Ok(()); } - - drop(lock); - Ok(()) + file.commit() + .map_err(|e| format!("cannot write {}: {e}", path.display())) } /// Remove a username from all gshadow membership and admin lists. fn remove_from_gshadow_members(path: &Path, login: &str) -> Result<(), String> { - let lock = FileLock::acquire(path).map_err(|e| format!("cannot lock gshadow file: {e}"))?; - - let (mut entries, layout) = gshadow::read_gshadow_with_layout(path) - .map_err(|e| format!("cannot read {}: {e}", path.display()))?; + let mut file = LockedFile::::open(path) + .map_err(|e| format!("cannot open gshadow file: {e}"))?; let mut changed = false; - for entry in &mut entries { - let before_m = entry.members.len(); - let before_a = entry.admins.len(); + for entry in file.entries_mut() { + let before = (entry.members.len(), entry.admins.len()); entry.members.retain(|m| m != login); entry.admins.retain(|a| a != login); - if entry.members.len() != before_m || entry.admins.len() != before_a { - changed = true; - } + changed |= (entry.members.len(), entry.admins.len()) != before; } - if changed { - atomic::atomic_write(path, |f| { - gshadow::write_gshadow_with_layout(&entries, &layout, f) - }) - .map_err(|e| format!("cannot write {}: {e}", path.display()))?; + if !changed { + return Ok(()); } - - drop(lock); - Ok(()) + file.commit() + .map_err(|e| format!("cannot write {}: {e}", path.display())) } /// Remove the user's private group — the group named after the login that @@ -489,19 +432,19 @@ fn remove_user_private_group( return Ok(()); } - let group_lock = - FileLock::acquire(&group_path).map_err(|e| format!("cannot lock group file: {e}"))?; - let (mut entries, group_layout) = group::read_group_with_layout(&group_path) - .map_err(|e| format!("cannot read group: {e}"))?; + // Every early return below drops the transaction, releasing the lock with + // the file untouched. + let mut groups = LockedFile::::open(&group_path) + .map_err(|e| format!("cannot open group file: {e}"))?; - let Some(idx) = entries.iter().position(|g| g.name == login) else { + let Some(idx) = groups.entries().iter().position(|g| g.name == login) else { return Ok(()); }; // Keep it if other users still belong to it. - if !entries[idx].members.is_empty() { + if !groups.entries()[idx].members.is_empty() { return Ok(()); } - let gid = entries[idx].gid; + let gid = groups.entries()[idx].gid; // Keep it if it is a private group whose GID is not the user's primary // one (then it is not really this user's), or another user's primary // group, unless forced. @@ -512,28 +455,24 @@ fn remove_user_private_group( return Ok(()); } - entries.remove(idx); - write_group_or_empty(&group_path, &entries, &group_layout) - .map_err(|e| format!("cannot write group: {e}"))?; - drop(group_lock); + groups.entries_mut().remove(idx); - // Mirror the removal in gshadow. + // The group and its gshadow row are removed together: a group in one file + // and not the other is a broken system, and validating both before writing + // either keeps a bad value from leaving them out of step. let gshadow_path = root.gshadow_path(); + let mut files: Vec> = vec![Box::new(groups)]; if gshadow_path.exists() { - let gs_lock = - FileLock::acquire(&gshadow_path).map_err(|e| format!("cannot lock gshadow: {e}"))?; - if let Ok((mut gs, gshadow_layout)) = gshadow::read_gshadow_with_layout(&gshadow_path) { - let before = gs.len(); - gs.retain(|g| g.name != login); - if gs.len() != before { - write_gshadow_or_empty(&gshadow_path, &gs, &gshadow_layout) - .map_err(|e| format!("cannot write gshadow: {e}"))?; - } + let mut gshadow = LockedFile::::open(&gshadow_path) + .map_err(|e| format!("cannot open gshadow: {e}"))?; + let before = gshadow.entries().len(); + gshadow.entries_mut().retain(|g| g.name != login); + if gshadow.entries().len() != before { + files.push(Box::new(gshadow)); } - drop(gs_lock); } - Ok(()) + transaction::commit_all(files).map_err(|e| format!("cannot write: {e}")) } /// Remove every subordinate-ID row owned by `login` from a subuid/subgid file. @@ -547,53 +486,15 @@ fn remove_subid_rows(path: &Path, login: &str) { if !path.exists() { return; } - let Ok(lock) = FileLock::acquire(path) else { + let Ok(mut file) = LockedFile::::open(path) else { return; }; - if let Ok((mut entries, layout)) = subid::read_subid_with_layout(path) { - let before = entries.len(); - entries.retain(|e| e.name != login); - if entries.len() != before { - if entries.is_empty() && layout.is_empty() { - let _ = std::fs::remove_file(path); - } else { - let _ = atomic::atomic_write(path, |f| { - subid::write_subid_with_layout(&entries, &layout, f) - }); - } - } - } - drop(lock); -} - -/// Write group entries, unlinking the file instead if the result is empty -/// (the atomic writer refuses a zero-length file, but an empty group file is -/// valid — and only reached in a fully torn-down `--prefix` tree). -fn write_group_or_empty( - path: &Path, - entries: &[group::GroupEntry], - layout: &group::Layout, -) -> Result<(), shadow_core::error::ShadowError> { - if entries.is_empty() && layout.is_empty() { - let _ = std::fs::remove_file(path); - Ok(()) - } else { - atomic::atomic_write(path, |f| group::write_group_with_layout(entries, layout, f)) - } -} - -fn write_gshadow_or_empty( - path: &Path, - entries: &[gshadow::GshadowEntry], - layout: &gshadow::Layout, -) -> Result<(), shadow_core::error::ShadowError> { - if entries.is_empty() && layout.is_empty() { - let _ = std::fs::remove_file(path); - Ok(()) - } else { - atomic::atomic_write(path, |f| { - gshadow::write_gshadow_with_layout(entries, layout, f) - }) + let before = file.entries().len(); + file.entries_mut().retain(|e| e.name != login); + if file.entries().len() != before { + // An absent subid file means "no ranges", so the last row going leaves + // nothing to write. + let _ = file.commit_or_remove(); } } diff --git a/src/uu/usermod/src/usermod.rs b/src/uu/usermod/src/usermod.rs index 10d54ce..481a352 100644 --- a/src/uu/usermod/src/usermod.rs +++ b/src/uu/usermod/src/usermod.rs @@ -15,13 +15,13 @@ use clap::{Arg, ArgAction, Command}; use uucore::error::{UError, UResult}; use shadow_core::audit; -use shadow_core::group::{self}; -use shadow_core::gshadow::{self}; -use shadow_core::lock::FileLock; -use shadow_core::passwd::{self}; -use shadow_core::shadow::{self}; +use shadow_core::group::{self, GroupEntry}; +use shadow_core::gshadow::GshadowEntry; +use shadow_core::passwd::PasswdEntry; +use shadow_core::shadow::{self, ShadowEntry}; use shadow_core::sysroot::SysRoot; -use shadow_core::{atomic, nscd, validate}; +use shadow_core::transaction::{self, Commit, LockedFile}; +use shadow_core::{nscd, validate}; mod options { pub const COMMENT: &str = "comment"; @@ -145,14 +145,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Modify /etc/passwd. let group_path_for_lookup = root.group_path(); let passwd_path = root.passwd_path(); - let lock = FileLock::acquire(&passwd_path) - .map_err(|e| UsermodError::CantUpdate(format!("cannot lock: {e}")))?; - - let (mut entries, passwd_layout) = passwd::read_passwd_with_layout(&passwd_path) - .map_err(|e| UsermodError::CantUpdate(format!("{e}")))?; + // The transaction locks, then reads, and releases on every path out -- + // including each early return below, where the file is left untouched. + let mut passwd_file = LockedFile::::open(&passwd_path) + .map_err(|e| UsermodError::CantUpdate(format!("cannot open passwd: {e}")))?; + let entries = passwd_file.entries_mut(); let Some(idx) = entries.iter().position(|e| e.name == *login) else { - drop(lock); return Err(UsermodError::UserNotFound(format!("user '{login}' does not exist")).into()); }; @@ -164,7 +163,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Check UID collision before mutating. if let Some(&uid) = matches.get_one::(options::UID) { if entries.iter().any(|e| e.uid == uid && e.name != *login) { - drop(lock); return Err(UsermodError::UidInUse(format!("UID {uid} already in use")).into()); } entries[idx].uid = uid; @@ -194,7 +192,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { groups.iter().find(|g| g.name == *group_arg).map(|g| g.gid) }; let Some(gid) = resolved else { - drop(lock); return Err( UsermodError::GroupNotFound(format!("group '{group_arg}' does not exist")).into(), ); @@ -211,7 +208,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .iter() .any(|e| e.name == *new_name && e.name != *login) { - drop(lock); return Err( UsermodError::NameInUse(format!("user '{new_name}' already exists")).into(), ); @@ -221,11 +217,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let new_uid = entries[idx].uid; - atomic::atomic_write(&passwd_path, |f| { - passwd::write_passwd_with_layout(&entries, &passwd_layout, f) - }) - .map_err(|e| UsermodError::CantUpdate(format!("{e}")))?; - drop(lock); + passwd_file + .commit() + .map_err(|e| UsermodError::CantUpdate(format!("{e}")))?; // Restore signals before potentially long-running recursive chown. drop(signals); @@ -257,14 +251,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { || new_password.is_some() || login_changing) { - let slock = FileLock::acquire(&shadow_path) - .map_err(|e| UsermodError::CantUpdate(format!("cannot lock shadow: {e}")))?; - - let (mut se, shadow_layout) = shadow::read_shadow_with_layout(&shadow_path) - .map_err(|e| UsermodError::CantUpdate(format!("{e}")))?; + let mut shadow_file = LockedFile::::open(&shadow_path) + .map_err(|e| UsermodError::CantUpdate(format!("cannot open shadow: {e}")))?; - let Some(s) = se.iter_mut().find(|e| e.name == *login) else { - drop(slock); + let Some(s) = shadow_file.find_mut(login) else { return Err(UsermodError::CantUpdate(format!( "user '{login}' not found in shadow file" )) @@ -277,7 +267,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { s.lock(); } if do_unlock && !s.unlock() { - drop(slock); return Err(UsermodError::BadArgument(format!( "unlocking '{login}' would leave the account without a password" )) @@ -299,60 +288,46 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { s.name.clone_from(new_name); } - atomic::atomic_write(&shadow_path, |f| { - shadow::write_shadow_with_layout(&se, &shadow_layout, f) - }) - .map_err(|e| UsermodError::CantUpdate(format!("{e}")))?; - drop(slock); + shadow_file + .commit() + .map_err(|e| UsermodError::CantUpdate(format!("{e}")))?; } // Rename user in group membership lists when --login changes the name. if let Some(new_name) = new_login { let group_path = root.group_path(); if group_path.exists() { - let glock = FileLock::acquire(&group_path) - .map_err(|e| UsermodError::CantUpdate(format!("cannot lock group: {e}")))?; + let mut group_file = LockedFile::::open(&group_path) + .map_err(|e| UsermodError::CantUpdate(format!("cannot open group: {e}")))?; - let (mut ge, group_layout) = group::read_group_with_layout(&group_path) - .map_err(|e| UsermodError::CantUpdate(format!("{e}")))?; - - let mut changed = false; - for g in &mut ge { + for g in group_file.entries_mut() { if let Some(m) = g.members.iter_mut().find(|m| **m == *login) { m.clone_from(new_name); - changed = true; } } - // Mirror the rename in gshadow's member and admin lists. + // Mirror the rename in gshadow's member and admin lists. Leaving + // it behind is exactly what grpck reports as "members differ", so + // the two files are committed together. + let mut files: Vec> = vec![Box::new(group_file)]; let gshadow_path = root.gshadow_path(); - if gshadow_path.exists() - && let Ok((mut gs, gs_layout)) = gshadow::read_gshadow_with_layout(&gshadow_path) - { - let mut gs_changed = false; - for g in &mut gs { + if gshadow_path.exists() { + let mut gshadow_file = + LockedFile::::open(&gshadow_path).map_err(|e| { + UsermodError::CantUpdateGroup(format!("cannot open gshadow: {e}")) + })?; + for g in gshadow_file.entries_mut() { for m in g.members.iter_mut().chain(g.admins.iter_mut()) { if *m == *login { m.clone_from(new_name); - gs_changed = true; } } } - if gs_changed { - atomic::atomic_write(&gshadow_path, |f| { - gshadow::write_gshadow_with_layout(&gs, &gs_layout, f) - }) - .map_err(|e| UsermodError::CantUpdateGroup(format!("{e}")))?; - } + files.push(Box::new(gshadow_file)); } - if changed { - atomic::atomic_write(&group_path, |f| { - group::write_group_with_layout(&ge, &group_layout, f) - }) - .map_err(|e| UsermodError::CantUpdate(format!("{e}")))?; - } - drop(glock); + transaction::commit_all(files) + .map_err(|e| UsermodError::CantUpdateGroup(format!("{e}")))?; } } @@ -375,22 +350,19 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // added a member that no longer exists. let member = new_login.unwrap_or(login); - let glock = FileLock::acquire(&group_path) - .map_err(|e| UsermodError::CantUpdateGroup(format!("cannot lock group: {e}")))?; - - let (mut ge, group_layout) = group::read_group_with_layout(&group_path) - .map_err(|e| UsermodError::CantUpdateGroup(format!("{e}")))?; + let mut group_file = LockedFile::::open(&group_path) + .map_err(|e| UsermodError::CantUpdateGroup(format!("cannot open group: {e}")))?; // Validate every requested group first: -G takes names or GIDs, // and each must exist (usermod(8) exit 6). let mut wanted: Vec = Vec::with_capacity(new_groups.len()); for gname in &new_groups { - let found = ge + let found = group_file + .entries() .iter() .find(|g| g.name == *gname || gname.parse::().is_ok_and(|id| g.gid == id)) .map(|g| g.name.clone()); let Some(name) = found else { - drop(glock); return Err(UsermodError::GroupNotFound(format!( "group '{gname}' does not exist" )) @@ -400,57 +372,44 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } if !append { - for g in &mut ge { + for g in group_file.entries_mut() { g.members.retain(|m| m != login && m != member); } } for gname in &wanted { - if let Some(g) = ge.iter_mut().find(|g| g.name == *gname) + if let Some(g) = group_file.find_mut(gname) && !g.members.iter().any(|m| m == member) { g.members.push(member.clone()); } } - atomic::atomic_write(&group_path, |f| { - group::write_group_with_layout(&ge, &group_layout, f) - }) - .map_err(|e| UsermodError::CantUpdateGroup(format!("{e}")))?; - drop(glock); - // /etc/gshadow carries the same membership lists; leaving it // behind is exactly what grpck reports as "members differ". + let mut files: Vec> = vec![Box::new(group_file)]; let gshadow_path = root.gshadow_path(); if gshadow_path.exists() { - let gshadow_guard = FileLock::acquire(&gshadow_path).map_err(|e| { - UsermodError::CantUpdateGroup(format!("cannot lock gshadow: {e}")) - })?; - let (mut gs, gs_layout) = gshadow::read_gshadow_with_layout(&gshadow_path) - .map_err(|e| UsermodError::CantUpdateGroup(format!("{e}")))?; - let mut gs_changed = false; + let mut gshadow_file = + LockedFile::::open(&gshadow_path).map_err(|e| { + UsermodError::CantUpdateGroup(format!("cannot open gshadow: {e}")) + })?; if !append { - for g in &mut gs { - let before = g.members.len(); + for g in gshadow_file.entries_mut() { g.members.retain(|m| m != login && m != member); - gs_changed |= g.members.len() != before; } } for gname in &wanted { - if let Some(g) = gs.iter_mut().find(|g| g.name == *gname) + if let Some(g) = gshadow_file.find_mut(gname) && !g.members.iter().any(|m| m == member) { g.members.push(member.clone()); - gs_changed = true; } } - if gs_changed { - atomic::atomic_write(&gshadow_path, |f| { - gshadow::write_gshadow_with_layout(&gs, &gs_layout, f) - }) - .map_err(|e| UsermodError::CantUpdateGroup(format!("{e}")))?; - } - drop(gshadow_guard); + files.push(Box::new(gshadow_file)); } + + transaction::commit_all(files) + .map_err(|e| UsermodError::CantUpdateGroup(format!("{e}")))?; } }