[pbs-devel] [PATCH proxmox-backup v3 1/1] fix #4380: check if file is excluded before running `stat()`

[Gabriel Goller]

Before running `stat()` we now make a `matches_path()` call, which checks
if the file/directory is excluded (`matches_path()` tries to match using
only the path, without the file mode). If the file is not excluded,
we `stat()` and then check again if the path matches using the file_mode and
the `matches()` function.
Added `pathpatterns` crate to local overrides in cargo.toml.

changes v2:
 - checking for `read` and `execute` permissions before entering directory,
   doesn't work because there are a lot of side-effects (executed by
   different user, AppArmor, SELinux, ...).
changes v1:
 - checking for excluded files with `matches()` before executing `stat()`,
   this doesn't work because we get the file_mode from `stat()` and don't 
   want to ignore it when matching.

Signed-off-by: Gabriel Goller <g.goller at proxmox.com>
---
 Cargo.toml                    |  1 +
 pbs-client/src/pxar/create.rs | 45 ++++++++++++++++++++++++++---------
 2 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 2a05c471..c54bc28b 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -262,6 +262,7 @@ proxmox-rrd.workspace = true
 
 #proxmox-apt = { path = "../proxmox-apt" }
 #proxmox-openid = { path = "../proxmox-openid-rs" }
+#pathpatterns = {path = "../pathpatterns" }
 
 #pxar = { path = "../pxar" }
 
diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
index 2577cf98..a6d608e7 100644
--- a/pbs-client/src/pxar/create.rs
+++ b/pbs-client/src/pxar/create.rs
@@ -232,7 +232,7 @@ impl Archiver {
             let entry_counter = self.entry_counter;
 
             let old_patterns_count = self.patterns.len();
-            self.read_pxar_excludes(dir.as_raw_fd())?;
+            self.read_pxar_excludes(&mut dir)?;
 
             let mut file_list = self.generate_directory_file_list(&mut dir, is_root)?;
 
@@ -315,8 +315,23 @@ impl Archiver {
         }
     }
 
-    fn read_pxar_excludes(&mut self, parent: RawFd) -> Result<(), Error> {
-        let fd = match self.open_file(parent, c_str!(".pxarexclude"), OFlag::O_RDONLY, false)? {
+    fn read_pxar_excludes(&mut self, parent: &mut Dir) -> Result<(), Error> {
+        let mut exclude_file_found = false;
+        for file in parent.iter() {
+            if file?.file_name().to_str()? == ".pxarexclude" {
+                exclude_file_found = true;
+                break;
+            }
+        }
+        if !exclude_file_found {
+            return Ok(());
+        }
+        let fd = match self.open_file(
+            parent.as_raw_fd(),
+            c_str!(".pxarexclude"),
+            OFlag::O_RDONLY,
+            false,
+        )? {
             Some(fd) => fd,
             None => return Ok(()),
         };
@@ -420,7 +435,7 @@ impl Archiver {
         for file in dir.iter() {
             let file = file?;
 
-            let file_name = file.file_name().to_owned();
+            let file_name = file.file_name();
             let file_name_bytes = file_name.to_bytes();
             if file_name_bytes == b"." || file_name_bytes == b".." {
                 continue;
@@ -434,9 +449,17 @@ impl Archiver {
             assert_single_path_component(os_file_name)?;
             let full_path = self.path.join(os_file_name);
 
+            let match_path = PathBuf::from("/").join(full_path.clone());
+            let pre_stat_match = self
+                .patterns
+                .matches_path(match_path.as_os_str().as_bytes());
+            if pre_stat_match == Ok(Some(MatchType::Exclude)) {
+                continue;
+            }
+
             let stat = match nix::sys::stat::fstatat(
                 dir_fd,
-                file_name.as_c_str(),
+                file_name.to_owned().as_c_str(),
                 nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW,
             ) {
                 Ok(stat) => stat,
@@ -444,11 +467,11 @@ impl Archiver {
                 Err(err) => return Err(err).context(format!("stat failed on {:?}", full_path)),
             };
 
-            let match_path = PathBuf::from("/").join(full_path.clone());
-            if self
-                .patterns
-                .matches(match_path.as_os_str().as_bytes(), Some(stat.st_mode))
-                == Some(MatchType::Exclude)
+            if pre_stat_match.is_err()
+                && self
+                    .patterns
+                    .matches(match_path.as_os_str().as_bytes(), Some(stat.st_mode))
+                    == Some(MatchType::Exclude)
             {
                 continue;
             }
@@ -462,7 +485,7 @@ impl Archiver {
             }
 
             file_list.push(FileListEntry {
-                name: file_name,
+                name: file_name.to_owned(),
                 path: full_path,
                 stat,
             });
-- 
2.39.2