On Fri, May 13, 2022 at 12:32:12PM -0400, Mike Snitzer wrote:
> On Wed, May 04 2022 at 3:54P -0400,
> Matthias Kaehlcke <m...@chromium.org> wrote:
>
> > Extend LoadPin to allow loading of kernel files from trusted dm-verity [1]
> > devices.
> >
> > This change adds the concept of trusted verity devices to LoadPin. LoadPin
> > maintains a list of root digests of verity devices it considers trusted.
> > Userspace can populate this list through an ioctl on the new LoadPin
> > securityfs entry 'dm-verity'. The ioctl receives a file descriptor of
> > a file with verity digests as parameter. Verity reads the digests from
> > this file after confirming that the file is located on the pinned root.
> > The list of trusted digests can only be set up once, which is typically
> > done at boot time.
> >
> > When a kernel file is read LoadPin first checks (as usual) whether the file
> > is located on the pinned root, if so the file can be loaded. Otherwise, if
> > the verity extension is enabled, LoadPin determines whether the file is
> > located on a verity backed device and whether the root digest of that
> > device is in the list of trusted digests. The file can be loaded if the
> > verity device has a trusted root digest.
> >
> > Background:
> >
> > As of now LoadPin restricts loading of kernel files to a single pinned
> > filesystem, typically the rootfs. This works for many systems, however it
> > can result in a bloated rootfs (and OTA updates) on platforms where
> > multiple boards with different hardware configurations use the same rootfs
> > image. Especially when 'optional' files are large it may be preferable to
> > download/install them only when they are actually needed by a given board.
> > Chrome OS uses Downloadable Content (DLC) [2] to deploy certain 'packages'
> > at runtime. As an example a DLC package could contain firmware for a
> > peripheral that is not present on all boards. DLCs use dm-verity to verify
> > the integrity of the DLC content.
> >
> > [1]
> > https://www.kernel.org/doc/html/latest/admin-guide/device-mapper/verity.html
> > [2]
> > https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/dlcservice/docs/developer.md
> >
> > Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
> > ---
> >
> > Changes in v3:
> > - added securityfs for LoadPin (currently only populated when
> > CONFIG_SECURITY_LOADPIN_VERITY=y)
> > - added uapi include for LoadPin
> > - changed the interface for setting up the list of trusted
> > digests from sysctl to ioctl on securityfs entry
> > - added stub for loadpin_is_fs_trusted() to be used
> > CONFIG_SECURITY_LOADPIN_VERITY is not select
> > - depend on CONFIG_SECURITYFS instead of CONFIG_SYSTCL
> > - updated Kconfig help
> > - minor changes in read_trusted_verity_root_digests()
> > - updated commit message
> >
> > Changes in v2:
> > - userspace now passes the path of the file with the verity digests
> > via systcl, instead of the digests themselves
> > - renamed sysctl file to 'trusted_verity_root_digests_path'
> > - have CONFIG_SECURITY_LOADPIN_VERITY depend on CONFIG_SYSCTL
> > - updated Kconfig doc
> > - updated commit message
> >
> > include/uapi/linux/loadpin.h | 19 ++++
> > security/loadpin/Kconfig | 16 +++
> > security/loadpin/loadpin.c | 184 ++++++++++++++++++++++++++++++++++-
> > 3 files changed, 218 insertions(+), 1 deletion(-)
> > create mode 100644 include/uapi/linux/loadpin.h
>
> I would certainly need some Reviewed-by:s from security and/or loadpin
> experts if I were to pick this patch up.
Yes, I think Kees (LoadPin maintainer) is generally on board with the idea,
but a more in depth review is still pending.
I'll send out a new revision which addresses the current outstanding
comments soon.
> Did you see the issues the kernel test robot emailed about?
>
> You'd do well to fix those issues up when submitting another revision
> of this patchset.
Yes, I plan to address those in the next revision. Thanks for the reminder!
> >
> > diff --git a/include/uapi/linux/loadpin.h b/include/uapi/linux/loadpin.h
> > new file mode 100644
> > index 000000000000..d303a582209b
> > --- /dev/null
> > +++ b/include/uapi/linux/loadpin.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > +/*
> > + * Copyright (c) 2022, Google LLC
> > + */
> > +
> > +#ifndef _UAPI_LINUX_LOOP_LOADPIN_H
> > +#define _UAPI_LINUX_LOOP_LOADPIN_H
> > +
> > +#define LOADPIN_IOC_MAGIC 'L'
> > +
> > +/**
> > + * LOADPIN_IOC_SET_TRUSTED_VERITY_DIGESTS - Set up the root digests of
> > verity devices
> > + * that loadpin should trust.
> > + *
> > + * Takes a file descriptor from which to read the root digests of trusted
> > verity devices.
> > + */
> > +#define LOADPIN_IOC_SET_TRUSTED_VERITY_DIGESTS _IOW(LOADPIN_IOC_MAGIC,
> > 0x00, unsigned int)
> > +
> > +#endif /* _UAPI_LINUX_LOOP_LOADPIN_H */
> > diff --git a/security/loadpin/Kconfig b/security/loadpin/Kconfig
> > index 91be65dec2ab..e319ca8e3f3d 100644
> > --- a/security/loadpin/Kconfig
> > +++ b/security/loadpin/Kconfig
> > @@ -18,3 +18,19 @@ config SECURITY_LOADPIN_ENFORCE
> > If selected, LoadPin will enforce pinning at boot. If not
> > selected, it can be enabled at boot with the kernel parameter
> > "loadpin.enforce=1".
> > +
> > +config SECURITY_LOADPIN_VERITY
> > + bool "Allow reading files from certain other filesystems that use
> > dm-verity"
> > + depends on DM_VERITY=y && SECURITYFS
> > + help
> > + If selected LoadPin can allow reading files from filesystems
> > + that use dm-verity. LoadPin maintains a list of verity root
> > + digests it considers trusted. A verity backed filesystem is
> > + considered trusted if its root digest is found in the list
> > + of trusted digests.
> > +
> > + The list of trusted verity can be populated through an ioctl
> > + on the LoadPin securityfs entry 'dm-verity'. The ioctl
> > + expects a file descriptor of a file with verity digests as
> > + parameter. The file must be located on the pinned root and
> > + contain a comma separated list of digests.
> > diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> > index b12f7d986b1e..c29ce562a366 100644
> > --- a/security/loadpin/loadpin.c
> > +++ b/security/loadpin/loadpin.c
> > @@ -18,6 +18,9 @@
> > #include <linux/path.h>
> > #include <linux/sched.h> /* current */
> > #include <linux/string_helpers.h>
> > +#include <linux/device-mapper.h>
> > +#include <linux/dm-verity-loadpin.h>
> > +#include <uapi/linux/loadpin.h>
> >
> > static void report_load(const char *origin, struct file *file, char
> > *operation)
> > {
> > @@ -43,6 +46,9 @@ static char *exclude_read_files[READING_MAX_ID];
> > static int ignore_read_file_id[READING_MAX_ID] __ro_after_init;
> > static struct super_block *pinned_root;
> > static DEFINE_SPINLOCK(pinned_root_spinlock);
> > +#ifdef CONFIG_SECURITY_LOADPIN_VERITY
> > +static LIST_HEAD(trusted_verity_root_digests);
> > +#endif
> >
> > #ifdef CONFIG_SYSCTL
> >
> > @@ -118,6 +124,24 @@ static void loadpin_sb_free_security(struct
> > super_block *mnt_sb)
> > }
> > }
> >
> > +#ifdef CONFIG_SECURITY_LOADPIN_VERITY
> > +static bool loadpin_is_fs_trusted(struct super_block *sb)
> > +{
> > + struct mapped_device *md = dm_get_md(sb->s_bdev->bd_dev);
> > + bool trusted;
> > +
> > + if (!md)
> > + return false;
> > +
> > + trusted = dm_verity_loadpin_is_md_trusted(md);
> > + dm_put(md);
> > +
> > + return trusted;
> > +}
> > +#else
> > +static inline bool loadpin_is_fs_trusted(struct super_block *sb) { return
> > false; };
> > +#endif
> > +
> > static int loadpin_read_file(struct file *file, enum kernel_read_file_id
> > id,
> > bool contents)
> > {
> > @@ -174,7 +198,8 @@ static int loadpin_read_file(struct file *file, enum
> > kernel_read_file_id id,
> > spin_unlock(&pinned_root_spinlock);
> > }
> >
> > - if (IS_ERR_OR_NULL(pinned_root) || load_root != pinned_root) {
> > + if (IS_ERR_OR_NULL(pinned_root) ||
> > + ((load_root != pinned_root) && !loadpin_is_fs_trusted(load_root))) {
> > if (unlikely(!enforce)) {
> > report_load(origin, file, "pinning-ignored");
> > return 0;
> > @@ -240,6 +265,7 @@ static int __init loadpin_init(void)
> > enforce ? "" : "not ");
> > parse_exclude();
> > security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
> > +
> > return 0;
> > }
> >
> > @@ -248,6 +274,162 @@ DEFINE_LSM(loadpin) = {
> > .init = loadpin_init,
> > };
> >
> > +#ifdef CONFIG_SECURITY_LOADPIN_VERITY
> > +
> > +enum loadpin_securityfs_interface_index {
> > + LOADPIN_DM_VERITY,
> > +};
> > +
> > +static int read_trusted_verity_root_digests(unsigned int fd)
> > +{
> > + struct fd f;
> > + void *data;
> > + int rc;
> > + char *p, *d;
> > +
> > + /* The list of trusted root digests can only be set up once */
> > + if (!list_empty(&trusted_verity_root_digests))
> > + return -EPERM;
> > +
> > + f = fdget(fd);
> > + if (!f.file)
> > + return -EINVAL;
> > +
> > + data = kzalloc(SZ_4K, GFP_KERNEL);
> > + if (!data) {
> > + rc = -ENOMEM;
> > + goto err;
> > + }
> > +
> > + rc = kernel_read_file(f.file, 0, &data, SZ_4K - 1, NULL,
> > READING_POLICY);
> > + if (rc < 0)
> > + goto err;
> > +
> > + ((char *)data)[rc] = '\0';
> > +
> > + p = strim(data);
> > + while ((d = strsep(&p, ",")) != NULL) {
> > + int len = strlen(d);
> > + struct trusted_root_digest *trd;
> > +
> > + if (len % 2) {
> > + rc = -EPROTO;
> > + goto err;
> > + }
> > +
> > + len /= 2;
> > +
> > + trd = kzalloc(sizeof(*trd), GFP_KERNEL);
> > + if (!trd) {
> > + rc = -ENOMEM;
> > + goto err;
> > + }
> > +
> > + trd->data = kzalloc(len, GFP_KERNEL);
> > + if (!trd->data) {
> > + kfree(trd);
> > + rc = -ENOMEM;
> > + goto err;
> > + }
> > +
> > + if (hex2bin(trd->data, d, len)) {
> > + kfree(trd);
> > + kfree(trd->data);
> > + rc = -EPROTO;
> > + goto err;
> > + }
> > +
> > + trd->len = len;
> > +
> > + list_add_tail(&trd->node, &trusted_verity_root_digests);
> > + }
> > +
> > + kfree(data);
> > + fdput(f);
> > +
> > + if (!list_empty(&trusted_verity_root_digests))
> > +
> > dm_verity_loadpin_set_trusted_root_digests(&trusted_verity_root_digests);
> > +
> > + return 0;
> > +
> > +err:
> > + kfree(data);
> > +
> > + {
> > + struct trusted_root_digest *trd, *tmp;
> > +
> > + list_for_each_entry_safe(trd, tmp,
> > &trusted_verity_root_digests, node) {
> > + kfree(trd->data);
> > + list_del(&trd->node);
> > + kfree(trd);
> > + }
> > + }
> > +
> > + fdput(f);
> > +
> > + return rc;
> > +}
> > +
> > +/******************************** securityfs
> > ********************************/
> > +
> > +static long dm_verity_ioctl(struct file *filp, unsigned int cmd, unsigned
> > long arg)
> > +{
> > + void __user *uarg = (void __user *)arg;
> > + unsigned int fd;
> > + int rc;
> > +
> > + switch (cmd) {
> > + case LOADPIN_IOC_SET_TRUSTED_VERITY_DIGESTS:
> > + rc = copy_from_user(&fd, uarg, sizeof(fd));
> > + if (rc)
> > + return rc;
> > +
> > + return read_trusted_verity_root_digests(fd);
> > +
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +static const struct file_operations loadpin_dm_verity_ops = {
> > + .unlocked_ioctl = dm_verity_ioctl,
> > + .compat_ioctl = compat_ptr_ioctl,
> > +};
> > +
> > +/**
> > + * init_loadpin_securityfs - create the securityfs directory for LoadPin
> > + *
> > + * We can not put this method normally under the loadpin_init() code path
> > since
> > + * the security subsystem gets initialized before the vfs caches.
> > + *
> > + * Returns 0 if the securityfs directory creation was successful.
> > + */
> > +static int __init init_loadpin_securityfs(void)
> > +{
> > + struct dentry *loadpin_dir, *dentry;
> > +
> > + loadpin_dir = securityfs_create_dir("loadpin", NULL);
> > + if (IS_ERR(loadpin_dir)) {
> > + pr_err("LoadPin: could not create securityfs dir: %d\n",
> > + PTR_ERR(loadpin_dir));
> > + return PTR_ERR(loadpin_dir);
> > + }
> > +
> > + dentry = securityfs_create_file("dm-verity", 0600, loadpin_dir,
> > + (void *)LOADPIN_DM_VERITY,
> > &loadpin_dm_verity_ops);
> > + if (IS_ERR(dentry)) {
> > + pr_err("LoadPin: could not create securityfs entry 'dm-verity':
> > %d\n",
> > + PTR_ERR(dentry));
> > + return PTR_ERR(dentry);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +fs_initcall(init_loadpin_securityfs);
> > +
> > +#endif /* CONFIG_SECURITY_LOADPIN_VERITY */
> > +
> > /* Should not be mutable after boot, so not listed in sysfs (perm == 0). */
> > module_param(enforce, int, 0);
> > MODULE_PARM_DESC(enforce, "Enforce module/firmware pinning");
> > --
> > 2.36.0.464.gb9c8b46e94-goog
> >
>
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
