Hi Stefan
Thank you very much for the work.

I would like to double confirm with you on several things:
1) S3 resume - According to security guideline, we can randomize platform 
hiearachy if S3 start state fail.

REF: 
https://github.com/tianocore/edk2-platforms/blob/master/Platform/Intel/MinPlatformPkg/Tcg/Tcg2PlatformPei/Tcg2PlatformPei.c

But I did not see your S3 solution there.

2) I am curious, why you don't use a DXE driver, but choose to like to BDS lib 
for the DXE case.
You also include a NULL lib there, which seems unnecessary, if you use a 
DXE/PEI module.

The downside of linking to BDS lib is that you have to change all BDS lib 
instance, which is a big burden.
And you still have code to choose NULL lib v.s. real Lib based upon TPM enable 
flag.

How about just include Tcg2PlatformPei/Tcg2PlatformDxe to securityPkg as well? 
Then we can remove the TcgPlatform from MinPlatform totally.

3) In some platform, you add TpmPlatformHierarchyLib to Tcg2Dxe. Would you 
please help me understand why?

  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
    <LibraryClasses>
      
Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf
      
TpmPlatformHierarchyLib|SecurityPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.inf
      NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf




> -----Original Message-----
> From: Stefan Berger <stef...@linux.vnet.ibm.com>
> Sent: Thursday, September 2, 2021 5:22 AM
> To: devel@edk2.groups.io
> Cc: mhaeu...@posteo.de; spbro...@outlook.com;
> marcandre.lur...@redhat.com; kra...@redhat.com; Yao, Jiewen
> <jiewen....@intel.com>; Stefan Berger <stef...@linux.vnet.ibm.com>
> Subject: [PATCH v5 0/8] Ovmf: Disable the TPM2 platform hierarchy
> 
> This series imports code from the edk2-platforms project related to
> disabling the TPM2 platform hierarchy in Ovmf and ArmVirtPkg. It
> addresses the Ovmf aspects of the following bugs:
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3510
> https://bugzilla.tianocore.org/show_bug.cgi?id=3499
> 
> I have patched the .dsc files and successfully test-built with most of
> them. Some I could not build because they failed for other reasons
> unrelated to this series.
> 
> I tested the changes with QEMU on x86 following the build of
> ArmVirtQemu.dsc and OvmfPkgX64.dsc.
> 
> The disablement of the platform hierarchy is done after possibly
> handling PPI. Following TPM 2 logs on Arm, only PCR extensions are
> following afterwards until GRUB takes over.
> 
> Neither one of the following commands should work anymore on first
> try:
> 
> With IBM tss2 tools:
> tsshierarchychangeauth -hi p -pwdn newpass
> 
> With Intel tss2 tools:
> tpm2_changeauth -c platform newpass
> 
> Regards,
>   Stefan
> 
> v5:
>  - Modified patch 1 copies the code from edk2-platforms
>  - Modified patch 2 fixes bugs in the code
>  - Modified patch 4 introduces required PCD
> 
> v4:
>  - Fixed and simplified code imported from edk2-platforms
> 
> v3:
>  - Referencing Null implementation on Bhyve and Xen platforms
>  - Add support in ArmVirtPkg
> 
> 
> Stefan Berger (8):
>   SecurityPkg/TPM: Import PeiDxeTpmPlatformHierarchyLib.c from
>     edk2-platforms
>   SecurityPkg/TPM: Fix bugs in imported PeiDxeTpmPlatformHierarchyLib
>   SecurityPkg/TPM: Add a NULL implementation of TpmPlatformHierarchyLib
>   SecurityPkg: Introduce new PCD PcdRandomizePlatformHierarchy
>   OvmfPkg: Reference new TPM classes in the build system for compilation
>   OvmfPkg: Disable the TPM2 platform hierarchy
>   ArmVirtPkg: Reference new TPM classes in the build system for
>     compilation
>   ArmVirtPkg: Disable the TPM2 platform hierarchy
> 
>  ArmVirtPkg/ArmVirtCloudHv.dsc                 |   1 +
>  ArmVirtPkg/ArmVirtQemu.dsc                    |   3 +
>  ArmVirtPkg/ArmVirtQemuKernel.dsc              |   1 +
>  ArmVirtPkg/ArmVirtXen.dsc                     |   1 +
>  .../PlatformBootManagerLib/PlatformBm.c       |   6 +
>  .../PlatformBootManagerLib.inf                |   2 +
>  OvmfPkg/AmdSev/AmdSevX64.dsc                  |   3 +
>  OvmfPkg/Bhyve/BhyveX64.dsc                    |   1 +
>  .../PlatformBootManagerLib/BdsPlatform.c      |   6 +
>  .../PlatformBootManagerLib.inf                |   1 +
>  .../PlatformBootManagerLibBhyve/BdsPlatform.c |   7 +
>  .../PlatformBootManagerLibGrub/BdsPlatform.c  |   7 +
>  OvmfPkg/OvmfPkgIa32.dsc                       |   3 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                    |   3 +
>  OvmfPkg/OvmfPkgX64.dsc                        |   3 +
>  OvmfPkg/OvmfXen.dsc                           |   1 +
>  .../Include/Library/TpmPlatformHierarchyLib.h |  27 ++
>  .../PeiDxeTpmPlatformHierarchyLib.c           | 255 ++++++++++++++++++
>  .../PeiDxeTpmPlatformHierarchyLib.inf         |  44 +++
>  .../PeiDxeTpmPlatformHierarchyLib.c           |  19 ++
>  .../PeiDxeTpmPlatformHierarchyLib.inf         |  31 +++
>  SecurityPkg/SecurityPkg.dec                   |   6 +
>  22 files changed, 431 insertions(+)
>  create mode 100644 SecurityPkg/Include/Library/TpmPlatformHierarchyLib.h
>  create mode 100644
> SecurityPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierar
> chyLib.c
>  create mode 100644
> SecurityPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierar
> chyLib.inf
>  create mode 100644
> SecurityPkg/Library/PeiDxeTpmPlatformHierarchyLibNull/PeiDxeTpmPlatformHi
> erarchyLib.c
>  create mode 100644
> SecurityPkg/Library/PeiDxeTpmPlatformHierarchyLibNull/PeiDxeTpmPlatformHi
> erarchyLib.inf
> 
> --
> 2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80267): https://edk2.groups.io/g/devel/message/80267
Mute This Topic: https://groups.io/mt/85316773/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to