From 74e5313dfa6719f7990c7e175e035d17c9b3f657 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Fri, 5 Jun 2020 23:44:43 +0200 Subject: Revert "OvmfPkg: use generic QEMU image loader for secure boot enabled builds" Notes about the RHEL-8.2/20190904-37eef91017ad [edk2-stable201908] -> RHEL-8.3/20200603-ca407c7246bf [edk2-stable202005] rebase: - new patch (to be dropped later, hopefully) This reverts commit ced77332cab626f35fbdb36630be27303d289d79. Upstream commit ced77332cab6 ("OvmfPkg: use generic QEMU image loader for secure boot enabled builds", 2020-03-05) changes the "Secure Boot threat model" in a way that is incompatible with at least two use cases. Namely, OVMF has always considered kernel images direct-booted via fw_cfg as trusted, bypassing Secure Boot validation. While that approach is rooted in a technicality (namely, OVMF doesn't load such images with the LoadImage() UEFI boot service / through the UEFI stub, but with the Linux/x86 Boot Protocol), that doesn't mean it's wrong. The direct-booted kernel from fw_cfg comes from the host side, and Secure Boot in the guest is a barrier between the guest firmware and the guest operating system -- it's not a barrier between host and guest. Upstream commit ced77332cab6 points out that the above (historical) OVMF behavior differs from ArmVirtQemu's -- the latter direct-boots kernels from fw_cfg with the LoadImage() / StartImage() boot services. While that difference indeed exists between OVMF and ArmVirtQemu, it's not relevant for RHEL downstream. That's because we never build the ArmVirtQemu firmware with the Secure Boot feature, so LoadImage() can never reject the direct-booted kernel due to a signing issue. Subjecting a kernel direct-booted via fw_cfg to Secure Boot verification breaks at least two use cases with OVMF: - It breaks the %check stage in the SPEC file. In that stage, we use the "ovmf-vars-generator" utility from the "qemu-ovmf-secureboot" project, for verifying whether the Secure Boot operational mode is enabled. The guest kernel is supposed to boot, and to print "Secure boot enabled". As guest kernel, we pick whatever host kernel is available in the Brew build root. The kernel in question may be a publicly released RHEL kernel, signed with "Red Hat Secure Boot (signing key 1)", or a development build, signed for example with "Red Hat Secure Boot Signing 3 (beta)". Either way, none of these keys are accepted by the certificates that were enrolled by "ovmf-vars-generator" / "EnrollDefaultKeys.efi" in the %build stage. Therefore, the %check stage fails. - It breaks "virt-install --location NETWORK-URL" Linux guest installations, if the variable store template used for the new domain has the Secure Boot operational mode enabled. "virt-install --location" fetches the kernel from the remote OS tree, and passes it to the guest firmware via fw_cfg. Therefore the above symptom appears (even for publicly released OSes). Importantly, if the user downloads the installer ISO of the publicly released Fedora / RHEL OS, and exposes the ISO to the guest for example as a virtio-scsi CD-ROM, then the installation with "virt-install" (without "--location") does succeed. That's because that way, "shim" is booted first, from the UEFI-bootable CD-ROM. "Shim" does pass Secure Boot verification against the Microsoft certificates, and then it is "shim" that accepts the "Red Hat Secure Boot (signing key 1)" signature on the guest kernel. Some ways to approach this problem (without reverting upstream commit ced77332cab6): - Equip "ovmf-vars-generator" / "EnrollDefaultKeys.efi" to enroll the public half of "Red Hat Secure Boot (signing key 1)" in the %build stage. Use a publicly released RHEL kernel in the %check stage. Downsides: - The Brew build root does not offer any particular released RHEL kernel, so either the %check stage would have to download it, or the SRPM would have to bundle it. However, Brew build environments do not have unfettered network access (rightly so), so the download wouldn't work. Furthermore, for bundling with the SRPM, such a kernel image could be considered too large. - Does not solve the "virt-install --location" issue for other vendors' signed kernels. - Invoke "ovmf-vars-generator" / "EnrollDefaultKeys.efi" multiple times during %build, to create multiple varstore templates. One that would accept publicly released RHEL kernels, and another to accept development kernels. Don't try to use a particular guest kernel for verification; instead, check what kernel Brew offers in the build environment, and use the varstore template matching *that* kernel. Downsides: - It may be considered useless to perform %check with a varstore template that is *not* the one that we ship. - Does not solve the "virt-install --location" issue for other vendors' signed kernels. - Sign the RHEL kernels such that the currently enrolled certificates accept them. Downsides: - Not feasible at all; it would require Microsoft to sign our kernels. "Shim" exists exactly to eliminate such signing requirements. - Modify "virt-install --location NETWORK-URL" such that it download a complete (UEFI-bootable) installer ISO image, rather than broken-out vmlinuz / initrd files. In other words, replace direct (fw_cfg) kernel boot with a CD-ROM / "shim" boot, internally to "virt-install". Downsides: - Defeats the goal of "virt-install --location NETWORK-URL", and defeats the network installation method of (for example) Anaconda. For now, revert upstream commit ced77332cab6, in order to return to the model we had used in RHEL-8.2 and before. The following ticket has been filed to investigate the problem separately: . Signed-off-by: Laszlo Ersek --- OvmfPkg/OvmfPkgIa32.dsc | 4 ---- OvmfPkg/OvmfPkgIa32X64.dsc | 4 ---- OvmfPkg/OvmfPkgX64.dsc | 4 ---- 3 files changed, 12 deletions(-) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index e8868136d8..5b1e757cb9 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -379,11 +379,7 @@ PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf -!if $(SECURE_BOOT_ENABLE) == TRUE - QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf -!else QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf -!endif !if $(TPM_ENABLE) == TRUE Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index d05275a324..5dffc32105 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -383,11 +383,7 @@ PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf -!if $(SECURE_BOOT_ENABLE) == TRUE - QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf -!else QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf -!endif !if $(TPM_ENABLE) == TRUE Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index cac4cecf18..a2a76fdeea 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -383,11 +383,7 @@ PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf -!if $(SECURE_BOOT_ENABLE) == TRUE - QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf -!else QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf -!endif !if $(TPM_ENABLE) == TRUE Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf -- 2.18.1