2lib: Deprecate vb2api_allow_recovery() and VB2_SD_FLAG_MANUAL_RECOVERY

2lib used vb2_api_allow_recovery() to differentiate between manual and
non-manual recovery in 2kernel and UI related areas.

With introducing the ctx->boot_mode, we could decide if it is a manual
recovery or a broken screen (a.k.a non-manual recovery in the original
design) once in vb2api_fw_phase1 and use this boot mode instead for
further justifications.

Also deprecate the sd flag VB2_SD_FLAG_MANUAL_RECOVERY and use the boot
mode instead to determine if it is a manual recovery boot.

BUG=b:181931817
BRANCH=none
TEST=make clean && make runtests
TEST=emerge coreboot vboot_reference depthcharge

Cq-Depend: chromium:3282875
Signed-off-by: Hsuan Ting Chen <roccochen@chromium.org>
Change-Id: Ief4ff6cf82285c5857f0051c1f348ad0f269b4a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3121926
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>

7 files changed, 48 insertions, 72 deletions

diff --git a/firmware/2lib/2kernel.c b/firmware/2lib/2kernel.c
index 86fb2863..5b18cad5 100644
--- a/firmware/2lib/2kernel.c
+++ b/firmware/2lib/2kernel.c
@@ -164,7 +164,7 @@ vb2_error_t vb2api_kernel_phase1(struct vb2_context *ctx)
 		/* Load recovery key from GBB. */
 		rv = vb2_gbb_read_recovery_key(ctx, &packed_key, NULL, &wb);
 		if (rv) {
-			if (vb2api_allow_recovery(ctx))
+			if (ctx->boot_mode != VB2_BOOT_MODE_BROKEN_SCREEN)
 				VB2_DIE("GBB read recovery key failed.\n");
 			else
 				/*
diff --git a/firmware/2lib/2misc.c b/firmware/2lib/2misc.c
index c1d860f1..6e926905 100644
--- a/firmware/2lib/2misc.c
+++ b/firmware/2lib/2misc.c
@@ -154,7 +154,6 @@ void vb2_check_recovery(struct vb2_context *ctx)
 		else
 			/* Recovery was forced. Override recovery reason */
 			sd->recovery_reason = VB2_RECOVERY_RO_MANUAL;
-		sd->flags |= VB2_SD_FLAG_MANUAL_RECOVERY;
 	}
 
 	/* If recovery reason is non-zero, tell caller we need recovery mode */
@@ -379,7 +378,7 @@ vb2_error_t vb2_select_fw_slot(struct vb2_context *ctx)
 
 vb2_error_t vb2api_enable_developer_mode(struct vb2_context *ctx)
 {
-	if (!vb2api_allow_recovery(ctx)) {
+	if (ctx->boot_mode != VB2_BOOT_MODE_MANUAL_RECOVERY) {
 		VB2_DEBUG("ERROR: Can only enable developer mode from manual "
 			  "recovery mode\n");
 		return VB2_ERROR_API_ENABLE_DEV_NOT_ALLOWED;
@@ -415,30 +414,6 @@ void vb2api_request_diagnostics(struct vb2_context *ctx) {
 	VB2_DEBUG("Diagnostics requested\n");
 }
 
-test_mockable
-int vb2api_allow_recovery(struct vb2_context *ctx)
-{
-	if (ctx->flags & VB2_CONTEXT_NO_BOOT)
-		return 0;
-
-	/* VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY forces this to always return
-	   true. */
-	if (vb2_get_gbb(ctx)->flags & VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY)
-		return 1;
-
-	/*
-	 * If EC is in RW, it implies recovery wasn't manually requested.
-	 * On some platforms, EC_IN_RW can't be reset by the EC, thus, this may
-	 * return false (=RW). That's ok because if recovery is manual, we will
-	 * get the right signal and that's the case we care about.
-	 */
-	if (!(ctx->flags & VB2_CONTEXT_EC_TRUSTED))
-		return 0;
-
-	/* Now we confidently check the recovery switch state at boot */
-	return !!(vb2_get_sd(ctx)->flags & VB2_SD_FLAG_MANUAL_RECOVERY);
-}
-
 void vb2_clear_recovery(struct vb2_context *ctx)
 {
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
@@ -450,13 +425,13 @@ void vb2_clear_recovery(struct vb2_context *ctx)
 			  reason, subcode,
 			  vb2_get_recovery_reason_string(reason));
 
-	/* Clear recovery request for both manual and non-manual. */
+	/* Clear recovery request for both the manual recovery and the broken
+	   screen. */
 	vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, VB2_RECOVERY_NOT_REQUESTED);
 	vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, 0);
 
-	/* But stow recovery reason as subcode for non-manual recovery. */
-	if ((ctx->flags & VB2_CONTEXT_RECOVERY_MODE) &&
-	    !vb2api_allow_recovery(ctx)) {
+	/* But stow recovery reason as subcode for the broken screen. */
+	if (ctx->boot_mode == VB2_BOOT_MODE_BROKEN_SCREEN) {
 		VB2_DEBUG("Stow recovery reason as subcode (%#x)\n",
 			  sd->recovery_reason);
 		vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, sd->recovery_reason);
@@ -737,13 +712,24 @@ char *vb2api_get_debug_info(struct vb2_context *ctx)
 void vb2_set_boot_mode(struct vb2_context *ctx)
 {
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
+	struct vb2_gbb_header *gbb = vb2_get_gbb(ctx);
 
 	/* Cast boot mode to non-constant and assign */
 	enum vb2_boot_mode *boot_mode = (enum vb2_boot_mode *)&ctx->boot_mode;
 	*boot_mode = VB2_BOOT_MODE_NORMAL;
 
-	if (sd->recovery_reason) {
-		if (vb2api_allow_recovery(ctx))
+	/*
+	 * The only way to pass this check and proceed to the recovery process
+	 * is to physically request a recovery (a.k.a. manual recovery).  All
+	 * other recovery requests including manual recovery requested by a
+	 * (compromised) host will end up with 'broken' screen.
+	 */
+	if ((ctx->flags & VB2_CONTEXT_FORCE_RECOVERY_MODE) &&
+	    !(ctx->flags & VB2_CONTEXT_NO_BOOT) &&
+	    (ctx->flags & VB2_CONTEXT_EC_TRUSTED)) {
+		*boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY;
+	} else if (sd->recovery_reason) {
+		if (gbb->flags & VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY)
 			*boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY;
 		else
 			*boot_mode = VB2_BOOT_MODE_BROKEN_SCREEN;
diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h
index cd558b05..14301111 100644
--- a/firmware/2lib/include/2api.h
+++ b/firmware/2lib/include/2api.h
@@ -1012,19 +1012,6 @@ enum vb2_dev_default_boot_target vb2api_get_dev_default_boot_target(
 int vb2api_use_short_dev_screen_delay(struct vb2_context *ctx);
 
 /**
- * Check whether recovery is allowed or not.
- *
- * The only way to pass this check and proceed to the recovery process is to
- * physically request a recovery (a.k.a. manual recovery).  All other recovery
- * requests including manual recovery requested by a (compromised) host will
- * end up with 'broken' screen.
- *
- * @param ctx		Vboot context
- * @return 1 if recovery is allowed; 0 if no or uncertain.
- */
-int vb2api_allow_recovery(struct vb2_context *ctx);
-
-/**
  * Request to enable developer mode.
  *
  * Enables the developer flag in vb2_context firmware secdata.  Note that
diff --git a/firmware/2lib/include/2struct.h b/firmware/2lib/include/2struct.h
index 310f4bc5..3e2c4228 100644
--- a/firmware/2lib/include/2struct.h
+++ b/firmware/2lib/include/2struct.h
@@ -23,8 +23,14 @@
 
 /* Flags for vb2_shared_data.flags */
 enum vb2_shared_data_flags {
-	/* User has explicitly and physically requested recovery */
-	VB2_SD_FLAG_MANUAL_RECOVERY = (1 << 0),
+	/*
+	 * VB2_SD_FLAG_MANUAL_RECOVERY (1 << 0) is deprecated. This flag is not
+	 * necessary since the introduction of persistent context (CL:1716351).
+	 * With introducing vb2_boot_mode and differentiating between manual
+	 * recovery and broken screen (CL:3274699), it is suggested to leverage
+	 * the vb2_boot_mode instead to determine if the user has physically
+	 * requested recovery.
+	 */
 
 	/* Developer mode is enabled */
 	VB2_SD_FLAG_DEV_MODE_ENABLED = (1 << 1),
diff --git a/tests/vb2_kernel_tests.c b/tests/vb2_kernel_tests.c
index 0dc0e749..0b3e94c7 100644
--- a/tests/vb2_kernel_tests.c
+++ b/tests/vb2_kernel_tests.c
@@ -24,6 +24,7 @@ static struct vb2_context *ctx;
 static struct vb2_shared_data *sd;
 static struct vb2_fw_preamble *fwpre;
 static const char fw_kernel_key_data[36] = "Test kernel key data";
+static enum vb2_boot_mode *boot_mode;
 
 /* Mocked function data */
 
@@ -82,6 +83,14 @@ static void reset_common_data(enum reset_type t)
 		mock_gbb.recovery_key.key_offset +
 		mock_gbb.recovery_key.key_size;
 
+	/* For boot_mode */
+	boot_mode = (enum vb2_boot_mode *)&ctx->boot_mode;
+	if (t == FOR_PHASE1)
+		*boot_mode = VB2_BOOT_MODE_BROKEN_SCREEN;
+	else if (t == FOR_NORMAL_BOOT)
+		*boot_mode = VB2_BOOT_MODE_NORMAL;
+	else
+		*boot_mode = VB2_BOOT_MODE_UNDEFINED;
 
 	if (t == FOR_PHASE1) {
 		uint8_t *kdata;
@@ -274,6 +283,7 @@ static void phase1_tests(void)
 	TEST_EQ(sd->kernel_key_offset, 0, "  workbuf key offset");
 	TEST_EQ(sd->kernel_key_size, 0, "  workbuf key size");
 	mock_gbb.h.flags |= VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY;
+	*boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY;
 	TEST_ABORT(vb2api_kernel_phase1(ctx), "  fatal for manual recovery");
 
 	reset_common_data(FOR_PHASE1);
@@ -284,6 +294,7 @@ static void phase1_tests(void)
 	TEST_EQ(sd->kernel_key_offset, 0, "  workbuf key offset");
 	TEST_EQ(sd->kernel_key_size, 0, "  workbuf key size");
 	mock_gbb.h.flags |= VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY;
+	*boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY;
 	mock_read_res_fail_on_call = 1;
 	TEST_ABORT(vb2api_kernel_phase1(ctx), "  fatal for manual recovery");
 
diff --git a/tests/vb2_misc_tests.c b/tests/vb2_misc_tests.c
index c552301e..99f8bae4 100644
--- a/tests/vb2_misc_tests.c
+++ b/tests/vb2_misc_tests.c
@@ -22,6 +22,7 @@ static struct vb2_context *ctx;
 static struct vb2_shared_data *sd;
 static struct vb2_gbb_header gbb;
 static struct vb2_secdata_fwmp *fwmp;
+static enum vb2_boot_mode *boot_mode;
 
 /* Mocked function data */
 static enum vb2_resource_index mock_resource_index;
@@ -29,7 +30,6 @@ static void *mock_resource_ptr;
 static uint32_t mock_resource_size;
 static int mock_tpm_clear_called;
 static int mock_tpm_clear_retval;
-static int allow_recovery_retval;
 
 static void reset_common_data(void)
 {
@@ -53,16 +53,13 @@ static void reset_common_data(void)
 
 	mock_tpm_clear_called = 0;
 	mock_tpm_clear_retval = VB2_SUCCESS;
-	allow_recovery_retval = 0;
+
+	boot_mode = (enum vb2_boot_mode *)&ctx->boot_mode;
+	*boot_mode = VB2_BOOT_MODE_NORMAL;
 };
 
 /* Mocked functions */
 
-int vb2api_allow_recovery(struct vb2_context *c)
-{
-	return allow_recovery_retval;
-}
-
 struct vb2_gbb_header *vb2_get_gbb(struct vb2_context *c)
 {
 	return &gbb;
@@ -430,8 +427,6 @@ static void recovery_tests(void)
 		0, "recovery not yet decided before testing check_recovery()");
 	vb2_check_recovery(ctx);
 	TEST_EQ(sd->recovery_reason, 0, "No recovery reason");
-	TEST_EQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
-		0, "Not manual recovery");
 	TEST_EQ(ctx->flags & VB2_CONTEXT_RECOVERY_MODE,
 		0, "Not recovery mode");
 	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
@@ -443,8 +438,6 @@ static void recovery_tests(void)
 	vb2_check_recovery(ctx);
 	TEST_EQ(sd->recovery_reason, 3, "Recovery reason from request");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST), 3, "NV not cleared");
-	TEST_EQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
-		0, "Not manual recovery");
 	TEST_NEQ(ctx->flags & VB2_CONTEXT_RECOVERY_MODE,
 		 0, "Recovery mode");
 	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
@@ -468,8 +461,6 @@ static void recovery_tests(void)
 	vb2_check_recovery(ctx);
 	TEST_EQ(sd->recovery_reason, VB2_RECOVERY_RO_MANUAL,
 		"Recovery reason forced");
-	TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
-		 0, "SD flag set");
 	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
 		 0, "Recovery decided");
 
@@ -480,8 +471,6 @@ static void recovery_tests(void)
 	vb2_check_recovery(ctx);
 	TEST_EQ(sd->recovery_reason, VB2_RECOVERY_RO_MANUAL,
 		"Recovery reason forced");
-	TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
-		 0, "SD flag set");
 	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
 		 0, "Recovery decided");
 
