Add ability to return event types used by a filter #74

mstemm · 2021-08-25T21:05:24Z

Add ability to return event types used by a filter

Add the ability to return the event types used by a filter. For
example, if a filter was "evt.type=open and fd.name=/tmp/foo", the
event types would be PPME_SYSCALL_OPEN*_{E,X}.

By default, an empty set is returned, meaning no specific events are
used.

Event types PPME_GENERIC_{E,X} are not included and it's assumed the
code using this will handle those event types directly.

This is used in programs like falco to provide a quick external test
against an event to see if it makes sense to evaluate the filter at
all. This can speed up event processing when falco has a large number
of loaded rules. Prior to this change, this was handled solely in
falco's lua code for loading rules. Moving responsibility to the
filter significantly simplifies the falco side of rule loading.

In the base classes, new methods
gen_event_filter_check::evttypes/possible_evttypes return a set of
event types. The base class implementation just returns a single event
type "1".

gen_event_filter_expression::evttypes() does all the work of iterating
over the filterchecks that make up an expression and combining sets of
event types. possible_evttypes is used for "not" operators, which
invert a set of event types to include everything outside the set.

The sinsp "base" class sinsp_filter_check just returns all event types
from 2 to PPM_EVENT_MAX.

The only actual implementation of evttypes() that does something is in
sinsp_filter_check_event for the field "evt.type". The method handles
=, in, and != as comparison operators.

Also add a unit test that compiles various filters and double-checks
the resulting set of event types.

Signed-off-by: Mark Stemm mark.stemm@gmail.com

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area build

/area driver-kmod

/area driver-ebpf

/area libscap

/area libsinsp

/area tests

/area proposals

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new: add the ability to report the event types that are relevant for a given filter. This is used by programs like Falco to speed up rule evaluation.

leogr · 2021-08-26T14:18:36Z

/ok-to-test

leogr · 2021-08-26T14:18:50Z

/ok-to-test

Modify rulesets to not keep track of the event types for a given set filter. Instead, using the changes in falcosecurity/libs#74 event types are returned directly by the filter. Within each ruleset, there's a vector that maps from event number to set of filters that are related to that event number. There's also a general set of filters for all event types. run() both indexes into the per-event vector as well as iterate over the all event types set. Also, used shared_ptr instead of direct pointers, which matches the updated interface used by lua_parser. This simplifies the bookkeeping a bit (no more delete when removing rulesets). Given these changes, there's no need for a separate falco_sinsp_ruleset class any longer, so remove it. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

gnosek

LGTM, though I'm vaguely bothered by the conventions, where an empty set means everything and we just union the sets together regardless of the filter boolean ops between them

userspace/libsinsp/gen_filter.cpp

userspace/libsinsp/filterchecks.cpp

userspace/libsinsp/gen_filter.cpp

leogr · 2021-09-15T14:21:57Z

Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

Please follow instructions in the contributing guide to update your commits with the DCO

Full details of the Developer Certificate of Origin can be found at developercertificate.org.

The list of commits missing DCO signoff:

ab720e7 WIP on refactoring event types to handle null/all events cases

149118a This verison works

299c673 This passes but needs a rebase/cleanup

Hey Mark, can you sign off these commits, please?

Add the ability to return the event types used by a filter. For example, if a filter was "evt.type=open and fd.name=/tmp/foo", the event types would be PPME_SYSCALL_OPEN*_{E,X}. By default, an empty set is returned, meaning no specific events are used. Event types PPME_GENERIC_{E,X} are not included and it's assumed the code using this will handle those event types directly. This is used in programs like falco to provide a quick external test against an event to see if it makes sense to evaluate the filter at all. This can speed up event processing when falco has a large number of loaded rules. Prior to this change, this was handled solely in falco's lua code for loading rules. Moving responsibility to the filter significantly simplifies the falco side of rule loading. In the base classes, new methods gen_event_filter_check::evttypes/possible_evttypes return a set of event types. The base class implementation just returns a single event type "1". gen_event_filter_expression::evttypes() does all the work of iterating over the filterchecks that make up an expression and combining sets of event types. possible_evttypes is used for "not" operators, which invert a set of event types to include everything outside the set. The sinsp "base" class sinsp_filter_check just returns all event types from 2 to PPM_EVENT_MAX. The only actual implementation of evttypes() that does something is in sinsp_filter_check_event for the field "evt.type". The method handles =, in, and != as comparison operators. Also add a unit test that compiles various filters and double-checks the resulting set of event types. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

mstemm · 2021-09-16T00:52:41Z

@leogr: sorry I was in the middle of some refactoring to handle the logical operators. Rebased to one commit with a sign-off message.

mstemm · 2021-09-16T00:54:54Z

@gnosek I improved this to tighten up the handling of the set of event types. No more games with approximation/empty set means all events or anything now--the set of event types should exactly match the set of event types that are applicable for a given filter, even for fairly complicated sets of filters with nesting, mixed and/or, etc. You can check the unit test to see the filters I tried. Please let me know if you can think of others!

leogr

LGTM now. Thank you!

/approve

poiana · 2021-09-16T14:20:09Z

LGTM label has been added.

Git tree hash: 65f13c25837c7b0f04c064c34503a30905daac7e

poiana · 2021-09-16T14:20:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leogr, mstemm

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [leogr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Pinned to a commit that contains PRs falcosecurity/libs#74, falcosecurity/libs#75, falcosecurity/libs#76, falcosecurity/libs#77 Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

Modify rulesets to not keep track of the event types for a given set filter. Instead, using the changes in falcosecurity/libs#74 event types are returned directly by the filter. Within each ruleset, there's a vector that maps from event number to set of filters that are related to that event number. There's also a general set of filters for all event types. run() both indexes into the per-event vector as well as iterate over the all event types set. Also, used shared_ptr instead of direct pointers, which matches the updated interface used by lua_parser. This simplifies the bookkeeping a bit (no more delete when removing rulesets). Given these changes, there's no need for a separate falco_sinsp_ruleset class any longer, so remove it. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

With the changes in falcosecurity/libs#74, there isn't any need to warn about the order of operators and the evt.type field--the set of event types for a filter should be exact now regardless of the order of operators. So update tests that were logging those warnings to note that the warnings won't occur any more. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

With the changes in falcosecurity/libs#74, there isn't any need to warn about the order of operators and the evt.type field--the set of event types for a filter should be exact now regardless of the order of operators. So update tests that were logging those warnings to note that the warnings won't occur any more. Also, some tests more accurately *do* note that they have an overly permissive evttype. Update the rules (which are generally testing other things) to reflect the new behavior. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

With the changes in falcosecurity/libs#74, there isn't any need to warn about the order of operators and the evt.type field--the set of event types for a filter should be exact now regardless of the order of operators. So update tests that were logging those warnings to note that the warnings won't occur any more. Also, some tests more accurately *do* note that they have an overly permissive evttype (e.g. ones related to syscalls, which are uncommon and are evaluated for all event types) to reflect the new behavior. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

Modify rulesets to not keep track of the event types for a given set filter. Instead, using the changes in falcosecurity/libs#74 event types are returned directly by the filter. Within each ruleset, there's a vector that maps from event number to set of filters that are related to that event number. There's also a general set of filters for all event types. run() both indexes into the per-event vector as well as iterate over the all event types set. Also, used shared_ptr instead of direct pointers, which matches the updated interface used by lua_parser. This simplifies the bookkeeping a bit (no more delete when removing rulesets). Given these changes, there's no need for a separate falco_sinsp_ruleset class any longer, so remove it. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

With the changes in falcosecurity/libs#74, there isn't any need to warn about the order of operators and the evt.type field--the set of event types for a filter should be exact now regardless of the order of operators. So update tests that were logging those warnings to note that the warnings won't occur any more. Also, some tests more accurately *do* note that they have an overly permissive evttype (e.g. ones related to syscalls, which are uncommon and are evaluated for all event types) to reflect the new behavior. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

Modify rulesets to not keep track of the event types for a given set filter. Instead, using the changes in falcosecurity/libs#74 event types are returned directly by the filter. Within each ruleset, there's a vector that maps from event number to set of filters that are related to that event number. There's also a general set of filters for all event types. run() both indexes into the per-event vector as well as iterate over the all event types set. Also, used shared_ptr instead of direct pointers, which matches the updated interface used by lua_parser. This simplifies the bookkeeping a bit (no more delete when removing rulesets). Given these changes, there's no need for a separate falco_sinsp_ruleset class any longer, so remove it. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

With the changes in falcosecurity/libs#74, there isn't any need to warn about the order of operators and the evt.type field--the set of event types for a filter should be exact now regardless of the order of operators. So update tests that were logging those warnings to note that the warnings won't occur any more. Also, some tests more accurately *do* note that they have an overly permissive evttype (e.g. ones related to syscalls, which are uncommon and are evaluated for all event types) to reflect the new behavior. Finally, in unit tests create an actual sinsp filter instead of a gen_event_filter, which is the base class and shouldn't be created directly. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

Modify rulesets to not keep track of the event types for a given set filter. Instead, using the changes in falcosecurity/libs#74 event types are returned directly by the filter. Within each ruleset, there's a vector that maps from event number to set of filters that are related to that event number. There's also a general set of filters for all event types. run() both indexes into the per-event vector as well as iterate over the all event types set. Also, used shared_ptr instead of direct pointers, which matches the updated interface used by lua_parser. This simplifies the bookkeeping a bit (no more delete when removing rulesets). Given these changes, there's no need for a separate falco_sinsp_ruleset class any longer, so remove it. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

With the changes in falcosecurity/libs#74, there isn't any need to warn about the order of operators and the evt.type field--the set of event types for a filter should be exact now regardless of the order of operators. So update tests that were logging those warnings to note that the warnings won't occur any more. Also, some tests more accurately *do* note that they have an overly permissive evttype (e.g. ones related to syscalls, which are uncommon and are evaluated for all event types) to reflect the new behavior. Finally, in unit tests create an actual sinsp filter instead of a gen_event_filter, which is the base class and shouldn't be created directly. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

In #74 we pushed down some code from falco that determines the sinsp event types that are applicable for a given filter. The set of event types starts with "all events", and as the filter condition is parsed, including logical operators like "and", "or", "not", etc, the set of event types is changed, honoring the logical operators. For example, for a condition proc.name=nginx, the filter is applicable for all event types, as the condition does not include any evt.type field. For a more complicated condition like evt.type=openat and proc.name=nginx, the first field restricts the event types to openat, which is logical anded against all event types from proc.name, resulting in only the event type openat. With the introduction of plugins, there's also a need to segregate plugin-related filterchecks from non-plugin-related filterchecks by event source, but that's handled at a higher level, using a notion of filter factories and formatter factories (#77). The bug is that plugin filtercheck fields like ct.name, json.value were mistakenly being restricted to only the plugin event PPME_PLUGINEVENT_E. This was being mistakenly inverted when conditions had a "not" operator, with the result being that they did not run on any event types at all. The fix is to treat plugin fields as working with all event types, just like almost all other fields like proc.name, etc. are. This allows the logical operators to combine event type sets properly. This, along with other small changes in falco + plugins, fixes falcosecurity/plugins#56. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

poiana added release-note kind/feature New feature or request dco-signoff: yes area/libsinsp area/tests labels Aug 25, 2021

poiana requested review from gnosek and leogr August 25, 2021 21:05

poiana added the size/XL label Aug 25, 2021

poiana added the ok-to-test label Aug 26, 2021

mstemm mentioned this pull request Aug 27, 2021

Gen events falco engine falcosecurity/falco#1715

Merged

gnosek reviewed Sep 9, 2021

View reviewed changes

userspace/libsinsp/gen_filter.cpp Outdated Show resolved Hide resolved

userspace/libsinsp/filterchecks.cpp Outdated Show resolved Hide resolved

userspace/libsinsp/gen_filter.cpp Outdated Show resolved Hide resolved

mstemm force-pushed the filter-add-evttypes branch from 3031bea to fd5eb71 Compare September 10, 2021 18:58

poiana added dco-signoff: no size/XXL and removed dco-signoff: yes size/XL labels Sep 14, 2021

mstemm force-pushed the filter-add-evttypes branch from 49e7167 to 9776a0b Compare September 16, 2021 00:49

poiana added dco-signoff: yes and removed dco-signoff: no labels Sep 16, 2021

leogr approved these changes Sep 16, 2021

View reviewed changes

poiana assigned leogr Sep 16, 2021

poiana added the lgtm label Sep 16, 2021

poiana added the approved label Sep 16, 2021

mstemm mentioned this pull request Sep 16, 2021

Add ability to return all fields exported by a factory #75

Merged

gnosek approved these changes Oct 1, 2021

View reviewed changes

poiana assigned gnosek Oct 1, 2021

poiana merged commit 01f2e2a into master Oct 1, 2021

poiana deleted the filter-add-evttypes branch October 1, 2021 16:33

mstemm mentioned this pull request Jan 26, 2022

empty string with double quotes in aws_cloudtrail event "" falcosecurity/plugins#56

Closed

mstemm mentioned this pull request Jan 26, 2022

Treat plugin fields like other sinsp fields (all evt types) #190

Closed

jasondellaluce mentioned this pull request Apr 6, 2022

refactor(userspace/engine): reduce memory usage for resolving evttypes falcosecurity/falco#1965

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to return event types used by a filter #74

Add ability to return event types used by a filter #74

mstemm commented Aug 25, 2021 •

edited by leogr

Loading

leogr commented Aug 26, 2021

leogr commented Aug 26, 2021

gnosek left a comment

leogr commented Sep 15, 2021 •

edited

Loading

mstemm commented Sep 16, 2021

mstemm commented Sep 16, 2021

leogr left a comment

poiana commented Sep 16, 2021

poiana commented Sep 16, 2021

Add ability to return event types used by a filter #74

Add ability to return event types used by a filter #74

Conversation

mstemm commented Aug 25, 2021 • edited by leogr Loading

leogr commented Aug 26, 2021

leogr commented Aug 26, 2021

gnosek left a comment

Choose a reason for hiding this comment

leogr commented Sep 15, 2021 • edited Loading

mstemm commented Sep 16, 2021

mstemm commented Sep 16, 2021

leogr left a comment

Choose a reason for hiding this comment

poiana commented Sep 16, 2021

poiana commented Sep 16, 2021

mstemm commented Aug 25, 2021 •

edited by leogr

Loading

leogr commented Sep 15, 2021 •

edited

Loading