vboot: fix up some host key functions for host_key2.c
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>
diff --git a/Makefile b/Makefile
index 0d55696..8dec736 100644
--- a/Makefile
+++ b/Makefile
@@ -353,7 +353,6 @@
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 @@
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 @@
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 @@
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/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 @@
${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 @@
${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 e90ee7f..d44baed 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 b51bab3..b504a08 100644
--- a/firmware/2lib/include/2return_codes.h
+++ b/firmware/2lib/include/2return_codes.h
@@ -888,6 +888,9 @@
/* 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 944ce35..0000000
--- 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 ee2c913..0000000
--- 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 9a2eec1..402b133 100644
--- a/futility/cmd_show.c
+++ b/futility/cmd_show.c
@@ -87,7 +87,7 @@
{
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 ce37e87..109287c 100644
--- a/futility/cmd_sign.c
+++ b/futility/cmd_sign.c
@@ -62,7 +62,7 @@
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 a0c001a..1684c18 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 35a4f1d..9d45269 100644
--- a/futility/file_type_bios.c
+++ b/futility/file_type_bios.c
@@ -95,7 +95,7 @@
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 @@
}
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 @@
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 6a7d8ff..1dc0ed5 100644
--- a/futility/updater.c
+++ b/futility/updater.c
@@ -666,7 +666,7 @@
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 5d8e414..36873a6 100644
--- a/futility/updater_archive.c
+++ b/futility/updater_archive.c
@@ -1035,8 +1035,8 @@
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 d1d7535..7439182 100644
--- a/futility/vb1_helper.c
+++ b/futility/vb1_helper.c
@@ -797,7 +797,7 @@
{
/* 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 3278d88..0000000
--- 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 f845f86..5849cf7 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 @@
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 @@
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 b00a501..2679272 100644
--- a/host/lib/include/host_key.h
+++ b/host/lib/include/host_key.h
@@ -117,9 +117,10 @@
* @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 470c2d2..3ce0cd7 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/vb2_host_key_tests.c b/tests/vb2_host_key_tests.c
new file mode 100644
index 0000000..82dd357
--- /dev/null
+++ b/tests/vb2_host_key_tests.c
@@ -0,0 +1,68 @@
+/* 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 host library vboot2 key functions
+ */
+
+#include "2common.h"
+#include "host_common.h"
+#include "test_common.h"
+
+/* Public key utility functions */
+static void public_key_tests(void)
+{
+ struct vb2_packed_key k[3];
+ struct vb2_packed_key j[5];
+
+ /* Fill some bits of the public key data */
+ memset(j, 0, sizeof(j));
+ memset(k, 0x42, sizeof(k));
+ k[1].key_size = 12345;
+ k[2].key_version = 67;
+
+ 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),
+ "vb2_init_packed_key key_offset");
+ TEST_EQ(k->key_size, 2 * sizeof(struct vb2_packed_key),
+ "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 */
+ vb2_init_packed_key(j, (uint8_t*)(j + 1),
+ 2 * sizeof(struct vb2_packed_key) - 1);
+ TEST_NEQ(0, vb2_copy_packed_key(j, k), "vb2_copy_packed_key too small");
+
+ /* Copying to same or larger size should succeed */
+ vb2_init_packed_key(j, (uint8_t*)(j + 2),
+ 2 * sizeof(struct vb2_packed_key) + 1);
+ 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),
+ "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),
+ "vb2_copy_packed_key key_size");
+ /* Other fields should have been copied */
+ 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),
+ "vb2_copy_packed_key data");
+}
+
+int main(int argc, char* argv[])
+{
+ public_key_tests();
+
+ return gTestSuccess ? 0 : 255;
+}
diff --git a/tests/vboot_common_tests.c b/tests/vboot_common_tests.c
deleted file mode 100644
index 50e2bb5..0000000
--- a/tests/vboot_common_tests.c
+++ /dev/null
@@ -1,72 +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.
- *
- * Tests for firmware vboot_common.c
- */
-
-#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)
-{
- struct vb2_packed_key k[3];
- struct vb2_packed_key j[5];
-
- /* Fill some bits of the public key data */
- memset(j, 0, sizeof(j));
- memset(k, 0x42, sizeof(k));
- k[1].key_size = 12345;
- k[2].key_version = 67;
-
- PublicKeyInit(k, (uint8_t*)(k + 1), 2 * sizeof(struct vb2_packed_key));
- TEST_EQ(k->key_offset, sizeof(struct vb2_packed_key),
- "PublicKeyInit 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");
-
- /* 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),
- 2 * sizeof(struct vb2_packed_key) - 1);
- TEST_NEQ(0, PublicKeyCopy(j, k), "PublicKeyCopy too small");
-
- /* Copying to same or larger size should succeed */
- PublicKeyInit(j, (uint8_t*)(j + 2),
- 2 * sizeof(struct vb2_packed_key) + 1);
- TEST_EQ(0, PublicKeyCopy(j, k), "PublicKeyCopy same");
- /* Offset in destination shouldn't have been modified */
- TEST_EQ(j->key_offset, 2 * sizeof(struct vb2_packed_key),
- "PublicKeyCopy 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");
- /* 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");
- /* Data should have been copied */
- TEST_EQ(0,
- memcmp(vb2_packed_key_data(k),
- vb2_packed_key_data(j), k->key_size),
- "PublicKeyCopy data");
-}
-
-int main(int argc, char* argv[])
-{
- PublicKeyTest();
-
- return gTestSuccess ? 0 : 255;
-}
diff --git a/tests/verify_kernel.c b/tests/verify_kernel.c
index ddb1fe3..1a4831d 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 3023e21..89de19e 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
