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

[Eric Biggers]

On Wed, Oct 13, 2021 at 12:06:31PM -0700, deven.desai@xxxxxxxxxxxxxxxxxxx wrote:
> From: Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx>

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