On 05/15/18 14:30, marcandre.lur...@redhat.com wrote: > From: Marc-André Lureau <marcandre.lur...@redhat.com> > > Call Tcg2PhysicalPresenceLibProcessRequest() to process pending PPI > requests from PlatformBootManagerAfterConsole(). > > Laszlo understanding of edk2 is that the PPI operation processing was > meant to occur *entirely* before End-Of-Dxe, so that 3rd party UEFI > drivers couldn't interfere with PPI opcode processing *at all*. > > He suggested that we should *not* call > Tcg2PhysicalPresenceLibProcessRequest() from BeforeConsole(). Because, > an "auth" console, i.e. one that does not depend on a 3rd party > driver, is *in general* impossible to guarantee. Instead we could opt > to trust 3rd party drivers, and use the "normal" console(s) in > AfterConsole(), in order to let the user confirm the PPI requests. It > will depend on the user to enable Secure Boot, so that the > trustworthiness of those 3rd party drivers is ensured. If an attacker > roots the guest OS from within, queues some TPM2 PPI requests, and > also modifies drivers on the EFI system partition and/or in GPU option > ROMs (?), then those drivers will not load after guest reboot, and > thus the dependent console(s) won't be used for confirming the PPI > requests. > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 8 ++++++++ > .../PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 ++ > 2 files changed, 10 insertions(+) > > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > index 004b753f4d26..8b1beaa3e207 100644 > --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > @@ -16,6 +16,7 @@ > #include <Guid/XenInfo.h> > #include <Guid/RootBridgesConnectedEventGroup.h> > #include <Protocol/FirmwareVolume2.h> > +#include <Library/Tcg2PhysicalPresenceLib.h> > > > // > @@ -1410,6 +1411,13 @@ PlatformBootManagerAfterConsole ( > // > PciAcpiInitialization (); > > + > + // > + // Process TPM PPI request > + // > + Tcg2PhysicalPresenceLibProcessRequest (NULL); > + > +
Please just keep one empty line before and after the new code. With that cleanup, for this patch: Reviewed-by: Laszlo Ersek <ler...@redhat.com> This series is a very nice work IMO, thank you both Stefan and Marc-André. I hope v2 can be merged! Thanks! Laszlo > // > // Process QEMU's -kernel command line option > // > diff --git > a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > index 27789b7377bc..4b72c44bcf0a 100644 > --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf > @@ -38,6 +38,7 @@ [Packages] > IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec > SourceLevelDebugPkg/SourceLevelDebugPkg.dec > OvmfPkg/OvmfPkg.dec > + SecurityPkg/SecurityPkg.dec > > [LibraryClasses] > BaseLib > @@ -56,6 +57,7 @@ [LibraryClasses] > LoadLinuxLib > QemuBootOrderLib > UefiLib > + Tcg2PhysicalPresenceLib > > [Pcd] > gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent >