linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Jan Kara <jack@suse.cz>,
	Matthew Bobrowski <mbobrowski@mbobrowski.org>,
	Steve Grubb <sgrubb@redhat.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v2 1/1] fanotify: introduce new event flag FAN_EXEC
Date: Tue, 2 Oct 2018 11:24:48 +0200	[thread overview]
Message-ID: <20181002092448.GA4135@quack2.suse.cz> (raw)
In-Reply-To: <CAOQ4uxjiuRtXvF=XZWk5EwnYTvWBi63+hnMrkY77AWoHESSDQQ@mail.gmail.com>

On Mon 01-10-18 17:01:23, Amir Goldstein wrote:
> 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.

Fully agreed.

> Then user can implement FAN_OPEN_NOEXEC by setting ignore mask
> with FAN_OPEN_EXEC.

Good point.

> 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.

So it seems we are in full agreement that we don't want "optional event
flags" as Matthew implemented it currently and rather want new event type
FAN_OPEN_EXEC?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

  reply	other threads:[~2018-10-02 16:07 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-27 13:05 [PATCH v2 1/1] fanotify: introduce new event flag FAN_EXEC Matthew Bobrowski
2018-09-27 13:57 ` Amir Goldstein
2018-09-28  1:27   ` Matthew Bobrowski
2018-09-28  5:39     ` Amir Goldstein
2018-10-01  8:21       ` Matthew Bobrowski
2018-10-01  9:13         ` Amir Goldstein
2018-10-01 10:58 ` Jan Kara
2018-10-01 14:01   ` Amir Goldstein
2018-10-02  9:24     ` Jan Kara [this message]
2018-10-02 10:37       ` Amir Goldstein
2018-10-03 15:40         ` Jan Kara
2018-10-03 16:18           ` Amir Goldstein
2018-10-03 16:33             ` Jan Kara
2018-10-03 20:45               ` Matthew Bobrowski
2018-10-07 11:13               ` Matthew Bobrowski
2018-10-07 13:40                 ` Amir Goldstein
2018-10-08  9:35                 ` Jan Kara
2018-10-02 11:50       ` Matthew Bobrowski
2018-10-03 15:45         ` Jan Kara
2018-10-01 11:06 ` Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181002092448.GA4135@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=amir73il@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mbobrowski@mbobrowski.org \
    --cc=sgrubb@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).