public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
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
Subject: Re: [PATCH 5/7] ovmf: link with Tcg2Dxe module
Date: Mon, 26 Feb 2018 10:50:49 +0100	[thread overview]
Message-ID: <53a8a301-0719-aca9-7b4c-90df90ccfda0@redhat.com> (raw)
In-Reply-To: <20180223132311.26555-6-marcandre.lureau@redhat.com>

On 02/23/18 14:23, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> 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 <lersek@redhat.com>
> CC: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  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 {
> +    <LibraryClasses>
> +      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


  reply	other threads:[~2018-02-26  9:44 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-23 13:23 [PATCH 0/7] RFC: ovmf: preliminary TPM2 support marcandre.lureau
2018-02-23 13:23 ` [PATCH 1/7] SecurityPkg/Tcg2Pei: drop Tcg2PhysicalPresenceLib dependency marcandre.lureau
2018-02-23 15:58   ` Laszlo Ersek
2018-02-24  0:09   ` Yao, Jiewen
2018-03-02 14:34     ` Laszlo Ersek
2018-02-23 13:23 ` [PATCH 2/7] ovmf: link with Tcg2ConfigPei module marcandre.lureau
2018-02-23 17:31   ` Laszlo Ersek
2018-03-01 14:59     ` Marc-André Lureau
2018-03-02 10:50       ` Laszlo Ersek
2018-02-23 13:23 ` [PATCH 3/7] HACK: HobLib: workaround infinite loop marcandre.lureau
2018-02-23 19:14   ` Laszlo Ersek
2018-02-23 19:45   ` Andrew Fish
2018-03-05 14:05     ` Marc-André Lureau
2018-03-05 18:22       ` Laszlo Ersek
2018-03-05 20:18         ` Andrew Fish
2018-03-06  0:45         ` Brian J. Johnson
2018-03-06  8:38           ` Laszlo Ersek
2018-03-06  2:02         ` Gao, Liming
2018-02-23 13:23 ` [PATCH 4/7] ovmf: link with Tcg2Pei module marcandre.lureau
2018-02-26  9:38   ` Laszlo Ersek
2018-03-01 15:08     ` Marc-André Lureau
2018-03-02 10:51       ` Laszlo Ersek
2018-02-23 13:23 ` [PATCH 5/7] ovmf: link with Tcg2Dxe module marcandre.lureau
2018-02-26  9:50   ` Laszlo Ersek [this message]
2018-03-05 15:45     ` Marc-André Lureau
2018-03-05 19:25       ` Laszlo Ersek
2018-02-23 13:23 ` [PATCH 6/7] ovmf: link with Tcg2ConfigDxe module marcandre.lureau
2018-02-26  9:58   ` Laszlo Ersek
2018-03-01 16:59     ` Stefan Berger
2018-03-02 11:12       ` Laszlo Ersek
2018-03-02 13:35         ` [Qemu-devel] " Stefan Berger
2018-02-23 13:23 ` [PATCH 7/7] ovmf: add DxeTpm2MeasureBootLib marcandre.lureau
2018-02-26 10:29   ` Laszlo Ersek
2018-02-23 15:55 ` [PATCH 0/7] RFC: ovmf: preliminary TPM2 support Laszlo Ersek
2018-03-01 16:36   ` [Qemu-devel] " Stefan Berger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53a8a301-0719-aca9-7b4c-90df90ccfda0@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox