This reverts commit ced77332cab626f35fbdb36630be27303d289d79.
The command
virt-install --location NETWORK-URL
downloads the vmlinuz and initrd files from the remote OS tree, and passes
them to the guest firmware via fw_cfg.
When used with IA32 / X64 guests, virt-install expects the guest firmware
to do two things, at the same time:
- launch the fw_cfg kernel image even if the latter does not pass SB
verification (SB checking is supposed to be bypassed entirely in favor
of the Linux/x86 Boot Protocol),
- still let the guest kernel perceive SB as enabled.
Commit ced77332cab6 prevented this, by removing the Linux/x86 Boot
Protocol from such an OVMF image that was built with SECURE_BOOT_ENALBE.
While that's the right thing in theory, in practice "virt-install
--location NETWORK-URL" is entrenched, and we shouldn't break it.
We can tolerate the Linux/x86 Boot Protocol as a one-of-a-kind SB bypass
for direct-booted kernels, because:
- the fw_cfg content comes from QEMU, and the guest is already at QEMU's
mercy,
- in the guest, OS boots after the initial installation will use "shim"
rather than an fw_cfg kernel, which we can consider somewhat similar to
"Audit Mode / Deployed Mode" (~ trust for install, lock down after).
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
Notes:
- pick up Ard's ACK from
http://mid.mail-archive.com/c06ee730-e421-0aa5-882f-bc09ae9c546f@arm.com
https://edk2.groups.io/g/devel/message/61169
- posting to the list to enable feedback on the commit message (I intend
to push the patch in one or two days)
- repo: https://pagure.io/lersek/edk2.git
branch: reenable_fwcfg_x86_boot_proto
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 d0df9cbbfb2b..16103d177374 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -379,11 +379,7 @@ [LibraryClasses.common.DXE_DRIVER]
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 b3ae62fee92b..9597ef6721da 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -383,11 +383,7 @@ [LibraryClasses.common.DXE_DRIVER]
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 f7fe75ebf531..a6e585c03d41 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -383,11 +383,7 @@ [LibraryClasses.common.DXE_DRIVER]
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.19.1.3.g30247aa5d201
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61301): https://edk2.groups.io/g/devel/message/61301
Mute This Topic: https://groups.io/mt/74895863/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 6/15/20 4:45 PM, Laszlo Ersek wrote: > This reverts commit ced77332cab626f35fbdb36630be27303d289d79. > > The command > > virt-install --location NETWORK-URL > > downloads the vmlinuz and initrd files from the remote OS tree, and passes > them to the guest firmware via fw_cfg. > > When used with IA32 / X64 guests, virt-install expects the guest firmware > to do two things, at the same time: > > - launch the fw_cfg kernel image even if the latter does not pass SB > verification (SB checking is supposed to be bypassed entirely in favor > of the Linux/x86 Boot Protocol), > > - still let the guest kernel perceive SB as enabled. > > Commit ced77332cab6 prevented this, by removing the Linux/x86 Boot > Protocol from such an OVMF image that was built with SECURE_BOOT_ENALBE. > While that's the right thing in theory, in practice "virt-install > --location NETWORK-URL" is entrenched, and we shouldn't break it. > > We can tolerate the Linux/x86 Boot Protocol as a one-of-a-kind SB bypass > for direct-booted kernels, because: > > - the fw_cfg content comes from QEMU, and the guest is already at QEMU's > mercy, > > - in the guest, OS boots after the initial installation will use "shim" > rather than an fw_cfg kernel, which we can consider somewhat similar to > "Audit Mode / Deployed Mode" (~ trust for install, lock down after). > > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Philippe Mathieu-Daudé <philmd@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com> > --- > > Notes: > - pick up Ard's ACK from > > http://mid.mail-archive.com/c06ee730-e421-0aa5-882f-bc09ae9c546f@arm.com > https://edk2.groups.io/g/devel/message/61169 > > - posting to the list to enable feedback on the commit message (I intend > to push the patch in one or two days) > > - repo: https://pagure.io/lersek/edk2.git > branch: reenable_fwcfg_x86_boot_proto > > 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 d0df9cbbfb2b..16103d177374 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -379,11 +379,7 @@ [LibraryClasses.common.DXE_DRIVER] > 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 b3ae62fee92b..9597ef6721da 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -383,11 +383,7 @@ [LibraryClasses.common.DXE_DRIVER] > 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 f7fe75ebf531..a6e585c03d41 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -383,11 +383,7 @@ [LibraryClasses.common.DXE_DRIVER] > 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 > Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61302): https://edk2.groups.io/g/devel/message/61302 Mute This Topic: https://groups.io/mt/74895863/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 06/15/20 16:49, Philippe Mathieu-Daudé wrote: > On 6/15/20 4:45 PM, Laszlo Ersek wrote: >> This reverts commit ced77332cab626f35fbdb36630be27303d289d79. >> >> The command >> >> virt-install --location NETWORK-URL >> >> downloads the vmlinuz and initrd files from the remote OS tree, and passes >> them to the guest firmware via fw_cfg. >> >> When used with IA32 / X64 guests, virt-install expects the guest firmware >> to do two things, at the same time: >> >> - launch the fw_cfg kernel image even if the latter does not pass SB >> verification (SB checking is supposed to be bypassed entirely in favor >> of the Linux/x86 Boot Protocol), >> >> - still let the guest kernel perceive SB as enabled. >> >> Commit ced77332cab6 prevented this, by removing the Linux/x86 Boot >> Protocol from such an OVMF image that was built with SECURE_BOOT_ENALBE. >> While that's the right thing in theory, in practice "virt-install >> --location NETWORK-URL" is entrenched, and we shouldn't break it. >> >> We can tolerate the Linux/x86 Boot Protocol as a one-of-a-kind SB bypass >> for direct-booted kernels, because: >> >> - the fw_cfg content comes from QEMU, and the guest is already at QEMU's >> mercy, >> >> - in the guest, OS boots after the initial installation will use "shim" >> rather than an fw_cfg kernel, which we can consider somewhat similar to >> "Audit Mode / Deployed Mode" (~ trust for install, lock down after). >> >> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> >> Cc: Jordan Justen <jordan.l.justen@intel.com> >> Cc: Philippe Mathieu-Daudé <philmd@redhat.com> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com> >> --- >> >> Notes: >> - pick up Ard's ACK from >> >> http://mid.mail-archive.com/c06ee730-e421-0aa5-882f-bc09ae9c546f@arm.com >> https://edk2.groups.io/g/devel/message/61169 >> >> - posting to the list to enable feedback on the commit message (I intend >> to push the patch in one or two days) >> >> - repo: https://pagure.io/lersek/edk2.git >> branch: reenable_fwcfg_x86_boot_proto >> >> 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 d0df9cbbfb2b..16103d177374 100644 >> --- a/OvmfPkg/OvmfPkgIa32.dsc >> +++ b/OvmfPkg/OvmfPkgIa32.dsc >> @@ -379,11 +379,7 @@ [LibraryClasses.common.DXE_DRIVER] >> 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 b3ae62fee92b..9597ef6721da 100644 >> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >> @@ -383,11 +383,7 @@ [LibraryClasses.common.DXE_DRIVER] >> 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 f7fe75ebf531..a6e585c03d41 100644 >> --- a/OvmfPkg/OvmfPkgX64.dsc >> +++ b/OvmfPkg/OvmfPkgX64.dsc >> @@ -383,11 +383,7 @@ [LibraryClasses.common.DXE_DRIVER] >> 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 >> > > Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com> Thanks! Merged as commit 82808b422617 ('Revert "OvmfPkg: use generic QEMU image loader for secure boot enabled ..."', 2020-06-16) via <https://github.com/tianocore/edk2/pull/703>, after truncating the subject line (originally auto-generated by git-revert), to pacify PatchCheck.py. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61358): https://edk2.groups.io/g/devel/message/61358 Mute This Topic: https://groups.io/mt/74895863/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.