Refactor/full analyze mode #3673

AlonZivony · 2023-11-05T16:09:54Z

1. Explain what the PR does

2. Explain how to test it

3. Other comments

…or process tree

AlonZivony · 2023-11-07T18:16:30Z

@rafaeldtinoco @josedonizetti the current analyze mode is working (checked manually) with process tree.
I need you LGTM to do the work left of tests, format and documentation.

AlonZivony · 2023-11-15T12:33:28Z

Optional feature - if stdin of tracee upon startup does not point to /dev/pts (normal stdin path), then run automatically tracee analyze.
For example:
cat tracee_output.json | ./tracee -e ld_preload will run in analyze mode on the events from tracee_output.json.
This will make the behavior similar to python for example.

A point of thought:
Do we really want analyze command?
We can just assume by the use of the --input flag that we are in analyze mode, avoiding the clumsy command line.

NDStrahilevitz · 2023-12-05T14:44:05Z

@AlonZivony When you go back to working on this, is it reasonable in your opinion to include the DNS Cache?

AlonZivony · 2023-12-06T12:39:57Z

@AlonZivony When you go back to working on this, is it reasonable in your opinion to include the DNS Cache?

This PR is not giving the analyze mode all the options of the normal mode.
You can see that I didn't support many flags of Tracee.
I am not sure about this DNS cache implementation you want, but I guess it could be possible. But not in this PR (it is big enough as it is)

NDStrahilevitz · 2024-01-24T10:26:29Z

pkg/events/usermode.go

