public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* measurement to command-line/initrd for loading kernel via -kernel option
@ 2022-09-17  2:52 Min Xu
  2022-09-18 12:52 ` Ard Biesheuvel
  0 siblings, 1 reply; 22+ messages in thread
From: Min Xu @ 2022-09-17  2:52 UTC (permalink / raw)
  To: Ard Biesheuvel, devel@edk2.groups.io
  Cc: Aktas, Erdem, James Bottomley, Yao, Jiewen, Xu, Min M,
	Gerd Hoffmann

[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]

Hi, Ard
I am checking the measurement behavior when loading the kernel via the QEMU -kernel option. I find it is implemented by below 2 driver/lib:
- OvmfPkg/QemuKernelLoaderFsDxe
 This is a separate DXE driver that exposes the virtual SimpleFileSystem implementation that carries the kernel and initrd passed via the QEMU command line.
- OvmfPkg/Library/X86QemuLoadImageLib
  This is the library that consumes above driver and call LoadImage/StartImage so that the kernel image gets authenticated and/or measured.
See https://edk2.groups.io/g/devel/message/55381

I have some questions about the implementation need your help.
1. In the QemuKernelLoaderFsDxe, AllocatePool is called to allocate memory. Why not call AllocatePages? Kernel image size may be around 15 MB, but initrd size maybe much bigger.
2. Kernel image is authenticated and/or measured in LoadImage. I am wondering if "command line" is measured as well? "Command line" can be treated as an external input and in my opinion it should be measured too.
3. The same question to initrd. Is it measured?

Thanks
Min

[-- Attachment #2: Type: text/html, Size: 8163 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: measurement to command-line/initrd for loading kernel via -kernel option
  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
  0 siblings, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2022-09-18 12:52 UTC (permalink / raw)
  To: Xu, Min M
  Cc: Ard Biesheuvel, devel@edk2.groups.io, Aktas, Erdem,
	James Bottomley, Yao, Jiewen, Gerd Hoffmann

Hello Min Xu,

On Sat, 17 Sept 2022 at 04:53, Xu, Min M <min.m.xu@intel.com> wrote:
>
> Hi, Ard
>
> I am checking the measurement behavior when loading the kernel via the QEMU -kernel option. I find it is implemented by below 2 driver/lib:
>
> - OvmfPkg/QemuKernelLoaderFsDxe
>
>  This is a separate DXE driver that exposes the virtual SimpleFileSystem implementation that carries the kernel and initrd passed via the QEMU command line.
>
> - OvmfPkg/Library/X86QemuLoadImageLib
>
>   This is the library that consumes above driver and call LoadImage/StartImage so that the kernel image gets authenticated and/or measured.
>
> See https://edk2.groups.io/g/devel/message/55381
>
>
>
> I have some questions about the implementation need your help.
>
> 1. In the QemuKernelLoaderFsDxe, AllocatePool is called to allocate memory. Why not call AllocatePages? Kernel image size may be around 15 MB, but initrd size maybe much bigger.
>

We use the same code for the command line, which may be much smaller
than a page. On some architectures (AARCH64), page allocations may be
rounded up to 64k multiples.

Note that AllocatePool() will automatically fall back to
AllocatePages() if the allocation is sufficiently large.

> 2. Kernel image is authenticated and/or measured in LoadImage. I am wondering if “command line” is measured as well? “Command line” can be treated as an external input and in my opinion it should be measured too.
>
> 3. The same question to initrd. Is it measured?
>

The initrd is measured by the EFI stub in Linux, and we are currently
adding measurement of the load options to that as well:
https://lore.kernel.org/all/20220916081441.1993492-2-ilias.apalodimas@linaro.org/

The initrd is Linux specific in any case, so there, the Linux OS
loader is a natural place to take care of this. The load options are
being added because of the oversight in the TCG spec, which only
covers load options if they are part of a Boot#### option, but between
LoadImage() and StartImage, you can pass any load options you want via
the loaded image protocol, so it needs to be measured as well.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-18 12:52 ` Ard Biesheuvel
@ 2022-09-19  2:13   ` Min Xu
  2022-09-19  6:58     ` Ard Biesheuvel
  0 siblings, 1 reply; 22+ messages in thread
From: Min Xu @ 2022-09-19  2:13 UTC (permalink / raw)
  To: devel@edk2.groups.io, ardb@kernel.org
  Cc: Ard Biesheuvel, Aktas, Erdem, James Bottomley, Yao, Jiewen,
	Gerd Hoffmann, Lu, Ken, Xu, Min M

On September 18, 2022 8:52 PM, Ard Biesheuvel wrote:
> Hello Min Xu,
> 
> On Sat, 17 Sept 2022 at 04:53, Xu, Min M <min.m.xu@intel.com> wrote:
> >
> > Hi, Ard
> >
> > I am checking the measurement behavior when loading the kernel via the
> QEMU -kernel option. I find it is implemented by below 2 driver/lib:
> >
> > - OvmfPkg/QemuKernelLoaderFsDxe
> >
> >  This is a separate DXE driver that exposes the virtual SimpleFileSystem
> implementation that carries the kernel and initrd passed via the QEMU
> command line.
> >
> > - OvmfPkg/Library/X86QemuLoadImageLib
> >
> >   This is the library that consumes above driver and call
> LoadImage/StartImage so that the kernel image gets authenticated and/or
> measured.
> >
> > See https://edk2.groups.io/g/devel/message/55381
> >
> >
> >
> > I have some questions about the implementation need your help.
> >
> > 2. Kernel image is authenticated and/or measured in LoadImage. I am
> wondering if “command line” is measured as well? “Command line” can be
> treated as an external input and in my opinion it should be measured too.
> >
> > 3. The same question to initrd. Is it measured?
> >
> 
> The initrd is measured by the EFI stub in Linux, and we are currently adding
> measurement of the load options to that as well:
> https://lore.kernel.org/all/20220916081441.1993492-2-
> ilias.apalodimas@linaro.org/
> 
> The initrd is Linux specific in any case, so there, the Linux OS loader is a
> natural place to take care of this. The load options are being added because
> of the oversight in the TCG spec, which only covers load options if they are
> part of a Boot#### option, but between
> LoadImage() and StartImage, you can pass any load options you want via the
> loaded image protocol, so it needs to be measured as well.
> 
Thanks Ard for the explanation. 
I was told that in grub boot cmd-line/initrd will be measured as well. So my question is that will they be measured twice? One in grub.efi, the other in efi-stub?

My understanding is that the loader should take the responsibility to do the measurement. 
For grub boot, grub.efi is the loader so it measures kernel-image/cmd-line/initrd. 
For direct boot, TryRunningQemuKernel() now measures kernel image (in CoreLoadImage). Shall it also measure cmd-line/initrd in the same time?

Thanks
Min

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  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:55       ` Lu, Ken
  0 siblings, 2 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2022-09-19  6:58 UTC (permalink / raw)
  To: Xu, Min M
  Cc: devel@edk2.groups.io, Ard Biesheuvel, Aktas, Erdem,
	James Bottomley, Yao, Jiewen, Gerd Hoffmann, Lu, Ken

On Mon, 19 Sept 2022 at 04:13, Xu, Min M <min.m.xu@intel.com> wrote:
>
> On September 18, 2022 8:52 PM, Ard Biesheuvel wrote:
> > Hello Min Xu,
> >
> > On Sat, 17 Sept 2022 at 04:53, Xu, Min M <min.m.xu@intel.com> wrote:
> > >
> > > Hi, Ard
> > >
> > > I am checking the measurement behavior when loading the kernel via the
> > QEMU -kernel option. I find it is implemented by below 2 driver/lib:
> > >
> > > - OvmfPkg/QemuKernelLoaderFsDxe
> > >
> > >  This is a separate DXE driver that exposes the virtual SimpleFileSystem
> > implementation that carries the kernel and initrd passed via the QEMU
> > command line.
> > >
> > > - OvmfPkg/Library/X86QemuLoadImageLib
> > >
> > >   This is the library that consumes above driver and call
> > LoadImage/StartImage so that the kernel image gets authenticated and/or
> > measured.
> > >
> > > See https://edk2.groups.io/g/devel/message/55381
> > >
> > >
> > >
> > > I have some questions about the implementation need your help.
> > >
> > > 2. Kernel image is authenticated and/or measured in LoadImage. I am
> > wondering if “command line” is measured as well? “Command line” can be
> > treated as an external input and in my opinion it should be measured too.
> > >
> > > 3. The same question to initrd. Is it measured?
> > >
> >
> > The initrd is measured by the EFI stub in Linux, and we are currently adding
> > measurement of the load options to that as well:
> > https://lore.kernel.org/all/20220916081441.1993492-2-
> > ilias.apalodimas@linaro.org/
> >
> > The initrd is Linux specific in any case, so there, the Linux OS loader is a
> > natural place to take care of this. The load options are being added because
> > of the oversight in the TCG spec, which only covers load options if they are
> > part of a Boot#### option, but between
> > LoadImage() and StartImage, you can pass any load options you want via the
> > loaded image protocol, so it needs to be measured as well.
> >
> Thanks Ard for the explanation.
> I was told that in grub boot cmd-line/initrd will be measured as well. So my question is that will they be measured twice? One in grub.efi, the other in efi-stub?
>

The EFI stub may be the only OS loader, so the EFI stub should measure
the command line and the initrd.

Whether or not a previous loader stage exists that may or may not
measure the same pieces is not for the EFI stub to reason about. And
in any case, measuring the same thing twice is much less of an issue
than not measuring it at all.

> My understanding is that the loader should take the responsibility to do the measurement.
> For grub boot, grub.efi is the loader so it measures kernel-image/cmd-line/initrd.

If the EFI stub is invoked, the EFI stub is the OS loader. We should
not be relying on the presence of absence of GRUB (or shim) in the
boot chain.

> For direct boot, TryRunningQemuKernel() now measures kernel image (in CoreLoadImage). Shall it also measure cmd-line/initrd in the same time?
>

No, I don't think it should. This is why we are adding this to the EFI
stub instead.

If we measure the initrd and command line in the EFI stub, we don't
have to measure it anywhere else, and we can use any generic EFI
loader on a measured boot system.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  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
  1 sibling, 1 reply; 22+ messages in thread
From: Min Xu @ 2022-09-20  0:20 UTC (permalink / raw)
  To: devel@edk2.groups.io, ardb@kernel.org
  Cc: Ard Biesheuvel, Aktas, Erdem, James Bottomley, Yao, Jiewen,
	Gerd Hoffmann, Lu, Ken, Xu, Min M

On September 19, 2022 2:59 PM, Ard Biesheuvel wrote:
> On Mon, 19 Sept 2022 at 04:13, Xu, Min M <min.m.xu@intel.com> wrote:
> >
> > On September 18, 2022 8:52 PM, Ard Biesheuvel wrote:
> > > Hello Min Xu,
> > >
> > > On Sat, 17 Sept 2022 at 04:53, Xu, Min M <min.m.xu@intel.com> wrote:
> > > >
> > > > Hi, Ard
> > > >
> > > > I am checking the measurement behavior when loading the kernel via
> > > > the
> > > QEMU -kernel option. I find it is implemented by below 2 driver/lib:
> > > >
> > > > - OvmfPkg/QemuKernelLoaderFsDxe
> > > >
> > > >  This is a separate DXE driver that exposes the virtual
> > > > SimpleFileSystem
> > > implementation that carries the kernel and initrd passed via the
> > > QEMU command line.
> > > >
> > > > - OvmfPkg/Library/X86QemuLoadImageLib
> > > >
> > > >   This is the library that consumes above driver and call
> > > LoadImage/StartImage so that the kernel image gets authenticated
> > > and/or measured.
> > > >
> > > > See https://edk2.groups.io/g/devel/message/55381
> > > >
> > > >
> > > >
> > > > I have some questions about the implementation need your help.
> > > >
> > > > 2. Kernel image is authenticated and/or measured in LoadImage. I
> > > > am
> > > wondering if “command line” is measured as well? “Command line” can
> > > be treated as an external input and in my opinion it should be measured
> too.
> > > >
> > > > 3. The same question to initrd. Is it measured?
> > > >
> > >
> > > The initrd is measured by the EFI stub in Linux, and we are
> > > currently adding measurement of the load options to that as well:
> > > https://lore.kernel.org/all/20220916081441.1993492-2-
> > > ilias.apalodimas@linaro.org/
> > >
> > > The initrd is Linux specific in any case, so there, the Linux OS
> > > loader is a natural place to take care of this. The load options are
> > > being added because of the oversight in the TCG spec, which only
> > > covers load options if they are part of a Boot#### option, but
> > > between
> > > LoadImage() and StartImage, you can pass any load options you want
> > > via the loaded image protocol, so it needs to be measured as well.
> > >
> > Thanks Ard for the explanation.
> > I was told that in grub boot cmd-line/initrd will be measured as well. So my
> question is that will they be measured twice? One in grub.efi, the other in efi-
> stub?
> >
> 
> The EFI stub may be the only OS loader, so the EFI stub should measure the
> command line and the initrd.
> 
> Whether or not a previous loader stage exists that may or may not measure
> the same pieces is not for the EFI stub to reason about. And in any case,
> measuring the same thing twice is much less of an issue than not measuring it
> at all.
> 
> > My understanding is that the loader should take the responsibility to do the
> measurement.
> > For grub boot, grub.efi is the loader so it measures kernel-image/cmd-
> line/initrd.
> 
> If the EFI stub is invoked, the EFI stub is the OS loader. We should not be
> relying on the presence of absence of GRUB (or shim) in the boot chain.
> 
> > For direct boot, TryRunningQemuKernel() now measures kernel image (in
> CoreLoadImage). Shall it also measure cmd-line/initrd in the same time?
> >
> 
> No, I don't think it should. This is why we are adding this to the EFI stub
> instead.
> 
> If we measure the initrd and command line in the EFI stub, we don't have to
> measure it anywhere else, and we can use any generic EFI loader on a
> measured boot system.
> 
Ard, thanks for the explanation.
The reason why I am checking the measurement of cmd-line and initrd is that I am thinking how to measure cmd-line and initrd in TDVF with the EFI_CC_MEASUREMENT_PROTOCOL which is defined in MdePkg/Include/Protocol/CcMeasurement.h. Actually EFI_CC_MEASUREMENT_PROTOCOL is designed for the Confidential Computing guest to support measurement. 

I check the patch in https://lore.kernel.org/all/20220916081441.1993492-1-ilias.apalodimas@linaro.org/. It supports the EFI_TCG2_PROTOCOL. I think it can also support EFI_CC_MEASUREMENT_PROTOCOL like below:
  efi_tcg2_protocol_t *tcg2 = NULL;
  efi_cc_protocol_t *cc = NULL;
  efi_bs_call(locate_protocol, &tcg2_guild, NULL, (void**)&tcg2);
  efi_bs_call(locate_protocol, &cc_guild, NULL, (void**)&cc);
  if (tcg2) {
      do_tcg2_measurement ();
  } else if (cc) {
      cc-> MapPcrToMrIndex ();  // because there is a map between TPM-PCR index to CC-MeasureRegister Index.
      do_cc_measurement ();
  }

I am wondering when your 2 patches would be accepted and merged so that I can submit my patch based on yours.

Thanks
Min

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-20  0:20       ` Min Xu
@ 2022-09-20 12:29         ` Ard Biesheuvel
  0 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2022-09-20 12:29 UTC (permalink / raw)
  To: Xu, Min M, Matthew Garrett, Peter Jones, Peter Gonda
  Cc: devel@edk2.groups.io, Aktas, Erdem, James Bottomley, Yao, Jiewen,
	Gerd Hoffmann, Lu, Ken, Ilias Apalodimas, Daniel Kiper,
	Daniel P. Smith, linux-efi

(cc Ilias, Matt, Peter[], Daniel[])

On Tue, 20 Sept 2022 at 02:20, Xu, Min M <min.m.xu@intel.com> wrote:
>
> On September 19, 2022 2:59 PM, Ard Biesheuvel wrote:
> > On Mon, 19 Sept 2022 at 04:13, Xu, Min M <min.m.xu@intel.com> wrote:
> > >
> > > On September 18, 2022 8:52 PM, Ard Biesheuvel wrote:
> > > > Hello Min Xu,
> > > >
> > > > On Sat, 17 Sept 2022 at 04:53, Xu, Min M <min.m.xu@intel.com> wrote:
> > > > >
> > > > > Hi, Ard
> > > > >
> > > > > I am checking the measurement behavior when loading the kernel via
> > > > > the
> > > > QEMU -kernel option. I find it is implemented by below 2 driver/lib:
> > > > >
> > > > > - OvmfPkg/QemuKernelLoaderFsDxe
> > > > >
> > > > >  This is a separate DXE driver that exposes the virtual
> > > > > SimpleFileSystem
> > > > implementation that carries the kernel and initrd passed via the
> > > > QEMU command line.
> > > > >
> > > > > - OvmfPkg/Library/X86QemuLoadImageLib
> > > > >
> > > > >   This is the library that consumes above driver and call
> > > > LoadImage/StartImage so that the kernel image gets authenticated
> > > > and/or measured.
> > > > >
> > > > > See https://edk2.groups.io/g/devel/message/55381
> > > > >
> > > > >
> > > > >
> > > > > I have some questions about the implementation need your help.
> > > > >
> > > > > 2. Kernel image is authenticated and/or measured in LoadImage. I
> > > > > am
> > > > wondering if “command line” is measured as well? “Command line” can
> > > > be treated as an external input and in my opinion it should be measured
> > too.
> > > > >
> > > > > 3. The same question to initrd. Is it measured?
> > > > >
> > > >
> > > > The initrd is measured by the EFI stub in Linux, and we are
> > > > currently adding measurement of the load options to that as well:
> > > > https://lore.kernel.org/all/20220916081441.1993492-2-
> > > > ilias.apalodimas@linaro.org/
> > > >
> > > > The initrd is Linux specific in any case, so there, the Linux OS
> > > > loader is a natural place to take care of this. The load options are
> > > > being added because of the oversight in the TCG spec, which only
> > > > covers load options if they are part of a Boot#### option, but
> > > > between
> > > > LoadImage() and StartImage, you can pass any load options you want
> > > > via the loaded image protocol, so it needs to be measured as well.
> > > >
> > > Thanks Ard for the explanation.
> > > I was told that in grub boot cmd-line/initrd will be measured as well. So my
> > question is that will they be measured twice? One in grub.efi, the other in efi-
> > stub?
> > >
> >
> > The EFI stub may be the only OS loader, so the EFI stub should measure the
> > command line and the initrd.
> >
> > Whether or not a previous loader stage exists that may or may not measure
> > the same pieces is not for the EFI stub to reason about. And in any case,
> > measuring the same thing twice is much less of an issue than not measuring it
> > at all.
> >
> > > My understanding is that the loader should take the responsibility to do the
> > measurement.
> > > For grub boot, grub.efi is the loader so it measures kernel-image/cmd-
> > line/initrd.
> >
> > If the EFI stub is invoked, the EFI stub is the OS loader. We should not be
> > relying on the presence of absence of GRUB (or shim) in the boot chain.
> >
> > > For direct boot, TryRunningQemuKernel() now measures kernel image (in
> > CoreLoadImage). Shall it also measure cmd-line/initrd in the same time?
> > >
> >
> > No, I don't think it should. This is why we are adding this to the EFI stub
> > instead.
> >
> > If we measure the initrd and command line in the EFI stub, we don't have to
> > measure it anywhere else, and we can use any generic EFI loader on a
> > measured boot system.
> >
> Ard, thanks for the explanation.
> The reason why I am checking the measurement of cmd-line and initrd is that I am thinking how to measure cmd-line and initrd in TDVF with the EFI_CC_MEASUREMENT_PROTOCOL which is defined in MdePkg/Include/Protocol/CcMeasurement.h. Actually EFI_CC_MEASUREMENT_PROTOCOL is designed for the Confidential Computing guest to support measurement.
>
> I check the patch in https://lore.kernel.org/all/20220916081441.1993492-1-ilias.apalodimas@linaro.org/. It supports the EFI_TCG2_PROTOCOL. I think it can also support EFI_CC_MEASUREMENT_PROTOCOL like below:
>   efi_tcg2_protocol_t *tcg2 = NULL;
>   efi_cc_protocol_t *cc = NULL;
>   efi_bs_call(locate_protocol, &tcg2_guild, NULL, (void**)&tcg2);
>   efi_bs_call(locate_protocol, &cc_guild, NULL, (void**)&cc);
>   if (tcg2) {
>       do_tcg2_measurement ();
>   } else if (cc) {
>       cc-> MapPcrToMrIndex ();  // because there is a map between TPM-PCR index to CC-MeasureRegister Index.
>       do_cc_measurement ();
>   }
>

Thanks for the example, this is helpful.

I am anticipating this if () to grow more branches pretty soon, so it
is high time we have a discussion about how to abstract this properly.

We will need to measure various assets for:
- TCG measured boot
- TDX measure boot
- SEV/SNP measured boot?
- DICE attestation
- ARM RME
- Trenchboot DRTM

The latter is the odd one out here, but it will also need to capture
system state at different times throughout the execution of the EFI
stub, so it makes sense to mention it here.

Ilias's patches add the following function to the Linux EFI stub

struct efi_measured_event {
   efi_tcg2_event_t event_data;
   efi_tcg2_tagged_event_t tagged_event;
   u8 tagged_event_data[];
};

void efi_measure_tagged_event(unsigned long load_addr, unsigned long load_size,
         const struct efi_measured_event *event);

which is geared towards the TCG2 types. I imagine this will need to
evolve into something more generic over time, making the use of the
TCG2 types specific to that particular measurement path.

As the Linux/EFI and OVMF maintainer, I am somewhat involved in all of
these efforts, but I am by no means an expert on any of them. So this
is really a call for collaboration to ensure that we converge on
something that is usable for everyone and not a constant source of
churn.

> I am wondering when your 2 patches would be accepted and merged so that I can submit my patch based on yours.
>

I just sent a v2 of Ilias's patches with some preliminary changes to
make these abstractions easier in the future. If there are no
objections, I will queue those patches tomorrow, so they should be in
linux-next by friday

https://lore.kernel.org/r/20220920122746.3553306-1-ardb@kernel.org

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-19  6:58     ` Ard Biesheuvel
  2022-09-20  0:20       ` Min Xu
@ 2022-09-20 12:55       ` Lu, Ken
  2022-09-20 13:03         ` Ard Biesheuvel
  2022-09-20 13:20         ` Gerd Hoffmann
  1 sibling, 2 replies; 22+ messages in thread
