Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auditbeat: Add exclude_files option to file integrity module #5888

Merged
merged 9 commits into from
Dec 18, 2017

Conversation

adriansr
Copy link
Contributor

Added a new config field, exclude_files consisting of regular expressions used to exclude files from monitoring by the file integrity module.

See #5342

@adriansr adriansr added Auditbeat review in progress Pull request is currently in progress. labels Dec 15, 2017
@adriansr
Copy link
Contributor Author

Adding a unit test

return true
}

func (r *capturingReporterV2) Done() <-chan struct{} {
return r.done
func (r *CapturingReporterV2) Done() <-chan struct{} {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported method CapturingReporterV2.Done should have comment or be unexported

}

func (r *capturingReporterV2) Event(event mb.Event) bool {
r.events = append(r.events, event)
func (r *CapturingReporterV2) Event(event mb.Event) bool {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported method CapturingReporterV2.Event should have comment or be unexported

type capturingReporterV2 struct {
events []mb.Event
done chan struct{}
type CapturingReporterV2 struct {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type CapturingReporterV2 should have comment or be unexported

@adriansr adriansr removed the in progress Pull request is currently in progress. label Dec 15, 2017
ScanRateBytesPerSec uint64 `config:",ignore"`
Recursive bool `config:"recursive"` // Recursive enables recursive monitoring of directories.
Exclude []string `config:"exclude_files"`
ExcludeMatchers []match.Matcher `config:",ignore"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

match.Matcher implements ucfg.StringUpacker so it will automatically be populated with the compiled regexes. If any of them fail to compile ucfg should emit an error. So I don't think you need the []string version at all.

@@ -167,6 +167,13 @@ func (ms *MetricSet) init(reporter mb.PushReporterV2) bool {
}

func (ms *MetricSet) reportEvent(reporter mb.PushReporterV2, event *Event) bool {
// ignore excluded paths
for _, matcher := range ms.config.ExcludeMatchers {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it is most convenient to apply the filtering here, this wouldn't be much different that using a drop_event processor with a regexp condition. In my mind the benefit of having a dedicated exclude_files option is that the filter can be applied earlier in order to save the CPU and I/O cycles associated with reading the file and hashing it. Would it be feasible to apply this filter earlier?

It would need to be applied to the scanner and fsnotify/fsevents readers. I think the events that are triggered by purgeDeleted would need filtered as well.

file := filepath.Join(dir, f)
ioutil.WriteFile(file, []byte("hello world"), 0600)
}
for i := 0; i < 100 && len(capture.Events) < 3; i++ {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for a certain number of events is a pretty common task in tests. We should make this easier to do with the mb/testing package.

return r.done
// Done returns the Done channel for this reporter.
func (r *CapturingReporterV2) Done() <-chan struct{} {
return r.DoneC
}

// RunPushMetricSetV2 run the given push metricset for the specific amount of
// time and returns all of the events and errors that occur during that period.
func RunPushMetricSetV2(duration time.Duration, metricSet mb.PushMetricSetV2) []mb.Event {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense to add another parameter (maxEvents) to RunPushMetricSetV2 for specifying the maximum number of events to wait for. So then the behavior would be run until you reach duration (probably should rename to timeout) or until the maximum number of events is received (if maxEvents > 0).

@@ -64,6 +68,12 @@ Linux.
*`paths`*:: A list of paths (directories or files) to watch. Globs are
not supported. The specified paths should exist when the metricset is started.

*`exclude_files`:: A list of regular expressions used to filter out events
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on questions I've seen for Filebeat, I think it would be good to explicitly state that this regular expression is matched against the file's absolute path.

@@ -107,6 +109,15 @@ func deduplicate(in []string) []string {
return out
}

func (c *Config) IsExcludedPath(path string) bool {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported method Config.IsExcludedPath should have comment or be unexported

@adriansr adriansr force-pushed the auditbeat/exclude branch 3 times, most recently from dbac025 to a59deaa Compare December 16, 2017 14:07
@andrewkroh andrewkroh merged commit 80e9b4a into elastic:master Dec 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants