Re: FAN_UNPRIVILEGED

[Amir Goldstein <amir73il@gmail.com>]

On Thu, Oct 1, 2020 at 2:00 PM Jan Kara <jack@suse.cz> wrote:
>
> I'm sorry for late reply on this one...
>
> On Tue 15-09-20 11:33:41, Amir Goldstein wrote:
> > On Tue, Sep 15, 2020 at 10:08 AM Jan Kara <jack@suse.cz> wrote:
> > >
> > > On Tue 15-09-20 01:27:43, Weiping Zhang wrote:
> > > > Now the IN_OPEN event can report all open events for a file, but it can
> > > > not distinguish if the file was opened for execute or read/write.
> > > > This patch add a new event IN_OPEN_EXEC to support that. If user only
> > > > want to monitor a file was opened for execute, they can pass a more
> > > > precise event IN_OPEN_EXEC to inotify_add_watch.
> > > >
> > > > Signed-off-by: Weiping Zhang <zhangweiping@didiglobal.com>
> > >
> > > Thanks for the patch but what I'm missing is a justification for it. Is
> > > there any application that cannot use fanotify that needs to distinguish
> > > IN_OPEN and IN_OPEN_EXEC? The OPEN_EXEC notification is for rather
> > > specialized purposes (e.g. audit) which are generally priviledged and need
> > > to use fanotify anyway so I don't see this as an interesting feature for
> > > inotify...
> >
> > That would be my queue to re- bring up FAN_UNPRIVILEGED [1].
> > Last time this was discussed [2], FAN_UNPRIVILEGED did not have
> > feature parity with inotify, but now it mostly does, short of (AFAIK):
> > 1. Rename cookie (*)
> > 2. System tunables for limits
> >
> > The question is - should I pursue it?
>
> So I think that at this point some form less priviledged fanotify use
> starts to make sense. So let's discuss how it would look like... What comes
> to my mind:
>
> 1) We'd need to make max_user_instances, max_user_watches, and
> max_queued_events configurable similarly as for inotify. The first two
> using ucounts so that the configuration is actually per-namespace as for
> inotify.
>
> 2) I don't quite like the FAN_UNPRIVILEDGED flag. I'd rather see the checks
> being done based on functionality requested in fanotify_init() /
> fanotify_mark(). E.g. FAN_UNLIMITED_QUEUE or permission events will require
> CAP_SYS_ADMIN, mount/sb marks will require CAP_DAC_READ_SEARCH, etc.
> We should also consider which capability checks should be system-global and
> which can be just user-namespace ones...

OK. That is not a problem to do.
But FAN_UNPRIVILEDGED flag also impacts:

    An unprivileged event listener does not get an open file descriptor in
    the event nor the process pid of another process.

Obviously, I can check CAP_SYS_ADMIN on fanotify_init() and set the
FAN_UNPRIVILEDGED flag as an internal flag.

The advantage of explicit FAN_UNPRIVILEDGED flag is that a privileged process
can create an unprivileged listener and pass the fd to another process.
Not a critical functionality at this point.

Thoughts?

Thanks,
Amir.