From: Lu, Ken @ 2022-09-20 12:55 UTC (permalink / raw)
  To: Ard Biesheuvel, Xu, Min M, Daniel Kiper
  Cc: devel@edk2.groups.io, Ard Biesheuvel, Aktas, Erdem,
	James Bottomley, Yao, Jiewen, Gerd Hoffmann

> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Monday, September 19, 2022 2:59 PM
> To: Xu, Min M <min.m.xu@intel.com>
> Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Aktas,
> Erdem <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Lu,
> Ken <ken.lu@intel.com>
> Subject: Re: [edk2-devel] measurement to command-line/initrd for loading
> kernel via -kernel option
> 
> On Mon, 19 Sept 2022 at 04:13, Xu, Min M <min.m.xu@intel.com> wrote:
> >
> > On September 18, 2022 8:52 PM, Ard Biesheuvel wrote:
> > > Hello Min Xu,
> > >
> > > On Sat, 17 Sept 2022 at 04:53, Xu, Min M <min.m.xu@intel.com> wrote:
> > > >
> > > > Hi, Ard
> > > >
> > > > I am checking the measurement behavior when loading the kernel via
> > > > the
> > > QEMU -kernel option. I find it is implemented by below 2 driver/lib:
> > > >
> > > > - OvmfPkg/QemuKernelLoaderFsDxe
> > > >
> > > >  This is a separate DXE driver that exposes the virtual
> > > > SimpleFileSystem
> > > implementation that carries the kernel and initrd passed via the
> > > QEMU command line.
> > > >
> > > > - OvmfPkg/Library/X86QemuLoadImageLib
> > > >
> > > >   This is the library that consumes above driver and call
> > > LoadImage/StartImage so that the kernel image gets authenticated
> > > and/or measured.
> > > >
> > > > See https://edk2.groups.io/g/devel/message/55381
> > > >
> > > >
> > > >
> > > > I have some questions about the implementation need your help.
> > > >
> > > > 2. Kernel image is authenticated and/or measured in LoadImage. I
> > > > am
> > > wondering if “command line” is measured as well? “Command line” can
> > > be treated as an external input and in my opinion it should be measured too.
> > > >
> > > > 3. The same question to initrd. Is it measured?
> > > >
> > >
> > > The initrd is measured by the EFI stub in Linux, and we are
> > > currently adding measurement of the load options to that as well:
> > > https://lore.kernel.org/all/20220916081441.1993492-2-
> > > ilias.apalodimas@linaro.org/
> > >
> > > The initrd is Linux specific in any case, so there, the Linux OS
> > > loader is a natural place to take care of this. The load options are
> > > being added because of the oversight in the TCG spec, which only
> > > covers load options if they are part of a Boot#### option, but
> > > between
> > > LoadImage() and StartImage, you can pass any load options you want
> > > via the loaded image protocol, so it needs to be measured as well.
> > >
> > Thanks Ard for the explanation.
> > I was told that in grub boot cmd-line/initrd will be measured as well. So my
> question is that will they be measured twice? One in grub.efi, the other in efi-
> stub?
> >
> 
> The EFI stub may be the only OS loader, so the EFI stub should measure the
> command line and the initrd.
> 
> Whether or not a previous loader stage exists that may or may not measure the
> same pieces is not for the EFI stub to reason about. And in any case, measuring
> the same thing twice is much less of an issue than not measuring it at all.
> 
> > My understanding is that the loader should take the responsibility to do the
> measurement.
> > For grub boot, grub.efi is the loader so it measures kernel-image/cmd-
> line/initrd.
> 
> If the EFI stub is invoked, the EFI stub is the OS loader. We should not be relying
> on the presence of absence of GRUB (or shim) in the boot chain.
> 
> > For direct boot, TryRunningQemuKernel() now measures kernel image (in
> CoreLoadImage). Shall it also measure cmd-line/initrd in the same time?
> >
> 
> No, I don't think it should. This is why we are adding this to the EFI stub instead.
> 
> If we measure the initrd and command line in the EFI stub, we don't have to
> measure it anywhere else, and we can use any generic EFI loader on a measured
> boot system.

