Add security alerts #136
Add security alerts #136
Conversation
| cfg.EventsToTrace = append(cfg.EventsToTrace, tracee.EventsNameToID["mmap_alert"]) | ||
| cfg.EventsToTrace = append(cfg.EventsToTrace, tracee.EventsNameToID["mprotect_alert"]) | ||
| } |
There was a problem hiding this comment.
I think these should be a single alert type, about memory protection. the fact that we implement it using mmap as a starting point is not important for the user.
There was a problem hiding this comment.
The user chooses "security-alerts", and doesn't have to know about mmap-alert and mprotect-alert. So, it's more flxible, and the user can ignore these event names
| @@ -28,6 +28,7 @@ const ( | |||
| ACCESS_MODE_T ArgType = 20 | |||
| PTRACE_REQ_T ArgType = 21 | |||
| PRCTL_OPT_T ArgType = 22 | |||
| ALERT_T ArgType = 23 | |||
There was a problem hiding this comment.
I don't think we should use one arg type for all security alerts. it depends on my other comment about joining both memory alerts into one, if we do that then we can call this MPROT_ALERT_T or something similar
There was a problem hiding this comment.
Why adding too many argument types? The alert_t only encodes some message in most of the cases, and I don't see a reason now to create new special alerts
tracee/event_monitor_ebpf.c
Outdated
| // Workaround to a bug in gobpf, where a kprobe event (e.g. mmap syscall) can't be attached to two different functions. | ||
| // As we need to use save_args in kprobe entry, but one of the events (functions) might be chosen and the other not, | ||
| // We need a mechanism that can tell if an event was chosen by the user, so we can save the args without enabling both events. | ||
| if (!event_chosen(id)) | ||
| return 0; | ||
|
|
There was a problem hiding this comment.
can you please also open an issue in tracee to explain this workaround as a reminder for us to remove this in the future? I think you will explain it better than me
There was a problem hiding this comment.
Actually when thinking about it - this workaround is actually not a bad thing if it will stay.
It is a good idea to have a mechanism that sends the event only if the user requested for it.
Maybe we should just remove the "workaround" comment? WDYT?
yanivagman
force-pushed
the
add_security_alerts
branch
2 times, most recently
from
2e00354
to
7f4ae59
Compare
yanivagman
force-pushed
the
add_security_alerts
branch
from
7f4ae59
to
a712294
Compare
No description provided.