From: Casey Schaufler <casey@schaufler-ca.com>
To: "Mickaël Salaün" <mic@digikod.net>, "Paul Moore" <paul@paul-moore.com>
Cc: "Arnd Bergmann" <arnd@arndb.de>,
"Dave Chinner" <david@fromorbit.com>,
"Günther Noack" <gnoack@google.com>,
"Christian Brauner" <brauner@kernel.org>,
"Allen Webb" <allenwebb@google.com>,
"Dmitry Torokhov" <dtor@google.com>,
"Jeff Xu" <jeffxu@google.com>,
"Jorge Lucangeli Obes" <jorgelo@chromium.org>,
"Konstantin Meskhidze" <konstantin.meskhidze@huawei.com>,
"Matt Bobrowski" <repnop@google.com>,
linux-fsdevel@vger.kernel.org,
linux-security-module@vger.kernel.org,
"Casey Schaufler" <casey@schaufler-ca.com>
Subject: Re: [RFC PATCH] fs: Add vfs_masks_device_ioctl*() helpers
Date: Fri, 8 Mar 2024 14:04:03 -0800 [thread overview]
Message-ID: <c0b43c71-455e-4a40-a45b-37fc4a809434@schaufler-ca.com> (raw)
In-Reply-To: <20240308.eeZ1uungeeSa@digikod.net>
On 3/8/2024 12:12 PM, Mickaël Salaün 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@digikod.net> wrote:
>>> On Fri, Mar 08, 2024 at 08:02:13AM +0100, Arnd Bergmann wrote:
>>>> On Fri, Mar 8, 2024, at 00:09, Dave Chinner wrote:
>>>>> On Thu, Mar 07, 2024 at 03:40:44PM -0500, Paul Moore wrote:
>>>>>> On Thu, Mar 7, 2024 at 7:57 AM Günther Noack <gnoack@google.com> wrote:
>>>>>> I need some more convincing as to why we need to introduce these new
>>>>>> hooks, or even the vfs_masked_device_ioctl() classifier as originally
>>>>>> proposed at the top of this thread. I believe I understand why
>>>>>> Landlock wants this, but I worry that we all might have different
>>>>>> definitions of a "safe" ioctl list, and encoding a definition into the
>>>>>> LSM hooks seems like a bad idea to me.
>>>>> I have no idea what a "safe" ioctl means here. Subsystems already
>>>>> restrict ioctls that can do damage if misused to CAP_SYS_ADMIN, so
>>>>> "safe" clearly means something different here.
>>>> That was my problem with the first version as well, but I think
>>>> drawing the line between "implemented in fs/ioctl.c" and
>>>> "implemented in a random device driver fops->unlock_ioctl()"
>>>> seems like a more helpful definition.
>>>>
>>>> This won't just protect from calling into drivers that are lacking
>>>> a CAP_SYS_ADMIN check, but also from those that end up being
>>>> harmful regardless of the ioctl command code passed into them
>>>> because of stupid driver bugs.
>>> Indeed.
>>>
>>> "safe" is definitely not the right word, it is too broad, relative to
>>> use cases and threat models. There is no "safe" IOCTL.
>>>
>>> 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.
I've been watching this thread with some interest, as one of my side projects
has been an attempt to address the "CAP_SYS_ADMIN problem", and there looks
to be a lot of similarity between that and the "ioctl problem". In both cases
it comes down to a matter of:
1. uniquely identifying the action
2. providing the information to code that can act upon it
3. providing "policy" to determine what to do about it
My thought for the CAP_SYS_ADMIN case was to provide a new LSM hook
security_sysadmin() that takes a single parameter which is the action ID.
I called the action ID a "chit", because it's short and all the good,
more descriptive words where taken. Calls to cap_able(CAP_SYS_ADMIN) could
be replaced by calls to security_sysadmin(chit). security_sysadmin() would
first call cap_able(CAP_SYSADMIN) and, if that succeeded, allow LSMs with
registered hooks (selinux_sysadmin() etc) the opportunity to disallow the
operation. I planned to include a small LSM (chits) that would allow the
operation only if the process had the chit on its chit list. Landlock could
add policy to deal with chits if so inclined.
A generalization of this scheme would be to leave the cap_able(CAP_SYS_ADMIN)
checks as they are and add an optional security_chit() hook for places where
additional enforcement information is desired. Adding
security_chit(CHIT_IOCTL_TTY_SOMETHING)
to an ioctl would allow any LSM to make policy decisions about that ioctl
operation. Adding
security_chit(CHIT_ERASE_TAPE_REGISTERS)
after a cap_able(CAP_SYS_ADMIN) could appease the driver writer who would
otherwise be begging for CAP_ERASE_TAPE_REGISTERS. My biggest concern with
this scheme is the management of chit values, which would have to be kept
in a uapi header.
A major advantage of this is that the security_chit() calls would only have
to be added where someone wants to take advantage of the mechanism. People
who are happy with CAP_SYS_ADMIN or ioctl as it is don't have to do anything,
and their code won't get churned for the new world order. The downside is the
potentially onerous process of deciding if an LSM cares about an action known
only by its number.
I have patches close to ready. The chit LSM isn't passing the laugh test quite
yet, so I'm holding it back for now. I wanted to bring this up before we go too
far down a more complicated path.
next prev parent reply other threads:[~2024-03-08 22:24 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-09 17:06 [PATCH v9 0/8] Landlock: IOCTL support Günther Noack
2024-02-09 17:06 ` [PATCH v9 1/8] landlock: Add IOCTL access right Günther Noack
2024-02-10 11:06 ` Günther Noack
2024-02-10 11:49 ` Arnd Bergmann
2024-02-12 11:09 ` Christian Brauner
2024-02-12 22:10 ` Günther Noack
2024-02-10 11:18 ` Günther Noack
2024-02-16 14:11 ` Mickaël Salaün
2024-02-16 15:51 ` Mickaël Salaün
2024-02-18 8:34 ` Günther Noack
2024-02-19 21:44 ` Günther Noack
2024-02-16 17:19 ` Mickaël Salaün
2024-02-19 18:34 ` Mickaël Salaün
2024-02-19 18:35 ` [RFC PATCH] fs: Add vfs_masks_device_ioctl*() helpers Mickaël Salaün
2024-03-01 13:42 ` Mickaël Salaün
2024-03-01 16:24 ` Arnd Bergmann
2024-03-01 18:35 ` Mickaël Salaün
2024-03-05 18:13 ` Günther Noack
2024-03-06 13:47 ` Mickaël Salaün
2024-03-06 15:18 ` Arnd Bergmann
2024-03-07 12:15 ` Christian Brauner
2024-03-07 12:21 ` Arnd Bergmann
2024-03-07 12:57 ` Günther Noack
2024-03-07 20:40 ` Paul Moore
2024-03-07 23:09 ` Dave Chinner
2024-03-07 23:35 ` Paul Moore
2024-03-08 7:02 ` Arnd Bergmann
2024-03-08 9:29 ` Mickaël Salaün
2024-03-08 19:22 ` Paul Moore
2024-03-08 20:12 ` Mickaël Salaün
2024-03-08 22:04 ` Casey Schaufler [this message]
2024-03-08 22:25 ` Paul Moore
2024-03-09 8:14 ` Günther Noack
2024-03-09 17:41 ` Casey Schaufler
2024-03-11 19:04 ` Paul Moore
2024-03-08 11:03 ` Günther Noack
2024-03-11 1:03 ` Dave Chinner
2024-03-11 9:01 ` Günther Noack
2024-03-11 22:12 ` Dave Chinner
2024-03-12 10:58 ` Mickaël Salaün
2024-02-28 12:57 ` [PATCH v9 1/8] landlock: Add IOCTL access right Günther Noack
2024-03-01 12:59 ` Mickaël Salaün
2024-03-01 13:38 ` Mickaël Salaün
2024-02-09 17:06 ` [PATCH v9 2/8] selftests/landlock: Test IOCTL support Günther Noack
2024-02-09 17:06 ` [PATCH v9 3/8] selftests/landlock: Test IOCTL with memfds Günther Noack
2024-02-09 17:06 ` [PATCH v9 4/8] selftests/landlock: Test ioctl(2) and ftruncate(2) with open(O_PATH) Günther Noack
2024-02-09 17:06 ` [PATCH v9 5/8] selftests/landlock: Test IOCTLs on named pipes Günther Noack
2024-02-09 17:06 ` [PATCH v9 6/8] selftests/landlock: Check IOCTL restrictions for named UNIX domain sockets Günther Noack
2024-02-09 17:06 ` [PATCH v9 7/8] samples/landlock: Add support for LANDLOCK_ACCESS_FS_IOCTL Günther Noack
2024-02-09 17:06 ` [PATCH v9 8/8] 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=c0b43c71-455e-4a40-a45b-37fc4a809434@schaufler-ca.com \
--to=casey@schaufler-ca.com \
--cc=allenwebb@google.com \
--cc=arnd@arndb.de \
--cc=brauner@kernel.org \
--cc=david@fromorbit.com \
--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=mic@digikod.net \
--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).