vboot2: tpm error doesn't block gbb dev flag

In recovery mode, the TPM may be bad / corrupt.  This prevents access to
the soft developer switch stored in secdata.  But it should not prevent
setting dev mode via GBB or context flags.  Those flags may be set
during manufacturing or testing, and override the contents of secdata
anyway.

BUG=chrome-os-partner:45511
BRANCH=ryu
TEST=make runtests

Change-Id: I242714528203cc7cf78a714c660b7f8bbd0e04d0
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/300731
Reviewed-by: Furquan Shaikh <furquan@chromium.org>
Commit-Queue: Furquan Shaikh <furquan@chromium.org>
Trybot-Ready: Furquan Shaikh <furquan@chromium.org>
Tested-by: Furquan Shaikh <furquan@chromium.org>

2 files changed, 123 insertions, 51 deletions

diff --git a/firmware/2lib/2misc.c b/firmware/2lib/2misc.c
index 36a79b7a..5f1dc4be 100644
--- a/firmware/2lib/2misc.c
+++ b/firmware/2lib/2misc.c
@@ -204,33 +204,45 @@ int vb2_fw_parse_gbb(struct vb2_context *ctx)
 int vb2_check_dev_switch(struct vb2_context *ctx)
 {
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
-	uint32_t flags;
+	uint32_t flags = 0;
 	uint32_t old_flags;
 	int is_dev = 0;
+	int use_secdata = 1;
 	int rv;
 
 	/* Read secure flags */
 	rv = vb2_secdata_get(ctx, VB2_SECDATA_FLAGS, &flags);
-	if (rv)
-		return rv;
-
+	if (rv) {
+		if (ctx->flags & VB2_CONTEXT_RECOVERY_MODE) {
+			/*
+			 * Recovery mode needs to check other ways developer
+			 * mode can be enabled, so don't give up yet.  But
+			 * since we can't read secdata, assume dev mode was
+			 * disabled.
+			 */
+			use_secdata = 0;
+			flags = 0;
+		} else {
+			/* Normal mode simply fails */
+			return rv;
+		}
+	}
 	old_flags = flags;
 
 	/* Handle dev disable request */
-	if (vb2_nv_get(ctx, VB2_NV_DISABLE_DEV_REQUEST)) {
+	if (use_secdata && vb2_nv_get(ctx, VB2_NV_DISABLE_DEV_REQUEST)) {
 		flags &= ~VB2_SECDATA_FLAG_DEV_MODE;
 
 		/* Clear the request */
 		vb2_nv_set(ctx, VB2_NV_DISABLE_DEV_REQUEST, 0);
 	}
 
-	if (ctx->flags & VB2_DISABLE_DEVELOPER_MODE) {
-		/*
-		 * Hardware switch and GBB flag will take precedence over
-		 * this.
-		 */
+	/*
+	 * Check if we've been asked by the caller to disable dev mode.  Note
+	 * that hardware switch and GBB flag will take precedence over this.
+	 */
+	if (ctx->flags & VB2_DISABLE_DEVELOPER_MODE)
 		flags &= ~VB2_SECDATA_FLAG_DEV_MODE;
-	}
 
 	/* Check virtual dev switch */
 	if (flags & VB2_SECDATA_FLAG_DEV_MODE)
@@ -275,22 +287,31 @@ int vb2_check_dev_switch(struct vb2_context *ctx)
 		 * done here instead of simply passing a flag to
 		 * vb2_check_tpm_clear(), because we don't want to update
 		 * last_boot_developer and then fail to clear the TPM owner.
+		 *
+		 * Note that we do this even if we couldn't read secdata, since
+		 * the TPM owner and secdata may be independent, and we want
+		 * the owner to be cleared if *this boot* is different than the
+		 * last one (perhaps due to GBB or hardware override).
 		 */
 		rv = vb2ex_tpm_clear_owner(ctx);
-		if (rv) {
-			/*
-			 * Note that this truncates rv to 8 bit.  Which is not
-			 * as useful as the full error code, but we don't have
-			 * NVRAM space to store the full 32-bit code.
-			 */
-			vb2_fail(ctx, VB2_RECOVERY_TPM_CLEAR_OWNER, rv);
-			return rv;
+		if (use_secdata) {
+			/* Check for failure to clear owner */
+			if (rv) {
+				/*
+				 * Note that this truncates rv to 8 bit.  Which
+				 * is not as useful as the full error code, but
+				 * we don't have NVRAM space to store the full
+				 * 32-bit code.
+				 */
+				vb2_fail(ctx, VB2_RECOVERY_TPM_CLEAR_OWNER, rv);
+				return rv;
+			}
+
+			/* Save new flags */
+			rv = vb2_secdata_set(ctx, VB2_SECDATA_FLAGS, flags);
+			if (rv)
+				return rv;
 		}
-
-		/* Save new flags */
-		rv = vb2_secdata_set(ctx, VB2_SECDATA_FLAGS, flags);
-		if (rv)
-			return rv;
 	}
 
 	return VB2_SUCCESS;
diff --git a/tests/vb2_misc_tests.c b/tests/vb2_misc_tests.c
index b8eee343..99414bad 100644
--- a/tests/vb2_misc_tests.c
+++ b/tests/vb2_misc_tests.c
@@ -290,9 +290,9 @@ static void dev_switch_tests(void)
 	/* Normal mode */
 	reset_common_data();
 	TEST_SUCC(vb2_check_dev_switch(&cc), "dev mode off");
-	TEST_EQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "sd not in dev");
-	TEST_EQ(cc.flags & VB2_CONTEXT_DEVELOPER_MODE, 0, "ctx not in dev");
-	TEST_EQ(mock_tpm_clear_called, 0, "no tpm clear");
+	TEST_EQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "  sd not in dev");
+	TEST_EQ(cc.flags & VB2_CONTEXT_DEVELOPER_MODE, 0, "  ctx not in dev");
+	TEST_EQ(mock_tpm_clear_called, 0, "  no tpm clear");
 
 	/* Dev mode */
 	reset_common_data();