@@ -492,8 +481,6 @@ static void recovery_tests(void)
 	vb2_check_recovery(ctx);
 	TEST_EQ(sd->recovery_reason, VB2_RECOVERY_US_TEST,
 		"Recovery reason forced from BROKEN");
-	TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
-		 0, "SD flag set");
 	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
 		 0, "Recovery decided");
 }
@@ -661,7 +648,6 @@ static void dev_switch_tests(void)
 static void enable_dev_tests(void)
 {
 	reset_common_data();
-	allow_recovery_retval = 0;
 	TEST_FAIL(vb2api_enable_developer_mode(ctx),
 		 "vb2api_enable_developer_mode - failed");
 	TEST_EQ(vb2_secdata_firmware_get(ctx, VB2_SECDATA_FIRMWARE_FLAGS) &
@@ -669,7 +655,7 @@ static void enable_dev_tests(void)
 		"  dev mode flag not set");
 
 	reset_common_data();
-	allow_recovery_retval = 1;
+	*boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY;
 	TEST_SUCC(vb2api_enable_developer_mode(ctx),
 		  "vb2api_enable_developer_mode - success");
 	TEST_NEQ(vb2_secdata_firmware_get(ctx, VB2_SECDATA_FIRMWARE_FLAGS) &
@@ -678,7 +664,7 @@ static void enable_dev_tests(void)
 
 	/* secdata_firmware not initialized, aborts */
 	reset_common_data();
-	allow_recovery_retval = 1;
+	*boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY;
 	sd->status &= ~VB2_SD_STATUS_SECDATA_FIRMWARE_INIT;
 	sd->status |= VB2_SD_STATUS_RECOVERY_DECIDED;
 	TEST_ABORT(vb2api_enable_developer_mode(ctx),
@@ -830,7 +816,7 @@ static void clear_recovery_tests(void)
 
 	/* Manual recovery */
 	reset_common_data();
-	allow_recovery_retval = 1;
+	*boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY;
 	sd->recovery_reason = 4;
 	ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
 	vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, 5);
@@ -841,9 +827,9 @@ static void clear_recovery_tests(void)
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_SUBCODE),
 		0, "  subcode cleared");
 
-	/* BROKEN recovery */
+	/* Broken screen */
 	reset_common_data();
-	allow_recovery_retval = 0;
+	*boot_mode = VB2_BOOT_MODE_BROKEN_SCREEN;
 	sd->recovery_reason = 4;
 	ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
 	vb2_nv_set(ctx, VB2_NV_RECOVERY_REQUEST, 5);
diff --git a/tests/vboot_api_kernel4_tests.c b/tests/vboot_api_kernel4_tests.c
index c1e71afa..e3f19a8c 100644
--- a/tests/vboot_api_kernel4_tests.c
+++ b/tests/vboot_api_kernel4_tests.c
@@ -328,7 +328,7 @@ static void select_and_load_kernel_tests(void)
 	reset_common_data();
 	sd->recovery_reason = VB2_RECOVERY_RO_MANUAL;
 	vb2_nv_set(ctx, VB2_NV_RECOVERY_SUBCODE, 13);
-	sd->flags |= VB2_SD_FLAG_MANUAL_RECOVERY;
+	ctx->flags &= VB2_CONTEXT_FORCE_RECOVERY_MODE;
 	test_slk(0, 0, "Manual recovery");
 	TEST_TRUE(commit_data_called, "  commit data");
 }