Re: [PATCH 2/3] landlock.7: Document Landlock ABI v2 (file reparenting; kernel 5.19)

[Alex Colomar <alx.manpages@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 7399 bytes --]

Hi Günther,

On 2/21/23 21:50, Günther Noack wrote:
> * Add the description for LANDLOCK_ACCESS_FS_REFER,
>    in line with recent update to the uapi headers:
>    https://lore.kernel.org/linux-security-module/20230202204623.10345-1-gnoack3000@gmail.com/T/
> * VERSIONS: Add a table of Landlock versions and their changes.
>    Briefly talk about how to probe ABI levels and warn users about the
>    special semantics of the LANDLOCK_ACCESS_FS_REFER right.
> * Add LANDLOCK_ACCESS_FS_REFER to the code example.
> 
> Code review threads for the "refer" feature:
> * https://lore.kernel.org/all/20220506161102.525323-1-mic@digikod.net/ (initial commit)
> * https://lore.kernel.org/all/20220823144123.633721-1-mic@digikod.net/ (bugfix)
> * https://lore.kernel.org/all/20230221165205.4231-1-gnoack3000@gmail.com/ (documentation update)

I finally found some time to review your work.  Thanks for the patience!

Please see some comments below.

Thanks!

Alex

> ---
>   man7/landlock.7 | 90 +++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/man7/landlock.7 b/man7/landlock.7
> index 099f68067..6321b56ab 100644
> --- a/man7/landlock.7
> +++ b/man7/landlock.7
> @@ -105,6 +105,53 @@ Create (or rename or link) a block device.
>   .TP
>   .B LANDLOCK_ACCESS_FS_MAKE_SYM
>   Create (or rename or link) a symbolic link.
> +.TP
> +.B LANDLOCK_ACCESS_FS_REFER
> +Link or rename a file from or to a different directory (i.e. reparent
> +a file hierarchy).

Please have a look at man-pages(7):
    Use semantic newlines
        In the source of a manual page, new sentences should be started
        on  new  lines,  long  sentences  should be split into lines at
        clause breaks (commas, semicolons, colons, and so on), and long
        clauses should be split at phrase boundaries.  This convention,
        sometimes known as "semantic newlines", makes it easier to  see
        the  effect of patches, which often operate at the level of in‐
        dividual sentences, clauses, or phrases.

Here,
that would mean
breaking the line right before the opening parenthesis;
please also apply it to the rest of the patch where appropriate.


> +.IP
> +This access right is available since the second version of the
> +Landlock ABI.
> +.IP
> +This is the only access right which is denied by default by any
> +ruleset, even if the right is not specified as handled at ruleset
> +creation time.  The only way to make a ruleset grant this right is to
> +explicitly allow it for a specific directory by adding a matching rule
> +to the ruleset.
> +.IP
> +In particular, when using the first Landlock ABI version, Landlock will
> +always deny attempts to reparent files between different directories.
> +.IP
> +In addition to the source and destination directories having the
> +.B LANDLOCK_ACCESS_FS_REFER
> +access right, the attempted link or rename operation must meet the
> +following constraints:
> +.RS
> +.IP \(bu 3

We now use \[bu] instead of \(bu (they are equivalent).

commit 36f73ba37945c7ff4aa2d8383f831519a38e3f27
Author: Alejandro Colomar <alx@kernel.org>
Date:   Sun Feb 5 22:59:22 2023 +0100

     man-pages.7: Recommend using \[..] instead of \(.. escapes

     They are more readable.

     Signed-off-by: Alejandro Colomar <alx@kernel.org>

diff --git a/man7/man-pages.7 b/man7/man-pages.7
index 4132ddabe..e5b111283 100644
--- a/man7/man-pages.7
+++ b/man7/man-pages.7
@@ -666,7 +666,7 @@ These represent a set of (normally) exclusive 
alternatives.
  .TP
  Bullet lists
  Elements are preceded by bullet symbols
-.RB ( \e(bu ).
+.RB ( \e[bu] ).
  Anything that doesn't fit elsewhere is
  usually covered by this type of list.
  .TP
@@
[...]

commit cdede5cdd1b0ba75135d3b32d96354026e96f866
Author: Alejandro Colomar <alx@kernel.org>
Date:   Sun Feb 5 23:14:38 2023 +0100

     Many pages: Use \[bu] instead of \(bu

     Signed-off-by: Alejandro Colomar <alx@kernel.org>



> +The reparented file may not gain more access rights in the destination
> +directory than it previously had in the source directory.  If this is
> +attempted, the operation results in an
> +.B EXDEV
> +error.
> +.IP \(bu 3
> +When linking or renaming, the
> +.B LANDLOCK_ACCESS_FS_MAKE_*
> +right for the respective file type must be granted for the destination
> +directory. Otherwise, the operation results in an
> +.BR EACCES
> +error.
> +.IP \(bu 3
> +When renaming, the
> +.B LANDLOCK_ACCESS_FS_REMOVE_*
> +right for the respective file type must be granted for the source directory. Otherwise, the operation results in an

80 columns is a strong limit.
Using semantic newlines as suggested above should fix this.

> +.B EACCES
> +error.
> +.RE
> +.IP
> +If multiple requirements are not met, the
> +.B EACCES
> +error code takes precedence over
> +.BR EXDEV .
>   .\"
>   .SS Layers of file path access rights
>   Each time a thread enforces a ruleset on itself,
> @@ -182,7 +229,45 @@ 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.
>   .SH VERSIONS
> -Landlock was added in Linux 5.13.
> +Landlock was introduced in Linux 5.13.
> +.PP
> +The availability of individual Landlock features is versioned through
> +ABI levels:
> +.TS
> +box;
> +ntb| ntb| lbx
> +nt| nt| lbx.
> +ABI	Kernel	Newly introduced access rights
> +_	_	_
> +1	5.13	LANDLOCK_ACCESS_FS_EXECUTE
> +\^	\^	LANDLOCK_ACCESS_FS_WRITE_FILE

What character do you want here?
If you want ASCII 0x5E,
then you want to use \[ha].

> +\^	\^	LANDLOCK_ACCESS_FS_READ_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_READ_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_REMOVE_FILE
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_CHAR
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_DIR
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_REG
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SOCK
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_FIFO
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_BLOCK
> +\^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
> +_	_	_
> +2	5.19	LANDLOCK_ACCESS_FS_REFER
> +.TE
> +.PP
> +To query the running kernel's Landlock ABI level, programs may pass
> +the
> +.B LANDLOCK_CREATE_RULESET_VERSION
> +flag to
> +.BR landlock_create_ruleset (2).
> +.PP
> +When building fallback mechanisms for compatibility with older kernels,
> +users are advised to consider the special semantics of the
> +.B LANDLOCK_ACCESS_FS_REFER
> +access right: In ABI v1, linking and moving of files between different
> +directories is always forbidden, so programs relying on such
> +operations are only compatible with Landlock ABI v2 and higher.
>   .SH NOTES
>   Landlock is enabled by
>   .BR CONFIG_SECURITY_LANDLOCK .
> @@ -242,7 +327,8 @@ attr.handled_access_fs =
>           LANDLOCK_ACCESS_FS_MAKE_SOCK |
>           LANDLOCK_ACCESS_FS_MAKE_FIFO |
>           LANDLOCK_ACCESS_FS_MAKE_BLOCK |
> -        LANDLOCK_ACCESS_FS_MAKE_SYM;
> +        LANDLOCK_ACCESS_FS_MAKE_SYM |
> +        LANDLOCK_ACCESS_FS_REFER;
>   
>   ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
>   if (ruleset_fd == -1) {

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]