@@ -300,9 +300,9 @@ static void dev_switch_tests(void)
 			(VB2_SECDATA_FLAG_DEV_MODE |
 			 VB2_SECDATA_FLAG_LAST_BOOT_DEVELOPER));
 	TEST_SUCC(vb2_check_dev_switch(&cc), "dev mode on");
-	TEST_NEQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "sd in dev");
-	TEST_NEQ(cc.flags & VB2_CONTEXT_DEVELOPER_MODE, 0, "ctx in dev");
-	TEST_EQ(mock_tpm_clear_called, 0, "no tpm clear");
+	TEST_NEQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "  sd in dev");
+	TEST_NEQ(cc.flags & VB2_CONTEXT_DEVELOPER_MODE, 0, "  ctx in dev");
+	TEST_EQ(mock_tpm_clear_called, 0, "  no tpm clear");
 
 	/* Any normal mode boot clears dev boot flags */
 	reset_common_data();
@@ -313,32 +313,32 @@ static void dev_switch_tests(void)
 	vb2_nv_set(&cc, VB2_NV_FASTBOOT_UNLOCK_IN_FW, 1);
 	TEST_SUCC(vb2_check_dev_switch(&cc), "dev mode off");
 	TEST_EQ(vb2_nv_get(&cc, VB2_NV_DEV_BOOT_USB),
-		0, "cleared dev boot usb");
+		0, "  cleared dev boot usb");
 	TEST_EQ(vb2_nv_get(&cc, VB2_NV_DEV_BOOT_LEGACY),
-		0, "cleared dev boot legacy");
+		0, "  cleared dev boot legacy");
 	TEST_EQ(vb2_nv_get(&cc, VB2_NV_DEV_BOOT_SIGNED_ONLY),
-		0, "cleared dev boot signed only");
+		0, "  cleared dev boot signed only");
 	TEST_EQ(vb2_nv_get(&cc, VB2_NV_DEV_BOOT_FASTBOOT_FULL_CAP),
-		0, "cleared dev boot fastboot full cap");
+		0, "  cleared dev boot fastboot full cap");
 
 	/* Normal-dev transition clears TPM */
 	reset_common_data();
 	vb2_secdata_set(&cc, VB2_SECDATA_FLAGS, VB2_SECDATA_FLAG_DEV_MODE);
 	TEST_SUCC(vb2_check_dev_switch(&cc), "to dev mode");
