Merge pull request #7898 from sylvestre/selinux-err

selinux: get closer to the GNU error msgs

Author: cakebaker

src/uu/chcon/src/chcon.rs

@@ -608,7 +608,7 @@ fn process_file(
     if result.is_ok() {
         if options.verbose {
             println!(
-                "{}: Changing security context of: {}",
+                "{}: changing security context of {}",
                 uucore::util_name(),
                 file_full_name.quote()
             );

src/uu/cp/src/cp.rs

@@ -1710,7 +1710,7 @@ pub(crate) fn copy_attributes(
             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}",
+                        "failed to set the security context of {}: {e}",
                         dest.display()
                     )));
                 }

src/uu/mkfifo/src/mkfifo.rs

@@ -74,10 +74,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
                     uucore::selinux::set_selinux_security_context(Path::new(&f), context)
                 {
                     let _ = fs::remove_file(f);
-                    return Err(USimpleError::new(
-                        1,
-                        format!("failed to set SELinux security context: {e}"),
-                    ));
+                    return Err(USimpleError::new(1, e.to_string()));
                 }
             }
         }

src/uu/mknod/src/mknod.rs

@@ -98,7 +98,7 @@ fn mknod(file_name: &str, config: Config) -> i32 {
             ) {
                 // if it fails, delete the file
                 let _ = std::fs::remove_dir(file_name);
-                eprintln!("failed to set SELinux security context: {}", e);
+                eprintln!("{}: {}", uucore::util_name(), e);
                 return 1;
             }
         }

src/uucore/src/lib/features/selinux.rs

@@ -3,6 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
+use std::error::Error;
 use std::path::Path;
 
 use selinux::SecurityContext;
@@ -13,16 +14,16 @@ pub enum SeLinuxError {
     #[error("SELinux is not enabled on this system")]
     SELinuxNotEnabled,
 
-    #[error("Failed to open the file: {0}")]
+    #[error("failed to open the file: {0}")]
     FileOpenFailure(String),
 
-    #[error("Failed to retrieve the security context: {0}")]
+    #[error("failed to retrieve the security context: {0}")]
     ContextRetrievalFailure(String),
 
-    #[error("Failed to set default file creation context to '{0}': {1}")]
+    #[error("failed to set default file creation context to '{0}': {1}")]
     ContextSetFailure(String, String),
 
-    #[error("Failed to set default file creation context to '{0}': {1}")]
+    #[error("failed to set default file creation context to '{0}': {1}")]
     ContextConversionFailure(String, String),
 }
 
@@ -45,6 +46,22 @@ pub fn is_selinux_enabled() -> bool {
     selinux::kernel_support() != selinux::KernelSupport::Unsupported
 }
 
+/// Returns a string describing the error and its causes.
+fn selinux_error_description(mut error: &dyn Error) -> String {
+    let mut description = String::new();
+    while let Some(source) = error.source() {
+        let error_text = source.to_string();
+        // Check if this is an OS error and trim it
+        if let Some(idx) = error_text.find(" (os error ") {
+            description.push_str(&error_text[..idx]);
+        } else {
+            description.push_str(&error_text);
+        }
+        error = source;
+    }
+    description
+}
+
 /// Sets the SELinux security context for the given filesystem path.
 ///
 /// If a specific context is provided, it attempts to set this context explicitly.
@@ -99,28 +116,40 @@ pub fn set_selinux_security_context(
     if let Some(ctx_str) = context {
         // Create a CString from the provided context string
         let c_context = std::ffi::CString::new(ctx_str.as_str()).map_err(|e| {
-            SeLinuxError::ContextConversionFailure(ctx_str.to_string(), e.to_string())
+            SeLinuxError::ContextConversionFailure(
+                ctx_str.to_string(),
+                selinux_error_description(&e),
+            )
         })?;
 
         // Convert the CString into an SELinux security context
         let security_context =
             selinux::OpaqueSecurityContext::from_c_str(&c_context).map_err(|e| {
-                SeLinuxError::ContextConversionFailure(ctx_str.to_string(), e.to_string())
+                SeLinuxError::ContextConversionFailure(
+                    ctx_str.to_string(),
+                    selinux_error_description(&e),
+                )
             })?;
 
         // Set the provided security context on the specified path
         SecurityContext::from_c_str(
             &security_context.to_c_string().map_err(|e| {
-                SeLinuxError::ContextConversionFailure(ctx_str.to_string(), e.to_string())
+                SeLinuxError::ContextConversionFailure(
+                    ctx_str.to_string(),
+                    selinux_error_description(&e),
+                )
             })?,
             false,
         )
         .set_for_path(path, false, false)
-        .map_err(|e| SeLinuxError::ContextSetFailure(ctx_str.to_string(), e.to_string()))
+        .map_err(|e| {
+            SeLinuxError::ContextSetFailure(ctx_str.to_string(), selinux_error_description(&e))
+        })
     } else {
         // If no context provided, set the default SELinux context for the path
-        SecurityContext::set_default_for_path(path)
-            .map_err(|e| SeLinuxError::ContextSetFailure(String::new(), e.to_string()))
+        SecurityContext::set_default_for_path(path).map_err(|e| {
+            SeLinuxError::ContextSetFailure(String::new(), selinux_error_description(&e))
+        })
     }
 }
 
