vboot/misc: Add validity check in vb2api_enable_developer_mode

Check vb2api_allow_recovery before actual changes in
vb2api_enable_developer_mode.

Add new API error type VB2_ERROR_API_ENABLE_DEV_NOT_ALLOWED in
vb2_error_t.

Add spaces in the output message of vb2_misc_tests for indentation.

BUG=b:172339016
TEST=export CC=x86_64-pc-linux-gnu-clang DEBUG=1 DETACHABLE=0; \
     make -j32 test_setup && make -j32 runtests;
TEST=export CC=x86_64-pc-linux-gnu-clang DEBUG=1 DETACHABLE=1; \
     make -j32 test_setup && make -j32 runtests;
TEST=Hayato booted into developer mode
BRANCH=none

Change-Id: Ifebdd01ca87cf944c394907eca73a09fc3bf5457
Signed-off-by: edisonhello <edisonhello@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3056240
Reviewed-by: Julius Werner <jwerner@chromium.org>

6 files changed, 39 insertions, 9 deletions

diff --git a/firmware/2lib/2misc.c b/firmware/2lib/2misc.c
index f2c6bb41..03766b44 100644
--- a/firmware/2lib/2misc.c
+++ b/firmware/2lib/2misc.c
@@ -375,8 +375,14 @@ vb2_error_t vb2_select_fw_slot(struct vb2_context *ctx)
 	return VB2_SUCCESS;
 }
 
-void vb2api_enable_developer_mode(struct vb2_context *ctx)
+vb2_error_t vb2api_enable_developer_mode(struct vb2_context *ctx)
 {
+	if (!vb2api_allow_recovery(ctx)) {
+		VB2_DEBUG("ERROR: Can only enable developer mode from manual "
+			  "recovery mode\n");
+		return VB2_ERROR_API_ENABLE_DEV_NOT_ALLOWED;
+	}
+
 	uint32_t flags;
 
 	VB2_DEBUG("Enabling developer mode...\n");
@@ -389,6 +395,8 @@ void vb2api_enable_developer_mode(struct vb2_context *ctx)
 		vb2_nv_set(ctx, VB2_NV_DEV_BOOT_EXTERNAL, 1);
 
 	VB2_DEBUG("Mode change will take effect on next reboot\n");
+
+	return VB2_SUCCESS;
 }
 
 vb2_error_t vb2api_disable_developer_mode(struct vb2_context *ctx)
diff --git a/firmware/2lib/2ui_screens.c b/firmware/2lib/2ui_screens.c
index 02dc14c4..a2ad2b63 100644
--- a/firmware/2lib/2ui_screens.c
+++ b/firmware/2lib/2ui_screens.c
@@ -524,7 +524,12 @@ static vb2_error_t recovery_to_dev_finalize(struct vb2_ui_context *ui)
 	}
 
 	VB2_DEBUG("Enabling dev mode and rebooting...\n");
-	vb2api_enable_developer_mode(ui->ctx);
+
+	if (vb2api_enable_developer_mode(ui->ctx) != VB2_SUCCESS) {
+		VB2_DEBUG("Enable developer mode failed\n");
+		return VB2_SUCCESS;
+	}
+
 	return VB2_REQUEST_REBOOT_EC_TO_RO;
 }
 
diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h
index 54388bb5..66594035 100644
--- a/firmware/2lib/include/2api.h
+++ b/firmware/2lib/include/2api.h
@@ -972,8 +972,10 @@ int vb2api_allow_recovery(struct vb2_context *ctx);
  * done on the next boot.
  *
  * @param ctx		Vboot context
+ * @return VB2_SUCCESS if success; error if enabling developer mode is not
+ * allowed.
  */
