[PATCH v5 1/3] landlock.7: Document Landlock ABI v2 (file reparenting; Linux 5.19)

[PATCH v5 1/3] landlock.7: Document Landlock ABI v2 (file reparenting; Linux 5.19)

[Günther Noack]

* 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://git.kernel.org/torvalds/c/cb44e4f061e16be65b8a16505e121490c66d30d0
* https://lore.kernel.org/all/20230221165205.4231-1-gnoack3000@gmail.com/ (documentation update)

Signed-off-by: Günther Noack <gnoack3000@gmail.com>
Reviewed-by: Mickaël Salaün <mic@digikod.net>
---
 man7/landlock.7 | 102 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 100 insertions(+), 2 deletions(-)

diff --git a/man7/landlock.7 b/man7/landlock.7
index bbe00d4c2..d3a7ec0d2 100644
--- a/man7/landlock.7
+++ b/man7/landlock.7
@@ -105,6 +105,56 @@ 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).
+.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
+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]
+When linking or renaming, the
+.BI 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]
+When renaming, the
+.BI LANDLOCK_ACCESS_FS_REMOVE_ *
+right for the respective file type must be granted
+for the source directory.
+Otherwise, the operation results in an
+.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 +232,54 @@ 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
+To determine which Landlock features are available,
+users should query the Landlock ABI version:
+.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
+\^	\^	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
+.sp 1
+.PP
+Users should use the Landlock ABI version rather than the kernel version
+to determine which features are available.
+The mainline kernel versions listed here are only included for orientation.
+Kernels from other sources may contain backported features,
+and their version numbers may not match.
+.PP
+To query the running kernel's Landlock ABI version,
+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 +339,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) {

base-commit: fd00f831b52d61a91d59cb3b46182869145d9700
-- 
2.39.2

[PATCH v5 2/3] landlock.7: Document Landlock ABI v3 (file truncation; Linux 6.2)

[Günther Noack]

https://git.kernel.org/torvalds/c/299e2b1967578b1442128ba8b3e86ed3427d3651

Signed-off-by: Günther Noack <gnoack3000@gmail.com>
Reviewed-by: Mickaël Salaün <mic@digikod.net>
---
 man7/landlock.7 | 83 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 81 insertions(+), 2 deletions(-)

diff --git a/man7/landlock.7 b/man7/landlock.7
index d3a7ec0d2..9c305edef 100644
--- a/man7/landlock.7
+++ b/man7/landlock.7
@@ -64,9 +64,39 @@ Execute a file.
 .TP
 .B LANDLOCK_ACCESS_FS_WRITE_FILE
 Open a file with write access.
+.IP
+When opening files for writing,
+you will often additionally need the
+.B LANDLOCK_ACCESS_FS_TRUNCATE
+right.
+In many cases,
+these system calls truncate existing files when overwriting them
+(e.g.,
+.BR creat (2)).
 .TP
 .B LANDLOCK_ACCESS_FS_READ_FILE
 Open a file with read access.
+.TP
+.B LANDLOCK_ACCESS_FS_TRUNCATE
+Truncate a file with
+.BR truncate (2),
+.BR ftruncate (2),
+.BR creat (2),
+or
+.BR open (2)
+with
+.BR O_TRUNC .
+Whether an opened file can be truncated with
+.BR ftruncate (2)
+is determined during
+.BR open (2),
+in the same way as read and write permissions are checked during
+.BR open (2)
+using
+.B LANDLOCK_ACCESS_FS_READ_FILE
+and
+.BR LANDLOCK_ACCESS_FS_WRITE_FILE .
+This access right is available since the third version of the Landlock ABI.
 .PP
 A directory can receive access rights related to files or directories.
 The following access right is applied to the directory itself,
@@ -231,6 +261,53 @@ To be allowed to use
 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.
+.\"
+.SS Truncating files
+The operations covered by
+.B LANDLOCK_ACCESS_FS_WRITE_FILE
+and
+.B 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.
+.PP
+A particularly surprising example is
+.BR 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.
+.PP
+It should also be noted that truncating files does not require the
+.B LANDLOCK_ACCESS_FS_WRITE_FILE
+right.
+Apart from the
+.BR truncate (2)
+system call, this can also be done through
+.BR open (2)
+with the flags
+.IR "O_RDONLY\ |\ O_TRUNC" .
+.PP
+When opening a file, the availability of the
+.B LANDLOCK_ACCESS_FS_TRUNCATE
+right is associated with the newly created file descriptor
+and will be used for subsequent truncation attempts using
+.BR ftruncate (2).
+The behavior is similar to opening a file for reading or writing,
+where permissions are checked during
+.BR open (2),
+but not during the subsequent
+.BR read (2)
+and
+.BR write (2)
+calls.
+.PP
+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.
 .SH VERSIONS
 Landlock was introduced in Linux 5.13.
 .PP
@@ -257,6 +334,8 @@ _	_	_
 \^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
 _	_	_
 2	5.19	LANDLOCK_ACCESS_FS_REFER
+_	_	_
+3	6.2	LANDLOCK_ACCESS_FS_TRUNCATE
 .TE
 .sp 1
 .PP
@@ -302,7 +381,6 @@ in kernel logs.
 It is currently not possible to restrict some file-related actions
 accessible through these system call families:
 .BR chdir (2),
-.BR truncate (2),
 .BR stat (2),
 .BR flock (2),
 .BR chmod (2),
@@ -340,7 +418,8 @@ attr.handled_access_fs =
         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;
 
 ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
 if (ruleset_fd == \-1) {
-- 
2.39.2

[PATCH v5 3/3] landlock.7: Explain the best-effort fallback mechanism in the example

[Günther Noack]

Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
 man7/landlock.7 | 65 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/man7/landlock.7 b/man7/landlock.7
index 9c305edef..d1214ba27 100644
--- a/man7/landlock.7
+++ b/man7/landlock.7
@@ -393,11 +393,14 @@ accessible through these system call families:
 Future Landlock evolutions will enable to restrict them.
 .SH EXAMPLES
 We first need to create the ruleset that will contain our rules.
+.PP
 For this example,
 the ruleset will contain rules that only allow read actions,
 but write actions will be denied.
 The ruleset then needs to handle both of these kinds of actions.
-See below for the description of filesystem actions.
+See the
+.B DESCRIPTION
+section for the description of filesystem actions.
 .PP
 .in +4n
 .EX
@@ -420,7 +423,57 @@ attr.handled_access_fs =
         LANDLOCK_ACCESS_FS_MAKE_SYM |
         LANDLOCK_ACCESS_FS_REFER |
         LANDLOCK_ACCESS_FS_TRUNCATE;
+.EE
+.in
+.PP
+To be compatible with older Linux versions,
+we detect the available Landlock ABI version,
+and only use the available subset of access rights:
+.PP
+.in +4n
+.EX
+/* Table of available file system access rights by ABI version */
+__u64 landlock_fs_access_rights[] = {
+    (1ULL << 13) \- 1,  /* ABI v1                 */
+    (1ULL << 14) \- 1,  /* ABI v2: add "refer"    */
+    (1ULL << 15) \- 1,  /* ABI v3: add "truncate" */
+};
+
+int abi = landlock_create_ruleset(NULL, 0,
+                                  LANDLOCK_CREATE_RULESET_VERSION);
+if (abi <= 0) {
+    perror("Giving up \- No Landlock support");
+    exit(EXIT_FAILURE);
+}
+if (abi > 3)
+    abi = 3;
 
+/* Only use the available rights in the ruleset. */
+attr.handled_access_fs &= landlock_fs_access_rights[abi \- 1];
+.EE
+.in
+.PP
+The available access rights for each ABI version are listed in the
+.B VERSIONS
+section.
+.PP
+If our program needed to create hard links or rename files between different directories
+.RB ( LANDLOCK_ACCESS_FS_REFER ),
+we would require the following change to the backwards compatibility logic:
+Directory reparenting is not possible in a process restricted with Landlock ABI version 1.
+Therefore,
+if the program needed to do file reparenting,
+and if only Landlock ABI version 1 was available,
+we could not restrict the process.
+.PP
+Now that the ruleset attributes are determined,
+we create the Landlock ruleset
+and acquire a file descriptor as a handle to it,
+using
+.BR landlock_create_ruleset (2):
+.PP
+.in +4n
+.EX
 ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
 if (ruleset_fd == \-1) {
     perror("Failed to create a ruleset");
@@ -429,9 +482,13 @@ if (ruleset_fd == \-1) {
 .EE
 .in
 .PP
-We can now add a new rule to this ruleset thanks to the returned file
-descriptor referring to this ruleset.
-The rule will only allow reading the file hierarchy
+We can now add a new rule to the ruleset through the ruleset's file descriptor.
+The requested access rights must be a subset of the access rights
+which were specified in
+.I attr.handled_access_fs
+at ruleset creation time.
+.PP
+In this example, the rule will only allow reading the file hierarchy
 .IR /usr .
 Without another rule, write actions would then be denied by the ruleset.
 To add
-- 
2.39.2

Re: [PATCH v5 3/3] landlock.7: Explain the best-effort fallback mechanism in the example

[Günther Noack]

P.S.: I went back and forth a bit with the example,
but ended up implementing the full backwards compatibility
across all three existing Landlock versions for now.

The example corresponds to the simple case 1 from
https://blog.gnoack.org/post/landlock-best-effort/,
where the program does *not* need the refer right
after Landlock restriction.
This is not much more complicated than the variant
which is compatible with ABI V2 and V3.

Below the new part of the example,
I added a paragraph to talk about case 2,
where the program *does* need the refer right.
I hope this will be enough to keep most people
from implementing the fallback the wrong way.

Case 3 from the weblog article (where it can be either)
can be deduced with normal logic reasoning and hopefully
should not need additional explanation. -- You need
to figure out at runtime whether you are in case 1 or 2,
and then apply the respective logic.

Let me know what you think!
–Günther

On Fri, Mar 24, 2023 at 06:24:19PM +0100, Günther Noack wrote:
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
>  man7/landlock.7 | 65 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 61 insertions(+), 4 deletions(-)
> 
> diff --git a/man7/landlock.7 b/man7/landlock.7
> index 9c305edef..d1214ba27 100644
> --- a/man7/landlock.7
> +++ b/man7/landlock.7
> @@ -393,11 +393,14 @@ accessible through these system call families:
>  Future Landlock evolutions will enable to restrict them.
>  .SH EXAMPLES
>  We first need to create the ruleset that will contain our rules.
> +.PP
>  For this example,
>  the ruleset will contain rules that only allow read actions,
>  but write actions will be denied.
>  The ruleset then needs to handle both of these kinds of actions.
> -See below for the description of filesystem actions.
> +See the
> +.B DESCRIPTION
> +section for the description of filesystem actions.
>  .PP
>  .in +4n
>  .EX
> @@ -420,7 +423,57 @@ attr.handled_access_fs =
>          LANDLOCK_ACCESS_FS_MAKE_SYM |
>          LANDLOCK_ACCESS_FS_REFER |
>          LANDLOCK_ACCESS_FS_TRUNCATE;
> +.EE
> +.in
> +.PP
> +To be compatible with older Linux versions,
> +we detect the available Landlock ABI version,
> +and only use the available subset of access rights:
> +.PP
> +.in +4n
> +.EX
> +/* Table of available file system access rights by ABI version */
> +__u64 landlock_fs_access_rights[] = {
> +    (1ULL << 13) \- 1,  /* ABI v1                 */
> +    (1ULL << 14) \- 1,  /* ABI v2: add "refer"    */
> +    (1ULL << 15) \- 1,  /* ABI v3: add "truncate" */
> +};
> +
> +int abi = landlock_create_ruleset(NULL, 0,
> +                                  LANDLOCK_CREATE_RULESET_VERSION);
> +if (abi <= 0) {
> +    perror("Giving up \- No Landlock support");
> +    exit(EXIT_FAILURE);
> +}
> +if (abi > 3)
> +    abi = 3;
>  
> +/* Only use the available rights in the ruleset. */
> +attr.handled_access_fs &= landlock_fs_access_rights[abi \- 1];
> +.EE
> +.in
> +.PP
> +The available access rights for each ABI version are listed in the
> +.B VERSIONS
> +section.
> +.PP
> +If our program needed to create hard links or rename files between different directories
> +.RB ( LANDLOCK_ACCESS_FS_REFER ),
> +we would require the following change to the backwards compatibility logic:
> +Directory reparenting is not possible in a process restricted with Landlock ABI version 1.
> +Therefore,
> +if the program needed to do file reparenting,
> +and if only Landlock ABI version 1 was available,
> +we could not restrict the process.
> +.PP
> +Now that the ruleset attributes are determined,
> +we create the Landlock ruleset
> +and acquire a file descriptor as a handle to it,
> +using
> +.BR landlock_create_ruleset (2):
> +.PP
> +.in +4n
> +.EX
>  ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
>  if (ruleset_fd == \-1) {
>      perror("Failed to create a ruleset");
> @@ -429,9 +482,13 @@ if (ruleset_fd == \-1) {
>  .EE
>  .in
>  .PP
> -We can now add a new rule to this ruleset thanks to the returned file
> -descriptor referring to this ruleset.
> -The rule will only allow reading the file hierarchy
> +We can now add a new rule to the ruleset through the ruleset's file descriptor.
> +The requested access rights must be a subset of the access rights
> +which were specified in
> +.I attr.handled_access_fs
> +at ruleset creation time.
> +.PP
> +In this example, the rule will only allow reading the file hierarchy
>  .IR /usr .
>  Without another rule, write actions would then be denied by the ruleset.
>  To add
> -- 
> 2.39.2
> 

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

[Alejandro Colomar]

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

Hi Günther, Mickaël,

On 3/24/23 18:24, 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://git.kernel.org/torvalds/c/cb44e4f061e16be65b8a16505e121490c66d30d0
> * https://lore.kernel.org/all/20230221165205.4231-1-gnoack3000@gmail.com/ (documentation update)
> 
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> Reviewed-by: Mickaël Salaün <mic@digikod.net>

Patch applied.

Thanks,
Alex

> ---
>  man7/landlock.7 | 102 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 100 insertions(+), 2 deletions(-)
> 
> diff --git a/man7/landlock.7 b/man7/landlock.7
> index bbe00d4c2..d3a7ec0d2 100644
> --- a/man7/landlock.7
> +++ b/man7/landlock.7
> @@ -105,6 +105,56 @@ 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).
> +.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
> +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]
> +When linking or renaming, the
> +.BI 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]
> +When renaming, the
> +.BI LANDLOCK_ACCESS_FS_REMOVE_ *
> +right for the respective file type must be granted
> +for the source directory.
> +Otherwise, the operation results in an
> +.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 +232,54 @@ 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
> +To determine which Landlock features are available,
> +users should query the Landlock ABI version:
> +.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
> +\^	\^	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
> +.sp 1
> +.PP
> +Users should use the Landlock ABI version rather than the kernel version
> +to determine which features are available.
> +The mainline kernel versions listed here are only included for orientation.
> +Kernels from other sources may contain backported features,
> +and their version numbers may not match.
> +.PP
> +To query the running kernel's Landlock ABI version,
> +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 +339,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) {
> 
> base-commit: fd00f831b52d61a91d59cb3b46182869145d9700

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

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

Re: [PATCH v5 2/3] landlock.7: Document Landlock ABI v3 (file truncation; Linux 6.2)

[Alejandro Colomar]

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



On 3/24/23 18:24, Günther Noack wrote:
> https://git.kernel.org/torvalds/c/299e2b1967578b1442128ba8b3e86ed3427d3651
> 
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> Reviewed-by: Mickaël Salaün <mic@digikod.net>

Patch applied.  Thanks!

Cheers,
Alex

> ---
>  man7/landlock.7 | 83 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 81 insertions(+), 2 deletions(-)
> 
> diff --git a/man7/landlock.7 b/man7/landlock.7
> index d3a7ec0d2..9c305edef 100644
> --- a/man7/landlock.7
> +++ b/man7/landlock.7
> @@ -64,9 +64,39 @@ Execute a file.
>  .TP
>  .B LANDLOCK_ACCESS_FS_WRITE_FILE
>  Open a file with write access.
> +.IP
> +When opening files for writing,
> +you will often additionally need the
> +.B LANDLOCK_ACCESS_FS_TRUNCATE
> +right.
> +In many cases,
> +these system calls truncate existing files when overwriting them
> +(e.g.,
> +.BR creat (2)).
>  .TP
>  .B LANDLOCK_ACCESS_FS_READ_FILE
>  Open a file with read access.
> +.TP
> +.B LANDLOCK_ACCESS_FS_TRUNCATE
> +Truncate a file with
> +.BR truncate (2),
> +.BR ftruncate (2),
> +.BR creat (2),
> +or
> +.BR open (2)
> +with
> +.BR O_TRUNC .
> +Whether an opened file can be truncated with
> +.BR ftruncate (2)
> +is determined during
> +.BR open (2),
> +in the same way as read and write permissions are checked during
> +.BR open (2)
> +using
> +.B LANDLOCK_ACCESS_FS_READ_FILE
> +and
> +.BR LANDLOCK_ACCESS_FS_WRITE_FILE .
> +This access right is available since the third version of the Landlock ABI.
>  .PP
>  A directory can receive access rights related to files or directories.
>  The following access right is applied to the directory itself,
> @@ -231,6 +261,53 @@ To be allowed to use
>  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.
> +.\"
> +.SS Truncating files
> +The operations covered by
> +.B LANDLOCK_ACCESS_FS_WRITE_FILE
> +and
> +.B 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.
> +.PP
> +A particularly surprising example is
> +.BR 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.
> +.PP
> +It should also be noted that truncating files does not require the
> +.B LANDLOCK_ACCESS_FS_WRITE_FILE
> +right.
> +Apart from the
> +.BR truncate (2)
> +system call, this can also be done through
> +.BR open (2)
> +with the flags
> +.IR "O_RDONLY\ |\ O_TRUNC" .
> +.PP
> +When opening a file, the availability of the
> +.B LANDLOCK_ACCESS_FS_TRUNCATE
> +right is associated with the newly created file descriptor
> +and will be used for subsequent truncation attempts using
> +.BR ftruncate (2).
> +The behavior is similar to opening a file for reading or writing,
> +where permissions are checked during
> +.BR open (2),
> +but not during the subsequent
> +.BR read (2)
> +and
> +.BR write (2)
> +calls.
> +.PP
> +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.
>  .SH VERSIONS
>  Landlock was introduced in Linux 5.13.
>  .PP
> @@ -257,6 +334,8 @@ _	_	_
>  \^	\^	LANDLOCK_ACCESS_FS_MAKE_SYM
>  _	_	_
>  2	5.19	LANDLOCK_ACCESS_FS_REFER
> +_	_	_
> +3	6.2	LANDLOCK_ACCESS_FS_TRUNCATE
>  .TE
>  .sp 1
>  .PP
> @@ -302,7 +381,6 @@ in kernel logs.
>  It is currently not possible to restrict some file-related actions
>  accessible through these system call families:
>  .BR chdir (2),
> -.BR truncate (2),
>  .BR stat (2),
>  .BR flock (2),
>  .BR chmod (2),
> @@ -340,7 +418,8 @@ attr.handled_access_fs =
>          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;
>  
>  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 --]

Re: [PATCH v5 3/3] landlock.7: Explain the best-effort fallback mechanism in the example

[Alejandro Colomar]

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

Hi Günther,

On 3/24/23 18:24, Günther Noack wrote:
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
>  man7/landlock.7 | 65 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 61 insertions(+), 4 deletions(-)
> 
> diff --git a/man7/landlock.7 b/man7/landlock.7
> index 9c305edef..d1214ba27 100644
> --- a/man7/landlock.7
> +++ b/man7/landlock.7
> @@ -393,11 +393,14 @@ accessible through these system call families:
>  Future Landlock evolutions will enable to restrict them.
>  .SH EXAMPLES
>  We first need to create the ruleset that will contain our rules.
> +.PP
>  For this example,
>  the ruleset will contain rules that only allow read actions,
>  but write actions will be denied.
>  The ruleset then needs to handle both of these kinds of actions.
> -See below for the description of filesystem actions.
> +See the
> +.B DESCRIPTION
> +section for the description of filesystem actions.
>  .PP
>  .in +4n
>  .EX
> @@ -420,7 +423,57 @@ attr.handled_access_fs =
>          LANDLOCK_ACCESS_FS_MAKE_SYM |
>          LANDLOCK_ACCESS_FS_REFER |
>          LANDLOCK_ACCESS_FS_TRUNCATE;
> +.EE
> +.in
> +.PP
> +To be compatible with older Linux versions,
> +we detect the available Landlock ABI version,
> +and only use the available subset of access rights:
> +.PP
> +.in +4n
> +.EX
> +/* Table of available file system access rights by ABI version */
> +__u64 landlock_fs_access_rights[] = {
> +    (1ULL << 13) \- 1,  /* ABI v1                 */
> +    (1ULL << 14) \- 1,  /* ABI v2: add "refer"    */
> +    (1ULL << 15) \- 1,  /* ABI v3: add "truncate" */

Do these magic numbers have macros?  Are users expected to use
the magic numbers directly?

> +};
> +
> +int abi = landlock_create_ruleset(NULL, 0,
> +                                  LANDLOCK_CREATE_RULESET_VERSION);
> +if (abi <= 0) {
> +    perror("Giving up \- No Landlock support");
> +    exit(EXIT_FAILURE);
> +}
> +if (abi > 3)
> +    abi = 3;
>  
> +/* Only use the available rights in the ruleset. */
> +attr.handled_access_fs &= landlock_fs_access_rights[abi \- 1];
> +.EE
> +.in
> +.PP
> +The available access rights for each ABI version are listed in the
> +.B VERSIONS
> +section.
> +.PP
> +If our program needed to create hard links or rename files between different directories

Please keep lines below 80 columns.  Break lines at phrase
boundaries as appropriate (e.g., in this line:)

s/ or /\nor /

Cheers,
Alex

> +.RB ( LANDLOCK_ACCESS_FS_REFER ),
> +we would require the following change to the backwards compatibility logic:
> +Directory reparenting is not possible in a process restricted with Landlock ABI version 1.
> +Therefore,
> +if the program needed to do file reparenting,
> +and if only Landlock ABI version 1 was available,
> +we could not restrict the process.
> +.PP
> +Now that the ruleset attributes are determined,
> +we create the Landlock ruleset
> +and acquire a file descriptor as a handle to it,
> +using
> +.BR landlock_create_ruleset (2):
> +.PP
> +.in +4n
> +.EX
>  ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
>  if (ruleset_fd == \-1) {
>      perror("Failed to create a ruleset");
> @@ -429,9 +482,13 @@ if (ruleset_fd == \-1) {
>  .EE
>  .in
>  .PP
> -We can now add a new rule to this ruleset thanks to the returned file
> -descriptor referring to this ruleset.
> -The rule will only allow reading the file hierarchy
> +We can now add a new rule to the ruleset through the ruleset's file descriptor.
> +The requested access rights must be a subset of the access rights
> +which were specified in
> +.I attr.handled_access_fs
> +at ruleset creation time.
> +.PP
> +In this example, the rule will only allow reading the file hierarchy
>  .IR /usr .
>  Without another rule, write actions would then be denied by the ruleset.
>  To add

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

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

Re: [PATCH v5 3/3] landlock.7: Explain the best-effort fallback mechanism in the example

[Günther Noack]

Hello Alejandro!

On Sat, Apr 01, 2023 at 12:29:35AM +0200, Alejandro Colomar wrote:
> Hi Günther,
> 
> On 3/24/23 18:24, Günther Noack wrote:
> > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > ---
> >  man7/landlock.7 | 65 ++++++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 61 insertions(+), 4 deletions(-)
> > 
> > diff --git a/man7/landlock.7 b/man7/landlock.7
> > index 9c305edef..d1214ba27 100644
> > --- a/man7/landlock.7
> > +++ b/man7/landlock.7
> > [...]
> > +.EX
> > +/* Table of available file system access rights by ABI version */
> > +__u64 landlock_fs_access_rights[] = {
> > +    (1ULL << 13) \- 1,  /* ABI v1                 */
> > +    (1ULL << 14) \- 1,  /* ABI v2: add "refer"    */
> > +    (1ULL << 15) \- 1,  /* ABI v3: add "truncate" */
> 
> Do these magic numbers have macros?  Are users expected to use
> the magic numbers directly?

You are right to point this out - it's the ugly part here :-/

These are bitmasks for the different access rights which are supported
at each ABI version, so they are a bitwise OR of a long list of
LANDLOCK_ACCESS_FS_* constants.

I am aware of three ways to write this out.
Please let me know which of these three you prefer.
(or maybe you have an idea for another alternative?).

(It feels out of scope for this documentation patch, but do you think
these bitmasks should be defined in the uapi/linux/landlock.h header?
You have looked at so many man pages already -- Do you happen to know
other places in the kernel API where such a problem has come up?)


1) Make assumptions about the numbers, for brevity
   (as done in the patch I sent).

__u64 landlock_fs_access_rights[] = {
    (1ULL << 13) - 1,  /* ABI v1                 */
    (1ULL << 14) - 1,  /* ABI v2: add "refer"    */
    (1ULL << 15) - 1,  /* ABI v3: add "truncate" */
}

This is the shortest variant,
but it does not use the Landlock constants from the header.


2) Use the constants from the header and OR them.

This is the "most correct" way, but I feel that it might be too
verbose for a documentation example.  What do you think?

__u64 landlock_fs_access_rights[] = {
    /* ABI v1 */
    LANDLOCK_ACCESS_FS_EXECUTE |
    LANDLOCK_ACCESS_FS_WRITE_FILE |
    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,

    /* ABI v2: Add "refer" */
    LANDLOCK_ACCESS_FS_EXECUTE |
    LANDLOCK_ACCESS_FS_WRITE_FILE |
    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 |
    LANDLOCK_ACCESS_FS_REFER,
    
    /* ABI v3: add "truncate" */
    LANDLOCK_ACCESS_FS_EXECUTE |
    LANDLOCK_ACCESS_FS_WRITE_FILE |
    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,
    LANDLOCK_ACCESS_FS_REFER |
    LANDLOCK_ACCESS_FS_TRUNCATE,
}

If I were to write production code, I would probably write it out like
that, to be explicit -- but it feels like a long code example for a
man page? WDYT?


3) Third option is the middle way,
   naming the "highest" known access right for each ABI version:

__u64 landlock_fs_access_rights[] = {
    (LANDLOCK_ACCESS_FS_MAKE_SYM << 1) - 1,  /* ABI v1                 */
    (LANDLOCK_ACCESS_FS_REFER << 1) - 1,     /* ABI v2: add "refer"    */
    (LANDLOCK_ACCESS_FS_TRUNCATE << 1) - 1,  /* ABI v3: add "truncate" */
}

In this case, we still make the assumption that the supported rights
are the "highest" right plus all of the bits in lower order places.


> > +/* Only use the available rights in the ruleset. */
> > +attr.handled_access_fs &= landlock_fs_access_rights[abi \- 1];
> > +.EE
> > +.in
> > +.PP
> > +The available access rights for each ABI version are listed in the
> > +.B VERSIONS
> > +section.
> > +.PP
> > +If our program needed to create hard links or rename files between different directories
> 
> Please keep lines below 80 columns.  Break lines at phrase
> boundaries as appropriate (e.g., in this line:)
> 
> s/ or /\nor /

Thanks, applied.  Will be fixed in the next patch version.


> > +.RB ( LANDLOCK_ACCESS_FS_REFER ),
> > +we would require the following change to the backwards compatibility logic:
> > +Directory reparenting is not possible in a process restricted with Landlock ABI version 1.

Fixed it on this line too.

Thank you for the review and for applying the two earlier patches!
–Günther

Re: [PATCH v5 3/3] landlock.7: Explain the best-effort fallback mechanism in the example

[Alejandro Colomar]

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

Hola Günther :)

On 4/1/23 19:19, Günther Noack wrote:
> Hello Alejandro!
> 
> On Sat, Apr 01, 2023 at 12:29:35AM +0200, Alejandro Colomar wrote:
>> Hi Günther,
>>
>> On 3/24/23 18:24, Günther Noack wrote:
>>> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
>>> ---
>>>  man7/landlock.7 | 65 ++++++++++++++++++++++++++++++++++++++++++++++---
>>>  1 file changed, 61 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/man7/landlock.7 b/man7/landlock.7
>>> index 9c305edef..d1214ba27 100644
>>> --- a/man7/landlock.7
>>> +++ b/man7/landlock.7
>>> [...]
>>> +.EX
>>> +/* Table of available file system access rights by ABI version */
>>> +__u64 landlock_fs_access_rights[] = {
>>> +    (1ULL << 13) \- 1,  /* ABI v1                 */
>>> +    (1ULL << 14) \- 1,  /* ABI v2: add "refer"    */
>>> +    (1ULL << 15) \- 1,  /* ABI v3: add "truncate" */
>>
>> Do these magic numbers have macros?  Are users expected to use
>> the magic numbers directly?
> 
> You are right to point this out - it's the ugly part here :-/
> 
> These are bitmasks for the different access rights which are supported
> at each ABI version, so they are a bitwise OR of a long list of
> LANDLOCK_ACCESS_FS_* constants.
> 
> I am aware of three ways to write this out.
> Please let me know which of these three you prefer.
> (or maybe you have an idea for another alternative?).
> 
> (It feels out of scope for this documentation patch, but do you think
> these bitmasks should be defined in the uapi/linux/landlock.h header?
> You have looked at so many man pages already -- Do you happen to know
> other places in the kernel API where such a problem has come up?)

I don't remember having seen something similar in other pages.

I think defining a macro in uapi headers could be the right thing to
do.  Something like LANDLOCK_ACCESS_FS_RIGHTS_MASK_ABI_{1,2,3} or
other similar name?

> 
> 
> 1) Make assumptions about the numbers, for brevity
>    (as done in the patch I sent).
> 
> __u64 landlock_fs_access_rights[] = {
>     (1ULL << 13) - 1,  /* ABI v1                 */
>     (1ULL << 14) - 1,  /* ABI v2: add "refer"    */
>     (1ULL << 15) - 1,  /* ABI v3: add "truncate" */
> }
> 
> This is the shortest variant,
> but it does not use the Landlock constants from the header.
> 
> 
> 2) Use the constants from the header and OR them.
> 
> This is the "most correct" way, but I feel that it might be too
> verbose for a documentation example.  What do you think?
> 
> __u64 landlock_fs_access_rights[] = {
>     /* ABI v1 */
>     LANDLOCK_ACCESS_FS_EXECUTE |
>     LANDLOCK_ACCESS_FS_WRITE_FILE |
>     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,
> 
>     /* ABI v2: Add "refer" */
>     LANDLOCK_ACCESS_FS_EXECUTE |
>     LANDLOCK_ACCESS_FS_WRITE_FILE |
>     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 |
>     LANDLOCK_ACCESS_FS_REFER,
>     
>     /* ABI v3: add "truncate" */
>     LANDLOCK_ACCESS_FS_EXECUTE |
>     LANDLOCK_ACCESS_FS_WRITE_FILE |
>     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,
>     LANDLOCK_ACCESS_FS_REFER |
>     LANDLOCK_ACCESS_FS_TRUNCATE,
> }
> 
> If I were to write production code, I would probably write it out like
> that, to be explicit -- but it feels like a long code example for a
> man page? WDYT?

Similar feelings here.

> 
> 
> 3) Third option is the middle way,
>    naming the "highest" known access right for each ABI version:
> 
> __u64 landlock_fs_access_rights[] = {
>     (LANDLOCK_ACCESS_FS_MAKE_SYM << 1) - 1,  /* ABI v1                 */
>     (LANDLOCK_ACCESS_FS_REFER << 1) - 1,     /* ABI v2: add "refer"    */
>     (LANDLOCK_ACCESS_FS_TRUNCATE << 1) - 1,  /* ABI v3: add "truncate" */
> }

I'm not sure if I like this one.  I'll leave it up to you to decide
the one you like.  :)

> 
> In this case, we still make the assumption that the supported rights
> are the "highest" right plus all of the bits in lower order places.
> 
> 
>>> +/* Only use the available rights in the ruleset. */
>>> +attr.handled_access_fs &= landlock_fs_access_rights[abi \- 1];
>>> +.EE
>>> +.in
>>> +.PP
>>> +The available access rights for each ABI version are listed in the
>>> +.B VERSIONS
>>> +section.
>>> +.PP
>>> +If our program needed to create hard links or rename files between different directories
>>
>> Please keep lines below 80 columns.  Break lines at phrase
>> boundaries as appropriate (e.g., in this line:)
>>
>> s/ or /\nor /
> 
> Thanks, applied.  Will be fixed in the next patch version.
> 
> 
>>> +.RB ( LANDLOCK_ACCESS_FS_REFER ),
>>> +we would require the following change to the backwards compatibility logic:
>>> +Directory reparenting is not possible in a process restricted with Landlock ABI version 1.
> 
> Fixed it on this line too.
> 
> Thank you for the review and for applying the two earlier patches!

:)

Cheers,
Alex

> –Günther

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

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

Re: [PATCH v5 3/3] landlock.7: Explain the best-effort fallback mechanism in the example

[Alejandro Colomar]

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

On 4/1/23 19:19, Günther Noack wrote:
> Hello Alejandro!
> 
> On Sat, Apr 01, 2023 at 12:29:35AM +0200, Alejandro Colomar wrote:
>> Hi Günther,
>>
[...]

>>> +.PP
>>> +If our program needed to create hard links or rename files between different directories
>>
>> Please keep lines below 80 columns.  Break lines at phrase
>> boundaries as appropriate (e.g., in this line:)
>>
>> s/ or /\nor /
> 
> Thanks, applied.  Will be fixed in the next patch version.
> 
> 
>>> +.RB ( LANDLOCK_ACCESS_FS_REFER ),
>>> +we would require the following change to the backwards compatibility logic:
>>> +Directory reparenting is not possible in a process restricted with Landlock ABI version 1.
> 
> Fixed it on this line too.

BTW, I forgot to tell you that you can check this yourself in future
man-pages patches following the ./CONTRIBUTING file instructions for
linting and checking patches.  You might find this useful.  See:


<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/CONTRIBUTING#n132>
   Lint & check
       If you plan to patch a manual page, consider running the linters and
       checks configured in the build system, to make sure your change doesn't
       add new warnings.  However, you might still get warnings that are not
       your fault.  To minimize that, do the following steps:

       (1)  First use make(1)'s -t option, so that make(1) knows that it only
            needs to lint & check again pages that you will touch.

                $ make -t lint check >/dev/null

       (2)  Run make(1) again, asking it to imagine that the page wou'll
            modify has been touched, to see which warnings you'll still see
            from that page that are not your fault.

                $ # replace 'man2/membarrier.2' by the page you'll modify
                $ make -W man2/membarrier.2 -k lint check

       (3)  Apply your changes, and then run make(1) again.  You can ignore
            warnings that you saw in step (2), but if you see any new ones,
            please fix them if you know how, or at least note them in your
            patch email.

                $ vi man2/membarrier.2  # do your work
                $ make -k lint check

       See <INSTALL> for a list of dependencies that this feature requires.
       If you can't meet them all, don't worry; it will still run the linters
       and checks that you have available.


> 
> Thank you for the review and for applying the two earlier patches!
> –Günther

Cheers,
Alex

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

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

Re: [PATCH v5 3/3] landlock.7: Explain the best-effort fallback mechanism in the example

[Günther Noack]

On Sun, Apr 02, 2023 at 03:21:56AM +0200, Alejandro Colomar wrote:
> On 4/1/23 19:19, Günther Noack wrote:
> > Hello Alejandro!
> > 
> > On Sat, Apr 01, 2023 at 12:29:35AM +0200, Alejandro Colomar wrote:
> >>> +.RB ( LANDLOCK_ACCESS_FS_REFER ),
> >>> +we would require the following change to the backwards compatibility logic:
> >>> +Directory reparenting is not possible in a process restricted with Landlock ABI version 1.
> > 
> > Fixed it on this line too.
> 
> BTW, I forgot to tell you that you can check this yourself in future
> man-pages patches following the ./CONTRIBUTING file instructions for
> linting and checking patches.  You might find this useful.  See:
> 
> 
> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/CONTRIBUTING#n132>
> ...

Thank you for pointing this out (and for reworking this
documentation)!  I had indeed missed the CONTRIBUTING doc.
The "make -t" trick is also new to me.

–Günther

Re: [PATCH v5 3/3] landlock.7: Explain the best-effort fallback mechanism in the example

[Günther Noack]

Hello!

On Sun, Apr 02, 2023 at 12:01:43AM +0200, Alejandro Colomar wrote:
> On 4/1/23 19:19, Günther Noack wrote:
> > (It feels out of scope for this documentation patch, but do you think
> > these bitmasks should be defined in the uapi/linux/landlock.h header?
> > You have looked at so many man pages already -- Do you happen to know
> > other places in the kernel API where such a problem has come up?)
> 
> I don't remember having seen something similar in other pages.
> 
> I think defining a macro in uapi headers could be the right thing to
> do.  Something like LANDLOCK_ACCESS_FS_RIGHTS_MASK_ABI_{1,2,3} or
> other similar name?

Noted it on my TODO list - it's probably best discussed on the kernel
list whether this is the right approach.


> > 1) Make assumptions about the numbers, for brevity
> >    (as done in the patch I sent).
> > 
> >    [...]
> > 
> > 2) Use the constants from the header and OR them.
> > 
> >    [...]
> > 
> > 3) Third option is the middle way,
> >    naming the "highest" known access right for each ABI version:
> > 
> > __u64 landlock_fs_access_rights[] = {
> >     (LANDLOCK_ACCESS_FS_MAKE_SYM << 1) - 1,  /* ABI v1                 */
> >     (LANDLOCK_ACCESS_FS_REFER << 1) - 1,     /* ABI v2: add "refer"    */
> >     (LANDLOCK_ACCESS_FS_TRUNCATE << 1) - 1,  /* ABI v3: add "truncate" */
> > }
> 
> I'm not sure if I like this one.  I'll leave it up to you to decide
> the one you like.  :)

I'll ponder it a bit and send a new patch soon.

Mickaël, do you have any opinions/preferences on this?

–Günther

Re: [PATCH v5 3/3] landlock.7: Explain the best-effort fallback mechanism in the example

[Alejandro Colomar]

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

Hello!

On 4/4/23 09:33, Günther Noack wrote:
> Hello!
> 
> On Sun, Apr 02, 2023 at 12:01:43AM +0200, Alejandro Colomar wrote:
>> On 4/1/23 19:19, Günther Noack wrote:
>>> (It feels out of scope for this documentation patch, but do you think
>>> these bitmasks should be defined in the uapi/linux/landlock.h header?
>>> You have looked at so many man pages already -- Do you happen to know
>>> other places in the kernel API where such a problem has come up?)
>>
>> I don't remember having seen something similar in other pages.
>>
>> I think defining a macro in uapi headers could be the right thing to
>> do.  Something like LANDLOCK_ACCESS_FS_RIGHTS_MASK_ABI_{1,2,3} or
>> other similar name?
> 
> Noted it on my TODO list - it's probably best discussed on the kernel
> list whether this is the right approach.

Sure!  Feel free to CC me there.

> 
> 
>>> 1) Make assumptions about the numbers, for brevity
>>>    (as done in the patch I sent).
>>>
>>>    [...]
>>>
>>> 2) Use the constants from the header and OR them.
>>>
>>>    [...]
>>>
>>> 3) Third option is the middle way,
>>>    naming the "highest" known access right for each ABI version:
>>>
>>> __u64 landlock_fs_access_rights[] = {
>>>     (LANDLOCK_ACCESS_FS_MAKE_SYM << 1) - 1,  /* ABI v1                 */
>>>     (LANDLOCK_ACCESS_FS_REFER << 1) - 1,     /* ABI v2: add "refer"    */
>>>     (LANDLOCK_ACCESS_FS_TRUNCATE << 1) - 1,  /* ABI v3: add "truncate" */
>>> }
>>
>> I'm not sure if I like this one.  I'll leave it up to you to decide
>> the one you like.  :)
> 
> I'll ponder it a bit and send a new patch soon.

Ok.  No hurries.

> 
> Mickaël, do you have any opinions/preferences on this?
> 
> –Günther

On 4/4/23 09:17, Günther Noack wrote:
>> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/CONTRIBUTING#n132>
>> ...
> Thank you for pointing this out (and for reworking this
> documentation)!

:-)

>  I had indeed missed the CONTRIBUTING doc.
> The "make -t" trick is also new to me.

Heh, I've fine-tuned makefiles too much to come up with this
workflow. :p

I met that feature long ago reading make(1)'s man page, but didn't
know what it would be useful for, until I recently realized it could
help in this use case.

Cheers,
Alex



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

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

Re: [PATCH v5 3/3] landlock.7: Explain the best-effort fallback mechanism in the example

[Mickaël Salaün]

On 04/04/2023 09:33, Günther Noack wrote:
> Hello!
> 
> On Sun, Apr 02, 2023 at 12:01:43AM +0200, Alejandro Colomar wrote:
>> On 4/1/23 19:19, Günther Noack wrote:
>>> (It feels out of scope for this documentation patch, but do you think
>>> these bitmasks should be defined in the uapi/linux/landlock.h header?
>>> You have looked at so many man pages already -- Do you happen to know
>>> other places in the kernel API where such a problem has come up?)
>>
>> I don't remember having seen something similar in other pages.
>>
>> I think defining a macro in uapi headers could be the right thing to
>> do.  Something like LANDLOCK_ACCESS_FS_RIGHTS_MASK_ABI_{1,2,3} or
>> other similar name?
> 
> Noted it on my TODO list - it's probably best discussed on the kernel
> list whether this is the right approach.
> 
> 
>>> 1) Make assumptions about the numbers, for brevity
>>>     (as done in the patch I sent).
>>>
>>>     [...]
>>>
>>> 2) Use the constants from the header and OR them.
>>>
>>>     [...]
>>>
>>> 3) Third option is the middle way,
>>>     naming the "highest" known access right for each ABI version:
>>>
>>> __u64 landlock_fs_access_rights[] = {
>>>      (LANDLOCK_ACCESS_FS_MAKE_SYM << 1) - 1,  /* ABI v1                 */
>>>      (LANDLOCK_ACCESS_FS_REFER << 1) - 1,     /* ABI v2: add "refer"    */
>>>      (LANDLOCK_ACCESS_FS_TRUNCATE << 1) - 1,  /* ABI v3: add "truncate" */
>>> }
>>
>> I'm not sure if I like this one.  I'll leave it up to you to decide
>> the one you like.  :)
> 
> I'll ponder it a bit and send a new patch soon.
> 
> Mickaël, do you have any opinions/preferences on this?

Sorry, I missed this thread. I prefer your third solution because it is 
explicit and relies on non-harcoded/magic values. I replied to the last 
version of this patch but it might be a bit late now.

I'm not sure that defining a mask per ABI should be part of UAPI 
(instead of dedicated libraries), but I'll think about that.

Re: [PATCH v5 3/3] landlock.7: Explain the best-effort fallback mechanism in the example

[Alejandro Colomar]

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

Hi Mickaël,

On 4/17/23 23:13, Mickaël Salaün wrote:
>> Mickaël, do you have any opinions/preferences on this?
> 
> Sorry, I missed this thread. I prefer your third solution because it is 
> explicit and relies on non-harcoded/magic values. I replied to the last 
> version of this patch but it might be a bit late now.

No, it's not late.  I already applied the patches, but new patches to fix
stuff are always welcome ;)

> 
> I'm not sure that defining a mask per ABI should be part of UAPI 
> (instead of dedicated libraries), but I'll think about that.

Thanks.

Cheers,
Alex

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

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