Hi Ard, I think it better let creator to measure instead of consumer to measure like today's implementation in grub[1]. The creator here means who load/create it. In direct boot, it is OVMF read kernel command line and initrd image. In grub boot, it is grub2.  Because the number of consumer like Linux kernel could be more than 1, but the creator is single. 
In another side, "EFI stub" is bind to EFI boot protocol and "EFI handover protocol" is deprecated in grub 2.06[2]. (CC to Daniel).

[1] - https://github.com/rhboot/grub2/blob/f506e21f1425bd320c501507ea0ce9952ee31c2d/grub-core/lib/cmdline.c#L130
[2] - https://www.kernel.org/doc/html/latest/x86/boot.html#efi-handover-protocol-deprecated

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  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:20         ` Gerd Hoffmann
  1 sibling, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2022-09-20 13:03 UTC (permalink / raw)
  To: Lu, Ken
  Cc: Xu, Min M, Daniel Kiper, devel@edk2.groups.io, Ard Biesheuvel,
	Aktas, Erdem, James Bottomley, Yao, Jiewen, Gerd Hoffmann

On Tue, 20 Sept 2022 at 14:56, Lu, Ken <ken.lu@intel.com> wrote:
>
> > -----Original Message-----
> > From: Ard Biesheuvel <ardb@kernel.org>
> > Sent: Monday, September 19, 2022 2:59 PM
> > To: Xu, Min M <min.m.xu@intel.com>
> > Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Aktas,
> > Erdem <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>;
> > Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Lu,
> > Ken <ken.lu@intel.com>
> > Subject: Re: [edk2-devel] measurement to command-line/initrd for loading
> > kernel via -kernel option
> >
> > On Mon, 19 Sept 2022 at 04:13, Xu, Min M <min.m.xu@intel.com> wrote:
> > >
> > > On September 18, 2022 8:52 PM, Ard Biesheuvel wrote:
> > > > Hello Min Xu,
> > > >
> > > > On Sat, 17 Sept 2022 at 04:53, Xu, Min M <min.m.xu@intel.com> wrote:
> > > > >
> > > > > Hi, Ard
> > > > >
> > > > > I am checking the measurement behavior when loading the kernel via
> > > > > the
> > > > QEMU -kernel option. I find it is implemented by below 2 driver/lib:
> > > > >
> > > > > - OvmfPkg/QemuKernelLoaderFsDxe
> > > > >
> > > > >  This is a separate DXE driver that exposes the virtual
> > > > > SimpleFileSystem
> > > > implementation that carries the kernel and initrd passed via the
> > > > QEMU command line.
> > > > >
> > > > > - OvmfPkg/Library/X86QemuLoadImageLib
> > > > >
> > > > >   This is the library that consumes above driver and call
> > > > LoadImage/StartImage so that the kernel image gets authenticated
> > > > and/or measured.
> > > > >
> > > > > See https://edk2.groups.io/g/devel/message/55381
> > > > >
> > > > >
> > > > >
> > > > > I have some questions about the implementation need your help.
> > > > >
> > > > > 2. Kernel image is authenticated and/or measured in LoadImage. I
> > > > > am
> > > > wondering if “command line” is measured as well? “Command line” can
> > > > be treated as an external input and in my opinion it should be measured too.
> > > > >
> > > > > 3. The same question to initrd. Is it measured?
> > > > >
> > > >
> > > > The initrd is measured by the EFI stub in Linux, and we are
> > > > currently adding measurement of the load options to that as well:
> > > > https://lore.kernel.org/all/20220916081441.1993492-2-
> > > > ilias.apalodimas@linaro.org/
> > > >
> > > > The initrd is Linux specific in any case, so there, the Linux OS
> > > > loader is a natural place to take care of this. The load options are
> > > > being added because of the oversight in the TCG spec, which only
> > > > covers load options if they are part of a Boot#### option, but
> > > > between
> > > > LoadImage() and StartImage, you can pass any load options you want
> > > > via the loaded image protocol, so it needs to be measured as well.
> > > >
> > > Thanks Ard for the explanation.
> > > I was told that in grub boot cmd-line/initrd will be measured as well. So my
> > question is that will they be measured twice? One in grub.efi, the other in efi-
> > stub?
> > >
> >
> > The EFI stub may be the only OS loader, so the EFI stub should measure the
> > command line and the initrd.
> >
> > Whether or not a previous loader stage exists that may or may not measure the
> > same pieces is not for the EFI stub to reason about. And in any case, measuring
> > the same thing twice is much less of an issue than not measuring it at all.
> >
> > > My understanding is that the loader should take the responsibility to do the
> > measurement.
> > > For grub boot, grub.efi is the loader so it measures kernel-image/cmd-
> > line/initrd.
> >
> > If the EFI stub is invoked, the EFI stub is the OS loader. We should not be relying
> > on the presence of absence of GRUB (or shim) in the boot chain.
> >
> > > For direct boot, TryRunningQemuKernel() now measures kernel image (in
> > CoreLoadImage). Shall it also measure cmd-line/initrd in the same time?
> > >
> >
> > No, I don't think it should. This is why we are adding this to the EFI stub instead.
> >
> > If we measure the initrd and command line in the EFI stub, we don't have to
> > measure it anywhere else, and we can use any generic EFI loader on a measured
> > boot system.
>
> Hi Ard, I think it better let creator to measure instead of consumer to measure like today's implementation in grub[1]. The creator here means who load/create it. In direct boot, it is OVMF read kernel command line and initrd image. In grub boot, it is grub2.  Because the number of consumer like Linux kernel could be more than 1, but the creator is single.

I agree with this in principle. However, there are corner cases that
we would like to cover, such as booting Linux from the EFI shell. Or
in general, any loader that knows how to load an image and pass a
command line, but may not be aware of whether or which flavor of
measured boot is being used by the platform.

> In another side, "EFI stub" is bind to EFI boot protocol and "EFI handover protocol" is deprecated in grub 2.06[2]. (CC to Daniel).
>

Apologies, I don't understand this sentence.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-20 12:55       ` Lu, Ken
  2022-09-20 13:03         ` Ard Biesheuvel
@ 2022-09-20 13:20         ` Gerd Hoffmann
  2022-09-20 13:38           ` Lu, Ken
  1 sibling, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2022-09-20 13:20 UTC (permalink / raw)
  To: Lu, Ken
  Cc: Ard Biesheuvel, Xu, Min M, Daniel Kiper, devel@edk2.groups.io,
	Ard Biesheuvel, Aktas, Erdem, James Bottomley, Yao, Jiewen

  Hi,

> Hi Ard, I think it better let creator to measure instead of consumer
> to measure like today's implementation in grub[1]. The creator here
> means who load/create it. In direct boot, it is OVMF read kernel
> command line and initrd image.

Nope.  OVMF just places kernel, initrd and cmdline images into a virtual
filesystem (see QemuKernelLoaderFsDxe), so the linux kernel efi stub is
able to load things using the efi file protocol.

take care,
  Gerd


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  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:51             ` Ard Biesheuvel
  0 siblings, 2 replies; 22+ messages in thread
