Re: [PATCH 2/9] tpm: Avoid code bloat when not using EFI_TCG2_PROTOCOL

Hi Heinrich

On Wed, 5 Jun 2024 at 07:09, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 6/5/24 05:25, Simon Glass wrote:

...

It does not make sense to enable all SHA algorithms unless they are needed. It bloats the code and in this case, causes chromebook_link to fail to build.

Why would chromebook_link fail to build? Is TPM used by U-Boot on that board at all?

...

Add a condition to TPM to correct this. Note that the original commit combines refactoring and new features, which makes it hard to see what is going on.

Fixes: 97707f12fda tpm: Support boot measurements

Signed-off-by: Simon Glass sjg@chromium.org

lib/Kconfig | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/Kconfig b/lib/Kconfig index 189e6eb31aa..70b32362ada 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -438,10 +438,10 @@ config TPM bool "Trusted Platform Module (TPM) Support" depends on DM imply DM_RNG

• select SHA1
  

• select SHA256
  

• select SHA384
  

• select SHA512
  

• select SHA1 if EFI_TCG2_PROTOCOL
  

• select SHA256 if EFI_TCG2_PROTOCOL
  

• select SHA384 if EFI_TCG2_PROTOCOL
  

• select SHA512 if EFI_TCG2_PROTOCOL
  

You need to consider CONFIG_MEASURED_BOOT which allows measured boot in the non-UEFI case.

Please, take into account

lib/tpm-v1.c:20: #error "TPM_AUTH_SESSIONS require SHA1 to be configured, too"

This #error should be replaced by a Kconfig constraint.

I would prefer the select statements to be in lib/efi_loader/Kconfig under EFI_TCG2_PROTOCOL.

@Ilias, Eddie:

Why do we require SHA1 which is considered insecure?

Shouldn't we change tpm2_supported_algorithms[] to include only algorithms selected in the configuration? This would allow replacing all the select statements in Simon's patch by imply.

The algorithms used and configured by the TPM are device runtime decisions, not compile-time ones and to my knowledge, there are no devices out there that disable SHA1.

Failing to extend a PCR in an active bank is a security vulnerability. It would allow the unsealing of secrets if an attacker can replay a good set of measurements into an unused bank.

We could potentially fix that in the future since we can configure the TPM active banks on boot, but IIRC we don't support that yet.

Regards /Ilias

Best regards

Heinrich

...

  help
    This enables support for TPMs which can be used to provide security
    features for your board. The TPM can be connected via LPC or I2C