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

Paul Moore
2023-11-02T00:40:27Z

On Mon, Oct 23, 2023 at 11:52 PM Paul Moore <paul@paul-moore.com> wrote:
> 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.

Eric and/or Ted, can we get either an ACK or some feedback on this patch?

For reference, the full patchset can be found on lore at the link below:

https://lore.kernel.org/linux-security-module/1696457386-3010-1-git-send-email-wufan@linux.microsoft.com/

> > 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