Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31260 )
Change subject: coreboot: check Cr50 PM mode on normal boot ......................................................................
coreboot: check Cr50 PM mode on normal boot
Under some scenarios the key ladder on the Cr50 can get disabled. If this state is detected, trigger a reboot of the Cr50 to restore full TPM functionality.
BUG=b:121463033 BRANCH=none TEST=Built coreboot on sarien and grunt platforms. TEST=Ran 'gsctool -a -m disable' and reboot. Verified coreboot sends VENDOR_CC_IMMEDIATE_RESET command to Cr50 and that the Cr50 resets and then the platform boots normally. TEST=Performed Cr50 rollback to 0.0.22 which does not support the VENDOR_CC_TPM_MODE command, confirmed that platform boots normally and the coreboot log captures the unsupported command. Tested-by: Keith Short keithshort@chromium.org
Change-Id: I70e012efaf1079d43890e909bc6b5015bef6835a Signed-off-by: Keith Short keithshort@chromium.org Reviewed-on: https://review.coreboot.org/c/31260 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/include/elog.h M src/mainboard/google/sarien/chromeos.c M src/security/tpm/tss/tcg-2.0/tss_marshaling.c M src/security/tpm/tss/tcg-2.0/tss_structures.h M src/security/tpm/tss/vendor/cr50/cr50.c M src/security/tpm/tss/vendor/cr50/cr50.h M src/security/tpm/tss_errors.h M src/vendorcode/google/chromeos/chromeos.h M src/vendorcode/google/chromeos/cr50_enable_update.c 9 files changed, 247 insertions(+), 20 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/include/elog.h b/src/include/elog.h index 31891e0..f1d5314 100644 --- a/src/include/elog.h +++ b/src/include/elog.h @@ -223,6 +223,9 @@ #define ELOG_SLEEP_PENDING_PM1_WAKE 0x01 #define ELOG_SLEEP_PENDING_GPE0_WAKE 0x02
+/* Cr50 reset to enable TPM */ +#define ELOG_TYPE_CR50_NEED_RESET 0xb2 + struct elog_event_extended_event { u8 event_type; u32 event_complement; diff --git a/src/mainboard/google/sarien/chromeos.c b/src/mainboard/google/sarien/chromeos.c index 4cd6e16..1e363fd 100644 --- a/src/mainboard/google/sarien/chromeos.c +++ b/src/mainboard/google/sarien/chromeos.c @@ -115,7 +115,7 @@ return 1; }
-void mainboard_cr50_update_reset(void) +void mainboard_prepare_cr50_reset(void) { #if ENV_RAMSTAGE /* Ensure system powers up after CR50 reset */ diff --git a/src/security/tpm/tss/tcg-2.0/tss_marshaling.c b/src/security/tpm/tss/tcg-2.0/tss_marshaling.c index f1c5a37..62bc6a9 100644 --- a/src/security/tpm/tss/tcg-2.0/tss_marshaling.c +++ b/src/security/tpm/tss/tcg-2.0/tss_marshaling.c @@ -266,6 +266,14 @@ uint16_t *sub_command = command_body;
switch (*sub_command) { + case TPM2_CR50_SUB_CMD_IMMEDIATE_RESET: + /* The 16-bit timeout parameter is optional for the + * IMMEDIATE_RESET command. However in coreboot, the timeout + * parameter must be specified. + */ + rc |= obuf_write_be16(ob, sub_command[0]); + rc |= obuf_write_be16(ob, sub_command[1]); + break; case TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS: rc |= obuf_write_be16(ob, *sub_command); break; @@ -276,6 +284,18 @@ case TPM2_CR50_SUB_CMD_GET_REC_BTN: rc |= obuf_write_be16(ob, *sub_command); break; + case TPM2_CR50_SUB_CMD_TPM_MODE: + /* The Cr50 TPM_MODE command supports an optional parameter. + * When the parameter is present the Cr50 will attempt to change + * the TPM state (enable or disable) and returns the new state + * in the response. When the parameter is absent, the Cr50 + * returns the current TPM state. + * + * coreboot currently only uses the TPM get capability and does + * not set a new TPM state with the Cr50. + */ + rc |= obuf_write_be16(ob, *sub_command); + break; default: /* Unsupported subcommand. */ printk(BIOS_WARNING, "Unsupported cr50 subcommand: 0x%04x\n", @@ -471,12 +491,16 @@ return -1;
switch (vcr->vc_subcommand) { + case TPM2_CR50_SUB_CMD_IMMEDIATE_RESET: + break; case TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS: break; case TPM2_CR50_SUB_CMD_TURN_UPDATE_ON: return ibuf_read_be8(ib, &vcr->num_restored_headers); case TPM2_CR50_SUB_CMD_GET_REC_BTN: return ibuf_read_be8(ib, &vcr->recovery_button_state); + case TPM2_CR50_SUB_CMD_TPM_MODE: + return ibuf_read_be8(ib, &vcr->tpm_mode); default: printk(BIOS_ERR, "%s:%d - unsupported vendor command %#04x!\n", diff --git a/src/security/tpm/tss/tcg-2.0/tss_structures.h b/src/security/tpm/tss/tcg-2.0/tss_structures.h index 6952169..991cbcf 100644 --- a/src/security/tpm/tss/tcg-2.0/tss_structures.h +++ b/src/security/tpm/tss/tcg-2.0/tss_structures.h @@ -298,6 +298,7 @@ union { uint8_t num_restored_headers; uint8_t recovery_button_state; + uint8_t tpm_mode; }; };
diff --git a/src/security/tpm/tss/vendor/cr50/cr50.c b/src/security/tpm/tss/vendor/cr50/cr50.c index 450ad97..1522ce6 100644 --- a/src/security/tpm/tss/vendor/cr50/cr50.c +++ b/src/security/tpm/tss/vendor/cr50/cr50.c @@ -26,7 +26,7 @@ if (response == NULL || (response && response->hdr.tpm_code)) { if (response) printk(BIOS_INFO, "%s: failed %x\n", __func__, - response->hdr.tpm_code); + response->hdr.tpm_code); else printk(BIOS_INFO, "%s: failed\n", __func__); return TPM_E_IOERROR; @@ -47,7 +47,7 @@ response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, command_body);
if (!response || response->hdr.tpm_code) - return TPM_E_INTERNAL_INCONSISTENCY; + return TPM_E_IOERROR;
*num_restored_headers = response->vcr.num_restored_headers; return TPM_SUCCESS; @@ -63,8 +63,67 @@ response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, &sub_command);
if (!response || response->hdr.tpm_code) - return TPM_E_INTERNAL_INCONSISTENCY; + return TPM_E_IOERROR;
*recovery_button_state = response->vcr.recovery_button_state; return TPM_SUCCESS; } + +uint32_t tlcl_cr50_get_tpm_mode(uint8_t *tpm_mode) +{ + struct tpm2_response *response; + uint16_t mode_command = TPM2_CR50_SUB_CMD_TPM_MODE; + *tpm_mode = TPM_MODE_INVALID; + + printk(BIOS_INFO, "Reading cr50 TPM mode\n"); + + response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, &mode_command); + + if (!response) + return TPM_E_IOERROR; + + if (response->hdr.tpm_code == VENDOR_RC_INTERNAL_ERROR) { + /* + * The Cr50 returns VENDOR_RC_INTERNAL_ERROR iff the key ladder + * is disabled. The Cr50 requires a reboot to re-enable the key + * ladder. + */ + return TPM_E_MUST_REBOOT; + } + + if (response->hdr.tpm_code == VENDOR_RC_NO_SUCH_COMMAND) { + /* + * Explicitly inform caller when command is not supported + */ + return TPM_E_NO_SUCH_COMMAND; + } + + if (response->hdr.tpm_code) { + /* Unexpected return code from Cr50 */ + return TPM_E_IOERROR; + } + + /* TPM command completed without error */ + *tpm_mode = response->vcr.tpm_mode; + + return TPM_SUCCESS; +} + +uint32_t tlcl_cr50_immediate_reset(uint16_t timeout_ms) +{ + struct tpm2_response *response; + uint16_t reset_command_body[] = { + TPM2_CR50_SUB_CMD_IMMEDIATE_RESET, timeout_ms}; + + /* + * Issue an immediate reset to the Cr50. + */ + printk(BIOS_INFO, "Issuing cr50 reset\n"); + response = tpm_process_command(TPM2_CR50_VENDOR_COMMAND, + &reset_command_body); + + if (!response) + return TPM_E_IOERROR; + + return TPM_SUCCESS; +} diff --git a/src/security/tpm/tss/vendor/cr50/cr50.h b/src/security/tpm/tss/vendor/cr50/cr50.h index a1ab539..6a160e0 100644 --- a/src/security/tpm/tss/vendor/cr50/cr50.h +++ b/src/security/tpm/tss/vendor/cr50/cr50.h @@ -23,9 +23,35 @@ to extending generically because the marshaling code is assuming all knowledge of all commands. */ #define TPM2_CR50_VENDOR_COMMAND ((TPM_CC)(TPM_CC_VENDOR_BIT_MASK | 0)) +#define TPM2_CR50_SUB_CMD_IMMEDIATE_RESET (19) #define TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS (21) #define TPM2_CR50_SUB_CMD_TURN_UPDATE_ON (24) #define TPM2_CR50_SUB_CMD_GET_REC_BTN (29) +#define TPM2_CR50_SUB_CMD_TPM_MODE (40) + +/* Cr50 vendor-specific error codes. */ +#define VENDOR_RC_ERR 0x00000500 +enum cr50_vendor_rc { + VENDOR_RC_INTERNAL_ERROR = (VENDOR_RC_ERR | 6), + VENDOR_RC_NO_SUCH_COMMAND = (VENDOR_RC_ERR | 127), +}; + +enum cr50_tpm_mode { + /* + * Default state: TPM is enabled, and may be set to either + * TPM_MODE_ENABLED or TPM_MODE_DISABLED. + */ + TPM_MODE_ENABLED_TENTATIVE = 0, + + /* TPM is enabled, and mode may not be changed. */ + TPM_MODE_ENABLED = 1, + + /* TPM is disabled, and mode may not be changed. */ + TPM_MODE_DISABLED = 2, + + TPM_MODE_INVALID, +}; +
/** * CR50 specific tpm command to enable nvmem commits before internal timeout @@ -53,4 +79,26 @@ */ uint32_t tlcl_cr50_get_recovery_button(uint8_t *recovery_button_state);
+/** + * CR50 specific TPM command sequence to query the current TPM mode. + * + * Returns TPM_SUCCESS if TPM mode command completed, the Cr50 does not need a + * reboot, and the tpm_mode parameter is set to the current TPM mode. + * Returns TPM_E_MUST_REBOOT if TPM mode command completed, but the Cr50 + * requires a reboot. + * Returns TPM_E_NO_SUCH_COMMAND if the Cr50 does not support the command. + * Other returns value indicate a failure accessing the TPM. + */ +uint32_t tlcl_cr50_get_tpm_mode(uint8_t *tpm_mode); + +/** + * CR50 specific TPM command sequence to trigger an immediate reset to the Cr50 + * device after the specified timeout in milliseconds. A timeout of zero means + * "IMMEDIATE REBOOT". + * + * Return value indicates success or failure of accessing the TPM. + */ +uint32_t tlcl_cr50_immediate_reset(uint16_t timeout_ms); + + #endif /* CR50_TSS_STRUCTURES_H_ */ diff --git a/src/security/tpm/tss_errors.h b/src/security/tpm/tss_errors.h index 316661c..ed6fc3d 100644 --- a/src/security/tpm/tss_errors.h +++ b/src/security/tpm/tss_errors.h @@ -42,5 +42,6 @@ #define TPM_E_NV_DEFINED ((uint32_t)0x0000500b) /* vboot local */ #define TPM_E_INVALID_ARG ((uint32_t)0x0000500c) #define TPM_E_HASH_ERROR ((uint32_t)0x0000500d) +#define TPM_E_NO_SUCH_COMMAND ((uint32_t)0x0000500e)
#endif /* TSS_ERRORS_H_ */ diff --git a/src/vendorcode/google/chromeos/chromeos.h b/src/vendorcode/google/chromeos/chromeos.h index f7e2ae9..6831261 100644 --- a/src/vendorcode/google/chromeos/chromeos.h +++ b/src/vendorcode/google/chromeos/chromeos.h @@ -33,8 +33,11 @@ static inline void reboot_from_watchdog(void) { return; } #endif /* CONFIG_CHROMEOS */
-/* Defined as weak function in cr50_enable_update.c */ -void mainboard_cr50_update_reset(void); +/** + * Perform any platform specific actions required prior to resetting the Cr50. + * Defined as weak function in cr50_enable_update.c + */ +void mainboard_prepare_cr50_reset(void);
struct romstage_handoff;
diff --git a/src/vendorcode/google/chromeos/cr50_enable_update.c b/src/vendorcode/google/chromeos/cr50_enable_update.c index da9a16d..660fe2e 100644 --- a/src/vendorcode/google/chromeos/cr50_enable_update.c +++ b/src/vendorcode/google/chromeos/cr50_enable_update.c @@ -23,7 +23,75 @@ #include <security/vboot/vboot_common.h> #include <vendorcode/google/chromeos/chromeos.h>
-void __weak mainboard_cr50_update_reset(void) {} +#define C50_RESET_DELAY_MS 1000 + +void __weak mainboard_prepare_cr50_reset(void) {} + +/** + * Check if the Cr50 TPM state requires a chip reset of the Cr50 device. + * + * Returns 0 if the Cr50 TPM state is good or if the TPM_MODE command is + * unsupported. Returns 1 if the Cr50 was reset. + */ +static int cr50_reset_if_needed(uint16_t timeout_ms) +{ + int ret; + int cr50_must_reset = 0; + uint8_t tpm_mode; + + ret = tlcl_cr50_get_tpm_mode(&tpm_mode); + + if (ret == TPM_E_NO_SUCH_COMMAND) { + printk(BIOS_INFO, + "Cr50 does not support TPM mode command\n"); + /* Older Cr50 firmware, assume no Cr50 reset is required */ + return 0; + } + + if (ret == TPM_E_MUST_REBOOT) { + /* + * Cr50 indicated a reboot is required to restore TPM + * functionality. + */ + cr50_must_reset = 1; + } else if (ret != TPM_SUCCESS) { + /* TPM command failed, continue booting. */ + printk(BIOS_ERR, + "ERROR: Attempt to get CR50 TPM mode failed: %x\n", ret); + return 0; + } + + /* If the TPM mode is not enabled-tentative, then the TPM mode is locked + * and cannot be changed. Perform a Cr50 reset because vboot may need + * to disable TPM as part of booting an untrusted OS. + * + * This is not an expected state, as the Cr50 always sets the TPM mode + * to TPM_MODE_ENABLED_TENTATIVE during any TPM reset action. + */ + if (tpm_mode != TPM_MODE_ENABLED_TENTATIVE) { + printk(BIOS_NOTICE, + "NOTICE: Unexpected Cr50 TPM mode (%d). " + "A Cr50 reset is required.\n", tpm_mode); + cr50_must_reset = 1; + } + + /* If TPM state is okay, no reset needed. */ + if (!cr50_must_reset) + return 0; + + ret = tlcl_cr50_immediate_reset(timeout_ms); + + if (ret != TPM_SUCCESS) { + /* TPM command failed, continue booting. */ + printk(BIOS_ERR, + "ERROR: Attempt to reset CR50 failed: %x\n", + ret); + return 0; + } + + /* Cr50 is about to be reset, caller needs to prepare */ + return 1; +}
static void enable_update(void *unused) { @@ -37,34 +105,54 @@ ret = tlcl_lib_init();
if (ret != VB2_SUCCESS) { - printk(BIOS_ERR, "tlcl_lib_init() failed for CR50 update: %x\n", - ret); + printk(BIOS_ERR, + "ERROR: tlcl_lib_init() failed for CR50 update: %x\n", + ret); return; }
/* Reboot in 1000 ms if necessary. */ - ret = tlcl_cr50_enable_update(1000, &num_restored_headers); + ret = tlcl_cr50_enable_update(C50_RESET_DELAY_MS, + &num_restored_headers);
if (ret != TPM_SUCCESS) { - printk(BIOS_ERR, "Attempt to enable CR50 update failed: %x\n", - ret); + printk(BIOS_ERR, + "ERROR: Attempt to enable CR50 update failed: %x\n", + ret); return; }
- /* If no headers were restored there is no reset forthcoming. */ - if (!num_restored_headers) - return; + if (!num_restored_headers) { + /* If no headers were restored there is no reset forthcoming due + * to a Cr50 firmware update. Also check if the Cr50 TPM mode + * requires a reset. + * + * TODO: to eliminate a TPM command during every boot, the + * TURN_UPDATE_ON command could be enhanced/replaced in the Cr50 + * firmware to perform the TPM mode/key-ladder check in addition + * to the FW version check. + */ + + /* + * If the Cr50 was not reset, continue booting. + */ + if (!cr50_reset_if_needed(C50_RESET_DELAY_MS)) + return; + + printk(BIOS_INFO, "Waiting for CR50 reset to enable TPM.\n"); + elog_add_event(ELOG_TYPE_CR50_NEED_RESET); + } else { + printk(BIOS_INFO, + "Waiting for CR50 reset to pick up update.\n"); + elog_add_event(ELOG_TYPE_CR50_UPDATE); + }
/* Give mainboard a chance to take action */ - mainboard_cr50_update_reset(); - - elog_add_event(ELOG_TYPE_CR50_UPDATE); + mainboard_prepare_cr50_reset();
/* clear current post code avoid chatty eventlog on subsequent boot*/ post_code(0);
- printk(BIOS_INFO, "Waiting for CR50 reset to pick up update.\n"); - if (IS_ENABLED(CONFIG_POWER_OFF_ON_CR50_UPDATE)) poweroff(); halt();