From: Lu, Ken @ 2022-09-20 13:24 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Xu, Min M, Daniel Kiper, devel@edk2.groups.io, Ard Biesheuvel,
	Aktas, Erdem, James Bottomley, Yao, Jiewen, Gerd Hoffmann

> > Hi Ard, I think it better let creator to measure instead of consumer to measure
> like today's implementation in grub[1]. The creator here means who load/create
> it. In direct boot, it is OVMF read kernel command line and initrd image. In grub
> boot, it is grub2.  Because the number of consumer like Linux kernel could be
> more than 1, but the creator is single.
> 
> I agree with this in principle.

So you are not against to do measurement in loader like current does in grub and OVMF, correct? I think it is OK even do twice measurements on cmdline and initrd for the corner case.
In past month, I just submit patch in grub to do CC measurement at https://git.savannah.gnu.org/cgit/grub.git/commit/?id=4c76565b6cb885b7e144dc27f3612066844e2d19

> However, there are corner cases that we would like
> to cover, such as booting Linux from the EFI shell. 

I remember Bottomley or someone mentioned to use CONFIG_CMDLINE and CONFIG_INITRAMFS_SOURCE, such as https://blog.decentriq.com/swiss-cheese-to-cheddar-securing-amd-sev-snp-early-boot-2/ for this corner case, especially for confidential container use case without grub.

Or in general, any loader that
> knows how to load an image and pass a command line, but may not be aware of
> whether or which flavor of measured boot is being used by the platform.
> 

This is headache.... but if loader do not know, why kernel know? How to guarantee both loader and kernel know for consistent measurement results?

> > In another side, "EFI stub" is bind to EFI boot protocol and "EFI handover
> protocol" is deprecated in grub 2.06[2]. (CC to Daniel).
> >
> 
> Apologies, I don't understand this sentence.

