From dbc1674b9dfc2cc71d49fd50f7a125a8650e6baa Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 25 Aug 2026 21:12:00 +0200 Subject: [PATCH 1/2] cp: label the destination of -Z when it is named relatively Three problems kept tests/cp/cp-a-selinux.sh from running and passing under the SELinux CI job: * build-gnu.sh stubbed out src/getlimits with an empty, non-executable file when SELINUX_ENABLED=1, on the assumption that the SELinux tests do not use it. cp-a-selinux.sh calls getlimits_, so the test died with "hard error: running getlimits" (exit 99) before running anything. Build the real program instead -- only that one, together with the generated sources it needs, so the job still skips the rest of the GNU tree. Key the build on the program being executable rather than merely existing, and do it also when reusing an existing GNU build directory, so a tree left behind by the previous version of the script repairs itself. Remove a stale copy in the uutils build directory before installing it, since copying onto an existing file keeps that file's permissions. * -Z asks the policy for the default context of the destination, and the policy only ever lists absolute names: a relative name matched nothing, the lookup reported success because "no default context" is not an error, and the file silently kept the context it had inherited or already carried. Resolve the name first, leaving the last component alone so a symbolic link is still labelled itself. * A failure to set the context lost its errno on the way into CpError, so the ENOTSUP that a mount with a fixed context returns could no longer be recognised, and -a printed a diagnostic where GNU stays quiet. Should make test tests/cp/cp-a-selinux.sh pass https://github.com/uutils/coreutils/issues/13841 --- src/uu/cp/locales/en-US.ftl | 2 +- src/uu/cp/locales/fr-FR.ftl | 2 +- src/uu/cp/src/cp.rs | 18 ++++++-- src/uucore/src/lib/features/selinux.rs | 59 +++++++++++++++++++++++++- tests/by-util/test_cp.rs | 44 +++++++++++++++++++ util/build-gnu.sh | 35 +++++++++++---- 6 files changed, 145 insertions(+), 15 deletions(-) diff --git a/src/uu/cp/locales/en-US.ftl b/src/uu/cp/locales/en-US.ftl index e41a1225017..49fec90cc9c 100644 --- a/src/uu/cp/locales/en-US.ftl +++ b/src/uu/cp/locales/en-US.ftl @@ -84,7 +84,7 @@ cp-error-reflink-not-supported = --reflink is only supported on linux and macOS cp-error-sparse-not-supported = --sparse is only supported on linux cp-error-not-a-directory = { $path } is not a directory cp-error-selinux-not-enabled = SELinux was not enabled during the compile time! -cp-error-selinux-set-context = failed to set the security context of { $path }: { $error } +cp-error-selinux-set-context = failed to set the security context of { $path } cp-error-selinux-get-context = failed to get security context of { $path } cp-error-selinux-error = SELinux error: { $error } cp-error-selinux-context-conflict = cannot combine --context (-Z) with --preserve=context diff --git a/src/uu/cp/locales/fr-FR.ftl b/src/uu/cp/locales/fr-FR.ftl index 77d1d4ce42a..43911f8280c 100644 --- a/src/uu/cp/locales/fr-FR.ftl +++ b/src/uu/cp/locales/fr-FR.ftl @@ -84,7 +84,7 @@ cp-error-reflink-not-supported = --reflink n'est pris en charge que sur linux et cp-error-sparse-not-supported = --sparse n'est pris en charge que sur linux cp-error-not-a-directory = { $path } n'est pas un répertoire cp-error-selinux-not-enabled = SELinux n'était pas activé lors de la compilation ! -cp-error-selinux-set-context = échec de la définition du contexte de sécurité de { $path } : { $error } +cp-error-selinux-set-context = échec de la définition du contexte de sécurité de { $path } cp-error-selinux-get-context = échec de l'obtention du contexte de sécurité de { $path } cp-error-selinux-error = Erreur SELinux : { $error } cp-error-selinux-context-conflict = impossible de combiner --context (-Z) avec --preserve=context diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 41872be6593..75ecba03da6 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1934,9 +1934,21 @@ pub(crate) fn copy_attributes( CpError::Error(translate!("cp-error-selinux-get-context", "path" => source.quote())) })?; if let Some(context) = context { - context.set_for_path(dest, false, false).map_err(|e|CpError::Error( - translate!("cp-error-selinux-set-context", "path" => dest.quote(), "error" => e), - ))?; + context.set_for_path(dest, false, false).map_err(|e| { + // Keep the errno: the ENOTSUP of a mount with a fixed context is + // what -a and --preserve=all have to stay quiet about. + let source = match e { + selinux::errors::Error::IO { source, .. } + | selinux::errors::Error::IO1Name { source, .. } + | selinux::errors::Error::IO1Path { source, .. } + | selinux::errors::Error::IO1Process { source, .. } => source, + e => io::Error::other(e), + }; + CpError::IoErrContext( + source, + translate!("cp-error-selinux-set-context", "path" => dest.quote()), + ) + })?; } Ok(()) })?; diff --git a/src/uucore/src/lib/features/selinux.rs b/src/uucore/src/lib/features/selinux.rs index 6f0bc026272..314b0b96775 100644 --- a/src/uucore/src/lib/features/selinux.rs +++ b/src/uucore/src/lib/features/selinux.rs @@ -5,6 +5,7 @@ // spell-checker:ignore defaultcon setfscreatecon //! Set of functions to manage SELinux security contexts +use std::borrow::Cow; use std::error::Error; use std::marker::PhantomData; use std::path::Path; @@ -290,7 +291,8 @@ pub fn set_selinux_security_context( }) } else { // If no context provided, set the default SELinux context for the path - SecurityContext::set_default_for_path(path).map_err(|e| match &e { + let path = absolute_for_policy_lookup(path); + SecurityContext::set_default_for_path(&path).map_err(|e| match &e { selinux::errors::Error::IO1Path { source, .. } if source.raw_os_error() == Some(libc::ENOTSUP) => { @@ -301,6 +303,35 @@ pub fn set_selinux_security_context( } } +/// Returns `path` as an absolute name, for the default context lookup. +/// +/// The policy's `file_contexts` only ever holds absolute names, so a relative +/// one matches nothing -- and libselinux reports the miss as success, leaving +/// the object with the context it already carried. +fn absolute_for_policy_lookup(path: &Path) -> Cow<'_, Path> { + if path.is_absolute() { + return Cow::Borrowed(path); + } + // Resolve the directory only, so that `.` and `..` do not reach the policy + // patterns; the last component is labelled itself, symbolic link or not. + let resolved = match (path.parent(), path.file_name()) { + (Some(directory), Some(name)) => { + let directory = if directory.as_os_str().is_empty() { + Path::new(".") + } else { + directory + }; + std::fs::canonicalize(directory).map(|directory| directory.join(name)) + } + // No last component to keep: the name ends in `.` or `..`, which names + // a directory and never a symbolic link, so resolve the whole of it. + // A lexical absolute name would keep those components and match no + // pattern. + _ => std::fs::canonicalize(path), + }; + resolved.map_or(Cow::Borrowed(path), Cow::Owned) +} + /// Gets the SELinux security context for the given filesystem path. /// /// Retrieves the security context of the specified filesystem path if SELinux is enabled @@ -532,6 +563,32 @@ mod tests { use super::*; use tempfile::NamedTempFile; + #[test] + fn test_absolute_for_policy_lookup() { + let directory = tempfile::tempdir().expect("Failed to create tempdir"); + let directory = std::fs::canonicalize(directory.path()).expect("Failed to canonicalize"); + std::fs::create_dir(directory.join("branch")).expect("Failed to create dir"); + let previous = std::env::current_dir().expect("Failed to read the working directory"); + std::env::set_current_dir(&directory).expect("Failed to change the working directory"); + + // An absolute name is handed to the policy untouched. + let untouched = Path::new("/opt/quokka"); + assert_eq!(absolute_for_policy_lookup(untouched), untouched); + + // A relative one becomes absolute, and `.`/`..` never reach the policy. + for (given, expected) in [ + ("quokka", directory.join("quokka")), + ("./quokka", directory.join("quokka")), + ("branch/../quokka", directory.join("quokka")), + ("branch/..", directory.clone()), + (".", directory.clone()), + ] { + assert_eq!(absolute_for_policy_lookup(Path::new(given)), expected); + } + + std::env::set_current_dir(previous).expect("Failed to restore the working directory"); + } + #[test] fn test_selinux_context_setting() { let tmpfile = NamedTempFile::new().expect("Failed to create tempfile"); diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index e8be391639b..8ea4e3fa418 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -7320,6 +7320,50 @@ fn test_cp_selinux() { } } +#[test] +#[cfg(all( + feature = "feat_selinux", + any(target_os = "linux", target_os = "android") +))] +fn test_cp_selinux_default_context_relative_dest() { + // -Z labels the destination with the context the policy has for its path, + // and the policy only lists absolute paths: a relative destination used to + // match nothing and silently keep the context it already carried. + use std::path::Path; + use uucore::selinux::set_selinux_security_context; + + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + at.touch(TEST_HELLO_WORLD_SOURCE); + + // A type the policy will not hand out for a path under the test directory, + // so that the comparison below still sees a difference when -Z leaves the + // relative destination alone. Usable even when mcstransd is not running. + let ctx = "root:object_r:etc_t:s0".to_string(); + for dest in ["relative", "absolute"] { + at.touch(dest); + if set_selinux_security_context(Path::new(&at.plus_as_string(dest)), Some(&ctx)).is_err() { + return; + } + } + + let absolute = at.plus_as_string("absolute"); + ts.ucmd() + .args(&["-Z", TEST_HELLO_WORLD_SOURCE, &absolute]) + .succeeds(); + ts.ucmd() + .args(&["-Z", TEST_HELLO_WORLD_SOURCE, "relative"]) + .succeeds(); + + // Compare the type only, the one field -Z is about. + let selinux_type = |context: &str| context.split(':').nth(2).unwrap_or("").to_string(); + assert_eq!( + selinux_type(&get_getfattr_output(&at.plus_as_string("relative"))), + selinux_type(&get_getfattr_output(&absolute)), + "-Z gave a relative and an absolute destination different contexts" + ); +} + #[test] #[cfg(all( feature = "feat_selinux", diff --git a/util/build-gnu.sh b/util/build-gnu.sh index cb382e3f4b3..aeaac7216ff 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -114,6 +114,9 @@ done # This ensures the correct path is used even if the repository was moved or rebuilt in a different location sed -i "s/^[[:blank:]]*PATH=.*/ PATH='${UU_BUILD_DIR//\//\\/}\$(PATH_SEPARATOR)'\"\$\$PATH\" \\\/" tests/local.mk +# Use GNU nproc for *BSD and macOS +NPROC="$(command -v nproc||command -v gnproc)" + if test -f gnu-built; then echo "GNU build already found. Skip" echo "'rm -f $(pwd)/{gnu-built,src/getlimits}' to force the build" @@ -132,12 +135,11 @@ else # Use a better diff sed -i 's|diff -c|diff -u|g' tests/Coreutils.pm - # Skip make if possible - # Use GNU nproc for *BSD and macOS - NPROC="$(command -v nproc||command -v gnproc)" - test "${SELINUX_ENABLED}" = 1 && touch src/getlimits # SELinux tests does not use it - test -f src/getlimits || make -j "$("${NPROC}")" - cp -f src/getlimits "${UU_BUILD_DIR}" + # Skip make if possible. The SELinux job does not build the GNU tree at all; + # the block after this "if" builds the one program its tests need. + if [ "${SELINUX_ENABLED}" != 1 ]; then + test -x src/getlimits || make -j "$("${NPROC}")" + fi # Handle generated factor tests t_first=00 @@ -161,9 +163,24 @@ else touch gnu-built fi -# Keep getlimits available on PATH for GNU shell and Perl tests even when -# reusing an existing GNU build directory. -test -f src/getlimits && cp -f src/getlimits "${UU_BUILD_DIR}" +# The GNU shell and Perl tests call getlimits_, so getlimits has to be a real +# program on PATH, also when reusing an existing GNU build directory. An earlier +# version of this script left an empty, non-executable stub behind under +# SELINUX_ENABLED, hence the test for an executable rather than for a file. +# Build only that program, plus the generated sources that the "all" target +# would otherwise pull in, so the SELinux job still skips the rest of the tree. +if ! test -x src/getlimits; then + rm -f src/getlimits + printf 'built-sources: $(BUILT_SOURCES)\n' | + make -f Makefile -f - -j "$("${NPROC}")" built-sources || true + make -j "$("${NPROC}")" src/getlimits || true +fi +# Remove the destination first: cp keeps the permissions of an existing file, so +# a leftover stub there would stay non-executable. +if test -x src/getlimits; then + rm -f "${UU_BUILD_DIR}/getlimits" + cp -f src/getlimits "${UU_BUILD_DIR}" +fi # Keep Makefile.in newer than the local.mk files we just modified, # and Makefile newer than Makefile.in, so make won't re-run From 1b724521e27a718394d0daa6c42a331d91b3a176 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 25 Aug 2026 22:31:00 +0200 Subject: [PATCH 2/2] build-gnu.sh: build getlimits after the Makefile timestamps are fixed Stripping the factor tests leaves tests/local.mk with a trailing backslash that automake rejects. Building getlimits while local.mk was still newer than Makefile.in sent make into the remake rule, which died there; the || true hid it, nothing was copied to the uutils build directory, and the tests calling getlimits_ ended in "hard error: running getlimits". Move the build below the touch that settles those timestamps, and say so when getlimits is missing instead of failing silently. Should make test tests/cp/cp-a-selinux.sh pass --- util/build-gnu.sh | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/util/build-gnu.sh b/util/build-gnu.sh index aeaac7216ff..7d5e2e86bc2 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -163,30 +163,35 @@ else touch gnu-built fi +# Keep Makefile.in newer than the local.mk files we just modified, +# and Makefile newer than Makefile.in, so make won't re-run +# automake or config.status and undo our edits. +touch Makefile.in Makefile + # The GNU shell and Perl tests call getlimits_, so getlimits has to be a real # program on PATH, also when reusing an existing GNU build directory. An earlier # version of this script left an empty, non-executable stub behind under # SELINUX_ENABLED, hence the test for an executable rather than for a file. # Build only that program, plus the generated sources that the "all" target # would otherwise pull in, so the SELinux job still skips the rest of the tree. +# This has to come after the touch above: stripping the factor tests leaves +# tests/local.mk with a trailing backslash that automake rejects, so a make that +# still sees it as newer than Makefile.in dies in the remake rule. if ! test -x src/getlimits; then rm -f src/getlimits printf 'built-sources: $(BUILT_SOURCES)\n' | make -f Makefile -f - -j "$("${NPROC}")" built-sources || true make -j "$("${NPROC}")" src/getlimits || true fi -# Remove the destination first: cp keeps the permissions of an existing file, so -# a leftover stub there would stay non-executable. if test -x src/getlimits; then + # Remove the destination first: cp keeps the permissions of an existing + # file, so a leftover stub there would stay non-executable. rm -f "${UU_BUILD_DIR}/getlimits" cp -f src/getlimits "${UU_BUILD_DIR}" +else + echo "WARNING: could not build getlimits; tests calling getlimits_ will fail" >&2 fi -# Keep Makefile.in newer than the local.mk files we just modified, -# and Makefile newer than Makefile.in, so make won't re-run -# automake or config.status and undo our edits. -touch Makefile.in Makefile - # Patch the Makefile PATH to point to uutils build dir instead of GNU src/ sed -i "s/^[[:blank:]]*PATH=.*/ PATH='${UU_BUILD_DIR//\//\\/}\$(PATH_SEPARATOR)'\"\$\$PATH\" \\\/" Makefile # Prevent make check from rebuilding the GNU binaries over the uutils ones