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 6E68A20955F07 for ; Mon, 26 Feb 2018 01:44:51 -0800 (PST) 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 15D5217CEF1; Mon, 26 Feb 2018 09:50:55 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-149.rdu2.redhat.com [10.10.120.149]) by smtp.corp.redhat.com (Postfix) with ESMTP id D4071202699C; Mon, 26 Feb 2018 09:50:49 +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: <20180223132311.26555-1-marcandre.lureau@redhat.com> <20180223132311.26555-6-marcandre.lureau@redhat.com> From: Laszlo Ersek Message-ID: <53a8a301-0719-aca9-7b4c-90df90ccfda0@redhat.com> Date: Mon, 26 Feb 2018 10:50:49 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180223132311.26555-6-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.2]); Mon, 26 Feb 2018 09:50:55 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Mon, 26 Feb 2018 09:50: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 5/7] ovmf: link with Tcg2Dxe module X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Feb 2018 09:44:51 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 02/23/18 14:23, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau > > This module measures and log the boot environment. It also produces > the Tcg2 protocol, which allows for example to read the log from OS: > > [ 0.000000] efi: EFI v2.70 by EDK II > [ 0.000000] efi: SMBIOS=0x3fa1f000 ACPI=0x3fbb6000 ACPI 2.0=0x3fbb6014 MEMATTR=0x3e7d4318 TPMEventLog=0x3db21018 > > $ python chipsec_util.py tpm parse_log binary_bios_measurements > > [CHIPSEC] Version 1.3.5.dev2 > [CHIPSEC] API mode: using OS native API (not using CHIPSEC kernel module) > [CHIPSEC] Executing command 'tpm' with args ['parse_log', '/tmp/binary_bios_measurements'] > > PCR: 0 type: EV_S_CRTM_VERSION size: 0x2 digest: 1489f923c4dca729178b3e3233458550d8dddf29 > + version: > PCR: 0 type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10 digest: fd39ced7c0d2a61f6830c78c7625f94826b05bcc > + base: 0x820000 length: 0xe0000 > PCR: 0 type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10 digest: 39ebc6783b72bc1e73c7d5bcfeb5f54a3f105d4c > + base: 0x900000 length: 0xa00000 > PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x35 digest: 57cd4dc19442475aa82743484f3b1caa88e142b8 > PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24 digest: 9b1387306ebb7ff8e795e7be77563666bbf4516e > PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26 digest: 9afa86c507419b8570c62167cb9486d9fc809758 > PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24 digest: 5bf8faa078d40ffbd03317c93398b01229a0e1e0 > PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26 digest: 734424c9fe8fc71716c42096f4b74c88733b175e > PCR: 7 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473 > PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x3e digest: 252f8ebb85340290b64f4b06a001742be8e5cab6 > PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x6e digest: 22a4f6ee9af6dba01d3528deb64b74b582fc182b > PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x80 digest: b7811d5bf30a7efd4e385c6179fe10d9290bb9e8 > PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x84 digest: 425e502c24fc924e231e0a62327b6b7d1f704573 > PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x9a digest: 0b5d2c98ac5de6148a4a1490ff9d5df69039f04e > PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0xbd digest: 20bd5f402271d57a88ea314fe35c1705956b1f74 > PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x88 digest: df5d6605cb8f4366d745a8464cfb26c1efdc305c > PCR: 4 type: EV_EFI_ACTION size: 0x28 digest: cd0fdb4531a6ec41be2753ba042637d6e5f7f256 > PCR: 0 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473 > PCR: 1 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473 > PCR: 2 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473 > PCR: 3 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473 > PCR: 4 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473 > PCR: 5 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473 > > $ tpm2_pcrlist > sha1 : > 0 : 35bd1786b6909daad610d7598b1d620352d33b8a > 1 : ec0511e860206e0af13c31da2f9e943fb6ca353d > 2 : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236 > 3 : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236 > 4 : 45a323382bd933f08e7f0e256bc8249e4095b1ec > 5 : d16d7e629fd8d08ca256f9ad3a3a1587c9e6cc1b > 6 : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236 > 7 : 518bd167271fbb64589c61e43d8c0165861431d8 > 8 : 0000000000000000000000000000000000000000 > 9 : 0000000000000000000000000000000000000000 > 10 : 0000000000000000000000000000000000000000 > 11 : 0000000000000000000000000000000000000000 > 12 : 0000000000000000000000000000000000000000 > 13 : 0000000000000000000000000000000000000000 > 14 : 0000000000000000000000000000000000000000 > 15 : 0000000000000000000000000000000000000000 > 16 : 0000000000000000000000000000000000000000 > 17 : ffffffffffffffffffffffffffffffffffffffff > 18 : ffffffffffffffffffffffffffffffffffffffff > 19 : ffffffffffffffffffffffffffffffffffffffff > 20 : ffffffffffffffffffffffffffffffffffffffff > 21 : ffffffffffffffffffffffffffffffffffffffff > 22 : ffffffffffffffffffffffffffffffffffffffff > 23 : 0000000000000000000000000000000000000000 > sha256 : > 0 : 9ae903dbae3357ac00d223660bac19ea5c021499a56201104332ab966631ce2c > 1 : acc611d90245cf04e77b0ca94901f90e7fa54770f0426f53c3049b532243d1b8 > 2 : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969 > 3 : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969 > 4 : 7a94ffe8a7729a566d3d3c577fcb4b6b1e671f31540375f80eae6382ab785e35 > 5 : a5ceb755d043f32431d63e39f5161464620a3437280494b5850dc1b47cc074e0 > 6 : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969 > 7 : 65caf8dd1e0ea7a6347b635d2b379c93b9a1351edc2afc3ecda700e534eb3068 > 8 : 0000000000000000000000000000000000000000000000000000000000000000 > 9 : 0000000000000000000000000000000000000000000000000000000000000000 > 10 : 0000000000000000000000000000000000000000000000000000000000000000 > 11 : 0000000000000000000000000000000000000000000000000000000000000000 > 12 : 0000000000000000000000000000000000000000000000000000000000000000 > 13 : 0000000000000000000000000000000000000000000000000000000000000000 > 14 : 0000000000000000000000000000000000000000000000000000000000000000 > 15 : 0000000000000000000000000000000000000000000000000000000000000000 > 16 : 0000000000000000000000000000000000000000000000000000000000000000 > 17 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > 18 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > 19 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > 20 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > 21 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > 22 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > 23 : 0000000000000000000000000000000000000000000000000000000000000000 > sha384 : > > The PhysicalPresenceLib is required, it sets some variables, but the > firmware doesn't act on it yet. > > CC: Laszlo Ersek > CC: Stefan Berger > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Marc-André Lureau > --- > OvmfPkg/OvmfPkgX64.dsc | 15 +++++++++++++++ > OvmfPkg/OvmfPkgX64.fdf | 4 ++++ > 2 files changed, 19 insertions(+) > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 34a7c2778e..9bd0709f98 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -213,6 +213,8 @@ > !if $(TPM2_ENABLE) == TRUE > Tpm12CommandLib|SecurityPkg/Library/Tpm12CommandLib/Tpm12CommandLib.inf > Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf > + Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf > + Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf > !endif > > [LibraryClasses.common] I'd prefer to review this part in v2, once the @@ hunk headers are set up at your end ("xfuncname"). > @@ -364,6 +366,11 @@ > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf > +!if $(TPM2_ENABLE) > + HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf > + Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf Can we drop TPM12? "Tcg2Dxe.inf" does not seem to depend on this lib class. > + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf > +!endif > > [LibraryClasses.common.UEFI_APPLICATION] > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > @@ -654,6 +661,14 @@ > NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf > NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf > } > + > + SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf { > + > + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf Can you please explain why Tpm2DeviceLib has to be resolved differently for DXE_DRIVER modules in general (see above) and for "Tcg2Dxe.inf" specifically? Hmmm... Looks like "Tpm2DeviceLibTcg2.inf" implements the APIs via the TPM2 protocol. Whereas "Tcg2Dxe.inf" itself provides that protocol, so obviously it cannot rely on the protocol; it has to access the device, which is done with the help of "Tpm2DeviceLibRouterDxe.inf" and the "Tpm2InstanceLibDTpm.inf" that is plugged in via NULL resolution below. Is that about correct? If so, can you please document it in the commit message? > + NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf > + NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf Again -- is SHA1 required? > + NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf > + } > !endif > > !if $(SECURE_BOOT_ENABLE) == TRUE > diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf > index 9558142a42..b8dd7ecae4 100644 > --- a/OvmfPkg/OvmfPkgX64.fdf > +++ b/OvmfPkg/OvmfPkgX64.fdf > @@ -397,6 +397,10 @@ INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf > INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > !endif > > +!if $(TPM2_ENABLE) == TRUE > +INF SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > +!endif > + > ################################################################################ > > [FV.FVMAIN_COMPACT] > Thanks! Laszlo