[PATCH 3/3] tpm_tcg2: hash algo optimization

Ilias Apalodimas ilias.apalodimas at linaro.org
Tue Aug 27 10:04:55 CEST 2024


Hi Benjamin,


On Fri, 23 Aug 2024 at 15:29, Benjamin BARATTE <benjamin.baratte at st.com> wrote:
>
> Hi @Ilias Apalodimas,
>
>
> ST Restricted
> > -----Original Message-----
> > From: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > Sent: Monday, July 29, 2024 4:10 PM
> > To: Benjamin BARATTE <benjamin.baratte at st.com>
> > Cc: u-boot at lists.denx.de; akashi.tkhro at gmail.com;
> > abdellatif.elkhlifi at arm.com; eajames at linux.ibm.com; xypron.glpk at gmx.de;
> > kojima.masahisa at socionext.com; sjg at chromium.org;
> > sughosh.ganu at linaro.org; tharvey at gateworks.com; trini at konsulko.com
> > Subject: Re: [PATCH 3/3] tpm_tcg2: hash algo optimization
> >
> > On Mon, Jul 15, 2024 at 01:33:19PM +0000, Benjamin BARATTE wrote:
> > > To properly enable code optimization with hash algorithm, all the
> > > reference of the hash algo should condition to hash enablement.
> > > It is possible to fine tune the list of hash algorithms based on dTPM
> > > configuration.
> > > Therefore if dTPM supports only one bank, on one hash algorithm could
> > > be selected (TCG PTP specification mention in case of single bank
> > > support that SHA256 must be support and default value)
> >
> > Yes, but...
> > In order to apply this we need a function that *configures* the TPM with only
> > the supported compiled algorithms.  If a TPM is configured with more than
> > what u-boot supports, there might be security implications since we will fail to
> > cap all active PCRs. On top of that the EFI TCG explicitly says that all the active
> > PCR banks needs to be updated when extending measurements.
> >
>
> I'm agree with you, but this is more a configuration issue in that case.
> Today to do such configuration, we need to do it with tpm2-tools under Linux
> But if U-boot is preventing the Linux kernel to boot, this will become problematic.

It's the same configuration issue for u-boot. If you enable all
hashing algorithms Linux will be just fine.

> And U-boot API to configure the TPM PCR will be more than welcome in U-boot.

Configuring the TPM PCR available banks shouldnt be too much code, we
already have a function to read the available ones.

>
> This will allow industrial user to manage this configuration in U-boot instead of Linux.

Failing to cap all available PCR banks might lead to misconfiguration
errors and eventually compromise the TPM security -- e.g unseal keys
you shouldnt. There's an extended discussion about the same issue here
[0]. Keeping that in mind, I don't want to pick patches that allow
people to shoot themselves in the foot.

[0] https://lore.kernel.org/linux-integrity/20240531010331.134441-7-ross.philipson@oracle.com/

Thanks
/Ilias


>
> Best Regards,
>
> Benjamin
>
> > Thanks
> > /Ilias
> >
> > >
> > > Signed-off-by: Benjamin BARATTE <benjamin.baratte at st.com>
> > > ---
> > >
> > >  include/tpm-v2.h       |  8 --------
> > >  lib/efi_loader/Kconfig |  4 ----
> > >  lib/tpm_tcg2.c         | 38 ++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 38 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/include/tpm-v2.h b/include/tpm-v2.h index
> > > 9848e1fd10..ec3504de44 100644
> > > --- a/include/tpm-v2.h
> > > +++ b/include/tpm-v2.h
> > > @@ -285,38 +285,30 @@ struct digest_info {
> > >
> > >
> > >  static const struct digest_info hash_algo_list[] = { -#if
> > > IS_ENABLED(CONFIG_SHA1)
> > >         {
> > >                 "sha1",
> > >                 TPM2_ALG_SHA1,
> > >                 TCG2_BOOT_HASH_ALG_SHA1,
> > >                 TPM2_SHA1_DIGEST_SIZE,
> > >         },
> > > -#endif
> > > -#if IS_ENABLED(CONFIG_SHA256)
> > >         {
> > >                 "sha256",
> > >                 TPM2_ALG_SHA256,
> > >                 TCG2_BOOT_HASH_ALG_SHA256,
> > >                 TPM2_SHA256_DIGEST_SIZE,
> > >         },
> > > -#endif
> > > -#if IS_ENABLED(CONFIG_SHA384)
> > >         {
> > >                 "sha384",
> > >                 TPM2_ALG_SHA384,
> > >                 TCG2_BOOT_HASH_ALG_SHA384,
> > >                 TPM2_SHA384_DIGEST_SIZE,
> > >         },
> > > -#endif
> > > -#if IS_ENABLED(CONFIG_SHA512)
> > >         {
> > >                 "sha512",
> > >                 TPM2_ALG_SHA512,
> > >                 TCG2_BOOT_HASH_ALG_SHA512,
> > >                 TPM2_SHA512_DIGEST_SIZE,
> > >         },
> > > -#endif
> > >         {
> > >                 "sha3_256",
> > >                 TPM2_ALG_SHA3_256,
> > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index
> > > ee71f41714..512fb710b5 100644
> > > --- a/lib/efi_loader/Kconfig
> > > +++ b/lib/efi_loader/Kconfig
> > > @@ -405,10 +405,6 @@ config EFI_TCG2_PROTOCOL
> > >         bool "EFI_TCG2_PROTOCOL support"
> > >         default y
> > >         depends on TPM_V2
> > > -       select SHA1
> > > -       select SHA256
> > > -       select SHA384
> > > -       select SHA512
> > >         select HASH
> > >         select SMBIOS_PARSER
> > >         help
> > > diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c index
> > > 7f868cc883..66573b838d 100644
> > > --- a/lib/tpm_tcg2.c
> > > +++ b/lib/tpm_tcg2.c
> > > @@ -96,9 +96,15 @@ int tcg2_create_digest(struct udevice *dev, const u8
> > *input, u32 length,
> > >                        struct tpml_digest_values *digest_list)  {
> > >         u8 final[sizeof(union tpmu_ha)];
> > > +#if IS_ENABLED(CONFIG_SHA256)
> > >         sha256_context ctx_256;
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA384) || IS_ENABLED(CONFIG_SHA512)
> > >         sha512_context ctx_512;
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA1)
> > >         sha1_context ctx;
> > > +#endif
> > >         u32 active;
> > >         size_t i;
> > >         u32 len;
> > > @@ -114,30 +120,38 @@ int tcg2_create_digest(struct udevice *dev, const
> > u8 *input, u32 length,
> > >                         continue;
> > >
> > >                 switch (hash_algo_list[i].hash_alg) {
> > > +#if IS_ENABLED(CONFIG_SHA1)
> > >                 case TPM2_ALG_SHA1:
> > >                         sha1_starts(&ctx);
> > >                         sha1_update(&ctx, input, length);
> > >                         sha1_finish(&ctx, final);
> > >                         len = TPM2_SHA1_DIGEST_SIZE;
> > >                         break;
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA256)
> > >                 case TPM2_ALG_SHA256:
> > >                         sha256_starts(&ctx_256);
> > >                         sha256_update(&ctx_256, input, length);
> > >                         sha256_finish(&ctx_256, final);
> > >                         len = TPM2_SHA256_DIGEST_SIZE;
> > >                         break;
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA384)
> > >                 case TPM2_ALG_SHA384:
> > >                         sha384_starts(&ctx_512);
> > >                         sha384_update(&ctx_512, input, length);
> > >                         sha384_finish(&ctx_512, final);
> > >                         len = TPM2_SHA384_DIGEST_SIZE;
> > >                         break;
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA512)
> > >                 case TPM2_ALG_SHA512:
> > >                         sha512_starts(&ctx_512);
> > >                         sha512_update(&ctx_512, input, length);
> > >                         sha512_finish(&ctx_512, final);
> > >                         len = TPM2_SHA512_DIGEST_SIZE;
> > >                         break;
> > > +#endif
> > >                 default:
> > >                         printf("%s: unsupported algorithm %x\n", __func__,
> > >                                hash_algo_list[i].hash_alg); @@ -236,10
> > > +250,18 @@ static int tcg2_log_init(struct udevice *dev, struct
> > tcg2_event_log *elog)
> > >                         continue;
> > >
> > >                 switch (hash_algo_list[i].hash_alg) {
> > > +#if IS_ENABLED(CONFIG_SHA1)
> > >                 case TPM2_ALG_SHA1:
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA256)
> > >                 case TPM2_ALG_SHA256:
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA384)
> > >                 case TPM2_ALG_SHA384:
> > > +#endif
> > > +#if  IS_ENABLED(CONFIG_SHA512)
> > >                 case TPM2_ALG_SHA512:
> > > +#endif
> > >                         count++;
> > >                         break;
> > >                 default:
> > > @@ -337,10 +359,18 @@ static int tcg2_replay_eventlog(struct
> > tcg2_event_log *elog,
> > >                         algo = get_unaligned_le16(log + pos);
> > >                         pos += offsetof(struct tpmt_ha, digest);
> > >                         switch (algo) {
> > > +#if IS_ENABLED(CONFIG_SHA1)
> > >                         case TPM2_ALG_SHA1:
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA256)
> > >                         case TPM2_ALG_SHA256:
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA384)
> > >                         case TPM2_ALG_SHA384:
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA512)
> > >                         case TPM2_ALG_SHA512:
> > > +#endif
> > >                                 len = tpm2_algorithm_to_len(algo);
> > >                                 break;
> > >                         default:
> > > @@ -450,10 +480,18 @@ static int tcg2_log_parse(struct udevice *dev,
> > struct tcg2_event_log *elog)
> > >                         return 0;
> > >
> > >                 switch (algo) {
> > > +#if IS_ENABLED(CONFIG_SHA1)
> > >                 case TPM2_ALG_SHA1:
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA256)
> > >                 case TPM2_ALG_SHA256:
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA384)
> > >                 case TPM2_ALG_SHA384:
> > > +#endif
> > > +#if IS_ENABLED(CONFIG_SHA512)
> > >                 case TPM2_ALG_SHA512:
> > > +#endif
> > >                         len = get_unaligned_le16(&event->digest_sizes[i].digest_size);
> > >                         if (tpm2_algorithm_to_len(algo) != len)
> > >                                 return 0;
> > > --
> > > 2.34.1
> > >
> > > ST Restricted


More information about the U-Boot mailing list