| Message ID | 20220908195805.128252-6-gnoack3000@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | landlock: truncate support | expand |
On 08/09/2022 21:58, 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 | 62 +++++++++++++++++++++--- > 1 file changed, 54 insertions(+), 8 deletions(-) > > diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst > index b8ea59493964..57802fd1e09b 100644 > --- a/Documentation/userspace-api/landlock.rst > +++ b/Documentation/userspace-api/landlock.rst > @@ -8,7 +8,7 @@ Landlock: unprivileged access control > ===================================== > > :Author: Mickaël Salaün > -:Date: May 2022 > +:Date: September 2022 > > The goal of Landlock is to enable to restrict ambient rights (e.g. global > filesystem access) for a set of processes. Because Landlock is a stackable > @@ -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,26 @@ 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` or `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("The running kernel does not enable to use Landlock"); > + return 1; > + case 1: > + /* 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 +138,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. This fix is correct, but it should not be part of this series. FYI, I have a patch almost ready to fix some documentation style issues. Please remove this hunk for the next series. I'll deal with the merge conflicts if any. > > 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 +262,32 @@ 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 I investigated and in fact we should use double backquotes almost everywhere because it render the same as when using "%" in header files. Please change this for the next series. I'll do the same on the patch I just talk about. > +`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 require the > +`LANDLOCK_ACCESS_FS_WRITE_FILE` right. Apart from the :manpage:`truncate(2)` > +system call, this can also be done through :manpage:`open(2)` with the flags > +`O_RDONLY | O_TRUNC`. > + > +When opening a file, the availability of the `LANDLOCK_ACCESS_FS_TRUNCATE` right > +is associated with the newly created file descriptor and will be used for > +subsequent truncation attempts using :manpage:`ftruncate(2)`. 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 don't have an enforced Landlock ruleset. Good addition. Please do not use contractions ("don't"). > + > Compatibility > ============= > > @@ -397,6 +434,15 @@ Starting with the Landlock ABI version 2, it is now possible to securely > control renaming and linking thanks to the new `LANDLOCK_ACCESS_FS_REFER` > access right. > > +File truncation (ABI < 3) > +------------------------- > + > +File truncation could not be denied before the third Landlock ABI, so it is > +always allowed when using a kernel that only supports the first or second ABI. > + > +Starting with the Landlock ABI version 3, it is now possible to securely control > +truncation thanks to the new `LANDLOCK_ACCESS_FS_TRUNCATE` access right. > + > .. _kernel_support: > > Kernel support
On Fri, Sep 09, 2022 at 03:51:35PM +0200, Mickaël Salaün wrote: > > On 08/09/2022 21:58, 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 | 62 +++++++++++++++++++++--- > > 1 file changed, 54 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst > > index b8ea59493964..57802fd1e09b 100644 > > --- a/Documentation/userspace-api/landlock.rst > > +++ b/Documentation/userspace-api/landlock.rst > > @@ -8,7 +8,7 @@ Landlock: unprivileged access control > > ===================================== > > :Author: Mickaël Salaün > > -:Date: May 2022 > > +:Date: September 2022 > > The goal of Landlock is to enable to restrict ambient rights (e.g. global > > filesystem access) for a set of processes. Because Landlock is a stackable > > @@ -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,26 @@ 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` or `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("The running kernel does not enable to use Landlock"); > > + return 1; > > + case 1: > > + /* 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 +138,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. > > This fix is correct, but it should not be part of this series. FYI, I have a > patch almost ready to fix some documentation style issues. Please remove > this hunk for the next series. I'll deal with the merge conflicts if any. Can you please clarify what part of it should not be part of this series? In this hunk, I've started using double backquote, but I've also changed the meaning of the sentence slightly so that it is still correct when the truncate right is introduced. It is still correct that the backwards compatibility check is not required because LANDLOCK_ACCESS_FS_REFER is not allowed by any rule. But with the new truncate flag, LANDLOCK_ACCESS_FS_TRUNCATE may also not be allowed by any rule so that we can skip this check. Should I remove this hunk entirely? Or maybe rather phrase it like 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` and `LANDLOCK_ACCESS_FS_TRUNCATE` are not allowed by any rule. ? > > 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 +262,32 @@ 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 > > I investigated and in fact we should use double backquotes almost everywhere > because it render the same as when using "%" in header files. Please change > this for the next series. I'll do the same on the patch I just talk about. Done. I'm changing double backquote style only in the hunks I touched so that it'll be easier to merge. > > > > +`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 require the > > +`LANDLOCK_ACCESS_FS_WRITE_FILE` right. Apart from the :manpage:`truncate(2)` > > +system call, this can also be done through :manpage:`open(2)` with the flags > > +`O_RDONLY | O_TRUNC`. > > + > > +When opening a file, the availability of the `LANDLOCK_ACCESS_FS_TRUNCATE` right > > +is associated with the newly created file descriptor and will be used for > > +subsequent truncation attempts using :manpage:`ftruncate(2)`. 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 don't have an enforced Landlock ruleset. > > Good addition. Please do not use contractions ("don't"). Done, thanks. > > > > + > > Compatibility > > ============= > > @@ -397,6 +434,15 @@ Starting with the Landlock ABI version 2, it is now possible to securely > > control renaming and linking thanks to the new `LANDLOCK_ACCESS_FS_REFER` > > access right. > > +File truncation (ABI < 3) > > +------------------------- > > + > > +File truncation could not be denied before the third Landlock ABI, so it is > > +always allowed when using a kernel that only supports the first or second ABI. > > + > > +Starting with the Landlock ABI version 3, it is now possible to securely control > > +truncation thanks to the new `LANDLOCK_ACCESS_FS_TRUNCATE` access right. Changed backquote style here as well. Thanks for the review! —Günther --
On 12/09/2022 17:46, Günther Noack wrote: > On Fri, Sep 09, 2022 at 03:51:35PM +0200, Mickaël Salaün wrote: >> >> On 08/09/2022 21:58, 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 | 62 +++++++++++++++++++++--- >>> 1 file changed, 54 insertions(+), 8 deletions(-) >>> >>> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst >>> index b8ea59493964..57802fd1e09b 100644 >>> --- a/Documentation/userspace-api/landlock.rst >>> +++ b/Documentation/userspace-api/landlock.rst >>> @@ -8,7 +8,7 @@ Landlock: unprivileged access control >>> ===================================== >>> :Author: Mickaël Salaün >>> -:Date: May 2022 >>> +:Date: September 2022 >>> The goal of Landlock is to enable to restrict ambient rights (e.g. global >>> filesystem access) for a set of processes. Because Landlock is a stackable >>> @@ -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,26 @@ 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` or `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("The running kernel does not enable to use Landlock"); >>> + return 1; >>> + case 1: >>> + /* 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 +138,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. >> >> This fix is correct, but it should not be part of this series. FYI, I have a >> patch almost ready to fix some documentation style issues. Please remove >> this hunk for the next series. I'll deal with the merge conflicts if any. > > Can you please clarify what part of it should not be part of this > series? My mistake, I guess I was reviewing something else… I was thinking about style changes, but it is not the case here. Using "``" is correct. > > In this hunk, I've started using double backquote, but I've also > changed the meaning of the sentence slightly so that it is still > correct when the truncate right is introduced. > > It is still correct that the backwards compatibility check is not > required because LANDLOCK_ACCESS_FS_REFER is not allowed by any rule. > But with the new truncate flag, LANDLOCK_ACCESS_FS_TRUNCATE may also > not be allowed by any rule so that we can skip this check. > > Should I remove this hunk entirely? Keep your changes, it's better like this. > > Or maybe rather phrase it like > > 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` and > `LANDLOCK_ACCESS_FS_TRUNCATE` are not allowed by any rule. > > ?
On Mon, Sep 12, 2022 at 07:47:11PM +0200, Mickaël Salaün wrote: > On 12/09/2022 17:46, Günther Noack wrote: > > On Fri, Sep 09, 2022 at 03:51:35PM +0200, Mickaël Salaün wrote: > > > On 08/09/2022 21:58, 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 | 62 +++++++++++++++++++++--- > > > > 1 file changed, 54 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst > > > > index b8ea59493964..57802fd1e09b 100644 > > > > --- a/Documentation/userspace-api/landlock.rst > > > > +++ b/Documentation/userspace-api/landlock.rst > > > > @@ -8,7 +8,7 @@ Landlock: unprivileged access control > > > > ===================================== > > > > :Author: Mickaël Salaün > > > > -:Date: May 2022 > > > > +:Date: September 2022 > > > > The goal of Landlock is to enable to restrict ambient rights (e.g. global > > > > filesystem access) for a set of processes. Because Landlock is a stackable > > > > @@ -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,26 @@ 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` or `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("The running kernel does not enable to use Landlock"); > > > > + return 1; > > > > + case 1: > > > > + /* 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 +138,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. > > > > > > This fix is correct, but it should not be part of this series. FYI, I have a > > > patch almost ready to fix some documentation style issues. Please remove > > > this hunk for the next series. I'll deal with the merge conflicts if any. > > > > Can you please clarify what part of it should not be part of this > > series? > > My mistake, I guess I was reviewing something else… I was thinking about > style changes, but it is not the case here. Using "``" is correct. > > > > > > In this hunk, I've started using double backquote, but I've also > > changed the meaning of the sentence slightly so that it is still > > correct when the truncate right is introduced. > > > > It is still correct that the backwards compatibility check is not > > required because LANDLOCK_ACCESS_FS_REFER is not allowed by any rule. > > But with the new truncate flag, LANDLOCK_ACCESS_FS_TRUNCATE may also > > not be allowed by any rule so that we can skip this check. > > > > Should I remove this hunk entirely? > > Keep your changes, it's better like this. Thanks, reverted that part then. —Günther --
On 08/09/2022 21:58, 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 | 62 +++++++++++++++++++++--- > 1 file changed, 54 insertions(+), 8 deletions(-) > > diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst > index b8ea59493964..57802fd1e09b 100644 > --- a/Documentation/userspace-api/landlock.rst > +++ b/Documentation/userspace-api/landlock.rst > @@ -8,7 +8,7 @@ Landlock: unprivileged access control > ===================================== > > :Author: Mickaël Salaün > -:Date: May 2022 > +:Date: September 2022 > > The goal of Landlock is to enable to restrict ambient rights (e.g. global > filesystem access) for a set of processes. Because Landlock is a stackable > @@ -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,26 @@ 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` or `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("The running kernel does not enable to use Landlock"); > + return 1; I think it would be easier to understand to explicitly check for abi < 0 in a dedicated block as in the sample, instead of case -1, and return 0 (instead of 1) with a comment to inform that Landlock is not handled but it is OK (expected error). > + case 1: > + /* 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.
On Mon, Sep 12, 2022 at 09:15:06PM +0200, Mickaël Salaün wrote: > > > On 08/09/2022 21:58, 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 | 62 +++++++++++++++++++++--- > > 1 file changed, 54 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst > > index b8ea59493964..57802fd1e09b 100644 > > --- a/Documentation/userspace-api/landlock.rst > > +++ b/Documentation/userspace-api/landlock.rst > > @@ -8,7 +8,7 @@ Landlock: unprivileged access control > > ===================================== > > :Author: Mickaël Salaün > > -:Date: May 2022 > > +:Date: September 2022 > > The goal of Landlock is to enable to restrict ambient rights (e.g. global > > filesystem access) for a set of processes. Because Landlock is a stackable > > @@ -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,26 @@ 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` or `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("The running kernel does not enable to use Landlock"); > > + return 1; > > I think it would be easier to understand to explicitly check for abi < 0 > in a dedicated block as in the sample, instead of case -1, and return 0 > (instead of 1) with a comment to inform that Landlock is not handled but > it is OK (expected error). Done. > > > > + case 1: > > + /* 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. --
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst index b8ea59493964..57802fd1e09b 100644 --- a/Documentation/userspace-api/landlock.rst +++ b/Documentation/userspace-api/landlock.rst @@ -8,7 +8,7 @@ Landlock: unprivileged access control ===================================== :Author: Mickaël Salaün -:Date: May 2022 +:Date: September 2022 The goal of Landlock is to enable to restrict ambient rights (e.g. global filesystem access) for a set of processes. Because Landlock is a stackable @@ -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,26 @@ 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` or `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("The running kernel does not enable to use Landlock"); + return 1; + case 1: + /* 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 +138,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 +262,32 @@ 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 require the +`LANDLOCK_ACCESS_FS_WRITE_FILE` right. Apart from the :manpage:`truncate(2)` +system call, this can also be done through :manpage:`open(2)` with the flags +`O_RDONLY | O_TRUNC`. + +When opening a file, the availability of the `LANDLOCK_ACCESS_FS_TRUNCATE` right +is associated with the newly created file descriptor and will be used for +subsequent truncation attempts using :manpage:`ftruncate(2)`. 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 don't have an enforced Landlock ruleset. + Compatibility ============= @@ -397,6 +434,15 @@ Starting with the Landlock ABI version 2, it is now possible to securely control renaming and linking thanks to the new `LANDLOCK_ACCESS_FS_REFER` access right. +File truncation (ABI < 3) +------------------------- + +File truncation could not be denied before the third Landlock ABI, so it is +always allowed when using a kernel that only supports the first or second ABI. + +Starting with the Landlock ABI version 3, it is now possible to securely control +truncation thanks to the new `LANDLOCK_ACCESS_FS_TRUNCATE` access right. + .. _kernel_support: Kernel support
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 | 62 +++++++++++++++++++++--- 1 file changed, 54 insertions(+), 8 deletions(-)