Re: [PATCH v2 1/1] fanotify: introduce new event flag FAN_EXEC

[Amir Goldstein <amir73il@gmail.com>]

On Mon, Oct 1, 2018 at 1:58 PM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 27-09-18 23:05:14, Matthew Bobrowski wrote:
> > This is a reduced version of a patch that I originally submitted a while ago.
> >
> > In short, the fanotify API currently does not provide any means for user space
> > programs to receive events specifically when a file has been opened with the
> > intent to be executed. The FAN_EXEC flag will be set within the event mask when
> > a object has been opened with one of the open flags being __FMODE_EXEC.
> >
> > Linux is used as an Operating System in some products, with an environment that
> > can be certified under the Common Criteria Operating System Protection Profile
> > (OSPP). This is a formal threat model for a class of technology. It requires
> > specific countermeasures to mitigate threats. It requires documentation to
> > explain how a product implements these countermeasures. It requires proof via a
> > test suite to demonstrate that the requirements are met, observed and checked by
> > an independent qualified third party. The latest set of requirements for OSPP
> > v4.2 can be found here:
> >
> > https://www.niap-ccevs.org/Profile/Info.cfm?PPID=424&id=424
> >
> > If you look on page 58, you will see the following requirement:
> >
> > FPT_SRP_EXT.1 Software Restriction Policies   FPT_SRP_EXT.1.1
> >
> > The OS shall restrict execution to only programs which match an administrator
> > specified [selection:
> >         file path,
> >         file digital signature,
> >         version,
> >         hash,
> >         [assignment: other characteristics]
> > ]
> >
> > This patch is to help aid in meeting this requirement.
> >
> > Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
>
> I agree with Amir's points wrt API so I won't repeat those. But I have one
> more API question:
>
> You implement FS_EXEC as a flag that can get set for certain FAN_OPEN
> events. That is a new API concept for fanotify. So far you can only request
> event of a certain type and then you get the same flag back when the event
> happens.  There is also a case of FAN_ONDIR where you can restrict set of
> events only to events on a particular inode type but that's again
> different. Hence my question: Is there a good reason why we don't create
> FAN_OPEN_EXEC event that would trigger only on executable opens?
>
> If someone is interested only in executable opens, he'd have less events to
> care about. OTOH additional FS_EXEC flag is probably more flexible (e.g.
> you can easily implement equivalent of FAN_OPEN_NOEXEC in userspace if you
> wished). Just the inconsistency of the FS_EXEC and e.g. how we handle
> FAN_CLOSE_NOWRITE & FAN_CLOSE_WRITE is bothering me...
>

I understand why the inconsitency is bothering you, but IMO it is too late
to change that. By trying to be more consistent in the *implementation* of
the flags, we will end up confusing users instead of making their life easy
by sticking to FAN_OPEN sematics they are used to.

IMO we need to report FAN_OPEN for every open like we do now.
and additionally report FAN_OPEN_EXEC for open for exec.
Then user can implement FAN_OPEN_NOEXEC by setting ignore mask
with FAN_OPEN_EXEC.

You can make the analogy to the compound event FAN_CLOSE.
If user sets a mask to the compound event FAN_CLOSE and sets ignore
mask with FAN_CLOSE_WRITE, then user effectively implemented
FAN_CLOSE_NOWRITE with similar semantics to implementation of
FAN_OPEN_NOEXEC.

Similarly, if user requests FAN_OPEN|FAN_CLOSE and then checks
(event->mask & FAN_OPEN) or (event->mask & FAN_CLOSE) it  has
similar meaning. Testing (event->mask & FAN_OPEN_EXEC) and
(event->mask & FAN_CLOSE_WRITE) in this case is similarly informative.

Honestly, I can't think of an application interested only in
FAN_CLOSE_NOWRITE nor only in FAN_OPEN_NOEXEC, but the
functionality is available for both. The fact that user *can* implement the
former without ignore mask and cannot implement to latter without
ignore mask is IMO the neccesary evil we need to carry for historic API
decisions.

Thanks,
Amir.