[PATCH 3/3] tpm_tcg2: hash algo optimization
Benjamin BARATTE
benjamin.baratte at st.com
Fri Sep 6 16:21:04 CEST 2024
Hi Ilas,
> -----Original Message-----
> From: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> Sent: Tuesday, August 27, 2024 10:05 AM
> 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
>
> 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 at oracle.com/
>
OK understood.
What could be done is to generate an error if, at runtime, there is a mismatch between U-Boot compilation and TPM configuration.
Best Regards,
Benjamin
> 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