On Thu, Nov 14, 2024 at 11:01 PM Song Liu <songliubraving@xxxxxxxx> wrote:
>
> >
> > I think bpf-lsm hook fires before fanotify, so bpf-lsm prog
> > implementing some security policy has to decide right
> > at the moment what to do with, say, security_file_open().
> > fanotify with or without bpf fastpath is too late.
>
> Actually, fanotify in permission mode can stop a file open.
The proposed patch 1 did:
+/* Return value of fp_handler */
+enum fanotify_fastpath_return {
+ /* The event should be sent to user space */
+ FAN_FP_RET_SEND_TO_USERSPACE = 0,
+ /* The event should NOT be sent to user space */
+ FAN_FP_RET_SKIP_EVENT = 1,
+};
It looked like a read-only notification to user space
where bpf prog is merely a filter.
> In current upstream code, fsnotify hook fsnotify_open_perm
> is actually part of security_file_open(). It will be moved
> to do_dentry_open(), right after security_file_open(). This
> move is done by 1cda52f1b461 in linux-next.
Separating fsnotify from LSM makes sense.
> In practice, we are not likely to use BPF LSM and fanotify
> on the same hook at the same time. Instead, we can use
> BPF LSM hooks to gather information and use fanotify to
> make allow/deny decision, or vice versa.
Pick one.
If the proposal is changing to let fsnotify-bpf prog to deny
file_open then it's a completely different discussion.
In such a case make it clear upfront that fsnotify will
rely on CONFIG_FANOTIFY_ACCESS_PERMISSIONS and
bpf-lsm part of file access will not be used,
since interaction of two callbacks at file_open makes little sense.
> > In general fanotify is not for security. It's notifying
> > user space of events that already happened, so I don't see
> > how these two can be combined.
>
> fanotify is actually used by AntiVirus softwares. For
> example, CalmAV (https://www.clamav.net/) uses fanotify
> for its Linux version (it also supports Window and MacOS).
It's relying on user space to send back FANOTIFY_PERM_EVENTS ?
fsnotify_open_perm->fsnotify->send_to_group->fanotify_handle_event.
is a pretty long path to call bpf prog and
preparing a giant 'struct fanotify_fastpath_event'
is not going to fast either.
If we want to accelerate that with bpf it needs to be done
sooner with negligible overhead.
>
> I guess I didn't state the motivation clearly. So let me
> try it now.
>
> Tracing is a critical part of a security solution. With
> LSM, blocking an operation is straightforward. However,
> knowing which operation should be blocked is not always
> easy. Also, security hooks (LSM or fanotify) sit in the
> critical path of user requests. It is very important to
> optimize the latency of a security hook. Ideally, the
> tracing logic should gather all the information ahead
> of time, and make the actual hook fast.
>
> For example, if security_file_open() only needs to read
> a flag from inode local storage, the overhead is minimal
> and predictable. If security_file_open() has to walk the
> dentry tree, or call d_path(), the overhead will be
> much higher. fanotify_file_perm() provides another
> level of optimization over security_file_open(). If a
> file is not being monitored, fanotify will not generate
> the event.
I agree with motivation, but don't see this in the patches.
The overhead to call into bpf prog is big.
Even if prog does nothing it's still going to be slower.
> Security solutions hold higher bars for the tracing logic:
>
> - It needs to be accurate, as false positives and false
> negatives can be very annoying and/or harmful.
> - It needs to be efficient, as security daemons run 24/7.
>
> Given these requirements of security solutions, I believe
> it is important to optimize tracing logic as much as
> possible. And BPF based fanotify fastpath handler can
> bring non-trivials benefit to BPF based security solutions.
Doing everything in the kernel is certainly faster than
going back and forth to user space,
but bpf-lsm should be able to do the same already.
Without patch 1 and only patches 4,5 that add few kfuncs,
bpf-lsm prog will be able to remember subtree dentry and
do the same is_subdir() to deny.
The patch 7 stays pretty much as-is. All in bpf-lsm.
Close to zero overhead without long chain of fsnotify callbacks.
> fanotify also has a feature that LSM doesn't provide.
> When a file is accessed, user space daemon can get a
> fd on this file from fanotify. OTOH, LSM can only send
> an ino or a path to user space, which is not always
> reliable.
That sounds useful, but we're mixing too many things.
If user space cares about fd it will be using the existing
mechanism with all accompanied overhead. fsnotify-bpf can
barely accelerate anything, since user space makes
ultimate decisions.
If user space is not in the driving seat then existing bpf-lsm
plus few kfuncs to remember dentry and call is_subdir()
will do the job and no need for patch 1.
