From 18459e070fec2052a65d3d2e31527bb29a5989ca Mon Sep 17 00:00:00 2001 From: Panos Koutsovasilis Date: Fri, 26 Apr 2024 16:01:10 +0300 Subject: [PATCH 1/2] [Auditbeat/FIM/fsnotify]: remove time window where a child file operation of a directory can be lost (#39133) * fix(auditbeat/fim/fsnotify): remove time window where a child file operation in of a directory can be lost * fix(auditbeat/fim/fsnotify): remove redundant error wrapping (cherry picked from commit 074f2219dab876531f88bc9ffe1b415d96e32d64) # Conflicts: # auditbeat/module/file_integrity/monitor/recursive.go --- CHANGELOG.next.asciidoc | 3 ++ .../file_integrity/monitor/monitor_test.go | 7 ++- .../file_integrity/monitor/recursive.go | 49 +++++++++++++++++-- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 719edead7bc..96f44e6075e 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -35,6 +35,9 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d *Auditbeat* +- Prevent scenario of losing children-related file events in a directory for recursive fsnotify backend of auditbeat file integrity module {pull}39133[39133] + + *Filebeat* - Fix handling of un-parsed JSON in O365 module. {issue}37800[37800] {pull}38709[38709] diff --git a/auditbeat/module/file_integrity/monitor/monitor_test.go b/auditbeat/module/file_integrity/monitor/monitor_test.go index 3842948ce0b..2b907f74d82 100644 --- a/auditbeat/module/file_integrity/monitor/monitor_test.go +++ b/auditbeat/module/file_integrity/monitor/monitor_test.go @@ -177,6 +177,11 @@ func TestRecursiveSubdirPermissions(t *testing.T) { t.Skip("Skipping permissions test on Windows") } + if os.Getuid() == 0 { + t.Skip("skipping as root can access every file and thus this unittest will fail") + return + } + // Create dir to be watched dir, err := ioutil.TempDir("", "monitor") @@ -242,7 +247,7 @@ func TestRecursiveSubdirPermissions(t *testing.T) { for { // No event is received ev, err := readTimeout(t, watcher) - if err == errReadTimeout { + if errors.Is(err, errReadTimeout) { break } assertNoError(t, err) diff --git a/auditbeat/module/file_integrity/monitor/recursive.go b/auditbeat/module/file_integrity/monitor/recursive.go index 14cc99379d5..e28d9275904 100644 --- a/auditbeat/module/file_integrity/monitor/recursive.go +++ b/auditbeat/module/file_integrity/monitor/recursive.go @@ -84,25 +84,62 @@ func (watcher *recursiveWatcher) ErrorChannel() <-chan error { return watcher.inner.Errors } +func (watcher *recursiveWatcher) watchFile(path string, info os.FileInfo) error { + var err error + if info == nil { + info, err = os.Lstat(path) + if err != nil { + return err + } + } + + if info.IsDir() { + if err = watcher.tree.AddDir(path); err != nil { + return err + } + + if err = watcher.inner.Add(path); err != nil { + return err + } + + return nil + } + + return watcher.tree.AddFile(path) +} + func (watcher *recursiveWatcher) addRecursive(path string) error { if watcher.isExcludedPath(path) { return nil } + if err := watcher.watchFile(path, nil); err != nil { + return fmt.Errorf("failed adding watcher to '%s': %w", path, err) + } + var errs multierror.Errors - err := filepath.Walk(path, func(path string, info os.FileInfo, fnErr error) error { - if watcher.isExcludedPath(path) { + err := filepath.Walk(path, func(walkPath string, info os.FileInfo, fnErr error) error { + if walkPath == path { + return nil + } + + if watcher.isExcludedPath(walkPath) { return nil } if fnErr != nil { +<<<<<<< HEAD errs = append(errs, errors.Wrapf(fnErr, "error walking path '%s'", path)) +======= + errs = append(errs, fmt.Errorf("error walking path '%s': %w", walkPath, fnErr)) +>>>>>>> 074f2219da ([Auditbeat/FIM/fsnotify]: remove time window where a child file operation of a directory can be lost (#39133)) // If FileInfo is not nil, the directory entry can be processed // even if there was some error if info == nil { return nil } } +<<<<<<< HEAD var err error if info.IsDir() { if err = watcher.tree.AddDir(path); err == nil { @@ -113,8 +150,14 @@ func (watcher *recursiveWatcher) addRecursive(path string) error { } } else { err = watcher.tree.AddFile(path) +======= + + if err := watcher.watchFile(walkPath, info); err != nil { + errs = append(errs, fmt.Errorf("failed adding watcher to '%s': %w", walkPath, err)) +>>>>>>> 074f2219da ([Auditbeat/FIM/fsnotify]: remove time window where a child file operation of a directory can be lost (#39133)) } - return err + + return nil }) watcher.log.Debugw("Added recursive watch", "path", path) From f13b10b7ac60394eb3207508d3c926b99baacad2 Mon Sep 17 00:00:00 2001 From: Panos Koutsovasilis Date: Fri, 26 Apr 2024 16:34:30 +0300 Subject: [PATCH 2/2] fix: resolve conflicts --- .../module/file_integrity/monitor/recursive.go | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/auditbeat/module/file_integrity/monitor/recursive.go b/auditbeat/module/file_integrity/monitor/recursive.go index e28d9275904..cf3957363b5 100644 --- a/auditbeat/module/file_integrity/monitor/recursive.go +++ b/auditbeat/module/file_integrity/monitor/recursive.go @@ -18,6 +18,7 @@ package monitor import ( + "fmt" "os" "path/filepath" @@ -128,33 +129,16 @@ func (watcher *recursiveWatcher) addRecursive(path string) error { } if fnErr != nil { -<<<<<<< HEAD - errs = append(errs, errors.Wrapf(fnErr, "error walking path '%s'", path)) -======= errs = append(errs, fmt.Errorf("error walking path '%s': %w", walkPath, fnErr)) ->>>>>>> 074f2219da ([Auditbeat/FIM/fsnotify]: remove time window where a child file operation of a directory can be lost (#39133)) // If FileInfo is not nil, the directory entry can be processed // even if there was some error if info == nil { return nil } } -<<<<<<< HEAD - var err error - if info.IsDir() { - if err = watcher.tree.AddDir(path); err == nil { - if err = watcher.inner.Add(path); err != nil { - errs = append(errs, errors.Wrapf(err, "failed adding watcher to '%s'", path)) - return nil - } - } - } else { - err = watcher.tree.AddFile(path) -======= if err := watcher.watchFile(walkPath, info); err != nil { errs = append(errs, fmt.Errorf("failed adding watcher to '%s': %w", walkPath, err)) ->>>>>>> 074f2219da ([Auditbeat/FIM/fsnotify]: remove time window where a child file operation of a directory can be lost (#39133)) } return nil