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

[Ilias Apalodimas]

Hi Heinrich

resending the reply, I accidentally sent half of the message...

On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 14.06.24 09:01, Ilias Apalodimas wrote:
> > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>
> >> On 6/14/24 08:03, Ilias Apalodimas wrote:
> >>> Hi Simon,
> >>>
> >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass <sjg at chromium.org> 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. That board does use the TPM, but not with measured boot,
> >>>> nor EFI.
> >>>>
> >>>> Since EFI_TCG2_PROTOCOL already selects these options, we just need to
> >>>> add them to MEASURED_BOOT as well.
> >>>>
> >>>> 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>
> >>>> ---
> >>>>
> >>>> Changes in v2:
> >>>> - Put the conditions under EFI_TCG2_PROTOCOL
> >>>> - Consider MEASURED_BOOT too
> >>>>
> >>>>    boot/Kconfig | 4 ++++
> >>>>    lib/Kconfig  | 4 ----
> >>>>    2 files changed, 4 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/boot/Kconfig b/boot/Kconfig
> >>>> index 6f3096c15a6..b061891e109 100644
> >>>> --- a/boot/Kconfig
> >>>> +++ b/boot/Kconfig
> >>>> @@ -734,6 +734,10 @@ config LEGACY_IMAGE_FORMAT
> >>>>    config MEASURED_BOOT
> >>>>           bool "Measure boot images and configuration when booting without EFI"
> >>>>           depends on HASH && TPM_V2
> >>>> +       select SHA1
> >>>> +       select SHA256
> >>>> +       select SHA384
> >>>> +       select SHA512
> >>>>           help
> >>>>             This option enables measurement of the boot process when booting
> >>>>             without UEFI . Measurement involves creating cryptographic hashes
> >>>> diff --git a/lib/Kconfig b/lib/Kconfig
> >>>> index 189e6eb31aa..568892fce44 100644
> >>>> --- a/lib/Kconfig
> >>>> +++ b/lib/Kconfig
> >>>> @@ -438,10 +438,6 @@ config TPM
> >>>>           bool "Trusted Platform Module (TPM) Support"
> >>>>           depends on DM
> >>>>           imply DM_RNG
> >>>> -       select SHA1
> >>>> -       select SHA256
> >>>> -       select SHA384
> >>>> -       select SHA512
> >>>
> >>> I am not sure this is the right way to deal with your problem.
> >>> The TPM main functionality is to measure and extend PCRs, so shaXXXX
> >>> is really required. To make things even worse, you don't know the PCR
> >>> banks that are enabled beforehand. This is a runtime config of the
> >>> TPM.
> >>
> >> If neither MEASURED_BOOT nor EFI_TCG2_PROTOCOL is selected, U-Boot
> >> cannot extend PCRs. So it seems fine to let these two select the
> >> complete set of hashing algorithms. As Simon pointed out for
> >> EFI_TCG2_PROTOCOL this is already done in lib/efi_loader/Kconfig.
> >
> > It can. The cmd we have can extend those pcrs -- e.g tpm2 pcr_extend 8
> > 0xb0000000
>
> So this patch should also consider CMD_TPM_V2 and CMD_TPM_V1.
>
> TPM v1 only needs SHA-1.

I still prefer to imply all algos.

>
> In cmd/tpm-v2.c do_tpm2_pcr_extend() and do_tpm_pcr_read() assume
> SHA256. Function tpm_pcr_extend() shows the same limitation. This bug
> should be fixed. But as is CMD_TPM_V2 seems only to require CONFIG_SHA256.

Isn't [0] fixing this?

[0] https://source.denx.de/u-boot/u-boot/-/commit/89aa8463cdf3919ca4f04fc24ec8b154ff56d97e
Thanks
/Ilias
>
> Best regards
>
> Heinrich
>
> >
> > Regards
> > /Ilias
> >>
> >> Even if U-Boot does not support measured boot (EFI or non-EFI) we might
> >> still be using the TPMs RNG.
> >>
> >> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> >>
> >>>
> >>>    So this would make the TPM pretty useless. Can't you remove something
> >>> that doesn't break functionality?
> >>>
> >>> Thanks
> >>> /Ilias
> >>>>           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
> >>>> --
> >>>> 2.34.1
> >>>>
> >>
>