On Wed, Mar 06, 2024 at 03:34:40PM -0800, Fan Wu 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>
As I've said before, this commit message needs some work. It currently doesn't
say anything about what the patch actually does.
BTW, please make sure you're Cc'ing the fsverity mailing list
(fsverity@xxxxxxxxxxxxxxx), not fscrypt (linux-fscrypt@xxxxxxxxxxxxxxx).
> diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> index 13e4b18e5dbb..64618a6141ab 100644
> --- a/Documentation/filesystems/fsverity.rst
> +++ b/Documentation/filesystems/fsverity.rst
> @@ -461,7 +461,9 @@ Enabling this option adds the following:
>
> 3. A new sysctl "fs.verity.require_signatures" is made available.
> When set to 1, the kernel requires that all verity files have a
> - correctly signed digest as described in (2).
> + correctly signed digest as described in (2). Note that verification
> + happens as long as the file's signature exists regardless the state of
> + "fs.verity.require_signatures".
>
> The data that the signature as described in (2) must be a signature of
> is the fs-verity file digest in the following format::
Doesn't anything else in this file need to be updated to document the IPE
support?
> diff --git a/fs/verity/open.c b/fs/verity/open.c
> index 6c31a871b84b..f917023255c8 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,
> + const struct fsverity_descriptor *desc)
> +{
> + return security_inode_setsecurity(inode, FS_VERITY_INODE_SEC_NAME,
> + desc->signature,
> + le32_to_cpu(desc->sig_size), 0);
Please call it something like FS_VERITY_INODE_BUILTIN_SIG to make it clear that
it's the builtin signature.
> +}
> +#else
> +static inline int fsverity_inode_setsecurity(struct inode *inode,
> + const struct fsverity_descriptor *desc)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_IPE_PROP_FS_VERITY*/
The above comment mentions CONFIG_IPE_PROP_FS_VERITY, but it doesn't appear
anywhere else in the patch.
> +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;
What is the "err == -EOPNOTSUPP" case intended to handle?
> diff --git a/fs/verity/signature.c b/fs/verity/signature.c
> index 90c07573dd77..42f58f4e45d0 100644
> --- a/fs/verity/signature.c
> +++ b/fs/verity/signature.c
> @@ -41,7 +41,9 @@ static struct key *fsverity_keyring;
> * @sig_size: size of signature in bytes, or 0 if no signature
> *
> * If the file includes a signature of its fs-verity file digest, verify it
> - * against the certificates in the fs-verity keyring.
> + * against the certificates in the fs-verity keyring. Note that verification
> + * happens as long as the file's signature exists regardless the state of
> + * fsverity_require_signatures.
Can you please make this mention explicitly that the LSM hook is relying on that
behavior?
- Eric
