From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:48292 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726930AbeGSK7i (ORCPT ); Thu, 19 Jul 2018 06:59:38 -0400 Date: Thu, 19 Jul 2018 12:17:08 +0200 From: Jan Kara To: Matthew Bobrowski Cc: Jan Kara , Steve Grubb , amir73il@gmail.com, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH] fanotify: introduce event flags FAN_EXEC and FAN_EXEC_PERM Message-ID: <20180719101708.ad754qhekwoyanps@quack2.suse.cz> References: <1531731011.19075.11.camel@mbobrowski.org> <20180716152653.odfm7tqagqi3wsuo@quack2.suse.cz> <7077707.M30l5tS7I4@x2> <20180717124423.fwzhgoa2ndtbjhgc@quack2.suse.cz> <1531912664.19075.19.camel@mbobrowski.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1531912664.19075.19.camel@mbobrowski.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed 18-07-18 21:17:44, Matthew Bobrowski wrote: > On Tue, 2018-07-17 at 14:44 +0200, Jan Kara wrote: > > On Mon 16-07-18 16:29:42, Steve Grubb wrote: > > > Hello, > > > > > > On Monday, July 16, 2018 11:26:53 AM EDT Jan Kara wrote: > > > > On Mon 16-07-18 18:50:11, Matthew Bobrowski wrote: > > > > > Currently, the fanotify API does not provide a means for user space > > > > > programs to register and receive events specifically when a file > > > > > has been > > > > > opened with the intent to be executed. Two new event flags FAN_EXEC > > > > > and > > > > > FAN_EXEC_PERM have been introduced to the fanotify API along with > > > > > updates > > > > > to the generic filesystem notification hooks fsnotify_open and > > > > > fsnotify_perm in order to support this capability. > > > > > > > > > > Signed-off-by: Matthew Bobrowski > > > > > > > > I miss one important part in this changelog: Why do you need this > > > > feature? > > > > Monitoring for read would be enough after all... > > We're in the midst of writing an application whitelisting program whereby > it's predominantly concerned about file accesses that are a result of an > execution attempt. Without going into semantics, the execution and overall > access control is enforced by an overarching policy that either permits or > denies a file from being executed. OK, and I guess my point is that fanotify is not, in my opinion, a suitable API for this. What you describe above is exactly what LSMs (Linux Security Modules) are designed to be used for (but admittedly I'm no expert on these). So you can use e.g. Apparmor to whitelist executables, that can be executed by your contained program. Or similarly you can use SELinux, tag executables appropriately and only properly tagged executables will be able to be executed by your contained program. There's no need to reinvent the functionality in fanotify + your framework... > The current implementation makes use of the FAN_PERM_OPEN event flag in > order to determine whether a file within a monitored mark point has been > potentially opened for execution. Although this does somewhat provide the > functionality that we're looking for, it's far from ideal and I personally > think we can do a lot better. If in a position where you're monitoring an > entire filesystem using the FAN_OPEN_PERM event flag you can only imagine > the number of events that applications of this class need to process > unnecessarily due to the significant number of files being opened system > wide. Having the additional FAN_EXEC* event flags will allow user space > applications of such type to explicitly receive and process events that are > in fact a result of file being opened and read with the intent on being > executed. > > Irrespective of our particular use case, I believe that such flags will be > something that other consumers of this API may also find relatively useful. Another problem I have with the flag is it is not reliable - e.g. shared libraries get executed as well but are not open with FMODE_EXEC. Shell scripts executed as ". script.sh" as well. So the semantics would be weakly defined in my opinion. > > > There are several reasons that I can think of. There are lots of file� > > > accesses. It is possible to guess which one is the beginning of an > > > execve,� > > > but you really don't know. Apps can be started in so many ways. They > > > can be� > > > run from the runtime linker, they have LD_PRELOAD, they can have an� > > > unexpected interpreter, they can be statically linked, they can be a > > > script� > > > which may present a new pattern of file accesses. With all the > > > variations in� > > > how programs can start up, it would be nice to have one anchor that� > > > unambiguously says we are overlaying this pid with a whole new app and > > > forget� > > > everything you knew about the pid. And the access pattern can be > > > accurately� > > > watched because we always have a marker to start the sequence. > > > > I don't quite buy your "marker that pid is starting from scratch" > > argument. > > Firstly, you'd have to place fanotify watches on all executables in your > > system to even have a chance to tell that, secondly, the process does not > > quite start a new - it still inherits some stuff from the old process > > like > > open file descriptors... So I understand exec might be interesting for > > audit purposes but then you should watch it using audit and not fanotify > > which is for handling / mediating filesystem accesses. > > > > And when you are looking at filesystem accesses, then there's no real > > difference between exec and read which is exactly why I'm not sure why > > the > > new feature is being added. > > Although I do completely agree with the fact that there's no real > difference between an exec and read, I do however believe that in the > context of filesystem accesses it's important to distinguish between the > type access i.e. the intent, right? Let me ask you this. Why is that we > have additional flags such as FAN_ACCESS and FAN_CLOSE_WRITE for example > available within the API? Wouldn't FAN_OPEN be sufficient in terms of > understanding whether some type of other access operation is being > performed on the file object that has been marked for monitoring? Sure, > maybe it would, but it's far from being specific and the entire objective > of all the additional flags is to presumably provide a means for a program > to receive filesystem access events that are a result of a specific intent > or action being performed on the object. Take the following for example: > > FAN_ACCESS: > > The events that come through for this particular flag are a result of a > file having its contents read. Before any contents can be read from it > however, the actual file needs to be opened. > > FAN_EXEC: > > The events that come through for this particular flag are a result of a > file being executed. Prior to the file being executed though it needs to be > opened, read, and mapped into memory accordingly. > > What's the difference? Both perform an open, so wouldn't that just be > enough to tell some type of access is undertaken? Well, no. It's the intent > that needs highlighting here. > > You may sit there and claim that part of a file being executed is to have > it opened and read, so why not just use either FAN_ACCESS or FAN_OPEN, > right? Well sure, but both operations open and read are in fact implicit > actions as a result of an actual explicit intent of the file being > executed. Now, how is FAN_ACCESS and any of the other event flags different > from the new flags that I've proposed here? After all, isn't exec just not > another type of file access despite also potentially having other file > actions embedded within it? So fanotify is a filesystem event notification API. For filesystem, open and read are fundamentally different events and as such we have different FAN_OPEN and FAN_ACCESS events in the API. The only disputable events we have in the API are FAN_CLOSE_WRITE vs FAN_CLOSE_NOWRITE - from fs POV there's no big difference. But at least this is 100% reliably (unlike FMODE_EXEC) telling you whether the user was able to modify the file or not and it caters to one of the use cases this API has been created for - virus scanners, file caching daemons, ... - i.e., triggering specific actions based on file contents. What you are implementing seems to me more like access mediation based on process (or its capabilities) which is really more a task for LSM. Honza -- Jan Kara SUSE Labs, CR