From 79e58a903e7065221fa59decf8fbee9417183be4 Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Fri, 2 Apr 2021 10:45:07 +0200 Subject: [PATCH] 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 --- src/pxar/dir_stack.rs | 12 ++++++------ src/pxar/extract.rs | 10 +++++++--- src/pxar/metadata.rs | 33 +++++++++++++++++++++++---------- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/pxar/dir_stack.rs b/src/pxar/dir_stack.rs index d4e8e662..86740ffd 100644 --- a/src/pxar/dir_stack.rs +++ b/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 + } } diff --git a/src/pxar/extract.rs b/src/pxar/extract.rs index 952e2d20..32796837 100644 --- a/src/pxar/extract.rs +++ b/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, ) } diff --git a/src/pxar/metadata.rs b/src/pxar/metadata.rs index 4992cc8f..666af70e 100644 --- a/src/pxar/metadata.rs +++ b/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( - acl::ACL_GROUP_OBJ, - None, - acl::mode_group_to_acl_permissions(metadata.stat.mode), - )?; + let mode = acl::mode_group_to_acl_permissions(metadata.stat.mode); + + acl.add_entry_full(acl::ACL_GROUP_OBJ, None, 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)?; + } } }