From fe3645d4d5c34ef830ef04edc5f5fe50edcdc472 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Fri, 10 Sep 2021 21:33:34 +0200 Subject: [PATCH 1/5] ls: add support for showing SELinux context (--context/-Z) --- Cargo.lock | 1 + Cargo.toml | 2 +- src/uu/ls/Cargo.toml | 4 ++ src/uu/ls/src/ls.rs | 164 +++++++++++++++++++++++++++++++++++++------ 4 files changed, 149 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 704d1eea1..ea3467b5a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2632,6 +2632,7 @@ dependencies = [ "lscolors", "number_prefix", "once_cell", + "selinux", "term_grid", "termsize", "unicode-width", diff --git a/Cargo.toml b/Cargo.toml index 3a2c5f12a..8d2f2da58 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -147,7 +147,7 @@ feat_os_unix_musl = [ # NOTE: # The selinux(-sys) crate requires `libselinux` headers and shared library to be accessible in the C toolchain at compile time. # Running a uutils compiled with `feat_selinux` requires an SELinux enabled Kernel at run time. -feat_selinux = ["cp/selinux", "id/selinux", "selinux", "feat_require_selinux"] +feat_selinux = ["cp/selinux", "id/selinux", "ls/selinux", "selinux", "feat_require_selinux"] # "feat_acl" == set of utilities providing support for acl (access control lists) if enabled with `--features feat_acl`. # NOTE: # On linux, the posix-acl/acl-sys crate requires `libacl` headers and shared library to be accessible in the C toolchain at compile time. diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml index dbe6bacaa..62dcd59a4 100644 --- a/src/uu/ls/Cargo.toml +++ b/src/uu/ls/Cargo.toml @@ -28,6 +28,7 @@ uucore = { version = ">=0.0.8", package = "uucore", path = "../../uucore", featu uucore_procs = { version=">=0.0.6", package = "uucore_procs", path = "../../uucore_procs" } once_cell = "1.7.2" atty = "0.2" +selinux = { version="0.2.1", optional = true } [target.'cfg(unix)'.dependencies] lazy_static = "1.4.0" @@ -35,3 +36,6 @@ lazy_static = "1.4.0" [[bin]] name = "ls" path = "src/main.rs" + +[features] +feat_selinux = ["selinux"] diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 6f63c2a4a..7b7fd873e 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -47,6 +47,11 @@ use unicode_width::UnicodeWidthStr; use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR}; use uucore::{fs::display_permissions, version_cmp::version_cmp}; +#[cfg(not(feature = "selinux"))] +static CONTEXT_HELP_TEXT: &str = "print any security context of each file (not enabled)"; +#[cfg(feature = "selinux")] +static CONTEXT_HELP_TEXT: &str = "print any security context of each file"; + fn usage() -> String { format!("{0} [OPTION]... [FILE]...", uucore::execution_phrase()) } @@ -126,6 +131,7 @@ pub mod options { pub static FULL_TIME: &str = "full-time"; pub static HIDE: &str = "hide"; pub static IGNORE: &str = "ignore"; + pub static CONTEXT: &str = "context"; } const DEFAULT_TERM_WIDTH: u16 = 80; @@ -236,6 +242,8 @@ struct Config { quoting_style: QuotingStyle, indicator_style: IndicatorStyle, time_style: TimeStyle, + context: bool, + selinux_supported: bool, } // Fields that can be removed or added to the long format @@ -247,9 +255,18 @@ struct LongFormat { numeric_uid_gid: bool, } +struct PaddingCollection { + longest_link_count_len: usize, + longest_uname_len: usize, + longest_group_len: usize, + longest_context_len: usize, + longest_size_len: usize, +} + impl Config { #[allow(clippy::cognitive_complexity)] fn from(options: &clap::ArgMatches) -> UResult { + let context = options.is_present(options::CONTEXT); let (mut format, opt) = if let Some(format_) = options.value_of(options::FORMAT) { ( match format_ { @@ -593,6 +610,17 @@ impl Config { quoting_style, indicator_style, time_style, + context, + selinux_supported: { + #[cfg(feature = "selinux")] + { + selinux::kernel_support() != selinux::KernelSupport::Unsupported + } + #[cfg(not(feature = "selinux"))] + { + false + } + }, }) } } @@ -611,6 +639,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(|v| v.map(Path::new).collect()) .unwrap_or_else(|| vec![Path::new(".")]); + if config.context && !config.selinux_supported { + show_warning!("--context (-Z) works only on an SELinux-enabled kernel",); + } + list(locs, config) } @@ -1154,6 +1186,12 @@ only ignore '.' and '..'.", .overrides_with(options::FULL_TIME) .help("like -l --time-style=full-iso"), ) + .arg( + Arg::with_name(options::CONTEXT) + .short("Z") + .long(options::CONTEXT) + .help(CONTEXT_HELP_TEXT), + ) // Positional arguments .arg( Arg::with_name(options::PATHS) @@ -1178,6 +1216,7 @@ struct PathData { // PathBuf that all above data corresponds to p_buf: PathBuf, must_dereference: bool, + security_context: String, } impl PathData { @@ -1221,12 +1260,39 @@ impl PathData { None => OnceCell::new(), }; + let security_context: String = if config.context { + if config.selinux_supported { + #[cfg(feature = "selinux")] + { + if let Ok(Some(context)) = + selinux::SecurityContext::of_path(&p_buf, must_dereference, false) + { + String::from_utf8_lossy(context.as_bytes()) + .trim_end_matches(char::from(0)) + .to_string() + } else { + // TODO: print warning with error to stderr + "?".to_string() + } + } + #[cfg(not(feature = "selinux"))] + { + "?".to_string() + } + } else { + "?".to_string() + } + } else { + String::new() + }; + Self { md: OnceCell::new(), ft, display_name, p_buf, must_dereference, + security_context, } } @@ -1390,7 +1456,7 @@ fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { } fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize, usize, usize) { - // TODO: Cache/memoize the display_* results so we don't have to recalculate them. + // TODO: Cache/memorize the display_* results so we don't have to recalculate them. if let Some(md) = entry.md() { ( display_symlink_count(md).len(), @@ -1403,31 +1469,40 @@ fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize, u } } -fn pad_left(string: String, count: usize) -> String { +fn pad_left(string: &str, count: usize) -> String { format!("{:>width$}", string, width = count) } -fn pad_right(string: String, count: usize) -> String { +fn pad_right(string: &str, count: usize) -> String { format!("{:) { + // `-Z`, `--context`: + // Display the SELinux security context or '?' if none is found. When used with the `-l` + // option, print the security context to the left of the size column. + if config.format == Format::Long { let ( mut longest_link_count_len, mut longest_uname_len, mut longest_group_len, + mut longest_context_len, mut longest_size_len, - ) = (1, 1, 1, 1); + ) = (1, 1, 1, 1, 1); let mut total_size = 0; for item in items { + let context_len = item.security_context.len(); let (link_count_len, uname_len, group_len, size_len) = display_dir_entry_size(item, config); longest_link_count_len = link_count_len.max(longest_link_count_len); longest_size_len = size_len.max(longest_size_len); longest_uname_len = uname_len.max(longest_uname_len); longest_group_len = group_len.max(longest_group_len); + if config.context { + longest_context_len = context_len.max(longest_context_len); + } longest_size_len = size_len.max(longest_size_len); total_size += item.md().map_or(0, |md| get_block_size(md, config)); } @@ -1439,16 +1514,31 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter display_grid(names, config.width, Direction::TopToBottom, out), @@ -1573,15 +1663,13 @@ fn display_grid( /// longest_link_count_len: usize, /// longest_uname_len: usize, /// longest_group_len: usize, +/// longest_context_len: usize, /// longest_size_len: usize, /// ``` /// that decide the maximum possible character count of each field. fn display_item_long( item: &PathData, - longest_link_count_len: usize, - longest_uname_len: usize, - longest_group_len: usize, - longest_size_len: usize, + padding: PaddingCollection, config: &Config, out: &mut BufWriter, ) { @@ -1602,16 +1690,23 @@ fn display_item_long( let _ = write!( out, - "{} {}", + "{}{} {}", display_permissions(md, true), - pad_left(display_symlink_count(md), longest_link_count_len), + if item.security_context != "?" { + // GNU `ls` uses a "." character to indicate a file with a security context, but not + // other alternate access method. + "." + } else { + "" + }, + pad_left(&display_symlink_count(md), padding.longest_link_count_len), ); if config.long.owner { let _ = write!( out, " {}", - pad_right(display_uname(md, config), longest_uname_len) + pad_right(&display_uname(md, config), padding.longest_uname_len) ); } @@ -1619,7 +1714,15 @@ fn display_item_long( let _ = write!( out, " {}", - pad_right(display_group(md, config), longest_group_len) + pad_right(&display_group(md, config), padding.longest_group_len) + ); + } + + if config.context { + let _ = write!( + out, + " {}", + pad_right(&item.security_context, padding.longest_context_len) ); } @@ -1629,19 +1732,19 @@ fn display_item_long( let _ = write!( out, " {}", - pad_right(display_uname(md, config), longest_uname_len) + pad_right(&display_uname(md, config), padding.longest_uname_len) ); } let _ = writeln!( out, " {} {} {}", - pad_left(display_size_or_rdev(md, config), longest_size_len), + pad_left(&display_size_or_rdev(md, config), padding.longest_size_len), display_date(md, config), // unwrap is fine because it fails when metadata is not available // but we already know that it is because it's checked at the // start of the function. - display_file_name(item, config).unwrap().contents, + display_file_name(item, config, None).unwrap().contents, ); } @@ -1865,10 +1968,15 @@ fn classify_file(path: &PathData) -> Option { /// * `config.indicator_style` to append specific characters to `name` using [`classify_file`]. /// * `config.format` to display symlink targets if `Format::Long`. This function is also /// responsible for coloring symlink target names if `config.color` is specified. +/// * `config.context` to prepend security context to `name` if compiled with `feat_selinux`. /// /// Note that non-unicode sequences in symlink targets are dealt with using /// [`std::path::Path::to_string_lossy`]. -fn display_file_name(path: &PathData, config: &Config) -> Option { +fn display_file_name( + path: &PathData, + config: &Config, + prefix_context: Option, +) -> Option { // This is our return value. We start by `&path.display_name` and modify it along the way. let mut name = escape_name(&path.display_name, &config.quoting_style); @@ -1960,6 +2068,20 @@ fn display_file_name(path: &PathData, config: &Config) -> Option { } } + // Prepend the security context to the `name` and adjust `width` in order + // to get correct alignment from later calls to`display_grid()`. + if config.context { + if let Some(pad_count) = prefix_context { + let security_context = if !matches!(config.format, Format::Commas) { + pad_left(&path.security_context, pad_count) + } else { + path.security_context.to_owned() + }; + name = format!("{} {}", security_context, name); + width += security_context.len() + 1; + } + } + Some(Cell { contents: name, width, From e018a45b54b96e1e3f8dd323b411e0bc6ae84673 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Fri, 10 Sep 2021 21:35:00 +0200 Subject: [PATCH 2/5] test_ls: add/adjust tests for SELinux context --- src/uu/ls/src/ls.rs | 12 ++-- tests/by-util/test_ls.rs | 127 +++++++++++++++++++++++++++------------ 2 files changed, 94 insertions(+), 45 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 7b7fd873e..5c26c8e26 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -639,10 +639,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(|v| v.map(Path::new).collect()) .unwrap_or_else(|| vec![Path::new(".")]); - if config.context && !config.selinux_supported { - show_warning!("--context (-Z) works only on an SELinux-enabled kernel",); - } - list(locs, config) } @@ -1260,7 +1256,7 @@ impl PathData { None => OnceCell::new(), }; - let security_context: String = if config.context { + let security_context = if config.context { if config.selinux_supported { #[cfg(feature = "selinux")] { @@ -1692,9 +1688,9 @@ fn display_item_long( out, "{}{} {}", display_permissions(md, true), - if item.security_context != "?" { - // GNU `ls` uses a "." character to indicate a file with a security context, but not - // other alternate access method. + if item.security_context.len() > 1 { + // GNU `ls` uses a "." character to indicate a file with a security context, + // but not other alternate access method. "." } else { "" diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 3d6092416..f14fc424a 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -347,6 +347,7 @@ fn test_ls_long_format() { // A line of the output should be: // One of the characters -bcCdDlMnpPsStTx? // rwx, with - for missing permissions, thrice. + // Zero or one "." for indicating a file with security context // A number, preceded by column whitespace, and followed by a single space. // A username, currently [^ ], followed by column whitespace, twice (or thrice for Hurd). // A number, followed by a single space. @@ -356,13 +357,13 @@ fn test_ls_long_format() { // and followed by a single space. // Whatever comes after is irrelevant to this specific test. scene.ucmd().arg(arg).arg("test-long-dir").succeeds().stdout_matches(&Regex::new( - r"\n[-bcCdDlMnpPsStTx?]([r-][w-][xt-]){3} +\d+ [^ ]+ +[^ ]+( +[^ ]+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ " + r"\n[-bcCdDlMnpPsStTx?]([r-][w-][xt-]){3}\.? +\d+ [^ ]+ +[^ ]+( +[^ ]+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ " ).unwrap()); } // This checks for the line with the .. entry. The uname and group should be digits. scene.ucmd().arg("-lan").arg("test-long-dir").succeeds().stdout_matches(&Regex::new( - r"\nd([r-][w-][xt-]){3} +\d+ \d+ +\d+( +\d+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ \.\." + r"\nd([r-][w-][xt-]){3}\.? +\d+ \d+ +\d+( +\d+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ \.\." ).unwrap()); } @@ -370,6 +371,7 @@ fn test_ls_long_format() { /// This test is mainly about coloring, but, the recursion, symlink `->` processing, /// and `.` and `..` being present in `-a` all need to work for the test to pass. /// This test does not really test anything provided by `-l` but the file names and symlinks. +#[cfg(all(feature = "ln", feature = "mkdir", feature = "touch"))] #[test] fn test_ls_long_symlink_color() { // If you break this test after breaking mkdir, touch, or ln, do not be alarmed! @@ -639,55 +641,57 @@ fn test_ls_long_formats() { let at = &scene.fixtures; at.touch(&at.plus_as_string("test-long-formats")); + // Zero or one "." for indicating a file with security context + // Regex for three names, so all of author, group and owner - let re_three = Regex::new(r"[xrw-]{9} \d ([-0-9_a-z]+ ){3}0").unwrap(); + let re_three = Regex::new(r"[xrw-]{9}\.? \d ([-0-9_a-z]+ ){3}0").unwrap(); #[cfg(unix)] - let re_three_num = Regex::new(r"[xrw-]{9} \d (\d+ ){3}0").unwrap(); + let re_three_num = Regex::new(r"[xrw-]{9}\.? \d (\d+ ){3}0").unwrap(); // Regex for two names, either: // - group and owner // - author and owner // - author and group - let re_two = Regex::new(r"[xrw-]{9} \d ([-0-9_a-z]+ ){2}0").unwrap(); + let re_two = Regex::new(r"[xrw-]{9}\.? \d ([-0-9_a-z]+ ){2}0").unwrap(); #[cfg(unix)] - let re_two_num = Regex::new(r"[xrw-]{9} \d (\d+ ){2}0").unwrap(); + let re_two_num = Regex::new(r"[xrw-]{9}\.? \d (\d+ ){2}0").unwrap(); // Regex for one name: author, group or owner - let re_one = Regex::new(r"[xrw-]{9} \d [-0-9_a-z]+ 0").unwrap(); + let re_one = Regex::new(r"[xrw-]{9}\.? \d [-0-9_a-z]+ 0").unwrap(); #[cfg(unix)] - let re_one_num = Regex::new(r"[xrw-]{9} \d \d+ 0").unwrap(); + let re_one_num = Regex::new(r"[xrw-]{9}\.? \d \d+ 0").unwrap(); // Regex for no names - let re_zero = Regex::new(r"[xrw-]{9} \d 0").unwrap(); + let re_zero = Regex::new(r"[xrw-]{9}\.? \d 0").unwrap(); - let result = scene + scene .ucmd() .arg("-l") .arg("--author") .arg("test-long-formats") - .succeeds(); - assert!(re_three.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_three); - let result = scene + scene .ucmd() .arg("-l1") .arg("--author") .arg("test-long-formats") - .succeeds(); - assert!(re_three.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_three); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .arg("--author") .arg("test-long-formats") - .succeeds(); - assert!(re_three_num.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_three_num); } for arg in &[ @@ -697,22 +701,22 @@ fn test_ls_long_formats() { "-lG --author", // only author and owner "-l --no-group --author", // only author and owner ] { - let result = scene + scene .ucmd() .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_two.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_two); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_two_num.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_two_num); } } @@ -726,22 +730,22 @@ fn test_ls_long_formats() { "-l --no-group", // only owner "-gG --author", // only author ] { - let result = scene + scene .ucmd() .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_one.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_one); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_one_num.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_one_num); } } @@ -758,22 +762,22 @@ fn test_ls_long_formats() { "-og1", "-og1l", ] { - let result = scene + scene .ucmd() .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_zero.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_zero); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_zero.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_zero); } } } @@ -1251,7 +1255,7 @@ fn test_ls_inode() { at.touch(file); let re_short = Regex::new(r" *(\d+) test_inode").unwrap(); - let re_long = Regex::new(r" *(\d+) [xrw-]{10} \d .+ test_inode").unwrap(); + let re_long = Regex::new(r" *(\d+) [xrw-]{10}\.? \d .+ test_inode").unwrap(); let result = scene.ucmd().arg("test_inode").arg("-i").succeeds(); assert!(re_short.is_match(result.stdout_str())); @@ -2275,3 +2279,52 @@ fn test_ls_dangling_symlinks() { .succeeds() // this should fail, though at the moment, ls lacks a way to propagate errors encountered during display .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); } + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_ls_context() { + use selinux::{self, KernelSupport}; + if selinux::kernel_support() == KernelSupport::Unsupported { + println!("test skipped: Kernel has no support for SElinux context",); + return; + } + let ts = TestScenario::new(util_name!()); + for c_flag in &["-Z", "--context"] { + ts.ucmd() + .args(&[c_flag, &"/"]) + .succeeds() + .stdout_only(unwrap_or_return!(expected_result(&ts, &[c_flag, &"/"])).stdout_str()); + } +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_ls_context_format() { + use selinux::{self, KernelSupport}; + if selinux::kernel_support() == KernelSupport::Unsupported { + println!("test skipped: Kernel has no support for SElinux context",); + return; + } + let ts = TestScenario::new(util_name!()); + // NOTE: + // --format=long/verbose matches the output of GNU's ls for --context + // except for the size count which may differ to the size count reported by GNU's ls. + for word in &[ + "across", + "commas", + "horizontal", + // "long", + "single-column", + // "verbose", + "vertical", + ] { + let format = format!("--format={}", word); + ts.ucmd() + .args(&[&"-Z", &format.as_str(), &"/"]) + .succeeds() + .stdout_only( + unwrap_or_return!(expected_result(&ts, &[&"-Z", &format.as_str(), &"/"])) + .stdout_str(), + ); + } +} From cd2153eac624232ff240b323eb4c04c110308e53 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Mon, 13 Sep 2021 13:24:44 +0200 Subject: [PATCH 3/5] ls: (--context/-Z) add support for invalid utf8 If security context byte slice contains invalid utf8: * issue warning * show replacement characters for invalid utf8 sequences --- src/uu/ls/src/ls.rs | 37 ++++++++++++++++++++++++++----------- tests/by-util/test_ls.rs | 18 +++++++++++++++++- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 5c26c8e26..be1964e43 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -40,6 +40,7 @@ use std::{ time::Duration, }; use term_grid::{Cell, Direction, Filling, Grid, GridOptions}; +use uucore::display::Quotable; use uucore::error::{set_exit_code, FromIo, UError, UResult}; use unicode_width::UnicodeWidthStr; @@ -1256,27 +1257,41 @@ impl PathData { None => OnceCell::new(), }; + let substitute_string = "?".to_string(); let security_context = if config.context { if config.selinux_supported { #[cfg(feature = "selinux")] { - if let Ok(Some(context)) = - selinux::SecurityContext::of_path(&p_buf, must_dereference, false) - { - String::from_utf8_lossy(context.as_bytes()) - .trim_end_matches(char::from(0)) - .to_string() - } else { - // TODO: print warning with error to stderr - "?".to_string() + match selinux::SecurityContext::of_path(&p_buf, must_dereference, false) { + Err(_r) => { + // TODO: show the actual reason why it failed + show_warning!("failed to get security context of: {}", p_buf.quote()); + substitute_string + } + Ok(None) => substitute_string, + Ok(Some(context)) => { + let mut context = context.as_bytes(); + if context.ends_with(&[0]) { + // TODO: replace with `strip_prefix()` when MSRV >= 1.51 + context = &context[..context.len() - 1] + }; + String::from_utf8(context.to_vec()).unwrap_or_else(|e| { + show_warning!( + "getting security context of: {}: {}", + p_buf.quote(), + e.to_string() + ); + String::from_utf8_lossy(context).into_owned() + }) + } } } #[cfg(not(feature = "selinux"))] { - "?".to_string() + substitute_string } } else { - "?".to_string() + substitute_string } } else { String::new() diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index f14fc424a..58119aac3 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2282,7 +2282,23 @@ fn test_ls_dangling_symlinks() { #[test] #[cfg(feature = "feat_selinux")] -fn test_ls_context() { +fn test_ls_context1() { + use selinux::{self, KernelSupport}; + if selinux::kernel_support() == KernelSupport::Unsupported { + println!("test skipped: Kernel has no support for SElinux context",); + return; + } + + let file = "test_ls_context_file"; + let expected = format!("unconfined_u:object_r:user_tmp_t:s0 {}\n", file); + let (at, mut ucmd) = at_and_ucmd!(); + at.touch(file); + ucmd.args(&["-Z", file]).succeeds().stdout_is(expected); +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_ls_context2() { use selinux::{self, KernelSupport}; if selinux::kernel_support() == KernelSupport::Unsupported { println!("test skipped: Kernel has no support for SElinux context",); From 8f229aad87187b4ffe11cfbdbb747029fff6c210 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Wed, 22 Sep 2021 12:24:27 +0200 Subject: [PATCH 4/5] ls: move SELinux String building logic to its own function --- src/uu/ls/src/ls.rs | 84 ++++++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 911323715..ba666d740 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1259,45 +1259,7 @@ impl PathData { None => OnceCell::new(), }; - let substitute_string = "?".to_string(); - let security_context = if config.context { - if config.selinux_supported { - #[cfg(feature = "selinux")] - { - match selinux::SecurityContext::of_path(&p_buf, must_dereference, false) { - Err(_r) => { - // TODO: show the actual reason why it failed - show_warning!("failed to get security context of: {}", p_buf.quote()); - substitute_string - } - Ok(None) => substitute_string, - Ok(Some(context)) => { - let mut context = context.as_bytes(); - if context.ends_with(&[0]) { - // TODO: replace with `strip_prefix()` when MSRV >= 1.51 - context = &context[..context.len() - 1] - }; - String::from_utf8(context.to_vec()).unwrap_or_else(|e| { - show_warning!( - "getting security context of: {}: {}", - p_buf.quote(), - e.to_string() - ); - String::from_utf8_lossy(context).into_owned() - }) - } - } - } - #[cfg(not(feature = "selinux"))] - { - substitute_string - } - } else { - substitute_string - } - } else { - String::new() - }; + let security_context = get_security_context(config, &p_buf, must_dereference); Self { md: OnceCell::new(), @@ -2124,3 +2086,47 @@ fn display_symlink_count(_metadata: &Metadata) -> String { fn display_symlink_count(metadata: &Metadata) -> String { metadata.nlink().to_string() } + +// This returns the SELinux security context as UTF8 `String`. +// In the long term this should be changed to `OsStr`, see discussions at #2621/#2656 +fn get_security_context(config: &Config, p_buf: &PathBuf, must_dereference: bool) -> String { + let substitute_string = "?".to_string(); + if config.context { + if config.selinux_supported { + #[cfg(feature = "selinux")] + { + match selinux::SecurityContext::of_path(p_buf, must_dereference, false) { + Err(_r) => { + // TODO: show the actual reason why it failed + show_warning!("failed to get security context of: {}", p_buf.quote()); + substitute_string + } + Ok(None) => substitute_string, + Ok(Some(context)) => { + let mut context = context.as_bytes(); + if context.ends_with(&[0]) { + // TODO: replace with `strip_prefix()` when MSRV >= 1.51 + context = &context[..context.len() - 1] + }; + String::from_utf8(context.to_vec()).unwrap_or_else(|e| { + show_warning!( + "getting security context of: {}: {}", + p_buf.quote(), + e.to_string() + ); + String::from_utf8_lossy(context).into_owned() + }) + } + } + } + #[cfg(not(feature = "selinux"))] + { + substitute_string + } + } else { + substitute_string + } + } else { + String::new() + } +} From 8cd8c25b0dd59aef697dbf772944fc6d57967cda Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Wed, 22 Sep 2021 13:49:08 +0200 Subject: [PATCH 5/5] ls: silence clippy warnings if feat_selinx is not set --- src/uu/ls/src/ls.rs | 69 +++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ba666d740..51dc0d247 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1259,7 +1259,11 @@ impl PathData { None => OnceCell::new(), }; - let security_context = get_security_context(config, &p_buf, must_dereference); + let security_context = if config.context { + get_security_context(config, &p_buf, must_dereference) + } else { + String::new() + }; Self { md: OnceCell::new(), @@ -2089,44 +2093,41 @@ fn display_symlink_count(metadata: &Metadata) -> String { // This returns the SELinux security context as UTF8 `String`. // In the long term this should be changed to `OsStr`, see discussions at #2621/#2656 -fn get_security_context(config: &Config, p_buf: &PathBuf, must_dereference: bool) -> String { +#[allow(unused_variables)] +fn get_security_context(config: &Config, p_buf: &Path, must_dereference: bool) -> String { let substitute_string = "?".to_string(); - if config.context { - if config.selinux_supported { - #[cfg(feature = "selinux")] - { - match selinux::SecurityContext::of_path(p_buf, must_dereference, false) { - Err(_r) => { - // TODO: show the actual reason why it failed - show_warning!("failed to get security context of: {}", p_buf.quote()); - substitute_string - } - Ok(None) => substitute_string, - Ok(Some(context)) => { - let mut context = context.as_bytes(); - if context.ends_with(&[0]) { - // TODO: replace with `strip_prefix()` when MSRV >= 1.51 - context = &context[..context.len() - 1] - }; - String::from_utf8(context.to_vec()).unwrap_or_else(|e| { - show_warning!( - "getting security context of: {}: {}", - p_buf.quote(), - e.to_string() - ); - String::from_utf8_lossy(context).into_owned() - }) - } + if config.selinux_supported { + #[cfg(feature = "selinux")] + { + match selinux::SecurityContext::of_path(p_buf, must_dereference, false) { + Err(_r) => { + // TODO: show the actual reason why it failed + show_warning!("failed to get security context of: {}", p_buf.quote()); + substitute_string + } + Ok(None) => substitute_string, + Ok(Some(context)) => { + let mut context = context.as_bytes(); + if context.ends_with(&[0]) { + // TODO: replace with `strip_prefix()` when MSRV >= 1.51 + context = &context[..context.len() - 1] + }; + String::from_utf8(context.to_vec()).unwrap_or_else(|e| { + show_warning!( + "getting security context of: {}: {}", + p_buf.quote(), + e.to_string() + ); + String::from_utf8_lossy(context).into_owned() + }) } } - #[cfg(not(feature = "selinux"))] - { - substitute_string - } - } else { + } + #[cfg(not(feature = "selinux"))] + { substitute_string } } else { - String::new() + substitute_string } }