May be I am wrong.  I mean whether "EFI stub" code is only valid for "EFI handover protocol", or is it also valid for Linux 32bit/64bit boot? See https://www.kernel.org/doc/html/latest/x86/boot.html
If it is only valid for "EFI handover protocol", then it is deprecated. So "EFI stub" code for measurement will not work for Linux 32bit/64bit boot.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-20 13:20         ` Gerd Hoffmann
@ 2022-09-20 13:38           ` Lu, Ken
  2022-09-20 14:18             ` Gerd Hoffmann
  0 siblings, 1 reply; 22+ messages in thread
From: Lu, Ken @ 2022-09-20 13:38 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Ard Biesheuvel, Xu, Min M, Daniel Kiper, devel@edk2.groups.io,
	Ard Biesheuvel, Aktas, Erdem, James Bottomley, Yao, Jiewen

> > Hi Ard, I think it better let creator to measure instead of consumer
> > to measure like today's implementation in grub[1]. The creator here
> > means who load/create it. In direct boot, it is OVMF read kernel
> > command line and initrd image.
> 
> Nope.  OVMF just places kernel, initrd and cmdline images into a virtual
> filesystem (see QemuKernelLoaderFsDxe), so the linux kernel efi stub is able to
> load things using the efi file protocol.

So there are two types loaders:
1. QemuKernelLoaderFsDxe  -   this way just put kernel/initrd blob into a FS for any future's usage, may be continue boot or not.
2. QemuLoadKernelImage,    -    this is consumed by TryRunningQemuKernel() - standard Qemu direct boot path

I think above two ways are different purpose, so it does not means there are two loaders but only one consumer - kernel boot.

> 
> take care,
>   Gerd


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  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
  1 sibling, 1 reply; 22+ messages in thread
From: James Bottomley @ 2022-09-20 13:43 UTC (permalink / raw)
  To: Lu, Ken, Ard Biesheuvel
  Cc: Xu, Min M, Daniel Kiper, devel@edk2.groups.io, Ard Biesheuvel,
	Aktas, Erdem, Yao, Jiewen, Gerd Hoffmann, Peter Jones

[pjones added because he's done a huge amount of work to get shim to
measure stuff correctly]
On Tue, 2022-09-20 at 13:24 +0000, Lu, Ken wrote:
> > > Hi Ard, I think it better let creator to measure instead of
> > > consumer to measure
> > like today's implementation in grub[1]. The creator here means who
> > load/create it. In direct boot, it is OVMF read kernel command line
> > and initrd image. In grub boot, it is grub2.  Because the number of
> > consumer like Linux kernel could be more than 1, but the creator is
> > single.
> > 
> > I agree with this in principle.
> 
> So you are not against to do measurement in loader like current does
> in grub and OVMF, correct? I think it is OK even do twice
> measurements on cmdline and initrd for the corner case.
> In past month, I just submit patch in grub to do CC measurement at 
> https://git.savannah.gnu.org/cgit/grub.git/commit/?id=4c76565b6cb885b7e144dc27f3612066844e2d19

Wait, we have two separate cases: when the kernel boots via grub, which
is not direct boot and grub measures eveything and when we do direct
boot and grub is not involved.  Ideally, we should be able to get to a
stable PCR8,9 for measurement, but grub isn't hugely helpful there
since it dumps every grub command into the PCRs so direct boot can
never match that whatever the EFI stub does.  The TCG spec isn't very
helpful on some things, but it is very clear that once you've measured
something, you don't measure it again, so we do want to avoid measuring
the same thing twice.

> 
> > However, there are corner cases that we would like
> > to cover, such as booting Linux from the EFI shell. 
> 
> I remember Bottomley or someone mentioned to use CONFIG_CMDLINE and
> CONFIG_INITRAMFS_SOURCE, such as 
> https://blog.decentriq.com/swiss-cheese-to-cheddar-securing-amd-sev-snp-early-boot-2/
> for this corner case, especially for confidential container use case
> without grub.

Well, you know, when you talk of the devil he bites your heels ...

Part of the problem is that if you look at the protocol, the LoadImage
measurement seems not to measure the command line.  If we can fix that,
then we can get something that will work both with direct boot (cmdline
is passed to the image) as well as direct executions of the kernel from
the EFI shell.  I think that's what we should aim for.  It would be too
disruptive now to try to get grub also to measure thisorrectly.

I think the key is agreeing with TCG that the argument list of an
executable, loaded by LoadImage should be measured separately.  There
are parts of the spec that hint at this, but by and large it seems to
assume that the measurement of the boot volume entry (which does
contain the command line [usually empty] is sufficient).

James



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-20 13:38           ` Lu, Ken
@ 2022-09-20 14:18             ` Gerd Hoffmann
  2022-09-20 14:30               ` Lu, Ken
  0 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2022-09-20 14:18 UTC (permalink / raw)
  To: Lu, Ken
  Cc: Ard Biesheuvel, Xu, Min M, Daniel Kiper, devel@edk2.groups.io,
	Ard Biesheuvel, Aktas, Erdem, James Bottomley, Yao, Jiewen

On Tue, Sep 20, 2022 at 01:38:05PM +0000, Lu, Ken wrote:
> > > Hi Ard, I think it better let creator to measure instead of consumer
> > > to measure like today's implementation in grub[1]. The creator here
> > > means who load/create it. In direct boot, it is OVMF read kernel
> > > command line and initrd image.
> > 
> > Nope.  OVMF just places kernel, initrd and cmdline images into a virtual
> > filesystem (see QemuKernelLoaderFsDxe), so the linux kernel efi stub is able to
> > load things using the efi file protocol.
> 
> So there are two types loaders:
> 1. QemuKernelLoaderFsDxe  -   this way just put kernel/initrd blob into a FS for any future's usage, may be continue boot or not.
> 2. QemuLoadKernelImage,    -    this is consumed by TryRunningQemuKernel() - standard Qemu direct boot path

Nope.  QemuLoadKernelImage loads the linux kernel from the virtual
filesystem created by QemuKernelLoaderFsDxe.  And for the initrd it'll
just pass 'inittd=initrd' and the stub loads it.

We have two variants:
  GenericQemuLoadImageLib - supports efi stub only
  X86QemuLoadImageLib     - has fallback code paths for the legacy
                            pre-efi-stub boot protocol (guess that
                            is the one grub has deprecated for 2.06).

So, yes, with the legacy protocol there is no stub which can measure
things, but for the snake of confidential computing we can completely
ignore that.  Kernels which are *that* old certainly will not have
support for SEV / TDX ...

