vboot: fix up some host key functions for host_key2.cfactory-excelsior-12812.B

Deprecate:
  PublicKeyInit --> vb2_init_packed_key
  PublicKeyCopy --> vb2_copy_packed_key

Rename:
  packed_key_looks_ok --> vb2_packed_key_looks_ok

Move vb2_packed_key_looks_ok from host_key.c to host_key2.c.
Move tests/vboot_common_tests.c to tests/vb2_host_key_tests.c.
Remove firmware/lib/vboot_common.c.
Remove host/lib/host_key.c.

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

Change-Id: I627b2af0416ac69460f9860614a69cad8bdb76a7
Signed-off-by: Joel Kitching <kitching@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1844597
Tested-by: Joel Kitching <kitching@chromium.org>
Reviewed-by: Joel Kitching <kitching@chromium.org>
Commit-Queue: Joel Kitching <kitching@chromium.org>

19 files changed, 61 insertions, 157 deletions

diff --git a/Makefile b/Makefile
index 0d55696e..8dec7361 100644
--- a/Makefile
+++ b/Makefile
@@ -353,7 +353,6 @@ FWLIB_SRCS = \
 	firmware/lib/utility_string.c \
 	firmware/lib/vboot_api_kernel.c \
 	firmware/lib/vboot_audio.c \
-	firmware/lib/vboot_common.c \
 	firmware/lib/vboot_display.c \
 	firmware/lib/vboot_kernel.c \
 	firmware/lib/vboot_ui.c \
@@ -463,7 +462,6 @@ UTILLIB_SRCS = \
 	host/lib/file_keys.c \
 	host/lib/fmap.c \
 	host/lib/host_common.c \
-	host/lib/host_key.c \
 	host/lib/host_key2.c \
 	host/lib/host_keyblock.c \
 	host/lib/host_misc.c \
@@ -697,7 +695,6 @@ TEST_NAMES = \
 	tests/vboot_api_kernel2_tests \
 	tests/vboot_api_kernel4_tests \
 	tests/vboot_api_kernel_tests \
-	tests/vboot_common_tests \
 	tests/vboot_detach_menu_tests \
 	tests/vboot_display_tests \
 	tests/vboot_kernel_tests \
@@ -729,6 +726,7 @@ TEST2X_NAMES = \
 	tests/vb2_common3_tests \
 	tests/vb2_ec_sync_tests \
 	tests/vb2_gbb_tests \
+	tests/vb2_host_key_tests \
 	tests/vb2_misc_tests \
 	tests/vb2_nvstorage_tests \
 	tests/vb2_rsa_utility_tests \
@@ -1149,6 +1147,7 @@ ${BUILD}/utility/pad_digest_utility: LDLIBS += ${CRYPTO_LIBS}
 ${BUILD}/utility/signature_digest_utility: LDLIBS += ${CRYPTO_LIBS}
 ${BUILD}/utility/verify_data: LDLIBS += ${CRYPTO_LIBS}
 
+${BUILD}/tests/vb2_host_key_tests: LDLIBS += ${CRYPTO_LIBS}
 ${BUILD}/tests/vb2_common2_tests: LDLIBS += ${CRYPTO_LIBS}
 ${BUILD}/tests/vb2_common3_tests: LDLIBS += ${CRYPTO_LIBS}
 ${BUILD}/tests/verify_kernel: LDLIBS += ${CRYPTO_LIBS}
@@ -1278,7 +1277,6 @@ endif
 	${RUNTEST} ${BUILD_RUN}/tests/vboot_api_kernel2_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vboot_api_kernel4_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vboot_api_kernel_tests
-	${RUNTEST} ${BUILD_RUN}/tests/vboot_common_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vboot_detach_menu_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vboot_display_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vboot_kernel_tests
@@ -1292,6 +1290,7 @@ run2tests: test_setup
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_common3_tests ${TEST_KEYS}
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_ec_sync_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_gbb_tests
+	${RUNTEST} ${BUILD_RUN}/tests/vb2_host_key_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_misc_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_nvstorage_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_rsa_utility_tests
diff --git a/firmware/2lib/2auxfw_sync.c b/firmware/2lib/2auxfw_sync.c
index e90ee7ff..d44baed4 100644
--- a/firmware/2lib/2auxfw_sync.c
+++ b/firmware/2lib/2auxfw_sync.c
@@ -11,7 +11,6 @@
 #include "2nvstorage.h"
 #include "2sysincludes.h"
 #include "vboot_api.h"
