Andrey Pronin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46772 )
Change subject: security/vboot: fix policy digest for nvmem spaces ......................................................................
security/vboot: fix policy digest for nvmem spaces
This CL fixes the policy digest that restricts deleting the nvmem spaces to specific PCR0 states for Chrome OS case. For the general case, the CL captures the issues with the currently used digest in the comment. Fixing the general case is only possible after understanding in which states the nvmem spaces should be deletable, so this CL doesn't attempt to fix it.
BRANCH=none BUG=b:140958855 TEST=verified that nvmem spaces created with this digest can be deleted in the intended states, and cannot be deleted in other states (test details for ChromeOS - in BUG comments).
Change-Id: I3cb7d644fdebda71cec3ae36de1dc76387e61ea7 --- M src/security/vboot/secdata_tpm.c 1 file changed, 37 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/46772/1
diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c index 65d9c83..bc3d982 100644 --- a/src/security/vboot/secdata_tpm.c +++ b/src/security/vboot/secdata_tpm.c @@ -108,14 +108,47 @@ .TPMA_NV_PLATFORMCREATE = 1, };
+#if CONFIG(CHROMEOS) +/* + * This policy digest was obtained using TPM2_PolicyOR on 3 digests + * corresponding to a sequence of + * -) TPM2_PolicyCommandCode(TPM_CC_NV_UndefineSpaceSpecial), + * -) TPM2_PolicyPCR(PCR0, <extended_value>). + * where <extended value> is + * 1) all zeros = initial, unextended state, + * 2) result of extending (SHA1(0x00|0x01|0x00) | 00s to SHA256 size), + * 3) result of extending (SHA1(0x01|0x01|0x00) | 00s to SHA256 size). + * Values #2 and #3 correspond to two forms of recovery mode as extended by + * vb2api_get_pcr_digest(). + * As a result, the digest allows deleting the space with UndefineSpaceSpecial + * at early RO stages (before extending PCR0) or from recovery mode. + */ +static const uint8_t pcr0_allowed_policy[] = { + 0x44, 0x44, 0x79, 0x00, 0xCB, 0xB8, 0x3F, 0x5B, 0x15, 0x76, 0x56, + 0x50, 0xEF, 0x96, 0x98, 0x0A, 0x2B, 0x96, 0x6E, 0xA9, 0x09, 0x04, + 0x4A, 0x01, 0xB8, 0x5F, 0xA5, 0x4A, 0x96, 0xFC, 0x59, 0x84}; +#else /* * This policy digest was obtained using TPM2_PolicyPCR * selecting only PCR_0 with a value of all zeros. + * + * TODO: Fixed for ChromeOS above, need to fix for the general case depending + * on the actual use. Issues with the current digest: + * 1) The actual digest for a single PolicyPCR(PCR0, 00..00) + * is 0x09, 0x3C, 0xEB, ... + * and not 0x09, 0x93, 0x3C, 0xCE, 0xEB, ... + * 2) UndefineSpaceSpecial uses ADMIN auth role for the index and thus + * requires that the used policy contains a command that restricts command + * code (to TPM_CC_NV_UndefineSpaceSpecial in this case). + * The currently used digest works as arbitrary value that doesn't have a known + * sequence of policy commands leading to it, essentially preventing undefining + * the space. */ -static const uint8_t pcr0_unchanged_policy[] = { +static const uint8_t pcr0_allowed_policy[] = { 0x09, 0x93, 0x3C, 0xCE, 0xEB, 0xB4, 0x41, 0x11, 0x18, 0x81, 0x1D, 0xD4, 0x47, 0x78, 0x80, 0x08, 0x88, 0x86, 0x62, 0x2D, 0xD7, 0x79, 0x94, 0x46, 0x62, 0x26, 0x68, 0x8E, 0xEE, 0xE6, 0x6A, 0xA1}; +#endif /* CONFIG(CHROMEOS) */
/* Nothing special in the TPM2 path yet. */ static uint32_t safe_write(uint32_t index, const void *data, uint32_t length) @@ -154,7 +187,7 @@ { return set_space("firmware", FIRMWARE_NV_INDEX, firmware_blob, VB2_SECDATA_FIRMWARE_SIZE, ro_space_attributes, - pcr0_unchanged_policy, sizeof(pcr0_unchanged_policy)); + pcr0_allowed_policy, sizeof(pcr0_allowed_policy)); }
static uint32_t set_kernel_space(const void *kernel_blob) @@ -167,8 +200,8 @@ { if (index == MRC_REC_HASH_NV_INDEX) { return set_space("RO MRC Hash", index, data, HASH_NV_SIZE, - ro_space_attributes, pcr0_unchanged_policy, - sizeof(pcr0_unchanged_policy)); + ro_space_attributes, pcr0_allowed_policy, + sizeof(pcr0_allowed_policy)); } else { return set_space("RW MRC Hash", index, data, HASH_NV_SIZE, rw_space_attributes, NULL, 0);