Re: [RFC PATCH v7 12/16] fsverity|security: add security hooks to fsverity digest and signature

Eric Biggers
2021-10-13T19:24:21Z

On Wed, Oct 13, 2021 at 12:06:31PM -0700, deven.desai@linux.microsoft.com wrote:
> From: Fan Wu <wufan@linux.microsoft.com>
>
> Add security_inode_setsecurity to fsverity signature verification.
> This can let LSMs save the signature data and digest hashes provided
> by fsverity.

Can you elaborate on why LSMs need this information?

>
> Also changes the implementaion inside the hook function to let
> multiple LSMs can add hooks.

Please split fs/verity/ changes and security/ changes into separate patches, if
possible.

>
> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>

> @@ -177,6 +178,17 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
> fsverity_err(inode, "Error %d computing file digest", err);
> goto out;
> }
> +
> + err = security_inode_setsecurity((struct inode *)inode,

If a non-const inode is needed, please propagate that into the callers rather
than randomly casting away the const.

> + FS_VERITY_DIGEST_SEC_NAME,
> + vi->file_digest,
> + vi->tree_params.hash_alg->digest_size,
> + 0);

The digest isn't meaningful without knowing the hash algorithm it uses.
It's available here, but you aren't passing it to this function.

> @@ -84,7 +85,9 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
>
> pr_debug("Valid signature for file digest %s:%*phN\n",
> hash_alg->name, hash_alg->digest_size, vi->file_digest);
> - return 0;
> + return security_inode_setsecurity((struct inode *)inode,

Likewise, please don't cast away const.

> + FS_VERITY_SIGNATURE_SEC_NAME,
> + signature, sig_size, 0);

This is only for fs-verity built-in signatures which aren't the only way to do
signatures with fs-verity. Are you sure this is what you're looking for? Can
you elaborate on your use case for fs-verity built-in signatures, and what the
LSM hook will do with them?

- Eric