Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46432 )
Change subject: security/vboot: Make mrc_cache hash functions generic ......................................................................
security/vboot: Make mrc_cache hash functions generic
We need to extend the functionality of the mrc_cache hash functions to work for both recovery and normal mrc_cache data. Updating the API of these functions to pass in an index to identify the hash indices for recovery and normal mode.
BUG=b:150502246 BRANCH=None TEST=make sure memory training still works on nami
Change-Id: I9c0bb25eafc731ca9c7a95113ab940f55997fc0f Signed-off-by: Shelley Chen shchen@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46432 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/drivers/mrc_cache/mrc_cache.c M src/security/vboot/antirollback.h M src/security/vboot/mrc_cache_hash_tpm.c M src/security/vboot/mrc_cache_hash_tpm.h M src/security/vboot/secdata_mock.c M src/security/vboot/secdata_tpm.c M src/security/vboot/vboot_logic.c 7 files changed, 78 insertions(+), 61 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c index 95abc4f..8d4df8f 100644 --- a/src/drivers/mrc_cache/mrc_cache.c +++ b/src/drivers/mrc_cache/mrc_cache.c @@ -10,6 +10,7 @@ #include <fmap.h> #include <ip_checksum.h> #include <region_file.h> +#include <security/vboot/antirollback.h> #include <security/vboot/mrc_cache_hash_tpm.h> #include <security/vboot/vboot_common.h> #include <spi_flash.h> @@ -179,6 +180,7 @@ void *data, size_t data_size) { uint16_t checksum; + uint32_t hash_idx = MRC_REC_HASH_NV_INDEX;
if (md->data_size != data_size) return -1; @@ -191,7 +193,7 @@ return -1; }
- if (CONFIG(MRC_SAVE_HASH_IN_TPM) && !mrc_cache_verify_hash(data, data_size)) + if (CONFIG(MRC_SAVE_HASH_IN_TPM) && !mrc_cache_verify_hash(hash_idx, data, data_size)) return -1;
return 0; @@ -393,6 +395,7 @@ const struct region_device *backing_rdev; struct region_device latest_rdev; const bool fail_bad_data = false; + uint32_t hash_idx = MRC_REC_HASH_NV_INDEX;
cr = lookup_region(®ion, type);
@@ -453,7 +456,7 @@ printk(BIOS_DEBUG, "MRC: updated '%s'.\n", cr->name); log_event_cache_update(cr->elog_slot, UPDATE_SUCCESS); if (CONFIG(MRC_SAVE_HASH_IN_TPM)) - mrc_cache_update_hash(new_data, new_data_size); + mrc_cache_update_hash(hash_idx, new_data, new_data_size); } }
diff --git a/src/security/vboot/antirollback.h b/src/security/vboot/antirollback.h index 595205d..8b183da 100644 --- a/src/security/vboot/antirollback.h +++ b/src/security/vboot/antirollback.h @@ -22,8 +22,9 @@ * want to use 0x1009 for something else. */ #define BACKUP_NV_INDEX 0x1009 #define FWMP_NV_INDEX 0x100a -#define REC_HASH_NV_INDEX 0x100b -#define REC_HASH_NV_SIZE VB2_SHA256_DIGEST_SIZE +/* 0x100b: Hash of MRC_CACHE training data for recovery boot */ +#define MRC_REC_HASH_NV_INDEX 0x100b +#define HASH_NV_SIZE VB2_SHA256_DIGEST_SIZE
/* Structure definitions for TPM spaces */
@@ -55,11 +56,25 @@ */ uint32_t antirollback_lock_space_firmware(void);
-/* Read recovery hash data from TPM. */ -uint32_t antirollback_read_space_rec_hash(uint8_t *data, uint32_t size); -/* Write new hash data to recovery space in TPM. */ -uint32_t antirollback_write_space_rec_hash(const uint8_t *data, uint32_t size); -/* Lock down recovery hash space in TPM. */ -uint32_t antirollback_lock_space_rec_hash(void); +/* + * Read recovery hash data from TPM. + * @param index index into TPM NVRAM where hash is stored + * @param data pointer to buffer where hash from TPM read into + * @param size size of buffer + */ +uint32_t antirollback_read_space_mrc_hash(uint32_t index, uint8_t *data, uint32_t size); +/* + * Write new hash data to recovery space in TPM.\ + * @param index index into TPM NVRAM where hash is stored + * @param data pointer to buffer of hash value to be written + * @param size size of buffer +*/ +uint32_t antirollback_write_space_mrc_hash(uint32_t index, const uint8_t *data, + uint32_t size); +/* + * Lock down recovery hash space in TPM. + * @param index index into TPM NVRAM where hash is stored +*/ +uint32_t antirollback_lock_space_mrc_hash(uint32_t index);
#endif /* ANTIROLLBACK_H_ */ diff --git a/src/security/vboot/mrc_cache_hash_tpm.c b/src/security/vboot/mrc_cache_hash_tpm.c index 24e7aaf..fede488 100644 --- a/src/security/vboot/mrc_cache_hash_tpm.c +++ b/src/security/vboot/mrc_cache_hash_tpm.c @@ -9,7 +9,7 @@ #include <console/console.h> #include <string.h>
-void mrc_cache_update_hash(const uint8_t *data, size_t size) +void mrc_cache_update_hash(uint32_t index, const uint8_t *data, size_t size) { uint8_t data_hash[VB2_SHA256_DIGEST_SIZE]; static const uint8_t dead_hash[VB2_SHA256_DIGEST_SIZE] = { @@ -40,26 +40,26 @@ printk(BIOS_ERR, "MRC: SHA-256 calculation failed for data. " "Not updating TPM hash space.\n"); /* - * Since data is being updated in recovery cache, the hash - * currently stored in TPM recovery hash space is no longer - * valid. If we are not able to calculate hash of the data being - * updated, reset all the bits in TPM recovery hash space to - * pre-defined hash pattern. + * Since data is being updated in mrc cache, the hash + * currently stored in TPM hash space is no longer + * valid. If we are not able to calculate hash of the + * data being updated, reset all the bits in TPM hash + * space to pre-defined hash pattern. */ hash_ptr = dead_hash; }
/* Write hash of data to TPM space. */ - if (antirollback_write_space_rec_hash(hash_ptr, VB2_SHA256_DIGEST_SIZE) + if (antirollback_write_space_mrc_hash(index, hash_ptr, VB2_SHA256_DIGEST_SIZE) != TPM_SUCCESS) { printk(BIOS_ERR, "MRC: Could not save hash to TPM.\n"); return; }
- printk(BIOS_INFO, "MRC: TPM MRC hash updated successfully.\n"); + printk(BIOS_INFO, "MRC: TPM MRC hash idx 0x%x updated successfully.\n", index); }
-int mrc_cache_verify_hash(const uint8_t *data, size_t size) +int mrc_cache_verify_hash(uint32_t index, const uint8_t *data, size_t size) { uint8_t data_hash[VB2_SHA256_DIGEST_SIZE]; uint8_t tpm_hash[VB2_SHA256_DIGEST_SIZE]; @@ -68,7 +68,7 @@ if (!vboot_recovery_mode_enabled()) return 1;
- /* Calculate hash of data read from RECOVERY_MRC_CACHE. */ + /* Calculate hash of data read from MRC_CACHE. */ if (vb2_digest_buffer(data, size, VB2_HASH_SHA256, data_hash, sizeof(data_hash))) { printk(BIOS_ERR, "MRC: SHA-256 calculation failed for data.\n"); @@ -82,7 +82,7 @@ }
/* Read hash of MRC data saved in TPM. */ - if (antirollback_read_space_rec_hash(tpm_hash, sizeof(tpm_hash)) + if (antirollback_read_space_mrc_hash(index, tpm_hash, sizeof(tpm_hash)) != TPM_SUCCESS) { printk(BIOS_ERR, "MRC: Could not read hash from TPM.\n"); return 0; @@ -93,7 +93,7 @@ return 0; }
- printk(BIOS_INFO, "MRC: Hash comparison successful. " - "Using data from RECOVERY_MRC_CACHE\n"); + printk(BIOS_INFO, "MRC: Hash idx 0x%x comparison successful.\n", index); + return 1; } diff --git a/src/security/vboot/mrc_cache_hash_tpm.h b/src/security/vboot/mrc_cache_hash_tpm.h index a1ecd8b..cf443f2 100644 --- a/src/security/vboot/mrc_cache_hash_tpm.h +++ b/src/security/vboot/mrc_cache_hash_tpm.h @@ -8,12 +8,12 @@ /* * Updates mrc cache hash if it differs. */ -void mrc_cache_update_hash(const uint8_t *data, size_t size); +void mrc_cache_update_hash(uint32_t index, const uint8_t *data, size_t size);
/* * Verifies mrc cache hash which is stored somewhere. * return 1 verification was successful and 0 for error. */ -int mrc_cache_verify_hash(const uint8_t *data, size_t size); +int mrc_cache_verify_hash(uint32_t index, const uint8_t *data, size_t size);
#endif /* _MRC_CACHE_HASH_TPM_H_ */ diff --git a/src/security/vboot/secdata_mock.c b/src/security/vboot/secdata_mock.c index ae124da..78cb3e6 100644 --- a/src/security/vboot/secdata_mock.c +++ b/src/security/vboot/secdata_mock.c @@ -42,17 +42,17 @@ return VB2_SUCCESS; }
-vb2_error_t antirollback_lock_space_rec_hash(void) +vb2_error_t antirollback_lock_space_mrc_hash(uint32_t index) { return VB2_SUCCESS; }
-vb2_error_t antirollback_read_space_rec_hash(uint8_t *data, uint32_t size) +vb2_error_t antirollback_read_space_mrc_hash(uint32_t index, uint8_t *data, uint32_t size) { return VB2_SUCCESS; }
-vb2_error_t antirollback_write_space_rec_hash(const uint8_t *data, +vb2_error_t antirollback_write_space_mrc_hash(uint32_t index, const uint8_t *data, uint32_t size) { return VB2_SUCCESS; diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index 691d2c0..451f043 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -71,10 +71,10 @@ return TPM_SUCCESS; }
-static uint32_t read_space_rec_hash(uint8_t *data) +static uint32_t read_space_mrc_hash(uint32_t index, uint8_t *data) { - RETURN_ON_FAILURE(tlcl_read(REC_HASH_NV_INDEX, data, - REC_HASH_NV_SIZE)); + RETURN_ON_FAILURE(tlcl_read(index, data, + HASH_NV_SIZE)); return TPM_SUCCESS; }
@@ -83,7 +83,7 @@ * it. Since there is no data available to calculate hash at the point where TPM * space is defined, initialize it to all 0s. */ -static const uint8_t rec_hash_data[REC_HASH_NV_SIZE] = { }; +static const uint8_t mrc_hash_data[HASH_NV_SIZE] = { };
#if CONFIG(TPM2) /* @@ -162,10 +162,9 @@ VB2_SECDATA_KERNEL_SIZE, rw_space_attributes, NULL, 0); }
-static uint32_t set_rec_hash_space(const uint8_t *data) +static uint32_t set_mrc_hash_space(uint32_t index, const uint8_t *data) { - return set_space("MRC Hash", REC_HASH_NV_INDEX, data, - REC_HASH_NV_SIZE, + return set_space("MRC Hash", index, data, HASH_NV_SIZE, ro_space_attributes, pcr0_unchanged_policy, sizeof(pcr0_unchanged_policy)); } @@ -185,7 +184,7 @@ RETURN_ON_FAILURE(set_kernel_space(ctx->secdata_kernel));
if (CONFIG(VBOOT_HAS_REC_HASH_SPACE)) - RETURN_ON_FAILURE(set_rec_hash_space(rec_hash_data)); + RETURN_ON_FAILURE(set_mrc_hash_space(MRC_REC_HASH_NV_INDEX, mrc_hash_data));
RETURN_ON_FAILURE(set_firmware_space(ctx->secdata_firmware));
@@ -197,9 +196,9 @@ return tlcl_lock_nv_write(FIRMWARE_NV_INDEX); }
-uint32_t antirollback_lock_space_rec_hash(void) +uint32_t antirollback_lock_space_mrc_hash(uint32_t index) { - return tlcl_lock_nv_write(REC_HASH_NV_INDEX); + return tlcl_lock_nv_write(index); }
#else @@ -239,14 +238,14 @@ } }
-static uint32_t set_rec_hash_space(const uint8_t *data) +static uint32_t set_mrc_hash_space(uint32_t index, const uint8_t *data) { - RETURN_ON_FAILURE(safe_define_space(REC_HASH_NV_INDEX, + RETURN_ON_FAILURE(safe_define_space(index, TPM_NV_PER_GLOBALLOCK | TPM_NV_PER_PPWRITE, - REC_HASH_NV_SIZE)); - RETURN_ON_FAILURE(safe_write(REC_HASH_NV_INDEX, data, - REC_HASH_NV_SIZE)); + HASH_NV_SIZE)); + RETURN_ON_FAILURE(safe_write(index, data, + HASH_NV_SIZE));
return TPM_SUCCESS; } @@ -307,7 +306,7 @@
/* Define and set rec hash space, if available. */ if (CONFIG(VBOOT_HAS_REC_HASH_SPACE)) - RETURN_ON_FAILURE(set_rec_hash_space(rec_hash_data)); + RETURN_ON_FAILURE(set_mrc_hash_space(MRC_REC_HASH_NV_INDEX, mrc_hash_data));
return TPM_SUCCESS; } @@ -317,7 +316,7 @@ return tlcl_set_global_lock(); }
-uint32_t antirollback_lock_space_rec_hash(void) +uint32_t antirollback_lock_space_mrc_hash(uint32_t index) { /* * Nothing needs to be done here, since global lock is already set while @@ -417,43 +416,43 @@ return safe_write(KERNEL_NV_INDEX, ctx->secdata_kernel, size); }
-uint32_t antirollback_read_space_rec_hash(uint8_t *data, uint32_t size) +uint32_t antirollback_read_space_mrc_hash(uint32_t index, uint8_t *data, uint32_t size) { - if (size != REC_HASH_NV_SIZE) { - VBDEBUG("TPM: Incorrect buffer size for rec hash. " - "(Expected=0x%x Actual=0x%x).\n", REC_HASH_NV_SIZE, + if (size != HASH_NV_SIZE) { + VBDEBUG("TPM: Incorrect buffer size for hash idx 0x%x. " + "(Expected=0x%x Actual=0x%x).\n", index, HASH_NV_SIZE, size); return TPM_E_READ_FAILURE; } - return read_space_rec_hash(data); + return read_space_mrc_hash(index, data); }
-uint32_t antirollback_write_space_rec_hash(const uint8_t *data, uint32_t size) +uint32_t antirollback_write_space_mrc_hash(uint32_t index, const uint8_t *data, uint32_t size) { - uint8_t spc_data[REC_HASH_NV_SIZE]; + uint8_t spc_data[HASH_NV_SIZE]; uint32_t rv;
- if (size != REC_HASH_NV_SIZE) { - VBDEBUG("TPM: Incorrect buffer size for rec hash. " - "(Expected=0x%x Actual=0x%x).\n", REC_HASH_NV_SIZE, + if (size != HASH_NV_SIZE) { + VBDEBUG("TPM: Incorrect buffer size for hash idx 0x%x. " + "(Expected=0x%x Actual=0x%x).\n", index, HASH_NV_SIZE, size); return TPM_E_WRITE_FAILURE; }
- rv = read_space_rec_hash(spc_data); + rv = read_space_mrc_hash(index, spc_data); if (rv == TPM_E_BADINDEX) { /* - * If space is not defined already for recovery hash, define + * If space is not defined already for hash, define * new space. */ - VBDEBUG("TPM: Initializing recovery hash space.\n"); - return set_rec_hash_space(data); + VBDEBUG("TPM: Initializing hash space.\n"); + return set_mrc_hash_space(index, data); }
if (rv != TPM_SUCCESS) return rv;
- return safe_write(REC_HASH_NV_INDEX, data, size); + return safe_write(index, data, size); }
vb2_error_t vb2ex_tpm_clear_owner(struct vb2_context *ctx) diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 0f18f9a..dbaa883 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -404,7 +404,7 @@
/* Lock rec hash space if available. */ if (CONFIG(VBOOT_HAS_REC_HASH_SPACE)) { - rv = antirollback_lock_space_rec_hash(); + rv = antirollback_lock_space_mrc_hash(MRC_REC_HASH_NV_INDEX); if (rv) { printk(BIOS_INFO, "Failed to lock rec hash space(%x)\n", rv);