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 v3 5/5] landlock: Document ioctl support
Date: Fri, 18 Aug 2023 18:28:41 +0200 [thread overview]
Message-ID: <20230818.ShaiGhu3wae9@digikod.net> (raw)
In-Reply-To: <20230814172816.3907299-6-gnoack@google.com>
This looks good!
On Mon, Aug 14, 2023 at 07:28:16PM +0200, Günther Noack wrote:
> In the paragraph above the fallback logic, use the shorter phrasing
> from the landlock(7) man page.
>
> Signed-off-by: Günther Noack <gnoack@google.com>
> ---
> Documentation/userspace-api/landlock.rst | 74 ++++++++++++++++++------
> 1 file changed, 57 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index d8cd8cd9ce25..e0e35e474307 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -61,18 +61,17 @@ the need to be explicit about the denied-by-default access rights.
> LANDLOCK_ACCESS_FS_MAKE_BLOCK |
> LANDLOCK_ACCESS_FS_MAKE_SYM |
> LANDLOCK_ACCESS_FS_REFER |
> - LANDLOCK_ACCESS_FS_TRUNCATE,
> + LANDLOCK_ACCESS_FS_TRUNCATE |
> + LANDLOCK_ACCESS_FS_IOCTL,
> };
>
> Because we may not know on which kernel version an application will be
> executed, it is safer to follow a best-effort security approach. Indeed, we
> should try to protect users as much as possible whatever the kernel they are
> -using. To avoid binary enforcement (i.e. either all security features or
> -none), we can leverage a dedicated Landlock command to get the current version
> -of the Landlock ABI and adapt the handled accesses. Let's check if we should
> -remove the ``LANDLOCK_ACCESS_FS_REFER`` or ``LANDLOCK_ACCESS_FS_TRUNCATE``
> -access rights, which are only supported starting with the second and third
> -version of the ABI.
> +using.
> +
> +To be compatible with older Linux versions, we detect the available Landlock ABI
> +version, and only use the available subset of access rights:
>
> .. code-block:: c
>
> @@ -92,6 +91,9 @@ version of the ABI.
> case 2:
> /* Removes LANDLOCK_ACCESS_FS_TRUNCATE for ABI < 3 */
> ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
> + case 3:
> + /* Removes LANDLOCK_ACCESS_FS_IOCTL for ABI < 4 */
> + ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_IOCTL;
> }
>
> This enables to create an inclusive ruleset that will contain our rules.
> @@ -190,6 +192,7 @@ access rights per directory enables to change the location of such directory
> without relying on the destination directory access rights (except those that
> are required for this operation, see ``LANDLOCK_ACCESS_FS_REFER``
> documentation).
> +
> Having self-sufficient hierarchies also helps to tighten the required access
> rights to the minimal set of data. This also helps avoid sinkhole directories,
> i.e. directories where data can be linked to but not linked from. However,
> @@ -283,18 +286,24 @@ It should also be noted that truncating files does not require the
> system call, this can also be done through :manpage:`open(2)` with the flags
> ``O_RDONLY | O_TRUNC``.
>
> -When opening a file, the availability of the ``LANDLOCK_ACCESS_FS_TRUNCATE``
> -right is associated with the newly created file descriptor and will be used for
> -subsequent truncation attempts using :manpage:`ftruncate(2)`. The behavior is
> -similar to opening a file for reading or writing, where permissions are checked
> -during :manpage:`open(2)`, but not during the subsequent :manpage:`read(2)` and
> +The truncate right is associated with the opened file (see below).
> +
> +Rights associated with file descriptors
> +---------------------------------------
> +
> +When opening a file, the availability of the ``LANDLOCK_ACCESS_FS_TRUNCATE`` and
> +``LANDLOCK_ACCESS_FS_IOCTL`` rights is associated with the newly created file
> +descriptor and will be used for subsequent truncation and ioctl attempts using
> +:manpage:`ftruncate(2)` and :manpage:`ioctl(2)`. The behavior is similar to
> +opening a file for reading or writing, where permissions are checked during
> +:manpage:`open(2)`, but not during the subsequent :manpage:`read(2)` and
> :manpage:`write(2)` calls.
>
> -As a consequence, it is possible to have multiple open file descriptors for the
> -same file, where one grants the right to truncate the file and the other does
> -not. It is also possible to pass such file descriptors between processes,
> -keeping their Landlock properties, even when these processes do not have an
> -enforced Landlock ruleset.
> +As a consequence, it is possible to have multiple open file descriptors
> +referring to the same file, where one grants the truncate or ioctl right and the
> +other does not. It is also possible to pass such file descriptors between
> +processes, keeping their Landlock properties, even when these processes do not
> +have an enforced Landlock ruleset.
>
> Compatibility
> =============
> @@ -422,6 +431,27 @@ Memory usage
> Kernel memory allocated to create rulesets is accounted and can be restricted
> by the Documentation/admin-guide/cgroup-v1/memory.rst.
>
> +IOCTL support
> +-------------
> +
> +The ``LANDLOCK_ACCESS_FS_IOCTL`` access right restricts the use of
> +:manpage:`ioctl(2)`, but it only applies to newly opened files. This means
> +specifically that pre-existing file descriptors like STDIN, STDOUT and STDERR
According to man pages (and unlike IOCTL commands) we should not
capitalize stdin, stdout and stderr.
> +are unaffected.
> +
> +Users should be aware that TTY devices have traditionally permitted to control
> +other processes on the same TTY through the ``TIOCSTI`` and ``TIOCLINUX`` IOCTL
> +commands. It is therefore recommended to close inherited TTY file descriptors.
Good to see such warnings in the documentation.
We could also propose a simple solution to still uses stdin, stdout and
stderr without complex TTY proxying: re-opening the TTY, or replacing
related FD thanks to /proc/self/fd/*
For instance, with shell scripts it would look like this:
exec </proc/self/fd/0
exec >/proc/self/fd/1
exec 2>/proc/self/fd/2
Because of TIOCGWINSZ and TCGETS, an interactive shell may not work as
expected though.
> +The :manpage:`isatty(3)` function checks whether a given file descriptor is a
> +TTY.
> +
> +Landlock's IOCTL support is coarse-grained at the moment, but may become more
> +fine-grained in the future. Until then, users are advised to establish the
> +guarantees that they need through the file hierarchy, by only permitting the
> +``LANDLOCK_ACCESS_FS_IOCTL`` right on files where it is really harmless. In
> +cases where you can control the mounts, the ``nodev`` mount option can help to
> +rule out that device files can be accessed.
> +
> Previous limitations
> ====================
>
> @@ -451,6 +481,16 @@ always allowed when using a kernel that only supports the first or second ABI.
> Starting with the Landlock ABI version 3, it is now possible to securely control
> truncation thanks to the new ``LANDLOCK_ACCESS_FS_TRUNCATE`` access right.
>
> +Ioctl (ABI < 4)
> +---------------
> +
> +IOCTL operations could not be denied before the fourth Landlock ABI, so
> +:manpage:`ioctl(2)` is always allowed when using a kernel that only supports an
> +earlier ABI.
> +
> +Starting with the Landlock ABI version 4, it is possible to restrict the use of
> +:manpage:`ioctl(2)` using the new ``LANDLOCK_ACCESS_FS_IOCTL`` access right.
> +
> .. _kernel_support:
>
> Kernel support
> --
> 2.41.0.694.ge786442a9b-goog
>
next prev parent reply other threads:[~2023-08-18 16:29 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-14 17:28 [PATCH v3 0/5] Landlock: IOCTL support Günther Noack
2023-08-14 17:28 ` [PATCH v3 1/5] landlock: Add ioctl access right Günther Noack
2023-08-14 17:43 ` Günther Noack
2023-08-14 17:28 ` [PATCH v3 2/5] selftests/landlock: Test ioctl support Günther Noack
2023-08-18 17:06 ` Mickaël Salaün
2023-08-25 15:51 ` Günther Noack
2023-08-25 17:07 ` Mickaël Salaün
2023-09-01 13:35 ` Günther Noack
2023-09-01 20:24 ` Mickaël Salaün
2023-08-14 17:28 ` [PATCH v3 3/5] selftests/landlock: Test ioctl with memfds Günther Noack
2023-08-14 17:28 ` [PATCH v3 4/5] samples/landlock: Add support for LANDLOCK_ACCESS_FS_IOCTL Günther Noack
2023-08-14 17:28 ` [PATCH v3 5/5] landlock: Document ioctl support Günther Noack
2023-08-18 16:28 ` Mickaël Salaün [this message]
2023-08-25 11:55 ` Günther Noack
2023-08-18 13:26 ` [PATCH v3 0/5] Landlock: IOCTL support Mickaël Salaün
2023-08-18 13:39 ` Mickaël Salaün
2023-08-25 15:03 ` Günther Noack
2023-08-25 16:50 ` Mickaël Salaün
2023-08-26 18:26 ` Mickaël Salaün
2023-09-02 11:53 ` Günther Noack
2023-09-04 18:08 ` Mickaël Salaün
2023-09-11 10:02 ` Günther Noack
2023-09-11 15:25 ` Mickaël Salaün
2023-09-11 16:34 ` Mickaël Salaün
2023-10-19 22:09 ` Günther Noack
2023-10-20 14:57 ` Mickaël Salaün
2023-10-25 22:07 ` Günther Noack
2023-10-26 14:55 ` Mickaël Salaün
2023-11-03 13:06 ` Günther Noack
2023-11-03 15:12 ` Mickaël Salaün
2023-08-22 14:39 ` [PATCH v3 0/5] Landlock: IOCTL support - TTY restrictions RFC Mickaël Salaün
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=20230818.ShaiGhu3wae9@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 NNTP newsgroup(s).