Re: [PATCH v19 13/20] ipe: add support for dm-verity as a trust provider

Paul Moore
2024-05-30T01:44:10Z

On May 24, 2024 Fan Wu <wufan@linux.microsoft.com> wrote:
>
> Allows author of IPE policy to indicate trust for a singular dm-verity
> volume, identified by roothash, through "dmverity_roothash" and all
> signed and validated dm-verity volumes, through "dmverity_signature".
>
> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> ---
> v2:
> + No Changes
>
> v3:
> + No changes
>
> v4:
> + No changes
>
> v5:
> + No changes
>
> v6:
> + Fix an improper cleanup that can result in
> a leak
>
> v7:
> + Squash patch 08/12, 10/12 to [11/16]
>
> v8:
> + Undo squash of 08/12, 10/12 - separating drivers/md/ from security/
> & block/
> + Use common-audit function for dmverity_signature.
> + Change implementation for storing the dm-verity digest to use the
> newly introduced dm_verity_digest structure introduced in patch
> 14/20.
>
> v9:
> + Adapt to the new parser
>
> v10:
> + Select the Kconfig when all dependencies are enabled
>
> v11:
> + No changes
>
> v12:
> + Refactor to use struct digest_info* instead of void*
> + Correct audit format
>
> v13:
> + Remove the CONFIG_IPE_PROP_DM_VERITY dependency inside the parser
> to make the policy grammar independent of the kernel config.
>
> v14:
> + No changes
>
> v15:
> + Fix one grammar issue in KCONFIG
> + Switch to use security_bdev_setintegrity() hook
>
> v16:
> + Refactor for enum integrity type
>
> v17:
> + Add years to license header
> + Fix code and documentation style issues
> + Return -EINVAL in ipe_bdev_setintegrity when passed type is not
> supported
> + Use new enum name LSM_INT_DMVERITY_SIG_VALID
>
> v18:
> + Add Kconfig IPE_PROP_DM_VERITY_SIGNATURE and make both DM_VERITY
> config auto-selected
>
> v19:
> + No changes
> ---
> security/ipe/Kconfig | 27 ++++++++
> security/ipe/Makefile | 1 +
> security/ipe/audit.c | 29 ++++++++-
> security/ipe/digest.c | 118 +++++++++++++++++++++++++++++++++++
> security/ipe/digest.h | 26 ++++++++
> security/ipe/eval.c | 93 ++++++++++++++++++++++++++-
> security/ipe/eval.h | 12 ++++
> security/ipe/hooks.c | 93 +++++++++++++++++++++++++++
> security/ipe/hooks.h | 8 +++
> security/ipe/ipe.c | 15 +++++
> security/ipe/ipe.h | 4 ++
> security/ipe/policy.h | 3 +
> security/ipe/policy_parser.c | 24 ++++++-
> 13 files changed, 449 insertions(+), 4 deletions(-)
> create mode 100644 security/ipe/digest.c
> create mode 100644 security/ipe/digest.h

...

> diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
> index b68719bf44fb..51f1e63c295c 100644
> --- a/security/ipe/hooks.c
> +++ b/security/ipe/hooks.c
> @@ -191,3 +193,94 @@ void ipe_unpack_initramfs(void)
> {
> ipe_sb(current->fs->root.mnt->mnt_sb)->initramfs = true;
> }
> +
> +#ifdef CONFIG_IPE_PROP_DM_VERITY
> +/**
> + * ipe_bdev_free_security() - Free IPE's LSM blob of block_devices.
> + * @bdev: Supplies a pointer to a block_device that contains the structure
> + * to free.
> + */
> +void ipe_bdev_free_security(struct block_device *bdev)
> +{
> + struct ipe_bdev *blob = ipe_bdev(bdev);
> +
> + ipe_digest_free(blob->root_hash);
> +}
> +
> +#ifdef CONFIG_IPE_PROP_DM_VERITY_SIGNATURE
> +static void ipe_set_dmverity_signature(struct ipe_bdev *blob,
> + const void *value,
> + size_t size)
> +{
> + blob->dm_verity_signed = size > 0 && value;
> +}
> +#else
> +static inline void ipe_set_dmverity_signature(struct ipe_bdev *blob,
> + const void *value,
> + size_t size)
> +{
> +}
> +#endif /* CONFIG_IPE_PROP_DM_VERITY_SIGNATURE */
> +
> +/**
> + * ipe_bdev_setintegrity() - Save integrity data from a bdev to IPE's LSM blob.
> + * @bdev: Supplies a pointer to a block_device that contains the LSM blob.
> + * @type: Supplies the integrity type.
> + * @value: Supplies the value to store.
> + * @size: The size of @value.
> + *
> + * This hook is currently used to save dm-verity's root hash or the existence
> + * of a validated signed dm-verity root hash into LSM blob.
> + *
> + * Return: %0 on success. If an error occurs, the function will return the
> + * -errno.
> + */
> +int ipe_bdev_setintegrity(struct block_device *bdev, enum lsm_integrity_type type,
> + const void *value, size_t size)
> +{
> + const struct dm_verity_digest *digest = NULL;
> + struct ipe_bdev *blob = ipe_bdev(bdev);
> + struct digest_info *info = NULL;
> +
> + if (type == LSM_INT_DMVERITY_ROOTHASH) {
> + if (!value) {
> + ipe_digest_free(blob->root_hash);
> + blob->root_hash = NULL;
> +
> + return 0;
> + }
> + digest = value;
> +
> + info = kzalloc(sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + info->digest = kmemdup(digest->digest, digest->digest_len,
> + GFP_KERNEL);
> + if (!info->digest)
> + goto dmv_roothash_err;
> +
> + info->alg = kstrdup(digest->alg, GFP_KERNEL);
> + if (!info->alg)
> + goto dmv_roothash_err;
> +
> + info->digest_len = digest->digest_len;
> +
> + if (blob->root_hash)
> + ipe_digest_free(blob->root_hash);

The above if/free looks like a new addition from v18 and I'm not quite
sure why the `blob->root_hash` NULL check is necessary as
ipe_digest_free() does a IS_ERR_OR_NULL() check right at the top.

Likely harmless and doubtful to have any noticable performance impact,
but I wanted to mention it just in case ...

> + blob->root_hash = info;
> +
> + return 0;
> +dmv_roothash_err:
> + ipe_digest_free(info);
> +
> + return -ENOMEM;
> + } else if (type == LSM_INT_DMVERITY_SIG_VALID) {
> + ipe_set_dmverity_signature(blob, value, size);
> +
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +#endif /* CONFIG_IPE_PROP_DM_VERITY */

--
paul-moore.com