-	TEST_EQ(mock_tpm_clear_called, 1, "tpm clear");
+	TEST_EQ(mock_tpm_clear_called, 1, "  tpm clear");
 	vb2_secdata_get(&cc, VB2_SECDATA_FLAGS, &v);
 	TEST_EQ(v, (VB2_SECDATA_FLAG_DEV_MODE |
 		    VB2_SECDATA_FLAG_LAST_BOOT_DEVELOPER),
-		"last boot developer now");
+		"  last boot developer now");
 
 	/* Dev-normal transition clears TPM too */
 	reset_common_data();
 	vb2_secdata_set(&cc, VB2_SECDATA_FLAGS,
 			VB2_SECDATA_FLAG_LAST_BOOT_DEVELOPER);
 	TEST_SUCC(vb2_check_dev_switch(&cc), "from dev mode");
-	TEST_EQ(mock_tpm_clear_called, 1, "tpm clear");
+	TEST_EQ(mock_tpm_clear_called, 1, "  tpm clear");
 	vb2_secdata_get(&cc, VB2_SECDATA_FLAGS, &v);
-	TEST_EQ(v, 0, "last boot not developer now");
+	TEST_EQ(v, 0, "  last boot not developer now");
 
 	/* Disable dev mode */
 	reset_common_data();
@@ -347,29 +347,29 @@ static void dev_switch_tests(void)
 			 VB2_SECDATA_FLAG_LAST_BOOT_DEVELOPER));
 	vb2_nv_set(&cc, VB2_NV_DISABLE_DEV_REQUEST, 1);
 	TEST_SUCC(vb2_check_dev_switch(&cc), "disable dev request");
-	TEST_EQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "sd not in dev");
+	TEST_EQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "  sd not in dev");
 	TEST_EQ(vb2_nv_get(&cc, VB2_NV_DISABLE_DEV_REQUEST),
-		0, "request cleared");
+		0, "  request cleared");
 
-	/* Force enabled by gbb */
+	/* Force enabled by GBB */
 	reset_common_data();
 	sd->gbb_flags |= VB2_GBB_FLAG_FORCE_DEV_SWITCH_ON;
 	TEST_SUCC(vb2_check_dev_switch(&cc), "dev on via gbb");
-	TEST_NEQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "sd in dev");
+	TEST_NEQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "  sd in dev");
 	vb2_secdata_get(&cc, VB2_SECDATA_FLAGS, &v);
 	TEST_EQ(v, VB2_SECDATA_FLAG_LAST_BOOT_DEVELOPER,
-		"doesn't set dev on in secdata but does set last boot dev");
-	TEST_EQ(mock_tpm_clear_called, 1, "tpm clear");
+		"  doesn't set dev on in secdata but does set last boot dev");
+	TEST_EQ(mock_tpm_clear_called, 1, "  tpm clear");
 
 	/* Force enabled by ctx flag */
 	reset_common_data();
 	cc.flags |= VB2_CONTEXT_FORCE_DEVELOPER_MODE;
 	TEST_SUCC(vb2_check_dev_switch(&cc), "dev on via ctx flag");
-	TEST_NEQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "sd in dev");
+	TEST_NEQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "  sd in dev");
 	vb2_secdata_get(&cc, VB2_SECDATA_FLAGS, &v);
 	TEST_EQ(v, VB2_SECDATA_FLAG_LAST_BOOT_DEVELOPER,
-		"doesn't set dev on in secdata but does set last boot dev");
-	TEST_EQ(mock_tpm_clear_called, 1, "tpm clear");
+		"  doesn't set dev on in secdata but does set last boot dev");
+	TEST_EQ(mock_tpm_clear_called, 1, "  tpm clear");
 
 	/* Simulate clear owner failure */
 	reset_common_data();
@@ -378,14 +378,65 @@ static void dev_switch_tests(void)
 	mock_tpm_clear_retval = VB2_ERROR_EX_TPM_CLEAR_OWNER;
 	TEST_EQ(vb2_check_dev_switch(&cc),
 		VB2_ERROR_EX_TPM_CLEAR_OWNER, "tpm clear fail");
-	TEST_EQ(mock_tpm_clear_called, 1, "tpm clear");
+	TEST_EQ(mock_tpm_clear_called, 1, "  tpm clear");
 	vb2_secdata_get(&cc, VB2_SECDATA_FLAGS, &v);
 	TEST_EQ(v, VB2_SECDATA_FLAG_LAST_BOOT_DEVELOPER,
-		"last boot still developer");
+		"  last boot still developer");
 	TEST_EQ(vb2_nv_get(&cc, VB2_NV_RECOVERY_REQUEST),
-		VB2_RECOVERY_TPM_CLEAR_OWNER, "requests recovery");
+		VB2_RECOVERY_TPM_CLEAR_OWNER, "  requests recovery");
 	TEST_EQ(vb2_nv_get(&cc, VB2_NV_RECOVERY_SUBCODE),
-		(uint8_t)VB2_ERROR_EX_TPM_CLEAR_OWNER, "recovery subcode");
+		(uint8_t)VB2_ERROR_EX_TPM_CLEAR_OWNER, "  recovery subcode");
+
+	/*
+	 * Secdata failure in normal mode fails and shows dev=0 even if dev
+	 * mode was on in the (inaccessible) secdata.
+	 */
+	reset_common_data();
+	vb2_secdata_set(&cc, VB2_SECDATA_FLAGS, VB2_SECDATA_FLAG_DEV_MODE);
+	sd->status &= ~VB2_SD_STATUS_SECDATA_INIT;
+	TEST_EQ(vb2_check_dev_switch(&cc), VB2_ERROR_SECDATA_GET_UNINITIALIZED,
+		"secdata fail normal");
+	TEST_EQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "  sd not in dev");
+	TEST_EQ(cc.flags & VB2_CONTEXT_DEVELOPER_MODE, 0, "  ctx not in dev");
+
+	/* Secdata failure in recovery mode continues */
+	reset_common_data();
+	cc.flags |= VB2_CONTEXT_RECOVERY_MODE;
+	sd->status &= ~VB2_SD_STATUS_SECDATA_INIT;
+	TEST_SUCC(vb2_check_dev_switch(&cc), "secdata fail recovery");
+	TEST_EQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "  sd not in dev");
+	TEST_EQ(cc.flags & VB2_CONTEXT_DEVELOPER_MODE, 0, "  ctx not in dev");
+
+	/* And doesn't check or clear dev disable request */
+	reset_common_data();
+	cc.flags |= VB2_CONTEXT_RECOVERY_MODE;
+	sd->status &= ~VB2_SD_STATUS_SECDATA_INIT;
+	vb2_nv_set(&cc, VB2_NV_DISABLE_DEV_REQUEST, 1);
+	TEST_SUCC(vb2_check_dev_switch(&cc), "secdata fail recovery disable");
+	TEST_EQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "  sd not in dev");
+	TEST_EQ(cc.flags & VB2_CONTEXT_DEVELOPER_MODE, 0, "  ctx not in dev");
+	TEST_EQ(vb2_nv_get(&cc, VB2_NV_DISABLE_DEV_REQUEST),
+		1, "  request not cleared");
+
+	/* Can still override with GBB flag */
+	reset_common_data();
+	cc.flags |= VB2_CONTEXT_RECOVERY_MODE;
+	sd->status &= ~VB2_SD_STATUS_SECDATA_INIT;
+	sd->gbb_flags |= VB2_GBB_FLAG_FORCE_DEV_SWITCH_ON;
+	TEST_SUCC(vb2_check_dev_switch(&cc), "secdata fail recovery gbb");
+	TEST_NEQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "  sd in dev");
+	TEST_NEQ(cc.flags & VB2_CONTEXT_DEVELOPER_MODE, 0, "  ctx in dev");
+	TEST_EQ(mock_tpm_clear_called, 1, "  tpm clear");
+
+	/* Can still override with context flag */
+	reset_common_data();
+	cc.flags |= VB2_CONTEXT_RECOVERY_MODE;
+	cc.flags |= VB2_CONTEXT_FORCE_DEVELOPER_MODE;
+	sd->status &= ~VB2_SD_STATUS_SECDATA_INIT;
+	TEST_SUCC(vb2_check_dev_switch(&cc), "secdata fail recovery ctx");
+	TEST_NEQ(sd->flags & VB2_SD_DEV_MODE_ENABLED, 0, "  sd in dev");
+	TEST_NEQ(cc.flags & VB2_CONTEXT_DEVELOPER_MODE, 0, "  ctx in dev");
+	TEST_EQ(mock_tpm_clear_called, 1, "  tpm clear");
 }
 
 static void tpm_clear_tests(void)