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

fix: derived event not triggering if base filtered #3280

Merged

Conversation

josedonizetti
Copy link
Collaborator

@josedonizetti josedonizetti commented Jun 28, 2023

1. Explain what the PR does

Fix #3279

The bug happens because we pass a different event to matchPolicies inside the for, instead of passing the event that goes down the pipeline, so the logic of changing the bitmap is lost, because it happened on different events.

2. Explain how to test it

name: pol1
description: general policy
scope:
  - global
defaultActions: 
  - log
rules:
  - event: cgroup_mkdir
    filters:
      - args.cgroup_path!=/system.slice*
  - event: container_create
./dist/tracee -p policy.yaml
docker run ubuntu:latest ls

The container_created should be triggered but not the cgroup_mkdir.

3. Other comments

Copy link
Collaborator

@yanivagman yanivagman left a comment

Choose a reason for hiding this comment

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

nice catch!
Please cherry-pick to v0.16.0 branch as well

@@ -519,7 +519,7 @@ func (t *Tracee) deriveEvents(ctx context.Context, in <-chan *trace.Event) (
case events.PrintMemDump:
default:
// Derived events might need filtering as well
if t.matchPolicies(&derivative) == 0 {
if t.matchPolicies(&derivatives[i]) == 0 {
Copy link
Member

@geyslan geyslan Jun 28, 2023

Choose a reason for hiding this comment

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

Good catch.

What about to remove the copied var usage above (derivative) and make all accesses via index to avoid further errors like that?

				for i := range derivatives {
					// Skip events that dont work with filtering due to missing types being handled.
					// https://github.com/aquasecurity/tracee/issues/2486
switch events.ID(derivatives[i].EventID) {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good idea. This will also avoid the extra copy of creating derivative instance in the loop

Copy link
Member

Choose a reason for hiding this comment

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

Good catch.

What about to remove the copied var usage above (derivative) and make all accesses via index to avoid further errors like that?

				for i := range derivatives {
					// Skip events that dont work with filtering due to missing types being handled.
					// https://github.com/aquasecurity/tracee/issues/2486
switch events.ID(derivatives[i].EventID) {

And the code suggestion was just to get us on the same page. Please be my guest on code style or naming, nits, anything 😄.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@geyslan can you review the change pls?

@josedonizetti josedonizetti force-pushed the fix-derived-filtered-base-event branch from 7a58e77 to 538488a Compare June 28, 2023 14:20
@josedonizetti josedonizetti merged commit 00f71ab into aquasecurity:main Jun 28, 2023
25 checks passed
@josedonizetti josedonizetti deleted the fix-derived-filtered-base-event branch June 28, 2023 18:04
@josedonizetti josedonizetti modified the milestone: v0.16.0 Jun 29, 2023
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.

derived events not triggering for filtered base events
4 participants