fix: filter dispatching to signatures #3729
Conversation
|
@josedonizetti I will let you review this one. I would not merge until E2E tests are cleared though. |
Agreed, also want to clear with E2E, but seems like they are currently broken. |
NDStrahilevitz
force-pushed
the
fix_signature_tracing_3
branch
from
e531259 to
1840ee1
Compare
That is exactly why I said that. Not because this change. |
Meant that I want to also specifically pre-run this on the E2E before merging, not just (hopefully) have it green post merge. Anyway we agree :p |
I agree to you agreeing with my agreement. Thanks. |
|
Related to this change ? https://github.com/aquasecurity/tracee/actions/runs/7090197920/job/19296758500?pr=3729#step:4:2873 |
Seems possible since I did a small change with event dependencies, I need to check. Didn't bother checking for now since I was occupied with other things and e2es were still red. |
NO rush on my end! Take your time and thanks. |
When creating event definitions for loaded signatures, the code would attempt to load all selected events from the core definitions. We only support core event dependencies right now, but when we separate event groups, the "Source" field is the likely candidate to indicate the difference. As such, for now we should only create a dependency if the selector declares "tracee" as its source.
NDStrahilevitz
force-pushed
the
fix_signature_tracing_3
branch
from
d4a2315 to
5e1180f
Compare
NDStrahilevitz
changed the title
pkg/signatures/engine/engine.go
Outdated
| Enabled bool // Enables the signatures engine to run in the events pipeline | ||
| SigNameToEventID map[string]int32 // Cache of loaded signature event names to event ids, used to filter in dispatching | ||
| // Callback from tracee to determine if event should be dispatched to signature. | ||
| // This is done as a callback becaues importing the events package breaks compilation. |
There was a problem hiding this comment.
It brakes compilation because of tracee-rules, right ?
# github.com/aquasecurity/tracee/cmd/tracee-rules
/usr/lib/go/pkg/tool/linux_amd64/link: running clang failed: exit status 1
/bin/ld: /tmp/go-link-2441279820/000028.o: in function `_cgo_a979db4ce92c_Cfunc_bpf_btf_get_fd_by_id':
/tmp/go-build/btf.cgo2.c:54:(.text+0x12): undefined reference to `bpf_btf_get_fd_by_id'
/bin/ld: /tmp/go-link-2441279820/000030.o: in function `_cgo_a979db4ce92c_Cfunc_perf_buffer__free':
/tmp/go-build/buf-perf.cgo2.c:49:(.text+0x4): undefined reference to `perf_buffer__free'
/bin/ld: /tmp/go-link-2441279820/000030.o: in function `_cgo_a979db4ce92c_Cfunc_perf_buffer__poll':
/tmp/go-build/buf-perf.cgo2.c:67:(.text+0x26): undefined reference to `perf_buffer__poll'
/bin/ld: /tmp/go-link-2441279820/000031.o: in function `_cgo_a979db4ce92c_Cfunc_ring_buffer__free':
/tmp/go-build/buf-ring.cgo2.c:49:(.text+0x4): undefined reference to `ring_buffer__free'
/bin/ld: /tmp/go-link-2441279820/000031.o: in function `_cgo_a979db4ce92c_Cfunc_ring_buffer__poll':
/tmp/go-build/buf-ring.cgo2.c:67:(.text+0x26): undefined reference to `ring_buffer__poll'
Meaning, tracee-rules doesn't link libbpf on its binary. It wouldn't be a problem for single binary, right ? Funny because this change is to fix single-binary.
- Either way I would improve this comment to mention all that (historically wise).
A callback is a creative way of picking eventStates, but when we consider the "eventStates" and the fact that we will have a policy coordinator (which @josedonizetti is working on I believe), then this is temporary. So, together with the comment I would mention that it should be changed by the policy orchestrator (forgot the name now) and maybe link the issue to it.
There was a problem hiding this comment.
Do you know what the issue id for that is? And i'll expand the comment appropriately and merge.
Up to this commit, all signatures would be loaded to the engine even if tracing them was not requested. This would consume runtime resources, and potentially cause unexpected logs from signatures which are supposodely disabled. Fix this by verifying at the signature engine if events are dispatched to currently traced signatures (as events). In the future, disabling events through the gRPC API should trigger a change in the event state reflected at this filter stage.
NDStrahilevitz
force-pushed
the
fix_signature_tracing_3
branch
from
5e1180f to
e0677e0
Compare
1. Explain what the PR does
1840ee1 fix: clean up untraced signatures
2. Explain how to test it
See the linked below issue.
3. Other comments
Fix #3656