vboot: update vb2api_kernel_phase1 to use GBB interface

vb2api_kernel_phase1 was previously written to read the GBB
headers, locate the recovery key, and then load it.  GBB headers
are now saved directly on workbuf in firmware phase.  Simply use
the vb2_gbb_read_recovery_key function to retrieve the key.

Update LoadKernel to read kernel subkey from vboot2 workbuf.
Update tests/verify_kernel.c to write subkey to vboot2 workbuf.

BUG=b:124141368, chromium:954774, chromium:1038260
TEST=make clean && make runtests
BRANCH=none

Change-Id: Ia85013da34bdab68bf486014a3401d48c95b3472
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1651221
Tested-by: Joel Kitching <kitching@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Commit-Queue: Joel Kitching <kitching@chromium.org>

7 files changed, 156 insertions, 165 deletions

diff --git a/firmware/2lib/include/2return_codes.h b/firmware/2lib/include/2return_codes.h
index 0394feed..64b375d1 100644
--- a/firmware/2lib/include/2return_codes.h
+++ b/firmware/2lib/include/2return_codes.h
@@ -684,8 +684,9 @@ enum vb2_return_code {
 	/* Buffer size for the digest is too small for vb2api_get_pcr_digest */
 	VB2_ERROR_API_PCR_DIGEST_BUF,
 
-	/* Work buffer too small for recovery key in vb2api_kernel_phase1() */
-	VB2_ERROR_API_KPHASE1_WORKBUF_REC_KEY,
+	/* Work buffer too small for recovery key in vb2api_kernel_phase1();
+	 * Deprecated: use vb2_gbb_read_recovery_key return values */
+	VB2_ERROR_DEPRECATED_API_KPHASE1_WORKBUF_REC_KEY,
 
 	/* Firmware preamble not present for vb2api_kernel_phase1() */
 	VB2_ERROR_API_KPHASE1_PREAMBLE,
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index d6fab619..0e2e8fb3 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -227,7 +227,6 @@ static vb2_error_t vb2_kernel_setup(struct vb2_context *ctx,
 				    VbSelectAndLoadKernelParams *kparams)
 {
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
-	vb2_error_t rv;
 
 	/* Set selected boot mode in context object.
 	   TODO: Confirm that this can be removed with persistent context. */
@@ -251,8 +250,6 @@ static vb2_error_t vb2_kernel_setup(struct vb2_context *ctx,
 	if (sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY)
 		shared->flags |= VBSD_BOOT_REC_SWITCH_ON;
 
-	vb2_nv_init(ctx);
-
 	/*
 	 * Save a pointer to the old vboot1 shared data, since we haven't
 	 * finished porting the library to use the new vb2 context and shared
@@ -275,29 +272,6 @@ static vb2_error_t vb2_kernel_setup(struct vb2_context *ctx,
 	kparams->flags = 0;
 	memset(kparams->partition_guid, 0, sizeof(kparams->partition_guid));
 
-	/*
-	 * Init secdata_kernel and secdata_fwmp spaces.  No need to init
-	 * secdata_firmware, since it was already read during firmware
-	 * verification.  Ignore errors in recovery mode.
-	 */
-	rv = vb2_secdata_kernel_init(ctx);
-	if (rv && !(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) {
-		VB2_DEBUG("TPM: init secdata_kernel returned %#x\n", rv);
-		vb2api_fail(ctx, VB2_RECOVERY_SECDATA_KERNEL_INIT, rv);
-		return rv;
-	}
-	rv = vb2_secdata_fwmp_init(ctx);
-	if (rv && !(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) {
-		VB2_DEBUG("TPM: init secdata_fwmp returned %#x\n", rv);
-		vb2api_fail(ctx, VB2_RECOVERY_SECDATA_FWMP_INIT, rv);
-		return rv;
-	}
-
-	/* Read kernel version from the TPM. */
-	shared->kernel_version_tpm =
-		vb2_secdata_kernel_get(ctx, VB2_SECDATA_KERNEL_VERSIONS);
-	shared->kernel_version_tpm_start = shared->kernel_version_tpm;
-
 	return VB2_SUCCESS;
 }
 
@@ -367,10 +341,18 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx,
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
 	vb2_error_t rv, call_rv;
 
+	/* Init nvstorage space. TODO(kitching): Remove once we add assertions
+	   to vb2_nv_get and vb2_nv_set. */
+	vb2_nv_init(ctx);
+
 	rv = vb2_kernel_setup(ctx, shared, kparams);
 	if (rv)
 		goto VbSelectAndLoadKernel_exit;
 
+	rv = vb2api_kernel_phase1(ctx);
+	if (rv)
+		goto VbSelectAndLoadKernel_exit;
+
 	VB2_DEBUG("GBB flags are %#x\n", vb2_get_gbb(ctx)->flags);
 
 	/*
diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c
index 72ea0ebc..9ca14bac 100644
--- a/firmware/lib/vboot_kernel.c
+++ b/firmware/lib/vboot_kernel.c
@@ -466,19 +466,10 @@ vb2_error_t LoadKernel(struct vb2_context *ctx, LoadKernelParams *params)
 	shcall->sector_count = params->streaming_lba_count;
 	shared->lk_call_count++;
 
-	/* Choose key to verify kernel */
-	struct vb2_packed_key *kernel_subkey;
-	if (kBootRecovery == shcall->boot_mode) {
-		/* Use the recovery key to verify the kernel */
-		rv = vb2_gbb_read_recovery_key(ctx, &kernel_subkey, NULL, &wb);
-		if (VB2_SUCCESS != rv) {
-			VB2_DEBUG("GBB read recovery key failed.\n");
-			goto load_kernel_exit;
-		}
-	} else {
-		/* Use the kernel subkey passed from firmware verification */
-		kernel_subkey = (struct vb2_packed_key *)&shared->kernel_subkey;
-	}
+	/* Locate key to verify kernel.  This will either be a recovery key, or
+	   a kernel subkey passed from firmware verification. */
+	struct vb2_packed_key *kernel_subkey =
+		vb2_member_of(sd, sd->kernel_key_offset);
 
 	/* Read GPT data */
 	GptData gpt;
@@ -658,7 +649,6 @@ gpt_done:
 		rv = VB2_ERROR_LK_NO_KERNEL_FOUND;
 	}
 
-load_kernel_exit:
 	shcall->return_code = (uint8_t)rv;
 	return rv;
 }
diff --git a/firmware/lib20/api_kernel.c b/firmware/lib20/api_kernel.c
index 3a4da4ce..3748420f 100644
--- a/firmware/lib20/api_kernel.c
+++ b/firmware/lib20/api_kernel.c
@@ -14,63 +14,59 @@
 #include "2sha.h"
 #include "2sysincludes.h"
 #include "vb2_common.h"
+#include "vboot_struct.h"
 
 vb2_error_t vb2api_kernel_phase1(struct vb2_context *ctx)
 {
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
 	struct vb2_workbuf wb;
-	uint8_t *key_data;
-	uint32_t key_size;
+	struct vb2_packed_key *packed_key;
 	vb2_error_t rv;
 
 	vb2_workbuf_from_ctx(ctx, &wb);
 
-	/* Initialize secure kernel data and read version */
+	/*
+	 * Init secdata_kernel and secdata_fwmp spaces.  No need to init
+	 * secdata_firmware, since it was already read during firmware
+	 * verification.  Ignore errors in recovery mode.
+	 */
 	rv = vb2_secdata_kernel_init(ctx);
 	if (rv && !(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) {
+		VB2_DEBUG("TPM: init secdata_kernel returned %#x\n", rv);
 		vb2api_fail(ctx, VB2_RECOVERY_SECDATA_KERNEL_INIT, rv);
 		return rv;
 	}
+	rv = vb2_secdata_fwmp_init(ctx);
+	if (rv && !(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) {
+		VB2_DEBUG("TPM: init secdata_fwmp returned %#x\n", rv);
+		vb2api_fail(ctx, VB2_RECOVERY_SECDATA_FWMP_INIT, rv);
+		return rv;
+	}
+
+	/* Read kernel version from secdata. */
 	sd->kernel_version_secdata =
 		vb2_secdata_kernel_get(ctx, VB2_SECDATA_KERNEL_VERSIONS);
+	sd->vbsd->kernel_version_tpm = sd->kernel_version_secdata;
+	sd->vbsd->kernel_version_tpm_start = sd->kernel_version_secdata;
 
 	/* Find the key to use to verify the kernel keyblock */
-	if (ctx->flags & VB2_CONTEXT_RECOVERY_MODE) {
-		/* Recovery key from GBB */
-		struct vb2_gbb_header *gbb;
-		uint32_t key_offset;
-
-		/* Read GBB header into next chunk of work buffer */
-		gbb = vb2_workbuf_alloc(&wb, sizeof(*gbb));
-		if (!gbb)
-			return VB2_ERROR_GBB_WORKBUF;
-
-		rv = vb2_read_gbb_header(ctx, gbb);
-		if (rv)
-			return rv;
-
-		/* Only need the recovery key position and size */
-		key_offset = gbb->recovery_key_offset;
-		key_size = gbb->recovery_key_size;
-
-		/* Free the GBB header */
-		vb2_workbuf_free(&wb, sizeof(*gbb));
-
-		/* Load the recovery key itself */
-		key_data = vb2_workbuf_alloc(&wb, key_size);
-		if (!key_data)
-			return VB2_ERROR_API_KPHASE1_WORKBUF_REC_KEY;
-
-		rv = vb2ex_read_resource(ctx, VB2_RES_GBB, key_offset,
-					 key_data, key_size);
-		if (rv)
-			return rv;
-
-		sd->kernel_key_offset = vb2_offset_of(sd, key_data);
+	if ((ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) {
+		/* Load recovery key from GBB. */
+		rv = vb2_gbb_read_recovery_key(ctx, &packed_key, NULL, &wb);
+		if (rv) {
+			if (vb2_allow_recovery(ctx))
+				VB2_DIE("GBB read recovery key failed.\n");
+			else
+				/*
+				 * If we're headed for the BROKEN screen,
+				 * we won't need the recovery key.  Just
+				 * short-circuit with success.
+				 */
+				return VB2_SUCCESS;
+		}
 	} else {
 		/* Kernel subkey from firmware preamble */
 		struct vb2_fw_preamble *pre;
-		struct vb2_packed_key *pre_key, *packed_key;
 
 		/* Make sure we have a firmware preamble loaded */
 		if (!sd->preamble_size)
@@ -78,39 +74,13 @@ vb2_error_t vb2api_kernel_phase1(struct vb2_context *ctx)
 
 		pre = (struct vb2_fw_preamble *)
 			vb2_member_of(sd, sd->preamble_offset);
-		pre_key = &pre->kernel_subkey;
-
-		/*
-		 * At this point, we no longer need the packed firmware
-		 * data key, firmware preamble, or hash data.  So move the
-		 * kernel key from the preamble down after the shared data.
-		 */
-		sd->kernel_key_offset = vb2_wb_round_up(sizeof(*sd));
-		key_data = vb2_member_of(sd, sd->kernel_key_offset);
-		packed_key = (struct vb2_packed_key *)key_data;
-		memmove(packed_key, pre_key, sizeof(*packed_key));
-		packed_key->key_offset = sizeof(*packed_key);
-		memmove(key_data + packed_key->key_offset,
-			(uint8_t *)pre_key + pre_key->key_offset,
-			pre_key->key_size);
-
-		key_size = packed_key->key_offset + packed_key->key_size;
+		packed_key = &pre->kernel_subkey;
 	}
 
-	/* Firmware stage structs are no longer present */
-	sd->data_key_size = 0;
-	sd->preamble_size = 0;
-	sd->hash_size = 0;
+	sd->kernel_key_offset = vb2_offset_of(sd, packed_key);
+	sd->kernel_key_size = packed_key->key_offset + packed_key->key_size;
 
-	/*
-	 * Kernel key will persist in the workbuf after we return.
-	 *
-	 * Work buffer now contains:
-	 *   - vb2_shared_data
-	 *   - kernel key
-	 */
-	sd->kernel_key_size = key_size;
-	vb2_set_workbuf_used(ctx, sd->kernel_key_offset + sd->kernel_key_size);
+	vb2_set_workbuf_used(ctx, vb2_offset_of(sd, wb.buf));
 
 	return VB2_SUCCESS;
 }
diff --git a/tests/vb20_api_kernel_tests.c b/tests/vb20_api_kernel_tests.c
index 646f8ec4..5fba07e4 100644
--- a/tests/vb20_api_kernel_tests.c
+++ b/tests/vb20_api_kernel_tests.c
@@ -16,12 +16,15 @@
 #include "2sysincludes.h"
 #include "test_common.h"
 #include "vb2_common.h"
+#include "vboot_struct.h"
 
 /* Common context for tests */
 static uint8_t workbuf[VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE]
 	__attribute__((aligned(VB2_WORKBUF_ALIGN)));
 static struct vb2_context *ctx;
 static struct vb2_shared_data *sd;
+static uint8_t shared_data[VB_SHARED_DATA_MIN_SIZE];
+static VbSharedDataHeader *shared = (VbSharedDataHeader *)shared_data;
 static struct vb2_fw_preamble *fwpre;
 static struct vb2_kernel_preamble *kpre;
 static struct vb2_packed_key *kdkey;
@@ -38,9 +41,9 @@ static struct {
 
 static int mock_read_res_fail_on_call;
 static int mock_unpack_key_retval;
-static int mock_read_gbb_header_retval;
 static int mock_load_kernel_keyblock_retval;
 static int mock_load_kernel_preamble_retval;
+static int mock_secdata_fwmp_check_retval;
 
 /* Type of test to reset for */
 enum reset_type {
@@ -59,6 +62,8 @@ static void reset_common_data(enum reset_type t)
 		  "vb2api_init failed");
 
 	sd = vb2_get_sd(ctx);
+	memset(&shared_data, 0, sizeof(shared_data));
+	sd->vbsd = shared;
 
 	vb2_nv_init(ctx);
 
@@ -68,11 +73,12 @@ static void reset_common_data(enum reset_type t)
 
 	mock_read_res_fail_on_call = 0;
 	mock_unpack_key_retval = VB2_SUCCESS;
-	mock_read_gbb_header_retval = VB2_SUCCESS;
 	mock_load_kernel_keyblock_retval = VB2_SUCCESS;
 	mock_load_kernel_preamble_retval = VB2_SUCCESS;
+	mock_secdata_fwmp_check_retval = VB2_SUCCESS;
 
 	/* Recovery key in mock GBB */
+	memset(&mock_gbb, 0, sizeof(mock_gbb));
 	mock_gbb.recovery_key.algorithm = 11;
 	mock_gbb.recovery_key.key_offset =
 		vb2_offset_of(&mock_gbb.recovery_key,
@@ -154,6 +160,16 @@ static void reset_common_data(enum reset_type t)
 
 /* Mocked functions */
 
+vb2_error_t vb2api_secdata_fwmp_check(struct vb2_context *c, uint8_t *size)
+{
+	return mock_secdata_fwmp_check_retval;
+}
+
+struct vb2_gbb_header *vb2_get_gbb(struct vb2_context *c)
+{
+	return &mock_gbb.h;
+}
+
 vb2_error_t vb2ex_read_resource(struct vb2_context *c,
 				enum vb2_resource_index index, uint32_t offset,
 				void *buf, uint32_t size)
@@ -180,13 +196,6 @@ vb2_error_t vb2ex_read_resource(struct vb2_context *c,
 	return VB2_SUCCESS;
 }
 
-vb2_error_t vb2_read_gbb_header(struct vb2_context *c,
-				struct vb2_gbb_header *gbb)
-{
-	memcpy(gbb, &mock_gbb.h, sizeof(*gbb));
-	return mock_read_gbb_header_retval;
-}
-
 vb2_error_t vb2_load_kernel_keyblock(struct vb2_context *c)
 {
 	return mock_load_kernel_keyblock_retval;
@@ -222,15 +231,14 @@ vb2_error_t vb2_verify_digest(const struct vb2_public_key *key,
 static void phase1_tests(void)
 {
 	struct vb2_packed_key *k;
-	uint32_t old_preamble_offset;
+	uint32_t wb_used_before;
 
 	/* Test successful call */
 	reset_common_data(FOR_PHASE1);
-	old_preamble_offset = sd->preamble_offset;
 	TEST_SUCC(vb2api_kernel_phase1(ctx), "phase1 good");
-	TEST_EQ(sd->preamble_size, 0, "  no more fw preamble");
 	/* Make sure normal key was loaded */
-	TEST_EQ(sd->kernel_key_offset, old_preamble_offset,
+	TEST_EQ(sd->kernel_key_offset, sd->preamble_offset +
+		offsetof(struct vb2_fw_preamble, kernel_subkey),
 		"  workbuf key offset");
 	k = vb2_member_of(sd, sd->kernel_key_offset);
 	TEST_EQ(sd->kernel_key_size, k->key_offset + k->key_size,
@@ -245,18 +253,21 @@ static void phase1_tests(void)
 		       k->key_size), 0, "  key data");
 	TEST_EQ(sd->kernel_version_secdata, 0x20002,
 		"  secdata_kernel version");
+	TEST_EQ(sd->vbsd->kernel_version_tpm, 0x20002,
+		"  secdata_kernel version (vboot1)");
+	TEST_EQ(sd->vbsd->kernel_version_tpm_start, 0x20002,
+		"  secdata_kernel version (vboot1)");
 
 	/* Test successful call in recovery mode */
 	reset_common_data(FOR_PHASE1);
 	ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
-	/* No preamble loaded in recovery mode */
-	old_preamble_offset = sd->preamble_offset;
+	/* No preamble needed in recovery mode */
 	sd->workbuf_used = sd->preamble_offset;
 	sd->preamble_offset = sd->preamble_size = 0;
+	wb_used_before = sd->workbuf_used;
 	TEST_SUCC(vb2api_kernel_phase1(ctx), "phase1 rec good");
-	TEST_EQ(sd->preamble_size, 0, "no more fw preamble");
 	/* Make sure recovery key was loaded */
-	TEST_EQ(sd->kernel_key_offset, old_preamble_offset,
+	TEST_EQ(sd->kernel_key_offset, wb_used_before,
 		"  workbuf key offset");
 	k = vb2_member_of(sd, sd->kernel_key_offset);
 	TEST_EQ(sd->kernel_key_size, k->key_offset + k->key_size,
@@ -272,45 +283,69 @@ static void phase1_tests(void)
 		"  key data");
 	TEST_EQ(sd->kernel_version_secdata, 0x20002,
 		"  secdata_kernel version");
+	TEST_EQ(sd->vbsd->kernel_version_tpm, 0x20002,
+		"  secdata_kernel version (vboot1)");
+	TEST_EQ(sd->vbsd->kernel_version_tpm_start, 0x20002,
+		"  secdata_kernel version (vboot1)");
 
 	/* Bad secdata_kernel causes failure in normal mode only */
 	reset_common_data(FOR_PHASE1);
 	ctx->secdata_kernel[0] ^= 0x33;
 	TEST_EQ(vb2api_kernel_phase1(ctx), VB2_ERROR_SECDATA_KERNEL_CRC,
-		"phase1 bad secdata");
+		"phase1 bad secdata_kernel");
+	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
+		VB2_RECOVERY_SECDATA_KERNEL_INIT, "  recovery reason");
 
 	reset_common_data(FOR_PHASE1);
 	ctx->secdata_kernel[0] ^= 0x33;
 	ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
-	TEST_SUCC(vb2api_kernel_phase1(ctx), "phase1 bad secdata rec");
+	TEST_SUCC(vb2api_kernel_phase1(ctx), "phase1 bad secdata_kernel rec");
 	TEST_EQ(sd->kernel_version_secdata, 0, "  secdata_kernel version");
-
-	/* Failures while reading recovery key */
+	TEST_EQ(sd->vbsd->kernel_version_tpm, 0,
+		"  secdata_kernel version (vboot1)");
+	TEST_EQ(sd->vbsd->kernel_version_tpm_start, 0,
+		"  secdata_kernel version (vboot1)");
+	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
+		VB2_RECOVERY_NOT_REQUESTED, "  no recovery");
+
+	/* Bad secdata_fwmp causes failure in normal mode only */
 	reset_common_data(FOR_PHASE1);
-	ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
-	sd->workbuf_used = sd->workbuf_size + VB2_WORKBUF_ALIGN -
-			   vb2_wb_round_up(sizeof(struct vb2_gbb_header));
-	TEST_EQ(vb2api_kernel_phase1(ctx), VB2_ERROR_GBB_WORKBUF,
-		"phase1 rec workbuf gbb header");
+	mock_secdata_fwmp_check_retval = VB2_ERROR_SECDATA_FWMP_CRC;
+	TEST_EQ(vb2api_kernel_phase1(ctx), mock_secdata_fwmp_check_retval,
+		"phase1 bad secdata_fwmp");
+	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
+		VB2_RECOVERY_SECDATA_FWMP_INIT, "  recovery reason");
 
 	reset_common_data(FOR_PHASE1);
+	mock_secdata_fwmp_check_retval = VB2_ERROR_SECDATA_FWMP_CRC;
 	ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
-	mock_read_gbb_header_retval = VB2_ERROR_MOCK;
-	TEST_EQ(vb2api_kernel_phase1(ctx), VB2_ERROR_MOCK,
-		"phase1 rec gbb read header");
+	TEST_SUCC(vb2api_kernel_phase1(ctx), "phase1 bad secdata_fwmp rec");
+	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
+		VB2_RECOVERY_NOT_REQUESTED, "  no recovery");
 
+	/* Failures while reading recovery key */
 	reset_common_data(FOR_PHASE1);
 	ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
 	mock_gbb.h.recovery_key_size = sd->workbuf_size - 1;
-	TEST_EQ(vb2api_kernel_phase1(ctx),
-		VB2_ERROR_API_KPHASE1_WORKBUF_REC_KEY,
+	mock_gbb.recovery_key.key_size =
+		mock_gbb.h.recovery_key_size - sizeof(mock_gbb.recovery_key);
+	TEST_EQ(vb2api_kernel_phase1(ctx), VB2_SUCCESS,
 		"phase1 rec workbuf key");
+	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;
+	TEST_ABORT(vb2api_kernel_phase1(ctx), "  fatal for manual recovery");
 
 	reset_common_data(FOR_PHASE1);
 	ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
 	mock_read_res_fail_on_call = 1;
-	TEST_EQ(vb2api_kernel_phase1(ctx), VB2_ERROR_MOCK,
+	TEST_EQ(vb2api_kernel_phase1(ctx), VB2_SUCCESS,
 		"phase1 rec gbb read key");
+	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;
+	mock_read_res_fail_on_call = 1;
+	TEST_ABORT(vb2api_kernel_phase1(ctx), "  fatal for manual recovery");
 
 	/* Failures while parsing subkey from firmware preamble */
 	reset_common_data(FOR_PHASE1);
diff --git a/tests/vboot_api_kernel4_tests.c b/tests/vboot_api_kernel4_tests.c
index 97a6b1d2..a376be09 100644
--- a/tests/vboot_api_kernel4_tests.c
+++ b/tests/vboot_api_kernel4_tests.c
@@ -38,6 +38,7 @@ static vb2_error_t commit_data_retval;
 static int commit_data_called;
 static vb2_error_t secdata_kernel_init_retval;
 static vb2_error_t secdata_fwmp_init_retval;
+static vb2_error_t kernel_phase1_retval;
 
 static uint32_t mock_switches[8];
 static uint32_t mock_switches_count;
@@ -58,6 +59,7 @@ static void ResetMocks(void)
 	sd = vb2_get_sd(ctx);
 	sd->flags |= VB2_SD_FLAG_DISPLAY_AVAILABLE;
 	ctx->flags |= VB2_CONTEXT_NO_SECDATA_FWMP;
+	sd->preamble_size = 1;
 
 	vb2_nv_init(ctx);
 	vb2_nv_set(ctx, VB2_NV_KERNEL_MAX_ROLLFORWARD, 0xffffffff);
@@ -70,6 +72,7 @@ static void ResetMocks(void)
 	vbboot_retval = VB2_SUCCESS;
 	secdata_kernel_init_retval = VB2_SUCCESS;
 	secdata_fwmp_init_retval = VB2_SUCCESS;
+	kernel_phase1_retval = VB2_SUCCESS;
 
 	memset(mock_switches, 0, sizeof(mock_switches));
 	mock_switches_count = 0;
@@ -78,6 +81,14 @@ static void ResetMocks(void)
 
 /* Mock functions */
 
+vb2_error_t vb2api_kernel_phase1(struct vb2_context *c)
+{
+	sd->kernel_version_secdata = kernel_version;
+	shared->kernel_version_tpm_start = kernel_version;
+	shared->kernel_version_tpm = kernel_version;
+	return kernel_phase1_retval;
+}
+
 vb2_error_t vb2ex_commit_data(struct vb2_context *c)
 {
 	commit_data_called = 1;
@@ -89,12 +100,6 @@ vb2_error_t vb2_secdata_kernel_init(struct vb2_context *c)
 	return secdata_kernel_init_retval;
 }
 
-uint32_t vb2_secdata_kernel_get(struct vb2_context *c,
-				enum vb2_secdata_kernel_param param)
-{
-	return kernel_version;
-}
-
 vb2_error_t vb2_secdata_fwmp_init(struct vb2_context *c)
 {
 	return secdata_fwmp_init_retval;
@@ -239,16 +244,10 @@ static void VbSlkTest(void)
 			  "  didn't commit nvdata");
 	}
 
-	/* Boot normal - secdata init failures */
-	ResetMocks();
-	secdata_kernel_init_retval = VB2_ERROR_UNKNOWN;
-	test_slk(secdata_kernel_init_retval, VB2_RECOVERY_SECDATA_KERNEL_INIT,
-		 "Normal secdata_kernel init error triggers recovery");
-
+	/* Boot normal - phase1 failure */
 	ResetMocks();
-	secdata_fwmp_init_retval = VB2_ERROR_UNKNOWN;
-	test_slk(secdata_fwmp_init_retval, VB2_RECOVERY_SECDATA_FWMP_INIT,
-		 "Normal secdata_fwmp init error triggers recovery");
+	kernel_phase1_retval = VB2_ERROR_MOCK;
+	test_slk(VB2_ERROR_MOCK, 0, "Normal phase1 failure");
 
 	/* Boot normal - commit data failures */
 	ResetMocks();
@@ -277,6 +276,12 @@ static void VbSlkTest(void)
 	test_slk(0, 0, "Dev doesn't roll forward");
 	TEST_EQ(kernel_version, 0x10002, "  version");
 
+	/* Boot dev - phase1 failure */
+	ResetMocks();
+	sd->flags |= VB2_SD_FLAG_DEV_MODE_ENABLED;
+	kernel_phase1_retval = VB2_ERROR_MOCK;
+	test_slk(VB2_ERROR_MOCK, 0, "Dev phase1 failure");
+
 	/* Boot recovery */
 	ResetMocks();
 	sd->recovery_reason = 123;
@@ -289,6 +294,12 @@ static void VbSlkTest(void)
 	test_slk(0, 0, "Recovery doesn't roll forward");
 	TEST_EQ(kernel_version, 0x10002, "  version");
 
+	/* Boot recovery - phase1 failure */
+	ResetMocks();
+	sd->recovery_reason = 123;
+	kernel_phase1_retval = VB2_ERROR_MOCK;
+	test_slk(VB2_ERROR_MOCK, 0, "Recovery phase1 failure");
+
 	/* Boot recovery - commit data failures */
 	ResetMocks();
 	sd->recovery_reason = 123;
diff --git a/tests/verify_kernel.c b/tests/verify_kernel.c
index 1a4831d5..ac4b45e6 100644
--- a/tests/verify_kernel.c
+++ b/tests/verify_kernel.c
@@ -115,16 +115,18 @@ int main(int argc, char *argv[])
 	sd = vb2_get_sd(ctx);
 	sd->vbsd = shared;
 
-	/* Copy kernel subkey to VBSD */
-	struct vb2_packed_key *dst = (struct vb2_packed_key *)
-		(shared_data + vb2_wb_round_up(sizeof(VbSharedDataHeader)));
-	shared->kernel_subkey.key_offset =
-		(uintptr_t)dst - (uintptr_t)&shared->kernel_subkey;
-	shared->kernel_subkey.key_size = kernkey->key_size;
-	shared->kernel_subkey.algorithm = kernkey->algorithm;
-	shared->kernel_subkey.key_version = kernkey->key_version;
-	memcpy(vb2_packed_key_data_mutable(dst), vb2_packed_key_data(kernkey),
-	       kernkey->key_size);
+	/* Copy kernel subkey to workbuf */
+	{
+		struct vb2_workbuf wb;
+		struct vb2_packed_key *dst;
+		uint32_t kernkey_size = kernkey->key_offset + kernkey->key_size;
+		vb2_workbuf_from_ctx(ctx, &wb);
+		dst = vb2_workbuf_alloc(&wb, kernkey_size);
+		memcpy(dst, kernkey, kernkey_size);
+		vb2_set_workbuf_used(ctx, vb2_offset_of(sd, wb.buf));
+		sd->kernel_key_offset = vb2_offset_of(sd, dst);
+		sd->kernel_key_size = kernkey_size;
+	}
 
 	/*
 	 * LoadKernel() cares only about VBNV_DEV_BOOT_SIGNED_ONLY, and only in