From: "Marc-André Lureau" <marcandre.lureau@gmail.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: edk2-devel@lists.01.org, Peter Jones <pjones@redhat.com>,
Jiewen Yao <jiewen.yao@intel.com>, QEMU <qemu-devel@nongnu.org>,
Javier Martinez Canillas <javierm@redhat.com>
Subject: Re: [PATCH 5/7] ovmf: link with Tcg2Dxe module
Date: Mon, 5 Mar 2018 16:45:50 +0100 [thread overview]
Message-ID: <CAJ+F1CLEiCtK4-gxb4NhFRn28Lzyy_-GFYezk=MKpzcTYTEqwA@mail.gmail.com> (raw)
In-Reply-To: <53a8a301-0719-aca9-7b4c-90df90ccfda0@redhat.com>
Hi
On Mon, Feb 26, 2018 at 10:50 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> 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.
>
indeed
>> + 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?
I followed the SecurityPkg.dsc, and tried some variants. This router
pattern can be found in other places, unfortunately, I can't explain
it. I can copy&paste your explanation if that helps.
>
>> + NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
>> + NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
>
> Again -- is SHA1 required?
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...
Any major drawback in keeping sha1 enabled? (same for Pei)
>> + 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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
thanks
--
Marc-André Lureau
next prev parent reply other threads:[~2018-03-05 15:39 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
2018-03-05 15:45 ` Marc-André Lureau [this message]
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='CAJ+F1CLEiCtK4-gxb4NhFRn28Lzyy_-GFYezk=MKpzcTYTEqwA@mail.gmail.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