Shelley Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46615 )
Change subject: security/vboot: remove tpm 1.2 functions ......................................................................
security/vboot: remove tpm 1.2 functions
Since MRC_SAVE_HASH_IN_TPM depends on TPM2, we can now remove all the tpm 1.2 functions in secdata_tpm.c.
BUG=b:150502246 BRANCH=None TEST=make sure boards are still compiling on coreboot Jenkins
Change-Id: I446dde36ce2233fc40687892da1fb515ce35b82b Signed-off-by: Shelley Chen shchen@google.com --- M src/security/vboot/secdata_tpm.c 1 file changed, 0 insertions(+), 155 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/46615/1
diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index 0304b92..686111b 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -7,9 +7,6 @@
#include <security/vboot/antirollback.h> #include <security/vboot/tpm_common.h> -#include <security/tpm/tspi.h> -#include <security/tpm/tss.h> -#include <security/tpm/tss/tcg-1.2/tss_structures.h> #include <vb2_api.h> #include <console/console.h>
@@ -37,26 +34,6 @@
uint32_t antirollback_read_space_kernel(struct vb2_context *ctx) { - if (!CONFIG(TPM2)) { - /* - * Before reading the kernel space, verify its permissions. If - * the kernel space has the wrong permission, we give up. This - * will need to be fixed by the recovery kernel. We will have - * to worry about this because at any time (even with PP turned - * off) the TPM owner can remove and redefine a PP-protected - * space (but not write to it). - */ - uint32_t perms; - - RETURN_ON_FAILURE(tlcl_get_permissions(KERNEL_NV_INDEX, - &perms)); - if (perms != TPM_NV_PER_PPWRITE) { - printk(BIOS_ERR, - "TPM: invalid secdata_kernel permissions\n"); - return TPM_E_CORRUPTED_STATE; - } - } - uint8_t size = VB2_SECDATA_KERNEL_MIN_SIZE;
RETURN_ON_FAILURE(tlcl_read(KERNEL_NV_INDEX, ctx->secdata_kernel, @@ -85,7 +62,6 @@ */ static const uint8_t mrc_hash_data[HASH_NV_SIZE] = { };
-#if CONFIG(TPM2) /* * Different sets of NVRAM space attributes apply to the "ro" spaces, * i.e. those which should not be possible to delete or modify once @@ -213,137 +189,6 @@ return tlcl_lock_nv_write(index); }
-#else - -/** - * Like tlcl_write(), but checks for write errors due to hitting the 64-write - * limit and clears the TPM when that happens. This can only happen when the - * TPM is unowned, so it is OK to clear it (and we really have no choice). - * This is not expected to happen frequently, but it could happen. - */ - -static uint32_t safe_write(uint32_t index, const void *data, uint32_t length) -{ - uint32_t result = tlcl_write(index, data, length); - if (result == TPM_E_MAXNVWRITES) { - RETURN_ON_FAILURE(tpm_clear_and_reenable()); - return tlcl_write(index, data, length); - } else { - return result; - } -} - -/** - * Similarly to safe_write(), this ensures we don't fail a DefineSpace because - * we hit the TPM write limit. This is even less likely to happen than with - * writes because we only define spaces once at initialization, but we'd - * rather be paranoid about this. - */ -static uint32_t safe_define_space(uint32_t index, uint32_t perm, uint32_t size) -{ - uint32_t result = tlcl_define_space(index, perm, size); - if (result == TPM_E_MAXNVWRITES) { - RETURN_ON_FAILURE(tpm_clear_and_reenable()); - return tlcl_define_space(index, perm, size); - } else { - return result; - } -} - -static uint32_t set_mrc_hash_space(uint32_t index, const uint8_t *data) -{ - RETURN_ON_FAILURE(safe_define_space(index, - TPM_NV_PER_GLOBALLOCK | - TPM_NV_PER_PPWRITE, - HASH_NV_SIZE)); - RETURN_ON_FAILURE(safe_write(index, data, - HASH_NV_SIZE)); - - return TPM_SUCCESS; -} - -static uint32_t _factory_initialize_tpm(struct vb2_context *ctx) -{ - TPM_PERMANENT_FLAGS pflags; - uint32_t result; - - vb2api_secdata_kernel_create_v0(ctx); - - result = tlcl_get_permanent_flags(&pflags); - if (result != TPM_SUCCESS) - return result; - - /* - * TPM may come from the factory without physical presence finalized. - * Fix if necessary. - */ - VBDEBUG("TPM: physicalPresenceLifetimeLock=%d\n", - pflags.physicalPresenceLifetimeLock); - if (!pflags.physicalPresenceLifetimeLock) { - VBDEBUG("TPM: Finalizing physical presence\n"); - RETURN_ON_FAILURE(tlcl_finalize_physical_presence()); - } - - /* - * The TPM will not enforce the NV authorization restrictions until the - * execution of a TPM_NV_DefineSpace with the handle of - * TPM_NV_INDEX_LOCK. Here we create that space if it doesn't already - * exist. */ - VBDEBUG("TPM: nvLocked=%d\n", pflags.nvLocked); - if (!pflags.nvLocked) { - VBDEBUG("TPM: Enabling NV locking\n"); - RETURN_ON_FAILURE(tlcl_set_nv_locked()); - } - - /* Clear TPM owner, in case the TPM is already owned for some reason. */ - VBDEBUG("TPM: Clearing owner\n"); - RETURN_ON_FAILURE(tpm_clear_and_reenable()); - - /* Define and write secdata_kernel space. */ - RETURN_ON_FAILURE(safe_define_space(KERNEL_NV_INDEX, - TPM_NV_PER_PPWRITE, - VB2_SECDATA_KERNEL_SIZE_V02)); - RETURN_ON_FAILURE(safe_write(KERNEL_NV_INDEX, - ctx->secdata_kernel, - VB2_SECDATA_KERNEL_SIZE_V02)); - - /* Define and write secdata_firmware space. */ - RETURN_ON_FAILURE(safe_define_space(FIRMWARE_NV_INDEX, - TPM_NV_PER_GLOBALLOCK | - TPM_NV_PER_PPWRITE, - VB2_SECDATA_FIRMWARE_SIZE)); - RETURN_ON_FAILURE(safe_write(FIRMWARE_NV_INDEX, - ctx->secdata_firmware, - VB2_SECDATA_FIRMWARE_SIZE)); - - /* - * Define and set rec hash space, if available. No need to - * create the RW hash space because we will definitely boot - * once in normal mode before shipping, meaning that the space - * will get created with correct permissions while still in in - * our hands. - */ - if (CONFIG(VBOOT_HAS_REC_HASH_SPACE)) - RETURN_ON_FAILURE(set_mrc_hash_space(MRC_REC_HASH_NV_INDEX, mrc_hash_data)); - - return TPM_SUCCESS; -} - -uint32_t antirollback_lock_space_firmware(void) -{ - return tlcl_set_global_lock(); -} - -uint32_t antirollback_lock_space_mrc_hash(uint32_t index) -{ - /* - * Nothing needs to be done here, since global lock is already set while - * locking firmware space. - */ - return TPM_SUCCESS; -} -#endif - /** * Perform one-time initializations. *
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46615 )
Change subject: security/vboot: Remove all tpm 1.2 functions for mrc hash in the tpm ......................................................................
Patch Set 3:
This change is ready for review.
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46615 )
Change subject: security/vboot: Remove all tpm 1.2 functions for mrc hash in the tpm ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46615/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46615/1//COMMIT_MSG@7 PS1, Line 7: remove tpm 1.2 functions
No this is not correct. We just need to drop the functions that deal with MRC hash in TPM. […]
Does this look correct?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46615 )
Change subject: security/vboot: Remove all tpm 1.2 functions for mrc hash in the tpm ......................................................................
Patch Set 3: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46615 )
Change subject: security/vboot: Remove all tpm 1.2 functions for mrc hash in the tpm ......................................................................
Patch Set 3: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46615 )
Change subject: security/vboot: Remove all tpm 1.2 functions for mrc hash in the tpm ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46615/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46615/1//COMMIT_MSG@7 PS1, Line 7: remove tpm 1.2 functions
Does this look correct?
LGTM.
Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46615 )
Change subject: security/vboot: Remove all tpm 1.2 functions for mrc hash in the tpm ......................................................................
security/vboot: Remove all tpm 1.2 functions for mrc hash in the tpm
Since MRC_SAVE_HASH_IN_TPM depends on TPM2, we can now remove the tpm 1.2 versions of functions that deal with mrc hash in the tpm as it will not be used by tpm 1.2 boards. Also move all antirollback functions that deal with mrc hash in the tpm under CONFIG(TPM2).
BUG=b:150502246 BRANCH=None TEST=make sure boards are still compiling on coreboot Jenkins
Change-Id: I446dde36ce2233fc40687892da1fb515ce35b82b Signed-off-by: Shelley Chen shchen@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46615 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/security/vboot/secdata_tpm.c 1 file changed, 41 insertions(+), 70 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Julius Werner: Looks good to me, approved
diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index 0304b92..65d9c83 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -71,6 +71,8 @@ return TPM_SUCCESS; }
+#if CONFIG(TPM2) + static uint32_t read_space_mrc_hash(uint32_t index, uint8_t *data) { RETURN_ON_FAILURE(tlcl_read(index, data, @@ -85,7 +87,6 @@ */ static const uint8_t mrc_hash_data[HASH_NV_SIZE] = { };
-#if CONFIG(TPM2) /* * Different sets of NVRAM space attributes apply to the "ro" spaces, * i.e. those which should not be possible to delete or modify once @@ -208,6 +209,45 @@ return tlcl_lock_nv_write(FIRMWARE_NV_INDEX); }
+uint32_t antirollback_read_space_mrc_hash(uint32_t index, uint8_t *data, uint32_t 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_mrc_hash(index, data); +} + +uint32_t antirollback_write_space_mrc_hash(uint32_t index, const uint8_t *data, uint32_t size) +{ + uint8_t spc_data[HASH_NV_SIZE]; + uint32_t rv; + + 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_mrc_hash(index, spc_data); + if (rv == TPM_E_BADINDEX) { + /* + * If space is not defined already for hash, define + * new space. + */ + VBDEBUG("TPM: Initializing hash space.\n"); + return set_mrc_hash_space(index, data); + } + + if (rv != TPM_SUCCESS) + return rv; + + return safe_write(index, data, size); +} + uint32_t antirollback_lock_space_mrc_hash(uint32_t index) { return tlcl_lock_nv_write(index); @@ -250,18 +290,6 @@ } }
-static uint32_t set_mrc_hash_space(uint32_t index, const uint8_t *data) -{ - RETURN_ON_FAILURE(safe_define_space(index, - TPM_NV_PER_GLOBALLOCK | - TPM_NV_PER_PPWRITE, - HASH_NV_SIZE)); - RETURN_ON_FAILURE(safe_write(index, data, - HASH_NV_SIZE)); - - return TPM_SUCCESS; -} - static uint32_t _factory_initialize_tpm(struct vb2_context *ctx) { TPM_PERMANENT_FLAGS pflags; @@ -316,16 +344,6 @@ ctx->secdata_firmware, VB2_SECDATA_FIRMWARE_SIZE));
- /* - * Define and set rec hash space, if available. No need to - * create the RW hash space because we will definitely boot - * once in normal mode before shipping, meaning that the space - * will get created with correct permissions while still in in - * our hands. - */ - if (CONFIG(VBOOT_HAS_REC_HASH_SPACE)) - RETURN_ON_FAILURE(set_mrc_hash_space(MRC_REC_HASH_NV_INDEX, mrc_hash_data)); - return TPM_SUCCESS; }
@@ -334,14 +352,6 @@ return tlcl_set_global_lock(); }
-uint32_t antirollback_lock_space_mrc_hash(uint32_t index) -{ - /* - * Nothing needs to be done here, since global lock is already set while - * locking firmware space. - */ - return TPM_SUCCESS; -} #endif
/** @@ -434,45 +444,6 @@ return safe_write(KERNEL_NV_INDEX, ctx->secdata_kernel, size); }
-uint32_t antirollback_read_space_mrc_hash(uint32_t index, uint8_t *data, uint32_t 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_mrc_hash(index, data); -} - -uint32_t antirollback_write_space_mrc_hash(uint32_t index, const uint8_t *data, uint32_t size) -{ - uint8_t spc_data[HASH_NV_SIZE]; - uint32_t rv; - - 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_mrc_hash(index, spc_data); - if (rv == TPM_E_BADINDEX) { - /* - * If space is not defined already for hash, define - * new space. - */ - VBDEBUG("TPM: Initializing hash space.\n"); - return set_mrc_hash_space(index, data); - } - - if (rv != TPM_SUCCESS) - return rv; - - return safe_write(index, data, size); -} - vb2_error_t vb2ex_tpm_clear_owner(struct vb2_context *ctx) { uint32_t rv;
-
Furquan Shaikh (Code Review) -
Julius Werner (Code Review)
-
Shelley Chen (Code Review)