Re: [RFC PATCH v9 03/16] ipe: add evaluation loop and introduce 'boot_verified' as a trust provider

On Thu, Mar 02, 2023 at 02:03:11PM -0500, Paul Moore wrote: > On Mon, Jan 30, 2023 at 5:58???PM Fan Wu <wufan(a)linux.microsoft.com&gt; wrote: > > > > From: Deven Bowers <deven.desai(a)linux.microsoft.com&gt; > > > > IPE must have a centralized function to evaluate incoming callers > > against IPE's policy. This iteration of the policy against the rules > > for that specific caller is known as the evaluation loop. > > > > In addition, IPE is designed to provide system level trust guarantees, > > this usually implies that trust starts from bootup with a hardware root > > of trust, which validates the bootloader. After this, the bootloader > > verifies the kernel and the initramfs. > > > > As there's no currently supported integrity method for initramfs, and > > it's typically already verified by the bootloader, introduce a property > > that causes the first superblock to have an execution to be "pinned", > > which is typically initramfs. > > > > Signed-off-by: Deven Bowers <deven.desai(a)linux.microsoft.com&gt; > > Signed-off-by: Fan Wu <wufan(a)linux.microsoft.com&gt; > > ... > > > --- > > security/ipe/Makefile | 1 + > > security/ipe/eval.c | 180 +++++++++++++++++++++++++++++++++++ > > security/ipe/eval.h | 28 ++++++ > > security/ipe/hooks.c | 25 +++++ > > security/ipe/hooks.h | 14 +++ > > security/ipe/ipe.c | 1 + > > security/ipe/policy.c | 20 ++++ > > security/ipe/policy.h | 3 + > > security/ipe/policy_parser.c | 8 +- > > 9 files changed, 279 insertions(+), 1 deletion(-) > > create mode 100644 security/ipe/eval.c > > create mode 100644 security/ipe/eval.h > > create mode 100644 security/ipe/hooks.c > > create mode 100644 security/ipe/hooks.h

> > diff --git a/security/ipe/eval.c b/security/ipe/eval.c > > new file mode 100644 > > index 000000000000..48b5104a3463 > > --- /dev/null > > +++ b/security/ipe/eval.c > > @@ -0,0 +1,180 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) Microsoft Corporation. All rights reserved. > > + */ > > + > > +#include "ipe.h" > > +#include "eval.h" > > +#include "hooks.h" > > +#include "policy.h" > > + > > +#include <linux/fs.h> > > +#include <linux/types.h> > > +#include <linux/slab.h> > > +#include <linux/file.h> > > +#include <linux/sched.h> > > +#include <linux/rcupdate.h> > > +#include <linux/spinlock.h> > > + > > +struct ipe_policy __rcu *ipe_active_policy; > > + > > +static struct super_block *pinned_sb; > > +static DEFINE_SPINLOCK(pin_lock); > > +#define FILE_SUPERBLOCK(f) ((f)->f_path.mnt->mnt_sb) > > + > > +/** > > + * pin_sb - Pin the underlying superblock of @f, marking it as trusted. > > + * @f: Supplies a file structure to source the super_block from. > > + */ > > +static void pin_sb(const struct file *f) > > +{ > > + if (!f) > > + return; > > + spin_lock(&pin_lock); > > + if (pinned_sb) > > + goto out; > > + pinned_sb = FILE_SUPERBLOCK(f); > > +out: > > + spin_unlock(&pin_lock); > > +} > > Since you don't actually use @f, just the super_block, you might > consider passing the super_block as the parameter and not the > associated file. > > I'd probably also flip the if-then to avoid the 'goto', for example: > > static void pin_sb(const struct super_block *sb) > { > if (!sb) > return; > spin_lock(&pin_lock); > if (!pinned_sb) > pinned_sb = sb; > spin_unlock(&pin_lock); > } > Sure, I can change the code accordingly. > Also, do we need to worry about the initramfs' being unmounted and the > super_block going away? If initramfs is being unmounted, the boot_verified property will never be TRUE, which is an expected behavior. In an actual use case, we can leverage this property to only enable files in initramfs during the booting stage, and later switch to another policy without the boot_verified property after unmounting the initramfs. This approach helps keep the allowed set of files minimum at each stage.