diff --git a/src/uu/ls/src/display.rs b/src/uu/ls/src/display.rs index 5bb1e773444..a30cd0279fc 100644 --- a/src/uu/ls/src/display.rs +++ b/src/uu/ls/src/display.rs @@ -314,14 +314,15 @@ fn pad_left(string: &str, count: usize) -> String { } /// Returns the alternate-access indicator that follows the 9-bit permission -/// string in long-format output: `.` for a non-trivial security context, `+` -/// for an ACL, otherwise a space (which acts as a placeholder so columns line -/// up across items that don't carry an indicator themselves). +/// string in long-format output: `+` for an ACL (even when the file also has a +/// security context, which is an extended attribute too), `.` for a non-trivial +/// security context alone, otherwise a space (which acts as a placeholder so +/// columns line up across items that don't carry an indicator themselves). fn alt_access_indicator(item: &PathData, config: &Config, is_acl_set: bool) -> u8 { - if item.security_context(config).len() > 1 { - b'.' - } else if is_acl_set { + if is_acl_set { b'+' + } else if item.security_context(config).len() > 1 { + b'.' } else { b' ' } diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs index a962718f4e8..ad46744c6ed 100644 --- a/src/uucore/src/lib/features/fsxattr.rs +++ b/src/uucore/src/lib/features/fsxattr.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore getxattr posix_acl_default posix_acl_access ENOTSUP EOPNOTSUPP renamer +// spell-checker:ignore getxattr posix_acl_default posix_acl_access nfs4_acl ENOTSUP EOPNOTSUPP renamer //! Set of functions to manage xattr on files and dirs @@ -249,8 +249,11 @@ pub fn apply_xattrs_fd_ignore_unsupported( /// Checks if a file has an Access Control List (ACL). /// -/// On macOS this asks the ACL API; elsewhere it looks at the file's extended -/// attributes. +/// On macOS this asks the ACL API. On Linux and Android it looks for the +/// attributes that store an ACL (for `NFSv4`, one that names someone besides the +/// owner, the group and everyone), so a security label (`security.selinux`) or a +/// `user.*` attribute alone is not an ACL. Elsewhere it looks at the file's +/// extended attributes. /// /// # Arguments /// @@ -259,14 +262,17 @@ pub fn apply_xattrs_fd_ignore_unsupported( /// /// # Returns /// -/// `true` if the file has an extended ACL (on macOS) or any extended attributes -/// (elsewhere), `false` otherwise. +/// `true` if the file has an extended ACL (on macOS, Linux and Android) or any +/// extended attributes (elsewhere), `false` otherwise. pub fn has_acl>(file: P, dereference: bool) -> bool { // On macOS most files carry some xattr (e.g. com.apple.provenance) and the // ACL is not one of them, so ask the ACL API instead. #[cfg(target_os = "macos")] return macos_acl::has_extended_acl(file.as_ref(), dereference); + #[cfg(not(target_os = "macos"))] + let file = file.as_ref(); + // don't use exacl here, it is doing more getxattr call then needed #[cfg(not(target_os = "macos"))] let attrs = if dereference { @@ -275,13 +281,75 @@ pub fn has_acl>(file: P, dereference: bool) -> bool { xattr::list(file) }; - #[cfg(not(target_os = "macos"))] - attrs.is_ok_and(|acl| { + // The attributes the kernel stores an ACL in. Other attributes, such as the + // SELinux or SMACK label, say nothing about an ACL. + #[cfg(any(target_os = "linux", target_os = "android"))] + return attrs.is_ok_and(|mut names| { + names.any(|name| { + if name == OsStr::new("system.posix_acl_access") + || name == OsStr::new("system.posix_acl_default") + { + return true; + } + if name != OsStr::new("system.nfs4_acl") { + return false; + } + // NFSv4 lists this attribute for every file, so look at its content. One that + // cannot be read is no evidence of an ACL. + let value = if dereference { + xattr::get_deref(file, &name) + } else { + xattr::get(file, &name) + }; + value + .ok() + .flatten() + .is_some_and(|raw| nfs4_acl_is_nontrivial(&raw)) + }) + }); + + #[cfg(not(any(target_os = "macos", target_os = "linux", target_os = "android")))] + attrs.is_ok_and(|mut acl| { // if we have extra attributes, we have an acl - acl.count() > 0 + acl.next().is_some() }) } +/// Whether an `NFSv4` ACL (the XDR value of `system.nfs4_acl`) names anyone besides +/// the owner, the group and everyone, which is all the mode bits can express. +/// A value that cannot be parsed counts as non-trivial. +#[cfg(any(target_os = "linux", target_os = "android"))] +fn nfs4_acl_is_nontrivial(raw: &[u8]) -> bool { + // Big-endian 32-bit words; strings are length-prefixed and padded to 4 bytes. + fn word(raw: &mut &[u8]) -> Option { + let (head, rest) = raw.split_first_chunk::<4>()?; + *raw = rest; + usize::try_from(u32::from_be_bytes(*head)).ok() + } + + fn named_principal(mut raw: &[u8]) -> Option { + let count = word(&mut raw)?; + for _ in 0..count { + // ACE type, flags and access mask, then the principal. + let [_, _, _, len] = [ + word(&mut raw)?, + word(&mut raw)?, + word(&mut raw)?, + word(&mut raw)?, + ]; + let padded = len.checked_add(3)? & !3; + let who = raw.get(..len)?; + raw = raw.get(padded..)?; + if !matches!(who, b"OWNER@" | b"GROUP@" | b"EVERYONE@") { + return Some(true); + } + } + Some(false) + } + + named_principal(raw).unwrap_or(true) +} + #[cfg(target_os = "macos")] mod macos_acl { use std::ffi::{CString, c_char, c_int, c_void}; @@ -555,15 +623,22 @@ mod tests { File::create(&file_path).unwrap(); - // FIXME: this fails on a system that uses SELinux assert!(!has_acl(&file_path, true)); let test_attr = "user.test_acl"; let test_value = b"test value"; xattr::set(&file_path, test_attr, test_value).unwrap(); - // On macOS an ACL isn't an xattr, so an xattr alone is not an ACL. - assert_eq!(has_acl(&file_path, true), cfg!(not(target_os = "macos"))); + // An ACL isn't any xattr: on macOS it is not an xattr at all, and on + // Linux only the posix_acl/nfs4_acl attributes are ACLs. + assert_eq!( + has_acl(&file_path, true), + cfg!(not(any( + target_os = "macos", + target_os = "linux", + target_os = "android" + ))) + ); assert!(!has_security_cap_acl(&file_path, true)); // FreeBSD/NetBSD's xattr library does not support the "security" namespace @@ -580,6 +655,75 @@ mod tests { } } + #[test] + #[cfg(any(target_os = "linux", target_os = "android"))] + fn test_nfs4_acl_is_nontrivial() { + // count, then per entry: type, flags, access mask, principal (length, padded). + fn acl(principals: &[&str]) -> Vec { + let mut raw = u32::try_from(principals.len()) + .unwrap() + .to_be_bytes() + .to_vec(); + for who in principals { + for word in [0u32, 0, 0x1f, u32::try_from(who.len()).unwrap()] { + raw.extend(word.to_be_bytes()); + } + raw.extend(who.as_bytes()); + raw.resize(raw.len().next_multiple_of(4), 0); + } + raw + } + + assert!(!nfs4_acl_is_nontrivial(&acl(&[]))); + assert!(!nfs4_acl_is_nontrivial(&acl(&[ + "OWNER@", + "GROUP@", + "EVERYONE@" + ]))); + assert!(nfs4_acl_is_nontrivial(&acl(&[ + "OWNER@", + "alice@example.com", + "EVERYONE@" + ]))); + assert!(nfs4_acl_is_nontrivial(&acl(&["1000"]))); + + // Truncated or garbage values are not trusted to be trivial. + let full = acl(&["OWNER@", "GROUP@"]); + assert!(nfs4_acl_is_nontrivial(&full[..full.len() - 1])); + assert!(nfs4_acl_is_nontrivial(&[0, 0])); + assert!(nfs4_acl_is_nontrivial(&[0xff; 8])); + } + + #[test] + #[cfg(any(target_os = "linux", target_os = "android"))] + fn test_file_has_acl_posix_attribute() { + let temp_dir = tempdir().unwrap(); + let file_path = temp_dir.path().join("test_file.txt"); + File::create(&file_path).unwrap(); + + // An extended POSIX ACL in its xattr form: version 2, then (tag, perm, id) + // entries. The named user entry is what makes it more than the mode bits, + // which the kernel would not store as an ACL. + let mut acl = 2u32.to_le_bytes().to_vec(); + for (tag, perm, id) in [ + (0x01u16, 6u16, u32::MAX), // user::rw- + (0x02, 4, 1000), // user:1000:r-- + (0x04, 4, u32::MAX), // group::r-- + (0x10, 4, u32::MAX), // mask::r-- + (0x20, 4, u32::MAX), // other::r-- + ] { + acl.extend(tag.to_le_bytes()); + acl.extend(perm.to_le_bytes()); + acl.extend(id.to_le_bytes()); + } + // Not every file system stores ACLs (or lets the test set them). + if xattr::set(&file_path, "system.posix_acl_access", &acl).is_err() { + return; + } + assert!(has_acl(&file_path, true)); + assert!(has_acl(&file_path, false)); + } + #[test] fn test_apply_and_retrieve_xattrs_fd() { use std::fs::OpenOptions; diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 6b02b890d36..8f3f7b0c2e7 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -5431,6 +5431,22 @@ fn test_ls_selinux_context_indicator() { ); } + // A file with an ACL as well as a context is listed with `+`: the context is + // an extended attribute too, so it must not hide the ACL. + at.touch("file_acl"); + if run_attr_tool( + "setfacl", + &["-m", "u:bin:r--", &at.plus_as_string("file_acl")], + ) { + let result = scene.ucmd().args(&["-l", "file_acl"]).succeeds(); + let first_line = result.stdout_str().lines().next().unwrap().to_string(); + assert_eq!( + first_line.chars().nth(10), + Some('+'), + "Expected '+' indicator for an ACL plus SELinux context, got: {first_line}", + ); + } + // Test that ls -lnZ properly shows the context for file in ["file", "link"] { let result = scene.ucmd().args(&["-lnZ", file]).succeeds(); @@ -7182,6 +7198,95 @@ fn test_acl_display_macos() { .stdout_matches(&re_without_acl); } +/// Runs an attribute tool (`setfacl`, `setfattr`) on a test file. Returns false, after +/// saying why, when it cannot be used here, so that the caller can skip its test. +#[cfg(any(target_os = "linux", target_os = "android"))] +fn run_attr_tool(tool: &str, args: &[&str]) -> bool { + use std::process::Command; + + // calling the command directly. xattr requires some dev packages to be installed + // and it adds a complex dependency just for a test + match Command::new(tool).args(args).status().map(|s| s.code()) { + Ok(Some(0)) => true, + Ok(_) => { + println!("test skipped: {tool} failed"); + false + } + Err(e) => { + println!("test skipped: {tool} failed with {e}"); + false + } + } +} + +// A real ACL is listed with `+`, so the check below that other attributes are not +// is not just passing because `+` never shows up. +#[cfg(any(target_os = "linux", target_os = "android"))] +#[test] +fn test_acl_display_access_acl_on_file() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("with_acl"); + at.touch("without_acl"); + + if !run_attr_tool( + "setfacl", + &["-m", "u:bin:r--", &at.plus_as_string("with_acl")], + ) { + return; + } + + let result = scene + .ucmd() + .args(&["-l", "with_acl", "without_acl"]) + .succeeds(); + let out = result.stdout_str(); + assert!( + Regex::new(r"(?m)^-[rwx-]{9}\+ .*with_acl$") + .unwrap() + .is_match(out), + "{out}" + ); + assert!( + Regex::new(r"(?m)^-[rwx-]{9}[^+].*without_acl$") + .unwrap() + .is_match(out), + "{out}" + ); +} + +// An extended attribute that is not an ACL (a security label is one too) must not +// be listed with the ACL `+`. +#[cfg(any(target_os = "linux", target_os = "android"))] +#[test] +fn test_acl_display_ignores_other_xattrs() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("with_xattr"); + + if !run_attr_tool( + "setfattr", + &[ + "-n", + "user.test", + "-v", + "1", + &at.plus_as_string("with_xattr"), + ], + ) { + return; + } + + // Whatever follows the mode bits (nothing, or `.` for a security context on + // an SELinux host), it is not the ACL `+`. + let re_without_acl = Regex::new(r"(?m)^-[rwx-]{9}[^+].*with_xattr$").unwrap(); + scene + .ucmd() + .args(&["-l", "with_xattr"]) + .succeeds() + .stdout_matches(&re_without_acl); +} + // Regression test for https://github.com/uutils/coreutils/issues/10980 // Each file with an ACL must not inflate the link-count column width. #[cfg(all(unix, not(target_vendor = "apple")))]