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

Add exceptions support #1427

Merged
merged 15 commits into from
Jan 19, 2021
Merged

Add exceptions support #1427

merged 15 commits into from
Jan 19, 2021

Conversation

mstemm
Copy link
Contributor

@mstemm mstemm commented Oct 3, 2020

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:

This adds support for exceptions as rule attributes as described in #1376. Also included are a set of changes to falco_rules.yaml to use exceptions whenever possible.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new: add support for exceptions as rule attributes to provide a compact way to add exceptions to Falco rules

The format of error responses has changed to include a summary of errors
and/or warnings. This changed many test cases that were looking for
specific outputs.

Update to add counts and other minor formatting changes.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Support exceptions properties on rules as described in
#1376.

- When parsing rules, add an empty exceptions table if not specified.
- If exceptions are specified, they must contain names and lists of
  fields, and optionally can contain lists of comps and lists of lists of
  values.
- If comps are not specified, = is used.
- If a rule has exceptions and append:true, add values to the original rule's
  exception values with the matching name.
- It's a warning but not an error to have exception values with a name
  not matching any fields.

After loading all rules, build the exception condition string based on
any exceptions:

- If an exception has a single value for the "fields" property, values are
  combined into a single set to build a condition string like "field
  cmp (val1, val2, ...)".
- Otherwise, iterate through each rule's exception
  values, finding the matching field names (field1, field2, ...) and
  comp operators (cmp1, cmp2, ...), then
  iterating over the list of field values (val1a, val1b, ...), (val2a,
  val2b, ...), building up a string of the form:
    and not ((field1 cmp1 val1a and field2 cmp2 val1b and ...) or
              (field1 cmp1 val2a and field2 cmp2 val2b and ...)...
	     )"
- If a value is not already quoted and contains a space, quote it in the
  string.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Handle various positive and negative cases. Should handle every error
and warning path when reading exceptions objects or rule exception
fields, and various positive cases of using exceptions to prevent
alerts.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
When running falco with -V/valdiate <rules file>, you won't get any
event counts. All prior tests didn't get this far as they also resulted
in rules parsing errors.

However, validating can now result in warnings only. This won't exit but
won't print event counts either.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Take advantage of the changes to support exceptions and refactor rules
to use them whenever feasible:

- Define exceptions for every rule. In cases where no practical
  exception exists e.g. "K8s <obj> Created/Deleted", define an empty
  exception property just to avoid warnings when loading rules.
- Go through all rules and convert macros-used-as-exceptions that
  matched against 2-3 filter fields into exceptions. In most cases,
  switching from equality (e.g proc.name=nginx) to in (e.g. proc.name
  in (nginx)) allowed for better groupings into a smaller set of
  exception items.
- In cases where the exception had complex combinations of fields, keep
  the macro as is.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
If a list:

- list: foo
  items: [a, b, c]

Was referenced in another list:

- list: bar
  items: [foo, d, e, f]

The first list would not be marked as used, when it should.

This avoids mistaken messages like "list xxx not refered to by any rule/macro/list"

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Remove old macros/lists that aren't being used by any current rules.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
The rules loader now allows objects with unknown keys.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Fix typo, "!" should be "!=".

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
It was renamed from falco_tests.yaml.in in
5bafa19.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
These define exceptions too.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
When returning a rule_result struct, also include a set of field names
used by all exceptions for this rule. This may make building exception
values a bit easier.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Copy link
Member

@leodido leodido left a comment

Choose a reason for hiding this comment

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

LGTM

But let's keep this on hold until next week (ie., after Falco 0.27.0 is released)

@poiana
Copy link

poiana commented Jan 11, 2021

LGTM label has been added.

Git tree hash: 378709e3b88732e0c555a675780ae1322194e246

@poiana
Copy link

poiana commented Jan 11, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, leodido, leogr

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 [fntlnz,leodido,leogr]

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

@fntlnz
Copy link
Contributor

fntlnz commented Jan 19, 2021

/hold cancel

@poiana poiana merged commit 8c4040b into master Jan 19, 2021
@poiana poiana deleted the add-exceptions-support branch January 19, 2021 09:37
@mstemm mstemm mentioned this pull request Jan 26, 2021
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.

None yet

5 participants