Events and Scope flags #3262

geyslan · 2023-06-23T19:08:54Z

1. Explain what the PR does

3429c8e feat(cmd/flags)!: split filter flag _{(2023/jun/29) Geyslan Gregório <geyslan@gmail.com>}

This commit splits the '--filter' flag into two new flags: '--scope' and
'--events'.

The '--scope' flag is used to specify the scope of the events to be
captured like 'comm', 'binary', 'pid', 'uid', 'mntns', 'pidns', 'uts',
'tree', 'follow' and 'container', while the '--events' flag is used to
specify the events and their userland filters like 'args', 'retval' and
'context'.

It also removes the set option previously available for the '--filter'
flag, as it is no longer needed. The '--events' flag is now used to
specify the sets of events to be captured, e.g. '--events fs'.

Other chores:

- analyze '--event' flag now is '--events', to be consistent.
- polcy_file.go validateContext() is removed since we have similar
  validation in further stages.
- Relocates unit tests from flags_test.go to respective new correlated
  files, making it easier to maintain and extend and changes integration
  tests to use PolicyFile instead of Policy.

BREAKING CHANGE: '--filter' flag is now replaced by '--scope' and
'--events' flags and analyze '--event' flag is now '--events'.

Co-authored-by: Yaniv Agman <yanivagman@gmail.com>

2. Explain how to test it

See documentation changes.

3. Other comments

This is the continuation of #3100.

geyslan · 2023-06-29T21:47:45Z

pkg/cmd/flags/filter.go

-  --filter openat.context.processName=ls                        | only trace 'openat' events that have 'processName' equal to 'ls'
-  --filter security_file_open.context.container                 | only trace 'security_file_open' events coming from a container
-  --filter comm=bash --filter follow                            | trace all events that originated from bash or from one of the processes spawned by bash
+Scope examples:


@yanivagman I kept both --help event and --help scope showing the same help message since they have a lot in common (operators info above). But let me know if we must split it as well.

Event flag only has a subset of the operators, and can have its own help message.
Yet I think it's ok to keep it like that for now.
We can address this in a future PR

docs/docs/config/overview.md

docs/docs/events/builtin/extra/hooked_syscalls.md

docs/docs/filters/filtering.md

pkg/cmd/flags/flags.go

pkg/ebpf/c/common/filtering.h

pkg/ebpf/events_pipeline.go

pkg/policy/policy_file.go

tests/e2e-inst-test.sh

pkg/cmd/flags/flags.go

pkg/cmd/flags/policy.go

This commit splits the '--filter' flag into two new flags: '--scope' and '--events'. The '--scope' flag is used to specify the scope of the events to be captured like 'comm', 'binary', 'pid', 'uid', 'mntns', 'pidns', 'uts', 'tree', 'follow' and 'container', while the '--events' flag is used to specify the events and their userland filters like 'args', 'retval' and 'context'. It also removes the set option previously available for the '--filter' flag, as it is no longer needed. The '--events' flag is now used to specify the sets of events to be captured, e.g. '--events fs'. Other chores: - analyze '--event' flag now is '--events', to be consistent. - polcy_file.go validateContext() is removed since we have similar validation in further stages. - Relocates unit tests from flags_test.go to respective new correlated files, making it easier to maintain and extend and changes integration tests to use PolicyFile instead of Policy. BREAKING CHANGE: '--filter' flag is now replaced by '--scope' and '--events' flags and analyze '--event' flag is now '--events'. Co-authored-by: Yaniv Agman <yanivagman@gmail.com>

geyslan · 2023-07-10T16:49:44Z

cmd/tracee/cmd/analyze.go

+	// TODO: decide if we want to bind this flag to viper, since we already have a similar
+	// flag in rootCmd, conflicting with each other.
+	// The same goes for the other flags (signatures-dir, rego), also in rootCmd.
+	//
+	// err := viper.BindPFlag("events", analyze.Flags().Lookup("events"))
+	// if err != nil {
+	// 	fmt.Fprintf(os.Stderr, "Error: %s\n", err)
+	// 	os.Exit(1)
+	// }


I've detected viper flags conflicts between rootCmd and analyze. The same viper instance is being used, which means that the settings have to be defined in the global config file. But rootCmd events (former filter) is a flag designedly not bound to the global config file as we can see below.

But there are other conflicts like signatures-dir and rego, both already bound in the rootCmd.