@@ -171,18 +200,23 @@ pub fn get_selinux_security_context(path: &Path) -> Result<String, SeLinuxError>
         return Err(SeLinuxError::SELinuxNotEnabled);
     }
 
-    let f = std::fs::File::open(path).map_err(|e| SeLinuxError::FileOpenFailure(e.to_string()))?;
+    let f = std::fs::File::open(path)
+        .map_err(|e| SeLinuxError::FileOpenFailure(selinux_error_description(&e)))?;
 
     // Get the security context of the file
     let context = match SecurityContext::of_file(&f, false) {
         Ok(Some(ctx)) => ctx,
         Ok(None) => return Ok(String::new()), // No context found, return empty string
-        Err(e) => return Err(SeLinuxError::ContextRetrievalFailure(e.to_string())),
+        Err(e) => {
+            return Err(SeLinuxError::ContextRetrievalFailure(
+                selinux_error_description(&e),
+            ));
+        }
     };
 
-    let context_c_string = context
-        .to_c_string()
-        .map_err(|e| SeLinuxError::ContextConversionFailure(String::new(), e.to_string()))?;
+    let context_c_string = context.to_c_string().map_err(|e| {
+        SeLinuxError::ContextConversionFailure(String::new(), selinux_error_description(&e))
+    })?;
 
     if let Some(c_str) = context_c_string {
         // Convert the C string to a Rust String
@@ -336,7 +370,8 @@ mod tests {
                     println!("Context conversion failure for '{}': {}", ctx, e);
                 }
                 SeLinuxError::ContextSetFailure(ctx, e) => {
-                    assert!(false);
+                    assert!(!e.is_empty(), "Error message should not be empty");
+                    println!("Context conversion failure for '{}': {}", ctx, e);
                 }
                 SeLinuxError::FileOpenFailure(e) => {
                     assert!(

tests/by-util/test_cp.rs

@@ -6315,7 +6315,7 @@ fn test_cp_selinux_invalid() {
             .arg(TEST_HELLO_WORLD_SOURCE)
             .arg(TEST_HELLO_WORLD_DEST)
             .fails()
-            .stderr_contains("Failed to");
+            .stderr_contains("failed to");
         if at.file_exists(TEST_HELLO_WORLD_DEST) {
             at.remove(TEST_HELLO_WORLD_DEST);
         }

tests/by-util/test_mkdir.rs

@@ -411,7 +411,7 @@ fn test_selinux_invalid() {
         .arg(at.plus_as_string(dest))
         .fails()
         .no_stdout()
-        .stderr_contains("Failed to set default file creation context to 'testtest':");
+        .stderr_contains("failed to set default file creation context to 'testtest':");
     // invalid context, so, no directory
     assert!(!at.dir_exists(dest));
 }

util/build-gnu.sh

@@ -365,3 +365,7 @@ sed -i 's/not supported/unexpected argument/' tests/mv/mv-exchange.sh
 # /nix/store/xxxxxxxxxxxx...xxxx/bin/tr
 # We just replace the references to `/usr/bin/tr` with the result of `$(which tr)`
 sed -i  's/\/usr\/bin\/tr/$(which tr)/' tests/init.sh
+
+# upstream doesn't having the program name in the error message
+# but we do. We should keep it that way.
+sed -i 's/echo "changing security context/echo "chcon: changing security context/' tests/chcon/chcon.sh