From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 386D8203B8CC8 for ; Thu, 17 May 2018 03:24:56 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C6EBF407572D; Thu, 17 May 2018 10:24:55 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-28.rdu2.redhat.com [10.10.120.28]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9D5C02026E0E; Thu, 17 May 2018 10:24:54 +0000 (UTC) To: marcandre.lureau@redhat.com, edk2-devel@lists.01.org Cc: qemu-devel@nongnu.org, javierm@redhat.com, pjones@redhat.com, jiewen.yao@intel.com References: <20180515123007.10164-1-marcandre.lureau@redhat.com> <20180515123007.10164-5-marcandre.lureau@redhat.com> From: Laszlo Ersek Message-ID: <1c61d02b-bd26-3eb7-239d-0f1458a649a3@redhat.com> Date: Thu, 17 May 2018 12:24:53 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180515123007.10164-5-marcandre.lureau@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Thu, 17 May 2018 10:24:55 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Thu, 17 May 2018 10:24:55 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH 4/4] ovmf: process TPM PPI request in AfterConsole() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 May 2018 10:24:57 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 05/15/18 14:30, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau > > 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 > --- > 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 > #include > #include > +#include > > > // > @@ -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 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 >