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

["Gerd Hoffmann" <kraxel@redhat.com>]

On Thu, Oct 21, 2021 at 12:13:51PM -0400, Stefan Berger wrote:
> 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.

I happily drop the option if it doesn't make sense.  I've already
wondered why it is there but assumed there is some valid reason for
it and left it as-is.

> - 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...

Exactly.  With the world moving to TPM 2 building OVMF with TPM 1.2 only
looks pointless to me.

> - 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.

With SHA1 being considered broken we want avoid SHA1 being used.
Ideally by removing support it altogether.  In case this is not possible
for backward compatibility reasons at least have it disabled by default.

So swtpm_setup not enabling the SHA1 bank by default is certainly a good
idea and a move into the right direction (independent from the patch #4
discussion).

Didn't do much testing yet to see whenever removing SHA1 support
altogether trips up operating systems.

> - 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.

I can pick this up for v2 if you don't mind.

take care,
  Gerd