1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH 0/3] OvmfPkg: remove PlatformBootManagerLibGrub
  4. View patch

[edk2-devel] [PATCH 2/3] OvmfPkg/AmdSev: stop using PlatformBootManagerLibGrub

Gerd Hoffmann posted 3 patches 2 years, 9 months ago
There is a newer version of this series
[edk2-devel] [PATCH 2/3] OvmfPkg/AmdSev: stop using PlatformBootManagerLibGrub
Posted by Gerd Hoffmann 2 years, 9 months ago 
Use PlatformBootManagerLib with PcdBootRestrictToFirmware
set to TRUE instead.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/AmdSev/AmdSevX64.dsc | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 943c4eed9831..b32049194d39 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -153,6 +153,7 @@ [LibraryClasses]
   UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
   UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
   DevicePathLib|MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLibDevicePathProtocol.inf
+  NvVarsFileLib|OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.inf
   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
   SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
@@ -339,7 +340,7 @@ [LibraryClasses.common.DXE_DRIVER]
 !else
   DebugLib|OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf
 !endif
-  PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.inf
+  PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
   PlatformBmPrintScLib|OvmfPkg/Library/PlatformBmPrintScLib/PlatformBmPrintScLib.inf
   QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
@@ -464,6 +465,8 @@ [PcdsFixedAtBuild]
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand|TRUE
+  gUefiOvmfPkgTokenSpaceGuid.PcdBootRestrictToFirmware|TRUE
+
 ################################################################################
 #
 # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform
@@ -619,7 +622,10 @@ [Components]
   MdeModulePkg/Universal/Metronome/Metronome.inf
   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
   MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
-  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf {
+    <LibraryClasses>
+      XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf
+  }
   MdeModulePkg/Logo/LogoDxe.inf
   MdeModulePkg/Application/UiApp/UiApp.inf {
     <LibraryClasses>
-- 
2.40.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103999): https://edk2.groups.io/g/devel/message/103999
Mute This Topic: https://groups.io/mt/98683761/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 2/3] OvmfPkg/AmdSev: stop using PlatformBootManagerLibGrub
Posted by James Bottomley 2 years, 9 months ago 
On Thu, 2023-05-04 at 15:32 +0200, Gerd Hoffmann wrote:
> Use PlatformBootManagerLib with PcdBootRestrictToFirmware
> set to TRUE instead.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/AmdSev/AmdSevX64.dsc | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc
> b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index 943c4eed9831..b32049194d39 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -153,6 +153,7 @@ [LibraryClasses]
>   
> UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEn
> tryPoint.inf
>   
> UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/Ue
> fiApplicationEntryPoint.inf
>   
> DevicePathLib|MdePkg/Library/UefiDevicePathLibDevicePathProtocol/Uefi
> DevicePathLibDevicePathProtocol.inf
> +  NvVarsFileLib|OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.inf

All additions apart from this look fine, but this one is a security
risk: EFI variables represent an unmeasured configuration for SEV boot
and, as such, can be used to influence the boot and potentially reveal
boot secrets, so the AmdSevPkg was designed to have read only EFI
variables that couldn't be subject to outside influence.

James



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104009): https://edk2.groups.io/g/devel/message/104009
Mute This Topic: https://groups.io/mt/98683761/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 2/3] OvmfPkg/AmdSev: stop using PlatformBootManagerLibGrub
Posted by Gerd Hoffmann 2 years, 9 months ago 
On Thu, May 04, 2023 at 10:16:05AM -0400, James Bottomley wrote:
> On Thu, 2023-05-04 at 15:32 +0200, Gerd Hoffmann wrote:
> > Use PlatformBootManagerLib with PcdBootRestrictToFirmware
> > set to TRUE instead.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  OvmfPkg/AmdSev/AmdSevX64.dsc | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc
> > b/OvmfPkg/AmdSev/AmdSevX64.dsc
> > index 943c4eed9831..b32049194d39 100644
> > --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> > +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> > @@ -153,6 +153,7 @@ [LibraryClasses]
> >   
> > UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEn
> > tryPoint.inf
> >   
> > UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/Ue
> > fiApplicationEntryPoint.inf
> >   
> > DevicePathLib|MdePkg/Library/UefiDevicePathLibDevicePathProtocol/Uefi
> > DevicePathLibDevicePathProtocol.inf
> > +  NvVarsFileLib|OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.inf
> 
> All additions apart from this look fine, but this one is a security
> risk: EFI variables represent an unmeasured configuration for SEV boot
> and, as such, can be used to influence the boot and potentially reveal
> boot secrets, so the AmdSevPkg was designed to have read only EFI
> variables that couldn't be subject to outside influence.

NvVarsFileLib gets disabled already case PcdSecureBootSupported is set.
Is that good enough?  If not I can extend that to also check
PcdBootRestrictToFirmware.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104030): https://edk2.groups.io/g/devel/message/104030
Mute This Topic: https://groups.io/mt/98683761/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 2/3] OvmfPkg/AmdSev: stop using PlatformBootManagerLibGrub
Posted by James Bottomley 2 years, 9 months ago 
On Thu, 2023-05-04 at 17:08 +0200, Gerd Hoffmann wrote:
> On Thu, May 04, 2023 at 10:16:05AM -0400, James Bottomley wrote:
> > On Thu, 2023-05-04 at 15:32 +0200, Gerd Hoffmann wrote:
> > > Use PlatformBootManagerLib with PcdBootRestrictToFirmware
> > > set to TRUE instead.
> > > 
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > ---
> > >  OvmfPkg/AmdSev/AmdSevX64.dsc | 10 ++++++++--
> > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc
> > > b/OvmfPkg/AmdSev/AmdSevX64.dsc
> > > index 943c4eed9831..b32049194d39 100644
> > > --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> > > +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> > > @@ -153,6 +153,7 @@ [LibraryClasses]
> > >   
> > > UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriv
> > > erEntryPoint.inf
> > >   
> > > UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoin
> > > t/UefiApplicationEntryPoint.inf
> > >   
> > > DevicePathLib|MdePkg/Library/UefiDevicePathLibDevicePathProtocol/
> > > UefiDevicePathLibDevicePathProtocol.inf
> > > +  NvVarsFileLib|OvmfPkg/Library/NvVarsFileLib/NvVarsFileLib.inf
> > 
> > All additions apart from this look fine, but this one is a security
> > risk: EFI variables represent an unmeasured configuration for SEV
> > boot and, as such, can be used to influence the boot and
> > potentially reveal boot secrets, so the AmdSevPkg was designed to
> > have read only EFI variables that couldn't be subject to outside
> > influence.
> 
> NvVarsFileLib gets disabled already case PcdSecureBootSupported is
> set. Is that good enough?  If not I can extend that to also check
> PcdBootRestrictToFirmware.

I think pcd disabling is good enough, although usually secure boot
isn't enabled for this (problem sharing the signing key if the
variables have to be fixed inside the OVMF file), so it would need to
be a more universal PCD.  What we need to prevent is the addition of a
file on the edk2 partition (which is unencrypted) from being able to
influence the boot configuration.

James



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104031): https://edk2.groups.io/g/devel/message/104031
Mute This Topic: https://groups.io/mt/98683761/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.