From: Marc-André Lureau <marcandre.lureau@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.lureau@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);
+
+
   //
   // 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
-- 
2.17.0.253.g3dd125b46d

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@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.lureau@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 <lersek@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
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel