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 1222A21ED1C49 for ; Thu, 8 Mar 2018 11:08:04 -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 781924068051; Thu, 8 Mar 2018 19:14:19 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-50.rdu2.redhat.com [10.10.120.50]) by smtp.corp.redhat.com (Postfix) with ESMTP id E67F1202322A; Thu, 8 Mar 2018 19:14:13 +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: <20180307155746.18526-1-marcandre.lureau@redhat.com> <20180307155746.18526-8-marcandre.lureau@redhat.com> From: Laszlo Ersek Message-ID: Date: Thu, 8 Mar 2018 20:14:13 +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: <20180307155746.18526-8-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.5]); Thu, 08 Mar 2018 19:14:20 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Thu, 08 Mar 2018 19:14:20 +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 v2 7/8] 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: Thu, 08 Mar 2018 19:08:05 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 03/07/18 16:57, 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. > > The linux kernel doesn't yet read the EFI_TCG2_EVENT_LOG_FORMAT_TCG_2, > which is required for crypto-agile log. In fact, only upcoming 4.16 > adds support EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2: > > [ 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. > > Laszlo Ersek explained on the list why Tpm2DeviceLib has to be > resolved differently for DXE_DRIVER modules in general and for > "Tcg2Dxe.inf" specifically: > > * We have a library class called Tpm2DeviceLib -- this is basically the > set of APIs declared in "SecurityPkg/Include/Library/Tpm2DeviceLib.h". > Its leading comment says "This library abstract how to access TPM2 > hardware device". > > There are two *sets* of APIs in "Tpm2DeviceLib.h": > > (a) functions that deal with the TPM2 device: > - Tpm2RequestUseTpm(), > - Tpm2SubmitCommand() > > This set of APIs is supposed to be used by clients that *consume* > the TPM2 device abstraction. > > (b) the function Tpm2RegisterTpm2DeviceLib(), which is supposed to be > used by *providers* of various TPM2 device abstractions. > > * Then, we have two implementations (instances) of the Tpm2DeviceLib class: > (1) SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf > (2) SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf > > (1) The first library instance ("Tpm2DeviceLibTcg2.inf") implements the > APIs listed under (a), and it does not implement (b) -- see > EFI_UNSUPPORTED. In other words, this lib instance is strictly meant for > drivers that *consume* the TPM2 device abstraction. And, the (a) group > of APIs is implemented by forwarding the requests to the TCG2 protocol. > > The idea here is that all the drivers that consume the TPM2 abstraction > do not have to be statically linked with a large TPM2 device library > instance; instead they are only linked (statically) with this "thin" > library instance, and all the actual work is delegated to whichever > driver that provides the singleton TCG2 protocol. > > (2) The second library instance ("Tpm2DeviceLibRouterDxe.inf") is meant > for the driver that offers (produces) the TCG2 protocol. This lib > instance implements both (a) and (b) API groups. > > * Here's how things fit together: > > (i) The "SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf" > library instance (which has no lib class) is linked into "Tcg2Dxe.inf" > via NULL class resolution. This simply means that before the > "Tcg2Dxe.inf" entry point function is entered, the constructor function > of "Tpm2InstanceLibDTpm.inf" will be called. > > (ii) This Tpm2InstanceLibDTpmConstructor() function calls API (b), and > registers its own actual TPM2 command implementation with the > "Tpm2DeviceLibRouter" library instance (also linked into the Tcg2Dxe > driver). This provides the back-end for the API set (a). > > TCG2 protocol provider (Tcg2Dxe.inf driver) launches > | > v > NULL class: Tpm2InstanceLibDTpm instance construction > | > v > Tpm2DeviceLib class: Tpm2DeviceLibRouter instance > backend registration for API set (a) > > (iii) The Tcg2Dxe driver exposes the TCG2 protocol. > > (iv) A TPM2 consumer calls API set (a) via lib instance (1). Such calls > land in Tcg2Dxe, via the protocol. > > (v) Tcg2Dxe serves the protocol request by forwarding it to API set (a) > from lib instance (2). > > (vi) Those functions call the "backend" functions registered by > Tpm2DeviceLibDTpm in step (ii). > > TPM 2 consumer driver > | > v > Tpm2DeviceLib class: Tpm2DeviceLibTcg2 instance > | > v > TCG2 protocol interface > | > v > TCG2 protocol provider: Tcg2Dxe.inf driver > | > v > Tpm2DeviceLib class: Tpm2DeviceLibRouter instance > | > v > NULL class: Tpm2InstanceLibDTpm instance > (via earlier registration) > | > v > TPM2 chip (actual hardware) > > * So that is the "router" pattern in edk2. Namely, > > - Consumers of an abstraction use a thin library instance. > > - The thin library instance calls a firmware-global (singleton) service, > i.e. a PPI (in the PEI phase) or protocol (in the DXE phase). > > - The PEIM providing the PPI, or the DXE driver providing the protocol, > don't themselves implement the actual service either. Instead they > offer a "registration" service too, and they only connect the incoming > "consumer" calls to the earlier registered back-end(s). > > - The "registration service", for back-ends to use, may take various > forms. > > It can be exposed globally to the rest of the firmware, as > another member function of the PPI / protocol structure. Then backends > can be provided by separate PEIMs / DXE drivers. > > Or else, the registration service can be exposed as just another > library API. In this case, the backends are provided as NULL class > library instances, and a platform DSC file links them into the PEIM / > DXE driver via NULL class resolutions. The backend lib instances call > the registration service in their own respective constructor > functions. > > Cc: Laszlo Ersek > Cc: Stefan Berger > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Marc-André Lureau > --- > OvmfPkg/OvmfPkgX64.dsc | 16 ++++++++++++++++ > OvmfPkg/OvmfPkgX64.fdf | 4 ++++ > 2 files changed, 20 insertions(+) (1) Please change the subject line to: OvmfPkg: include Tcg2Dxe module > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 3fa1a31f4c37..7753852144fb 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -211,6 +211,8 @@ [LibraryClasses] > > !if $(TPM2_ENABLE) == TRUE > Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf > + Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf > + Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf > !endif > > [LibraryClasses.common] > @@ -362,6 +364,10 @@ [LibraryClasses.common.DXE_DRIVER] > 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 (2) Can you move this HashLib resolution under Tcg2Dxe.inf? > + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf > +!endif > > [LibraryClasses.common.UEFI_APPLICATION] > PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf > @@ -642,6 +648,16 @@ [Components] > > MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf > > +!if $(TPM2_ENABLE) == TRUE > + SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf { > + > + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf > + NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf > + NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf > + NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf > + } > +!endif > + > MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { > > !if $(SECURE_BOOT_ENABLE) == TRUE > diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf > index c0173e7adf5f..1a46104fc63d 100644 > --- a/OvmfPkg/OvmfPkgX64.fdf > +++ b/OvmfPkg/OvmfPkgX64.fdf > @@ -392,6 +392,10 @@ [FV.DXEFV] > INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > !endif > > +!if $(TPM2_ENABLE) == TRUE > +INF SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > +!endif > + (3) In the DSC file, you are adding Tcg2Dxe between RuntimeDxe and SecurityStubDxe; can you please stick with the same in the FDF file? It's easier to read. Alternatively, you can add Tcg2Dxe after VariableRuntimeDxe too, of course, but then the DSC file should follow suit. Makes no difference functionally, but it's easier to read. > ################################################################################ > > [FV.FVMAIN_COMPACT] > Thanks! Laszlo