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

Gen events falco engine #1715

Merged
merged 18 commits into from
Oct 12, 2021
Merged

Gen events falco engine #1715

merged 18 commits into from
Oct 12, 2021

Conversation

mstemm
Copy link
Contributor

@mstemm mstemm commented Aug 26, 2021

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

If contributing rules or changes to rules, please make sure to also uncomment one of the following line:

/kind rule-update

/kind rule-create

Any specific area of the project related to this PR?

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

/area build

/area engine

/area rules

/area tests

/area proposals

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

This also allows creating a "lite" version of falco that does not have explicit syscall support if we wish.

Does this PR introduce a user-facing change?:

update(engine): refactor Falco engine to be agnostic to specific event sources

@poiana
Copy link
Contributor

poiana commented Aug 26, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

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

@mstemm
Copy link
Contributor Author

mstemm commented Aug 27, 2021

BTW, this depends on the following falcosecurity/libs PRs:

Once they're merged, the tests should start passing (or at least falco will build properly)

@leogr leogr changed the title Gen events falco engine wip: Gen events falco engine Sep 21, 2021
@leogr leogr added this to the 0.31.0 milestone Sep 28, 2021
@mstemm mstemm force-pushed the gen-events-falco-engine branch from 50932b3 to 8241683 Compare October 4, 2021 16:37
@mstemm mstemm force-pushed the gen-events-falco-engine branch from b3d3676 to 442ac79 Compare October 4, 2021 18:39
@mstemm mstemm force-pushed the gen-events-falco-engine branch 4 times, most recently from f6d1c32 to e7d51dd Compare October 5, 2021 03:17
@mstemm mstemm force-pushed the gen-events-falco-engine branch from e02a7e6 to 18f60d2 Compare October 5, 2021 04:33
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 <[email protected]>
mstemm added 5 commits October 7, 2021 16:14
Use the new falco engine interface with support for generic events
instead of event-specific process_xxx_event methods.

Signed-off-by: Mark Stemm <[email protected]>
Add a function is_defined_field(source, fldname) that returns whether
a field with name fldname exists for the given event source. This uses
the filter factory to create a filtercheck, and returns true if an
object was created.

This prevents having to push down the entire set of defined fields
before calling load_rules().

Signed-off-by: Mark Stemm <[email protected]>
Update the lua side of rule loading to reflect other changes:

- install_filter renamed to create_filter_obj, and takes just a
  lua_parser object created via falco_rules.create_lua_parser() and
  uses a single lua callback "filter" instead of separate ones for
  syscall/k8s_audit. It can return an error, including about
  undefined fields

- is_defined_filter, which used to be local and based on the result of
  sinsp_rule_utils.check_for_ignored_syscalls_events, is now a
  lua_callback falco_rules.is_defined_field().

- Don't need to pass down sinsp_lua_parser/json_lua_parser now,
  creating filters is handled via lua callbacks.

- Checking for ignored syscalls/events is now done in falco itself,
  after loading rules.

- add_xxx_filter replaced by add_filter + source.

- Use is_format_valid instead of formats.formatter/formats.free_formatter.

- We don't need the functions in sinsp_rule_utils any longer, so
  remove the file and don't import it.

Signed-off-by: Mark Stemm <[email protected]>
This allows for working with the default ruleset like other methods.

Signed-off-by: Mark Stemm <[email protected]>
This step used to be done in the lua rule loading code, but now we can
get it directly from the filters, so do it in falco instead.

Signed-off-by: Mark Stemm <[email protected]>
@mstemm mstemm force-pushed the gen-events-falco-engine branch from dbf2bf5 to de7dc73 Compare October 7, 2021 23:14
This has recent changes to support more general purpose event
formatting.

Signed-off-by: Mark Stemm <[email protected]>
This makes the output of --list a bit more precise to only include
filter fields and not output fields.

Signed-off-by: Mark Stemm <[email protected]>
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 <[email protected]>
It turns out that the macro inbound_outbound had a logical bug where
joining the beginning and end of the macro with "or" led to the macro
matching all event types by accident.

Most of the time this isn't harmful but it turns out some trace files
will do operations on inet connection fds like "dup", and those get
mistakenly picked up by this macro, as the fd for the event does
happen to be a network connection fd.

This fixes the macro to only match those event types *and* when the fd
is a inet connection fd.

Signed-off-by: Mark Stemm <[email protected]>
@mstemm mstemm force-pushed the gen-events-falco-engine branch from de7dc73 to 71d9fd0 Compare October 11, 2021 18:57
fntlnz
fntlnz previously approved these changes Oct 11, 2021
@poiana
Copy link
Contributor

poiana commented Oct 11, 2021

LGTM label has been added.

Git tree hash: 8c5864baf699484ebeac6f8d837e046f4e62d169

leodido
leodido previously approved these changes Oct 11, 2021
The trace file traces-positive/run-shell-untrusted.scap has an old
execve event number (PPME_SYSCALL_EXECVE_18), which was replaced by
PPME_SYSCALL_EXECVE_19 in 2018.

Given the changes in falcosecurity/libs#94,
these events are now skipped. So change the test to note that *no*
events will be detected.

As a bit of context, event numbers won't be changing any longer--a
change around the same time 298fbde8029020ce3fbddd07e2910b59cc402b8b
allowed for extending existing events to add new parameters instead of
having to define a new event number just to add a new parameter. So
the notion of "old events" should not exist for any event created
after mid-to-late 2018.

Signed-off-by: Mark Stemm <[email protected]>
@mstemm mstemm dismissed stale reviews from leodido and fntlnz via 22c8c28 October 11, 2021 23:31
@poiana poiana removed the lgtm label Oct 11, 2021
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

LGTM
Thank you!

@poiana
Copy link
Contributor

poiana commented Oct 12, 2021

LGTM label has been added.

Git tree hash: 64b1a1f77376c33125d9d368fac5af85e06c3083

@poiana poiana merged commit f7893fb into master Oct 12, 2021
@poiana poiana deleted the gen-events-falco-engine branch October 12, 2021 15:59
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.

5 participants