[PATCH v3] landlock: Clarify documentation for the LANDLOCK_ACCESS_FS_REFER right

[PATCH v3] landlock: Clarify documentation for the LANDLOCK_ACCESS_FS_REFER right

[Günther Noack]

Clarify the "refer" documentation by splitting up a big paragraph of text.

- Call out specifically that the denial by default applies to ABI v1 as well.
- Turn the three additional constraints for link/rename operations
  into bullet points, to give it more structure.

Includes wording and semantics corrections by Mickaël Salaün.

Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
 include/uapi/linux/landlock.h | 45 +++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index f3223f96469..e549ad6360b 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -130,21 +130,36 @@ struct landlock_path_beneath_attr {
  * - %LANDLOCK_ACCESS_FS_MAKE_BLOCK: Create (or rename or link) a block device.
  * - %LANDLOCK_ACCESS_FS_MAKE_SYM: Create (or rename or link) a symbolic link.
  * - %LANDLOCK_ACCESS_FS_REFER: Link or rename a file from or to a different
- *   directory (i.e. reparent a file hierarchy).  This access right is
- *   available since the second version of the Landlock ABI.  This is also the
- *   only access right which is always considered handled by any ruleset in
- *   such a way that reparenting a file hierarchy is always denied by default.
- *   To avoid privilege escalation, it is not enough to add a rule with this
- *   access right.  When linking or renaming a file, the destination directory
- *   hierarchy must also always have the same or a superset of restrictions of
- *   the source hierarchy.  If it is not the case, or if the domain doesn't
- *   handle this access right, such actions are denied by default with errno
- *   set to ``EXDEV``.  Linking also requires a ``LANDLOCK_ACCESS_FS_MAKE_*``
- *   access right on the destination directory, and renaming also requires a
- *   ``LANDLOCK_ACCESS_FS_REMOVE_*`` access right on the source's (file or
- *   directory) parent.  Otherwise, such actions are denied with errno set to
- *   ``EACCES``.  The ``EACCES`` errno prevails over ``EXDEV`` to let user space
- *   efficiently deal with an unrecoverable error.
+ *   directory (i.e. reparent a file hierarchy).
+ *
+ *   This access right is available since the second version of the Landlock
+ *   ABI.  This is also the only access right which is implicitly handled by any
+ *   ruleset, even if the right is not specified at the time of creating the
+ *   ruleset.  So, by default, Landlock will deny linking and reparenting files
+ *   between different directories, and will only grant this right when it is
+ *   explicitly permitted for a directory by adding a rule.
+ *
+ *   When using the first Landlock ABI version, Landlock will always deny the
+ *   reparenting of files between different directories.
+ *
+ *   In addition to the source and destination directories having the
+ *   %LANDLOCK_ACCESS_FS_REFER access right, the attempted link or rename
+ *   operation must meet the following constraints:
+ *
+ *   * 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 ``EXDEV`` error.
+ *
+ *   * When linking or renaming, the ``LANDLOCK_ACCESS_FS_MAKE_*`` right for the
+ *     respective file type must be granted for the destination directory.
+ *     Otherwise, the operation results in an ``EACCES`` error.
+ *
+ *   * When renaming, the ``LANDLOCK_ACCESS_FS_REMOVE_*`` right for the
+ *     respective file type must be granted for the source directory.  Otherwise,
+ *     the operation results in an ``EACCES`` error.
+ *
+ *   If multiple requirements are not met, the ``EACCES`` error code takes
+ *   precedence over ``EXDEV``.
  *
  * .. warning::
  *

base-commit: ceaa837f96adb69c0df0397937cd74991d5d821a
-- 
2.39.1

Re: [PATCH v3] landlock: Clarify documentation for the LANDLOCK_ACCESS_FS_REFER right

[Mickaël Salaün]

(adding linux-doc in Cc)

On 16/02/2023 21:07, Günther Noack wrote:
> Clarify the "refer" documentation by splitting up a big paragraph of text.
> 
> - Call out specifically that the denial by default applies to ABI v1 as well.
> - Turn the three additional constraints for link/rename operations
>    into bullet points, to give it more structure.
> 
> Includes wording and semantics corrections by Mickaël Salaün.

No need to add this line, It's part of the maintainer job. ;)

