Re: [RFC PATCH 0/7] efi/libstub: measurement initrd data loaded by the EFI stub

[Ard Biesheuvel <ardb@kernel.org>]

On Tue, 3 Nov 2020 at 06:52, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Ard,
>
> On Mon, Nov 02, 2020 at 06:06:27PM +0100, Ard Biesheuvel wrote:
> > This series enables measurement of the initrd data loaded directly by the
> > EFI stub into the TPM, using the TCG2 protocol exposed by the firmware (if
> > available). This ensures that the initrd observed and used by the OS is the
> > same one that got measured into the TPM, which is more difficult to guarantee
> > in the current situation.
> >
>
> I like this. The OS gets the ability to 'self-measure' one critical component.
> This can of course be done in the bootloader or GRUB, but having the functionality
> in the stub will allow you to boot with a verified initrd, if even GRUB isn't
> there or the bootloader doesn't measure the initrd.
>

Well, that is one aspect of it. But the most important issue is that
the LoadFile2 protocol or the initrd= method are ambiguous: any initrd
loaded by the bootloader and passed via bootparams or DT might be
superseded by the EFI stub's initrd loader, and even if the command
line is measured as well, 'initrd=foo.bin' does not give any
guarantees about the contents of that file.

So measuring what actually gets loaded by the stub and passed to the
kernel is the most robust way to deal with this, and measuring 'no
stub-loaded initrd' if none is provided is important there.

But that does raise a question regarding double measurement:
unconditionally taking measurements might break existing boot flows
when upgrading the kernel. The alternative is to measure only if an
initrd was provided, but that is fragile as well.

So as Matthew suggested, it would be good to have a more high level
talk about the policies here, and about what kind of complications we
are prepared to put up with to make changes like this one possible.


> > This is posted as an RFC since it is mostly an invitation to discuss how
> > we can fit this into a longer term strategy for arch-agnostic secure and
> > measured boot that does not hinge on the Shim+GRUB tandem, or on deep
> > knowledge on the part of the bootloader regarding device trees, bootparams
> > structs, allocation and placement policies of various artifacts etc etc
> >
> > Open questions:
> > - Should we do this?
>
> I think so. I can't find any arguments why we shouldn't.
>
> > - Are Linux systems in the field using PCR value prediction when updating the
> >   initrd? Does this approach interfere with that?
> > - Which PCR and event type to use
>
> No idea. I think distros will have an opinion on that
>

PCR #9 with EV_IPL seems like the de facto standard, so I will stick with that.


> > - Is a separator event needed here, given that the initrd measurement is
> >   recorded even if no initrd was loaded by the stub?
>
> I think having the event make sense, but if we going to make a standard
> measurement for the initrd, we need to discuss this a bit more.
>

The question is about separator events: in the above case, for
instance, not extending the PCR if no initrd measurement is taken
could open the door to hacks where a rogue initrd is passed via
bootparams while the system is configured to use LoadFile2, and in
some way (EBS hook?), the firmware manages to extend the 'desired'
value into the PCR instead of the hash of the actual initrd that is
loaded. If we measure a separator event unconditionally after
measuring the initrd (or not), this cannot happen.

This might sound a bit convoluted, but real-world hacks typically are.

> >
> > Note that the EFI stub ignores the initrd provided directly via bootparams or
> > the device tree, and it would be nice if we could keep doing that.
> >
> > Build tested only.
>
>
> Cheers
> /Ilias
>
> >
> > Cc: Peter Jones <pjones@redhat.com>
> > Cc: Leif Lindholm <leif@nuviainc.com>
> > Cc: Arvind Sankar <nivedita@alum.mit.edu>
> > Cc: Matthew Garrett <mjg59@google.com>
> > Cc: Daniel Kiper <daniel.kiper@oracle.com>
> > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> >
> > Ard Biesheuvel (7):
> >   efi/libstub: whitespace cleanup
> >   efi/libstub: fix prototype of efi_tcg2_protocol::get_event_log()
> >   efi/libstub: x86/mixed: increase supported argument count
> >   efi/libstub: move TPM related prototypes into efistub.h
> >   efi/libstub: add prototype of
> >     efi_tcg2_protocol::hash_log_extend_event()
> >   efi/libstub: consolidate initrd handling across architectures
> >   efi/libstub: measure loaded initrd info into the TPM
> >
> >  arch/x86/boot/compressed/efi_thunk_64.S       | 17 ++++--
> >  arch/x86/include/asm/efi.h                    | 13 +++--
> >  arch/x86/platform/efi/efi_thunk_64.S          | 17 ++++--
> >  .../firmware/efi/libstub/efi-stub-helper.c    | 56 +++++++++++++++----
> >  drivers/firmware/efi/libstub/efi-stub.c       | 10 +---
> >  drivers/firmware/efi/libstub/efistub.h        | 34 ++++++++++-
> >  drivers/firmware/efi/libstub/x86-stub.c       | 26 ++++-----
> >  include/linux/efi.h                           | 13 +----
> >  8 files changed, 123 insertions(+), 63 deletions(-)
> >
> > --
> > 2.17.1
> >