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

[PATCH v4] 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.

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

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index f3223f96469..81d09ef9aa5 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -130,21 +130,37 @@ 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 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.
+ *
+ *   In particular, when using the first Landlock ABI version, Landlock will
+ *   always deny attempts to reparent 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: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
-- 
2.39.2

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

[Mickaël Salaün]

Perfect, thanks for your perseverance Günther! I've applied it to my 
-next tree: 
https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git/log/?h=next

On 21/02/2023 17:52, 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.
> 
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
>   include/uapi/linux/landlock.h | 46 +++++++++++++++++++++++------------
>   1 file changed, 31 insertions(+), 15 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f3223f96469..81d09ef9aa5 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -130,21 +130,37 @@ 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 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.
> + *
> + *   In particular, when using the first Landlock ABI version, Landlock will
> + *   always deny attempts to reparent 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: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c

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

[Alex Colomar]

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

Hi Günther,

On 2/21/23 17:52, Günther Noack wrote:
[...]

Sorry for taking so much time in looking into this!

> + *
> + *   * 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``.

Is this something worth guaranteeing, or just an implementation detail 
that may easily change?

Cheers,

Alex

>    *
>    * .. warning::
>    *
> 
> base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c

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


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

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

[Mickaël Salaün]

On 25/02/2023 00:00, Alex Colomar wrote:
> Hi Günther,
> 
> On 2/21/23 17:52, Günther Noack wrote:
> [...]
> 
> Sorry for taking so much time in looking into this!
> 
>> + *
>> + *   * 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``.
> 
> Is this something worth guaranteeing, or just an implementation detail
> that may easily change?

There is a deliberate effort to keep this error priority to be able to 
know if a rename or link action can be replaced with a file copy, which 
is useful e.g., for `mv`.

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

[Alejandro Colomar]

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

Hi Mickaël,

On 2/27/23 12:02, Mickaël Salaün wrote:
> 
> On 25/02/2023 00:00, Alex Colomar wrote:
>> Hi Günther,
>>
>> On 2/21/23 17:52, Günther Noack wrote:
>> [...]
>>
>> Sorry for taking so much time in looking into this!
>>
>>> + *
>>> + *   * 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``.
>>
>> Is this something worth guaranteeing, or just an implementation detail
>> that may easily change?
> 
> There is a deliberate effort to keep this error priority to be able to 
> know if a rename or link action can be replaced with a file copy, which 
> is useful e.g., for `mv`.

Thanks.  Then it LGTM.

Cheers!

Alex


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

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