Re: [PATCH v16 16/20] fsverity: consume fsverity built-in signatures via LSM hook

[Eric Biggers]

On Thu, Mar 28, 2024 at 01:17:23PM -0700, Fan Wu wrote:
> fsverity: consume fsverity built-in signatures via LSM hook

Nothing is being "consumed" in this patch.  I think you might mean something
like "expose verified fsverity built-in signatures to LSMs".

> It enables a policy enforcement layer within LSMs for fsverity, offering
> granular control over the usage of authenticity claims. For instance, a policy
> could be established to permit the execution of all files with built-in
> fsverity signatures while restricting kernel module loading to specified
> hashes.

No, this patch does not enable "restricting kernel module loading to specified
hashes."  That can be done without this patch.

> The introduction of a security_inode_setintegrity() hook call within
> fsverity's workflow ensures that the verified built-in signature of a file
> is stored in the inode's LSM blobs.

No, it doesn't.  As I said on v15, this is not what IPE actually uses it for.

Also, even if IPE did cache the built-in signature in i_security, the mere fact
that it's cached would say nothing about what it's actually used for.

> diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> index 13e4b18e5dbb..e13cf10211c8 100644
> --- a/Documentation/filesystems/fsverity.rst
> +++ b/Documentation/filesystems/fsverity.rst
> @@ -86,6 +86,19 @@ authenticating fs-verity file hashes include:
>    signature in their "security.ima" extended attribute, as controlled
>    by the IMA policy.  For more information, see the IMA documentation.
>  
> +- Integrity Policy Enforcement (IPE).  IPE supports enforcing access
> +  control decisions based on immutable security properties of files,
> +  including those protected by fs-verity's built-in signatures.
> +  "IPE policy" specifically allows for the authorization of fs-verity
> +  files using properties such as ``fsverity_digest`` for identifying
> +  files by their verity digest, and ``fsverity_signature`` to validate
> +  files signed with fs-verity's built-in signature mechanism.

Maybe leave out the "such as" above, since fsverity_digest and
fsverity_signature are all the IPE properties related to fs-verity.

> +  This integration enhances security by ensuring the integrity and
> +  authenticity of files on a per-file basis, leveraging fs-verity's
> +  robust protection capabilities in conjunction with IPE's policy-driven
> +  access control.

This reads a bit like a marketing blurb and feels a bit out of place, especially
when it comes right after the paragraph about IMA which didn't include a similar
sentence even though the exact same sentence would apply to IMA too.  Maybe just
leave this sentence out.

> @@ -457,7 +470,10 @@ Enabling this option adds the following:
>     On success, the ioctl persists the signature alongside the Merkle
>     tree.  Then, any time the file is opened, the kernel verifies the
>     file's actual digest against this signature, using the certificates
> -   in the ".fs-verity" keyring.
> +   in the ".fs-verity" keyring. This verification happens as long as the
> +   file's signature exists, regardless of the state of the sysctl variable
> +   "fs.verity.require_signatures" described in the next item. The IPE LSM
> +   relies on this behavior to save verified signatures into LSM blobs.

No, IPE doesn't do that.

- Eric