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

["James Bottomley" <jejb@linux.ibm.com>]

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