Change inotify_watch event to security_path_notify #3913
Conversation
inotify_watch used the inotify_find_inode kernel function which is static and cannot be hooked on all kernels. This replacement event hooks security_path_notify instead (using a kprobe), which is always available. This function also handles notify requests from dnotify and fanotify, which are used for similar purposes. The new event also includes the mask and object type fields, which can be used to determine the exact characteristics of the watch.
|
|
pkg/events/core.go
Outdated
| params: []trace.ArgMeta{ | ||
| {Type: "const char*", Name: "pathname"}, | ||
| {Type: "unsigned long", Name: "inode"}, | ||
| {Type: "dev_t", Name: "dev"}, | ||
| {Type: "u64", Name: "mask"}, |
There was a problem hiding this comment.
You will need to support parsing this value, as it is a bit mask.
This is done in libbpfgo, you can have a look on a PR doing it, like the GUP flags parsing
pkg/events/core.go
Outdated
| @@ -12837,22 +12837,23 @@ var CoreEvents = map[ID]Definition{ | |||
| }, | |||
| }, | |||
| }, | |||
| InotifyWatch: { | |||
| id: InotifyWatch, | |||
| SecurityPathNotify: { | |||
There was a problem hiding this comment.
You will need to add a documentation page for the event.
The documentation pages reside in the "docs/docs/events/builtin/extra" directory.
Just copy the format.md file and fill it.
| } | ||
|
|
||
| int main(void) { | ||
| mkdir_exist_ok(DNOTIFY_PATH); |
There was a problem hiding this comment.
Make sure to clean afterwards
|
@yanivagman Are we ok with straight up replacing the event and breaking users? I think we should start adding new events and deprecating older versions if needed instead as a practice. |
You are probably right. |
I added back the old event. How should it be marked as deprecated? |
|
Since we're still in v0.x.y and the event is very specific and still new, I'm ok with removing it. In the future we should add the mechanism to mark events as deprecated |
security_path_notify is now a separate event for backwards compatibility.
oshaked1
force-pushed
the
fsnotify_event
branch
from
6129e80 to
b84c03e
Compare
| case SecurityPathNotify: | ||
| if maskArg := GetArg(event, "mask"); maskArg != nil { | ||
| if mask, isUint64 := maskArg.Value.(uint64); isUint64 { | ||
| fsNotifyMaskArgument := helpers.ParseFsNotifyMask(mask) | ||
| parseOrEmptyString(maskArg, fsNotifyMaskArgument, nil) | ||
| } | ||
| } | ||
| if objTypeArg := GetArg(event, "obj_type"); objTypeArg != nil { | ||
| if objType, isUint := objTypeArg.Value.(uint32); isUint { | ||
| objTypeArgument, err := helpers.ParseFsNotifyObjType(uint64(objType)) | ||
| parseOrEmptyString(objTypeArg, objTypeArgument, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Please note that if you use a threat detection event which is based on this event you should not rely on the parsed arguments values but on the raw arguments values
| static void handle_dnotify_event(int sig, siginfo_t *si, void *ucontext) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
What's the use of that?
There was a problem hiding this comment.
Forgot to remove it, fixed.
Added should_trace() and should_submit() checks and removed unused function from testing script.
oshaked1
force-pushed
the
fsnotify_event
branch
from
3042ce5 to
9388ef3
Compare
1. Explain what the PR does
Changed inotify_watch event to security_path_notify.
inotify_watch used the inotify_find_inode kernel function which is static and cannot be hooked on all kernels.
This replacement event hooks security_path_notify instead (using a kprobe), which is always available.
This function also handles notify requests from dnotify and fanotify, which are used for similar purposes.
The new event also includes the mask and object type fields,
which can be used to determine the exact characteristics of the watch.
2. Explain how to test it
Run tracee with
--events security_path_notify, generate the event by registering a FS watch using any of thednotify,inotifyorfanotifyAPIs.