Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/59681 )
Change subject: cbfs | tspi: Join hash calculation for verification and measurement ......................................................................
cbfs | tspi: Join hash calculation for verification and measurement
This patch moves the CBFS file measurement when CONFIG_TPM_MEASURED_BOOT is enabled from the lookup step into the code where a file is actually loaded or mapped from flash. This has the advantage that CBFS routines which just look up a file to inspect its metadata (e.g. cbfs_get_size()) do not cause the file to be measured twice. It also removes the existing inefficiency that files are loaded twice when measurement is enabled (once to measure and then again when they are used). When CBFS verification is enabled and uses the same hash algorithm as the TPM, we are even able to only hash the file a single time and use the result for both purposes.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I70d7066c6768195077f083c7ffdfa30d9182b2b7 Reviewed-on: https://review.coreboot.org/c/coreboot/+/59681 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Raul Rangel rrangel@chromium.org --- M src/lib/cbfs.c M src/security/tpm/tspi.h M src/security/tpm/tspi/crtm.c M src/security/tpm/tspi/crtm.h M src/security/tpm/tspi/tspi.c 5 files changed, 59 insertions(+), 97 deletions(-)
Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index b4b6eb1..0b58913 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -81,10 +81,6 @@ if (rdev_chain(rdev, &cbd->rdev, data_offset, be32toh(mdata->h.len))) return CB_ERR;
- if (tspi_measure_cbfs_hook(rdev, name, be32toh(mdata->h.type))) { - printk(BIOS_ERR, "CBFS ERROR: error when measuring '%s'\n", name); - } - return CB_SUCCESS; }
@@ -134,9 +130,8 @@ type = &dummy_type;
ret = cbfs_locate(fh, &rdev, name, type); - if (!ret) - if (tspi_measure_cbfs_hook(&rdev, name, *type)) - LOG("error measuring %s in region %s\n", name, region_name); + /* No more measuring here, this function will be removed next patch. */ + return ret; }
@@ -193,19 +188,36 @@ static bool cbfs_file_hash_mismatch(const void *buffer, size_t size, const union cbfs_mdata *mdata, bool skip_verification) { - /* Avoid linking hash functions when verification is disabled. */ - if (!CONFIG(CBFS_VERIFICATION) || skip_verification) + /* Avoid linking hash functions when verification and measurement are disabled. */ + if (!CONFIG(CBFS_VERIFICATION) && !CONFIG(TPM_MEASURED_BOOT)) return false;
- const struct vb2_hash *hash = cbfs_file_hash(mdata); - if (!hash) { - ERROR("'%s' does not have a file hash!\n", mdata->h.filename); - return true; + const struct vb2_hash *hash = NULL; + + if (CONFIG(CBFS_VERIFICATION) && !skip_verification) { + hash = cbfs_file_hash(mdata); + if (!hash) { + ERROR("'%s' does not have a file hash!\n", mdata->h.filename); + return true; + } + if (vb2_hash_verify(buffer, size, hash) != VB2_SUCCESS) { + ERROR("'%s' file hash mismatch!\n", mdata->h.filename); + return true; + } }
- if (vb2_hash_verify(buffer, size, hash) != VB2_SUCCESS) { - ERROR("'%s' file hash mismatch!\n", mdata->h.filename); - return true; + if (CONFIG(TPM_MEASURED_BOOT) && !ENV_SMM) { + struct vb2_hash calculated_hash; + + /* No need to re-hash file if we already have it from verification. */ + if (!hash || hash->algo != TPM_MEASURE_ALGO) { + vb2_hash_calculate(buffer, size, TPM_MEASURE_ALGO, &calculated_hash); + hash = &calculated_hash; + } + + if (tspi_cbfs_measurement(mdata->h.filename, be32toh(mdata->h.type), hash)) + ERROR("failed to measure '%s' into TCPA log\n", mdata->h.filename); + /* We intentionally continue to boot on measurement errors. */ }
return false; @@ -528,9 +540,6 @@ if (rdev_chain(&file_rdev, &area_rdev, data_offset, be32toh(mdata.h.len))) return NULL;
- if (tspi_measure_cbfs_hook(&file_rdev, name, be32toh(mdata.h.type))) - ERROR("error measuring '%s' in '%s'\n", name, area); - return do_alloc(&mdata, &file_rdev, allocator, arg, size_out, true); }
diff --git a/src/security/tpm/tspi.h b/src/security/tpm/tspi.h index ed642c3..7157b4d 100644 --- a/src/security/tpm/tspi.h +++ b/src/security/tpm/tspi.h @@ -51,7 +51,7 @@ * @return TPM_SUCCESS on success. If not a tpm error is returned */ uint32_t tpm_extend_pcr(int pcr, enum vb2_hash_algorithm digest_algo, - uint8_t *digest, size_t digest_len, + const uint8_t *digest, size_t digest_len, const char *name);
/** diff --git a/src/security/tpm/tspi/crtm.c b/src/security/tpm/tspi/crtm.c index b64bbbf..8cd0793 100644 --- a/src/security/tpm/tspi/crtm.c +++ b/src/security/tpm/tspi/crtm.c @@ -6,37 +6,6 @@ #include "crtm.h" #include <string.h>
-/* - * This function sets the TCPA log namespace - * for the cbfs file (region) lookup. - */ -static int create_tcpa_metadata(const struct region_device *rdev, - const char *cbfs_name, char log_string[TCPA_PCR_HASH_NAME]) -{ - int i; - struct region_device fmap; - static const char *const fmap_cbfs_names[] = { - "COREBOOT", - "FW_MAIN_A", - "FW_MAIN_B", - "RW_LEGACY" - }; - - for (i = 0; i < ARRAY_SIZE(fmap_cbfs_names); i++) { - if (fmap_locate_area_as_rdev(fmap_cbfs_names[i], &fmap) == 0) { - if (region_is_subregion(region_device_region(&fmap), - region_device_region(rdev))) { - snprintf(log_string, TCPA_PCR_HASH_NAME, - "FMAP: %s CBFS: %s", - fmap_cbfs_names[i], cbfs_name); - return 0; - } - } - } - - return -1; -} - static int tcpa_log_initialized; static inline int tcpa_log_available(void) { @@ -64,8 +33,6 @@ */ static uint32_t tspi_init_crtm(void) { - struct prog bootblock = PROG_INIT(PROG_BOOTBLOCK, "bootblock"); - /* Initialize TCPA PRERAM log. */ if (!tcpa_log_available()) { tcpa_preram_log_clear(); @@ -87,7 +54,6 @@ }
/* measure bootblock from RO */ - struct cbfsf bootblock_data; struct region_device bootblock_fmap; if (fmap_locate_area_as_rdev("BOOTBLOCK", &bootblock_fmap) == 0) { if (tpm_measure_region(&bootblock_fmap, @@ -95,16 +61,16 @@ "FMAP: BOOTBLOCK")) return VB2_ERROR_UNKNOWN; } else { - if (cbfs_boot_locate(&bootblock_data, - prog_name(&bootblock), NULL)) { - /* - * measurement is done in - * tspi_measure_cbfs_hook() - */ + /* Mapping measures the file. We know we can safely map here because + bootblock-as-a-file is only used on x86, where we don't need cache to map. */ + enum cbfs_type type = CBFS_TYPE_BOOTBLOCK; + void *mapping = cbfs_ro_type_map("bootblock", NULL, &type); + if (!mapping) { printk(BIOS_INFO, "TSPI: Couldn't measure bootblock into CRTM!\n"); return VB2_ERROR_UNKNOWN; } + cbfs_unmap(mapping); }
return VB2_SUCCESS; @@ -129,8 +95,7 @@ return !strcmp(allowlist, name); }
-uint32_t tspi_measure_cbfs_hook(const struct region_device *rdev, const char *name, - uint32_t cbfs_type) +uint32_t tspi_cbfs_measurement(const char *name, uint32_t type, const struct vb2_hash *hash) { uint32_t pcr_index; char tcpa_metadata[TCPA_PCR_HASH_NAME]; @@ -144,7 +109,7 @@ printk(BIOS_DEBUG, "CRTM initialized.\n"); }
- switch (cbfs_type) { + switch (type) { case CBFS_TYPE_MRC_CACHE: pcr_index = TPM_RUNTIME_DATA_PCR; break; @@ -166,10 +131,10 @@ break; }
- if (create_tcpa_metadata(rdev, name, tcpa_metadata) < 0) - return VB2_ERROR_UNKNOWN; + snprintf(tcpa_metadata, TCPA_PCR_HASH_NAME, "CBFS: %s", name);
- return tpm_measure_region(rdev, pcr_index, tcpa_metadata); + return tpm_extend_pcr(pcr_index, hash->algo, hash->raw, vb2_digest_size(hash->algo), + tcpa_metadata); }
int tspi_measure_cache_to_pcr(void) diff --git a/src/security/tpm/tspi/crtm.h b/src/security/tpm/tspi/crtm.h index 011fa26..8ebb661 100644 --- a/src/security/tpm/tspi/crtm.h +++ b/src/security/tpm/tspi/crtm.h @@ -7,6 +7,7 @@ #include <program_loading.h> #include <security/tpm/tspi.h> #include <types.h> +#include <vb2_sha.h>
/* CRTM */ #define TPM_CRTM_PCR 2 @@ -16,21 +17,16 @@ */ #define TPM_RUNTIME_DATA_PCR 3
+#define TPM_MEASURE_ALGO (CONFIG(TPM1) ? VB2_HASH_SHA1 : VB2_HASH_SHA256) + /** * Measure digests cached in TCPA log entries into PCRs */ int tspi_measure_cache_to_pcr(void);
-#if !ENV_SMM && CONFIG(TPM_MEASURED_BOOT) -/* - * Measures cbfs data via hook (cbfs) - * rdev covers the file data (not metadata) - * return 0 if successful, else an error +/** + * Extend a measurement hash taken for a CBFS file into the appropriate PCR. */ -uint32_t tspi_measure_cbfs_hook(const struct region_device *rdev, - const char *name, uint32_t cbfs_type); -#else -#define tspi_measure_cbfs_hook(rdev, name, cbfs_type) 0 -#endif +uint32_t tspi_cbfs_measurement(const char *name, uint32_t type, const struct vb2_hash *hash);
#endif /* __SECURITY_TSPI_CRTM_H__ */ diff --git a/src/security/tpm/tspi/tspi.c b/src/security/tpm/tspi/tspi.c index b1bea41..e2d6d1e 100644 --- a/src/security/tpm/tspi/tspi.c +++ b/src/security/tpm/tspi/tspi.c @@ -220,7 +220,7 @@ }
uint32_t tpm_extend_pcr(int pcr, enum vb2_hash_algorithm digest_algo, - uint8_t *digest, size_t digest_len, const char *name) + const uint8_t *digest, size_t digest_len, const char *name) { uint32_t result;
@@ -234,16 +234,22 @@ return result; }
- printk(BIOS_DEBUG, "TPM: Extending digest for %s into PCR %d\n", name, pcr); + printk(BIOS_DEBUG, "TPM: Extending digest for `%s` into PCR %d\n", name, pcr); result = tlcl_extend(pcr, digest, NULL); - if (result != TPM_SUCCESS) + if (result != TPM_SUCCESS) { + printk(BIOS_ERR, "TPM: Extending hash for `%s` into PCR %d failed.\n", + name, pcr); return result; + } }
if (CONFIG(TPM_MEASURED_BOOT)) tcpa_log_add_table_entry(name, pcr, digest_algo, digest, digest_len);
+ printk(BIOS_DEBUG, "TPM: Digest of `%s` to PCR %d %s\n", + name, pcr, tspi_tpm_is_setup() ? "measured" : "logged"); + return TPM_SUCCESS; }
@@ -253,23 +259,16 @@ { uint8_t digest[TPM_PCR_MAX_LEN], digest_len; uint8_t buf[HASH_DATA_CHUNK_SIZE]; - uint32_t result, offset; + uint32_t offset; size_t len; struct vb2_digest_context ctx; - enum vb2_hash_algorithm hash_alg;
if (!rdev || !rname) return TPM_E_INVALID_ARG;
- if (CONFIG(TPM1)) { - hash_alg = VB2_HASH_SHA1; - } else { /* CONFIG_TPM2 */ - hash_alg = VB2_HASH_SHA256; - } - - digest_len = vb2_digest_size(hash_alg); + digest_len = vb2_digest_size(TPM_MEASURE_ALGO); assert(digest_len <= sizeof(digest)); - if (vb2_digest_init(&ctx, hash_alg)) { + if (vb2_digest_init(&ctx, TPM_MEASURE_ALGO)) { printk(BIOS_ERR, "TPM: Error initializing hash.\n"); return TPM_E_HASH_ERROR; } @@ -294,13 +293,6 @@ printk(BIOS_ERR, "TPM: Error finalizing hash.\n"); return TPM_E_HASH_ERROR; } - result = tpm_extend_pcr(pcr, hash_alg, digest, digest_len, rname); - if (result != TPM_SUCCESS) { - printk(BIOS_ERR, "TPM: Extending hash into PCR failed.\n"); - return result; - } - printk(BIOS_DEBUG, "TPM: Digest of %s to PCR %d %s\n", - rname, pcr, tspi_tpm_is_setup() ? "measured" : "logged"); - return TPM_SUCCESS; + return tpm_extend_pcr(pcr, TPM_MEASURE_ALGO, digest, digest_len, rname); } #endif /* VBOOT_LIB */