> 
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
>   include/uapi/linux/landlock.h | 45 +++++++++++++++++++++++------------
>   1 file changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f3223f96469..e549ad6360b 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -130,21 +130,36 @@ struct landlock_path_beneath_attr {
>    * - %LANDLOCK_ACCESS_FS_MAKE_BLOCK: Create (or rename or link) a block device.
>    * - %LANDLOCK_ACCESS_FS_MAKE_SYM: Create (or rename or link) a symbolic link.
>    * - %LANDLOCK_ACCESS_FS_REFER: Link or rename a file from or to a different
> - *   directory (i.e. reparent a file hierarchy).  This access right is
> - *   available since the second version of the Landlock ABI.  This is also the
> - *   only access right which is always considered handled by any ruleset in
> - *   such a way that reparenting a file hierarchy is always denied by default.
> - *   To avoid privilege escalation, it is not enough to add a rule with this
> - *   access right.  When linking or renaming a file, the destination directory
> - *   hierarchy must also always have the same or a superset of restrictions of
> - *   the source hierarchy.  If it is not the case, or if the domain doesn't
> - *   handle this access right, such actions are denied by default with errno
> - *   set to ``EXDEV``.  Linking also requires a ``LANDLOCK_ACCESS_FS_MAKE_*``
> - *   access right on the destination directory, and renaming also requires a
> - *   ``LANDLOCK_ACCESS_FS_REMOVE_*`` access right on the source's (file or
> - *   directory) parent.  Otherwise, such actions are denied with errno set to
> - *   ``EACCES``.  The ``EACCES`` errno prevails over ``EXDEV`` to let user space
> - *   efficiently deal with an unrecoverable error.
> + *   directory (i.e. reparent a file hierarchy).
> + *
> + *   This access right is available since the second version of the Landlock
> + *   ABI.  This is also the only access right which is implicitly handled by any
> + *   ruleset, even if the right is not specified at the time of creating the
> + *   ruleset.  So, by default, Landlock will deny linking and reparenting files > + *   between different directories, and will only grant this right 
when it is
> + *   explicitly permitted for a directory by adding a rule.
> + *
> + *   When using the first Landlock ABI version, Landlock will always deny the
> + *   reparenting of files between different directories.
> + *
> + *   In addition to the source and destination directories having the
> + *   %LANDLOCK_ACCESS_FS_REFER access right, the attempted link or rename

Some of my suggestions are about style, so feel free to ignore them if 
you think the original is better. Anyway, I'm not a native english 
speaker either, so there are good chances I'm not correct on some 
suggestions. What about that?:

This is the only access right implicitly handled by any ruleset, even if 
this right is not specified at ruleset creation time. Reparenting files 
will then always be denied by default. Given that 
%LANDLOCK_ACCESS_FS_REFER is available since the second Landlock ABI 
version, using the first Landlock ABI version will always forbid file 
reparenting.

For these kind of link or rename actions to be possible, one or two 
rules must explicitly allow %LANDLOCK_ACCESS_FS_REFER on the source and 
the destination hierarchies. In addition, the following constraints must 
be met:


> + *   operation must meet the following constraints:
> + *
> + *   * 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 ``EXDEV`` error.
> + *
> + *   * When linking or renaming, the ``LANDLOCK_ACCESS_FS_MAKE_*`` right for the
> + *     respective file type must be granted for the destination directory.
> + *     Otherwise, the operation results in an ``EACCES`` error.
> + *
> + *   * When renaming, the ``LANDLOCK_ACCESS_FS_REMOVE_*`` right for the
> + *     respective file type must be granted for the source directory.  Otherwise,
> + *     the operation results in an ``EACCES`` error.
> + *
> + *   If multiple requirements are not met, the ``EACCES`` error code takes
> + *   precedence over ``EXDEV``.
>    *
>    * .. warning::
>    *
> 
> base-commit: ceaa837f96adb69c0df0397937cd74991d5d821a

Re: [PATCH v3] landlock: Clarify documentation for the LANDLOCK_ACCESS_FS_REFER right

[Günther Noack]

On Fri, Feb 17, 2023 at 08:28:41PM +0100, Mickaël Salaün wrote:
> On 16/02/2023 21:07, Günther Noack wrote:
> > Clarify the "refer" documentation by splitting up a big paragraph of text.
> > 
> > - Call out specifically that the denial by default applies to ABI v1 as well.
> > - Turn the three additional constraints for link/rename operations
> >    into bullet points, to give it more structure.
> > 
> > Includes wording and semantics corrections by Mickaël Salaün.
> 
> No need to add this line, It's part of the maintainer job. ;)

OK, removed for V4.

> Some of my suggestions are about style, so feel free to ignore them if you
> think the original is better. Anyway, I'm not a native english speaker
> either, so there are good chances I'm not correct on some suggestions. What
> about that?:
> 
> This is the only access right implicitly handled by any ruleset, even if
> this right is not specified at ruleset creation time. Reparenting files will
> then always be denied by default. Given that %LANDLOCK_ACCESS_FS_REFER is
> available since the second Landlock ABI version, using the first Landlock
> ABI version will always forbid file reparenting.
> 
> For these kind of link or rename actions to be possible, one or two rules
> must explicitly allow %LANDLOCK_ACCESS_FS_REFER on the source and the
> destination hierarchies. In addition, the following constraints must be met:

I reworded it again, it's meeting somewhere in the middle I hope. It
should be a bit better now. (Sending another version.)

Documentation is hard... it's difficult to find an objective best wording.

–-Günther