take care,
  Gerd


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-20 14:18             ` Gerd Hoffmann
@ 2022-09-20 14:30               ` Lu, Ken
  2022-09-21  7:14                 ` Gerd Hoffmann
  0 siblings, 1 reply; 22+ messages in thread
From: Lu, Ken @ 2022-09-20 14:30 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Ard Biesheuvel, Xu, Min M, Daniel Kiper, devel@edk2.groups.io,
	Ard Biesheuvel, Aktas, Erdem, James Bottomley, Yao, Jiewen

> > So there are two types loaders:
> > 1. QemuKernelLoaderFsDxe  -   this way just put kernel/initrd blob into a FS
> for any future's usage, may be continue boot or not.
> > 2. QemuLoadKernelImage,    -    this is consumed by TryRunningQemuKernel()
> - standard Qemu direct boot path
> 
> Nope.  QemuLoadKernelImage loads the linux kernel from the virtual filesystem
> created by QemuKernelLoaderFsDxe.  And for the initrd it'll just pass
> 'inittd=initrd' and the stub loads it.
> 
> We have two variants:
>   GenericQemuLoadImageLib - supports efi stub only
>   X86QemuLoadImageLib     - has fallback code paths for the legacy
>                             pre-efi-stub boot protocol (guess that
>                             is the one grub has deprecated for 2.06).
> 
> So, yes, with the legacy protocol there is no stub which can measure things, but
> for the snake of confidential computing we can completely ignore that.  Kernels
> which are *that* old certainly will not have support for SEV / TDX ...
> 

Thanks Hoffman. Hmm.. GenericQemuLoadImageLib sound like is used by ArmVirtQemu.dsc, OvmfXen.dsc, AmdSevX64.dsc,.....
But X86QemuLoadImageLib is used by OvmfPkgX64.dsc and Intel TDX~~

Headache.... do you want use GenericQemuLoadImageLib to replace X86 one for OvmfPkgX64.dsc also?

But either in GenericQemuLoadImageLib, it can do measurement for command line and initrd, correct?

> take care,
>   Gerd


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-20 13:43             ` James Bottomley
@ 2022-09-20 14:34               ` Ard Biesheuvel
  0 siblings, 0 replies; 22+ messages in thread
From: Ard Biesheuvel @ 2022-09-20 14:34 UTC (permalink / raw)
  To: jejb
  Cc: Lu, Ken, Xu, Min M, Daniel Kiper, devel@edk2.groups.io,
	Ard Biesheuvel, Aktas, Erdem, Yao, Jiewen, Gerd Hoffmann,
	Peter Jones

On Tue, 20 Sept 2022 at 15:44, James Bottomley <jejb@linux.ibm.com> wrote:
>
> [pjones added because he's done a huge amount of work to get shim to
> measure stuff correctly]
> On Tue, 2022-09-20 at 13:24 +0000, Lu, Ken wrote:
> > > > Hi Ard, I think it better let creator to measure instead of
> > > > consumer to measure
> > > like today's implementation in grub[1]. The creator here means who
> > > load/create it. In direct boot, it is OVMF read kernel command line
> > > and initrd image. In grub boot, it is grub2.  Because the number of
> > > consumer like Linux kernel could be more than 1, but the creator is
> > > single.
> > >
> > > I agree with this in principle.
> >
> > So you are not against to do measurement in loader like current does
> > in grub and OVMF, correct? I think it is OK even do twice
> > measurements on cmdline and initrd for the corner case.
> > In past month, I just submit patch in grub to do CC measurement at
> > https://git.savannah.gnu.org/cgit/grub.git/commit/?id=4c76565b6cb885b7e144dc27f3612066844e2d19
>
> Wait, we have two separate cases: when the kernel boots via grub, which
> is not direct boot and grub measures eveything and when we do direct
> boot and grub is not involved.  Ideally, we should be able to get to a
> stable PCR8,9 for measurement, but grub isn't hugely helpful there
> since it dumps every grub command into the PCRs so direct boot can
> never match that whatever the EFI stub does.  The TCG spec isn't very
> helpful on some things, but it is very clear that once you've measured
> something, you don't measure it again, so we do want to avoid measuring
> the same thing twice.
>

Of course, in a vertically integrated boot stack design, you would
never measure the same thing twice.

However, we have created such a spectacular mess for ourselves, with
loaders that call load/startimage, loaders that call loadimage but not
startimage, loaders that call neither, loaders that measure the initrd
and command line but not the kernel, loaders that measure everything,,
loaders that only are broken unless they were invoked by shim etc etc,
that we simply cannot rely on anything from the EFI stub's point of
view.

So to me, measuring the initrd and command line from the EFI stub
seems like a reasonable (if somewhat belt-and-braces) approach in this
case.

> >
> > > However, there are corner cases that we would like
> > > to cover, such as booting Linux from the EFI shell.
> >
> > I remember Bottomley or someone mentioned to use CONFIG_CMDLINE and
> > CONFIG_INITRAMFS_SOURCE, such as
> > https://blog.decentriq.com/swiss-cheese-to-cheddar-securing-amd-sev-snp-early-boot-2/
> > for this corner case, especially for confidential container use case
> > without grub.
>
> Well, you know, when you talk of the devil he bites your heels ...
>
> Part of the problem is that if you look at the protocol, the LoadImage
> measurement seems not to measure the command line.  If we can fix that,
> then we can get something that will work both with direct boot (cmdline
> is passed to the image) as well as direct executions of the kernel from
> the EFI shell.  I think that's what we should aim for.  It would be too
> disruptive now to try to get grub also to measure thisorrectly.
>
> I think the key is agreeing with TCG that the argument list of an
> executable, loaded by LoadImage should be measured separately.  There
> are parts of the spec that hint at this, but by and large it seems to
> assume that the measurement of the boot volume entry (which does
> contain the command line [usually empty] is sufficient).
>

I think this is something that should be fixed. Measuring the Boot####
variable that the BDS chose to boot instead of what LoadImage()
actually consumes seems like a huge oversight to me.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-20 13:24           ` Lu, Ken
  2022-09-20 13:43             ` James Bottomley
@ 2022-09-20 14:51             ` Ard Biesheuvel
  2022-09-20 15:14               ` Lu, Ken
  1 sibling, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2022-09-20 14:51 UTC (permalink / raw)
  To: Lu, Ken
  Cc: Xu, Min M, Daniel Kiper, devel@edk2.groups.io, Ard Biesheuvel,
	Aktas, Erdem, James Bottomley, Yao, Jiewen, Gerd Hoffmann

On Tue, 20 Sept 2022 at 15:24, Lu, Ken <ken.lu@intel.com> wrote:
>
> > > Hi Ard, I think it better let creator to measure instead of consumer to measure
> > like today's implementation in grub[1]. The creator here means who load/create
> > it. In direct boot, it is OVMF read kernel command line and initrd image. In grub
> > boot, it is grub2.  Because the number of consumer like Linux kernel could be
> > more than 1, but the creator is single.
> >
> > I agree with this in principle.
>
> So you are not against to do measurement in loader like current does in grub and OVMF, correct? I think it is OK even do twice measurements on cmdline and initrd for the corner case.
> In past month, I just submit patch in grub to do CC measurement at https://git.savannah.gnu.org/cgit/grub.git/commit/?id=4c76565b6cb885b7e144dc27f3612066844e2d19
>

Not fundamentally, no. But between the measurement of the image itself
(which the firmware should do) and the measurement of the initrd and
command line (which the EFI stub will do), I'm not sure there is that
much left.

In general, I think the combinatorial explosion of CC attestation
protocols multiplied by the number of boot stages and loaders is going
to be a concern. We really need some abstractions here.

> > However, there are corner cases that we would like
> > to cover, such as booting Linux from the EFI shell.
>
> I remember Bottomley or someone mentioned to use CONFIG_CMDLINE and CONFIG_INITRAMFS_SOURCE, such as https://blog.decentriq.com/swiss-cheese-to-cheddar-securing-amd-sev-snp-early-boot-2/ for this corner case, especially for confidential container use case without grub.
>
> Or in general, any loader that
> > knows how to load an image and pass a command line, but may not be aware of
> > whether or which flavor of measured boot is being used by the platform.
> >
>
> This is headache.... but if loader do not know, why kernel know? How to guarantee both loader and kernel know for consistent measurement results?
>
> > > In another side, "EFI stub" is bind to EFI boot protocol and "EFI handover
> > protocol" is deprecated in grub 2.06[2]. (CC to Daniel).
> > >
> >
> > Apologies, I don't understand this sentence.
>
> May be I am wrong.  I mean whether "EFI stub" code is only valid for "EFI handover protocol", or is it also valid for Linux 32bit/64bit boot? See https://www.kernel.org/doc/html/latest/x86/boot.html
> If it is only valid for "EFI handover protocol", then it is deprecated.

No it is not deprecated. The EFI handover protocol uses LoadImage()
but not StartImage(). Instead, it jumps directly to an alternative
entrypoint in the image which has a Linux/x86 specific prototype, and
passes additional data.

> So "EFI stub" code for measurement will not work for Linux 32bit/64bit boot.

No you misunderstood. The EFI stub is an integral part of the boot
flow. The only thing we deprecated is invoking it without going
through StartImage().

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-20 14:51             ` Ard Biesheuvel
@ 2022-09-20 15:14               ` Lu, Ken
  0 siblings, 0 replies; 22+ messages in thread
From: Lu, Ken @ 2022-09-20 15:14 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Xu, Min M, Daniel Kiper, devel@edk2.groups.io, Ard Biesheuvel,
	Aktas, Erdem, James Bottomley, Yao, Jiewen, Gerd Hoffmann

> >
> 
> Not fundamentally, no. But between the measurement of the image itself (which
> the firmware should do) and the measurement of the initrd and command line
> (which the EFI stub will do), I'm not sure there is that much left.


> In general, I think the combinatorial explosion of CC attestation protocols
> multiplied by the number of boot stages and loaders is going to be a concern.
> We really need some abstractions here.

[Lu, Ken]  I understand your now. It might looks reasonable if think 
- the LoadImage() is a common abstraction for all EFI application, so image itself should be measured by OVMF/BIOS.
- but kernel command and initrd stuff should belong to EFI stub, who has more knowledge of kernel booting than a general image loading.

> 
> > > However, there are corner cases that we would like to cover, such as
> > > booting Linux from the EFI shell.
> >
> > I remember Bottomley or someone mentioned to use CONFIG_CMDLINE and
> CONFIG_INITRAMFS_SOURCE, such as https://blog.decentriq.com/swiss-
> cheese-to-cheddar-securing-amd-sev-snp-early-boot-2/ for this corner case,
> especially for confidential container use case without grub.
> >
> > Or in general, any loader that
> > > knows how to load an image and pass a command line, but may not be
> > > aware of whether or which flavor of measured boot is being used by the
> platform.
> > >
> >
> > This is headache.... but if loader do not know, why kernel know? How to
> guarantee both loader and kernel know for consistent measurement results?
> >
> > > > In another side, "EFI stub" is bind to EFI boot protocol and "EFI
> > > > handover
> > > protocol" is deprecated in grub 2.06[2]. (CC to Daniel).
> > > >
> > >
> > > Apologies, I don't understand this sentence.
> >
> > May be I am wrong.  I mean whether "EFI stub" code is only valid for
> > "EFI handover protocol", or is it also valid for Linux 32bit/64bit
> > boot? See https://www.kernel.org/doc/html/latest/x86/boot.html
> > If it is only valid for "EFI handover protocol", then it is deprecated.
> 
> No it is not deprecated. The EFI handover protocol uses LoadImage() but not
> StartImage(). Instead, it jumps directly to an alternative entrypoint in the image
> which has a Linux/x86 specific prototype, and passes additional data.
> 
> > So "EFI stub" code for measurement will not work for Linux 32bit/64bit boot.
> 
> No you misunderstood. The EFI stub is an integral part of the boot flow. The
> only thing we deprecated is invoking it without going through StartImage().
[Lu, Ken] Thanks your clarification!

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-20 14:30               ` Lu, Ken
@ 2022-09-21  7:14                 ` Gerd Hoffmann
  2022-09-21 11:24                   ` Lu, Ken
  0 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2022-09-21  7:14 UTC (permalink / raw)
  To: Lu, Ken
  Cc: Ard Biesheuvel, Xu, Min M, Daniel Kiper, devel@edk2.groups.io,
	Ard Biesheuvel, Aktas, Erdem, James Bottomley, Yao, Jiewen

