pxar: handle missing GROUP_OBJ ACL entries

Previously we did not store GROUP_OBJ ACL entries for
directories, this means that these were lost which may
potentially elevate group permissions if they were masked
before via ACLs, so we also show a warning.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

src/pxar/dir_stack.rs

@@ -1,6 +1,6 @@
-use std::ffi::{OsStr, OsString};
+use std::ffi::OsString;
 use std::os::unix::io::{AsRawFd, RawFd};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 use anyhow::{bail, format_err, Error};
 use nix::dir::Dir;
@@ -78,10 +78,6 @@ impl PxarDir {
     pub fn metadata(&self) -> &Metadata {
         &self.metadata
     }
-
-    pub fn file_name(&self) -> &OsStr {
-        &self.file_name
-    }
 }
 
 pub struct PxarDirStack {
@@ -159,4 +155,8 @@ impl PxarDirStack {
             .try_as_borrowed_fd()
             .ok_or_else(|| format_err!("lost track of directory file descriptors"))
     }
+
+    pub fn path(&self) -> &Path {
+        &self.path
+    }
 }

src/pxar/extract.rs

@@ -285,6 +285,8 @@ impl Extractor {
 
     /// When done with a directory we can apply its metadata if it has been created.
     pub fn leave_directory(&mut self) -> Result<(), Error> {
+        let path_info = self.dir_stack.path().to_owned();
+
         let dir = self
             .dir_stack
             .pop()
@@ -296,7 +298,7 @@ impl Extractor {
                 self.feature_flags,
                 dir.metadata(),
                 fd.as_raw_fd(),
-                &CString::new(dir.file_name().as_bytes())?,
+                &path_info,
                 &mut self.on_error,
             )
             .map_err(|err| format_err!("failed to apply directory metadata: {}", err))?;
@@ -329,6 +331,7 @@ impl Extractor {
             metadata,
             parent,
             file_name,
+            self.dir_stack.path(),
             &mut self.on_error,
         )
     }
@@ -382,6 +385,7 @@ impl Extractor {
             metadata,
             parent,
             file_name,
+            self.dir_stack.path(),
             &mut self.on_error,
         )
     }
@@ -437,7 +441,7 @@ impl Extractor {
             self.feature_flags,
             metadata,
             file.as_raw_fd(),
-            file_name,
+            self.dir_stack.path(),
             &mut self.on_error,
         )
     }
@@ -494,7 +498,7 @@ impl Extractor {
             self.feature_flags,
             metadata,
             file.as_raw_fd(),
-            file_name,
+            self.dir_stack.path(),
             &mut self.on_error,
         )
     }

src/pxar/metadata.rs

@@ -1,5 +1,6 @@
 use std::ffi::{CStr, CString};
 use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
+use std::path::Path;
 
 use anyhow::{bail, format_err, Error};
 use nix::errno::Errno;
@@ -62,6 +63,7 @@ pub fn apply_at(
     metadata: &Metadata,
     parent: RawFd,
     file_name: &CStr,
+    path_info: &Path,
     on_error: &mut (dyn FnMut(Error) -> Result<(), Error> + Send),
 ) -> Result<(), Error> {
     let fd = proxmox::tools::fd::Fd::openat(
@@ -71,7 +73,7 @@ pub fn apply_at(
         Mode::empty(),
     )?;
 
-    apply(flags, metadata, fd.as_raw_fd(), file_name, on_error)
+    apply(flags, metadata, fd.as_raw_fd(), path_info, on_error)
 }
 
 pub fn apply_initial_flags(
@@ -94,7 +96,7 @@ pub fn apply(
     flags: Flags,
     metadata: &Metadata,
     fd: RawFd,
-    file_name: &CStr,
+    path_info: &Path,
     on_error: &mut (dyn FnMut(Error) -> Result<(), Error> + Send),
 ) -> Result<(), Error> {
     let c_proc_path = CString::new(format!("/proc/self/fd/{}", fd)).unwrap();
@@ -116,7 +118,7 @@ pub fn apply(
     apply_xattrs(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs)
         .or_else(&mut *on_error)?;
     add_fcaps(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs).or_else(&mut *on_error)?;
-    apply_acls(flags, &c_proc_path, metadata)
+    apply_acls(flags, &c_proc_path, metadata, path_info)
         .map_err(|err| format_err!("failed to apply acls: {}", err))
         .or_else(&mut *on_error)?;
     apply_quota_project_id(flags, fd, metadata).or_else(&mut *on_error)?;
@@ -147,7 +149,7 @@ pub fn apply(
         Err(err) => {
             on_error(format_err!(
                 "failed to restore mtime attribute on {:?}: {}",
-                file_name,
+                path_info,
                 err
             ))?;
         }
@@ -227,7 +229,12 @@ fn apply_xattrs(
     Ok(())
 }
 
-fn apply_acls(flags: Flags, c_proc_path: &CStr, metadata: &Metadata) -> Result<(), Error> {
+fn apply_acls(
+    flags: Flags,
+    c_proc_path: &CStr,
+    metadata: &Metadata,
+    path_info: &Path,
+) -> Result<(), Error> {
     if !flags.contains(Flags::WITH_ACL) || metadata.acl.is_empty() {
         return Ok(());
     }
@@ -257,11 +264,17 @@ fn apply_acls(flags: Flags, c_proc_path: &CStr, metadata: &Metadata) -> Result<(
             acl.add_entry_full(acl::ACL_GROUP_OBJ, None, group_obj.permissions.0)?;
         }
         None => {
-            acl.add_entry_full(
+            let mode = acl::mode_group_to_acl_permissions(metadata.stat.mode);
-                acl::ACL_GROUP_OBJ,
+
-                None,
+            acl.add_entry_full(acl::ACL_GROUP_OBJ, None, mode)?;
-                acl::mode_group_to_acl_permissions(metadata.stat.mode),
+
-            )?;
+            if !metadata.acl.users.is_empty() || !metadata.acl.groups.is_empty() {
+                eprintln!(
+                    "Warning: {:?}: Missing GROUP_OBJ entry in ACL, resetting to value of MASK",
+                    path_info,
+                );
+                acl.add_entry_full(acl::ACL_MASK, None, mode)?;
+            }
         }
     }