From e7fdd3dfba3c25e50979a2586fd40dee3d086d7b Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Thu, 1 May 2025 14:59:53 +0200 Subject: [PATCH 1/4] selinux: add support in cp --- src/uu/cp/Cargo.toml | 2 +- src/uu/cp/src/cp.rs | 57 +++++++++--- tests/by-util/test_cp.rs | 189 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 224 insertions(+), 24 deletions(-) diff --git a/src/uu/cp/Cargo.toml b/src/uu/cp/Cargo.toml index 7dd1cfdb4..fd5b4696e 100644 --- a/src/uu/cp/Cargo.toml +++ b/src/uu/cp/Cargo.toml @@ -47,5 +47,5 @@ name = "cp" path = "src/main.rs" [features] -feat_selinux = ["selinux"] +feat_selinux = ["selinux", "uucore/selinux"] feat_acl = ["exacl"] diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 60d9c98fe..49841d741 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -16,7 +16,7 @@ use std::path::{Path, PathBuf, StripPrefixError}; #[cfg(all(unix, not(target_os = "android")))] use uucore::fsxattr::copy_xattrs; -use clap::{Arg, ArgAction, ArgMatches, Command, builder::ValueParser}; +use clap::{Arg, ArgAction, ArgMatches, Command, builder::ValueParser, value_parser}; use filetime::FileTime; use indicatif::{ProgressBar, ProgressStyle}; use quick_error::ResultExt; @@ -311,6 +311,10 @@ pub struct Options { pub verbose: bool, /// `-g`, `--progress` pub progress_bar: bool, + /// -Z + pub set_selinux_context: bool, + // --context + pub context: Option, } impl Default for Options { @@ -337,6 +341,8 @@ impl Default for Options { debug: false, verbose: false, progress_bar: false, + set_selinux_context: false, + context: None, } } } @@ -448,6 +454,7 @@ mod options { pub const RECURSIVE: &str = "recursive"; pub const REFLINK: &str = "reflink"; pub const REMOVE_DESTINATION: &str = "remove-destination"; + pub const SELINUX: &str = "Z"; pub const SPARSE: &str = "sparse"; pub const STRIP_TRAILING_SLASHES: &str = "strip-trailing-slashes"; pub const SYMBOLIC_LINK: &str = "symbolic-link"; @@ -476,6 +483,7 @@ const PRESERVE_DEFAULT_VALUES: &str = if cfg!(unix) { } else { "mode,timestamp" }; + pub fn uu_app() -> Command { const MODE_ARGS: &[&str] = &[ options::LINK, @@ -709,24 +717,25 @@ pub fn uu_app() -> Command { .value_parser(ShortcutValueParser::new(["never", "auto", "always"])) .help("control creation of sparse files. See below"), ) - // TODO: implement the following args .arg( - Arg::new(options::COPY_CONTENTS) - .long(options::COPY_CONTENTS) - .overrides_with(options::ATTRIBUTES_ONLY) - .help("NotImplemented: copy contents of special files when recursive") + Arg::new(options::SELINUX) + .short('Z') + .help("set SELinux security context of destination file to default type") .action(ArgAction::SetTrue), ) .arg( Arg::new(options::CONTEXT) .long(options::CONTEXT) .value_name("CTX") + .value_parser(value_parser!(String)) .help( - "NotImplemented: set SELinux security context of destination file to \ - default type", - ), + "like -Z, or if CTX is specified then set the SELinux or SMACK security \ + context to CTX", + ) + .num_args(0..=1) + .require_equals(true) + .default_missing_value(""), ) - // END TODO .arg( // The 'g' short flag is modeled after advcpmv // See this repo: https://github.com/jarun/advcpmv @@ -739,6 +748,15 @@ pub fn uu_app() -> Command { Note: this feature is not supported by GNU coreutils.", ), ) + // TODO: implement the following args + .arg( + Arg::new(options::COPY_CONTENTS) + .long(options::COPY_CONTENTS) + .overrides_with(options::ATTRIBUTES_ONLY) + .help("NotImplemented: copy contents of special files when recursive") + .action(ArgAction::SetTrue), + ) + // END TODO .arg( Arg::new(options::PATHS) .action(ArgAction::Append) @@ -971,7 +989,6 @@ impl Options { let not_implemented_opts = vec![ #[cfg(not(any(windows, unix)))] options::ONE_FILE_SYSTEM, - options::CONTEXT, #[cfg(windows)] options::FORCE, ]; @@ -1018,7 +1035,6 @@ impl Options { return Err(Error::NotADirectory(dir.clone())); } }; - // cp follows POSIX conventions for overriding options such as "-a", // "-d", "--preserve", and "--no-preserve". We can use clap's // override-all behavior to achieve this, but there's a challenge: when @@ -1112,6 +1128,15 @@ impl Options { } } + // Extract the SELinux related flags and options + let set_selinux_context = matches.get_flag(options::SELINUX); + + let context = if matches.contains_id(options::CONTEXT) { + matches.get_one::(options::CONTEXT).cloned() + } else { + None + }; + let options = Self { attributes_only: matches.get_flag(options::ATTRIBUTES_ONLY), copy_contents: matches.get_flag(options::COPY_CONTENTS), @@ -1172,6 +1197,8 @@ impl Options { recursive, target_dir, progress_bar: matches.get_flag(options::PROGRESS_BAR), + set_selinux_context: set_selinux_context || context.is_some(), + context, }; Ok(options) @@ -2422,6 +2449,12 @@ fn copy_file( copy_attributes(source, dest, &options.attributes)?; } + #[cfg(feature = "selinux")] + if options.set_selinux_context && uucore::selinux::is_selinux_enabled() { + // Set the given selinux permissions on the copied file. + uucore::selinux::set_selinux_security_context(dest, options.context.as_ref())?; + } + copied_files.insert( FileInformation::from_path(source, options.dereference(source_in_command_line))?, dest.to_path_buf(), diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index a95e1b599..19d0d5a77 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.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 (flags) reflink (fs) tmpfs (linux) rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR procfs outfile uufs xattrs -// spell-checker:ignore bdfl hlsl IRWXO IRWXG +// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined use uutests::at_and_ucmd; use uutests::new_ucmd; use uutests::path_concat; @@ -908,32 +908,32 @@ fn test_cp_arg_no_clobber_twice() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - at.touch("source.txt"); + at.touch(TEST_HELLO_WORLD_SOURCE); scene .ucmd() .arg("--no-clobber") - .arg("source.txt") - .arg("dest.txt") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HELLO_WORLD_DEST) .arg("--debug") .succeeds() .no_stderr(); - assert_eq!(at.read("source.txt"), ""); + assert_eq!(at.read(TEST_HELLO_WORLD_SOURCE), ""); - at.append("source.txt", "some-content"); + at.append(TEST_HELLO_WORLD_SOURCE, "some-content"); scene .ucmd() .arg("--no-clobber") - .arg("source.txt") - .arg("dest.txt") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HELLO_WORLD_DEST) .arg("--debug") .succeeds() - .stdout_contains("skipped 'dest.txt'"); + .stdout_contains(format!("skipped '{}'", TEST_HELLO_WORLD_DEST)); - assert_eq!(at.read("source.txt"), "some-content"); + assert_eq!(at.read(TEST_HELLO_WORLD_SOURCE), "some-content"); // Should be empty as the "no-clobber" should keep // the previous version - assert_eq!(at.read("dest.txt"), ""); + assert_eq!(at.read(TEST_HELLO_WORLD_DEST), ""); } #[test] @@ -6248,3 +6248,170 @@ fn test_cp_update_none_interactive_prompt_no() { assert_eq!(at.read(old_file), "old content"); assert_eq!(at.read(new_file), "new content"); } + +#[cfg(feature = "feat_selinux")] +fn get_getfattr_output(f: &str) -> String { + use std::process::Command; + + let getfattr_output = Command::new("getfattr") + .arg(f) + .arg("-n") + .arg("security.selinux") + .output() + .expect("Failed to run `getfattr` on the destination file"); + println!("{:?}", getfattr_output); + assert!( + getfattr_output.status.success(), + "getfattr did not run successfully: {}", + String::from_utf8_lossy(&getfattr_output.stderr) + ); + + String::from_utf8_lossy(&getfattr_output.stdout) + .split('"') + .nth(1) + .unwrap_or("") + .to_string() +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_cp_selinux() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + let args = ["-Z", "--context=unconfined_u:object_r:user_tmp_t:s0"]; + at.touch(TEST_HELLO_WORLD_SOURCE); + for arg in args { + ts.ucmd() + .arg(arg) + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HELLO_WORLD_DEST) + .succeeds(); + assert!(at.file_exists(TEST_HELLO_WORLD_DEST)); + + let selinux_perm = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); + + assert!( + selinux_perm.contains("unconfined_u"), + "Expected '{}' not found in getfattr output:\n{}", + "foo", + selinux_perm + ); + at.remove(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); + } +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_cp_selinux_invalid() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch(TEST_HELLO_WORLD_SOURCE); + let args = [ + "--context=a", + "--context=unconfined_u:object_r:user_tmp_t:s0:a", + "--context=nconfined_u:object_r:user_tmp_t:s0", + ]; + for arg in args { + new_ucmd!() + .arg(arg) + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HELLO_WORLD_DEST) + .fails() + .stderr_contains("Failed to"); + if at.file_exists(TEST_HELLO_WORLD_DEST) { + at.remove(TEST_HELLO_WORLD_DEST); + } + } +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_cp_preserve_selinux() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + let args = ["-Z", "--context=unconfined_u:object_r:user_tmp_t:s0"]; + at.touch(TEST_HELLO_WORLD_SOURCE); + for arg in args { + ts.ucmd() + .arg(arg) + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HELLO_WORLD_DEST) + .arg("--preserve=all") + .succeeds(); + assert!(at.file_exists(TEST_HELLO_WORLD_DEST)); + let selinux_perm_dest = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); + assert!( + selinux_perm_dest.contains("unconfined_u"), + "Expected '{}' not found in getfattr output:\n{}", + "foo", + selinux_perm_dest + ); + assert_eq!( + get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_SOURCE)), + selinux_perm_dest + ); + + #[cfg(all(unix, not(target_os = "freebsd")))] + { + // Assert that the mode, ownership, and timestamps are preserved + // NOTICE: the ownership is not modified on the src file, because that requires root permissions + let metadata_src = at.metadata(TEST_HELLO_WORLD_SOURCE); + let metadata_dst = at.metadata(TEST_HELLO_WORLD_DEST); + assert_metadata_eq!(metadata_src, metadata_dst); + } + + at.remove(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); + } +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_cp_preserve_selinux_admin_context() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + let admin_context = "system_u:object_r:admin_home_t:s0"; + + at.touch(TEST_HELLO_WORLD_SOURCE); + + let cmd_result = ts + .ucmd() + .arg("-Z") + .arg(format!("--context={admin_context}")) + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HELLO_WORLD_DEST) + .run(); + + if !cmd_result.succeeded() { + println!("Skipping test: Cannot set SELinux context, system may not support this context"); + return; + } + + let actual_context = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); + + at.remove(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); + + ts.ucmd() + .arg("-Z") + .arg(format!("--context={}", actual_context)) + .arg(TEST_HELLO_WORLD_SOURCE) + .arg(TEST_HELLO_WORLD_DEST) + .arg("--preserve=all") + .succeeds(); + + assert!(at.file_exists(TEST_HELLO_WORLD_DEST)); + + let selinux_perm_dest = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); + let selinux_perm_src = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_SOURCE)); + + // Verify that the SELinux contexts match, whatever they may be + assert_eq!(selinux_perm_src, selinux_perm_dest); + + #[cfg(all(unix, not(target_os = "freebsd")))] + { + let metadata_src = at.metadata(TEST_HELLO_WORLD_SOURCE); + let metadata_dst = at.metadata(TEST_HELLO_WORLD_DEST); + assert_metadata_eq!(metadata_src, metadata_dst); + } + + at.remove(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); +} From 74e72f527b5953afbaf4a95c7a62f9cfede1d353 Mon Sep 17 00:00:00 2001 From: SLASHLogin Date: Fri, 2 May 2025 08:53:03 +0200 Subject: [PATCH 2/4] cp: add -Z flag & add --context=[CTX] flag --- src/uu/cp/src/copydir.rs | 4 ++- src/uu/cp/src/cp.rs | 65 +++++++++++++++++++++++++++++----------- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index d2e367c5c..3137a2053 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -436,6 +436,7 @@ pub(crate) fn copy_directory( &entry.source_absolute, &entry.local_to_target, &options.attributes, + options, )?; } } @@ -466,6 +467,7 @@ pub(crate) fn copy_directory( &entry.source_absolute, &entry.local_to_target, &options.attributes, + options, )?; } } @@ -476,7 +478,7 @@ pub(crate) fn copy_directory( let dest = target.join(root.file_name().unwrap()); for (x, y) in aligned_ancestors(root, dest.as_path()) { if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) { - copy_attributes(&src, y, &options.attributes)?; + copy_attributes(&src, y, &options.attributes, options)?; } } } diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 49841d741..7bfd29de0 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1117,7 +1117,7 @@ impl Options { } } - #[cfg(not(feature = "feat_selinux"))] + #[cfg(not(feature = "selinux"))] if let Preserve::Yes { required } = attributes.context { let selinux_disabled_error = Error::Error("SELinux was not enabled during the compile time!".to_string()); @@ -1492,7 +1492,7 @@ fn copy_source( if options.parents { for (x, y) in aligned_ancestors(source, dest.as_path()) { if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) { - copy_attributes(&src, y, &options.attributes)?; + copy_attributes(&src, y, &options.attributes, options)?; } } } @@ -1640,10 +1640,12 @@ fn copy_extended_attrs(source: &Path, dest: &Path) -> CopyResult<()> { } /// Copy the specified attributes from one path to another. +#[allow(unused_variables)] pub(crate) fn copy_attributes( source: &Path, dest: &Path, attributes: &Attributes, + options: &Options, ) -> CopyResult<()> { let context = &*format!("{} -> {}", source.quote(), dest.quote()); let source_metadata = fs::symlink_metadata(source).context(context)?; @@ -1704,21 +1706,48 @@ pub(crate) fn copy_attributes( })?; #[cfg(feature = "feat_selinux")] - handle_preserve(&attributes.context, || -> CopyResult<()> { - let context = selinux::SecurityContext::of_path(source, false, false).map_err(|e| { - format!( - "failed to get security context of {}: {e}", - source.display(), - ) - })?; - if let Some(context) = context { - context.set_for_path(dest, false, false).map_err(|e| { - format!("failed to set security context for {}: {e}", dest.display(),) + { + if options.set_selinux_context { + // -Z flag takes precedence over --context and --preserve=context + uucore::selinux::set_selinux_security_context(dest, None).map_err(|e| { + Error::Error(format!("failed to set SELinux security context: {}", e)) + })?; + } else if let Some(ctx) = &options.context { + // --context option takes precedence over --preserve=context + if ctx.is_empty() { + // --context without a value is equivalent to -Z + uucore::selinux::set_selinux_security_context(dest, None).map_err(|e| { + Error::Error(format!("failed to set SELinux security context: {}", e)) + })?; + } else { + // --context=CTX sets the specified context + uucore::selinux::set_selinux_security_context(dest, Some(ctx)).map_err(|e| { + Error::Error(format!( + "failed to set SELinux security context to {}: {}", + ctx, e + )) + })?; + } + } else { + // Existing context preservation code + handle_preserve(&attributes.context, || -> CopyResult<()> { + let context = + selinux::SecurityContext::of_path(source, false, false).map_err(|e| { + format!( + "failed to get security context of {}: {e}", + source.display(), + ) + })?; + if let Some(context) = context { + context.set_for_path(dest, false, false).map_err(|e| { + format!("failed to set security context for {}: {e}", dest.display(),) + })?; + } + + Ok(()) })?; } - - Ok(()) - })?; + } handle_preserve(&attributes.xattr, || -> CopyResult<()> { #[cfg(all(unix, not(target_os = "android")))] @@ -2436,7 +2465,7 @@ fn copy_file( if options.dereference(source_in_command_line) { if let Ok(src) = canonicalize(source, MissingHandling::Normal, ResolveMode::Physical) { if src.exists() { - copy_attributes(&src, dest, &options.attributes)?; + copy_attributes(&src, dest, &options.attributes, options)?; } } } else if source_is_stream && source.exists() { @@ -2444,9 +2473,9 @@ fn copy_file( // like anonymous pipes. Thus, we can't really copy its // attributes. However, this is already handled in the stream // copy function (see `copy_stream` under platform/linux.rs). - copy_attributes(source, dest, &options.attributes)?; + copy_attributes(source, dest, &options.attributes, options)?; } else { - copy_attributes(source, dest, &options.attributes)?; + copy_attributes(source, dest, &options.attributes, options)?; } #[cfg(feature = "selinux")] From b3a2b74ca1906efff91050d2e344d7a663b50945 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 2 May 2025 09:24:19 +0200 Subject: [PATCH 3/4] cp/selinx: improve the support of --preserve-context and simplify the code. + Add test for the selinux changes with context SLASHLogin Improves the coverage of tests/cp/cp-a-selinux.sh --- src/uu/cp/src/cp.rs | 57 +++----- tests/by-util/test_cp.rs | 301 +++++++++++++++++++++++++++++++++++---- 2 files changed, 293 insertions(+), 65 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 7bfd29de0..fcdeca7f8 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1705,49 +1705,26 @@ pub(crate) fn copy_attributes( Ok(()) })?; - #[cfg(feature = "feat_selinux")] - { - if options.set_selinux_context { - // -Z flag takes precedence over --context and --preserve=context - uucore::selinux::set_selinux_security_context(dest, None).map_err(|e| { - Error::Error(format!("failed to set SELinux security context: {}", e)) - })?; - } else if let Some(ctx) = &options.context { - // --context option takes precedence over --preserve=context - if ctx.is_empty() { - // --context without a value is equivalent to -Z - uucore::selinux::set_selinux_security_context(dest, None).map_err(|e| { - Error::Error(format!("failed to set SELinux security context: {}", e)) - })?; - } else { - // --context=CTX sets the specified context - uucore::selinux::set_selinux_security_context(dest, Some(ctx)).map_err(|e| { - Error::Error(format!( - "failed to set SELinux security context to {}: {}", - ctx, e - )) - })?; + #[cfg(feature = "selinux")] + handle_preserve(&attributes.context, || -> CopyResult<()> { + // Get the source context and apply it to the destination + if let Ok(context) = selinux::SecurityContext::of_path(source, false, false) { + if let Some(context) = context { + if let Err(e) = context.set_for_path(dest, false, false) { + return Err(Error::Error(format!( + "failed to set security context for {}: {e}", + dest.display() + ))); + } } } else { - // Existing context preservation code - handle_preserve(&attributes.context, || -> CopyResult<()> { - let context = - selinux::SecurityContext::of_path(source, false, false).map_err(|e| { - format!( - "failed to get security context of {}: {e}", - source.display(), - ) - })?; - if let Some(context) = context { - context.set_for_path(dest, false, false).map_err(|e| { - format!("failed to set security context for {}: {e}", dest.display(),) - })?; - } - - Ok(()) - })?; + return Err(Error::Error(format!( + "failed to get security context of {}", + source.display() + ))); } - } + Ok(()) + })?; handle_preserve(&attributes.xattr, || -> CopyResult<()> { #[cfg(all(unix, not(target_os = "android")))] diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 19d0d5a77..515d7dab4 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.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 (flags) reflink (fs) tmpfs (linux) rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR procfs outfile uufs xattrs -// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined +// spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel use uutests::at_and_ucmd; use uutests::new_ucmd; use uutests::path_concat; @@ -6369,49 +6369,300 @@ fn test_cp_preserve_selinux() { fn test_cp_preserve_selinux_admin_context() { let ts = TestScenario::new(util_name!()); let at = &ts.fixtures; - let admin_context = "system_u:object_r:admin_home_t:s0"; at.touch(TEST_HELLO_WORLD_SOURCE); + // Get the default SELinux context for the destination file path + // on Debian/Ubuntu, this program is provided by the selinux-utils package + // on Fedora/RHEL, this program is provided by the libselinux-devel package + let output = std::process::Command::new("matchpathcon") + .arg(at.plus_as_string(TEST_HELLO_WORLD_DEST)) + .output() + .expect("failed to execute matchpathcon command"); + + assert!( + output.status.success(), + "matchpathcon command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let output_str = String::from_utf8_lossy(&output.stdout); + let default_context = output_str + .split_whitespace() + .nth(1) + .unwrap_or_default() + .to_string(); + + assert!( + !default_context.is_empty(), + "Unable to determine default SELinux context for the test file" + ); + let cmd_result = ts .ucmd() .arg("-Z") - .arg(format!("--context={admin_context}")) + .arg(format!("--context={}", default_context)) .arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_HELLO_WORLD_DEST) .run(); + println!("cp command result: {:?}", cmd_result); + if !cmd_result.succeeded() { println!("Skipping test: Cannot set SELinux context, system may not support this context"); return; } - let actual_context = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); - - at.remove(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); - - ts.ucmd() - .arg("-Z") - .arg(format!("--context={}", actual_context)) - .arg(TEST_HELLO_WORLD_SOURCE) - .arg(TEST_HELLO_WORLD_DEST) - .arg("--preserve=all") - .succeeds(); - assert!(at.file_exists(TEST_HELLO_WORLD_DEST)); let selinux_perm_dest = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); - let selinux_perm_src = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_SOURCE)); + println!("Destination SELinux context: {}", selinux_perm_dest); - // Verify that the SELinux contexts match, whatever they may be - assert_eq!(selinux_perm_src, selinux_perm_dest); - - #[cfg(all(unix, not(target_os = "freebsd")))] - { - let metadata_src = at.metadata(TEST_HELLO_WORLD_SOURCE); - let metadata_dst = at.metadata(TEST_HELLO_WORLD_DEST); - assert_metadata_eq!(metadata_src, metadata_dst); - } + assert_eq!(default_context, selinux_perm_dest); at.remove(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); } + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_cp_selinux_context_priority() { + // This test verifies that the priority order is respected: + // -Z > --context > --preserve=context + + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + // Create two different files + at.write(TEST_HELLO_WORLD_SOURCE, "source content"); + + // First, set a known context on source file (only if system supports it) + let setup_result = ts + .ucmd() + .arg("--context=unconfined_u:object_r:user_tmp_t:s0") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg("initial_context.txt") + .run(); + + // If the system doesn't support setting contexts, skip the test + if !setup_result.succeeded() { + println!("Skipping test: System doesn't support setting SELinux contexts"); + return; + } + + // Create different copies with different context options + + // 1. Using --preserve=context + ts.ucmd() + .arg("--preserve=context") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg("preserve.txt") + .succeeds(); + + // 2. Using --context with a different context (we already know this works from setup) + ts.ucmd() + .arg("--context=unconfined_u:object_r:user_tmp_t:s0") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg("context.txt") + .succeeds(); + + // 3. Using -Z (should use default type context) + ts.ucmd() + .arg("-Z") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg("z_flag.txt") + .succeeds(); + + // 4. Using both -Z and --context (Z should win) + ts.ucmd() + .arg("-Z") + .arg("--context=unconfined_u:object_r:user_tmp_t:s0") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg("z_and_context.txt") + .succeeds(); + + // 5. Using both -Z and --preserve=context (Z should win) + ts.ucmd() + .arg("-Z") + .arg("--preserve=context") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg("z_and_preserve.txt") + .succeeds(); + + // Get all the contexts + let source_ctx = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_SOURCE)); + let preserve_ctx = get_getfattr_output(&at.plus_as_string("preserve.txt")); + let context_ctx = get_getfattr_output(&at.plus_as_string("context.txt")); + let z_ctx = get_getfattr_output(&at.plus_as_string("z_flag.txt")); + let z_and_context_ctx = get_getfattr_output(&at.plus_as_string("z_and_context.txt")); + let z_and_preserve_ctx = get_getfattr_output(&at.plus_as_string("z_and_preserve.txt")); + + if source_ctx.is_empty() { + println!("Skipping test assertions: Failed to get SELinux contexts"); + return; + } + assert_eq!( + source_ctx, preserve_ctx, + "--preserve=context should match the source context" + ); + assert_eq!( + source_ctx, context_ctx, + "--preserve=context should match the source context" + ); + assert_eq!( + z_ctx, z_and_context_ctx, + "-Z context should be the same regardless of --context" + ); + assert_eq!( + z_ctx, z_and_preserve_ctx, + "-Z context should be the same regardless of --preserve=context" + ); +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_cp_selinux_empty_context() { + // This test verifies that --context without a value works like -Z + + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + at.write(TEST_HELLO_WORLD_SOURCE, "test content"); + + // Try creating copies - if this fails, the system doesn't support SELinux properly + let z_result = ts + .ucmd() + .arg("-Z") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg("z_flag.txt") + .run(); + + if !z_result.succeeded() { + println!("Skipping test: SELinux contexts not supported"); + return; + } + + // Now try with --context (no value) + let context_result = ts + .ucmd() + .arg("--context") + .arg(TEST_HELLO_WORLD_SOURCE) + .arg("empty_context.txt") + .run(); + + if !context_result.succeeded() { + println!("Skipping test: Empty context parameter not supported"); + return; + } + + let z_ctx = get_getfattr_output(&at.plus_as_string("z_flag.txt")); + let empty_ctx = get_getfattr_output(&at.plus_as_string("empty_context.txt")); + + if !z_ctx.is_empty() && !empty_ctx.is_empty() { + assert_eq!( + z_ctx, empty_ctx, + "--context without a value should behave like -Z" + ); + } +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_cp_selinux_recursive() { + // Test SELinux context preservation in recursive directory copies + + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("source_dir"); + at.write("source_dir/file1.txt", "file1 content"); + at.mkdir("source_dir/subdir"); + at.write("source_dir/subdir/file2.txt", "file2 content"); + + let setup_result = ts + .ucmd() + .arg("--context=unconfined_u:object_r:user_tmp_t:s0") + .arg("source_dir/file1.txt") + .arg("source_dir/context_set.txt") + .run(); + + if !setup_result.succeeded() { + println!("Skipping test: System doesn't support setting SELinux contexts"); + return; + } + + ts.ucmd() + .arg("-rZ") + .arg("source_dir") + .arg("dest_dir_z") + .succeeds(); + + ts.ucmd() + .arg("-r") + .arg("--preserve=context") + .arg("source_dir") + .arg("dest_dir_preserve") + .succeeds(); + + let z_dir_ctx = get_getfattr_output(&at.plus_as_string("dest_dir_z")); + let preserve_dir_ctx = get_getfattr_output(&at.plus_as_string("dest_dir_preserve")); + + if !z_dir_ctx.is_empty() && !preserve_dir_ctx.is_empty() { + assert!( + z_dir_ctx.contains("_u:"), + "SELinux contexts not properly set with -Z flag" + ); + + assert!( + preserve_dir_ctx.contains("_u:"), + "SELinux contexts not properly preserved with --preserve=context" + ); + } +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_cp_preserve_context_root() { + use uutests::util::run_ucmd_as_root; + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let source_file = "c"; + let dest_file = "e"; + at.touch(source_file); + + let context = "root:object_r:tmp_t:s0"; + + let chcon_result = std::process::Command::new("chcon") + .arg(context) + .arg(at.plus_as_string(source_file)) + .status(); + + if !chcon_result.is_ok_and(|status| status.success()) { + println!("Skipping test: Failed to set context: {}", context); + return; + } + + // Copy the file with preserved context + // Only works at root + if let Ok(result) = run_ucmd_as_root(&scene, &["--preserve=context", source_file, dest_file]) { + let src_ctx = get_getfattr_output(&at.plus_as_string(source_file)); + let dest_ctx = get_getfattr_output(&at.plus_as_string(dest_file)); + println!("Source context: {}", src_ctx); + println!("Destination context: {}", dest_ctx); + + if !result.succeeded() { + println!("Skipping test: Failed to copy with preserved context"); + return; + } + + let dest_context = get_getfattr_output(&at.plus_as_string(dest_file)); + + assert!( + dest_context.contains("root:object_r:tmp_t"), + "Expected context '{}' not found in destination context: '{}'", + context, + dest_context + ); + } else { + print!("Test skipped; requires root user"); + } +} From 5148ba12d6b7b4237c81bf0eebf3249db5929c75 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 26 Apr 2025 15:21:12 +0200 Subject: [PATCH 4/4] set_selinux_security_context: also display the error from the crate + fix comments from review --- src/uu/cp/src/copydir.rs | 4 +--- src/uu/cp/src/cp.rs | 15 ++++++++------- tests/by-util/test_cp.rs | 15 +++++---------- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index 3137a2053..d2e367c5c 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -436,7 +436,6 @@ pub(crate) fn copy_directory( &entry.source_absolute, &entry.local_to_target, &options.attributes, - options, )?; } } @@ -467,7 +466,6 @@ pub(crate) fn copy_directory( &entry.source_absolute, &entry.local_to_target, &options.attributes, - options, )?; } } @@ -478,7 +476,7 @@ pub(crate) fn copy_directory( let dest = target.join(root.file_name().unwrap()); for (x, y) in aligned_ancestors(root, dest.as_path()) { if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) { - copy_attributes(&src, y, &options.attributes, options)?; + copy_attributes(&src, y, &options.attributes)?; } } } diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index fcdeca7f8..200a903aa 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1492,7 +1492,7 @@ fn copy_source( if options.parents { for (x, y) in aligned_ancestors(source, dest.as_path()) { if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) { - copy_attributes(&src, y, &options.attributes, options)?; + copy_attributes(&src, y, &options.attributes)?; } } } @@ -1640,12 +1640,10 @@ fn copy_extended_attrs(source: &Path, dest: &Path) -> CopyResult<()> { } /// Copy the specified attributes from one path to another. -#[allow(unused_variables)] pub(crate) fn copy_attributes( source: &Path, dest: &Path, attributes: &Attributes, - options: &Options, ) -> CopyResult<()> { let context = &*format!("{} -> {}", source.quote(), dest.quote()); let source_metadata = fs::symlink_metadata(source).context(context)?; @@ -2442,7 +2440,7 @@ fn copy_file( if options.dereference(source_in_command_line) { if let Ok(src) = canonicalize(source, MissingHandling::Normal, ResolveMode::Physical) { if src.exists() { - copy_attributes(&src, dest, &options.attributes, options)?; + copy_attributes(&src, dest, &options.attributes)?; } } } else if source_is_stream && source.exists() { @@ -2450,15 +2448,18 @@ fn copy_file( // like anonymous pipes. Thus, we can't really copy its // attributes. However, this is already handled in the stream // copy function (see `copy_stream` under platform/linux.rs). - copy_attributes(source, dest, &options.attributes, options)?; } else { - copy_attributes(source, dest, &options.attributes, options)?; + copy_attributes(source, dest, &options.attributes)?; } #[cfg(feature = "selinux")] if options.set_selinux_context && uucore::selinux::is_selinux_enabled() { // Set the given selinux permissions on the copied file. - uucore::selinux::set_selinux_security_context(dest, options.context.as_ref())?; + if let Err(e) = + uucore::selinux::set_selinux_security_context(dest, options.context.as_ref()) + { + return Err(Error::Error(format!("SELinux error: {}", e))); + } } copied_files.insert( diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 515d7dab4..9fc2b05a7 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -6292,9 +6292,7 @@ fn test_cp_selinux() { assert!( selinux_perm.contains("unconfined_u"), - "Expected '{}' not found in getfattr output:\n{}", - "foo", - selinux_perm + "Expected 'foo' not found in getfattr output:\n{selinux_perm}" ); at.remove(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); } @@ -6342,9 +6340,7 @@ fn test_cp_preserve_selinux() { let selinux_perm_dest = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); assert!( selinux_perm_dest.contains("unconfined_u"), - "Expected '{}' not found in getfattr output:\n{}", - "foo", - selinux_perm_dest + "Expected 'foo' not found in getfattr output:\n{selinux_perm_dest}" ); assert_eq!( get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_SOURCE)), @@ -6373,8 +6369,8 @@ fn test_cp_preserve_selinux_admin_context() { at.touch(TEST_HELLO_WORLD_SOURCE); // Get the default SELinux context for the destination file path - // on Debian/Ubuntu, this program is provided by the selinux-utils package - // on Fedora/RHEL, this program is provided by the libselinux-devel package + // On Debian/Ubuntu, this program is provided by the selinux-utils package + // On Fedora/RHEL, this program is provided by the libselinux-devel package let output = std::process::Command::new("matchpathcon") .arg(at.plus_as_string(TEST_HELLO_WORLD_DEST)) .output() @@ -6432,7 +6428,6 @@ fn test_cp_selinux_context_priority() { let ts = TestScenario::new(util_name!()); let at = &ts.fixtures; - // Create two different files at.write(TEST_HELLO_WORLD_SOURCE, "source content"); // First, set a known context on source file (only if system supports it) @@ -6642,7 +6637,7 @@ fn test_cp_preserve_context_root() { } // Copy the file with preserved context - // Only works at root + // Only works as root if let Ok(result) = run_ucmd_as_root(&scene, &["--preserve=context", source_file, dest_file]) { let src_ctx = get_getfattr_output(&at.plus_as_string(source_file)); let dest_ctx = get_getfattr_output(&at.plus_as_string(dest_file));