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

feat: remove default usage of parse-arguments #4331

Merged
merged 6 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ require (
github.com/Masterminds/sprig/v3 v3.2.3
github.com/aquasecurity/libbpfgo v0.7.0-libbpf-1.4.0.20240729111821-61d531acf4ca
github.com/aquasecurity/tracee/api v0.0.0-20240905132323-d1eaeef6a19f
github.com/aquasecurity/tracee/signatures/helpers v0.0.0-20240607205742-90c301111aee
github.com/aquasecurity/tracee/signatures/helpers v0.0.0-20241009193135-0b23713fa9f9
github.com/aquasecurity/tracee/types v0.0.0-20241008181102-d40bc1f81863
github.com/containerd/containerd v1.7.21
github.com/docker/docker v26.1.5+incompatible
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,8 @@ github.com/aquasecurity/libbpfgo v0.7.0-libbpf-1.4.0.20240729111821-61d531acf4ca
github.com/aquasecurity/libbpfgo v0.7.0-libbpf-1.4.0.20240729111821-61d531acf4ca/go.mod h1:UpO6kTehEgAGGKR2twztBxvzjTiLiV/cb2xmlYb+TfE=
github.com/aquasecurity/tracee/api v0.0.0-20240905132323-d1eaeef6a19f h1:O4UmMQViaaP1wKL1eXe7C6VylwrUmUB5mYM+roqnUZg=
github.com/aquasecurity/tracee/api v0.0.0-20240905132323-d1eaeef6a19f/go.mod h1:Gn6xVkaBkVe1pOQ0++uuHl+lMMClv0TPY8mCQ6j88aA=
github.com/aquasecurity/tracee/signatures/helpers v0.0.0-20240607205742-90c301111aee h1:1KJy6Z2bSpmKQVPShU7hhbXgGVOgMwvzf9rjoWMTYEg=
github.com/aquasecurity/tracee/signatures/helpers v0.0.0-20240607205742-90c301111aee/go.mod h1:SX08YRCsPFh8CvCvzkV8FSn1sqWAarNVEJq9RSZoF/8=
github.com/aquasecurity/tracee/signatures/helpers v0.0.0-20241009193135-0b23713fa9f9 h1:sB84YYSDgUAYNSonXeMPweaN6dviCld8UNqcKDn1jBM=
github.com/aquasecurity/tracee/signatures/helpers v0.0.0-20241009193135-0b23713fa9f9/go.mod h1:/eGxScU8+vnxYhchZ72Y0lv1HqTSooLvtGCt9x7450I=
github.com/aquasecurity/tracee/types v0.0.0-20241008181102-d40bc1f81863 h1:domVTTQICTuCvX+ZW5EjvdUBz8EH7FedBj5lRqwpgf4=
github.com/aquasecurity/tracee/types v0.0.0-20241008181102-d40bc1f81863/go.mod h1:Jwh9OOuiMHXDoGQY12N9ls5YB+j1FlRcXvFMvh1CmIU=
github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0 h1:jfIu9sQUG6Ig+0+Ap1h4unLjW6YQJpKZVmUzxsD4E/Q=
Expand Down
3 changes: 0 additions & 3 deletions pkg/cmd/cobra/cobra.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,9 +331,6 @@ func GetTraceeRunner(c *cobra.Command, version string) (cmd.Runner, error) {
runner.Printer = p
runner.InstallPath = traceeInstallPath

// parse arguments must be enabled if the rule engine is part of the pipeline
runner.TraceeConfig.Output.ParseArguments = true

runner.TraceeConfig.EngineConfig = engine.Config{
Enabled: true,
SigNameToEventID: sigNameToEventId,
Expand Down
24 changes: 13 additions & 11 deletions pkg/ebpf/events_pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/binary"
"slices"
"strconv"
"sync"
"unsafe"
Expand Down Expand Up @@ -536,10 +537,13 @@ func (t *Tracee) deriveEvents(ctx context.Context, in <-chan *trace.Event) (
// acting on the derived event.

eventCopy := *event
// shallow clone the event arguments (new slice is created) before deriving the copy,
// to ensure the original event arguments are not modified by the derivation stage.
argsCopy := slices.Clone(event.Args)
out <- event

// Note: event is being derived before any of its args are parsed.
derivatives, errors := t.eventDerivations.DeriveEvent(eventCopy)
derivatives, errors := t.eventDerivations.DeriveEvent(eventCopy, argsCopy)

for _, err := range errors {
t.handleError(err)
Expand Down Expand Up @@ -614,8 +618,8 @@ func (t *Tracee) sinkEvents(ctx context.Context, in <-chan *trace.Event) <-chan
// Populate the event with the names of the matched policies.
event.MatchedPolicies = t.policyManager.MatchedNames(event.MatchedPoliciesUser)

// Parse args here if the rule engine is not enabled (parsed there if it is).
if !t.config.EngineConfig.Enabled {
// Parse args here if the rule engine is NOT enabled (parsed there if it is).
if t.config.Output.ParseArguments && !t.config.EngineConfig.Enabled {
Comment on lines +621 to +622
Copy link
Collaborator

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: #4368

err := t.parseArguments(event)
if err != nil {
t.handleError(err)
Expand Down Expand Up @@ -727,15 +731,13 @@ func (t *Tracee) handleError(err error) {
// cmd/tracee-rules), it happens on the "sink" stage of the pipeline (close to the
// printers).
func (t *Tracee) parseArguments(e *trace.Event) error {
if t.config.Output.ParseArguments {
err := events.ParseArgs(e)
if err != nil {
return errfmt.WrapError(err)
}
err := events.ParseArgs(e)
if err != nil {
return errfmt.WrapError(err)
}

if t.config.Output.ParseArgumentsFDs {
return events.ParseArgsFDs(e, uint64(e.Timestamp), t.FDArgPathMap)
}
if t.config.Output.ParseArgumentsFDs {
return events.ParseArgsFDs(e, uint64(e.Timestamp), t.FDArgPathMap)
}

return nil
Expand Down
66 changes: 38 additions & 28 deletions pkg/ebpf/signature_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ebpf

import (
"context"
"slices"

"github.com/aquasecurity/tracee/pkg/containers"
"github.com/aquasecurity/tracee/pkg/dnscache"
Expand Down Expand Up @@ -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)

err := t.parseArguments(event)
Comment on lines +84 to +89
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to parse the arguments here anyway?

  1. If signatures are now using raw arguments - won't this be a bug?
  2. Shouldn't we leave data parsing to the sink stage?

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The 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
}
Expand Down
7 changes: 6 additions & 1 deletion pkg/events/derive/derive.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package derive

import (
"slices"

"github.com/aquasecurity/tracee/pkg/events"
"github.com/aquasecurity/tracee/types/trace"
)
Expand Down Expand Up @@ -41,12 +43,15 @@ func (t Table) Register(deriveFrom, deriveTo events.ID, deriveCondition func() b
}

// DeriveEvent takes a trace.Event and checks if it can derive additional events from it as defined by a derivationTable.
func (t Table) DeriveEvent(event trace.Event) ([]trace.Event, []error) {
func (t Table) DeriveEvent(event trace.Event, origArgs []trace.Argument) ([]trace.Event, []error) {
derivatives := []trace.Event{}
errors := []error{}
deriveFns := t[events.ID(event.EventID)]
for id, deriveFn := range deriveFns {
if deriveFn.Enabled() {
// at each derivation, we need use a copy of the original arguments,
// since they might be modified by a previous derivation.
event.Args = slices.Clone(origArgs)
derivative, errs := deriveFn.DeriveFunction(event)
for _, err := range errs {
errors = append(errors, deriveError(id, err))
Expand Down
4 changes: 3 additions & 1 deletion pkg/events/derive/derive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package derive

import (
"fmt"
"slices"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -74,7 +75,8 @@ func Test_DeriveEvent(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

derived, errors := mockDerivationTable.DeriveEvent(tc.event)
argsCopy := slices.Clone(tc.event.Args)
derived, errors := mockDerivationTable.DeriveEvent(tc.event, argsCopy)
assert.Equal(t, tc.expectedDerived, derived)
assert.Equal(t, tc.expectedErrors, errors)
})
Expand Down
25 changes: 18 additions & 7 deletions pkg/signatures/benchmark/signature/golang/anti_debugging.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package golang
import (
"fmt"

"github.com/aquasecurity/tracee/pkg/events/parsers"
"github.com/aquasecurity/tracee/signatures/helpers"
"github.com/aquasecurity/tracee/types/detect"
"github.com/aquasecurity/tracee/types/protocol"
Expand All @@ -12,6 +13,7 @@ import (
type antiDebugging struct {
cb detect.SignatureHandler
metadata detect.SignatureMetadata
logger detect.Logger
}

func NewAntiDebuggingSignature() (detect.Signature, error) {
Expand All @@ -30,6 +32,7 @@ func NewAntiDebuggingSignature() (detect.Signature, error) {

func (sig *antiDebugging) Init(ctx detect.SignatureContext) error {
sig.cb = ctx.Callback
sig.logger = ctx.Logger
return nil
}

Expand All @@ -52,22 +55,30 @@ func (sig *antiDebugging) OnEvent(event protocol.Event) error {
if ee.EventName != "ptrace" {
return nil
}
request, err := helpers.GetTraceeArgumentByName(ee, "request", helpers.GetArgOps{DefaultArgs: false})
requestArg, err := helpers.GetTraceeIntArgumentByName(ee, "request")
if err != nil {
return err
}
requestString, ok := request.Value.(string)
if !ok {
return fmt.Errorf("failed to cast request's value")
}
if requestString != "PTRACE_TRACEME" {

if uint64(requestArg) != parsers.PTRACE_TRACEME.Value() {
return nil
}

var ptraceRequestData string
requestString, err := parsers.ParsePtraceRequestArgument(uint64(requestArg))

if err != nil {
ptraceRequestData = fmt.Sprint(requestArg)
sig.logger.Debugw("anti_debugging sig: failed to parse ptrace request argument: %v", err)
} else {
ptraceRequestData = requestString.String()
}

sig.cb(&detect.Finding{
SigMetadata: sig.metadata,
Event: event,
Data: map[string]interface{}{
"ptrace request": requestString,
"ptrace request": ptraceRequestData,
},
})
return nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/signatures/benchmark/signature/golang/code_injection.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ func (sig *codeInjection) OnEvent(event protocol.Event) error {
}
switch ee.EventName {
case "open", "openat":
flags, err := helpers.GetTraceeArgumentByName(ee, "flags", helpers.GetArgOps{DefaultArgs: false})
flags, err := helpers.GetTraceeIntArgumentByName(ee, "flags")
if err != nil {
return fmt.Errorf("%v %#v", err, ee)
}
if helpers.IsFileWrite(flags.Value.(string)) {
if helpers.IsFileWrite(flags) {
pathname, err := helpers.GetTraceeArgumentByName(ee, "pathname", helpers.GetArgOps{DefaultArgs: false})
if err != nil {
return err
Expand Down
7 changes: 7 additions & 0 deletions pkg/signatures/regosig/aio.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/open-policy-agent/opa/compile"
"github.com/open-policy-agent/opa/rego"

"github.com/aquasecurity/tracee/pkg/events"
"github.com/aquasecurity/tracee/types/detect"
"github.com/aquasecurity/tracee/types/protocol"
"github.com/aquasecurity/tracee/types/trace"
Expand Down Expand Up @@ -194,6 +195,12 @@ func (a *aio) OnEvent(event protocol.Event) error {
if !ok {
return fmt.Errorf("failed to cast event's payload")
}

err := events.ParseArgs(&ee)
if err != nil {
return fmt.Errorf("rego aio: failed to parse event data: %v", err)
}

input := rego.EvalInput(ee)

ctx := context.TODO()
Expand Down
15 changes: 14 additions & 1 deletion pkg/signatures/regosig/traceerego.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import (
"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/rego"

"github.com/aquasecurity/tracee/pkg/events"
"github.com/aquasecurity/tracee/types/detect"
"github.com/aquasecurity/tracee/types/protocol"
"github.com/aquasecurity/tracee/types/trace"
)

// RegoSignature is an abstract signature that is implemented in rego
Expand Down Expand Up @@ -158,7 +160,18 @@ func (sig *RegoSignature) getSelectedEvents(pkgName string) ([]detect.SignatureE
// if bool is "returned", a true evaluation will generate a Finding with no data
// if document is "returned", any non-empty evaluation will generate a Finding with the document as the Finding's "Data"
func (sig *RegoSignature) OnEvent(event protocol.Event) error {
input := rego.EvalInput(event.Payload)
ee, ok := event.Payload.(trace.Event)

if !ok {
return fmt.Errorf("failed to cast event's payload")
}

err := events.ParseArgs(&ee)
if err != nil {
return fmt.Errorf("rego aio: failed to parse event data: %v", err)
}

input := rego.EvalInput(ee)
results, err := sig.matchPQ.Eval(context.TODO(), input)
if err != nil {
return fmt.Errorf("evaluating rego: %w", err)
Expand Down
1 change: 1 addition & 0 deletions pkg/signatures/regosig/traceerego_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ func OnEventSpec(t *testing.T, target string, partial bool) {
Payload: "just some stuff",
},
finding: nil,
error: "failed to cast event's payload",
},
}

Expand Down
7 changes: 4 additions & 3 deletions signatures/golang/anti_debugging_ptraceme.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"fmt"

"github.com/aquasecurity/tracee/pkg/events/parsers"
"github.com/aquasecurity/tracee/signatures/helpers"
"github.com/aquasecurity/tracee/types/detect"
"github.com/aquasecurity/tracee/types/protocol"
Expand All @@ -11,12 +12,12 @@ import (

type AntiDebuggingPtraceme struct {
cb detect.SignatureHandler
ptraceTraceMe string
ptraceTraceMe int
}

func (sig *AntiDebuggingPtraceme) Init(ctx detect.SignatureContext) error {
sig.cb = ctx.Callback
sig.ptraceTraceMe = "PTRACE_TRACEME"
sig.ptraceTraceMe = int(parsers.PTRACE_TRACEME.Value())
return nil
}

Expand Down Expand Up @@ -52,7 +53,7 @@ func (sig *AntiDebuggingPtraceme) OnEvent(event protocol.Event) error {

switch eventObj.EventName {
case "ptrace":
requestArg, err := helpers.GetTraceeStringArgumentByName(eventObj, "request")
requestArg, err := helpers.GetTraceeIntArgumentByName(eventObj, "request")
if err != nil {
return err
}
Expand Down
Loading
Loading