On Tue, Sep 20, 2022 at 02:30:01PM +0000, Lu, Ken wrote:
> > > So there are two types loaders:
> > > 1. QemuKernelLoaderFsDxe  -   this way just put kernel/initrd blob into a FS
> > for any future's usage, may be continue boot or not.
> > > 2. QemuLoadKernelImage,    -    this is consumed by TryRunningQemuKernel()
> > - standard Qemu direct boot path
> > 
> > Nope.  QemuLoadKernelImage loads the linux kernel from the virtual filesystem
> > created by QemuKernelLoaderFsDxe.  And for the initrd it'll just pass
> > 'inittd=initrd' and the stub loads it.
> > 
> > We have two variants:
> >   GenericQemuLoadImageLib - supports efi stub only
> >   X86QemuLoadImageLib     - has fallback code paths for the legacy
> >                             pre-efi-stub boot protocol (guess that
> >                             is the one grub has deprecated for 2.06).
> > 
> > So, yes, with the legacy protocol there is no stub which can measure things, but
> > for the snake of confidential computing we can completely ignore that.  Kernels
> > which are *that* old certainly will not have support for SEV / TDX ...
> > 
> 
> Thanks Hoffman. Hmm.. GenericQemuLoadImageLib sound like is used by ArmVirtQemu.dsc, OvmfXen.dsc, AmdSevX64.dsc,.....
> But X86QemuLoadImageLib is used by OvmfPkgX64.dsc and Intel TDX~~
> 
> Headache.... do you want use GenericQemuLoadImageLib to replace X86 one for OvmfPkgX64.dsc also?

I think most x86 should stick to X86QemuLoadImageLib for backward
compatibility reasons.

The AmdSevX64 and IntelTDX variants can most likely be switched over to
GenericQemuLoadImageLib without breaking stuff.  Linux kernels new
enough to support sev / tdx should not need the legacy support.

> 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?

take care,
  Gerd


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-21  7:14                 ` Gerd Hoffmann
@ 2022-09-21 11:24                   ` Lu, Ken
  2022-09-21 12:27                     ` Gerd Hoffmann
  0 siblings, 1 reply; 22+ messages in thread
From: Lu, Ken @ 2022-09-21 11:24 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Ard Biesheuvel, Xu, Min M, Daniel Kiper, devel@edk2.groups.io,
	Ard Biesheuvel, Aktas, Erdem, James Bottomley, Yao, Jiewen

> 
> > 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. Do we also need remove today's measurement in Grub (I have submitted some patch for TDX in grub...)? According to Bottomley, the same measurement should not be done twice. Or only the one who use GenericQemuLoadImageLib, will give the Linux kernel efi stub for measure?
I hope the final decision could be a consistent approach across direct and grub boot, then we can follow.

> 
> take care,
>   Gerd


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-21 11:24                   ` Lu, Ken
@ 2022-09-21 12:27                     ` Gerd Hoffmann
  2022-09-21 15:41                       ` Ard Biesheuvel
  0 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2022-09-21 12:27 UTC (permalink / raw)
  To: Lu, Ken
  Cc: Ard Biesheuvel, Xu, Min M, Daniel Kiper, devel@edk2.groups.io,
	Ard Biesheuvel, Aktas, Erdem, James Bottomley, Yao, Jiewen

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.

> 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.

> 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.

take care,
  Gerd


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-21 12:27                     ` Gerd Hoffmann
@ 2022-09-21 15:41                       ` Ard Biesheuvel
  2022-09-23  9:34                         ` Ilias Apalodimas
  0 siblings, 1 reply; 22+ messages in thread
From: Ard Biesheuvel @ 2022-09-21 15:41 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Lu, Ken, Xu, Min M, Daniel Kiper, devel@edk2.groups.io,
	Ard Biesheuvel, Aktas, Erdem, James Bottomley, Yao, Jiewen

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.

> > 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.

> > 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.
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [edk2-devel] measurement to command-line/initrd for loading kernel via -kernel option
  2022-09-21 15:41                       ` Ard Biesheuvel
@ 2022-09-23  9:34                         ` Ilias Apalodimas
  0 siblings, 0 replies; 22+ messages in thread
From: Ilias Apalodimas @ 2022-09-23  9:34 UTC (permalink / raw)
  To: devel, ardb
  Cc: Gerd Hoffmann, Lu, Ken, Xu, Min M, Daniel Kiper, Ard Biesheuvel,
	Aktas, Erdem, James Bottomley, Yao, Jiewen

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.
> >
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2022-09-23  9:34 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox