[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <YqdwrWFXNtsKaTm7@google.com>
Date: Mon, 13 Jun 2022 10:15:25 -0700
From: Matthias Kaehlcke <mka@...omium.org>
To: Alasdair Kergon <agk@...hat.com>,
Mike Snitzer <snitzer@...nel.org>,
Kees Cook <keescook@...omium.org>,
James Morris <jmorris@...ei.org>,
"Serge E . Hallyn" <serge@...lyn.com>
Cc: Milan Broz <gmazyland@...il.com>, dm-devel@...hat.com,
linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org,
Song Liu <song@...nel.org>,
Douglas Anderson <dianders@...omium.org>,
linux-security-module@...r.kernel.org
Subject: Re: [PATCH v6 1/3] dm: Add verity helpers for LoadPin
Mike, does this look good to you or are any further changes needed
to get this landed?
Thanks
Matthias
On Mon, May 23, 2022 at 02:13:58PM -0700, Matthias Kaehlcke wrote:
> LoadPin limits loading of kernel modules, firmware and certain
> other files to a 'pinned' file system (typically a read-only
> rootfs). To provide more flexibility LoadPin is being extended
> to also allow loading these files from trusted dm-verity
> devices. For that purpose LoadPin can be provided with a list
> of verity root digests that it should consider as trusted.
>
> Add a bunch of helpers to allow LoadPin to check whether a DM
> device is a trusted verity device. The new functions broadly
> fall in two categories: those that need access to verity
> internals (like the root digest), and the 'glue' between
> LoadPin and verity. The new file dm-verity-loadpin.c contains
> the glue functions.
>
> Signed-off-by: Matthias Kaehlcke <mka@...omium.org>
> Acked-by: Kees Cook <keescook@...omium.org>
> ---
>
> Changes in v6:
> - s/loadpin_trusted_verity_root_digests/dm_verity_loadpin_trusted_root_digests/
> - s/trusted_root_digest/dm_verity_loadpin_trusted_root_digest/
> - removed unnecessary symbol exports
>
> Changes in v5:
> - changed dm_verity_loadpin_is_sb_trusted() to
> dm_verity_loadpin_is_bdev_trusted()
> - bumped version number to 1.8.1
> - deleted bad semicolon in declaration of stub for
> dm_verity_loadpin_is_bdev_trusted()
> - added 'Acked-by' tag from Kees
>
> Changes in v4:
> - a trusted verity device must have a single target of
> type 'verity'
> - share list of verity digests with loadpin, deleted
> dm_verity_loadpin_set_trusted_root_digests()
> - dm_verity_loadpin_is_md_trusted() is now dm_verity_loadpin_is_sb_trusted(),
> it receives a super_block instead of mapped_device. Updated kernel doc.
> - changed struct trusted_root_digest to have an unsized
> u8 array instead of a pointer
> - extend 'dm-verity-objs' instead of 'dm-mod-objs'
>
> Changes in v3:
> - none
>
> Changes in v2:
> - none
>
> drivers/md/Makefile | 6 +++
> drivers/md/dm-verity-loadpin.c | 74 +++++++++++++++++++++++++++++++
> drivers/md/dm-verity-target.c | 33 +++++++++++++-
> drivers/md/dm-verity.h | 4 ++
> include/linux/dm-verity-loadpin.h | 27 +++++++++++
> 5 files changed, 143 insertions(+), 1 deletion(-)
> create mode 100644 drivers/md/dm-verity-loadpin.c
> create mode 100644 include/linux/dm-verity-loadpin.h
>
> diff --git a/drivers/md/Makefile b/drivers/md/Makefile
> index 0454b0885b01..71771901c823 100644
> --- a/drivers/md/Makefile
> +++ b/drivers/md/Makefile
> @@ -108,6 +108,12 @@ ifeq ($(CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG),y)
> dm-verity-objs += dm-verity-verify-sig.o
> endif
>
> +ifeq ($(CONFIG_DM_VERITY),y)
> +ifeq ($(CONFIG_SECURITY_LOADPIN),y)
> +dm-verity-objs += dm-verity-loadpin.o
> +endif
> +endif
> +
> ifeq ($(CONFIG_DM_AUDIT),y)
> dm-mod-objs += dm-audit.o
> endif
> diff --git a/drivers/md/dm-verity-loadpin.c b/drivers/md/dm-verity-loadpin.c
> new file mode 100644
> index 000000000000..10c18bc1652c
> --- /dev/null
> +++ b/drivers/md/dm-verity-loadpin.c
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/list.h>
> +#include <linux/kernel.h>
> +#include <linux/dm-verity-loadpin.h>
> +
> +#include "dm.h"
> +#include "dm-verity.h"
> +
> +#define DM_MSG_PREFIX "verity-loadpin"
> +
> +LIST_HEAD(dm_verity_loadpin_trusted_root_digests);
> +
> +static bool is_trusted_verity_target(struct dm_target *ti)
> +{
> + u8 *root_digest;
> + unsigned int digest_size;
> + struct dm_verity_loadpin_trusted_root_digest *trd;
> + bool trusted = false;
> +
> + if (!dm_is_verity_target(ti))
> + return false;
> +
> + if (dm_verity_get_root_digest(ti, &root_digest, &digest_size))
> + return false;
> +
> + list_for_each_entry(trd, &dm_verity_loadpin_trusted_root_digests, node) {
> + if ((trd->len == digest_size) &&
> + !memcmp(trd->data, root_digest, digest_size)) {
> + trusted = true;
> + break;
> + }
> + }
> +
> + kfree(root_digest);
> +
> + return trusted;
> +}
> +
> +/*
> + * Determines whether the file system of a superblock is located on
> + * a verity device that is trusted by LoadPin.
> + */
> +bool dm_verity_loadpin_is_bdev_trusted(struct block_device *bdev)
> +{
> + struct mapped_device *md;
> + struct dm_table *table;
> + struct dm_target *ti;
> + int srcu_idx;
> + bool trusted = false;
> +
> + if (list_empty(&dm_verity_loadpin_trusted_root_digests))
> + return false;
> +
> + md = dm_get_md(bdev->bd_dev);
> + if (!md)
> + return false;
> +
> + table = dm_get_live_table(md, &srcu_idx);
> +
> + if (dm_table_get_num_targets(table) != 1)
> + goto out;
> +
> + ti = dm_table_get_target(table, 0);
> +
> + if (is_trusted_verity_target(ti))
> + trusted = true;
> +
> +out:
> + dm_put_live_table(md, srcu_idx);
> + dm_put(md);
> +
> + return trusted;
> +}
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index 80133aae0db3..c5395e93525a 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -19,6 +19,7 @@
> #include <linux/module.h>
> #include <linux/reboot.h>
> #include <linux/scatterlist.h>
> +#include <linux/string.h>
>
> #define DM_MSG_PREFIX "verity"
>
> @@ -1310,9 +1311,39 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
> return r;
> }
>
> +/*
> + * Check whether a DM target is a verity target.
> + */
> +bool dm_is_verity_target(struct dm_target *ti)
> +{
> + return ti->type->module == THIS_MODULE;
> +}
> +
> +/*
> + * Get the root digest of a verity target.
> + *
> + * Returns a copy of the root digest, the caller is responsible for
> + * freeing the memory of the digest.
> + */
> +int dm_verity_get_root_digest(struct dm_target *ti, u8 **root_digest, unsigned int *digest_size)
> +{
> + struct dm_verity *v = ti->private;
> +
> + if (!dm_is_verity_target(ti))
> + return -EINVAL;
> +
> + *root_digest = kmemdup(v->root_digest, v->digest_size, GFP_KERNEL);
> + if (*root_digest == NULL)
> + return -ENOMEM;
> +
> + *digest_size = v->digest_size;
> +
> + return 0;
> +}
> +
> static struct target_type verity_target = {
> .name = "verity",
> - .version = {1, 8, 0},
> + .version = {1, 8, 1},
> .module = THIS_MODULE,
> .ctr = verity_ctr,
> .dtr = verity_dtr,
> diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
> index 4e769d13473a..c832cc3e3d24 100644
> --- a/drivers/md/dm-verity.h
> +++ b/drivers/md/dm-verity.h
> @@ -129,4 +129,8 @@ extern int verity_hash(struct dm_verity *v, struct ahash_request *req,
> extern int verity_hash_for_block(struct dm_verity *v, struct dm_verity_io *io,
> sector_t block, u8 *digest, bool *is_zero);
>
> +extern bool dm_is_verity_target(struct dm_target *ti);
> +extern int dm_verity_get_root_digest(struct dm_target *ti, u8 **root_digest,
> + unsigned int *digest_size);
> +
> #endif /* DM_VERITY_H */
> diff --git a/include/linux/dm-verity-loadpin.h b/include/linux/dm-verity-loadpin.h
> new file mode 100644
> index 000000000000..fb695ecaa5d5
> --- /dev/null
> +++ b/include/linux/dm-verity-loadpin.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __LINUX_DM_VERITY_LOADPIN_H
> +#define __LINUX_DM_VERITY_LOADPIN_H
> +
> +#include <linux/list.h>
> +
> +struct block_device;
> +
> +extern struct list_head dm_verity_loadpin_trusted_root_digests;
> +
> +struct dm_verity_loadpin_trusted_root_digest {
> + struct list_head node;
> + unsigned int len;
> + u8 data[];
> +};
> +
> +#if IS_ENABLED(CONFIG_SECURITY_LOADPIN) && IS_BUILTIN(CONFIG_DM_VERITY)
> +bool dm_verity_loadpin_is_bdev_trusted(struct block_device *bdev);
> +#else
> +static inline bool dm_verity_loadpin_is_bdev_trusted(struct block_device *bdev)
> +{
> + return false;
> +}
> +#endif
> +
> +#endif /* __LINUX_DM_VERITY_LOADPIN_H */
> --
> 2.36.1.124.g0e6072fb45-goog
>
Powered by blists - more mailing lists