* [PATCH 3/3] tpm_tcg2: hash algo optimization @ 2024-07-15 13:33 Benjamin BARATTE 2024-07-29 14:09 ` Ilias Apalodimas 0 siblings, 1 reply; 6+ messages in thread From: Benjamin BARATTE @ 2024-07-15 13:33 UTC (permalink / raw) To: u-boot@lists.denx.de Cc: akashi.tkhro@gmail.com, abdellatif.elkhlifi@arm.com, eajames@linux.ibm.com, xypron.glpk@gmx.de, kojima.masahisa@socionext.com, sjg@chromium.org, sughosh.ganu@linaro.org, tharvey@gateworks.com, trini@konsulko.com 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) Signed-off-by: Benjamin BARATTE <benjamin.baratte@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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] tpm_tcg2: hash algo optimization 2024-07-15 13:33 [PATCH 3/3] tpm_tcg2: hash algo optimization Benjamin BARATTE @ 2024-07-29 14:09 ` Ilias Apalodimas 2024-08-23 12:29 ` Benjamin BARATTE 0 siblings, 1 reply; 6+ messages in thread From: Ilias Apalodimas @ 2024-07-29 14:09 UTC (permalink / raw) To: Benjamin BARATTE Cc: u-boot@lists.denx.de, akashi.tkhro@gmail.com, abdellatif.elkhlifi@arm.com, eajames@linux.ibm.com, xypron.glpk@gmx.de, kojima.masahisa@socionext.com, sjg@chromium.org, sughosh.ganu@linaro.org, tharvey@gateworks.com, trini@konsulko.com 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. Thanks /Ilias > > Signed-off-by: Benjamin BARATTE <benjamin.baratte@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 3/3] tpm_tcg2: hash algo optimization 2024-07-29 14:09 ` Ilias Apalodimas @ 2024-08-23 12:29 ` Benjamin BARATTE 2024-08-27 8:04 ` Ilias Apalodimas 0 siblings, 1 reply; 6+ messages in thread From: Benjamin BARATTE @ 2024-08-23 12:29 UTC (permalink / raw) To: Ilias Apalodimas Cc: u-boot@lists.denx.de, akashi.tkhro@gmail.com, abdellatif.elkhlifi@arm.com, eajames@linux.ibm.com, xypron.glpk@gmx.de, kojima.masahisa@socionext.com, sjg@chromium.org, sughosh.ganu@linaro.org, tharvey@gateworks.com, trini@konsulko.com Hi @Ilias Apalodimas, ST Restricted > -----Original Message----- > From: Ilias Apalodimas <ilias.apalodimas@linaro.org> > Sent: Monday, July 29, 2024 4:10 PM > To: Benjamin BARATTE <benjamin.baratte@st.com> > Cc: u-boot@lists.denx.de; akashi.tkhro@gmail.com; > abdellatif.elkhlifi@arm.com; eajames@linux.ibm.com; xypron.glpk@gmx.de; > kojima.masahisa@socionext.com; sjg@chromium.org; > sughosh.ganu@linaro.org; tharvey@gateworks.com; trini@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. And U-boot API to configure the TPM PCR will be more than welcome in U-boot. This will allow industrial user to manage this configuration in U-boot instead of Linux. Best Regards, Benjamin > Thanks > /Ilias > > > > > Signed-off-by: Benjamin BARATTE <benjamin.baratte@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] tpm_tcg2: hash algo optimization 2024-08-23 12:29 ` Benjamin BARATTE @ 2024-08-27 8:04 ` Ilias Apalodimas 2024-09-06 14:21 ` Benjamin BARATTE 0 siblings, 1 reply; 6+ messages in thread From: Ilias Apalodimas @ 2024-08-27 8:04 UTC (permalink / raw) To: Benjamin BARATTE Cc: u-boot@lists.denx.de, akashi.tkhro@gmail.com, abdellatif.elkhlifi@arm.com, eajames@linux.ibm.com, xypron.glpk@gmx.de, kojima.masahisa@socionext.com, sjg@chromium.org, sughosh.ganu@linaro.org, tharvey@gateworks.com, trini@konsulko.com Hi Benjamin, On Fri, 23 Aug 2024 at 15:29, Benjamin BARATTE <benjamin.baratte@st.com> wrote: > > Hi @Ilias Apalodimas, > > > ST Restricted > > -----Original Message----- > > From: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > Sent: Monday, July 29, 2024 4:10 PM > > To: Benjamin BARATTE <benjamin.baratte@st.com> > > Cc: u-boot@lists.denx.de; akashi.tkhro@gmail.com; > > abdellatif.elkhlifi@arm.com; eajames@linux.ibm.com; xypron.glpk@gmx.de; > > kojima.masahisa@socionext.com; sjg@chromium.org; > > sughosh.ganu@linaro.org; tharvey@gateworks.com; trini@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@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 3/3] tpm_tcg2: hash algo optimization 2024-08-27 8:04 ` Ilias Apalodimas @ 2024-09-06 14:21 ` Benjamin BARATTE 2024-09-09 11:18 ` Ilias Apalodimas 0 siblings, 1 reply; 6+ messages in thread From: Benjamin BARATTE @ 2024-09-06 14:21 UTC (permalink / raw) To: Ilias Apalodimas Cc: u-boot@lists.denx.de, akashi.tkhro@gmail.com, abdellatif.elkhlifi@arm.com, eajames@linux.ibm.com, xypron.glpk@gmx.de, kojima.masahisa@socionext.com, sjg@chromium.org, sughosh.ganu@linaro.org, tharvey@gateworks.com, trini@konsulko.com Hi Ilas, > -----Original Message----- > From: Ilias Apalodimas <ilias.apalodimas@linaro.org> > Sent: Tuesday, August 27, 2024 10:05 AM > To: Benjamin BARATTE <benjamin.baratte@st.com> > Cc: u-boot@lists.denx.de; akashi.tkhro@gmail.com; > abdellatif.elkhlifi@arm.com; eajames@linux.ibm.com; xypron.glpk@gmx.de; > kojima.masahisa@socionext.com; sjg@chromium.org; > sughosh.ganu@linaro.org; tharvey@gateworks.com; trini@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@st.com> wrote: > > > > Hi @Ilias Apalodimas, > > > > > > ST Restricted > > > -----Original Message----- > > > From: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > Sent: Monday, July 29, 2024 4:10 PM > > > To: Benjamin BARATTE <benjamin.baratte@st.com> > > > Cc: u-boot@lists.denx.de; akashi.tkhro@gmail.com; > > > abdellatif.elkhlifi@arm.com; eajames@linux.ibm.com; > > > xypron.glpk@gmx.de; kojima.masahisa@socionext.com; > sjg@chromium.org; > > > sughosh.ganu@linaro.org; tharvey@gateworks.com; trini@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/ > 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@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] tpm_tcg2: hash algo optimization 2024-09-06 14:21 ` Benjamin BARATTE @ 2024-09-09 11:18 ` Ilias Apalodimas 0 siblings, 0 replies; 6+ messages in thread From: Ilias Apalodimas @ 2024-09-09 11:18 UTC (permalink / raw) To: Benjamin BARATTE Cc: u-boot@lists.denx.de, akashi.tkhro@gmail.com, abdellatif.elkhlifi@arm.com, eajames@linux.ibm.com, xypron.glpk@gmx.de, kojima.masahisa@socionext.com, sjg@chromium.org, sughosh.ganu@linaro.org, tharvey@gateworks.com, trini@konsulko.com Hi Benjamin On Fri, 6 Sept 2024 at 17:21, Benjamin BARATTE <benjamin.baratte@st.com> wrote: > > Hi Ilas, > > > -----Original Message----- > > From: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > Sent: Tuesday, August 27, 2024 10:05 AM > > To: Benjamin BARATTE <benjamin.baratte@st.com> > > Cc: u-boot@lists.denx.de; akashi.tkhro@gmail.com; > > abdellatif.elkhlifi@arm.com; eajames@linux.ibm.com; xypron.glpk@gmx.de; > > kojima.masahisa@socionext.com; sjg@chromium.org; > > sughosh.ganu@linaro.org; tharvey@gateworks.com; trini@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@st.com> wrote: > > > > > > Hi @Ilias Apalodimas, > > > > > > > > > ST Restricted > > > > -----Original Message----- > > > > From: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > Sent: Monday, July 29, 2024 4:10 PM > > > > To: Benjamin BARATTE <benjamin.baratte@st.com> > > > > Cc: u-boot@lists.denx.de; akashi.tkhro@gmail.com; > > > > abdellatif.elkhlifi@arm.com; eajames@linux.ibm.com; > > > > xypron.glpk@gmx.de; kojima.masahisa@socionext.com; > > sjg@chromium.org; > > > > sughosh.ganu@linaro.org; tharvey@gateworks.com; trini@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/ > > > > 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. Isn't e7505b3b8bcd enough? Thanks /Ilias > > Best Regards, > > Benjamin > > Thanks > > /Ilias > > > > > > > > > > Best Regards, > > > > > > Benjamin > > > > > > > Thanks > > > > /Ilias > > > > > > > > > > > > > > Signed-off-by: Benjamin BARATTE <benjamin.baratte@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-09 11:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-15 13:33 [PATCH 3/3] tpm_tcg2: hash algo optimization Benjamin BARATTE 2024-07-29 14:09 ` Ilias Apalodimas 2024-08-23 12:29 ` Benjamin BARATTE 2024-08-27 8:04 ` Ilias Apalodimas 2024-09-06 14:21 ` Benjamin BARATTE 2024-09-09 11:18 ` Ilias Apalodimas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox