| Message ID | 20220804193746.9161-5-gnoack3000@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | landlock: truncate support | expand |
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@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 > ============= >
On Fri, Aug 12, 2022 at 01:19:47PM +0200, Mickaël Salaün wrote: > > 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@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" Done, good point. > > > + 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 */ Done, added them to both the sample and the documentation. > > + 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. Acknowledged. > > > > +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. Sounds good, applied that in this place and a few others that I touched. > > > + > > Compatibility > > ============= --
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"); + return 1; + case 1: + ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_REFER; + __attribute__((fallthrough)); + case 2: + 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 +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`. + Compatibility =============
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@gmail.com> --- Documentation/userspace-api/landlock.rst | 41 ++++++++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-)