-
Notifications
You must be signed in to change notification settings - Fork 433
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
feat: remove default usage of parse-arguments #4331
Changes from all commits
ddc219a
ec478cc
0fce183
3cfe5c5
cb48e1f
47451f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package ebpf | |
|
||
import ( | ||
"context" | ||
"slices" | ||
|
||
"github.com/aquasecurity/tracee/pkg/containers" | ||
"github.com/aquasecurity/tracee/pkg/dnscache" | ||
|
@@ -52,51 +53,60 @@ func (t *Tracee) engineEvents(ctx context.Context, in <-chan *trace.Event) (<-ch | |
|
||
go t.sigEngine.Start(ctx) | ||
|
||
// Create a function for feeding the engine with an event | ||
feedFunc := func(event *trace.Event) { | ||
if event == nil { | ||
return // might happen during initialization (ctrl+c seg faults) | ||
} | ||
// TODO: in the upcoming releases, the rule engine should be changed to receive trace.Event, | ||
// and return a trace.Event, which should remove the necessity of converting trace.Event to protocol.Event, | ||
// and converting detect.Finding into trace.Event | ||
|
||
id := events.ID(event.EventID) | ||
go func() { | ||
defer close(out) | ||
defer close(errc) | ||
defer close(engineInput) | ||
defer close(engineOutput) | ||
|
||
// feedEngine feeds an event to the rules engine | ||
feedEngine := func(event *trace.Event) { | ||
if event == nil { | ||
return // might happen during initialization (ctrl+c seg faults) | ||
} | ||
|
||
id := events.ID(event.EventID) | ||
|
||
// if the event is marked as submit, we pass it to the engine | ||
if t.policyManager.IsEventToSubmit(id) { | ||
err := t.parseArguments(event) | ||
if err != nil { | ||
t.handleError(err) | ||
// if the event is NOT marked as submit, it is not sent to the rules engine | ||
if !t.policyManager.IsEventToSubmit(id) { | ||
return | ||
} | ||
|
||
// Get a copy of our event before sending it down the pipeline. | ||
// This is needed because a later modification of the event (in | ||
// particular of the matched policies) can affect engine stage. | ||
// Get a copy of event before parsing it or sending it down the pipeline. | ||
// This is needed because a later modification of the event (matched policies or | ||
// arguments parsing) can affect engine stage. | ||
eventCopy := *event | ||
|
||
if t.config.Output.ParseArguments { | ||
// shallow clone the event arguments before parsing them (new slice is created), | ||
// to keep the eventCopy with raw arguments. | ||
eventCopy.Args = slices.Clone(event.Args) | ||
geyslan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
err := t.parseArguments(event) | ||
Comment on lines
+84
to
+89
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to parse the arguments here anyway?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with you, but we still need it here until the internal migration is finalised. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have a "todo" opened so we won't forget to remove it once finalised? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: #4368 |
||
if err != nil { | ||
t.handleError(err) | ||
return | ||
} | ||
} | ||
|
||
// pass the event to the sink stage, if the event is also marked as emit | ||
// it will be sent to print by the sink stage | ||
out <- event | ||
|
||
// send the event to the rule event | ||
// send the copied event to the rules engine | ||
engineInput <- eventCopy.ToProtocol() | ||
} | ||
} | ||
|
||
// TODO: in the upcoming releases, the rule engine should be changed to receive trace.Event, | ||
// and return a trace.Event, which should remove the necessity of converting trace.Event to protocol.Event, | ||
// and converting detect.Finding into trace.Event | ||
|
||
go func() { | ||
defer close(out) | ||
defer close(errc) | ||
defer close(engineInput) | ||
defer close(engineOutput) | ||
|
||
for { | ||
select { | ||
case event := <-in: | ||
feedFunc(event) | ||
feedEngine(event) | ||
case event := <-engineOutputEvents: | ||
feedFunc(event) | ||
feedEngine(event) | ||
case <-ctx.Done(): | ||
return | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this still hold?
If I understand correctly, parse arguments is no longer "always on" when the rule engine is enabled.
So shouldn't the condition be
if t.config.Output.ParseArguments
only?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: #4368