Closed terylt closed 1 year ago
I agree, this is a not so big change that would offer a notable new feature. :100: from me. I don't think we have the bandwidth to implement this for Falco 0.35 (and driver 5.0.0); i'll set the next driver milestone for now! /milestone next-driver
Thank you very much for the input and idea!
I found a similar case related to the rename
syscall family (do_renameat2
) which I would like to share.
When a file is renamed, it could also be valuable info to know whether the destination file was present (and eventually replaced by the rename operation) or a new entry was created.
It seems this is checked at this point: https://elixir.bootlin.com/linux/v6.4-rc1/source/fs/namei.c#L4958, since the RENAME_NOREPLACE
flag specifies whether a destination file can be replaced or not.
Looking deeper at the d_is_positive
implementation, I think also on the rename exit we may have a field similar to the file.is_created
that @FedeDP proposed, using this value:
(new_dentry->d_flags & DCACHE_ENTRY_TYPE) == DCACHE_MISS_TYPE
Or maybe file.is_replaced
is more appropriate here, defined as (new_dentry->d_flags & DCACHE_ENTRY_TYPE) != DCACHE_MISS_TYPE
.
ei @gentooise thank you for the hint, this is something we can think to add. The rename case seems quite similar to the open one so yes we can try to provide support for both, of course, it has a cost because we need to read in some kernel structs, so we need to be sure that this won't impact too much performance
Hi,
In regards to this request, from what I can see, when a user calls open
with O_CREATE
and the file already exists, they will get an error and errno=EEXIST
, so this is not an issue that can not be handled by application using the lib (as I understand it).
On the other hand, the rename
syscall has many use cases (oldpath
and/or newpath
may be a directory or a file), according the rename
man page, in some cases the user can get an errno
but in some cases like when renaming an existing file (newpath
exists), user will not get any errno
since it is a valid usage of the syscall.
So I guess rename
and family do deserve to get some indication to trickle to user.
My guess is that adding a syscall additional behavior (which was not specified by libc or the syscall man page) is welcome when our goal is to identify security related operations than need to be flagged.
Hi, In regards to this request, from what I can see, when a user calls
open
withO_CREATE
and the file already exists, they will get an error anderrno=EEXIST
, so this is not an issue that can not be handled by application using the lib (as I understand it).
Hei :wave: unfortunately this is not completely true, when we use O_CREATE
flag open will fail only if this flag is specified in conjunction with O_EXCL
, in all other cases it won't fail. Moreover, there are other flags to create files like O_TMPFILE
so we cannot definitely rely on just the errno
:( Here the aim is to reliably trace all the times a new file is created by the open
syscall and probably the way suggested by @terylt (https://elixir.bootlin.com/linux/latest/source/fs/namei.c#L3409) is the right way to go. We need to add some custom code in our drivers to intercept this FILE_MODE_CREATED
flag and we need to send it to userspace like if it was a flag of the open syscall. Maybe there other options but this seems a possible one...
On the other hand, the
rename
syscall has many use cases (oldpath
and/ornewpath
may be a directory or a file), according therename
man page, in some cases the user can get anerrno
but in some cases like when renaming an existing file (newpath
exists), user will not get anyerrno
since it is a valid usage of the syscall. So I guessrename
and family do deserve to get some indication to trickle to user.My guess is that adding a syscall additional behavior (which was not specified by libc or the syscall man page) is welcome when our goal is to identify security related operations than need to be flagged.
Yeah probably here we should do something similar to what described here but let's try to tackle the open case first
Hi,
OK, so this would add an additional parameter to the PPME_SYSCALL_OPEN_X
which will be like a 32bit flags (assuming 32 will suffice for future usage) and based on the (file->f_mode & FMODE_CREATED)
status will set a new bit field flag which will be set in the open exit routine for user-space to check.
Based on kernel grep there are some other places FMODE_CREATED
is set (in regards to directory open) like in nfs/fuse/cifs but I assume directory creation should also be caught as file creation.
Will look for an example of exit syscall field adding to see that I cover all drivers/documentation/tests ...
Hi, OK, so this would add an additional parameter to the
PPME_SYSCALL_OPEN_X
which will be like a 32bit flags (assuming 32 will suffice for future usage) and based on the(file->f_mode & FMODE_CREATED)
status will set a new bit field flag which will be set in the open exit routine for user-space to check.
Yes but you don't have to introduce a new flags
field we already have it:
https://github.com/falcosecurity/libs/blob/dca429258d697e246fa18c8ce2db08b6243026c5/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/open.bpf.c#L75 you just need to add a new PPM_
internal notation
https://github.com/falcosecurity/libs/blob/dca429258d697e246fa18c8ce2db08b6243026c5/driver/ppm_events_public.h#L86
So yes we need a new flag but not a new field
Based on kernel grep there are some other places
FMODE_CREATED
is set (in regards to directory open) like in nfs/fuse/cifs but I assume directory creation should also be caught as file creation. Will look for an example of exit syscall field adding to see that I cover all drivers/documentation/tests ...
Yes i think it's fine to catch also directory creation in the end in userspace we can differentiate between directory or file and chose which consider if we want
OK, though I was thinking that the PPM_O_flags are aligned with the kernel, but now I see that some flags are and some are not, since it is an internal flag which gets converted when passing between worlds it should be fine, great and thanks, that reduces the code changes I need to do :)
Motivation
Currently, the libs can't tell whether a file opened for writing is a newly created file, or is an already existing file. This information can be used, for example, to tell us whether a binary is being modified or newly written and can be valuable for assessing program behavior.
Feature
The file struct in the kernel has a
f_mode
attribute which gets set toFILE_MODE_CREATED
when a file is created by an open() system call. https://elixir.bootlin.com/linux/latest/source/fs/namei.c#L3409 It would be really cool if we could expose this as afile_flags
attribute on an open() exit to the libs. @FedeDP had a great idea about also having a filter checkfile.is_created
that checks file_flags & FMODE_CREATED for falco as well.Alternatives
I haven't considered any other way, but am open to ideas.
Additional context
N/A