apro: fail if the V1 data is corrupted

It shouldn't be possible to have an unsupported ap_ro_check type and
the data shouldn't get corrupted. Fail verification, so the user can
tell that something is wrong. If the space is empty or the board id is
blocked, still treat verification as unsupported and allow the device to
boot.

Move the AP RO failed processing into a function, so cr50 can fail
immediately.

BUG=none
TEST=manual
	use a DBG image to write ap_ro_check data with the wrong type.
	Verify verification fails immediately
	[65.918056 RO Validation triggered]
	[65.920169 ap_ro_check_unsupported: unable to read ap ro space]
	[65.922733 do_ap_ro_check: bad v1 data]
	[65.924049 enable_spi_pinmux: AP]
	[65.927314 spi_hash_pp_done: AP]
	[65.928829 spi_hash_disable]
	[65.929904 AP RO FAILED!]

	Erase V1 data. Check verification is skipped because it's
	unsupported.
	[3.724384 RO Validation triggered]
	[3.726524 ap_ro_check_unsupported: RO verification not programmed]
	[3.728363 do_ap_ro_check: unsupported]
	[3.906272 AP UART on]
	[4.296054 deferred_tpm_rst_isr]
	[4.297027 AP on]
	[4.297588 tpm_reset_request(0, 0)]
	[4.298374 tpm_reset_now(0)]
	[4.299095 Committing NVMEM changes.]

	Write V1 data normally. Check verification runs normally.
	[35.977050 RO Validation triggered]
	[35.978744 do_ap_ro_check: found v1 data]
	[35.979732 enable_spi_pinmux: AP]
	[35.982574 spi_hash_pp_done: AP]
	[35.983276 get_saved_gbbd: not programmed]
	[36.145401 validate_gbb_flags: ok]
	[36.146457 Using 0 for GBB flags.]
	[36.147239 usb_spi_sha256_update: c00000:500c]
	[36.190986 usb_spi_sha256_update: c05010:3faff0]
	[43.365467 matched gbb 0]
	[43.367374 do_ap_ro_check: saved gbbd]
	[43.368988 spi_hash_disable]
	[43.370231 AP RO PASS!]

Change-Id: I9be2a900dc69009b40c32e12dec250e54977a08a
Signed-off-by: Mary Ruthven <mruthven@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4004357
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

2 files changed, 55 insertions, 27 deletions

diff --git a/common/ap_ro_integrity_check.c b/common/ap_ro_integrity_check.c
index 726a03b72a..0db205b118 100644
--- a/common/ap_ro_integrity_check.c
+++ b/common/ap_ro_integrity_check.c
@@ -1094,6 +1094,29 @@ int ec_rst_override(void)
 	return !apro_fail_status_cleared && apro_result == AP_RO_FAIL;
 }
 
+/*
+ * AP RO verification failed. Log the event type, disable spi, and hold the
+ * device in reset.
+ */
+static uint8_t ap_ro_failed_verification(enum ap_ro_verification_ev event)
+{
+	disable_ap_spi_hash_shortcut();
+	CPRINTS("AP RO FAILED! evt(%d)", event);
+	apro_result = AP_RO_FAIL;
+	ap_ro_add_flash_event(event);
+	keep_ec_in_reset();
+	/*
+	 * Map failures into EC_ERROR_CRC, this will make sure that in case this
+	 * was invoked by the operator keypress, the device will not continue
+	 * booting.
+	 *
+	 * Both explicit failure to verify OR any error if cached descriptor was
+	 * found should block the booting.
+	 */
+	return EC_ERROR_CRC;
+
+}
+
 static uint8_t do_ap_ro_check(void)
 {
 	enum ap_ro_check_result rv;
@@ -1101,11 +1124,25 @@ static uint8_t do_ap_ro_check(void)
 
 	apro_result = AP_RO_IN_PROGRESS;
 	apro_fail_status_cleared = 0;
-	if (ap_ro_check_unsupported(true) != ARCVE_OK ||
-	    p_chk->header.type != AP_RO_HASH_TYPE_FACTORY) {
+
+	switch (ap_ro_check_unsupported(true)) {
+	case ARCVE_OK:
+		CPRINTS("%s: found v1 data", __func__);
+		break;
+	case ARCVE_NOT_PROGRAMMED:
+	case ARCVE_BOARD_ID_BLOCKED:
+		CPRINTS("%s: unsupported", __func__);
 		apro_result = AP_RO_UNSUPPORTED_TRIGGERED;
 		ap_ro_add_flash_event(APROF_CHECK_UNSUPPORTED);
 		return EC_ERROR_UNIMPLEMENTED;
+	default:
+		/*
+		 * If reading the V1 data failed. Fail verification immediately.
+		 * The hash data is corrupted or the wrong version, so it can't
+		 * be used to verify the AP RO flash.
+		 */
+		CPRINTS("%s: bad v1 data", __func__);
+		return ap_ro_failed_verification(APROF_FAIL_CORRUPTED_V1_DATA);
 	}
 
 	enable_ap_spi_hash_shortcut();
@@ -1157,35 +1194,20 @@ static uint8_t do_ap_ro_check(void)
 				ap_ro_add_flash_event(APROF_SAVED_GBBD);
 			} else {
 				CPRINTS("%s: save gbbd failed", __func__);
-				ap_ro_add_flash_event(
-					APROF_FAILED_TO_SAVE_GBBD);
+				ap_ro_add_flash_event(APROF_FAIL_TO_SAVE_GBBD);
 			}
 		}
 		break;
 	default:
-		CPRINTS("%s: FAIL corrupted GBBD", __func__);
-		break;
+		CPRINTS("%s: bad gbbd", __func__);
+		return ap_ro_failed_verification(APROF_FAIL_CORRUPTED_GBBD);
 	}
 
 	disable_ap_spi_hash_shortcut();
 
-	/* Failure reason has already been reported. */
-	if (rv != ROV_SUCCEEDED) {
-		CPRINTS("AP RO FAILED!");
-		apro_result = AP_RO_FAIL;
-		ap_ro_add_flash_event(APROF_CHECK_FAILED);
-		keep_ec_in_reset();
-		/*
-		 * Map failures into EC_ERROR_CRC, this will make sure
-		 * that in case this was invoked by the operator
-		 * keypress, the device will not continue booting.
-		 *
-		 * Both explicit failure to verify OR any error if
-		 * cached descriptor was found should block the
-		 * booting.
-		 */
-		return EC_ERROR_CRC;
-	}
+	/* AP RO failed to match the hash. */
+	if (rv != ROV_SUCCEEDED)
+		return ap_ro_failed_verification(APROF_CHECK_FAILED);
 
 	apro_result = AP_RO_PASS;
 	ap_ro_add_flash_event(APROF_CHECK_SUCCEEDED);
@@ -1332,6 +1354,7 @@ static enum vendor_cmd_rc vc_get_ap_ro_status(enum vendor_cmd_cc code,
 {
 	uint8_t rv = apro_result;
 	uint8_t *response = buf;
+	enum ap_ro_check_vc_errors v1_check;
 
 	CPRINTS("Check AP RO status");
 
@@ -1339,9 +1362,12 @@ static enum vendor_cmd_rc vc_get_ap_ro_status(enum vendor_cmd_cc code,
 	if (input_size)
 		return VENDOR_RC_BOGUS_ARGS;
 
-	if ((apro_result != AP_RO_UNSUPPORTED_TRIGGERED) &&
-	    (ap_ro_check_unsupported(false) != ARCVE_OK))
-		rv = AP_RO_UNSUPPORTED_NOT_TRIGGERED;
+	if (apro_result != AP_RO_UNSUPPORTED_TRIGGERED) {
+		v1_check = ap_ro_check_unsupported(false);
+		if (v1_check == ARCVE_NOT_PROGRAMMED ||
+		    v1_check == ARCVE_BOARD_ID_BLOCKED)
+			rv = AP_RO_UNSUPPORTED_NOT_TRIGGERED;
+	}
 
 	*response_size = 1;
 	response[0] = rv;
diff --git a/include/flash_log.h b/include/flash_log.h
index 407a184e2f..0db93eecda 100644
--- a/include/flash_log.h
+++ b/include/flash_log.h
@@ -103,7 +103,9 @@ enum ap_ro_verification_ev {
 	APROF_CHECK_UNSUPPORTED = 8,
 	APROF_FAIL_CLEARED = 9,
 	APROF_SAVED_GBBD = 10,
-	APROF_FAILED_TO_SAVE_GBBD = 11,
+	APROF_FAIL_TO_SAVE_GBBD = 11,
+	APROF_FAIL_CORRUPTED_V1_DATA = 12,
+	APROF_FAIL_CORRUPTED_GBBD = 13,
 };
 
 struct ap_ro_entry_payload {