@@ -38,12 +39,13 @@ func InitNamespacesEvent() trace.Event {
 	initNamespacesArgs := getInitNamespaceArguments()

 	initNamespacesEvent := trace.Event{


I suggest you turn this to a data source.

So for now this is used as state initializer.
This info could be also saved to a datasource.

I'm saying we make this a builtin data source, instead of having an event which initializes some shared state (which is what data sources are for). I think this will only be in v0.21 or v0.22, so this can be done as a prep PR.

But you will still need an event to initialize the datasource in analyze mode...

I had in mind using signals for things like that. In addition to what i've written below, I think --export-analyze should give a separate output of signals, which can be fed back to the control plane running in analyze.

You're right, this needs to be an event or a signal. But, it doesn't need to be an externally visible event. So initialize a data source with this info after the userspace code which would previously emit the event. If running with --export-analyze, dump it as a signal as well.

NDStrahilevitz · 2024-01-24T10:27:13Z

pkg/producer/producer.go

+)
+
+// EventsProducer is a type that is able to generate events
+type EventsProducer interface {


I think the eBPF event decoder could be considered such an object no? This could be a useful abstraction.

Yea, this was the idea :)
I tried to fit to the current design

NDStrahilevitz · 2024-01-24T10:31:50Z

pkg/cmd/flags/input.go

+	"github.com/aquasecurity/tracee/pkg/capabilities"
+	"github.com/aquasecurity/tracee/pkg/config"
+	"github.com/aquasecurity/tracee/pkg/errfmt"
+	cap2 "kernel.org/pub/linux/libs/security/libcap/cap"


NDStrahilevitz · 2024-01-24T10:34:19Z

pkg/ebpf/tracee.go

@@ -122,6 +123,8 @@ type Tracee struct {
 	streamsManager *streams.StreamsManager
 	// policyManager manages policy state
 	policyManager *policyManager
+	// producer produce events in analyze mode instead of eBPF programs
+	producer producer.EventsProducer


Perhaps not relevant for this PR, but I see a future where this is a one-tracee-many-producers relation. @josedonizetti WDYT?

NDStrahilevitz

The commits need to be organized. It also seems like this is still a WIP. Are you planning on merging this release or the next one?
Anyway, there should also be a test for the mode before merging.

NDStrahilevitz · 2024-01-24T10:36:34Z

pkg/ebpf/processor.go

@@ -118,6 +120,8 @@ func (t *Tracee) registerEventProcessors() {
 		// Convert all time relate args to nanoseconds since epoch.
 		// NOTE: Make sure to convert time related args (of your event) in here.
 		t.RegisterEventProcessor(events.SchedProcessFork, t.processSchedProcessFork)
+	}
+	if !t.config.Output.ExportAnalyze {


Why disable? This adds another layer of confusion to normalization. I'd also urge again that normalization is done as the first procession as its easier to reason about for a user writing an extension.

So we need the timestamps to match the kernel times in the analyze mode for the process tree to work there.
We can do it by either change the timestamps in the back when reading in analyze mode, or when exporting.
For now it was more convenient for this POC to export kernel times.

NDStrahilevitz · 2024-01-24T10:37:57Z

pkg/ebpf/tracee.go

@@ -820,6 +834,10 @@ func (t *Tracee) getOptionsConfig() uint32 {
 		cOptVal = cOptVal | optForkProcTree // tell sched_process_fork to be prolix
 	}

+	if t.config.Output.ExportAnalyze {


Sounds like exportanalyze should just enable the process tree, instead of having its own logic.

You are right :)

AlonZivony · 2024-01-29T12:09:32Z

The commits need to be organized. It also seems like this is still a WIP. Are you planning on merging this release or the next one? Anyway, there should also be a test for the mode before merging.

You are right, this is more of a POC PR.
I didn't want to implement the specifics before the overall idea is approved.
You can think of this as a DR stage, where I want to have approval on all the patterns, abstractions and general solution.
As you noticed, I didn't implement tests, kept many features blocked for this to work in this current stage and didn't document to much (as it is prone to change).
If I get the OK for this, we can discuss how to move on with the full implementation.

NDStrahilevitz · 2024-01-30T17:06:19Z

The commits need to be organized. It also seems like this is still a WIP. Are you planning on merging this release or the next one? Anyway, there should also be a test for the mode before merging.

You are right, this is more of a POC PR. I didn't want to implement the specifics before the overall idea is approved. You can think of this as a DR stage, where I want to have approval on all the patterns, abstractions and general solution. As you noticed, I didn't implement tests, kept many features blocked for this to work in this current stage and didn't document to much (as it is prone to change). If I get the OK for this, we can discuss how to move on with the full implementation.

The ability added in this PR is necessary for us to deprecate tracee-ebpf and tracee-rules. So this, or something similar is necessary as well. And it does seem like a useful feature to have anyway.

I think the concept of the event producer is good, and I already have some ideas on where to use it in the future.

I think it would be even better if we would make the pipeline its own object, which we can build as needed depending on the context (basically making it more declarative). I think it would make implementing this idea much nicer. I'm less sure of how easy it would be as a PR (probably not as it would be a big refactor), but I also fear it is becoming more necessary as we approach extensions/plugins and basically allowed external change of behavior in tracee.

geyslan · 2024-02-21T11:01:30Z

We have this update #3875

Please rebase your PR against main to make use of the new workflow setup.

github-actions bot assigned AlonZivony Nov 5, 2023

github-actions bot added area/ebpf area/UX area/events area/flags labels Nov 5, 2023

AlonZivony added 7 commits November 5, 2023 18:11

Working pipeline

f43a5a2

Close Tracee upon EOF

4d2b374

Small names change

befb065

Fix signatures engine possible close before drain

bf2d981

More established analyze mode, missing export of sched_process_fork f…

5954153

…or process tree

Add configuration for exporting analyze mode compatible events

78c6d37

Fix documentations

d638623

AlonZivony force-pushed the refactor/full-analyze-mode branch from 480b866 to d638623 Compare November 7, 2023 18:13

github-actions bot added area/signatures kind/documentation labels Nov 7, 2023

AlonZivony marked this pull request as ready for review November 13, 2023 14:06

rafaeldtinoco self-requested a review November 20, 2023 17:11

rafaeldtinoco requested review from yanivagman and removed request for rafaeldtinoco January 5, 2024 16:21

NDStrahilevitz reviewed Jan 24, 2024

View reviewed changes

yanivagman added the milestone/v0.22.0 label May 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor/full analyze mode #3673

Refactor/full analyze mode #3673

AlonZivony commented Nov 5, 2023

AlonZivony commented Nov 7, 2023

AlonZivony commented Nov 15, 2023

NDStrahilevitz commented Dec 5, 2023

AlonZivony commented Dec 6, 2023

NDStrahilevitz Jan 24, 2024

AlonZivony Jan 29, 2024

NDStrahilevitz Jan 30, 2024

AlonZivony Jan 30, 2024

NDStrahilevitz Jan 30, 2024

NDStrahilevitz Jan 24, 2024

AlonZivony Jan 29, 2024

NDStrahilevitz Jan 24, 2024

NDStrahilevitz Jan 24, 2024

NDStrahilevitz left a comment

NDStrahilevitz Jan 24, 2024

AlonZivony Jan 29, 2024 •

edited

NDStrahilevitz Jan 24, 2024

AlonZivony Jan 29, 2024

AlonZivony commented Jan 29, 2024

NDStrahilevitz commented Jan 30, 2024

geyslan commented Feb 21, 2024

		@@ -38,12 +39,13 @@ func InitNamespacesEvent() trace.Event {
		initNamespacesArgs := getInitNamespaceArguments()

		initNamespacesEvent := trace.Event{

Refactor/full analyze mode #3673

Are you sure you want to change the base?

Refactor/full analyze mode #3673

Conversation

AlonZivony commented Nov 5, 2023

1. Explain what the PR does

2. Explain how to test it

3. Other comments

AlonZivony commented Nov 7, 2023

AlonZivony commented Nov 15, 2023

NDStrahilevitz commented Dec 5, 2023

AlonZivony commented Dec 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NDStrahilevitz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlonZivony Jan 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlonZivony commented Jan 29, 2024

NDStrahilevitz commented Jan 30, 2024

geyslan commented Feb 21, 2024

AlonZivony Jan 29, 2024 •

edited