[DRAFT] Sig on sigs #4314
[DRAFT] Sig on sigs #4314
Conversation
cmd/tracee-rules/main.go
Outdated
| @@ -4,6 +4,8 @@ import ( | |||
| "context" | |||
| "errors" | |||
| "fmt" | |||
| "github.com/aquasecurity/tracee/pkg/cmd/initialize" | |||
There was a problem hiding this comment.
This is causing the embedding issue.
There was a problem hiding this comment.
resolved by decoupling the sigs from the initialize package
cmd/tracee-rules/main.go
Outdated
| @@ -142,7 +144,7 @@ func main() { | |||
| if err != nil { | |||
| return err | |||
| } | |||
|
|
|||
| _ = initialize.CreateEventsFromSignatures(events.StartSignatureID, sigs) | |||
There was a problem hiding this comment.
To solve undefine error on undefined: embed.BPFBundleInjected, copy CreateEventsFromSignatures() to this file.
There was a problem hiding this comment.
resolved by decoupling the sigs from the initialize package
AsafEitani
force-pushed
the
sig_on_sigs
branch
from
fdb8aa5
to
894f514
Compare
AsafEitani
force-pushed
the
sig_on_sigs
branch
from
894f514
to
6e6366b
Compare
… from initialize package
AsafEitani
force-pushed
the
sig_on_sigs
branch
from
6e6366b
to
8290aa6
Compare
There was a problem hiding this comment.
Ok, if I understand it right from the code, the issue was that just as events have event dependencies which we need to ensure are loaded even if the users does not select them, so goes the case now for signatures.
Now, since I know the internal usecase for this, If I got the code right then I'm not sure that it is actually the desired behavior. The desired usecase was for a signature aggregating data on the behavior of all other signatures with certain characteristics. But this does not imply that we would want to load each such signature. Rather, it implies that it would aggregate this info based on the selected signatures, which sounds about right.
Now, I might be wrong about what the code does. Whether this is the case or not, it lacks documentation on:
- What is the resolved gap in the code (the code is meant to handle signatures as dependent on others, it didn't work because of X)
- How the gap was resolved (Signatures which are dependencies are now selected for emission - previously they were only loaded)
- More concrete documentation in the code explaining the logic, the word "processing" for example was used as a generic descriptor for a concrete action - finding dependent signatures which are available in the plugin but not selected for emitting.
Finally, please fix the style so that the tests pass.
| @@ -27,6 +29,8 @@ const ( | |||
| signatureBufferFlag = "sig-buffer" | |||
| ) | |||
|
|
|||
| var inputs engine.EventSources | |||
There was a problem hiding this comment.
What's the purpose of moving this?
There was a problem hiding this comment.
feeding the signatures as events back to the input channel.
| @@ -142,7 +144,7 @@ func main() { | |||
| if err != nil { | |||
| return err | |||
| } | |||
|
|
|||
| _ = initialize_sigs.CreateEventsFromSignatures(events.StartSignatureID, sigs) | |||
There was a problem hiding this comment.
Why is there no error check?
There was a problem hiding this comment.
copied from the one binary mode.
| return | ||
| } | ||
| default: | ||
| inputs.Tracee <- e.ToProtocol() |
There was a problem hiding this comment.
So the inputs from the previous file is now a global variable for this? What's the reasoning?
There was a problem hiding this comment.
feeding back the signatures.
|
@NDStrahilevitz So the purpose of the PR is to allow signatures to process other signatures. |
|
I'm debugging internal integration (bump) and faced this commit, it seems related to this current effort: https://github.com/aquasecurity/tracee/pull/3681/commits |
|
Turned to draft in favor of #4327 |
1. Explain what the PR does
"Replace me with
make check-proutput"2. Explain how to test it
3. Other comments