summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRandall Spangler <rspangler@chromium.org>2018-05-18 15:02:40 -0700
committerchrome-bot <chrome-bot@chromium.org>2018-05-23 20:35:12 -0700
commitf07e300fe457575394c21d040cfc80e6dc2829f0 (patch)
tree2957d7410b0fd17095ba46a9e9c3e809242aa99e
parent7784fba59d7eb7992c2f0e2df24a819fe06c2828 (diff)
downloadchrome-ec-f07e300fe457575394c21d040cfc80e6dc2829f0.tar.gz
cr50: tpm_alt_extension() specifies command origin is USB
Previously, calls to tpm_alt_extension() were treated as if they came from the AP via the TPM interface, even though they actually originated from the cr50 console, which is accessible via the USB interface. This affects the following console commands: spi_hash - was already allowed as both a safe console command and via the USB vendor command interface. No change. rma_auth - was allowed as a safe console command, but not via the USB vendor command interface. Now allowed from both. No change in security, since anyone could already do it via the console. Unfortunately, getting a challenge fails because commands issued via the USB vendor command interface have a maximum payload of 32 bytes and the challenge is bigger than that; that's tracked in b:80098603. ccd - was already allowed as a safe console command. This directly called ccd_command_wrapper() for lock, open, and password subcommands. It made an extra check for password set for the unlock subcommand. Moved the unlock check to the vendor command handler. Also changed the order of checks so that FWMP disabling unlock and open supersedes an existing password; this matches go/ccd-open-simple. (That has no effect on existing systems, because CCD is disabled at a higher level.) Reduces code size by 8 bytes. BUG=b:79983505 BRANCH=cr50 TEST=manual, on a CR50_DEV=1 build Compile with DEBUG_EXTENSION defined to print extra debug output 'ccd lock' now shows as coming from USB 'ccd unlock' fails because no password is set 'ccd unlock' and 'ccd open' fail if FWMP disallows unlock 'rma_auth' prints a challenge 'gsctool -t -r' prints a challenge from AP root shell 'gsctool -r 12345678' returns error 6 (incorrect challenge), rather than error 127 (no such command). 'gsctool -I' works from the host 'gsctool -t -I' still works from AP root shell Change-Id: I2cd1027f5135b9c336df97ee4b1b1a15354728b4 Signed-off-by: Randall Spangler <rspangler@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1068102 Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
-rw-r--r--common/ccd_config.c71
-rw-r--r--common/extension.c2
-rw-r--r--common/tpm_registers.c9
-rw-r--r--include/tpm_registers.h4
4 files changed, 41 insertions, 45 deletions
diff --git a/common/ccd_config.c b/common/ccd_config.c
index 74b20faed7..78fc822d9f 100644
--- a/common/ccd_config.c
+++ b/common/ccd_config.c
@@ -859,6 +859,14 @@ static enum vendor_cmd_rc ccd_open(void *buf,
if (ccd_state == CCD_STATE_OPENED)
return VENDOR_RC_SUCCESS;
+ /* FWMP blocks open even if a password is set */
+ if (!board_fwmp_allows_unlock()) {
+ *response_size = 1;
+ buffer[0] = EC_ERROR_ACCESS_DENIED;
+ return VENDOR_RC_NOT_ALLOWED;
+ }
+
+ /* If a password is set, check it */
if (raw_has_password()) {
if (!input_size) {
*response_size = 1;
@@ -877,10 +885,6 @@ static enum vendor_cmd_rc ccd_open(void *buf,
buffer[0] = rv;
return VENDOR_RC_INTERNAL_ERROR;
}
- } else if (!board_fwmp_allows_unlock()) {
- *response_size = 1;
- buffer[0] = EC_ERROR_ACCESS_DENIED;
- return VENDOR_RC_NOT_ALLOWED;
}
/* Fail and abort if already checking physical presence */
@@ -944,47 +948,30 @@ static enum vendor_cmd_rc ccd_unlock(void *buf,
return VENDOR_RC_SUCCESS;
}
- if (raw_has_password()) {
- if (!input_size) {
- *response_size = 1;
- buffer[0] = EC_ERROR_PARAM_COUNT;
- return VENDOR_RC_PASSWORD_REQUIRED;
- }
-
- /*
- * We know there is plenty of room in the TPM buffer this is
- * stored in.
- */
- buffer[input_size] = '\0';
- rv = raw_check_password(buffer);
- if (rv) {
- *response_size = 1;
- buffer[0] = rv;
- return VENDOR_RC_INTERNAL_ERROR;
- }
- } else if (!board_fwmp_allows_unlock()) {
+ /* Only allowed if password is already set, and not blocked by FWMP */
+ if (!raw_has_password() || !board_fwmp_allows_unlock()) {
*response_size = 1;
buffer[0] = EC_ERROR_ACCESS_DENIED;
return VENDOR_RC_NOT_ALLOWED;
- } else {
- /*
- * When unlock is requested via the console, physical presence
- * is required unless disabled by config. This prevents a
- * malicious peripheral from setitng a password.
- *
- * If this were a TPM vendor command from the AP, we would
- * instead check unlock restrictions based on the user login
- * state stored in ccd_unlock_restrict:
- *
- * 1) Unlock from the AP is unrestricted before any users
- * login, so enrollment policy scripts can update CCD config.
- *
- * 2) Owner accounts can unlock, but require physical presence
- * to prevent OS-level compromises from setting a password.
- *
- * 3) A non-owner account logging in blocks CCD config until
- * the next AP reboot, as implied by TPM reboot.
- */
+ }
+
+ /* Make sure password was specified */
+ if (!input_size) {
+ *response_size = 1;
+ buffer[0] = EC_ERROR_PARAM_COUNT;
+ return VENDOR_RC_PASSWORD_REQUIRED;
+ }
+
+ /*
+ * Check the password. We know there is plenty of room in the TPM
+ * buffer this is stored in.
+ */
+ buffer[input_size] = '\0';
+ rv = raw_check_password(buffer);
+ if (rv) {
+ *response_size = 1;
+ buffer[0] = rv;
+ return VENDOR_RC_INTERNAL_ERROR;
}
/* Fail and abort if already checking physical presence */
diff --git a/common/extension.c b/common/extension.c
index f63734025f..d75bbbe4f3 100644
--- a/common/extension.c
+++ b/common/extension.c
@@ -35,7 +35,9 @@ uint32_t extension_route_command(uint16_t command_code,
case VENDOR_CC_SET_BOARD_ID:
#endif /* defined(CR50_DEV) */
case EXTENSION_POST_RESET: /* Always need to reset. */
+ case VENDOR_CC_CCD:
case VENDOR_CC_GET_BOARD_ID:
+ case VENDOR_CC_RMA_CHALLENGE_RESPONSE:
case VENDOR_CC_SPI_HASH: /* Requires physical presence. */
case VENDOR_CC_TURN_UPDATE_ON:
break;
diff --git a/common/tpm_registers.c b/common/tpm_registers.c
index 959ca1c814..a8a3546109 100644
--- a/common/tpm_registers.c
+++ b/common/tpm_registers.c
@@ -630,7 +630,8 @@ size_t tpm_get_burst_size(void)
(code & TPM_CC_VENDOR_BIT_MASK))
static void call_extension_command(struct tpm_cmd_header *tpmh,
- size_t *total_size)
+ size_t *total_size,
+ uint32_t flags)
{
size_t command_size = be32toh(tpmh->size);
uint32_t rc;
@@ -653,7 +654,7 @@ static void call_extension_command(struct tpm_cmd_header *tpmh,
command_size -
sizeof(struct tpm_cmd_header),
total_size,
- 0);
+ flags);
/* Add the header size back. */
*total_size += sizeof(struct tpm_cmd_header);
tpmh->size = htobe32(*total_size);
@@ -995,7 +996,9 @@ void tpm_task(void)
#ifdef CONFIG_EXTENSION_COMMAND
if (IS_CUSTOM_CODE(command_code)) {
response_size = buffer_size;
- call_extension_command(tpmh, &response_size);
+ call_extension_command(tpmh, &response_size,
+ alt_if_command ?
+ VENDOR_CMD_FROM_USB : 0);
} else
#endif
{
diff --git a/include/tpm_registers.h b/include/tpm_registers.h
index b82a355170..0cbcdca39d 100644
--- a/include/tpm_registers.h
+++ b/include/tpm_registers.h
@@ -91,6 +91,10 @@ struct tpm_cmd_header {
* TPM task how much room there is to store the response.
*
* Command execution result is reported in the response body.
+ *
+ * The extension command handler will consider all these commands to come from
+ * the USB interface, since the only current users for this are console
+ * commands.
*/
void tpm_alt_extension(struct tpm_cmd_header *tpmh, size_t buffer_size);