diff options
author | Randall Spangler <rspangler@chromium.org> | 2018-05-24 14:56:14 -0700 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2018-05-25 20:31:57 -0700 |
commit | d7705eb311f919ab4c93aeea401ba58771c28dd4 (patch) | |
tree | c233b833fa4fe944f8ccfaa57e9a638de90bcffa /common | |
parent | b3218b9533b607dd53fec13671e3d91b50c0122a (diff) | |
download | chrome-ec-d7705eb311f919ab4c93aeea401ba58771c28dd4.tar.gz |
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 <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1072957
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Diffstat (limited to 'common')
-rw-r--r-- | common/ccd_config.c | 149 |
1 files changed, 41 insertions, 108 deletions
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) @@ -1177,104 +1200,6 @@ DECLARE_SAFE_CONSOLE_COMMAND(ccd, command_ccd, "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. * * The payload of the command is a text string to use to set or clear the @@ -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; |