| Message ID | 20230221165205.4231-1-gnoack3000@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v4] landlock: Clarify documentation for the LANDLOCK_ACCESS_FS_REFER right | expand |
Perfect, thanks for your perseverance Günther! I've applied it to my -next tree: https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git/log/?h=next On 21/02/2023 17:52, Günther Noack wrote: > Clarify the "refer" documentation by splitting up a big paragraph of text. > > - Call out specifically that the denial by default applies to ABI v1 as well. > - Turn the three additional constraints for link/rename operations > into bullet points, to give it more structure. > > Signed-off-by: Günther Noack <gnoack3000@gmail.com> > --- > include/uapi/linux/landlock.h | 46 +++++++++++++++++++++++------------ > 1 file changed, 31 insertions(+), 15 deletions(-) > > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h > index f3223f96469..81d09ef9aa5 100644 > --- a/include/uapi/linux/landlock.h > +++ b/include/uapi/linux/landlock.h > @@ -130,21 +130,37 @@ struct landlock_path_beneath_attr { > * - %LANDLOCK_ACCESS_FS_MAKE_BLOCK: Create (or rename or link) a block device. > * - %LANDLOCK_ACCESS_FS_MAKE_SYM: Create (or rename or link) a symbolic link. > * - %LANDLOCK_ACCESS_FS_REFER: Link or rename a file from or to a different > - * directory (i.e. reparent a file hierarchy). This access right is > - * available since the second version of the Landlock ABI. This is also the > - * only access right which is always considered handled by any ruleset in > - * such a way that reparenting a file hierarchy is always denied by default. > - * To avoid privilege escalation, it is not enough to add a rule with this > - * access right. When linking or renaming a file, the destination directory > - * hierarchy must also always have the same or a superset of restrictions of > - * the source hierarchy. If it is not the case, or if the domain doesn't > - * handle this access right, such actions are denied by default with errno > - * set to ``EXDEV``. Linking also requires a ``LANDLOCK_ACCESS_FS_MAKE_*`` > - * access right on the destination directory, and renaming also requires a > - * ``LANDLOCK_ACCESS_FS_REMOVE_*`` access right on the source's (file or > - * directory) parent. Otherwise, such actions are denied with errno set to > - * ``EACCES``. The ``EACCES`` errno prevails over ``EXDEV`` to let user space > - * efficiently deal with an unrecoverable error. > + * directory (i.e. reparent a file hierarchy). > + * > + * This access right is available since the second version of the Landlock > + * ABI. > + * > + * This is the only access right which is denied by default by any ruleset, > + * even if the right is not specified as handled at ruleset creation time. > + * The only way to make a ruleset grant this right is to explicitly allow it > + * for a specific directory by adding a matching rule to the ruleset. > + * > + * In particular, when using the first Landlock ABI version, Landlock will > + * always deny attempts to reparent files between different directories. > + * > + * In addition to the source and destination directories having the > + * %LANDLOCK_ACCESS_FS_REFER access right, the attempted link or rename > + * operation must meet the following constraints: > + * > + * * The reparented file may not gain more access rights in the destination > + * directory than it previously had in the source directory. If this is > + * attempted, the operation results in an ``EXDEV`` error. > + * > + * * When linking or renaming, the ``LANDLOCK_ACCESS_FS_MAKE_*`` right for the > + * respective file type must be granted for the destination directory. > + * Otherwise, the operation results in an ``EACCES`` error. > + * > + * * When renaming, the ``LANDLOCK_ACCESS_FS_REMOVE_*`` right for the > + * respective file type must be granted for the source directory. Otherwise, > + * the operation results in an ``EACCES`` error. > + * > + * If multiple requirements are not met, the ``EACCES`` error code takes > + * precedence over ``EXDEV``. > * > * .. warning:: > * > > base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
Hi Günther, On 2/21/23 17:52, Günther Noack wrote: [...] Sorry for taking so much time in looking into this! > + * > + * * The reparented file may not gain more access rights in the destination > + * directory than it previously had in the source directory. If this is > + * attempted, the operation results in an ``EXDEV`` error. > + * > + * * When linking or renaming, the ``LANDLOCK_ACCESS_FS_MAKE_*`` right for the > + * respective file type must be granted for the destination directory. > + * Otherwise, the operation results in an ``EACCES`` error. > + * > + * * When renaming, the ``LANDLOCK_ACCESS_FS_REMOVE_*`` right for the > + * respective file type must be granted for the source directory. Otherwise, > + * the operation results in an ``EACCES`` error. > + * > + * If multiple requirements are not met, the ``EACCES`` error code takes > + * precedence over ``EXDEV``. Is this something worth guaranteeing, or just an implementation detail that may easily change? Cheers, Alex > * > * .. warning:: > * > > base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
On 25/02/2023 00:00, Alex Colomar wrote: > Hi Günther, > > On 2/21/23 17:52, Günther Noack wrote: > [...] > > Sorry for taking so much time in looking into this! > >> + * >> + * * The reparented file may not gain more access rights in the destination >> + * directory than it previously had in the source directory. If this is >> + * attempted, the operation results in an ``EXDEV`` error. >> + * >> + * * When linking or renaming, the ``LANDLOCK_ACCESS_FS_MAKE_*`` right for the >> + * respective file type must be granted for the destination directory. >> + * Otherwise, the operation results in an ``EACCES`` error. >> + * >> + * * When renaming, the ``LANDLOCK_ACCESS_FS_REMOVE_*`` right for the >> + * respective file type must be granted for the source directory. Otherwise, >> + * the operation results in an ``EACCES`` error. >> + * >> + * If multiple requirements are not met, the ``EACCES`` error code takes >> + * precedence over ``EXDEV``. > > Is this something worth guaranteeing, or just an implementation detail > that may easily change? There is a deliberate effort to keep this error priority to be able to know if a rename or link action can be replaced with a file copy, which is useful e.g., for `mv`.
Hi Mickaël, On 2/27/23 12:02, Mickaël Salaün wrote: > > On 25/02/2023 00:00, Alex Colomar wrote: >> Hi Günther, >> >> On 2/21/23 17:52, Günther Noack wrote: >> [...] >> >> Sorry for taking so much time in looking into this! >> >>> + * >>> + * * The reparented file may not gain more access rights in the destination >>> + * directory than it previously had in the source directory. If this is >>> + * attempted, the operation results in an ``EXDEV`` error. >>> + * >>> + * * When linking or renaming, the ``LANDLOCK_ACCESS_FS_MAKE_*`` right for the >>> + * respective file type must be granted for the destination directory. >>> + * Otherwise, the operation results in an ``EACCES`` error. >>> + * >>> + * * When renaming, the ``LANDLOCK_ACCESS_FS_REMOVE_*`` right for the >>> + * respective file type must be granted for the source directory. Otherwise, >>> + * the operation results in an ``EACCES`` error. >>> + * >>> + * If multiple requirements are not met, the ``EACCES`` error code takes >>> + * precedence over ``EXDEV``. >> >> Is this something worth guaranteeing, or just an implementation detail >> that may easily change? > > There is a deliberate effort to keep this error priority to be able to > know if a rename or link action can be replaced with a file copy, which > is useful e.g., for `mv`. Thanks. Then it LGTM. Cheers! Alex
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h index f3223f96469..81d09ef9aa5 100644 --- a/include/uapi/linux/landlock.h +++ b/include/uapi/linux/landlock.h @@ -130,21 +130,37 @@ struct landlock_path_beneath_attr { * - %LANDLOCK_ACCESS_FS_MAKE_BLOCK: Create (or rename or link) a block device. * - %LANDLOCK_ACCESS_FS_MAKE_SYM: Create (or rename or link) a symbolic link. * - %LANDLOCK_ACCESS_FS_REFER: Link or rename a file from or to a different - * directory (i.e. reparent a file hierarchy). This access right is - * available since the second version of the Landlock ABI. This is also the - * only access right which is always considered handled by any ruleset in - * such a way that reparenting a file hierarchy is always denied by default. - * To avoid privilege escalation, it is not enough to add a rule with this - * access right. When linking or renaming a file, the destination directory - * hierarchy must also always have the same or a superset of restrictions of - * the source hierarchy. If it is not the case, or if the domain doesn't - * handle this access right, such actions are denied by default with errno - * set to ``EXDEV``. Linking also requires a ``LANDLOCK_ACCESS_FS_MAKE_*`` - * access right on the destination directory, and renaming also requires a - * ``LANDLOCK_ACCESS_FS_REMOVE_*`` access right on the source's (file or - * directory) parent. Otherwise, such actions are denied with errno set to - * ``EACCES``. The ``EACCES`` errno prevails over ``EXDEV`` to let user space - * efficiently deal with an unrecoverable error. + * directory (i.e. reparent a file hierarchy). + * + * This access right is available since the second version of the Landlock + * ABI. + * + * This is the only access right which is denied by default by any ruleset, + * even if the right is not specified as handled at ruleset creation time. + * The only way to make a ruleset grant this right is to explicitly allow it + * for a specific directory by adding a matching rule to the ruleset. + * + * In particular, when using the first Landlock ABI version, Landlock will + * always deny attempts to reparent files between different directories. + * + * In addition to the source and destination directories having the + * %LANDLOCK_ACCESS_FS_REFER access right, the attempted link or rename + * operation must meet the following constraints: + * + * * The reparented file may not gain more access rights in the destination + * directory than it previously had in the source directory. If this is + * attempted, the operation results in an ``EXDEV`` error. + * + * * When linking or renaming, the ``LANDLOCK_ACCESS_FS_MAKE_*`` right for the + * respective file type must be granted for the destination directory. + * Otherwise, the operation results in an ``EACCES`` error. + * + * * When renaming, the ``LANDLOCK_ACCESS_FS_REMOVE_*`` right for the + * respective file type must be granted for the source directory. Otherwise, + * the operation results in an ``EACCES`` error. + * + * If multiple requirements are not met, the ``EACCES`` error code takes + * precedence over ``EXDEV``. * * .. warning:: *
Clarify the "refer" documentation by splitting up a big paragraph of text. - Call out specifically that the denial by default applies to ABI v1 as well. - Turn the three additional constraints for link/rename operations into bullet points, to give it more structure. Signed-off-by: Günther Noack <gnoack3000@gmail.com> --- include/uapi/linux/landlock.h | 46 +++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 15 deletions(-) base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c