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

[Ilias Apalodimas]

Hi Heinrich

On Wed, 5 Jun 2024 at 07:09, Heinrich Schuchardt <xypron.glpk at 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 at 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
>