Refactor/full analyze mode #3673
Conversation
480b866 to
d638623
Compare
|
@rafaeldtinoco @josedonizetti the current analyze mode is working (checked manually) with process tree. |
|
Optional feature - if stdin of tracee upon startup does not point to /dev/pts (normal stdin path), then run automatically A point of thought: |
|
@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. |
| @@ -38,12 +39,13 @@ func InitNamespacesEvent() trace.Event { | |||
| initNamespacesArgs := getInitNamespaceArguments() | |||
|
|
|||
| initNamespacesEvent := trace.Event{ | |||
There was a problem hiding this comment.
I suggest you turn this to a data source.
There was a problem hiding this comment.
So for now this is used as state initializer.
This info could be also saved to a datasource.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
But you will still need an event to initialize the datasource in analyze mode...
There was a problem hiding this comment.
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.
| ) | ||
|
|
||
| // EventsProducer is a type that is able to generate events | ||
| type EventsProducer interface { |
There was a problem hiding this comment.
I think the eBPF event decoder could be considered such an object no? This could be a useful abstraction.
There was a problem hiding this comment.
Yea, this was the idea :)
I tried to fit to the current design
| "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" |
| @@ -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 | |||
There was a problem hiding this comment.
Perhaps not relevant for this PR, but I see a future where this is a one-tracee-many-producers relation. @josedonizetti WDYT?
| @@ -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 { | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| @@ -820,6 +834,10 @@ func (t *Tracee) getOptionsConfig() uint32 { | |||
| cOptVal = cOptVal | optForkProcTree // tell sched_process_fork to be prolix | |||
| } | |||
|
|
|||
| if t.config.Output.ExportAnalyze { | |||
There was a problem hiding this comment.
Sounds like exportanalyze should just enable the process tree, instead of having its own logic.
There was a problem hiding this comment.
You are right :)
You are right, this is more of a POC PR. |
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. |
|
We have this update #3875 Please rebase your PR against main to make use of the new workflow setup. |
|
@AlonZivony will you be able to rebase this? |
I have a different branch for the PR, as I made some changes after the reviews. |
1. Explain what the PR does
Fix #3520
2. Explain how to test it
3. Other comments