[PATCH v3 4/4] landlock: Document Landlock's file truncation support

[Mickaël Salaün]

On 04/08/2022 21:37, Günther Noack wrote:
> Use the LANDLOCK_ACCESS_FS_TRUNCATE flag in the tutorial.
> 
> Adapt the backwards compatibility example and discussion to remove the
> truncation flag where needed.
> 
> Point out potential surprising behaviour related to truncate.
> 
> Signed-off-by: Günther Noack <gnoack3000 at gmail.com>
> ---
>   Documentation/userspace-api/landlock.rst | 41 ++++++++++++++++++++----
>   1 file changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index d92e335380d4..9c3c9fa65958 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -60,7 +60,8 @@ the need to be explicit about the denied-by-default access rights.
>               LANDLOCK_ACCESS_FS_MAKE_FIFO |
>               LANDLOCK_ACCESS_FS_MAKE_BLOCK |
>               LANDLOCK_ACCESS_FS_MAKE_SYM |
> -            LANDLOCK_ACCESS_FS_REFER,
> +            LANDLOCK_ACCESS_FS_REFER |
> +            LANDLOCK_ACCESS_FS_TRUNCATE,
>       };
>   
>   Because we may not know on which kernel version an application will be
> @@ -69,16 +70,24 @@ 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` access right which is only supported
> -starting with the second version of the ABI.
> +remove the `LANDLOCK_ACCESS_FS_REFER` and `LANDLOCK_ACCESS_FS_TRUNCATE` access
> +rights, which are only supported starting with the second and third version of
> +the ABI.
>   
>   .. code-block:: c
>   
>       int abi;
>   
>       abi = landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
> -    if (abi < 2) {
> -        ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER;
> +    switch (abi) {
> +    case -1:
> +            perror("Landlock is not supported with the running kernel");

Because there is a distinction between "supported" and "enabled" (as 
explained in the sample), let's make this message more generic. The 
additional strerror() output would then be enough to distinguish the 
error type.

"The running kernel does not enable to use Landlock"

> +            return 1;
> +    case 1:

This switch/case logic might be a bit confusing; let's explain it for 
this doc *and the sample code*:

/* Removes LANDLOCK_ACCESS_FS_REFER for ABI < 2 */

> +            ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER;
> +            __attribute__((fallthrough));
> +    case 2:

/* Removes LANDLOCK_ACCESS_FS_TRUNCATE for ABI < 3 */

> +            ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
>       }
>   
>   This enables to create an inclusive ruleset that will contain our rules.
> @@ -127,8 +136,8 @@ descriptor.
>   
>   It may also be required to create rules following the same logic as explained
>   for the ruleset creation, by filtering access rights according to the Landlock
> -ABI version.  In this example, this is not required because
> -`LANDLOCK_ACCESS_FS_REFER` is not allowed by any rule.
> +ABI version.  In this example, this is not required because all of the requested
> +``allowed_access`` rights are already available in ABI 1.
>   
>   We now have a ruleset with one rule allowing read access to ``/usr`` while
>   denying all other handled accesses for the filesystem.  The next step is to
> @@ -251,6 +260,24 @@ To be allowed to use :manpage:`ptrace(2)` and related syscalls on a target
>   process, a sandboxed process should have a subset of the target process rules,
>   which means the tracee must be in a sub-domain of the tracer.
>   
> +Truncating files
> +----------------
> +
> +The operations covered by `LANDLOCK_ACCESS_FS_WRITE_FILE` and
> +`LANDLOCK_ACCESS_FS_TRUNCATE` both change the contents of a file and
> +sometimes overlap in non-intuitive ways. It is recommended to always
> +specify both of these together.
> +
> +A particularly surprising example is :manpage:`creat(2)`. The name
> +suggests that this system call requires the rights to create and write
> +files. However, it also requires the truncate right if an existing
> +file under the same name is already present.
> +
> +It should also be noted that truncating files does not necessarily

FYI, I'll send a standalone patch to remove all contractions and get a 
more consistent documentation. Please, keep it this way.


> +require the `LANDLOCK_ACCESS_FS_WRITE_FILE` right. Apart from the
> +obvious :manpage:`truncate(2)` system call, this can also be done
> +through :manpage:`open(2)` with the flags `O_RDONLY` and `O_TRUNC`.

Good!

nit: you can use a 80-columns limit.

> +
>   Compatibility
>   =============
>