-#include "vboot_common.h"
 #include "vboot_display.h"
 
 /**
diff --git a/firmware/2lib/include/2return_codes.h b/firmware/2lib/include/2return_codes.h
index b51bab3d..b504a087 100644
--- a/firmware/2lib/include/2return_codes.h
+++ b/firmware/2lib/include/2return_codes.h
@@ -888,6 +888,9 @@ enum vb2_return_code {
 	/* Unable to copy packed key */
 	VB2_ERROR_PACKED_KEY_COPY,
 
+	/* Packed key with invalid version */
+	VB2_ERROR_PACKED_KEY_VERSION,
+
 	/**********************************************************************
 	 * Errors generated by host library signature functions
 	 */
diff --git a/firmware/lib/include/vboot_common.h b/firmware/lib/include/vboot_common.h
deleted file mode 100644
index 944ce352..00000000
--- a/firmware/lib/include/vboot_common.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/* Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- *
- * Common functions between firmware and kernel verified boot.
- */
-
-#ifndef VBOOT_REFERENCE_VBOOT_COMMON_H_
-#define VBOOT_REFERENCE_VBOOT_COMMON_H_
-
-#include "2api.h"
-#include "2struct.h"
-#include "vboot_struct.h"
-
-/**
- * Initialize a public key to refer to [key_data].
- */
-void PublicKeyInit(struct vb2_packed_key *key,
-		   uint8_t *key_data, uint64_t key_size);
-
-/**
- * Copy a public key from [src] to [dest].
- *
- * Returns 0 if success, non-zero if error.
- */
-int PublicKeyCopy(struct vb2_packed_key *dest,
-		  const struct vb2_packed_key *src);
-
-#endif  /* VBOOT_REFERENCE_VBOOT_COMMON_H_ */
diff --git a/firmware/lib/vboot_common.c b/firmware/lib/vboot_common.c
deleted file mode 100644
index ee2c9134..00000000
--- a/firmware/lib/vboot_common.c
+++ /dev/null
@@ -1,39 +0,0 @@
-/* Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- *
- * Common functions between firmware and kernel verified boot.
- * (Firmware portion)
- */
-
-#include "2common.h"
-#include "2misc.h"
-#include "2rsa.h"
-#include "2sha.h"
-#include "2sysincludes.h"
-#include "utility.h"
-#include "vboot_api.h"
-#include "vboot_common.h"
-
-void PublicKeyInit(struct vb2_packed_key *key,
-		   uint8_t *key_data, uint64_t key_size)
-{
-	key->key_offset = vb2_offset_of(key, key_data);
-	key->key_size = key_size;
-	key->algorithm = VB2_ALG_COUNT; /* Key not present yet */
-	key->key_version = 0;
-}
-
-int PublicKeyCopy(struct vb2_packed_key *dest, const struct vb2_packed_key *src)
-{
-	if (dest->key_size < src->key_size)
-		return 1;
-
-	dest->key_size = src->key_size;
-	dest->algorithm = src->algorithm;
-	dest->key_version = src->key_version;
-	memcpy(vb2_packed_key_data_mutable(dest),
-	       vb2_packed_key_data(src),
-	       src->key_size);
-	return 0;
-}
diff --git a/futility/cmd_show.c b/futility/cmd_show.c
index 9a2eec19..402b1330 100644
--- a/futility/cmd_show.c
+++ b/futility/cmd_show.c
@@ -87,7 +87,7 @@ int ft_show_pubkey(const char *name, uint8_t *buf, uint32_t len, void *data)
 {
 	struct vb2_packed_key *pubkey = (struct vb2_packed_key *)buf;
 
-	if (!packed_key_looks_ok(pubkey, len)) {
+	if (vb2_packed_key_looks_ok(pubkey, len)) {
 		printf("%s looks bogus\n", name);
 		return 1;
 	}
diff --git a/futility/cmd_sign.c b/futility/cmd_sign.c
index ce37e87e..109287c9 100644
--- a/futility/cmd_sign.c
+++ b/futility/cmd_sign.c
@@ -62,7 +62,7 @@ int ft_sign_pubkey(const char *name, uint8_t *buf, uint32_t len, void *data)
 	struct vb2_packed_key *data_key = (struct vb2_packed_key *)buf;
 	struct vb2_keyblock *block;
 
-	if (!packed_key_looks_ok(data_key, len)) {
+	if (vb2_packed_key_looks_ok(data_key, len)) {
 		fprintf(stderr, "Public key looks bad.\n");
 		return 1;
 	}
diff --git a/futility/cmd_vbutil_kernel.c b/futility/cmd_vbutil_kernel.c
index a0c001ac..1684c185 100644
--- a/futility/cmd_vbutil_kernel.c
+++ b/futility/cmd_vbutil_kernel.c
@@ -27,7 +27,6 @@
 #include "kernel_blob.h"
 #include "vb1_helper.h"
 #include "vb2_common.h"
-#include "vboot_common.h"
 
 /* Global opts */
 static int opt_verbose;
diff --git a/futility/file_type_bios.c b/futility/file_type_bios.c
index 35a4f1dd..9d45269c 100644
--- a/futility/file_type_bios.c
+++ b/futility/file_type_bios.c
@@ -95,7 +95,7 @@ int ft_show_gbb(const char *name, uint8_t *buf, uint32_t len, void *data)
 
 	struct vb2_packed_key *pubkey =
 		(struct vb2_packed_key *)(buf + gbb->rootkey_offset);
-	if (packed_key_looks_ok(pubkey, gbb->rootkey_size)) {
+	if (vb2_packed_key_looks_ok(pubkey, gbb->rootkey_size) == VB2_SUCCESS) {
 		if (state) {
 			state->rootkey.offset =
 				state->area[BIOS_FMAP_GBB].offset +
@@ -112,7 +112,8 @@ int ft_show_gbb(const char *name, uint8_t *buf, uint32_t len, void *data)
 	}
 
 	pubkey = (struct vb2_packed_key *)(buf + gbb->recovery_key_offset);
-	if (packed_key_looks_ok(pubkey, gbb->recovery_key_size)) {
+	if (vb2_packed_key_looks_ok(pubkey, gbb->recovery_key_size)
+	    == VB2_SUCCESS) {
 		if (state) {
 			state->recovery_key.offset =
 				state->area[BIOS_FMAP_GBB].offset +
@@ -264,9 +265,9 @@ static int fmap_sign_fw_preamble(const char *name, uint8_t *buf, uint32_t len,
 		goto whatever;
 	}
 
-	if (!packed_key_looks_ok(&keyblock->data_key,
-				 keyblock->data_key.key_offset +
-				 keyblock->data_key.key_size)) {
+	if (vb2_packed_key_looks_ok(&keyblock->data_key,
+				    keyblock->data_key.key_offset +
+				    keyblock->data_key.key_size)) {
 		fprintf(stderr, "Warning: %s public key is invalid. "
 			"Signing the entire FW FMAP region...\n", name);
 		goto whatever;
diff --git a/futility/updater.c b/futility/updater.c
index 6a7d8ffa..1dc0ed50 100644
--- a/futility/updater.c
+++ b/futility/updater.c
@@ -666,7 +666,7 @@ static const struct vb2_packed_key *get_rootkey(
 	struct vb2_packed_key *key = NULL;
 
 	key = (struct vb2_packed_key *)((uint8_t *)gbb + gbb->rootkey_offset);
-	if (!packed_key_looks_ok(key, gbb->rootkey_size)) {
+	if (vb2_packed_key_looks_ok(key, gbb->rootkey_size)) {
 		ERROR("Invalid root key.\n");
 		return NULL;
 	}
diff --git a/futility/updater_archive.c b/futility/updater_archive.c
index 5d8e414d..36873a68 100644
--- a/futility/updater_archive.c
+++ b/futility/updater_archive.c
@@ -1035,8 +1035,8 @@ static const char *get_gbb_key_hash(const struct vb2_gbb_header *gbb,
 	if (!gbb)
 		return "<No GBB>";
 	key = (struct vb2_packed_key *)((uint8_t *)gbb + offset);
-	if (!packed_key_looks_ok(key, size))
-	    return "<Invalid key>";
+	if (vb2_packed_key_looks_ok(key, size))
+		return "<Invalid key>";
 	return packed_key_sha1_string(key);
 }
 
diff --git a/futility/vb1_helper.c b/futility/vb1_helper.c
index d1d7535e..74391827 100644
--- a/futility/vb1_helper.c
+++ b/futility/vb1_helper.c
@@ -797,7 +797,7 @@ enum futil_file_type ft_recognize_vb1_key(uint8_t *buf, uint32_t len)
 {
 	/* Maybe just a packed public key? */
 	const struct vb2_packed_key *pubkey = (struct vb2_packed_key *)buf;
-	if (packed_key_looks_ok(pubkey, len))
+	if (vb2_packed_key_looks_ok(pubkey, len) == VB2_SUCCESS)
 		return FILE_TYPE_PUBKEY;
 
 	/* How about a private key? */
diff --git a/host/lib/host_key.c b/host/lib/host_key.c
deleted file mode 100644
index 3278d88a..00000000
--- a/host/lib/host_key.c
+++ /dev/null
@@ -1,40 +0,0 @@
-/* Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- *
- * Host functions for keys.
- */
-
-/* TODO: change all 'return 0', 'return 1' into meaningful return codes */
-
-#include <openssl/pem.h>
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-
-#include "2common.h"
-#include "2rsa.h"
-#include "2sha.h"
-#include "2sysincludes.h"
-#include "host_common.h"
-#include "host_misc.h"
-#include "vb2_common.h"
-
-int packed_key_looks_ok(const struct vb2_packed_key *key, uint32_t size)
-{
-	struct vb2_public_key pubkey;
-	if (VB2_SUCCESS != vb2_unpack_key_buffer(&pubkey,
-						 (const uint8_t *)key,
-						 size))
-		return 0;
-
-	if (key->key_version > VB2_MAX_KEY_VERSION) {
-		/* Currently, TPM only supports 16-bit version */
-		fprintf(stderr, "%s() packed key invalid version\n", __func__);
-		return 0;
-	}
-
-	/* Success */
-	return 1;
-}
diff --git a/host/lib/host_key2.c b/host/lib/host_key2.c
index f845f86f..5849cf76 100644
--- a/host/lib/host_key2.c
+++ b/host/lib/host_key2.c
@@ -5,8 +5,6 @@
  * Host functions for keys.
  */
 
-/* TODO: change all 'return 0', 'return 1' into meaningful return codes */
-
 #include <openssl/pem.h>
 
 #include <stdio.h>
@@ -211,7 +209,7 @@ struct vb2_packed_key *vb2_read_packed_key(const char *filename)
 		return NULL;
 	}
 
-	if (packed_key_looks_ok(key, file_size))
+	if (vb2_packed_key_looks_ok(key, file_size) == VB2_SUCCESS)
 		return key;
 
 	/* Error */
@@ -279,3 +277,22 @@ vb2_error_t vb2_write_packed_key(const char *filename,
 	free(kcopy);
 	return rv;
 }
+
+vb2_error_t vb2_packed_key_looks_ok(const struct vb2_packed_key *key,
+				    uint32_t size)
+{
+	struct vb2_public_key pubkey;
+	vb2_error_t rv;
+
+	rv = vb2_unpack_key_buffer(&pubkey, (const uint8_t *)key, size);
+	if (rv)
+		return rv;
+
+	if (key->key_version > VB2_MAX_KEY_VERSION) {
+		/* Currently, TPM only supports 16-bit version */
+		VB2_DEBUG("packed key invalid version\n");
+		return VB2_ERROR_PACKED_KEY_VERSION;
+	}
+
+	return VB2_SUCCESS;
+}
diff --git a/host/lib/include/host_key.h b/host/lib/include/host_key.h
index b00a501d..26792720 100644
--- a/host/lib/include/host_key.h
+++ b/host/lib/include/host_key.h
@@ -117,9 +117,10 @@ struct vb2_packed_key *vb2_read_packed_key(const char *filename);
  * @param key	     	Key to check
  * @param size		Size of key buffer in bytes
  *
- * @return True if the key struct appears valid.
+ * @return VB2_SUCCESS, or non-zero if error.
  */
-int packed_key_looks_ok(const struct vb2_packed_key *key, uint32_t size);
+vb2_error_t vb2_packed_key_looks_ok(const struct vb2_packed_key *key,
+				    uint32_t size);
 
 /**
  * Read a packed key from a .keyb file.
diff --git a/tests/vb2_auxfw_sync_tests.c b/tests/vb2_auxfw_sync_tests.c
index 470c2d2a..3ce0cd7f 100644
--- a/tests/vb2_auxfw_sync_tests.c
+++ b/tests/vb2_auxfw_sync_tests.c
@@ -18,7 +18,6 @@
 #include "secdata_tpm.h"
 #include "test_common.h"
 #include "vboot_audio.h"
-#include "vboot_common.h"
 #include "vboot_display.h"
 #include "vboot_kernel.h"
 #include "vboot_struct.h"
diff --git a/tests/vboot_common_tests.c b/tests/vb2_host_key_tests.c
index 50e2bb59..82dd3572 100644
--- a/tests/vboot_common_tests.c
+++ b/tests/vb2_host_key_tests.c
@@ -1,22 +1,16 @@
-/* Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
+/* Copyright 2019 The Chromium OS Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  *
- * Tests for firmware vboot_common.c
+ * Tests for host library vboot2 key functions
  */
 
-#include <stdint.h>
-#include <stdio.h>
-#include <stdlib.h>
-
 #include "2common.h"
 #include "host_common.h"
 #include "test_common.h"
-#include "utility.h"
-#include "vboot_common.h"
 
 /* Public key utility functions */
-static void PublicKeyTest(void)
+static void public_key_tests(void)
 {
 	struct vb2_packed_key k[3];
 	struct vb2_packed_key j[5];
@@ -27,46 +21,48 @@ static void PublicKeyTest(void)
 	k[1].key_size = 12345;
 	k[2].key_version = 67;
 
-	PublicKeyInit(k, (uint8_t*)(k + 1), 2 * sizeof(struct vb2_packed_key));
+	vb2_init_packed_key(k, (uint8_t*)(k + 1),
+			    2 * sizeof(struct vb2_packed_key));
 	TEST_EQ(k->key_offset, sizeof(struct vb2_packed_key),
-		"PublicKeyInit key_offset");
+		"vb2_init_packed_key key_offset");
 	TEST_EQ(k->key_size, 2 * sizeof(struct vb2_packed_key),
-		"PublicKeyInit key_size");
-	TEST_EQ(k->algorithm, VB2_ALG_COUNT, "PublicKeyInit algorithm");
-	TEST_EQ(k->key_version, 0, "PublicKeyInit key_version");
+		"vb2_init_packed_key key_size");
+	TEST_EQ(k->algorithm, VB2_ALG_COUNT, "vb2_init_packed_key algorithm");
+	TEST_EQ(k->key_version, 0, "vb2_init_packed_key key_version");
 
 	/* Set algorithm and version, so we can tell if they get copied */
 	k->algorithm = 3;
 	k->key_version = 21;
 
 	/* Copying to a smaller destination should fail */
-	PublicKeyInit(j, (uint8_t*)(j + 1),
+	vb2_init_packed_key(j, (uint8_t*)(j + 1),
 		      2 * sizeof(struct vb2_packed_key) - 1);
-	TEST_NEQ(0, PublicKeyCopy(j, k), "PublicKeyCopy too small");
+	TEST_NEQ(0, vb2_copy_packed_key(j, k), "vb2_copy_packed_key too small");
 
 	/* Copying to same or larger size should succeed */
-	PublicKeyInit(j, (uint8_t*)(j + 2),
+	vb2_init_packed_key(j, (uint8_t*)(j + 2),
 		      2 * sizeof(struct vb2_packed_key) + 1);
-	TEST_EQ(0, PublicKeyCopy(j, k), "PublicKeyCopy same");
+	TEST_EQ(0, vb2_copy_packed_key(j, k), "vb2_copy_packed_key same");
 	/* Offset in destination shouldn't have been modified */
 	TEST_EQ(j->key_offset, 2 * sizeof(struct vb2_packed_key),
-		"PublicKeyCopy key_offset");
+		"vb2_copy_packed_key key_offset");
 	/* Size should have been reduced to match the source */
 	TEST_EQ(k->key_size, 2 * sizeof(struct vb2_packed_key),
-		"PublicKeyCopy key_size");
+		"vb2_copy_packed_key key_size");
 	/* Other fields should have been copied */
-	TEST_EQ(k->algorithm, j->algorithm, "PublicKeyCopy algorithm");
-	TEST_EQ(k->key_version, j->key_version, "PublicKeyCopy key_version");
+	TEST_EQ(k->algorithm, j->algorithm, "vb2_copy_packed_key algorithm");
+	TEST_EQ(k->key_version, j->key_version,
+		"vb2_copy_packed_key key_version");
 	/* Data should have been copied */
 	TEST_EQ(0,
 		memcmp(vb2_packed_key_data(k),
 		       vb2_packed_key_data(j), k->key_size),
-		"PublicKeyCopy data");
+		"vb2_copy_packed_key data");
 }
 
 int main(int argc, char* argv[])
 {
-	PublicKeyTest();
+	public_key_tests();
 
 	return gTestSuccess ? 0 : 255;
 }
diff --git a/tests/verify_kernel.c b/tests/verify_kernel.c
index ddb1fe30..1a4831d5 100644
--- a/tests/verify_kernel.c
+++ b/tests/verify_kernel.c
@@ -15,7 +15,6 @@
 #include "2nvstorage.h"
 #include "host_common.h"
 #include "util_misc.h"
-#include "vboot_common.h"
 #include "vboot_api.h"
 #include "vboot_kernel.h"
 
diff --git a/utility/load_kernel_test.c b/utility/load_kernel_test.c
index 3023e219..89de19e8 100644
--- a/utility/load_kernel_test.c
+++ b/utility/load_kernel_test.c
@@ -16,7 +16,6 @@
 #include "host_common.h"
 #include "load_kernel_fw.h"
 #include "secdata_tpm.h"
-#include "vboot_common.h"
 #include "vboot_kernel.h"
 
 #define LBA_BYTES 512