Re: [PATCH] landlock: Clarify documentation for struct landlock_ruleset_attr

["Mickaël Salaün" <mic@digikod.net>]

On Wed, Jul 10, 2024 at 12:01:34PM +0000, Günther Noack wrote:
> The explanation for @handled_access_fs and @handled_access_net has
> significant overlap and is better explained together.  I tried to clarify
> the wording and break up longer sentences as well.  I am putting emphasis
> on the word "handled" to make it clearer that "handled" is a term with
> special meaning in the context of Landlock.
> 
> I'd like to transfer this wording into the man pages as well.

Thanks for working on this.

> 
> Signed-off-by: Günther Noack <gnoack@google.com>
> Cc: Alejandro Colomar <alx@kernel.org>
> Cc: Mickaël Salaün <mic@digikod.net>
> Cc: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
> Cc: linux-security-module@vger.kernel.org
> ---
>  include/uapi/linux/landlock.h | 42 ++++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 68625e728f43..6f1b05c6995b 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -12,30 +12,32 @@
>  #include <linux/types.h>
>  
>  /**
> - * struct landlock_ruleset_attr - Ruleset definition
> + * struct landlock_ruleset_attr - Ruleset definition.
>   *
> - * Argument of sys_landlock_create_ruleset().  This structure can grow in
> - * future versions.
> + * @handled_access_fs: Bitmask of handled filesystem actions (cf. `Filesystem flags`_)
> + * @handled_access_net: Bitmask of handled network actions (cf. `Network flags`_)

These @handled_* lines should be kept close the the related fields to
ease maintenance and consistency.  It looks like the Sphinx rendering
would be the same.

> + *
> + * Argument of sys_landlock_create_ruleset().
> + *
> + * This struct defines a set of *handled access rights*, a set of actions on
> + * different object types, which should be denied by default when the ruleset is

> + * enacted.  Vice versa, access rights that are not specifically listed here are
> + * going to be allowed when the ruleset is enacted.

They could still be denied because of other access controls or parent
Landlock domains.

> + *
> + * One exception is the %LANDLOCK_ACCESS_FS_REFER access right, which is always
> + * implicitly *handled*, even when its bit is not set in @handled_access_fs.

I wrote this sentence but I now think it might be confusing.
LANDLOCK_ACCESS_FS_REFER couldn't be handled before it was introduced
(with Linux 5.19).  I couldn't find a better way to explain it though.

> + * However, in order to add new rules with this access right, the bit must still
> + * be set explicitly.
> + *
> + * The explicit listing of *handled access rights* is required for backwards
> + * compatibility reasons.  In most use cases, processes that use Landlock will
> + * *handle* a wide range or all access rights that they know about at build
> + * time.

...and that they tested with a kernel supporting all of them.

> + *
> + * This structure can grow in future Landlock versions.
>   */
>  struct landlock_ruleset_attr {
> -	/**
> -	 * @handled_access_fs: Bitmask of actions (cf. `Filesystem flags`_)
> -	 * that is handled by this ruleset and should then be forbidden if no
> -	 * rule explicitly allow them: it is a deny-by-default list that should
> -	 * contain as much Landlock access rights as possible. Indeed, all
> -	 * Landlock filesystem access rights that are not part of
> -	 * handled_access_fs are allowed.  This is needed for backward
> -	 * compatibility reasons.  One exception is the
> -	 * %LANDLOCK_ACCESS_FS_REFER access right, which is always implicitly
> -	 * handled, but must still be explicitly handled to add new rules with
> -	 * this access right.
> -	 */
>  	__u64 handled_access_fs;
> -	/**
> -	 * @handled_access_net: Bitmask of actions (cf. `Network flags`_)
> -	 * that is handled by this ruleset and should then be forbidden if no
> -	 * rule explicitly allow them.
> -	 */
>  	__u64 handled_access_net;
>  };
>  
> -- 
> 2.45.2.803.g4e1b14247a-goog
> 
>