Hi Heinrich,
On Fri, 14 Jun 2024 at 12:04, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > On 14.06.24 09:01, Ilias Apalodimas wrote: > > On Fri, 14 Jun 2024 at 09:59, Heinrich Schuchardt <xypron.g...@gmx.de> > > wrote: > >> > >> On 6/14/24 08:03, Ilias Apalodimas wrote: > >>> Hi Simon, > >>> > >>> On Mon, 10 Jun 2024 at 17:59, Simon Glass <s...@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 <s...@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 leave the TPM in a working state tbh. > > 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. > > 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.g...@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 > >>>> > >> >