Linux-FSCrypt Archive mirror
 help / color / mirror / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Fan Wu <wufan@linux.microsoft.com>
Cc: corbet@lwn.net, zohar@linux.ibm.com, jmorris@namei.org,
	serge@hallyn.com, tytso@mit.edu, ebiggers@kernel.org,
	axboe@kernel.dk, agk@redhat.com, snitzer@kernel.org,
	eparis@redhat.com, linux-doc@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-fscrypt@vger.kernel.org, linux-block@vger.kernel.org,
	dm-devel@redhat.com, audit@vger.kernel.org,
	roberto.sassu@huawei.com, linux-kernel@vger.kernel.org,
	Deven Bowers <deven.desai@linux.microsoft.com>
Subject: Re: [PATCH RFC v11 5/19] ipe: introduce 'boot_verified' as a trust provider
Date: Fri, 3 Nov 2023 18:30:16 -0400	[thread overview]
Message-ID: <CAHC9VhQLbgvg6syOB2_GYFK+zpWbyx6zrDoM1aBnA18u_Qjj4g@mail.gmail.com> (raw)
In-Reply-To: <CAHC9VhR9scT7V7dvN5zhAYdExORB9arWaR7Gbix1AUtAMDPHcg@mail.gmail.com>

On Fri, Nov 3, 2023 at 6:15 PM Paul Moore <paul@paul-moore.com> wrote:
> On Thu, Nov 2, 2023 at 6:46 PM Fan Wu <wufan@linux.microsoft.com> wrote:
> > On 10/26/2023 3:12 PM, Paul Moore wrote:
> > > On Thu, Oct 26, 2023 at 5:33 PM Fan Wu <wufan@linux.microsoft.com> wrote:
> > >> On 10/23/2023 8:52 PM, Paul Moore wrote:
> > >>> On Oct  4, 2023 Fan Wu <wufan@linux.microsoft.com> wrote:
> > >>>>
> > >>>> 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.
> > >>>>
> > >>>> When the "pinned" device is unmounted, it will be "unpinned" and
> > >>>> `boot_verified` property will always evaluate to false afterward.
> > >>>>
> > >>>> We use a pointer with a spin_lock to "pin" the device instead of rcu
> > >>>> because rcu synchronization may sleep, which is not allowed when
> > >>>> unmounting a device.
> > >>>>
> > >>>> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> > >>>> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> > >> ...
> > >>>> ---
> > >>>>    security/ipe/eval.c          | 72 +++++++++++++++++++++++++++++++++++-
> > >>>>    security/ipe/eval.h          |  2 +
> > >>>>    security/ipe/hooks.c         | 12 ++++++
> > >>>>    security/ipe/hooks.h         |  2 +
> > >>>>    security/ipe/ipe.c           |  1 +
> > >>>>    security/ipe/policy.h        |  2 +
> > >>>>    security/ipe/policy_parser.c | 35 +++++++++++++++++-
> > >>>>    7 files changed, 124 insertions(+), 2 deletions(-)
>
> ...
>
> > >>>> +/**
> > >>>> + * from_pinned - Determine whether @sb is the pinned super_block.
> > >>>> + * @sb: Supplies a super_block to check against the pinned super_block.
> > >>>> + *
> > >>>> + * Return:
> > >>>> + * * true   - @sb is the pinned super_block
> > >>>> + * * false  - @sb is not the pinned super_block
> > >>>> + */
> > >>>> +static bool from_pinned(const struct super_block *sb)
> > >>>> +{
> > >>>> +    bool rv;
> > >>>> +
> > >>>> +    if (!sb)
> > >>>> +            return false;
> > >>>> +    spin_lock(&pin_lock);
> > >>>> +    rv = !IS_ERR_OR_NULL(pinned_sb) && pinned_sb == sb;
> > >>>> +    spin_unlock(&pin_lock);
> > >>>
> > >>> It's okay for an initial version, but I still think you need to get
> > >>> away from this spinlock in from_pinned() as quickly as possible.
> > >>> Maybe I'm wrong, but this looks like a major source of lock contention.
> > >>>
> > >>> I understand the issue around RCU and the potential for matching on
> > >>> a reused buffer/address, but if you modified IPE to have its own LSM
> > >>> security blob in super_block::security you could mark the superblock
> > >>> when it was mounted and do a lockless lookup here in from_pinned().
> > >>
> > >> Thank you for the suggestion. After some testing, I discovered that
> > >> switching to RCU to pin the super block and using a security blob to
> > >> mark a pinned super block works. This approach do avoid many spinlock
> > >> operations. I'll incorporate these changes in the next version of the patch.
> > >
> > > I probably wasn't as clear as I should have been, I was thinking of
> > > doing away with the @pinned_sb global variable entirely, as well as
> > > its associated lock problems and simply marking the initramfs/initrd
> > > superblock when it was mounted.  I will admit that I haven't fully
> > > thought about all the implementation details, but I think you could
> > > leverage the security_sb_mount() hook to set a flag in IPE's
> > > superblock metadata when the initramfs was mounted.
> >
> > I wasn't able to find a way to let LSM pin initramfs/initrd during mount
> > time ...
>
> I haven't had to look at the kernel init code in a while, and I don't
> recall ever looking at the initramfs code, but I spent some time
> digging through the code and I wonder if it would be possible to mark
> the initramfs superblock in wait_for_initramfs() via a new LSM hook
> using @current->fs->root.mnt->mnt_sb?  Although I'm not completely
> sure that it's populated.  Have you already looked at an approach like
> this?

Thinking about this more, the current IPE approach of treating the
first file access as being present in the initramfs is not correct
(one could build a system without an initramfs).  I think we need to
do something like the above where the initramfs is explicitly marked
in the initramfs code.

> > But I think we could replace the global variable with a flag
> > variable ipe_sb_state so we could use atomic operation to only mark one
> > drive as pinned without any lock. The code will be like:
> >
> > static void pin_sb(const struct super_block *sb)
> > {
> >         if (!sb)
> >                 return;
> >
> >         if (!test_and_set_bit_lock(IPE_SB_PINNED, &ipe_sb_state)) {
> >                 ipe_sb(sb)->pinned = true;
> >         }
> > }
> >
> > Would this sound better?

-- 
paul-moore.com

  reply	other threads:[~2023-11-03 22:30 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-04 22:09 [RFC PATCH v11 00/19] Integrity Policy Enforcement LSM (IPE) Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 01/19] security: add ipe lsm Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 02/19] ipe: add policy parser Fan Wu
