From: "Mickaël Salaün" <mic@digikod.net>
To: "Günther Noack" <gnoack@google.com>
Cc: 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
Subject: Re: [PATCH v8 4/9] landlock: Add IOCTL access right
Date: Thu, 14 Dec 2023 10:26:48 +0100 [thread overview]
Message-ID: <20231214.feeZ6Hahwaem@digikod.net> (raw)
In-Reply-To: <20231208155121.1943775-5-gnoack@google.com>
On Fri, Dec 08, 2023 at 04:51:16PM +0100, Günther Noack wrote:
> Introduces the LANDLOCK_ACCESS_FS_IOCTL access right
> and increments the Landlock ABI version to 5.
>
> Like the truncate right, these rights are associated with a file
> descriptor at the time of open(2), and get respected even when the
> file descriptor is used outside of the thread which it was originally
> opened in.
>
> A newly enabled Landlock policy therefore does not apply to file
> descriptors which are already open.
>
> If the LANDLOCK_ACCESS_FS_IOCTL right is handled, only a small number
> of safe IOCTL commands will be permitted on newly opened files. The
> permitted IOCTLs can be configured through the ruleset in limited ways
> now. (See documentation for details.)
>
> Specifically, when LANDLOCK_ACCESS_FS_IOCTL is handled, granting this
> right on a file or directory will *not* permit to do all IOCTL
> commands, but only influence the IOCTL commands which are not already
> handled through other access rights. The intent is to keep the groups
> of IOCTL commands more fine-grained.
>
> Noteworthy scenarios which require special attention:
>
> TTY devices support IOCTLs like TIOCSTI and TIOCLINUX, which can be
> used to control shell processes on the same terminal which run at
> different privilege levels, which may make it possible to escape a
> sandbox. Because stdin, stdout and stderr are normally inherited
> rather than newly opened, IOCTLs are usually permitted on them even
> after the Landlock policy is enforced.
>
> Some legitimate file system features, like setting up fscrypt, are
> exposed as IOCTL commands on regular files and directories -- users of
> Landlock are advised to double check that the sandboxed process does
> not need to invoke these IOCTLs.
>
> Known limitations:
>
> The LANDLOCK_ACCESS_FS_IOCTL access right is a coarse-grained control
> over IOCTL commands. Future work will enable a more fine-grained
> access control for IOCTLs.
>
> In the meantime, Landlock users may use path-based restrictions in
> combination with their knowledge about the file system layout to
> control what IOCTLs can be done. Mounting file systems with the nodev
> option can help to distinguish regular files and devices, and give
> guarantees about the affected files, which Landlock alone can not give
> yet.
>
> Signed-off-by: Günther Noack <gnoack@google.com>
> ---
> include/uapi/linux/landlock.h | 58 +++++-
> security/landlock/fs.c | 176 ++++++++++++++++++-
> security/landlock/fs.h | 2 +
> security/landlock/limits.h | 11 +-
> security/landlock/ruleset.h | 2 +-
> security/landlock/syscalls.c | 19 +-
> tools/testing/selftests/landlock/base_test.c | 2 +-
> tools/testing/selftests/landlock/fs_test.c | 5 +-
> 8 files changed, 253 insertions(+), 22 deletions(-)
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 9ba989ef46a5..81ce41e9e6db 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -7,12 +7,14 @@
> * Copyright © 2021-2022 Microsoft Corporation
> */
>
> +#include <asm/ioctls.h>
> #include <linux/atomic.h>
> #include <linux/bitops.h>
> #include <linux/bits.h>
> #include <linux/compiler_types.h>
> #include <linux/dcache.h>
> #include <linux/err.h>
> +#include <linux/falloc.h>
> #include <linux/fs.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> @@ -28,6 +30,7 @@
> #include <linux/types.h>
> #include <linux/wait_bit.h>
> #include <linux/workqueue.h>
> +#include <uapi/linux/fiemap.h>
> #include <uapi/linux/landlock.h>
>
> #include "common.h"
> @@ -83,6 +86,145 @@ static const struct landlock_object_underops landlock_fs_underops = {
> .release = release_inode
> };
>
> +/* IOCTL helpers */
> +
> +/*
> + * These are synthetic access rights, which are only used within the kernel, but
> + * not exposed to callers in userspace. The mapping between these access rights
> + * and IOCTL commands is defined in the required_ioctl_access() helper function.
> + */
> +#define LANDLOCK_ACCESS_FS_IOCTL_GROUP1 (LANDLOCK_LAST_PUBLIC_ACCESS_FS << 1)
> +#define LANDLOCK_ACCESS_FS_IOCTL_GROUP2 (LANDLOCK_LAST_PUBLIC_ACCESS_FS << 2)
> +#define LANDLOCK_ACCESS_FS_IOCTL_GROUP3 (LANDLOCK_LAST_PUBLIC_ACCESS_FS << 3)
> +#define LANDLOCK_ACCESS_FS_IOCTL_GROUP4 (LANDLOCK_LAST_PUBLIC_ACCESS_FS << 4)
> +
> +/* ioctl_groups - all synthetic access rights for IOCTL command groups */
> +/* clang-format off */
> +#define IOCTL_GROUPS ( \
> + LANDLOCK_ACCESS_FS_IOCTL_GROUP1 | \
> + LANDLOCK_ACCESS_FS_IOCTL_GROUP2 | \
> + LANDLOCK_ACCESS_FS_IOCTL_GROUP3 | \
> + LANDLOCK_ACCESS_FS_IOCTL_GROUP4)
> +/* clang-format on */
> +
> +static_assert((IOCTL_GROUPS & LANDLOCK_MASK_ACCESS_FS) == IOCTL_GROUPS);
> +
> +/**
> + * required_ioctl_access(): Determine required IOCTL access rights.
> + *
> + * @cmd: The IOCTL command that is supposed to be run.
> + *
> + * Returns: The access rights that must be granted on an opened file in order to
> + * use the given @cmd.
> + */
> +static access_mask_t required_ioctl_access(unsigned int cmd)
You can add __attribute_const__ after "static", and also constify cmd.
> +{
> + switch (cmd) {
> + case FIOCLEX:
> + case FIONCLEX:
> + case FIONBIO:
> + case FIOASYNC:
> + /*
> + * FIOCLEX, FIONCLEX, FIONBIO and FIOASYNC manipulate the FD's
> + * close-on-exec and the file's buffered-IO and async flags.
> + * These operations are also available through fcntl(2),
> + * and are unconditionally permitted in Landlock.
> + */
> + return 0;
> + case FIOQSIZE:
> + return LANDLOCK_ACCESS_FS_IOCTL_GROUP1;
> + case FS_IOC_FIEMAP:
> + case FIBMAP:
> + case FIGETBSZ:
> + return LANDLOCK_ACCESS_FS_IOCTL_GROUP2;
> + case FIONREAD:
> + case FIDEDUPERANGE:
> + return LANDLOCK_ACCESS_FS_IOCTL_GROUP3;
> + case FICLONE:
> + case FICLONERANGE:
> + case FS_IOC_RESVSP:
> + case FS_IOC_RESVSP64:
> + case FS_IOC_UNRESVSP:
> + case FS_IOC_UNRESVSP64:
> + case FS_IOC_ZERO_RANGE:
> + return LANDLOCK_ACCESS_FS_IOCTL_GROUP4;
> + default:
> + /*
> + * Other commands are guarded by the catch-all access right.
> + */
> + return LANDLOCK_ACCESS_FS_IOCTL;
> + }
> +}
> +
> +/**
> + * expand_ioctl() - Return the dst flags from either the src flag or the
> + * %LANDLOCK_ACCESS_FS_IOCTL flag, depending on whether the
> + * %LANDLOCK_ACCESS_FS_IOCTL and src access rights are handled or not.
> + *
> + * @handled: Handled access rights.
> + * @access: The access mask to copy values from.
> + * @src: A single access right to copy from in @access.
> + * @dst: One or more access rights to copy to.
> + *
> + * Returns: @dst, or 0.
> + */
> +static access_mask_t expand_ioctl(const access_mask_t handled,
static __attribute_const__
> + const access_mask_t access,
> + const access_mask_t src,
> + const access_mask_t dst)
> +{
> + access_mask_t copy_from;
> +
> + if (!(handled & LANDLOCK_ACCESS_FS_IOCTL))
> + return 0;
> +
> + copy_from = (handled & src) ? src : LANDLOCK_ACCESS_FS_IOCTL;
> + if (access & copy_from)
> + return dst;
> +
> + return 0;
> +}
> +
> +/**
> + * landlock_expand_access_fs() - Returns @access with the synthetic IOCTL group
> + * flags enabled if necessary.
> + *
> + * @handled: Handled FS access rights.
> + * @access: FS access rights to expand.
> + *
> + * Returns: @access expanded by the necessary flags for the synthetic IOCTL
> + * access rights.
> + */
> +static access_mask_t landlock_expand_access_fs(const access_mask_t handled,
static __attribute_const__
> + const access_mask_t access)
> +{
> + return access |
> + expand_ioctl(handled, access, LANDLOCK_ACCESS_FS_WRITE_FILE,
> + LANDLOCK_ACCESS_FS_IOCTL_GROUP1 |
> + LANDLOCK_ACCESS_FS_IOCTL_GROUP2 |
> + LANDLOCK_ACCESS_FS_IOCTL_GROUP4) |
> + expand_ioctl(handled, access, LANDLOCK_ACCESS_FS_READ_FILE,
> + LANDLOCK_ACCESS_FS_IOCTL_GROUP1 |
> + LANDLOCK_ACCESS_FS_IOCTL_GROUP2 |
> + LANDLOCK_ACCESS_FS_IOCTL_GROUP3) |
> + expand_ioctl(handled, access, LANDLOCK_ACCESS_FS_READ_DIR,
> + LANDLOCK_ACCESS_FS_IOCTL_GROUP1);
> +}
> +
> +/**
> + * landlock_expand_handled_access_fs() - add synthetic IOCTL access rights to an
> + * access mask of handled accesses.
> + *
> + * @handled: The handled accesses of a ruleset that is being created.
> + *
> + * Returns: @handled, with the bits for the synthetic IOCTL access rights set,
> + * if %LANDLOCK_ACCESS_FS_IOCTL is handled.
> + */
> +access_mask_t landlock_expand_handled_access_fs(const access_mask_t handled)
__attribute_const__ access_mask_t
> +{
> + return landlock_expand_access_fs(handled, handled);
> +}
> +
> diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> index 488e4813680a..c88fe7bda37b 100644
> --- a/security/landlock/fs.h
> +++ b/security/landlock/fs.h
> @@ -92,4 +92,6 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> const struct path *const path,
> access_mask_t access_hierarchy);
>
> +access_mask_t landlock_expand_handled_access_fs(const access_mask_t handled);
__attribute_const__ access_mask_t
> +
> #endif /* _SECURITY_LANDLOCK_FS_H */
next prev parent reply other threads:[~2023-12-14 9:26 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-08 15:51 [PATCH v8 0/9] Landlock: IOCTL support Günther Noack
2023-12-08 15:51 ` [PATCH v8 1/9] landlock: Remove remaining "inline" modifiers in .c files Günther Noack
2023-12-08 15:51 ` [PATCH v8 2/9] selftests/landlock: Rename "permitted" to "allowed" in ftruncate tests Günther Noack
2023-12-08 15:51 ` [PATCH v8 3/9] landlock: Optimize the number of calls to get_access_mask slightly Günther Noack
2024-01-05 9:38 ` Mickaël Salaün
2023-12-08 15:51 ` [PATCH v8 4/9] landlock: Add IOCTL access right Günther Noack
2023-12-14 9:26 ` Mickaël Salaün [this message]
2023-12-14 10:14 ` Mickaël Salaün
2023-12-14 14:28 ` Mickaël Salaün
2024-01-30 18:13 ` Günther Noack
2024-01-31 16:52 ` Mickaël Salaün
2023-12-08 15:51 ` [PATCH v8 5/9] selftests/landlock: Test IOCTL support Günther Noack
2023-12-15 12:52 ` Aishwarya TCV
2024-01-12 17:31 ` Günther Noack
2024-01-12 18:59 ` Mark Brown
2024-01-15 14:20 ` Günther Noack
2023-12-08 15:51 ` [PATCH v8 6/9] selftests/landlock: Test IOCTL with memfds Günther Noack
2023-12-08 15:51 ` [PATCH v8 7/9] selftests/landlock: Test ioctl(2) and ftruncate(2) with open(O_PATH) Günther Noack
2023-12-08 15:51 ` [PATCH v8 8/9] samples/landlock: Add support for LANDLOCK_ACCESS_FS_IOCTL Günther Noack
2023-12-08 15:51 ` [PATCH v8 9/9] landlock: Document IOCTL support Günther Noack
2023-12-11 7:04 ` Mickaël Salaün
2023-12-11 8:49 ` Günther Noack
2023-12-13 11:21 ` Mickaël Salaün
2023-12-13 11:25 ` Mickaël Salaün
2024-01-12 11:51 ` 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=20231214.feeZ6Hahwaem@digikod.net \
--to=mic@digikod.net \
--cc=allenwebb@google.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=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).