-void vb2api_enable_developer_mode(struct vb2_context *ctx);
+vb2_error_t vb2api_enable_developer_mode(struct vb2_context *ctx);
 
 /**
  * Request to disable developer mode by setting VB2_NV_DIAG_REQUEST.
diff --git a/firmware/2lib/include/2return_codes.h b/firmware/2lib/include/2return_codes.h
index f3bf50e0..33cb7623 100644
--- a/firmware/2lib/include/2return_codes.h
+++ b/firmware/2lib/include/2return_codes.h
@@ -704,6 +704,9 @@ enum vb2_return_code {
 	/* Disabling developer mode is not allowed by GBB flags */
 	VB2_ERROR_API_DISABLE_DEV_NOT_ALLOWED,
 
+	/* Enabling developer mode is not allowed in non-recovery mode */
+	VB2_ERROR_API_ENABLE_DEV_NOT_ALLOWED,
+
 	/**********************************************************************
 	 * Errors which may be generated by implementations of vb2ex functions.
 	 * Implementation may also return its own specific errors, which should
diff --git a/tests/vb2_misc_tests.c b/tests/vb2_misc_tests.c
index 042c0f71..c4b3ce44 100644
--- a/tests/vb2_misc_tests.c
+++ b/tests/vb2_misc_tests.c
@@ -640,24 +640,35 @@ static void dev_switch_tests(void)
 static void enable_dev_tests(void)
 {
 	reset_common_data();
-	vb2api_enable_developer_mode(ctx);
+	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) &
+		VB2_SECDATA_FIRMWARE_FLAG_DEV_MODE, 0,
+		"  dev mode flag not set");
+
+	reset_common_data();
+	allow_recovery_retval = 1;
+	TEST_SUCC(vb2api_enable_developer_mode(ctx),
+		  "vb2api_enable_developer_mode - success");
 	TEST_NEQ(vb2_secdata_firmware_get(ctx, VB2_SECDATA_FIRMWARE_FLAGS) &
 	         VB2_SECDATA_FIRMWARE_FLAG_DEV_MODE, 0,
-		 "dev mode flag set");
+		 "  dev mode flag set");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_DEV_BOOT_EXTERNAL), BOOT_EXTERNAL_ON_DEV,
-		"NV_DEV_BOOT_EXTERNAL set according to compile-time flag");
+		"  NV_DEV_BOOT_EXTERNAL set according to compile-time flag");
 
 	/* secdata_firmware not initialized, aborts */
 	reset_common_data();
+	allow_recovery_retval = 1;
 	sd->status &= ~VB2_SD_STATUS_SECDATA_FIRMWARE_INIT;
 	TEST_ABORT(vb2api_enable_developer_mode(ctx),
 		   "secdata_firmware no init, enable dev mode aborted");
 	sd->status |= VB2_SD_STATUS_SECDATA_FIRMWARE_INIT;
 	TEST_EQ(vb2_secdata_firmware_get(ctx, VB2_SECDATA_FIRMWARE_FLAGS) &
 	        VB2_SECDATA_FIRMWARE_FLAG_DEV_MODE, 0,
-		"dev mode flag not set");
+		"  dev mode flag not set");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_DEV_BOOT_EXTERNAL), 0,
-		"NV_DEV_BOOT_EXTERNAL not set");
+		"  NV_DEV_BOOT_EXTERNAL not set");
 }
 
 static void tpm_clear_tests(void)
diff --git a/tests/vb2_ui_tests.c b/tests/vb2_ui_tests.c
index ca9a9987..95d22900 100644
--- a/tests/vb2_ui_tests.c
+++ b/tests/vb2_ui_tests.c
@@ -527,9 +527,10 @@ int vb2ex_physical_presence_pressed(void)
 	return mock_pp_pressed[mock_iters];
 }
 
-void vb2api_enable_developer_mode(struct vb2_context *c)
+vb2_error_t vb2api_enable_developer_mode(struct vb2_context *c)
 {
 	mock_enable_dev_mode = 1;
+	return VB2_SUCCESS;
 }
 
 const char *vb2ex_get_debug_info(struct vb2_context *c)