I suppose that analyze events flag is not a global config flag, right? So, probably we can remove these mentioned bindings in analyze. WDYT @josedonizetti?

@geyslan I believe this is related: spf13/viper#233

@geyslan I believe this is related: spf13/viper#233

Thanks for the reference. A possible solution, if we decide to bind these flags to global config file, is to do it in PreRun(), so it won't override as in init(): spf13/viper#233 (comment)

For now, leaving them commented.

geyslan · 2023-07-10T16:52:03Z

@yanivagman ready for review.

yanivagman

Some nit comments, but otherwise LGTM

docs/docs/config/overview.md

docs/docs/events/custom/analyze.md

docs/docs/filters/filtering.md

pkg/cmd/flags/filter.go

Co-authored-by: Yaniv Agman <yanivagman@gmail.com>

pkg/cmd/flags/filter.go

geyslan · 2023-07-12T10:36:03Z

@yanivagman @rafaeldtinoco @josedonizetti, just a note to, when merging this, squash the commits leaving the message of the main one as it is (the other commits were just review suggestions accepted directly in github).

docs/docs/config/overview.md

docs/docs/filters/filtering.md

geyslan · 2023-07-12T12:35:36Z

@yanivagman @rafaeldtinoco @josedonizetti, just a note to, when merging this, squash the commits leaving the message of the main one as it is (the other commits were just review suggestions accepted directly in github).

Synced offline and merging it.

After the changes of aquasecurity#3262, at this stage, policies.Map() length is always greater than 0.

After the changes of #3262, at this stage, policies.Map() length is always greater than 0.

After the changes of aquasecurity#3262, at this stage, policies.Map() length is always greater than 0.

geyslan self-assigned this Jun 23, 2023

github-actions bot added area/flags area/testing area/UX labels Jun 23, 2023

geyslan mentioned this pull request Jun 26, 2023

Event flag #3100

Closed

rafaeldtinoco added the backport/v0.17.0 label Jun 26, 2023

josedonizetti added milestone/v0.17.0 and removed backport/v0.17.0 labels Jun 26, 2023

geyslan force-pushed the 1543-2810-event-flag branch from c85a0b7 to d615b39 Compare June 27, 2023 13:26

github-actions bot added area/ebpf area/signatures kind/documentation labels Jun 27, 2023

geyslan force-pushed the 1543-2810-event-flag branch 2 times, most recently from 1519a5f to f3621a1 Compare June 28, 2023 21:14

geyslan added the kind/feature label Jun 29, 2023

geyslan force-pushed the 1543-2810-event-flag branch 2 times, most recently from 3ee0b5e to cb2201e Compare June 29, 2023 21:39

geyslan requested a review from yanivagman June 29, 2023 21:40

geyslan marked this pull request as ready for review June 29, 2023 21:40

geyslan requested a review from josedonizetti June 29, 2023 21:41

geyslan commented Jun 29, 2023

View reviewed changes

geyslan force-pushed the 1543-2810-event-flag branch from cb2201e to 0ea668f Compare June 29, 2023 22:07

yanivagman requested changes Jul 5, 2023

View reviewed changes

geyslan force-pushed the 1543-2810-event-flag branch from 0ea668f to dfbd913 Compare July 5, 2023 23:52

geyslan commented Jul 5, 2023

View reviewed changes

pkg/cmd/flags/flags.go Show resolved Hide resolved

geyslan force-pushed the 1543-2810-event-flag branch from dfbd913 to e1f8238 Compare July 6, 2023 00:01

geyslan requested a review from yanivagman July 6, 2023 00:04

geyslan commented Jul 6, 2023

View reviewed changes

pkg/cmd/flags/policy.go Show resolved Hide resolved

geyslan changed the title ~~Event flag~~ Event and Scope flags Jul 6, 2023

geyslan force-pushed the 1543-2810-event-flag branch from e1f8238 to c460974 Compare July 10, 2023 15:39

geyslan force-pushed the 1543-2810-event-flag branch from c460974 to 67e06f8 Compare July 10, 2023 15:45

geyslan force-pushed the 1543-2810-event-flag branch from 67e06f8 to 3429c8e Compare July 10, 2023 16:37

geyslan commented Jul 10, 2023

View reviewed changes

geyslan changed the title ~~Event and Scope flags~~ Events and Scope flags Jul 10, 2023

yanivagman approved these changes Jul 12, 2023

