Re: [PATCH 0/4] OvmfPkg: rework TPM configuration.

["Stefan Berger" <stefanb@linux.ibm.com>]

A few more comments to this series:

- Is there a use case where TPM2_ENABLE_CONFIG is disabled, meaning 
where there should not be a TPM 2 menu entry? It's worth considering 
dropping this option because a user does need to have control over 
certain aspects of the TPM 2 configuration. Most of this control may be 
reachable via the physical presence interface (PPI) inside the VM where 
root can write codes into the /sys/devices/.../ppi/request file to 
achieve similar outcomes, but it's really low level and I wouldn't know 
how to do this if on Windows for example or maybe BSD or other OSes 
running inside the VM.

- Should it be possible to enable TPM 1.2 independent of TPM 2? For me 
it's fine as-is since TPM 2 is mostly used these days...

- I would drop patch 4 if it means that an active SHA1 bank doesn't get 
PCR extensions (haven't tested yet). swtpm_setup currently sets up a 
swtpm with active SHA1 and SHA256 PCR banks ( 
https://github.com/stefanberger/swtpm/blob/master/src/swtpm_setup/swtpm_setup.c#L65 
). We can change this for swtpm v0.7.0 to only activate the SHA256 bank, 
if that's what is needed here. However, this doesn't prevent a user to 
activate the SHA1 PCR bank either via PPI 'request' file or UEFI TPM 
menu and when it is active it must get PCR extensions.

- Since TPM 1.2 is still supported we need to add a TPM menu for it as 
well using this patch here. I would put this under the TPM1_ENABLE 
config option since having TPM 1.2 support without a menu is quite 
useless. I can send a patch for this once this series has gone through.

diff --git a/OvmfPkg/OvmfTpmComponentsDxe.dsc.inc 
b/OvmfPkg/OvmfTpmComponentsDxe.dsc.inc
index 6806eb245e..43acd2c755 100644
--- a/OvmfPkg/OvmfTpmComponentsDxe.dsc.inc
+++ b/OvmfPkg/OvmfTpmComponentsDxe.dsc.inc
@@ -22,6 +22,7 @@
      <LibraryClasses>
Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
    }
+  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf^M
  !endif
    SecurityPkg/Tcg/Tcg2PlatformDxe/Tcg2PlatformDxe.inf {
      <LibraryClasses>
diff --git a/OvmfPkg/OvmfTpmDxe.fdf.inc b/OvmfPkg/OvmfTpmDxe.fdf.inc
index fa74972678..d22e069af0 100644
--- a/OvmfPkg/OvmfTpmDxe.fdf.inc
+++ b/OvmfPkg/OvmfTpmDxe.fdf.inc
@@ -5,6 +5,7 @@
  !if $(TPM2_ENABLE) == TRUE
  !if $(TPM1_ENABLE) == TRUE
  INF  SecurityPkg/Tcg/TcgDxe/TcgDxe.inf
+INF  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf^M
  !endif
  INF  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
  INF  SecurityPkg/Tcg/Tcg2PlatformDxe/Tcg2PlatformDxe.inf

    Stefan


On 10/21/21 8:19 AM, Gerd Hoffmann wrote:
> Allows to enable/disable TPM 1.2 support in OVMF.
> Allows to enable SHA-1 support for TPM hashing.
>
> Gerd Hoffmann (4):
>    OvmfPkg: move tcg configuration to dsc and fdf include files
>    OvmfPkg: create Tcg2ConfigPeiCompat12.inf
>    OvmfPkg: rework TPM configuration
>    OvmfPkg: add TPM2_SHA1_ENABLE build option
>
>   OvmfPkg/OvmfTpmComponentsDxe.dsc.inc          | 32 +++++++
>   OvmfPkg/OvmfTpmComponentsPei.dsc.inc          | 28 ++++++
>   OvmfPkg/OvmfTpmDefines.dsc.inc                | 10 +++
>   OvmfPkg/OvmfTpmLibs.dsc.inc                   | 16 ++++
>   OvmfPkg/OvmfTpmLibsDxe.dsc.inc                | 10 +++
>   OvmfPkg/OvmfTpmLibsPeim.dsc.inc               | 11 +++
>   OvmfPkg/OvmfTpmPcds.dsc.inc                   |  7 ++
>   OvmfPkg/OvmfTpmPcdsHii.dsc.inc                |  8 ++
>   OvmfPkg/OvmfTpmSecurityStub.dsc.inc           | 10 +++
>   OvmfPkg/AmdSev/AmdSevX64.dsc                  | 85 +++---------------
>   OvmfPkg/OvmfPkgIa32.dsc                       | 88 +++----------------
>   OvmfPkg/OvmfPkgIa32X64.dsc                    | 85 +++---------------
>   OvmfPkg/OvmfPkgX64.dsc                        | 85 +++---------------
>   OvmfPkg/AmdSev/AmdSevX64.fdf                  | 17 +---
>   OvmfPkg/OvmfPkgIa32.fdf                       | 17 +---
>   OvmfPkg/OvmfPkgIa32X64.fdf                    | 17 +---
>   OvmfPkg/OvmfPkgX64.fdf                        | 17 +---
>   OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf      |  9 --
>   ...onfigPei.inf => Tcg2ConfigPeiCompat12.inf} |  9 +-
>   OvmfPkg/OvmfTpmDxe.fdf.inc                    | 14 +++
>   OvmfPkg/OvmfTpmPei.fdf.inc                    | 15 ++++
>   .../.azurepipelines/Ubuntu-GCC5.yml           |  6 +-
>   .../.azurepipelines/Windows-VS2019.yml        |  6 +-
>   OvmfPkg/PlatformCI/ReadMe.md                  |  2 +-
>   24 files changed, 221 insertions(+), 383 deletions(-)
>   create mode 100644 OvmfPkg/OvmfTpmComponentsDxe.dsc.inc
>   create mode 100644 OvmfPkg/OvmfTpmComponentsPei.dsc.inc
>   create mode 100644 OvmfPkg/OvmfTpmDefines.dsc.inc
>   create mode 100644 OvmfPkg/OvmfTpmLibs.dsc.inc
>   create mode 100644 OvmfPkg/OvmfTpmLibsDxe.dsc.inc
>   create mode 100644 OvmfPkg/OvmfTpmLibsPeim.dsc.inc
>   create mode 100644 OvmfPkg/OvmfTpmPcds.dsc.inc
>   create mode 100644 OvmfPkg/OvmfTpmPcdsHii.dsc.inc
>   create mode 100644 OvmfPkg/OvmfTpmSecurityStub.dsc.inc
>   copy OvmfPkg/Tcg/Tcg2Config/{Tcg2ConfigPei.inf => Tcg2ConfigPeiCompat12.inf} (84%)
>   create mode 100644 OvmfPkg/OvmfTpmDxe.fdf.inc
>   create mode 100644 OvmfPkg/OvmfTpmPei.fdf.inc
>