Re: [PATCH RFC v11 15/19] fsverity: consume builtin signature via LSM hook

Paul Moore
2023-10-24T03:52:38Z

On Oct 4, 2023 Fan Wu <wufan@linux.microsoft.com> wrote:
>
> fsverity represents a mechanism to support both integrity and
> authenticity protection of a file, supporting both signed and unsigned
> digests.
>
> An LSM which controls access to a resource based on authenticity and
> integrity of said resource, can then use this data to make an informed
> decision on the authorization (provided by the LSM's policy) of said
> claim.
>
> This effectively allows the extension of a policy enforcement layer in
> LSM for fsverity, allowing for more granular control of how a
> particular authenticity claim can be used. For example, "all (built-in)
> signed fsverity files should be allowed to execute, but only these
> hashes are allowed to be loaded as kernel modules".
>
> This enforcement must be done in kernel space, as a userspace only
> solution would fail a simple litmus test: Download a self-contained
> malicious binary that never touches the userspace stack. This
> binary would still be able to execute.
>
> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> ---
> v1-v6:
> + Not present
>
> v7:
> Introduced
>
> v8:
> + Split fs/verity/ changes and security/ changes into separate patches
> + Change signature of fsverity_create_info to accept non-const inode
> + Change signature of fsverity_verify_signature to accept non-const inode
> + Don't cast-away const from inode.
> + Digest functionality dropped in favor of:
> ("fs-verity: define a function to return the integrity protected
> file digest")
> + Reworded commit description and title to match changes.
> + Fix a bug wherein no LSM implements the particular fsverity @name
> (or LSM is disabled), and returns -EOPNOTSUPP, causing errors.
>
> v9:
> + No changes
>
> v10:
> + Rename the signature blob key
> + Cleanup redundant code
> + Make the hook call depends on CONFIG_FS_VERITY_BUILTIN_SIGNATURES
>
> v11:
> + No changes
> ---
> fs/verity/fsverity_private.h | 2 +-
> fs/verity/open.c | 26 +++++++++++++++++++++++++-
> include/linux/fsverity.h | 2 ++
> 3 files changed, 28 insertions(+), 2 deletions(-)

We need an ACK from some VFS folks on this.

> diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
> index d071a6e32581..4a82716e852f 100644
> --- a/fs/verity/fsverity_private.h
> +++ b/fs/verity/fsverity_private.h
> @@ -108,7 +108,7 @@ int fsverity_init_merkle_tree_params(struct merkle_tree_params *params,
> unsigned int log_blocksize,
> const u8 *salt, size_t salt_size);
>
> -struct fsverity_info *fsverity_create_info(const struct inode *inode,
> +struct fsverity_info *fsverity_create_info(struct inode *inode,
> struct fsverity_descriptor *desc);
>
> void fsverity_set_info(struct inode *inode, struct fsverity_info *vi);
> diff --git a/fs/verity/open.c b/fs/verity/open.c
> index 6c31a871b84b..5b48e2c39086 100644
> --- a/fs/verity/open.c
> +++ b/fs/verity/open.c
> @@ -8,6 +8,7 @@
> #include "fsverity_private.h"
>
> #include <linux/mm.h>
> +#include <linux/security.h>
> #include <linux/slab.h>
>
> static struct kmem_cache *fsverity_info_cachep;
> @@ -172,12 +173,28 @@ static int compute_file_digest(const struct fsverity_hash_alg *hash_alg,
> return err;
> }
>
> +#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
> +static int fsverity_inode_setsecurity(struct inode *inode,
> + struct fsverity_descriptor *desc)
> +{
> + return security_inode_setsecurity(inode, FS_VERITY_INODE_SEC_NAME,
> + desc->signature,
> + le32_to_cpu(desc->sig_size), 0);
> +}
> +#else
> +static inline int fsverity_inode_setsecurity(struct inode *inode,
> + struct fsverity_descriptor *desc)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_IPE_PROP_FS_VERITY*/
> +
> /*
> * Create a new fsverity_info from the given fsverity_descriptor (with optional
> * appended builtin signature), and check the signature if present. The
> * fsverity_descriptor must have already undergone basic validation.
> */
> -struct fsverity_info *fsverity_create_info(const struct inode *inode,
> +struct fsverity_info *fsverity_create_info(struct inode *inode,
> struct fsverity_descriptor *desc)
> {
> struct fsverity_info *vi;
> @@ -242,6 +259,13 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
> spin_lock_init(&vi->hash_page_init_lock);
> }
>
> + err = fsverity_inode_setsecurity(inode, desc);
> + if (err == -EOPNOTSUPP)
> + err = 0;
> +
> + if (err)
> + goto fail;
> +
> return vi;
>
> fail:
> diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
> index 1eb7eae580be..9666721baf15 100644
> --- a/include/linux/fsverity.h
> +++ b/include/linux/fsverity.h
> @@ -319,4 +319,6 @@ static inline int fsverity_prepare_setattr(struct dentry *dentry,
> return 0;
> }
>
> +#define FS_VERITY_INODE_SEC_NAME "fsverity.builtin-sig"
> +
> #endif /* _LINUX_FSVERITY_H */
> --
> 2.25.1

--
paul-moore.com