[PATCH v2 00/11] Tpm exit with error when algorithm dismatches
This patch series add the logic to report errors when: 1. An eventlog is handed over from the previous boot stage but TPM device was configurated with an algorithm that does not exist in the eventlog. 2. TPM device was configurated with an algorithm that is not supported by U-Boot. 3. Failures observed when parsing the eventlog.
In all above cases, tpm_tcg2 should exit with error and prompt logs.
Moveover, this series include miscellaneous fixes and refactoring to simplify the logics in tpm and tpm_tcg2.
Ilias Apalodimas (8): efi_loader: Don't warn if the TCG2 FinalEvents table is not installed tpm: Rename tpm2_is_active_pcr() tpm: Rename tpm2_allow_extend() tpm: Don't create an EventLog if algorithms are misconfigured tpm: Keep the active PCRs in the chip private data tpm: Simplify tcg2_create_digest() tpm: Simplify tcg2_log_init() tpm: Don't replay an EventLog if tcg2_log_parse() fails
Raymond Mao (3): tpm: refactor tcg2_get_pcr_info() tpm: add flag in hash_algo_list and API to check if algorithm is supported tpm: add kconfig control in tcg2_create_digest()
include/tpm-common.h | 18 +++- include/tpm-v2.h | 65 ++++++++----- include/tpm_tcg2.h | 12 +-- lib/efi_loader/efi_tcg2.c | 2 +- lib/tpm-v2.c | 72 +++++++++++++-- lib/tpm_tcg2.c | 190 ++++++++++++++++++++------------------ 6 files changed, 230 insertions(+), 129 deletions(-)
From: Ilias Apalodimas ilias.apalodimas@linaro.org
When the TCG2 protocol installation fails, we are trying to remove all the objects we created in tcg2_uninit().
However, there are cases when this function runs before the config table was installed. So instead of printing an error unconditionally check against EFI_NOT_FOUND and don't print anything if the table wasn't installed to begin with.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - None.
lib/efi_loader/efi_tcg2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 572c6b5bf6..a15c73162e 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -791,7 +791,7 @@ static void tcg2_uninit(void) efi_status_t ret;
ret = efi_install_configuration_table(&efi_guid_final_events, NULL); - if (ret != EFI_SUCCESS) + if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND) log_err("Failed to delete final events config table\n");
efi_free_pool(event_log.buffer);
On 12/24/24 17:01, Raymond Mao wrote:
From: Ilias Apalodimas ilias.apalodimas@linaro.org
When the TCG2 protocol installation fails, we are trying to remove all the objects we created in tcg2_uninit().
However, there are cases when this function runs before the config table was installed. So instead of printing an error unconditionally check against EFI_NOT_FOUND and don't print anything if the table wasn't installed to begin with.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
None.
lib/efi_loader/efi_tcg2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 572c6b5bf6..a15c73162e 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -791,7 +791,7 @@ static void tcg2_uninit(void) efi_status_t ret;
ret = efi_install_configuration_table(&efi_guid_final_events, NULL);
- if (ret != EFI_SUCCESS)
- if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND) log_err("Failed to delete final events config table\n");
tcg2_uninit() is called after another problem has occurred.
The user probably is much more interested why efi_tcg2_register() failed but and less why this secondary issue occurred.
We can drop this message completely.
@Ilias, Raymond
Beyond this patch the following needs to be fixed:
The TCG2 driver should open the TCG2 protocol with BY_DRIVER to ensure that it cannot be uninstalled without taking down the driver.
Best regards
Heinrich
efi_free_pool(event_log.buffer);
From: Ilias Apalodimas ilias.apalodimas@linaro.org
This function is checking for active PCR banks, so rename it to something that's easier to read and closer to what the function does.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - None.
include/tpm-v2.h | 6 +++--- lib/tpm-v2.c | 4 ++-- lib/tpm_tcg2.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 4fd19c52fd..8c43f4fd9b 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -740,12 +740,12 @@ u16 tpm2_algorithm_to_len(enum tpm2_algorithms algo); bool tpm2_allow_extend(struct udevice *dev);
/** - * tpm2_is_active_pcr() - check the pcr_select. If at least one of the PCRs - * supports the algorithm add it on the active ones + * tpm2_is_active_bank() - check the pcr_select. If at least one of the PCRs + * supports the algorithm add it on the active ones * * @selection: PCR selection structure * Return: True if the algorithm is active */ -bool tpm2_is_active_pcr(struct tpms_pcr_selection *selection); +bool tpm2_is_active_bank(struct tpms_pcr_selection *selection);
#endif /* __TPM_V2_H */ diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index ad2b5ab0c3..cb636414de 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -847,7 +847,7 @@ u32 tpm2_enable_nvcommits(struct udevice *dev, uint vendor_cmd, return 0; }
-bool tpm2_is_active_pcr(struct tpms_pcr_selection *selection) +bool tpm2_is_active_bank(struct tpms_pcr_selection *selection) { int i;
@@ -907,7 +907,7 @@ bool tpm2_allow_extend(struct udevice *dev) return false;
for (i = 0; i < pcrs.count; i++) { - if (tpm2_is_active_pcr(&pcrs.selection[i]) && + if (tpm2_is_active_bank(&pcrs.selection[i]) && !tpm2_algorithm_to_len(pcrs.selection[i].hash)) return false; } diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c index 7f868cc883..edbe5f3aaf 100644 --- a/lib/tpm_tcg2.c +++ b/lib/tpm_tcg2.c @@ -43,7 +43,7 @@ int tcg2_get_pcr_info(struct udevice *dev, u32 *supported_pcr, u32 *active_pcr,
if (hash_mask) { *supported_pcr |= hash_mask; - if (tpm2_is_active_pcr(&pcrs.selection[i])) + if (tpm2_is_active_bank(&pcrs.selection[i])) *active_pcr |= hash_mask; } else { printf("%s: unknown algorithm %x\n", __func__,
From: Ilias Apalodimas ilias.apalodimas@linaro.org
When that function was introduced we were only using it to check if extending a PCR was allowed, so the name made sense. A few patches ago we used that function to reason about the EventLog creation and general usage of PCRs , so let's rename it to something more generic that makes more sense in all contexts.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - None.
include/tpm-v2.h | 5 +++-- lib/tpm-v2.c | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 8c43f4fd9b..87b2c614ad 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -732,12 +732,13 @@ u16 tpm2_algorithm_to_len(enum tpm2_algorithms algo); */
/** - * tpm2_allow_extend() - Check if extending PCRs is allowed and safe + * tpm2_check_active_banks() - Check if the active PCR banks are supported by + * our configuration * * @dev: TPM device * Return: true if allowed */ -bool tpm2_allow_extend(struct udevice *dev); +bool tpm2_check_active_banks(struct udevice *dev);
/** * tpm2_is_active_bank() - check the pcr_select. If at least one of the PCRs diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index cb636414de..0edb0aa90c 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -197,7 +197,7 @@ u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 algorithm, if (!digest) return -EINVAL;
- if (!tpm2_allow_extend(dev)) { + if (!tpm2_check_active_banks(dev)) { log_err("Cannot extend PCRs if all the TPM enabled algorithms are not supported\n"); return -EINVAL; } @@ -896,7 +896,7 @@ u16 tpm2_algorithm_to_len(enum tpm2_algorithms algo) return 0; }
-bool tpm2_allow_extend(struct udevice *dev) +bool tpm2_check_active_banks(struct udevice *dev) { struct tpml_pcr_selection pcrs; size_t i;
Rename the arguments of tcg2_get_pcr_info() to clarify they are bank masks, not PCR mask. Remove the unused local variable.
Signed-off-by: Raymond Mao raymond.mao@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- Changes in v2 - Rebased to 'next'.
include/tpm_tcg2.h | 12 ++++++------ lib/tpm_tcg2.c | 19 ++++++++----------- 2 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/include/tpm_tcg2.h b/include/tpm_tcg2.h index 6519004cc4..eb6afe49e7 100644 --- a/include/tpm_tcg2.h +++ b/include/tpm_tcg2.h @@ -94,17 +94,17 @@ struct tcg_pcr_event { } __packed;
/** - * tcg2_get_pcr_info() - get the supported, active PCRs and number of banks + * tcg2_get_pcr_info() - get the supported, active banks and number of banks * * @dev: TPM device - * @supported_pcr: bitmask with the algorithms supported - * @active_pcr: bitmask with the active algorithms - * @pcr_banks: number of PCR banks + * @supported_bank: bitmask with the algorithms supported + * @active_bank: bitmask with the active algorithms + * @bank_num: number of PCR banks * * @return 0 on success, code of operation or negative errno on failure */ -int tcg2_get_pcr_info(struct udevice *dev, u32 *supported_pcr, u32 *active_pcr, - u32 *pcr_banks); +int tcg2_get_pcr_info(struct udevice *dev, u32 *supported_bank, u32 *active_bank, + u32 *bank_num);
/** * Crypto Agile Log Entry Format diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c index edbe5f3aaf..0e267ff0a7 100644 --- a/lib/tpm_tcg2.c +++ b/lib/tpm_tcg2.c @@ -20,19 +20,16 @@ #include <linux/unaligned/le_byteshift.h> #include "tpm-utils.h"
-int tcg2_get_pcr_info(struct udevice *dev, u32 *supported_pcr, u32 *active_pcr, - u32 *pcr_banks) +int tcg2_get_pcr_info(struct udevice *dev, u32 *supported_bank, u32 *active_bank, + u32 *bank_num) { - u8 response[(sizeof(struct tpms_capability_data) - - offsetof(struct tpms_capability_data, data))]; struct tpml_pcr_selection pcrs; size_t i; u32 ret;
- *supported_pcr = 0; - *active_pcr = 0; - *pcr_banks = 0; - memset(response, 0, sizeof(response)); + *supported_bank = 0; + *active_bank = 0; + *bank_num = 0;
ret = tpm2_get_pcr_info(dev, &pcrs); if (ret) @@ -42,16 +39,16 @@ int tcg2_get_pcr_info(struct udevice *dev, u32 *supported_pcr, u32 *active_pcr, u32 hash_mask = tcg2_algorithm_to_mask(pcrs.selection[i].hash);
if (hash_mask) { - *supported_pcr |= hash_mask; + *supported_bank |= hash_mask; if (tpm2_is_active_bank(&pcrs.selection[i])) - *active_pcr |= hash_mask; + *active_bank |= hash_mask; } else { printf("%s: unknown algorithm %x\n", __func__, pcrs.selection[i].hash); } }
- *pcr_banks = pcrs.count; + *bank_num = pcrs.count;
return 0; }
Add a bool var into hash_algo_list to indicate whether the algorithm is supported or not and move the IS_ENABLED to only cover this var. So that we can have the name, hash, mask and size no matter the digest kconfigs are enabled or not.
In before, tpm2_algorithm_to_len() and tcg2_algorithm_to_mask() are used to identify an unsupported algorithm when they return 0. It is not the case now when hash_algo_list always provides algorithm size and mask, thus a new API is introduced to check if an algorithm is supported by U-Boot.
Suggested-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- Changes in v2 - None.
include/tpm-v2.h | 37 +++++++++++++++++++++++++++++-------- lib/tpm-v2.c | 14 +++++++++++++- lib/tpm_tcg2.c | 17 +++++++++-------- 3 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 87b2c614ad..c49eadda26 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -268,6 +268,7 @@ struct digest_info { u16 hash_alg; u32 hash_mask; u16 hash_len; + bool supported; };
/* Algorithm Registry */ @@ -278,38 +279,50 @@ struct digest_info { #define TCG2_BOOT_HASH_ALG_SM3_256 0x00000010
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, - }, +#if IS_ENABLED(CONFIG_SHA1) + true, +#else + false, #endif -#if IS_ENABLED(CONFIG_SHA256) + }, { "sha256", TPM2_ALG_SHA256, TCG2_BOOT_HASH_ALG_SHA256, TPM2_SHA256_DIGEST_SIZE, - }, +#if IS_ENABLED(CONFIG_SHA256) + true, +#else + false, #endif -#if IS_ENABLED(CONFIG_SHA384) + }, { "sha384", TPM2_ALG_SHA384, TCG2_BOOT_HASH_ALG_SHA384, TPM2_SHA384_DIGEST_SIZE, - }, +#if IS_ENABLED(CONFIG_SHA384) + true, +#else + false, #endif -#if IS_ENABLED(CONFIG_SHA512) + }, { "sha512", TPM2_ALG_SHA512, TCG2_BOOT_HASH_ALG_SHA512, TPM2_SHA512_DIGEST_SIZE, - }, +#if IS_ENABLED(CONFIG_SHA512) + true, +#else + false, #endif + }, };
/* NV index attributes */ @@ -704,6 +717,14 @@ enum tpm2_algorithms tpm2_name_to_algorithm(const char *name); */ const char *tpm2_algorithm_name(enum tpm2_algorithms);
+/** + * tpm2_algorithm_supported() - Check if the algorithm supported by U-Boot + * + * @algorithm_id: algorithm defined in enum tpm2_algorithms + * Return: true if supported, otherwise false + */ +bool tpm2_algorithm_supported(enum tpm2_algorithms algo); + /** * tpm2_algorithm_to_len() - Return an algorithm length for supported algorithm id * diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 0edb0aa90c..96c164f2a5 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -884,6 +884,18 @@ const char *tpm2_algorithm_name(enum tpm2_algorithms algo) return ""; }
+bool tpm2_algorithm_supported(enum tpm2_algorithms algo) +{ + size_t i; + + for (i = 0; i < ARRAY_SIZE(hash_algo_list); ++i) { + if (hash_algo_list[i].hash_alg == algo) + return hash_algo_list[i].supported; + } + + return false; +} + u16 tpm2_algorithm_to_len(enum tpm2_algorithms algo) { size_t i; @@ -908,7 +920,7 @@ bool tpm2_check_active_banks(struct udevice *dev)
for (i = 0; i < pcrs.count; i++) { if (tpm2_is_active_bank(&pcrs.selection[i]) && - !tpm2_algorithm_to_len(pcrs.selection[i].hash)) + !tpm2_algorithm_supported(pcrs.selection[i].hash)) return false; }
diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c index 0e267ff0a7..99671804e3 100644 --- a/lib/tpm_tcg2.c +++ b/lib/tpm_tcg2.c @@ -36,16 +36,17 @@ int tcg2_get_pcr_info(struct udevice *dev, u32 *supported_bank, u32 *active_bank return ret;
for (i = 0; i < pcrs.count; i++) { - u32 hash_mask = tcg2_algorithm_to_mask(pcrs.selection[i].hash); + struct tpms_pcr_selection *sel = &pcrs.selection[i]; + u32 hash_mask = tcg2_algorithm_to_mask(sel->hash);
- if (hash_mask) { + if (tpm2_algorithm_supported(sel->hash)) *supported_bank |= hash_mask; - if (tpm2_is_active_bank(&pcrs.selection[i])) - *active_bank |= hash_mask; - } else { - printf("%s: unknown algorithm %x\n", __func__, - pcrs.selection[i].hash); - } + else + log_warning("%s: unknown algorithm %x\n", __func__, + sel->hash); + + if (tpm2_is_active_bank(sel)) + *active_bank |= hash_mask; }
*bank_num = pcrs.count;
From: Ilias Apalodimas ilias.apalodimas@linaro.org
We already check the active banks vs what U-Boot was compiled with when trying to extend a PCR and we refuse to do so if the TPM active ones don't match the ones U-Boot supports.
Do the same thing for the EventLog creation since extending will fail anyway and print a message so the user can figure out the missing algorithms.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Co-developed-by: Raymond Mao raymond.mao@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - None.
include/tpm-v2.h | 7 +++++++ lib/tpm-v2.c | 23 +++++++++++++++++++++++ lib/tpm_tcg2.c | 27 ++++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/include/tpm-v2.h b/include/tpm-v2.h index c49eadda26..6b3f2175b7 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -770,4 +770,11 @@ bool tpm2_check_active_banks(struct udevice *dev); */ bool tpm2_is_active_bank(struct tpms_pcr_selection *selection);
+/** + * tpm2_print_active_banks() - Print the active TPM PCRs + * + * @dev: TPM device + */ +void tpm2_print_active_banks(struct udevice *dev); + #endif /* __TPM_V2_H */ diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index 96c164f2a5..bac6fd9101 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -926,3 +926,26 @@ bool tpm2_check_active_banks(struct udevice *dev)
return true; } + +void tpm2_print_active_banks(struct udevice *dev) +{ + struct tpml_pcr_selection pcrs; + size_t i; + int rc; + + rc = tpm2_get_pcr_info(dev, &pcrs); + if (rc) { + log_err("Can't retrieve active PCRs\n"); + return; + } + + for (i = 0; i < pcrs.count; i++) { + if (tpm2_is_active_bank(&pcrs.selection[i])) { + const char *str; + + str = tpm2_algorithm_name(pcrs.selection[i].hash); + if (str) + log_info("%s\n", str); + } + } +} diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c index 99671804e3..e77a904129 100644 --- a/lib/tpm_tcg2.c +++ b/lib/tpm_tcg2.c @@ -567,11 +567,36 @@ int tcg2_log_prepare_buffer(struct udevice *dev, struct tcg2_event_log *elog, bool ignore_existing_log) { struct tcg2_event_log log; - int rc; + int rc, i;
elog->log_position = 0; elog->found = false;
+ /* + * Make sure U-Boot is compiled with all the active PCRs + * since we are about to create an EventLog and we won't + * measure anything if the PCR banks don't match + */ + if (!tpm2_check_active_banks(dev)) { + log_err("Cannot create EventLog\n"); + log_err("Mismatch between U-Boot and TPM hash algos\n"); + log_info("TPM:\n"); + tpm2_print_active_banks(dev); + log_info("U-Boot:\n"); + for (i = 0; i < ARRAY_SIZE(hash_algo_list); i++) { + const struct digest_info *algo = &hash_algo_list[i]; + const char *str; + + if (!algo->supported) + continue; + + str = tpm2_algorithm_name(algo->hash_alg); + if (str) + log_info("%s\n", str); + } + return -EINVAL; + } + rc = tcg2_platform_get_log(dev, (void **)&log.log, &log.log_size); if (!rc) { log.log_position = 0;
From: Ilias Apalodimas ilias.apalodimas@linaro.org
We have a lot of code trying to reason about the active TPM PCRs when creating an EventLog. Since changing the active banks can't be done on the fly and requires a TPM reset, let's store them in the chip private data instead.
Upcoming patches will use this during the EventLog creation.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - None.
include/tpm-common.h | 18 +++++++++++++++++- include/tpm-v2.h | 10 ---------- lib/tpm-v2.c | 27 +++++++++++++++++++++++++-- 3 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/include/tpm-common.h b/include/tpm-common.h index 1ba81386ce..fd33cba6ef 100644 --- a/include/tpm-common.h +++ b/include/tpm-common.h @@ -42,12 +42,22 @@ enum tpm_version { TPM_V2, };
+/* + * We deviate from this draft of the specification by increasing the value of + * TPM2_NUM_PCR_BANKS from 3 to 16 to ensure compatibility with TPM2 + * implementations that have enabled a larger than typical number of PCR + * banks. This larger value for TPM2_NUM_PCR_BANKS is expected to be included + * in a future revision of the specification. + */ +#define TPM2_NUM_PCR_BANKS 16 + /** * struct tpm_chip_priv - Information about a TPM, stored by the uclass * - * These values must be set up by the device's probe() method before + * Some of hese values must be set up by the device's probe() method before * communcation is attempted. If the device has an xfer() method, this is * not needed. There is no need to set up @buf. + * The active_banks is only valid for TPMv2 after the device is initialized. * * @version: TPM stack to be used * @duration_ms: Length of each duration type in milliseconds @@ -55,6 +65,8 @@ enum tpm_version { * @buf: Buffer used during the exchanges with the chip * @pcr_count: Number of PCR per bank * @pcr_select_min: Minimum size in bytes of the pcrSelect array + * @active_bank_count: Number of active PCR banks + * @active_banks: Array of active PCRs * @plat_hier_disabled: Platform hierarchy has been disabled (TPM is locked * down until next reboot) */ @@ -68,6 +80,10 @@ struct tpm_chip_priv { /* TPM v2 specific data */ uint pcr_count; uint pcr_select_min; +#if IS_ENABLED(CONFIG_TPM_V2) + u8 active_bank_count; + u32 active_banks[TPM2_NUM_PCR_BANKS]; +#endif bool plat_hier_disabled; };
diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 6b3f2175b7..6e9bc794f9 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -34,16 +34,6 @@ struct udevice;
#define TPM2_HDR_LEN 10
-/* - * We deviate from this draft of the specification by increasing the value of - * TPM2_NUM_PCR_BANKS from 3 to 16 to ensure compatibility with TPM2 - * implementations that have enabled a larger than typical number of PCR - * banks. This larger value for TPM2_NUM_PCR_BANKS is expected to be included - * in a future revision of the specification. - */ -#define TPM2_NUM_PCR_BANKS 16 - -/* Definition of (UINT32) TPM2_CAP Constants */ #define TPM2_CAP_PCRS 0x00000005U #define TPM2_CAP_TPM_PROPERTIES 0x00000006U
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c index bac6fd9101..bc750b7ca1 100644 --- a/lib/tpm-v2.c +++ b/lib/tpm-v2.c @@ -23,6 +23,27 @@
#include "tpm-utils.h"
+static int tpm2_update_active_banks(struct udevice *dev) +{ + struct tpm_chip_priv *priv = dev_get_uclass_priv(dev); + struct tpml_pcr_selection pcrs; + int ret, i; + + ret = tpm2_get_pcr_info(dev, &pcrs); + if (ret) + return ret; + + priv->active_bank_count = 0; + for (i = 0; i < pcrs.count; i++) { + if (!tpm2_is_active_bank(&pcrs.selection[i])) + continue; + priv->active_banks[priv->active_bank_count] = pcrs.selection[i].hash; + priv->active_bank_count++; + } + + return 0; +} + u32 tpm2_startup(struct udevice *dev, enum tpm2_startup_types mode) { const u8 command_v2[12] = { @@ -41,7 +62,7 @@ u32 tpm2_startup(struct udevice *dev, enum tpm2_startup_types mode) if (ret && ret != TPM2_RC_INITIALIZE) return ret;
- return 0; + return tpm2_update_active_banks(dev); }
u32 tpm2_self_test(struct udevice *dev, enum tpm2_yes_no full_test) @@ -69,8 +90,10 @@ u32 tpm2_auto_start(struct udevice *dev)
rc = tpm2_self_test(dev, TPMI_YES); } + if (rc) + return rc;
- return rc; + return tpm2_update_active_banks(dev); }
u32 tpm2_clear(struct udevice *dev, u32 handle, const char *pw,
From: Ilias Apalodimas ilias.apalodimas@linaro.org
A previous patch is storing the active PCR banks on the TPM private data. Instead of parsing them on the fly use the stored values. This allows us to simplify our checks during the log creation and parsing.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - None.
lib/tpm_tcg2.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c index e77a904129..9e63204f24 100644 --- a/lib/tpm_tcg2.c +++ b/lib/tpm_tcg2.c @@ -93,25 +93,18 @@ u32 tcg2_event_get_size(struct tpml_digest_values *digest_list) int tcg2_create_digest(struct udevice *dev, const u8 *input, u32 length, struct tpml_digest_values *digest_list) { + struct tpm_chip_priv *priv = dev_get_uclass_priv(dev); u8 final[sizeof(union tpmu_ha)]; sha256_context ctx_256; sha512_context ctx_512; sha1_context ctx; - u32 active; size_t i; u32 len; - int rc; - - rc = tcg2_get_active_pcr_banks(dev, &active); - if (rc) - return rc;
digest_list->count = 0; - for (i = 0; i < ARRAY_SIZE(hash_algo_list); ++i) { - if (!(active & hash_algo_list[i].hash_mask)) - continue; + for (i = 0; i < priv->active_bank_count; i++) {
- switch (hash_algo_list[i].hash_alg) { + switch (priv->active_banks[i]) { case TPM2_ALG_SHA1: sha1_starts(&ctx); sha1_update(&ctx, input, length); @@ -138,12 +131,12 @@ int tcg2_create_digest(struct udevice *dev, const u8 *input, u32 length, break; default: printf("%s: unsupported algorithm %x\n", __func__, - hash_algo_list[i].hash_alg); + priv->active_banks[i]); continue; }
digest_list->digests[digest_list->count].hash_alg = - hash_algo_list[i].hash_alg; + priv->active_banks[i]; memcpy(&digest_list->digests[digest_list->count].digest, final, len); digest_list->count++;
From: Ilias Apalodimas ilias.apalodimas@linaro.org
A previous patch is storing the active PCR banks on the TPM private data. Instead of parsing them on the fly use the stored values. This allows us to simplify our checks during the log creation and parsing.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - None.
lib/tpm_tcg2.c | 42 +++++++----------------------------------- 1 file changed, 7 insertions(+), 35 deletions(-)
diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c index 9e63204f24..6c72688b80 100644 --- a/lib/tpm_tcg2.c +++ b/lib/tpm_tcg2.c @@ -207,37 +207,17 @@ static int tcg2_log_append_check(struct tcg2_event_log *elog, u32 pcr_index,
static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log *elog) { + struct tpm_chip_priv *priv = dev_get_uclass_priv(dev); struct tcg_efi_spec_id_event *ev; struct tcg_pcr_event *log; u32 event_size; u32 count = 0; u32 log_size; - u32 active; size_t i; u16 len; - int rc; - - rc = tcg2_get_active_pcr_banks(dev, &active); - if (rc) - return rc;
+ count = priv->active_bank_count; event_size = offsetof(struct tcg_efi_spec_id_event, digest_sizes); - for (i = 0; i < ARRAY_SIZE(hash_algo_list); ++i) { - if (!(active & hash_algo_list[i].hash_mask)) - continue; - - switch (hash_algo_list[i].hash_alg) { - case TPM2_ALG_SHA1: - case TPM2_ALG_SHA256: - case TPM2_ALG_SHA384: - case TPM2_ALG_SHA512: - count++; - break; - default: - continue; - } - } - event_size += 1 + (sizeof(struct tcg_efi_spec_id_event_algorithm_size) * count); log_size = offsetof(struct tcg_pcr_event, event) + event_size; @@ -264,19 +244,11 @@ static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log *elog) ev->uintn_size = sizeof(size_t) / sizeof(u32); put_unaligned_le32(count, &ev->number_of_algorithms);
- count = 0; - for (i = 0; i < ARRAY_SIZE(hash_algo_list); ++i) { - if (!(active & hash_algo_list[i].hash_mask)) - continue; - - len = hash_algo_list[i].hash_len; - if (!len) - continue; - - put_unaligned_le16(hash_algo_list[i].hash_alg, - &ev->digest_sizes[count].algorithm_id); - put_unaligned_le16(len, &ev->digest_sizes[count].digest_size); - count++; + for (i = 0; i < count; ++i) { + len = tpm2_algorithm_to_len(priv->active_banks[i]); + put_unaligned_le16(priv->active_banks[i], + &ev->digest_sizes[i].algorithm_id); + put_unaligned_le16(len, &ev->digest_sizes[i].digest_size); }
*((u8 *)ev + (event_size - 1)) = 0;
From: Ilias Apalodimas ilias.apalodimas@linaro.org
We used to stop replaying an EventLog if parsing failed, but that got lost in commit 97707f12fdab ("tpm: Support boot measurements").
When an EventLog is passed yo us from a previous bootloader, we want to validate it as much as we can and make sure the defined PCR banks of the log exist in our TPM and firmware so we can replay it if needed or use it as-in, in case the PCRs are already extended.
So let's add the checks back and while at it simplify the logic of rejecting an EventLog.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - None.
lib/tpm_tcg2.c | 56 ++++++++++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 25 deletions(-)
diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c index 6c72688b80..37818fcc85 100644 --- a/lib/tpm_tcg2.c +++ b/lib/tpm_tcg2.c @@ -359,7 +359,6 @@ static int tcg2_log_parse(struct udevice *dev, struct tcg2_event_log *elog) u16 len; int rc; u32 i; - u16 j;
if (elog->log_size <= offsetof(struct tcg_pcr_event, event)) return 0; @@ -398,40 +397,51 @@ static int tcg2_log_parse(struct udevice *dev, struct tcg2_event_log *elog) if (evsz != calc_size) return 0;
- rc = tcg2_get_active_pcr_banks(dev, &active); - if (rc) - return rc; - + /* + * Go through the algorithms the EventLog contains. If the EventLog + * algorithms don't match the active TPM ones exit and report the + * erroneous banks. + * We've already checked that U-Boot supports all the enabled TPM + * algorithms, so just check the EvenLog against the TPM active ones. + */ digest_list.count = 0; log_active = 0; - for (i = 0; i < count; ++i) { algo = get_unaligned_le16(&event->digest_sizes[i].algorithm_id); mask = tcg2_algorithm_to_mask(algo);
- if (!(active & mask)) - return 0; - switch (algo) { case TPM2_ALG_SHA1: case TPM2_ALG_SHA256: case TPM2_ALG_SHA384: case TPM2_ALG_SHA512: len = get_unaligned_le16(&event->digest_sizes[i].digest_size); - if (tpm2_algorithm_to_len(algo) != len) - return 0; + if (tpm2_algorithm_to_len(algo) != len) { + log_err("EventLog invalid algorithm length\n"); + return -1; + } digest_list.digests[digest_list.count++].hash_alg = algo; break; default: - return 0; + /* + * We can ignore this if the TPM PCRs is not extended + * by the previous bootloader. But for now just exit + */ + log_err("EventLog has unsupported algorithm 0x%x\n", + algo); + return -1; } - log_active |= mask; }
- /* Ensure the previous firmware extended all the PCRs. */ - if (log_active != active) - return 0; + rc = tcg2_get_active_pcr_banks(dev, &active); + if (rc) + return rc; + /* If the EventLog and active algorithms don't match exit */ + if (log_active != active) { + log_err("EventLog doesn't contain all active PCR banks\n"); + return -1; + }
/* Read PCR0 to check if previous firmware extended the PCRs or not. */ rc = tcg2_pcr_read(dev, 0, &digest_list); @@ -439,17 +449,13 @@ static int tcg2_log_parse(struct udevice *dev, struct tcg2_event_log *elog) return rc;
for (i = 0; i < digest_list.count; ++i) { - len = tpm2_algorithm_to_len(digest_list.digests[i].hash_alg); - for (j = 0; j < len; ++j) { - if (digest_list.digests[i].digest.sha512[j]) - break; - } + u8 hash_buf[TPM2_SHA512_DIGEST_SIZE] = { 0 }; + u16 hash_alg = digest_list.digests[i].hash_alg;
- /* PCR is non-zero; it has been extended, so skip extending. */ - if (j != len) { + if (memcmp((u8 *)&digest_list.digests[i].digest, hash_buf, + tpm2_algorithm_to_len(hash_alg))) digest_list.count = 0; - break; - } + }
return tcg2_replay_eventlog(elog, dev, &digest_list,
To allow disabling algorithms for tcg2, in function tcg2_create_digest(), each hash algorithm operations should under the hash kconfig control to avoid building errors when the algorithm is disabled.
Signed-off-by: Raymond Mao raymond.mao@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- Changes in v2 - None.
lib/tpm_tcg2.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/lib/tpm_tcg2.c b/lib/tpm_tcg2.c index 37818fcc85..4134d93a35 100644 --- a/lib/tpm_tcg2.c +++ b/lib/tpm_tcg2.c @@ -95,9 +95,15 @@ int tcg2_create_digest(struct udevice *dev, const u8 *input, u32 length, { struct tpm_chip_priv *priv = dev_get_uclass_priv(dev); u8 final[sizeof(union tpmu_ha)]; +#if IS_ENABLED(CONFIG_SHA256) sha256_context ctx_256; +#endif +#if IS_ENABLED(CONFIG_SHA512) sha512_context ctx_512; +#endif +#if IS_ENABLED(CONFIG_SHA1) sha1_context ctx; +#endif size_t i; u32 len;
@@ -105,30 +111,38 @@ int tcg2_create_digest(struct udevice *dev, const u8 *input, u32 length, for (i = 0; i < priv->active_bank_count; i++) {
switch (priv->active_banks[i]) { +#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__, priv->active_banks[i]);
participants (2)
-
Heinrich Schuchardt -
Raymond Mao