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

[Paul Moore]

On Mon, Oct 23, 2023 at 11:52 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> On Oct  4, 2023 Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxx/

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