2023-10-24  3:52   ` [PATCH RFC v11 2/19] " Paul Moore
2023-10-25 22:45     ` Fan Wu
2023-10-26 21:36       ` Paul Moore
2023-10-04 22:09 ` [RFC PATCH v11 03/19] ipe: add evaluation loop Fan Wu
2023-10-24  3:52   ` [PATCH RFC v11 3/19] " Paul Moore
2023-10-26  0:15     ` Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 04/19] ipe: add LSM hooks on execution and kernel read Fan Wu
2023-10-24  3:52   ` [PATCH RFC v11 4/19] " Paul Moore
2023-10-04 22:09 ` [RFC PATCH v11 05/19] ipe: introduce 'boot_verified' as a trust provider Fan Wu
2023-10-24  3:52   ` [PATCH RFC v11 5/19] " Paul Moore
2023-10-26 21:33     ` Fan Wu
2023-10-26 22:12       ` Paul Moore
2023-11-02 22:46         ` Fan Wu
2023-11-03 22:15           ` Paul Moore
2023-11-03 22:30             ` Paul Moore [this message]
2023-10-04 22:09 ` [RFC PATCH v11 06/19] security: add new securityfs delete function Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 07/19] ipe: add userspace interface Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 08/19] uapi|audit|ipe: add ipe auditing support Fan Wu
2023-10-24  3:52   ` [PATCH RFC v11 8/19] " Paul Moore
2023-11-02 22:55     ` Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 09/19] ipe: add permissive toggle Fan Wu
2023-10-24  3:52   ` [PATCH RFC v11 9/19] " Paul Moore
2023-11-02 22:56     ` Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 10/19] block|security: add LSM blob to block_device Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 11/19] dm verity: set DM_TARGET_SINGLETON feature flag Fan Wu
2023-10-24  3:52   ` [PATCH RFC " Paul Moore
2023-11-02  0:40     ` Paul Moore
2023-10-04 22:09 ` [RFC PATCH v11 12/19] dm: add finalize hook to target_type Fan Wu
2023-10-24  3:52   ` [PATCH RFC " Paul Moore
2023-11-02  0:41     ` Paul Moore
2023-10-04 22:09 ` [RFC PATCH v11 13/19] dm verity: consume root hash digest and signature data via LSM hook Fan Wu
2023-10-24  3:52   ` [PATCH RFC " Paul Moore
2023-11-02  0:41     ` Paul Moore
2023-10-04 22:09 ` [RFC PATCH v11 14/19] ipe: add support for dm-verity as a trust provider Fan Wu
2023-10-24  3:52   ` [PATCH RFC " Paul Moore
2023-11-02 22:40     ` Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 15/19] fsverity: consume builtin signature via LSM hook Fan Wu
2023-10-05  2:27   ` Eric Biggers
2023-10-05  2:49     ` Fan Wu
2023-10-24  3:52   ` [PATCH RFC " Paul Moore
2023-11-02  0:40     ` Paul Moore
2023-11-02  2:53       ` Eric Biggers
2023-11-02 15:42         ` Paul Moore
2023-11-02 19:33           ` Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 16/19] ipe: enable support for fs-verity as a trust provider Fan Wu
2023-10-04 23:58   ` Randy Dunlap
2023-10-05  2:45     ` Fan Wu
2023-10-24  3:52   ` [PATCH RFC " Paul Moore
2023-10-04 22:09 ` [RFC PATCH v11 17/19] scripts: add boot policy generation program Fan Wu
2023-10-24  3:52   ` [PATCH RFC " Paul Moore
2023-11-02 23:09     ` Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 18/19] ipe: kunit test for parser Fan Wu
2023-10-24  3:52   ` [PATCH RFC " Paul Moore
2023-11-02 23:11     ` Fan Wu
2023-10-04 22:09 ` [RFC PATCH v11 19/19] documentation: add ipe documentation Fan Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHC9VhQLbgvg6syOB2_GYFK+zpWbyx6zrDoM1aBnA18u_Qjj4g@mail.gmail.com \
    --to=paul@paul-moore.com \
    --cc=agk@redhat.com \
    --cc=audit@vger.kernel.org \
    --cc=axboe@kernel.dk \
    --cc=corbet@lwn.net \
    --cc=deven.desai@linux.microsoft.com \
    --cc=dm-devel@redhat.com \
    --cc=ebiggers@kernel.org \
    --cc=eparis@redhat.com \
    --cc=jmorris@namei.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=roberto.sassu@huawei.com \
    --cc=serge@hallyn.com \
    --cc=snitzer@kernel.org \
    --cc=tytso@mit.edu \
    --cc=wufan@linux.microsoft.com \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).