View reviewed changes

josedonizetti approved these changes Jul 12, 2023

View reviewed changes

geyslan and others added 4 commits July 12, 2023 07:31

Update docs/docs/config/overview.md

deab709

Co-authored-by: Yaniv Agman <yanivagman@gmail.com>

Update docs/docs/events/custom/analyze.md

7b35676

Co-authored-by: Yaniv Agman <yanivagman@gmail.com>

Update docs/docs/filters/filtering.md

7ed9108

Co-authored-by: Yaniv Agman <yanivagman@gmail.com>

Update pkg/cmd/flags/filter.go

8b05725

Co-authored-by: Yaniv Agman <yanivagman@gmail.com>

geyslan commented Jul 12, 2023

View reviewed changes

pkg/cmd/flags/filter.go Outdated Show resolved Hide resolved

Update pkg/cmd/flags/filter.go

5c0729e

geyslan commented Jul 12, 2023

View reviewed changes

docs/docs/config/overview.md Outdated Show resolved Hide resolved

geyslan commented Jul 12, 2023

View reviewed changes

docs/docs/filters/filtering.md Outdated Show resolved Hide resolved

Apply suggestions from code review

fe2a813

geyslan merged commit 2555190 into aquasecurity:main Jul 12, 2023
25 checks passed

This was referenced Jul 12, 2023

remove --help, fix help command for flags #3302

Closed

parse - and ! operators in respective parsers #3312

Closed

geyslan deleted the 1543-2810-event-flag branch July 31, 2023 22:18

geyslan added a commit to geyslan/tracee that referenced this pull request Oct 10, 2023

chore(flags): remove leftover

6ce8a36

After the changes of aquasecurity#3262, at this stage, policies.Map() length is always greater than 0.

geyslan added a commit to geyslan/tracee that referenced this pull request Oct 10, 2023

chore(flags): remove leftover

7bd94f6

After the changes of aquasecurity#3262, at this stage, policies.Map() length is always greater than 0.

geyslan added a commit to geyslan/tracee that referenced this pull request Oct 16, 2023

chore(flags): remove leftover

be315f5

After the changes of aquasecurity#3262, at this stage, policies.Map() length is always greater than 0.

geyslan added a commit to geyslan/tracee that referenced this pull request Oct 16, 2023

chore(flags): remove leftover

19381ad

After the changes of aquasecurity#3262, at this stage, policies.Map() length is always greater than 0.

geyslan added a commit to geyslan/tracee that referenced this pull request Oct 26, 2023

chore(flags): remove leftover

4db6a48

After the changes of aquasecurity#3262, at this stage, policies.Map() length is always greater than 0.

geyslan added a commit that referenced this pull request Oct 30, 2023

chore(flags): remove leftover

3bad75e

After the changes of #3262, at this stage, policies.Map() length is always greater than 0.

AnaisUrlichs pushed a commit to AnaisUrlichs/tracee that referenced this pull request Nov 2, 2023

chore(flags): remove leftover

2d45410

After the changes of aquasecurity#3262, at this stage, policies.Map() length is always greater than 0.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Events and Scope flags #3262

Events and Scope flags #3262

geyslan commented Jun 23, 2023 •

edited

geyslan Jun 29, 2023

yanivagman Jul 5, 2023

geyslan Jul 10, 2023

yanivagman Jul 12, 2023

geyslan Jul 12, 2023

geyslan commented Jul 10, 2023

yanivagman left a comment

geyslan commented Jul 12, 2023 •

edited

geyslan commented Jul 12, 2023

Events and Scope flags #3262

Events and Scope flags #3262

Conversation

geyslan commented Jun 23, 2023 • edited

1. Explain what the PR does

2. Explain how to test it

3. Other comments

geyslan Jun 29, 2023

Choose a reason for hiding this comment

yanivagman Jul 5, 2023

Choose a reason for hiding this comment

geyslan Jul 10, 2023

Choose a reason for hiding this comment

yanivagman Jul 12, 2023

Choose a reason for hiding this comment

geyslan Jul 12, 2023

Choose a reason for hiding this comment

geyslan commented Jul 10, 2023

yanivagman left a comment

Choose a reason for hiding this comment

geyslan commented Jul 12, 2023 • edited

geyslan commented Jul 12, 2023

geyslan commented Jun 23, 2023 •

edited

geyslan commented Jul 12, 2023 •

edited