Message-ID: <ZLUIViihakhyPV1N@redhat.com>
Date:   Mon, 17 Jul 2023 10:22:51 +0100
From:   Daniel P. Berrangé <berrange@...hat.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Luca Boccassi <bluca@...ian.org>, Borislav Petkov <bp@...en8.de>,
        Emanuele Giuseppe Esposito <eesposit@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        Thomas Gleixner <tglx@...utronix.de>, lennart@...ttering.net,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Alexander Potapenko <glider@...gle.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v2] x86/boot: add .sbat section to the bzImage

On Sun, Jul 16, 2023 at 08:28:10PM +0200, Greg KH wrote:
> On Sun, Jul 16, 2023 at 06:41:04PM +0100, Luca Boccassi wrote:
> > On Sat, 15 Jul 2023 at 07:52, Greg KH <gregkh@...uxfoundation.org> wrote:
> > >
> > > If you are not willing to take the time to determine how this proposed
> > > change will affect the kernel developers and infrastructure by doing
> > > some modeling based on our past history, then we have no reason to even
> > > consider accepting this change as you are stating that you have no idea
> > > how it will affect us.
> > 
> > There's no need for that to tell you how this will affect you: it will
> > not. Every now and then you'll receive a one-liner patch to apply.
> > What's so terrible about that?

I think that's not entirely accurate, as this *will* have an impact on
anyone involved in backporting fixes for the kernel stable trees, when
they need to resolve conflicts on the SBAT file. It shouldn't have a
big impact, but we should be honest that it will be a non-zero impact.

Lets say mainline branch has had 2 security vulnerabilities A and B,
each of which was associated with an increment of the SBAT version
number. The first flaw A changed SBAT from 7 to 8,and then the second
flaw B changed SBAT from 8 to 9.

If someone wants to backport the fix for bug "B" they will get a
conflict on the SBAT file when cherry-picking the patch. When that
happens they must decide:

  * It is acceptable to ignore issue A, because it didn't affect
    that branch. The conflict is resolved by having the backported
    patch increase SBAT version from 7 to 9 directly.

  * It is required to first backport issue A, because that also
    affects that branch. The conflict is resolved by first backporting
    the code fix & SBAT change for A, and then backporting the code
    fix and SBAT change for B. SBAT changes from 7 to 8 to 9 just
    like on master.

IOW whomever is doing backport patches for stable needs to understand
the semantics of SBAT and how to resolve conflicts on it. If they get
this wrong, then it breaks the protection offered by SBAT, which would
then require a 3rd SBAT change to fix the mistake.

This likely means that stable tree maintainers themselves need to
understand the SBAT change rules, so they can review conflict resolution
for any proposed changes, to sanity check what is being proposed.

> And who will be sending that to me?  For what releases?  For how long
> will they be agreeing to do this work for?  How will it be tracked?
> What will they be using to determine when the number will be changed?
> How will they know it should be changed?

Before we consider SBAT, there is the code bug & its patch.

Someone finds bug in the early boot process on EFI systems and cooks up
a patch for it. This is work that is already done today, whether entirely
upstream in the normal context of day-to-day kernel development, or
downstream with a vendor receiving a bug report and triaging a response
to it which eventually turns into an upstream submission.

Today there is the question of whether to assign a CVE to such fixes.

If upstream doesn't have an associated CVE identified when merging the
code patch, the downstream vendors act as a backstop and can decide to
assign a CVE after the fact. This is relatively easy as assigning a
CVE doesn't result in any code patches, it is just paperwork and does
not really impact upstream at that point.

In terms of triage, deciding to increment SBAT is little different
from deciding to assign a CVE. The analysis & decision can be done
upstream, but if not, downstream vendors can act as a backstop to
do the analysis after a code patch is already in upstream. I would
probably assume that any flaw serious enough to break SecureBoot
is likely going to arrive via an embargoed security report involving
downstream vendors, so SBT changes would be co-ordinated via the
vendor triage & response.

The key difference with SBAT is that if a downstream vendor identifies
needs for an SBAT version change, after a  patch is already merged in
upstream, this would typically trigger a 2nd followup patch to be sent
upstream from the vendor.

In terms of who will be sending SBAT changes. It could be the person
who first writes & submits the patch that addresses a EFI boot process
vulnerability. If not, it would likely be one of the various downstream
vendors, with their security/kernel team sending a change after the fact.


In terms of what branches would be impacted. The minimum bar would be
to only make SBAT changes in master. If that is all that is ever done
upstream, the mechanism would work as intended. There will inevitably
be backports to stable trees though, and people involved in this will
need to understand the rules for resolving conflicts when backporting
fixes that change SBAT as mentioned earlier.

I think the assumption would have to be that any stable branch is a
possible candidate for receiving backports that imply SBAT changes,
just like any stable branch might receive a backport for a regular
CVE fix. Whomever is interested in submitting changes to a particular
branch decides which particular patches get backported. The precense
of SBAT does constrain ordering of backports though, in the (hopefull)
unlikely case where multiple SBAT changes arrive close together

> None of this has been answered, and that's the real issue here.  This
> "magic number" is saying it is going to reflect some specific "security
> issue" yet no one is saying how those issues are going to be determined,
> or anything else about it.

Luca gave a (non-exhaustive) list of examples of areas of the code
which are most relevant earlier in the thread

[quote]
Kernel module signature enforcement, Lockdown LSM, ensuring
ExitBootServices is called at the right time, etc.
[/quote]

IIUC, your request here could potentially be satisfied if there was
first a patch that added a file 'Documentation/security/sbat.rst'
explaining in much more detail which set of kernel features are
relevant when considering SBAT, and how to evaluate bugs to decide
whether a SBAT change is justified or not.

That docs file also needs to explain the implications and criteria
for backporting patches to stable branches, and how to deal with SBAT
if upstream didn't make a change to it and it needs changing after
the fact.

> Again, as I've repeated numerous times, tell us how often this number
> would have changed in the past X years to give us an idea of how you
> will be changing it going forward.  To not provide any of this means
> this patch adding this magic number means nothing as no one knows what
> it actually means.

The SBAT concept was introduced after discussions with Microsoft
after the Grub BootHole[1] vulnerability was identified in 2021.

I'm not aware of any kernel CVEs since that point in time that
would have implied SBAT changes, but admittedly I've not paid
close enough attention to be entirely confident. Is going back
through 2 years of kernel CVEs (to the point where SBAT was
invented) a long enough timeframe to satisfy this request for
info on the frequency of changes ?

With regards,
Daniel

[1] https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00007.html
    https://eclypsium.com/research/theres-a-hole-in-the-boot/
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Powered by blists - more mailing lists