Re: [RFC PATCH v7 11/16] ipe: add support for dm-verity as a trust provider

[Deven Bowers <deven.desai@linux.microsoft.com>]

On 11/25/2021 1:37 AM, Roberto Sassu wrote:
>> From: deven.desai@linux.microsoft.com
>> [mailto:deven.desai@linux.microsoft.com]
>> Sent: Wednesday, October 13, 2021 9:07 PM
>> From: Deven Bowers <deven.desai@linux.microsoft.com>

..snip

>> diff --git a/security/ipe/modules/Makefile b/security/ipe/modules/Makefile
>> index e0045ec65434..84fadce85193 100644
>> --- a/security/ipe/modules/Makefile
>> +++ b/security/ipe/modules/Makefile
>> @@ -6,3 +6,5 @@
>>   #
>>
>>   obj-$(CONFIG_IPE_PROP_BOOT_VERIFIED) += boot_verified.o
>> +obj-$(CONFIG_IPE_PROP_DM_VERITY_SIGNATURE) += dmverity_signature.o
>> +obj-$(CONFIG_IPE_PROP_DM_VERITY_ROOTHASH) += dmverity_roothash.o
>> diff --git a/security/ipe/modules/dmverity_roothash.c
>> b/security/ipe/modules/dmverity_roothash.c
>> new file mode 100644
>> index 000000000000..0f82bec3b842
>> --- /dev/null
>> +++ b/security/ipe/modules/dmverity_roothash.c
>> @@ -0,0 +1,80 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) Microsoft Corporation. All rights reserved.
>> + */
>> +
>> +#include "ipe_module.h"
>> +
>> +#include <linux/fs.h>
>> +#include <linux/types.h>
>> +
>> +struct counted_array {
>> +	size_t	len;
>> +	u8     *data;
>> +};
>> +
>> +static int dvrh_parse(const char *valstr, void **value)
>> +{
>> +	int rv = 0;
>> +	struct counted_array *arr;
>> +
>> +	arr = kzalloc(sizeof(*arr), GFP_KERNEL);
>> +	if (!arr) {
>> +		rv = -ENOMEM;
>> +		goto err;
>> +	}
>> +
>> +	arr->len = (strlen(valstr) / 2);
>> +
>> +	arr->data = kzalloc(arr->len, GFP_KERNEL);
>> +	if (!arr->data) {
>> +		rv = -ENOMEM;
>> +		goto err;
>> +	}
>> +
>> +	rv = hex2bin(arr->data, valstr, arr->len);
>> +	if (rv != 0)
>> +		goto err2;
>> +
>> +	*value = arr;
>> +	return rv;
>> +err2:
>> +	kfree(arr->data);
>> +err:
>> +	kfree(arr);
>> +	return rv;
>> +}
>> +
>> +static bool dvrh_eval(const struct ipe_eval_ctx *ctx, const void *val)
>> +{
>> +	const u8 *src;
>> +	struct counted_array *expect = (struct counted_array *)val;
>> +
>> +	if (!ctx->ipe_bdev)
>> +		return false;
>> +
>> +	if (ctx->ipe_bdev->hashlen != expect->len)
>> +		return false;
>> +
>> +	src = ctx->ipe_bdev->hash;
>> +
>> +	return !memcmp(expect->data, src, expect->len);
> Hi Deven
>
> I was curious to see if determining the property at run-time
> could apply also to dm-verity. It seems it could be done
> (I omit some checks, I also keep the expected value in hex
> format):
>
> ---
>          md = dm_get_md(file_inode(ctx->file)->i_sb->s_dev);
>          table = dm_get_live_table(md, &srcu_idx);
>          num_targets = dm_table_get_num_targets(table);
>
>          for (i = 0; i < num_targets; i++) {
>                  struct dm_target *ti = dm_table_get_target(table, i);
>
>                  if (strcmp(ti->type->name, "verity"))
>                          continue;
>
>                  ti->type->status(ti, STATUSTYPE_IMA, 0, result, sizeof(result));
>          }
>
>          dm_put_live_table(md, srcu_idx);
>          dm_put(md);
>
>          root_digest_ptr = strstr(result, "root_digest=");
>          return !strncmp(expect->data, root_digest_ptr + 12, expect->len);
> ---
>
> Only dm_table_get_target() is not exported yet, but I guess it could
> be. dm_table_get_num_targets() is exported.

I had tried something similar in a very early draft of IPE. The issue
that comes with this is that when you compile device-mapper as a module
(CONFIG_BLK_DEV_DM=m) you start to get linking errors with this
approach.

Obviously, we can fix this in the IPE's module Kconfig by setting the
dependency to be =y, but it's something to highlight. My general
preference is to support the =m configuration by using these blobs.

The runtime approach does work with fs-verity, because fs-verity is a
file-system level feature that cannot be compiled as a module.

> With this code, you would not have to manage security blobs
> outside IPE. Maybe you could add a blob for the super block, so
> that you verify the dm-verity property just once per filesystem.
>
> Roberto
>
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
>
>> +}
>> +
>> +static int dvrh_free(void **val)
>> +{
>> +	struct counted_array *expect = (struct counted_array *)val;
>> +
>> +	kfree(expect->data);
>> +	kfree(expect);
>> +
>> +	return 0;
>> +}
>> +
>> +IPE_MODULE(dvrh) = {
>> +	.name = "dmverity_roothash",
>> +	.version = 1,
>> +	.parse = dvrh_parse,
>> +	.free = dvrh_free,
>> +	.eval = dvrh_eval,
>> +};
>> diff --git a/security/ipe/modules/dmverity_signature.c
>> b/security/ipe/modules/dmverity_signature.c
>> new file mode 100644
>> index 000000000000..08746fcbcb3e
>> --- /dev/null
>> +++ b/security/ipe/modules/dmverity_signature.c
>> @@ -0,0 +1,25 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) Microsoft Corporation. All rights reserved.
>> + */
>> +
>> +#include "ipe_module.h"
>> +
>> +#include <linux/fs.h>
>> +#include <linux/types.h>
>> +
>> +static bool dvv_eval(const struct ipe_eval_ctx *ctx, const void *val)
>> +{
>> +	bool expect = (bool)val;
>> +	bool eval = ctx->ipe_bdev && (!!ctx->ipe_bdev->sigdata);
>> +
>> +	return expect == eval;
>> +}
>> +
>> +IPE_MODULE(dvv) = {
>> +	.name = "dmverity_signature",
>> +	.version = 1,
>> +	.parse = ipe_bool_parse,
>> +	.free = NULL,
>> +	.eval = dvv_eval,
>> +};
>> --
>> 2.33.0