[PATCH] x86/loader: only patch linux kernels

[PATCH] x86/loader: only patch linux kernels

[Gerd Hoffmann]

If the binary loaded via -kernel is *not* a linux kernel (in which
case protocol == 0), do not patch the linux kernel header fields.

It's (a) pointless and (b) might break binaries by random patching
and (c) changes the binary hash which in turn breaks secure boot
verification.

Background: OVMF happily loads and runs not only linux kernels but
any efi binary via direct kernel boot.

Note: Breaking the secure boot verification is a problem for linux
kernels too, but fixed that is left for another day ...

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index ffbda48917fd..765899eebe43 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1108,7 +1108,7 @@ void x86_load_linux(X86MachineState *x86ms,
      * kernel on the other side of the fw_cfg interface matches the hash of the
      * file the user passed in.
      */
-    if (!sev_enabled()) {
+    if (!sev_enabled() && protocol > 0) {
         memcpy(setup, header, MIN(sizeof(header), setup_size));
     }
 
-- 
2.44.0

Re: [PATCH] x86/loader: only patch linux kernels

[Michael S. Tsirkin]

On Wed, Apr 10, 2024 at 09:21:26AM +0200, Gerd Hoffmann wrote:
> If the binary loaded via -kernel is *not* a linux kernel (in which
> case protocol == 0), do not patch the linux kernel header fields.
> 
> It's (a) pointless and (b) might break binaries by random patching
> and (c) changes the binary hash which in turn breaks secure boot
> verification.
> 
> Background: OVMF happily loads and runs not only linux kernels but
> any efi binary via direct kernel boot.
> 
> Note: Breaking the secure boot verification is a problem for linux
> kernels too, but fixed that is left for another day ...

Um we kind of care about Linux ;)

What's the plan?  I suspect we should just add a command line flag
to skip patching? And once we do that, it seems safer to just
always rely on the flag?

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/i386/x86.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index ffbda48917fd..765899eebe43 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -1108,7 +1108,7 @@ void x86_load_linux(X86MachineState *x86ms,
>       * kernel on the other side of the fw_cfg interface matches the hash of the
>       * file the user passed in.
>       */
> -    if (!sev_enabled()) {
> +    if (!sev_enabled() && protocol > 0) {
>          memcpy(setup, header, MIN(sizeof(header), setup_size));
>      }
>  
> -- 
> 2.44.0

secure boot & direct kernel load (was: Re: [PATCH] x86/loader: only patch linux kernels)

[Gerd Hoffmann]

On Wed, Apr 10, 2024 at 03:26:29AM -0400, Michael S. Tsirkin wrote:
> On Wed, Apr 10, 2024 at 09:21:26AM +0200, Gerd Hoffmann wrote:
> > If the binary loaded via -kernel is *not* a linux kernel (in which
> > case protocol == 0), do not patch the linux kernel header fields.
> > 
> > It's (a) pointless and (b) might break binaries by random patching
> > and (c) changes the binary hash which in turn breaks secure boot
> > verification.
> > 
> > Background: OVMF happily loads and runs not only linux kernels but
> > any efi binary via direct kernel boot.
> > 
> > Note: Breaking the secure boot verification is a problem for linux
> > kernels too, but fixed that is left for another day ...
> 
> Um we kind of care about Linux ;)
> 
> What's the plan?  I suspect we should just add a command line flag
> to skip patching? And once we do that, it seems safer to just
> always rely on the flag?

Well, there are more problems to solve here than just the patching.  So
lets have a look at the bigger picture before discussion the details ...

[ Cc'ing Daniel + Cole ]

Current state of affairs is that OVMF supports two ways to boot a linux
kernel:

 (1) Just load it as EFI binary and boot via linux kernel EFI stub,
     which is the modern way to load a linux kernel (which is why you
     can boot not only linux kernels but any efi binary).

 (2) Use the old EFI handover protocol.  Which is the RHEL-6 era way to
     boot a linux kernel on EFI.

For method (1) secure boot verification must pass.  For (2) not.  So if
you try to use direct kernel boot with secure boot enabled OVMF will
first try (1), which will fail, then go fallback to (2).

The reason for the failure is not only the patching, but also the fact
that the linux kernel is typically verified by shim.efi (and the distro
keys compiled into the binary) instead of the firmware.

Going though (2) is not ideal for multiple reasons, so we need some
strategy how we'll go handle direct kernel load with uefi and secure
boot in a way that (1) works.

Options I see:

  (a) Stop using direct kernel boot, let virt-install & other tools
      create vfat boot media with shim+kernel+initrd instead.

  (b) Enroll the distro signing keys in the efi variable store, so
      booting the kernel without shim.efi works.

  (c) Add support for loading shim to qemu (and ovmf), for example
      with a new '-shim' command line option which stores shim.efi
      in some new fw_cfg file.

(b) + (c) both require a fix for the patching issue.  The options
I see here are:

  (A) Move the patching from qemu to the linuxboot option rom.
      Strictly speaking it belongs there anyway.  It doesn't look
      that easy though, for qemu it is easier to gather all
      information needed ...

  (B) Provide both patched and unpatched setup header, so the
      guest can choose what it needs.

  (C) When implementing (c) above we can piggyback on the -shim
      switch and skip patching in case it is present.

  (D) Add a flag to skip the patching.

Comments?  Other/better ideas?

take care,
  Gerd

Re: secure boot & direct kernel load (was: Re: [PATCH] x86/loader: only patch linux kernels)

[Michael S. Tsirkin]

On Wed, Apr 10, 2024 at 12:35:13PM +0200, Gerd Hoffmann wrote:
> On Wed, Apr 10, 2024 at 03:26:29AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Apr 10, 2024 at 09:21:26AM +0200, Gerd Hoffmann wrote:
> > > If the binary loaded via -kernel is *not* a linux kernel (in which
> > > case protocol == 0), do not patch the linux kernel header fields.
> > > 
> > > It's (a) pointless and (b) might break binaries by random patching
> > > and (c) changes the binary hash which in turn breaks secure boot
> > > verification.
> > > 
> > > Background: OVMF happily loads and runs not only linux kernels but
> > > any efi binary via direct kernel boot.
> > > 
> > > Note: Breaking the secure boot verification is a problem for linux
> > > kernels too, but fixed that is left for another day ...
> > 
> > Um we kind of care about Linux ;)
> > 
> > What's the plan?  I suspect we should just add a command line flag
> > to skip patching? And once we do that, it seems safer to just
> > always rely on the flag?
> 
> Well, there are more problems to solve here than just the patching.  So
> lets have a look at the bigger picture before discussion the details ...
> 
> [ Cc'ing Daniel + Cole ]
> 
> Current state of affairs is that OVMF supports two ways to boot a linux
> kernel:
> 
>  (1) Just load it as EFI binary and boot via linux kernel EFI stub,
>      which is the modern way to load a linux kernel (which is why you
>      can boot not only linux kernels but any efi binary).
> 
>  (2) Use the old EFI handover protocol.  Which is the RHEL-6 era way to
>      boot a linux kernel on EFI.
> 
> For method (1) secure boot verification must pass.  For (2) not.  So if
> you try to use direct kernel boot with secure boot enabled OVMF will
> first try (1), which will fail, then go fallback to (2).
> 
> The reason for the failure is not only the patching, but also the fact
> that the linux kernel is typically verified by shim.efi (and the distro
> keys compiled into the binary) instead of the firmware.
> 
> Going though (2) is not ideal for multiple reasons, so we need some
> strategy how we'll go handle direct kernel load with uefi and secure
> boot in a way that (1) works.
> 
> Options I see:
> 
>   (a) Stop using direct kernel boot, let virt-install & other tools
>       create vfat boot media with shim+kernel+initrd instead.
> 
>   (b) Enroll the distro signing keys in the efi variable store, so
>       booting the kernel without shim.efi works.
> 
>   (c) Add support for loading shim to qemu (and ovmf), for example
>       with a new '-shim' command line option which stores shim.efi
>       in some new fw_cfg file.
> 
> (b) + (c) both require a fix for the patching issue.  The options
> I see here are:
> 
>   (A) Move the patching from qemu to the linuxboot option rom.
>       Strictly speaking it belongs there anyway.  It doesn't look
>       that easy though, for qemu it is easier to gather all
>       information needed ...
> 
>   (B) Provide both patched and unpatched setup header, so the
>       guest can choose what it needs.
> 
>   (C) When implementing (c) above we can piggyback on the -shim
>       switch and skip patching in case it is present.
> 
>   (D) Add a flag to skip the patching.
> 
> Comments?  Other/better ideas?
> 
> take care,
>   Gerd

So if you didn't decide whether to do b or c then I guess D is
easiest and covers both cases?

-- 
MST

Re: secure boot & direct kernel load (was: Re: [PATCH] x86/loader: only patch linux kernels)

[Gerd Hoffmann]

On Wed, Apr 10, 2024 at 07:10:22AM -0400, Michael S. Tsirkin wrote:
> On Wed, Apr 10, 2024 at 12:35:13PM +0200, Gerd Hoffmann wrote:
> > On Wed, Apr 10, 2024 at 03:26:29AM -0400, Michael S. Tsirkin wrote:
> > > On Wed, Apr 10, 2024 at 09:21:26AM +0200, Gerd Hoffmann wrote:
> > > > If the binary loaded via -kernel is *not* a linux kernel (in which
> > > > case protocol == 0), do not patch the linux kernel header fields.
> > > > 
> > > > It's (a) pointless and (b) might break binaries by random patching
> > > > and (c) changes the binary hash which in turn breaks secure boot
> > > > verification.
> > > > 
> > > > Background: OVMF happily loads and runs not only linux kernels but
> > > > any efi binary via direct kernel boot.
> > > > 
> > > > Note: Breaking the secure boot verification is a problem for linux
> > > > kernels too, but fixed that is left for another day ...
> > > 
> > > Um we kind of care about Linux ;)
> > > 
> > > What's the plan?  I suspect we should just add a command line flag
> > > to skip patching? And once we do that, it seems safer to just
> > > always rely on the flag?
> > 
> > Well, there are more problems to solve here than just the patching.  So
> > lets have a look at the bigger picture before discussion the details ...
> > 
> > [ Cc'ing Daniel + Cole ]
> > 
> > Current state of affairs is that OVMF supports two ways to boot a linux
> > kernel:
> > 
> >  (1) Just load it as EFI binary and boot via linux kernel EFI stub,
> >      which is the modern way to load a linux kernel (which is why you
> >      can boot not only linux kernels but any efi binary).
> > 
> >  (2) Use the old EFI handover protocol.  Which is the RHEL-6 era way to
> >      boot a linux kernel on EFI.
> > 
> > For method (1) secure boot verification must pass.  For (2) not.  So if
> > you try to use direct kernel boot with secure boot enabled OVMF will
> > first try (1), which will fail, then go fallback to (2).
> > 
> > The reason for the failure is not only the patching, but also the fact
> > that the linux kernel is typically verified by shim.efi (and the distro
> > keys compiled into the binary) instead of the firmware.
> > 
> > Going though (2) is not ideal for multiple reasons, so we need some
> > strategy how we'll go handle direct kernel load with uefi and secure
> > boot in a way that (1) works.
> > 
> > Options I see:
> > 
> >   (a) Stop using direct kernel boot, let virt-install & other tools
> >       create vfat boot media with shim+kernel+initrd instead.
> > 
> >   (b) Enroll the distro signing keys in the efi variable store, so
> >       booting the kernel without shim.efi works.
> > 
> >   (c) Add support for loading shim to qemu (and ovmf), for example
> >       with a new '-shim' command line option which stores shim.efi
> >       in some new fw_cfg file.
> > 
> > (b) + (c) both require a fix for the patching issue.  The options
> > I see here are:
> > 
> >   (A) Move the patching from qemu to the linuxboot option rom.
> >       Strictly speaking it belongs there anyway.  It doesn't look
> >       that easy though, for qemu it is easier to gather all
> >       information needed ...
> > 
> >   (B) Provide both patched and unpatched setup header, so the
> >       guest can choose what it needs.
> > 
> >   (C) When implementing (c) above we can piggyback on the -shim
> >       switch and skip patching in case it is present.
> > 
> >   (D) Add a flag to skip the patching.
> > 
> > Comments?  Other/better ideas?
> > 
> > take care,
> >   Gerd
> 
> So if you didn't decide whether to do b or c then I guess D is
> easiest and covers both cases?

Easiest if you look at qemu only.  Adding a new config option adds
burdens elsewhere though.  Users and the management stack have to
learn to use the new option.

Both (A) and (B) work automatically and can be combined with both (b)
and (c).  (B) is probably much easier to implement, drawback is it
requires an firmware update too.

take care,
  Gerd

Re: secure boot & direct kernel load (was: Re: [PATCH] x86/loader: only patch linux kernels)

[Gerd Hoffmann]

> > > Options I see:
> > > 
> > >   (a) Stop using direct kernel boot, let virt-install & other tools
> > >       create vfat boot media with shim+kernel+initrd instead.
> > > 
> > >   (b) Enroll the distro signing keys in the efi variable store, so
> > >       booting the kernel without shim.efi works.
> > > 
> > >   (c) Add support for loading shim to qemu (and ovmf), for example
> > >       with a new '-shim' command line option which stores shim.efi
> > >       in some new fw_cfg file.
> > > 
> > > (b) + (c) both require a fix for the patching issue.  The options
> > > I see here are:
> > > 
> > >   (A) Move the patching from qemu to the linuxboot option rom.
> > >       Strictly speaking it belongs there anyway.  It doesn't look
> > >       that easy though, for qemu it is easier to gather all
> > >       information needed ...
> > > 
> > >   (B) Provide both patched and unpatched setup header, so the
> > >       guest can choose what it needs.
> > > 
> > >   (C) When implementing (c) above we can piggyback on the -shim
> > >       switch and skip patching in case it is present.
> > > 
> > >   (D) Add a flag to skip the patching.
> > > 
> > > Comments?  Other/better ideas?
> > > 
> > > take care,
> > >   Gerd
> > 
> > So if you didn't decide whether to do b or c then I guess D is
> > easiest and covers both cases?
> 
> Easiest if you look at qemu only.  Adding a new config option adds
> burdens elsewhere though.  Users and the management stack have to
> learn to use the new option.
> 
> Both (A) and (B) work automatically and can be combined with both (b)
> and (c).  (B) is probably much easier to implement, drawback is it
> requires an firmware update too.

Sneak preview for (c) + (B) is here:
  https://git.kraxel.org/cgit/qemu/log/?h=sirius/direct-secure-boot

(well, almost, instead of unpatched setup header it exposes an unpatched
kernel binary).

Currently looking at the ovmf side of things to make sure the idea
actually works before posting patches to the list.

take care,
  Gerd

Re: secure boot & direct kernel load (was: Re: [PATCH] x86/loader: only patch linux kernels)

[Daniel P. Berrangé]

On Wed, Apr 10, 2024 at 12:35:13PM +0200, Gerd Hoffmann wrote:
> On Wed, Apr 10, 2024 at 03:26:29AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Apr 10, 2024 at 09:21:26AM +0200, Gerd Hoffmann wrote:
> > > If the binary loaded via -kernel is *not* a linux kernel (in which
> > > case protocol == 0), do not patch the linux kernel header fields.
> > > 
> > > It's (a) pointless and (b) might break binaries by random patching
> > > and (c) changes the binary hash which in turn breaks secure boot
> > > verification.
> > > 
> > > Background: OVMF happily loads and runs not only linux kernels but
> > > any efi binary via direct kernel boot.
> > > 
> > > Note: Breaking the secure boot verification is a problem for linux
> > > kernels too, but fixed that is left for another day ...
> > 
> > Um we kind of care about Linux ;)
> > 
> > What's the plan?  I suspect we should just add a command line flag
> > to skip patching? And once we do that, it seems safer to just
> > always rely on the flag?
> 
> Well, there are more problems to solve here than just the patching.  So
> lets have a look at the bigger picture before discussion the details ...
> 
> [ Cc'ing Daniel + Cole ]
> 
> Current state of affairs is that OVMF supports two ways to boot a linux
> kernel:
> 
>  (1) Just load it as EFI binary and boot via linux kernel EFI stub,
>      which is the modern way to load a linux kernel (which is why you
>      can boot not only linux kernels but any efi binary).
> 
>  (2) Use the old EFI handover protocol.  Which is the RHEL-6 era way to
>      boot a linux kernel on EFI.
> 
> For method (1) secure boot verification must pass.  For (2) not.  So if
> you try to use direct kernel boot with secure boot enabled OVMF will
> first try (1), which will fail, then go fallback to (2).
> 
> The reason for the failure is not only the patching, but also the fact
> that the linux kernel is typically verified by shim.efi (and the distro
> keys compiled into the binary) instead of the firmware.
> 
> Going though (2) is not ideal for multiple reasons, so we need some
> strategy how we'll go handle direct kernel load with uefi and secure
> boot in a way that (1) works.
> 
> Options I see:
> 
>   (a) Stop using direct kernel boot, let virt-install & other tools
>       create vfat boot media with shim+kernel+initrd instead.
> 
>   (b) Enroll the distro signing keys in the efi variable store, so
>       booting the kernel without shim.efi works.
> 
>   (c) Add support for loading shim to qemu (and ovmf), for example
>       with a new '-shim' command line option which stores shim.efi
>       in some new fw_cfg file.

The problem with this is that now virt-install  has to actually
find the correct a shim.efi binary. It is already somewhat hard
to find a suitable kerenl+initrd binary, and AFAIK, the places
where we get these binaries don't have shim.efi alongside.

eg for RHEL/Fedora we grab kernel+initrd from the pxeboot dir:

  https://fedora.mirrorservice.org/fedora/linux/development/rawhide/Everything/x86_64/os/images/pxeboot/

This same problem with affect both options (a) and (c).

In various forums we have discussed adding the secureboot
certs to the libosinfo database, so that we can have a
customized EFI varstore with minimized certs, even for the
ISO / HDD boot scenario. If we do that, then (b) is trivial
for direct kernel boot too. (b) kills all birds with the
same stone :-)

> 
> (b) + (c) both require a fix for the patching issue.  The options
> I see here are:
> 
>   (A) Move the patching from qemu to the linuxboot option rom.
>       Strictly speaking it belongs there anyway.  It doesn't look
>       that easy though, for qemu it is easier to gather all
>       information needed ...
> 
>   (B) Provide both patched and unpatched setup header, so the
>       guest can choose what it needs.
> 
>   (C) When implementing (c) above we can piggyback on the -shim
>       switch and skip patching in case it is present.
> 
>   (D) Add a flag to skip the patching.
> 
> Comments?  Other/better ideas?

I guess (b) + (D) is probably my preference.


With regards,
Daniel
-- 
|: 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 :|

Re: secure boot & direct kernel load (was: Re: [PATCH] x86/loader: only patch linux kernels)

[Gerd Hoffmann]

  Hi,

> > Options I see:
> > 
> >   (a) Stop using direct kernel boot, let virt-install & other tools
> >       create vfat boot media with shim+kernel+initrd instead.
> > 
> >   (b) Enroll the distro signing keys in the efi variable store, so
> >       booting the kernel without shim.efi works.
> > 
> >   (c) Add support for loading shim to qemu (and ovmf), for example
> >       with a new '-shim' command line option which stores shim.efi
> >       in some new fw_cfg file.
> 
> The problem with this is that now virt-install  has to actually
> find the correct a shim.efi binary. It is already somewhat hard
> to find a suitable kerenl+initrd binary, and AFAIK, the places
> where we get these binaries don't have shim.efi alongside.
> 
> eg for RHEL/Fedora we grab kernel+initrd from the pxeboot dir:
> 
>   https://fedora.mirrorservice.org/fedora/linux/development/rawhide/Everything/x86_64/os/images/pxeboot/

shim is https://fedora.mirrorservice.org/fedora/linux/development/rawhide/Everything/x86_64/os/EFI/BOOT/BOOTX64.EFI

> In various forums we have discussed adding the secureboot
> certs to the libosinfo database, so that we can have a
> customized EFI varstore with minimized certs, even for the
> ISO / HDD boot scenario.

Well.  It's not that easy unfortunately.  At least the "minimized certs"
part.  shim often is signed with the microsoft keys only, so you can't
drop that without rendering the install.iso unbootable.

Only adding the distro certs without removing the microsoft certs works
of course.

> If we do that, then (b) is trivial for direct kernel boot too.

Yep.

> (b) kills all birds with the same stone :-)

See above.  I'd love this being true but it is not.

> > (b) + (c) both require a fix for the patching issue.  The options
> > I see here are:
> > 
> >   (A) Move the patching from qemu to the linuxboot option rom.
> >       Strictly speaking it belongs there anyway.  It doesn't look
> >       that easy though, for qemu it is easier to gather all
> >       information needed ...
> > 
> >   (B) Provide both patched and unpatched setup header, so the
> >       guest can choose what it needs.
> > 
> >   (C) When implementing (c) above we can piggyback on the -shim
> >       switch and skip patching in case it is present.
> > 
> >   (D) Add a flag to skip the patching.
> > 
> > Comments?  Other/better ideas?
> 
> I guess (b) + (D) is probably my preference.

I prefer (B) over (D) because that doesn't require a new config option
(which probably needs support in libvirt and possibly higher up in the
management stack too ...).

Patch series implementing (B) and the -shim switch:
https://lore.kernel.org/qemu-devel/20240411094830.1337658-1-kraxel@redhat.com/

Using -shim is optional, so it's up to virt-install whenever it wants go
for (b) or (c).

take care,
  Gerd

Re: secure boot & direct kernel load (was: Re: [PATCH] x86/loader: only patch linux kernels)

[Daniel P. Berrangé]

On Mon, Apr 15, 2024 at 03:30:32PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > Options I see:
> > > 
> > >   (a) Stop using direct kernel boot, let virt-install & other tools
> > >       create vfat boot media with shim+kernel+initrd instead.
> > > 
> > >   (b) Enroll the distro signing keys in the efi variable store, so
> > >       booting the kernel without shim.efi works.
> > > 
> > >   (c) Add support for loading shim to qemu (and ovmf), for example
> > >       with a new '-shim' command line option which stores shim.efi
> > >       in some new fw_cfg file.
> > 
> > The problem with this is that now virt-install  has to actually
> > find the correct a shim.efi binary. It is already somewhat hard
> > to find a suitable kerenl+initrd binary, and AFAIK, the places
> > where we get these binaries don't have shim.efi alongside.
> > 
> > eg for RHEL/Fedora we grab kernel+initrd from the pxeboot dir:
> > 
> >   https://fedora.mirrorservice.org/fedora/linux/development/rawhide/Everything/x86_64/os/images/pxeboot/
> 
> shim is https://fedora.mirrorservice.org/fedora/linux/development/rawhide/Everything/x86_64/os/EFI/BOOT/BOOTX64.EFI
> 
> > In various forums we have discussed adding the secureboot
> > certs to the libosinfo database, so that we can have a
> > customized EFI varstore with minimized certs, even for the
> > ISO / HDD boot scenario.
> 
> Well.  It's not that easy unfortunately.  At least the "minimized certs"
> part.  shim often is signed with the microsoft keys only, so you can't
> drop that without rendering the install.iso unbootable.
> 
> Only adding the distro certs without removing the microsoft certs works
> of course.

In that scenario libosinfo would report that the given OS
requires both the microsoft & $distro certs to be
enrolled.

Only if shim were signed by the $distro certs, would
libosifo omit reporting the microsoft certs.

Basically libosinfo would have to report whatever set
of 'n' certs are required to make boot work.


With regards,
Daniel
-- 
|: 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 :|