Linux-Security-Module Archive mirror
 help / color / mirror / Atom feed
From: "Mickaël Salaün" <mic@digikod.net>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Günther Noack" <gnoack@google.com>,
	linux-security-module@vger.kernel.org,
	"Jeff Xu" <jeffxu@google.com>,
	"Jorge Lucangeli Obes" <jorgelo@chromium.org>,
	"Allen Webb" <allenwebb@google.com>,
	"Dmitry Torokhov" <dtor@google.com>,
	"Paul Moore" <paul@paul-moore.com>,
	"Konstantin Meskhidze" <konstantin.meskhidze@huawei.com>,
	"Matt Bobrowski" <repnop@google.com>,
	linux-fsdevel@vger.kernel.org,
	"Christian Brauner" <brauner@kernel.org>
Subject: Re: [PATCH v12 1/9] security: Introduce ENOFILEOPS return value for IOCTL hooks
Date: Tue, 26 Mar 2024 11:10:08 +0100	[thread overview]
Message-ID: <20240326.ahyaaPa0ohs6@digikod.net> (raw)
In-Reply-To: <83b0f28a-92a5-401a-a7f0-d0b0539fc1e5@app.fastmail.com>

On Tue, Mar 26, 2024 at 10:33:23AM +0100, Arnd Bergmann wrote:
> On Tue, Mar 26, 2024, at 09:32, Mickaël Salaün wrote:
> > On Mon, Mar 25, 2024 at 04:19:25PM +0100, Arnd Bergmann wrote:
> >> On Mon, Mar 25, 2024, at 14:39, Günther Noack wrote:
> >> > If security_file_ioctl or security_file_ioctl_compat return
> >> > ENOFILEOPS, the IOCTL logic in fs/ioctl.c will permit the given IOCTL
> >> > command, but only as long as the IOCTL command is implemented directly
> >> > in fs/ioctl.c and does not use the f_ops->unhandled_ioctl or
> >> > f_ops->compat_ioctl operations, which are defined by the given file.
> >> >
> >> > The possible return values for security_file_ioctl and
> >> > security_file_ioctl_compat are now:
> >> >
> >> >  * 0 - to permit the IOCTL
> >> >  * ENOFILEOPS - to permit the IOCTL, but forbid it if it needs to fall
> >> >    back to the file implementation.
> >> >  * any other error - to forbid the IOCTL and return that error
> >> >
> >> > This is an alternative to the previously discussed approaches [1] and [2],
> >> > and implements the proposal from [3].
> >> 
> >> Thanks for trying it out, I think this is a good solution
> >> and I like how the code turned out.
> >
> > This is indeed a simpler solution but unfortunately this doesn't fit
> > well with the requirements for an access control, especially when we
> > need to log denied accesses.  Indeed, with this approach, the LSM (or
> > any other security mechanism) that returns ENOFILEOPS cannot know for
> > sure if the related request will allowed or not, and then it cannot
> > create reliable logs (unlike with EACCES or EPERM).
> 
> Where does the requirement come from specifically, i.e.
> who is the consumer of that log?

The audit framework may be used by LSMs to log denials.

> 
> Even if the log doesn't tell you directly whether the ioctl
> was ultimately denied, I would think logging the ENOFILEOPS
> along with the command number is enough to reconstruct what
> actually happened from reading the log later.

We could indeed log ENOFILEOPS but that could include a lot of allowed
requests and we usually only want unlegitimate access requests to be
logged.  Recording all ENOFILEOPS would mean 1/ that logs would be
flooded by legitimate requests and 2/ that user space log parsers would
need to deduce if a request was allowed or not, which require to know
the list of IOCTL commands implemented by fs/ioctl.c, which would defeat
the goal of this specific patch.

  reply	other threads:[~2024-03-26 10:10 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-25 13:39 [PATCH v12 0/9] Landlock: IOCTL support Günther Noack
2024-03-25 13:39 ` [PATCH v12 1/9] security: Introduce ENOFILEOPS return value for IOCTL hooks Günther Noack
2024-03-25 14:28   ` Günther Noack
2024-03-25 15:19   ` Arnd Bergmann
2024-03-26  8:32     ` Mickaël Salaün
2024-03-26  9:33       ` Arnd Bergmann
2024-03-26 10:10         ` Mickaël Salaün [this message]
2024-03-26 11:58           ` Arnd Bergmann
2024-03-26 13:09             ` Günther Noack
2024-03-26 14:28               ` Mickaël Salaün
2024-03-26 18:52   ` Paul Moore
2024-03-25 13:39 ` [PATCH v12 2/9] landlock: Add IOCTL access right for character and block devices Günther Noack
2024-03-25 13:39 ` [PATCH v12 3/9] selftests/landlock: Test IOCTL support Günther Noack
2024-03-25 13:39 ` [PATCH v12 4/9] selftests/landlock: Test IOCTL with memfds Günther Noack
2024-03-25 13:40 ` [PATCH v12 5/9] selftests/landlock: Test ioctl(2) and ftruncate(2) with open(O_PATH) Günther Noack
2024-03-25 13:40 ` [PATCH v12 6/9] selftests/landlock: Test IOCTLs on named pipes Günther Noack
2024-03-25 13:40 ` [PATCH v12 7/9] selftests/landlock: Check IOCTL restrictions for named UNIX domain sockets Günther Noack
2024-03-25 13:40 ` [PATCH v12 8/9] samples/landlock: Add support for LANDLOCK_ACCESS_FS_IOCTL_DEV Günther Noack
2024-03-25 13:40 ` [PATCH v12 9/9] landlock: Document IOCTL support Günther Noack

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=20240326.ahyaaPa0ohs6@digikod.net \
    --to=mic@digikod.net \
    --cc=allenwebb@google.com \
    --cc=arnd@arndb.de \
    --cc=brauner@kernel.org \
    --cc=dtor@google.com \
    --cc=gnoack@google.com \
    --cc=jeffxu@google.com \
    --cc=jorgelo@chromium.org \
    --cc=konstantin.meskhidze@huawei.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=repnop@google.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 read-only IMAP folder(s) and NNTP newsgroup(s).