public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ilias Apalodimas" <ilias.apalodimas@linaro.org>
To: devel@edk2.groups.io, ardb@kernel.org
Cc: Gerd Hoffmann <kraxel@redhat.com>, "Lu, Ken" <ken.lu@intel.com>,
	"Xu, Min M" <min.m.xu@intel.com>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	"Aktas, Erdem" <erdemaktas@google.com>,
	James Bottomley <jejb@linux.ibm.com>,
	"Yao, Jiewen" <jiewen.yao@intel.com>
Subject: Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
Date: Fri, 23 Sep 2022 12:34:36 +0300	[thread overview]
Message-ID: <Yy19rK3YNv4n9Pen@hades> (raw)
In-Reply-To: <CAMj1kXFLMNZLdd+Rm2EnhxJrnG2kahXgjA=AM2vFm08AkuXt4w@mail.gmail.com>

Hi all, 

Sorry for being late to the party.  Ard cc'ed me in a prior mail, but that
got lost along the way

On Wed, Sep 21, 2022 at 05:41:14PM +0200, Ard Biesheuvel wrote:
> On Wed, 21 Sept 2022 at 14:27, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Wed, Sep 21, 2022 at 11:24:11AM +0000, Lu, Ken wrote:
> > > >
> > > > > But either in GenericQemuLoadImageLib, it can do measurement for
> > > > command line and initrd, correct?
> > > >
> > > > Yes, it could.  But why given that the linux kernel efi stub measures anyway?
> >
> > > If the final decision is the measurement should be done by efi stub in
> > > Linux kernel.
> >
> > The reference should be the workflow when you boot linux from efi shell
> > or using a BootNNNN entry.  Which I think is:
> >
> >   (1) linux kernel is loaded + measured via Loadimage().
> >   (2) linux kernel is started via efi stub entry point.
> >   (3) linux kernel efi stub loads and measures the initrd.
> >
> > Not fully sure about the command line measurement, IIRC Ard described
> > that in one of the replies.
> >
> 
> If the image was booted from a BootNNNN entry, the entire variable
> will be measured into the TPM, including the load options aka command
> line.
> 
> If you use the shell or another loader that has no explicit awareness
> of secure boot or measured boot, the load options are not measured at
> all.
> 

As others mentioned before me, I think the proposal here is reasonable.
Yes the TCG spec tries to avoid measuring things twice,  but the reality is
that if we do so we'll probably end up with binaries or config options not
being measured depending on how the OS booted.  On top of that the efi-stub
measures the contents of the initramfs and the LoadOptions in PCR9,  which
is meant to be used by the OS.  That should give enough freedom to people
when deciding which PCRs to use on sealing etc.

> > > Do we also need remove today's measurement in Grub (I
> > > have submitted some patch for TDX in grub...)?
> >
> > Those patches are perfectly fine, tpm measurement and tdx measurement
> > should be consistent.  In case the grub measurement workflow needs
> > changes to avoid double measurements (not sure this is actually the
> > case) those changes should apply to both tpm and tdx.
> >
> 
> Agreed. I think the decision what to measure and what not to measure
> is orthogonal to the type of measured boot that is being used.

I'd like to take the opportunity here and explain what happens on the
efi-stub measurements.  GRUB uses the EV_IPL type for the eventlog.
This event was marked as deprecated in older versions of the spec.  However
on the latest versions that changed and it now says:

- "May be used by Boot Manager Code to measure events. The contents of the
   event field are specified by the caller."

Since it mentions "boot manager" and the efi-stub is an OS loader,
we used EV_EVENT_TAG for the initrd contents and the LoadOptions.  The
description for that type is:

- "Used for PCRs defined for OS and application usage. Defined for use by
  Host Platform Operating System or Software", which is a closer match.
On top of that the opaque EV_EVENT_TAG event is a bit cleaner and allows
us to mark events using its u32 tagged EventID and it easily extended for
any future measurements we want to add. 

However when I re-read the event description there's a confusing sentence
on it's description saying "The digests field MUST contain the tagged
hash of the event data for each bank.".  This contradicts the 
TCG_PCR_EVENT2 Structure description for the digests field, as it allows
you to measure either the event data or external data.  It also means that
when measuring the initrd we'll end up with an > 80mb event data, which is silly. 

Anyone knows if that's a typo on the spec?  Or as we abusing EV_EVENT_TAG?

Thanks
/Ilias

> 
> > > According to Bottomley, the same measurement should not be done twice.
> >
> > Yes, this is the way it should be, although the current state of affairs
> > is a bit messy and I think we are a bit away from that ideal.
> >
> > > Or only the one who use GenericQemuLoadImageLib, will give the Linux
> > > kernel efi stub for measure?
> >
> > I think we don't have to do anything special in GenericQemuLoadImageLib
> > because the lib uses Loadimage() which should handle measurement.
> >
> 
> 
> 
> 
> 

      reply	other threads:[~2022-09-23  9:34 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-17  2:52 measurement to command-line/initrd for loading kernel via -kernel option Min Xu
2022-09-18 12:52 ` Ard Biesheuvel
2022-09-19  2:13   ` [edk2-devel] " Min Xu
2022-09-19  6:58     ` Ard Biesheuvel
2022-09-20  0:20       ` Min Xu
2022-09-20 12:29         ` Ard Biesheuvel
2022-09-20 12:55       ` Lu, Ken
2022-09-20 13:03         ` Ard Biesheuvel
2022-09-20 13:24           ` Lu, Ken
2022-09-20 13:43             ` James Bottomley
2022-09-20 14:34               ` Ard Biesheuvel
2022-09-20 14:51             ` Ard Biesheuvel
2022-09-20 15:14               ` Lu, Ken
2022-09-20 13:20         ` Gerd Hoffmann
2022-09-20 13:38           ` Lu, Ken
2022-09-20 14:18             ` Gerd Hoffmann
2022-09-20 14:30               ` Lu, Ken
2022-09-21  7:14                 ` Gerd Hoffmann
2022-09-21 11:24                   ` Lu, Ken
2022-09-21 12:27                     ` Gerd Hoffmann
2022-09-21 15:41                       ` Ard Biesheuvel
2022-09-23  9:34                         ` Ilias Apalodimas [this message]

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-list 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=Yy19rK3YNv4n9Pen@hades \
    --to=devel@edk2.groups.io \
    /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