From d7705eb311f919ab4c93aeea401ba58771c28dd4 Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Thu, 24 May 2018 14:56:14 -0700 Subject: ccd_config: Simplify open and password Allow setting password from the AP, but not from USB. Remove the old password control logic, which is no longer needed. Allow open if: - Not explicitly blocked - Not blocked via FWMP - One of the following is true: - A password is set - Battery is removed (also doesn't require physical presence) - Dev mode is on, and request came from the AP Reduces cr50 binary by 152 bytes. BUG=b:79983505 BRANCH=cr50 TEST=manual, with a CR50_DEV=1 build ccd oops ccd lock ccd unlock -> fails gsctool -U -> fails from host gsctool -t -U -> fails from AP ccd oops ccd password foo -> fails from console gsctool -P -> fails from host gsctool -t -P -> works from AP ccd get -> confirms password set ccd lock ccd unlock foo -> works ccd lock gsctool -U -> works from host, if correct password supplied ccd lock gsctool -t -U -> works from AP, if correct password supplied ccd open foo -> works ccd lock gsctool -O -> works from host, if correct password supplied ccd lock gsctool -t -O -> works from AP, if correct password supplied ccd oops ccd lock (remove battery) ccd open -> works without physical presence (reattach battery) ccd lock gsctool -O -> works from host ccd lock gsctool -t -O -> works from AP, if dev mode is enabled Change-Id: I364b322d03db250e7dd140767d7a22dbb3ac1eef Signed-off-by: Randall Spangler Reviewed-on: https://chromium-review.googlesource.com/1072957 Reviewed-by: Vadim Bendebury --- board/cr50/tpm2/platform.c | 17 +++++- common/ccd_config.c | 149 +++++++++++++-------------------------------- include/tpm_vendor_cmds.h | 10 +-- 3 files changed, 60 insertions(+), 116 deletions(-) diff --git a/board/cr50/tpm2/platform.c b/board/cr50/tpm2/platform.c index 3068c903e3..a275a82264 100644 --- a/board/cr50/tpm2/platform.c +++ b/board/cr50/tpm2/platform.c @@ -66,5 +66,20 @@ void _plat__GetFwVersion(uint32_t *firmwareV1, uint32_t *firmwareV2) void _plat__ResetCallback(void) { pinweaver_init(); - ccd_tpm_reset_callback(); + + /* + * Eventually, we'll want to allow CCD unlock with no password, so + * enterprise policy can set a password to block CCD instead of locking + * it out via the FWMP. + * + * When we do that, we'll allow unlock without password between a real + * TPM startup (not just a resume) - which is this callback - and + * explicit disabling of that feature via a to-be-created vendor + * command. That vendor command will be called after enterprize policy + * is updated, or the device is determined not to be enrolled. + * + * But for now, we'll just block unlock entirely if no password is set, + * so we don't yet need to tell CCD that a real TPM startup has + * occurred. + */ } diff --git a/common/ccd_config.c b/common/ccd_config.c index 677eb025ca..28f393d87d 100644 --- a/common/ccd_config.c +++ b/common/ccd_config.c @@ -838,26 +838,26 @@ static enum vendor_cmd_rc ccd_open(struct vendor_cmd_params *p) int need_pp = 1; int rv; char *buffer = p->buffer; + const char *why_denied = "forced"; - if (force_disabled) { - p->out_size = 1; - buffer[0] = EC_ERROR_ACCESS_DENIED; - return VENDOR_RC_NOT_ALLOWED; - } + if (force_disabled) + goto denied; if (ccd_state == CCD_STATE_OPENED) return VENDOR_RC_SUCCESS; /* FWMP blocks open even if a password is set */ if (!board_fwmp_allows_unlock()) { - p->out_size = 1; - buffer[0] = EC_ERROR_ACCESS_DENIED; - return VENDOR_RC_NOT_ALLOWED; + why_denied = "fwmp"; + goto denied; } - /* If a password is set, check it */ + /* Make sure open is allowed */ if (raw_has_password()) { + /* Open allowed if correct password is specified */ + if (!p->in_size) { + /* ...which it wasn't */ p->out_size = 1; buffer[0] = EC_ERROR_PARAM_COUNT; return VENDOR_RC_PASSWORD_REQUIRED; @@ -874,6 +874,22 @@ static enum vendor_cmd_rc ccd_open(struct vendor_cmd_params *p) buffer[0] = rv; return VENDOR_RC_INTERNAL_ERROR; } + } else if (!board_battery_is_present()) { + /* Open allowed with no password if battery is removed */ + } else if (board_vboot_dev_mode_enabled() && + !(p->flags & VENDOR_CMD_FROM_USB)) { + /* + * Open allowed with no password if dev mode enabled and + * command came from the AP. + */ + } else { + /* + * - Password not set + * - Battery is present + * - Either not in developer mode or the command came from USB + */ + why_denied = "nopwd"; + goto denied; } /* Fail and abort if already checking physical presence */ @@ -912,6 +928,13 @@ static enum vendor_cmd_rc ccd_open(struct vendor_cmd_params *p) ccd_open_done(1); return VENDOR_RC_SUCCESS; + +denied: + /* Open not allowed for some reason */ + CPRINTS("%s denied: %s", __func__, why_denied); + p->out_size = 1; + buffer[0] = EC_ERROR_ACCESS_DENIED; + return VENDOR_RC_NOT_ALLOWED; } static enum vendor_cmd_rc ccd_unlock(struct vendor_cmd_params *p) @@ -1176,104 +1199,6 @@ DECLARE_SAFE_CONSOLE_COMMAND(ccd, command_ccd, "[help | ...]", "Configure case-closed debugging"); -/* - * Password handling on Cr50 passes the following states: - * - * - password setting is not allowed after Cr50 reset until an upstart (as - * opposed to resume) TPM startup happens, as signalled by the TPM callback. - * After the proper TPM reset the state changes to 'POST_RESET_STATE' which - * means that the device was just reset/rebooted (not resumed) and no user - * logged in yet. - * - * - if the owner logs in in this state, the state changes to - * 'PASSWORD_ALLOWED_STATE'. The owner can open crosh session and set the - * password. - * - * - when the owner logs out or any user but the owner logs in, the state - * changes to PASSWORD_NOT_ALLOWED_STATE and does not change until TPM is - * reset. This makes sure that password can be set only by the owner and - * only before anybody else logged in. - */ -enum password_reset_phase { - POST_RESET_STATE, - PASSWORD_ALLOWED_STATE, - PASSWORD_NOT_ALLOWED_STATE -}; - -static uint8_t password_state = PASSWORD_NOT_ALLOWED_STATE; - -void ccd_tpm_reset_callback(void) -{ - CPRINTS("%s: TPM Startup processed", __func__); - password_state = POST_RESET_STATE; -} - -/* - * Handle the VENDOR_CC_MANAGE_CCD_PASSWORD command. - * - * The payload of the command is a single byte Boolean which sets the controls - * if CCD password can be set or not. - * - * After reset the pasword can not be set using VENDOR_CC_CCD_PASSWORD; once - * this command is received with value of True, the phase stars when the - * password can be set. As soon as this command is received with a value of - * False, the password can not be set any more until device is rebooted, even - * if this command is re-sent with the value of True. - */ -static enum vendor_cmd_rc manage_ccd_password(enum vendor_cmd_cc code, - void *buf, - size_t input_size, - size_t *response_size) -{ - uint8_t prev_state = password_state; - /* The vendor command status code. */ - enum vendor_cmd_rc rv = VENDOR_RC_SUCCESS; - /* Actual error code. */ - uint8_t error_code = EC_SUCCESS; - - do { - int value; - - if (input_size != 1) { - rv = VENDOR_RC_INTERNAL_ERROR; - error_code = EC_ERROR_PARAM1; - break; - } - - value = *((uint8_t *)buf); - - if (!value) { - /* No more password setting allowed. */ - password_state = PASSWORD_NOT_ALLOWED_STATE; - break; - } - - if (password_state == POST_RESET_STATE) { - /* The only way to allow password setting. */ - password_state = PASSWORD_ALLOWED_STATE; - break; - } - - password_state = PASSWORD_NOT_ALLOWED_STATE; - rv = VENDOR_RC_BOGUS_ARGS; - error_code = EC_ERROR_INVAL; - } while (0); - - if (prev_state != password_state) - CPRINTF("%s: state change from %d to %d\n", - __func__, prev_state, password_state); - - if (rv == VENDOR_RC_SUCCESS) { - *response_size = 0; - } else { - *response_size = 1; - ((uint8_t *)buf)[0] = error_code; - } - - return rv; -} -DECLARE_VENDOR_COMMAND(VENDOR_CC_MANAGE_CCD_PWD, manage_ccd_password); - /* * Handle the CCVD_PASSWORD subcommand. * @@ -1286,7 +1211,15 @@ static enum vendor_cmd_rc ccd_password(struct vendor_cmd_params *p) char password[CCD_MAX_PASSWORD_SIZE + 1]; char *response = p->buffer; - if (password_state != PASSWORD_ALLOWED_STATE) { + /* + * Only allow setting a password from the AP, not USB. This increases + * the effort required for an attacker to set one externally, even if + * they have access to a system someone left in the opened state. + * + * An attacker can still set testlab mode or open up the CCD config, + * but those changes are reversible by the device owner. + */ + if (p->flags & VENDOR_CMD_FROM_USB) { p->out_size = 1; *response = EC_ERROR_ACCESS_DENIED; return VENDOR_RC_NOT_ALLOWED; diff --git a/include/tpm_vendor_cmds.h b/include/tpm_vendor_cmds.h index 2d460badfa..72dc670b7d 100644 --- a/include/tpm_vendor_cmds.h +++ b/include/tpm_vendor_cmds.h @@ -35,9 +35,7 @@ enum vendor_cmd_cc { VENDOR_CC_IMMEDIATE_RESET = 19, VENDOR_CC_INVALIDATE_INACTIVE_RW = 20, VENDOR_CC_COMMIT_NVMEM = 21, - - /* A gap left for the deep sleep control command. */ - + /* DEPRECATED(22): deep sleep control command. */ VENDOR_CC_REPORT_TPM_STATE = 23, VENDOR_CC_TURN_UPDATE_ON = 24, VENDOR_CC_GET_BOARD_ID = 25, @@ -46,11 +44,9 @@ enum vendor_cmd_cc { VENDOR_CC_POP_LOG_ENTRY = 28, VENDOR_CC_GET_REC_BTN = 29, VENDOR_CC_RMA_CHALLENGE_RESPONSE = 30, - - /* A gap left for the no longer supported CCD password command. */ - + /* DEPRECATED(31): CCD password command (now part of VENDOR_CC_CCD) */ VENDOR_CC_DISABLE_RMA = 32, - VENDOR_CC_MANAGE_CCD_PWD = 33, + /* DEPRECATED(33): Manage CCD password phase */ VENDOR_CC_CCD = 34, VENDOR_CC_GET_ALERTS_DATA = 35, VENDOR_CC_SPI_HASH = 36, -- cgit v1.2.1