[RFC PATCH] fs: Add vfs_masks_device_ioctl*() helpers

[Paul Moore]

On Sat, Mar 9, 2024 at 3:14 AM Günther Noack <gnoack at google.com> wrote:
> On Fri, Mar 08, 2024 at 05:25:21PM -0500, Paul Moore wrote:
> > On Fri, Mar 8, 2024 at 3:12 PM Mickaël Salaün <mic at digikod.net> wrote:
> > > On Fri, Mar 08, 2024 at 02:22:58PM -0500, Paul Moore wrote:
> > > > On Fri, Mar 8, 2024 at 4:29 AM Mickaël Salaün <mic at digikod.net> wrote:
> > > > > Let's replace "safe IOCTL" with "IOCTL always allowed in a Landlock
> > > > > sandbox".
> > > >
> > > > Which is a problem from a LSM perspective as we want to avoid hooks
> > > > which are tightly bound to a single LSM or security model.  It's okay
> > > > if a new hook only has a single LSM implementation, but the hook's
> > > > definition should be such that it is reasonably generalized to support
> > > > multiple LSM/models.
> > >
> > > As any new hook, there is a first user.  Obviously this new hook would
> > > not be restricted to Landlock, it is a generic approach.  I'm pretty
> > > sure a few hooks are only used by one LSM though. ;)
> >
> > Sure, as I said above, it's okay for there to only be a single LSM
> > implementation, but the basic idea behind the hook needs to have some
> > hope of being generic.  Your "let's redefine a safe ioctl as 'IOCTL
> > always allowed in a Landlock sandbox'" doesn't fill me with confidence
> > about the hook being generic; who knows, maybe it will be, but in the
> > absence of a patch, I'm left with descriptions like those.
>
> FWIW, the existing IOCTL hook is used in the following places:
>
> * TOMOYO: seemingly configurable per IOCTL command?  (I did not dig deeper)
> * SELinux: has a hardcoded switch of IOCTL commands, some with special checks.
>   These are also a subset of the do_vfs_ioctl() commands,
>   plus KDSKBENT, KDSKBSENT (from ioctl_console(2)).

One should be careful using the term "hardcoded" here as I believe it
is misleading in the SELinux case.  SELinux has 11 explicitly defined
ioctls, with an additional two configurable on a per-policy basis
depending on the state of the SELinux IOCTL_SKIP_CLOEXEC policy
capability.  The security policy associated with these explicit ioctl
checks is not hardcoded into the kernel, it is defined as part of the
greater SELinux security policy.  One could make an argument that
FIONBIO and FIOASYNC look a bit hardcoded, but there is some subtlety
there that is probably not worth exploring further in this context but
I'm happy to discuss in a different thread if that is helpful.

All the ioctls that are not explicitly defined in the SELinux code,
are still subject to SELinux policy through both the file/ioctl
permission and the extended permission (xperm) functionality.  The
SELinux xperm functionality, when tied to an ioctl operation, allows
policy developers to allow or deny specific ioctl operations on a
per-domain basis.

> * Smack: Decomposes the IOCTL command number to look at the _IOC_WRITE and
>   _IOC_READ bits. (This is a known problematic approach, because (1) these bits
>   describe whether the argument is getting read or written, not whether the
>   operation is a mutating one, and (2) some IOCTL commands do not adhere to the
>   convention and don't use these macros)
>
> AppArmor does not use the LSM IOCTL hook.

-- 
paul-moore.com