diff --git a/src/uu/usermod/src/usermod.rs b/src/uu/usermod/src/usermod.rs index bfc1ed4..27fdec8 100644 --- a/src/uu/usermod/src/usermod.rs +++ b/src/uu/usermod/src/usermod.rs @@ -163,8 +163,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { return Err(UsermodError::UserNotFound(format!("user '{login}' does not exist")).into()); }; - // Save the old UID and home dir before mutation so we can chown if needed. + // Save the old UID, GID and home dir before mutation so we can chown if needed. let old_uid = entries[idx].uid; + let old_primary_gid = entries[idx].gid; let home_for_chown = entries[idx].home.clone(); let home_is_changing = matches.get_one::(options::HOME).is_some(); @@ -224,6 +225,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } let new_uid = entries[idx].uid; + let new_primary_gid = entries[idx].gid; passwd_file .commit() @@ -232,14 +234,25 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Restore signals before potentially long-running recursive chown. drop(signals); - // If the UID changed and the home directory was not explicitly moved, - // recursively chown the existing home directory to the new UID. - // Only files owned by old_uid are touched (files owned by other users - // are left alone, matching GNU shadow-utils behavior). - if new_uid != old_uid && !home_is_changing && !home_for_chown.is_empty() { + // If the UID or the primary group changed and the home directory was not + // explicitly moved, recursively chown the existing home directory. + // usermod(8): only files owned by the old UID get the new UID, and only + // files in the old primary group get the new group. + if (new_uid != old_uid || new_primary_gid != old_primary_gid) + && !home_is_changing + && !home_for_chown.is_empty() + { + use std::os::unix::fs::MetadataExt; + + // Like GNU, only a home directory the user owns is walked, and a + // symlinked home is not followed. let home_path = root.resolve(&home_for_chown); - if home_path.exists() { - recursive_chown(&home_path, old_uid, new_uid); + if std::fs::symlink_metadata(&home_path).is_ok_and(|m| m.is_dir() && m.uid() == old_uid) { + recursive_chown( + &home_path, + (old_uid, new_uid), + (old_primary_gid, new_primary_gid), + ); } } @@ -507,42 +520,47 @@ fn apply_subid_changes( Ok(()) } -/// Recursively chown all files and directories under `path` that are owned by -/// `old_uid` to `new_uid`. Files owned by other users are left untouched. +/// Recursively chown all files and directories under `path`: those owned by +/// the old UID get the new UID, and those whose group is the old GID get the +/// new GID. Other owners and groups are left untouched. /// /// Uses `fchownat` with `AT_SYMLINK_NOFOLLOW` so symlinks themselves are /// re-owned without following them. -fn recursive_chown(path: &Path, old_uid: u32, new_uid: u32) { - use std::os::unix::fs::MetadataExt; - +fn recursive_chown(path: &Path, owner: (u32, u32), group: (u32, u32)) { if let Ok(entries) = std::fs::read_dir(path) { for entry in entries.flatten() { let entry_path = entry.path(); if let Ok(meta) = std::fs::symlink_metadata(&entry_path) { - if meta.uid() == old_uid { - let _ = rustix::fs::chownat( - rustix::fs::CWD, - &entry_path, - Some(rustix::process::Uid::from_raw(new_uid)), - None, - rustix::fs::AtFlags::SYMLINK_NOFOLLOW, - ); - } + chown_matching(&entry_path, &meta, owner, group); if meta.is_dir() { - recursive_chown(&entry_path, old_uid, new_uid); + recursive_chown(&entry_path, owner, group); } } } } // Also chown the directory itself. - if let Ok(meta) = std::fs::symlink_metadata(path) - && meta.uid() == old_uid - { + if let Ok(meta) = std::fs::symlink_metadata(path) { + chown_matching(path, &meta, owner, group); + } +} + +/// Give `path` the new UID if it has the old one, and the new GID if it has +/// the old one. +fn chown_matching(path: &Path, meta: &std::fs::Metadata, owner: (u32, u32), group: (u32, u32)) { + use std::os::unix::fs::MetadataExt; + + let (old_owner, new_owner) = owner; + let (old_group, new_group) = group; + let set_owner = (old_owner != new_owner && meta.uid() == old_owner) + .then(|| rustix::process::Uid::from_raw(new_owner)); + let set_group = (old_group != new_group && meta.gid() == old_group) + .then(|| rustix::process::Gid::from_raw(new_group)); + if set_owner.is_some() || set_group.is_some() { let _ = rustix::fs::chownat( rustix::fs::CWD, path, - Some(rustix::process::Uid::from_raw(new_uid)), - None, + set_owner, + set_group, rustix::fs::AtFlags::SYMLINK_NOFOLLOW, ); } diff --git a/tests/by-util/test_usermod.rs b/tests/by-util/test_usermod.rs index 8ec5cbf..1f1257c 100644 --- a/tests/by-util/test_usermod.rs +++ b/tests/by-util/test_usermod.rs @@ -209,6 +209,147 @@ fn test_change_uid() { ); } +/// Owner and group of `path`, without following a symlink. +fn owner_of(path: &std::path::Path) -> (u32, u32) { + use std::os::unix::fs::MetadataExt; + + let meta = std::fs::symlink_metadata(path).expect("stat failed"); + (meta.uid(), meta.gid()) +} + +/// A prefix with testuser (1000:1000) and group dev (2000). The home holds a +/// file of testuser's, a root-owned file in the old group, a file in another +/// group, and a symlink in the old group to a file outside the home. +fn setup_home_with_files() -> (tempfile::TempDir, std::path::PathBuf) { + let dir = setup_prefix( + "testuser:x:1000:1000:Test:/home/testuser:/bin/bash\n", + "testuser:$6$hash:19500:0:99999:7:::\n", + "testuser:x:1000:\ndev:x:2000:\n", + ); + let home = dir.path().join("home/testuser"); + std::fs::create_dir_all(home.join("sub")).expect("failed to create home"); + for (path, uid, gid) in [ + (home.join("mine"), 1000, 1000), + (home.join("root-owned"), 0, 1000), + (home.join("other-group"), 1000, 0), + (dir.path().join("outside"), 1000, 1000), + ] { + std::fs::write(&path, "").expect("failed to write file"); + std::os::unix::fs::chown(&path, Some(uid), Some(gid)).expect("chown failed"); + } + std::os::unix::fs::symlink(dir.path().join("outside"), home.join("link")) + .expect("failed to create symlink"); + std::os::unix::fs::lchown(home.join("link"), Some(1000), Some(1000)).expect("chown failed"); + std::os::unix::fs::chown(&home, Some(1000), Some(1000)).expect("chown failed"); + std::os::unix::fs::chown(home.join("sub"), Some(1000), Some(1000)).expect("chown failed"); + (dir, home) +} + +#[test] +fn test_change_gid_regroups_home_files() { + if crate::common::skip_unless_root() { + return; + } + + let (dir, home) = setup_home_with_files(); + let code = run_with_prefix(&dir, &["-g", "dev", "testuser"]); + assert_eq!(code, 0, "changing the primary group should succeed"); + + for (name, expected) in [ + ("", (1000, 2000)), + ("sub", (1000, 2000)), + ("mine", (1000, 2000)), + ("root-owned", (0, 2000)), + ("other-group", (1000, 0)), + ("link", (1000, 2000)), + ] { + assert_eq!(owner_of(&home.join(name)), expected, "home entry '{name}'"); + } + assert_eq!( + owner_of(&dir.path().join("outside")), + (1000, 1000), + "the symlink target outside the home should be left alone" + ); +} + +#[test] +fn test_change_uid_and_gid_rechowns_home_files() { + if crate::common::skip_unless_root() { + return; + } + + let (dir, home) = setup_home_with_files(); + let code = run_with_prefix(&dir, &["-u", "1500", "-g", "dev", "testuser"]); + assert_eq!(code, 0, "changing the UID and primary group should succeed"); + + for (name, expected) in [ + ("", (1500, 2000)), + ("sub", (1500, 2000)), + ("mine", (1500, 2000)), + ("root-owned", (0, 2000)), + ("other-group", (1500, 0)), + ("link", (1500, 2000)), + ] { + assert_eq!(owner_of(&home.join(name)), expected, "home entry '{name}'"); + } + assert_eq!( + owner_of(&dir.path().join("outside")), + (1000, 1000), + "the symlink target outside the home should be left alone" + ); +} + +#[test] +fn test_home_not_owned_by_user_is_left_alone() { + if crate::common::skip_unless_root() { + return; + } + + for args in [["-g", "dev"], ["-u", "1500"]] { + for symlinked in [false, true] { + let dir = setup_prefix( + "testuser:x:1000:1000:Test:/home/testuser:/bin/bash\n", + "testuser:$6$hash:19500:0:99999:7:::\n", + "testuser:x:1000:\ndev:x:2000:\n", + ); + let home = dir.path().join("home/testuser"); + // A symlinked home points at a real directory the user owns. + let real = if symlinked { + dir.path().join("real-home") + } else { + home.clone() + }; + std::fs::create_dir_all(&real).expect("failed to create home"); + std::fs::write(real.join("mine"), "").expect("failed to write file"); + std::os::unix::fs::chown(real.join("mine"), Some(1000), Some(1000)) + .expect("chown failed"); + let real_owner = if symlinked { 1000 } else { 0 }; + std::os::unix::fs::chown(&real, Some(real_owner), Some(1000)).expect("chown failed"); + if symlinked { + std::fs::create_dir_all(home.parent().expect("home has a parent")) + .expect("failed to create /home"); + std::os::unix::fs::symlink(&real, &home).expect("failed to create symlink"); + } + + let code = run_with_prefix(&dir, &[args[0], args[1], "testuser"]); + // GNU exits 12 for a symlinked home: #351. + if !symlinked { + assert_eq!(code, 0, "{args:?} should succeed"); + } + assert_eq!( + owner_of(&real), + (real_owner, 1000), + "{args:?}, symlinked {symlinked}: home should be left alone" + ); + assert_eq!( + owner_of(&real.join("mine")), + (1000, 1000), + "{args:?}, symlinked {symlinked}: files under it should be left alone" + ); + } + } +} + #[test] fn test_add_to_groups() { if crate::common::skip_unless_root() {