From c6b3ad3760aeb27e272c85f7a519189f7124c0de Mon Sep 17 00:00:00 2001 From: Kangheui Won Date: Thu, 13 Aug 2020 15:48:41 +1000 Subject: vboot2: use hwcrypto for RSA when allowed Add vb2ex_hwcrypto_rsa_verify support for RSA verification. If firmware implements the function it will used instead of SW implementation in vboot. Also separate hwcrypto stubs to 2stub_hwcrypto.c for depthcharge and coreboot. Depthcharge needs stubs but fails to compile 2stub.c BRANCH=none BUG=b:163710320, b:161205813 TEST=make runtests TEST=check hwcrypto is allowed/disallowed depending on nvmem flag Change-Id: I85573e7cff31f32043db4b0a6b24b642856024e3 Signed-off-by: Kangheui Won Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2353775 Reviewed-by: Julius Werner Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2377545 Reviewed-by: Furquan Shaikh Commit-Queue: Furquan Shaikh Tested-by: Furquan Shaikh --- Makefile | 2 ++ firmware/2lib/2api.c | 2 ++ firmware/2lib/2common.c | 17 +++++++++++++ firmware/2lib/2kernel.c | 1 + firmware/2lib/2stub.c | 27 -------------------- firmware/2lib/2stub_hwcrypto.c | 36 ++++++++++++++++++++++++++ firmware/2lib/include/2rsa.h | 1 + firmware/2lib/include/2secdata.h | 31 ++++++++++++++++++++++ firmware/lib20/misc.c | 4 +++ firmware/lib20/packed_key.c | 3 +++ tests/vb20_misc_tests.c | 55 ++++++++++++++++++++++++++++++++++++++++ tests/vb2_api_tests.c | 26 +++++++++++++++++++ tests/vb2_common2_tests.c | 46 +++++++++++++++++++++++++++++++++ 13 files changed, 224 insertions(+), 27 deletions(-) create mode 100644 firmware/2lib/2stub_hwcrypto.c diff --git a/Makefile b/Makefile index e025907f..f77e79e9 100644 --- a/Makefile +++ b/Makefile @@ -378,6 +378,7 @@ FWLIB_SRCS = \ firmware/2lib/2sha256.c \ firmware/2lib/2sha512.c \ firmware/2lib/2sha_utility.c \ + firmware/2lib/2stub_hwcrypto.c \ firmware/2lib/2tpm_bootmode.c \ firmware/2lib/2ui.c \ firmware/2lib/2ui_screens.c \ @@ -503,6 +504,7 @@ HOSTLIB_SRCS = \ firmware/2lib/2sha512.c \ firmware/2lib/2sha_utility.c \ firmware/2lib/2stub.c \ + firmware/2lib/2stub_hwcrypto.c \ firmware/lib/cgptlib/cgptlib_internal.c \ firmware/lib/cgptlib/crc32.c \ firmware/lib/gpt_misc.c \ diff --git a/firmware/2lib/2api.c b/firmware/2lib/2api.c index d49b8d31..95dc6c2d 100644 --- a/firmware/2lib/2api.c +++ b/firmware/2lib/2api.c @@ -355,6 +355,8 @@ vb2_error_t vb2api_check_hash_get_digest(struct vb2_context *ctx, vb2_member_of(sd, sd->data_key_offset), sd->data_key_size)); + key.allow_hwcrypto = vb2_hwcrypto_rsa_allowed(ctx); + /* * Check digest vs. signature. Note that this destroys the signature. * That's ok, because we only check each signature once per boot. diff --git a/firmware/2lib/2common.c b/firmware/2lib/2common.c index 09379cc2..782ea635 100644 --- a/firmware/2lib/2common.c +++ b/firmware/2lib/2common.c @@ -164,6 +164,23 @@ vb2_error_t vb2_verify_digest(const struct vb2_public_key *key, return VB2_ERROR_VDATA_SIG_SIZE; } + if (key->allow_hwcrypto) { + vb2_error_t rv = + vb2ex_hwcrypto_rsa_verify_digest(key, sig_data, digest); + + if (rv != VB2_ERROR_EX_HWCRYPTO_UNSUPPORTED) { + VB2_DEBUG("Using HW RSA engine for sig_alg %d %s\n", + key->sig_alg, + rv ? "failed" : "succeeded"); + return rv; + } + + VB2_DEBUG("HW RSA for sig_alg %d not supported, using SW\n", + key->sig_alg); + } else { + VB2_DEBUG("HW RSA forbidden, using SW\n"); + } + return vb2_rsa_verify_digest(key, sig_data, digest, wb); } diff --git a/firmware/2lib/2kernel.c b/firmware/2lib/2kernel.c index 4b6badb6..7a827c3a 100644 --- a/firmware/2lib/2kernel.c +++ b/firmware/2lib/2kernel.c @@ -149,6 +149,7 @@ vb2_error_t vb2api_kernel_phase1(struct vb2_context *ctx) flags &= ~VB2_SECDATA_KERNEL_FLAG_PHONE_RECOVERY_DISABLED; flags |= VB2_SECDATA_KERNEL_FLAG_PHONE_RECOVERY_UI_DISABLED; flags |= VB2_SECDATA_KERNEL_FLAG_DIAGNOSTIC_UI_DISABLED; + flags |= VB2_SECDATA_KERNEL_FLAG_HWCRYPTO_ALLOWED; vb2_secdata_kernel_set(ctx, VB2_SECDATA_KERNEL_FLAGS, flags); } diff --git a/firmware/2lib/2stub.c b/firmware/2lib/2stub.c index fc539fe9..ed42f748 100644 --- a/firmware/2lib/2stub.c +++ b/firmware/2lib/2stub.c @@ -40,33 +40,6 @@ vb2_error_t vb2ex_read_resource(struct vb2_context *ctx, return VB2_ERROR_EX_UNIMPLEMENTED; } -__attribute__((weak)) -vb2_error_t vb2ex_hwcrypto_digest_init(enum vb2_hash_algorithm hash_alg, - uint32_t data_size) -{ - return VB2_ERROR_EX_HWCRYPTO_UNSUPPORTED; -} - -__attribute__((weak)) -vb2_error_t vb2ex_hwcrypto_digest_extend(const uint8_t *buf, uint32_t size) -{ - return VB2_ERROR_SHA_EXTEND_ALGORITHM; /* Should not be called. */ -} - -__attribute__((weak)) -vb2_error_t vb2ex_hwcrypto_digest_finalize(uint8_t *digest, - uint32_t digest_size) -{ - return VB2_ERROR_SHA_FINALIZE_ALGORITHM; /* Should not be called. */ -} - -__attribute__((weak)) -vb2_error_t vb2ex_hwcrypto_rsa_verify_digest(const struct vb2_public_key *key, - const uint8_t *sig, const uint8_t *digest) -{ - return VB2_ERROR_EX_HWCRYPTO_UNSUPPORTED; -} - __attribute__((weak)) vb2_error_t vb2ex_tpm_set_mode(enum vb2_tpm_mode mode_val) { diff --git a/firmware/2lib/2stub_hwcrypto.c b/firmware/2lib/2stub_hwcrypto.c new file mode 100644 index 00000000..542a5edc --- /dev/null +++ b/firmware/2lib/2stub_hwcrypto.c @@ -0,0 +1,36 @@ +/* Copyright 2020 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. + * + * Stub hwcrypto API implementations which should be implemented by the caller. + */ + +#include "2api.h" + +__attribute__((weak)) +vb2_error_t vb2ex_hwcrypto_digest_init(enum vb2_hash_algorithm hash_alg, + uint32_t data_size) +{ + return VB2_ERROR_EX_HWCRYPTO_UNSUPPORTED; +} + +__attribute__((weak)) +vb2_error_t vb2ex_hwcrypto_digest_extend(const uint8_t *buf, uint32_t size) +{ + return VB2_ERROR_SHA_EXTEND_ALGORITHM; /* Should not be called. */ +} + +__attribute__((weak)) +vb2_error_t vb2ex_hwcrypto_digest_finalize(uint8_t *digest, + uint32_t digest_size) +{ + return VB2_ERROR_SHA_FINALIZE_ALGORITHM; /* Should not be called. */ +} + +__attribute__((weak)) +vb2_error_t vb2ex_hwcrypto_rsa_verify_digest(const struct vb2_public_key *key, + const uint8_t *sig, const uint8_t *digest) +{ + return VB2_ERROR_EX_HWCRYPTO_UNSUPPORTED; +} + diff --git a/firmware/2lib/include/2rsa.h b/firmware/2lib/include/2rsa.h index 4b1febc4..6d5ffcbe 100644 --- a/firmware/2lib/include/2rsa.h +++ b/firmware/2lib/include/2rsa.h @@ -22,6 +22,7 @@ struct vb2_public_key { const char *desc; /* Description */ uint32_t version; /* Key version */ const struct vb2_id *id; /* Key ID */ + int allow_hwcrypto; /* Is hwcrypto allowed for key */ }; /** diff --git a/firmware/2lib/include/2secdata.h b/firmware/2lib/include/2secdata.h index 425dcfff..880a2ec3 100644 --- a/firmware/2lib/include/2secdata.h +++ b/firmware/2lib/include/2secdata.h @@ -107,6 +107,18 @@ enum vb2_secdata_kernel_flags { * disallowing the user from booting into the diagnostic UI. */ VB2_SECDATA_KERNEL_FLAG_DIAGNOSTIC_UI_DISABLED = (1 << 2), + + /* + * Allow HW acceleration for RSA. + * + * RW firmware currently set this flag to enable RSA acceleration. + * Verstage will use HW implementation for RSA only when + * this flag is set. + * + * Note: this will only allow/disallow HWCRYPTO for RSA. + * Using HW for hash digest is controlled by flag in the FW preamble. + */ + VB2_SECDATA_KERNEL_FLAG_HWCRYPTO_ALLOWED = (1 << 3), }; /** @@ -204,4 +216,23 @@ int vb2_secdata_fwmp_get_flag(struct vb2_context *ctx, */ uint8_t *vb2_secdata_fwmp_get_dev_key_hash(struct vb2_context *ctx); +/* + * Helper function to check if hwcrypto for RSA is allowed + */ +static inline int vb2_hwcrypto_rsa_allowed(struct vb2_context *ctx) { + + /* disable hwcrypto in recovery mode */ + if (ctx->flags & VB2_CONTEXT_RECOVERY_MODE) + return 0; + + /* enable hwcrypto only if RW firmware set the flag */ + if (vb2_secdata_kernel_get(ctx, VB2_SECDATA_KERNEL_FLAGS) + & VB2_SECDATA_KERNEL_FLAG_HWCRYPTO_ALLOWED) + return 1; + + return 0; + +} + + #endif /* VBOOT_REFERENCE_2SECDATA_H_ */ diff --git a/firmware/lib20/misc.c b/firmware/lib20/misc.c index 4e1250c0..a2d5b230 100644 --- a/firmware/lib20/misc.c +++ b/firmware/lib20/misc.c @@ -43,6 +43,8 @@ vb2_error_t vb2_load_fw_keyblock(struct vb2_context *ctx) /* Unpack the root key */ VB2_TRY(vb2_unpack_key_buffer(&root_key, key_data, key_size)); + root_key.allow_hwcrypto = vb2_hwcrypto_rsa_allowed(ctx); + /* Load the firmware keyblock header after the root key */ kb = vb2_workbuf_alloc(&wb, sizeof(*kb)); if (!kb) @@ -147,6 +149,8 @@ vb2_error_t vb2_load_fw_preamble(struct vb2_context *ctx) VB2_TRY(vb2_unpack_key_buffer(&data_key, key_data, key_size)); + data_key.allow_hwcrypto = vb2_hwcrypto_rsa_allowed(ctx); + /* Load the firmware preamble header */ pre = vb2_workbuf_alloc(&wb, sizeof(*pre)); if (!pre) diff --git a/firmware/lib20/packed_key.c b/firmware/lib20/packed_key.c index e2e9b22b..3870288f 100644 --- a/firmware/lib20/packed_key.c +++ b/firmware/lib20/packed_key.c @@ -57,6 +57,9 @@ vb2_error_t vb2_unpack_key_buffer(struct vb2_public_key *key, key->n = buf32 + 2; key->rr = buf32 + 2 + key->arrsize; + /* disable hwcrypto for RSA by default */ + key->allow_hwcrypto = 0; + #ifdef __COVERITY__ __coverity_tainted_data_sanitize__(key); __coverity_tainted_data_sanitize__(buf); diff --git a/tests/vb20_misc_tests.c b/tests/vb20_misc_tests.c index 73fae538..fdab37e3 100644 --- a/tests/vb20_misc_tests.c +++ b/tests/vb20_misc_tests.c @@ -76,6 +76,9 @@ static void reset_common_data(enum reset_type t) vb2api_secdata_firmware_create(ctx); vb2_secdata_firmware_init(ctx); + vb2api_secdata_kernel_create(ctx); + vb2_secdata_kernel_init(ctx); + mock_read_res_fail_on_call = 0; mock_unpack_key_retval = VB2_SUCCESS; mock_verify_keyblock_retval = VB2_SUCCESS; @@ -156,10 +159,13 @@ vb2_error_t vb2_unpack_key_buffer(struct vb2_public_key *key, return mock_unpack_key_retval; } +static struct vb2_public_key last_used_key; + vb2_error_t vb2_verify_keyblock(struct vb2_keyblock *block, uint32_t size, const struct vb2_public_key *key, const struct vb2_workbuf *wb) { + memcpy(&last_used_key, key, sizeof(struct vb2_public_key)); return mock_verify_keyblock_retval; } @@ -168,6 +174,7 @@ vb2_error_t vb2_verify_fw_preamble(struct vb2_fw_preamble *preamble, const struct vb2_public_key *key, const struct vb2_workbuf *wb) { + memcpy(&last_used_key, key, sizeof(struct vb2_public_key)); return mock_verify_preamble_retval; } @@ -208,6 +215,29 @@ static void verify_keyblock_tests(void) sd->data_key_size), "workbuf used after"); + /* Test hwcrypto conditions */ + reset_common_data(FOR_KEYBLOCK); + + TEST_SUCC(vb2_load_fw_keyblock(ctx), "keyblock verify"); + TEST_EQ(last_used_key.allow_hwcrypto, 0, + "hwcrypto is forbidden by TPM flag"); + + ctx->flags |= VB2_CONTEXT_RECOVERY_MODE; + TEST_SUCC(vb2_load_fw_keyblock(ctx), "keyblock verify"); + TEST_EQ(last_used_key.allow_hwcrypto, 0, + "hwcrypto is forbidden by TPM flag on recovery mode"); + + vb2_secdata_kernel_set(ctx, VB2_SECDATA_KERNEL_FLAGS, + VB2_SECDATA_KERNEL_FLAG_HWCRYPTO_ALLOWED); + + TEST_SUCC(vb2_load_fw_keyblock(ctx), "keyblock verify"); + TEST_EQ(last_used_key.allow_hwcrypto, 0, + "hwcrypto is forbidden on recovery mode"); + + ctx->flags &= ~VB2_CONTEXT_RECOVERY_MODE; + TEST_SUCC(vb2_load_fw_keyblock(ctx), "keyblock verify"); + TEST_EQ(last_used_key.allow_hwcrypto, 1, "hwcrypto is allowed"); + /* Test failures */ reset_common_data(FOR_KEYBLOCK); sd->workbuf_used = sd->workbuf_size + VB2_WORKBUF_ALIGN - @@ -298,6 +328,31 @@ static void verify_preamble_tests(void) sd->preamble_size), "workbuf used"); + /* Test hwcrypto conditions */ + reset_common_data(FOR_PREAMBLE); + + TEST_SUCC(vb2_load_fw_preamble(ctx), "preamble good"); + TEST_EQ(last_used_key.allow_hwcrypto, 0, + "hwcrypto is forbidden by TPM flag"); + + ctx->flags |= VB2_CONTEXT_RECOVERY_MODE; + TEST_SUCC(vb2_load_fw_preamble(ctx), "preamble good"); + TEST_EQ(last_used_key.allow_hwcrypto, 0, + "hwcrypto is forbidden by TPM flag on recovery mode"); + + vb2_secdata_kernel_set(ctx, VB2_SECDATA_KERNEL_FLAGS, + VB2_SECDATA_KERNEL_FLAG_HWCRYPTO_ALLOWED); + + TEST_SUCC(vb2_load_fw_preamble(ctx), "preamble good"); + TEST_EQ(last_used_key.allow_hwcrypto, 0, + "hwcrypto is forbidden on recovery mode"); + + ctx->flags &= ~VB2_CONTEXT_RECOVERY_MODE; + TEST_SUCC(vb2_load_fw_preamble(ctx), "preamble good"); + TEST_EQ(last_used_key.allow_hwcrypto, 1, + "hwcrypto is allowed"); + + /* Expected failures */ reset_common_data(FOR_PREAMBLE); sd->data_key_size = 0; diff --git a/tests/vb2_api_tests.c b/tests/vb2_api_tests.c index 3a32d4b7..c5e45097 100644 --- a/tests/vb2_api_tests.c +++ b/tests/vb2_api_tests.c @@ -81,6 +81,7 @@ static void reset_common_data(enum reset_type t) vb2api_secdata_firmware_create(ctx); vb2api_secdata_kernel_create(ctx); + vb2_secdata_kernel_init(ctx); force_dev_mode = 0; retval_vb2_fw_init_gbb = VB2_SUCCESS; @@ -260,10 +261,13 @@ uint32_t vb2_rsa_sig_size(enum vb2_signature_algorithm sig_alg) return mock_sig_size; } +static struct vb2_public_key last_used_key; + vb2_error_t vb2_rsa_verify_digest(const struct vb2_public_key *key, uint8_t *sig, const uint8_t *digest, const struct vb2_workbuf *wb) { + memcpy(&last_used_key, key, sizeof(struct vb2_public_key)); return retval_vb2_verify_digest; } @@ -736,6 +740,28 @@ static void check_hash_tests(void) TEST_SUCC(memcmp(digest_result, &digest_value, sizeof(digest_value)), "check digest value"); + /* Test hwcrypto conditions */ + reset_common_data(FOR_CHECK_HASH); + TEST_SUCC(vb2api_check_hash(ctx), "check hash good"); + TEST_EQ(last_used_key.allow_hwcrypto, 0, + "hwcrypto is forbidden by TPM flag"); + + ctx->flags |= VB2_CONTEXT_RECOVERY_MODE; + TEST_SUCC(vb2api_check_hash(ctx), "check hash good"); + TEST_EQ(last_used_key.allow_hwcrypto, 0, + "hwcrypto is forbidden by TPM flag on recovery mode"); + + vb2_secdata_kernel_set(ctx, VB2_SECDATA_KERNEL_FLAGS, + VB2_SECDATA_KERNEL_FLAG_HWCRYPTO_ALLOWED); + + TEST_SUCC(vb2api_check_hash(ctx), "check hash good"); + TEST_EQ(last_used_key.allow_hwcrypto, 0, + "hwcrypto is forbidden on recovery mode"); + + ctx->flags &= ~VB2_CONTEXT_RECOVERY_MODE; + TEST_SUCC(vb2api_check_hash(ctx), "check hash good"); + TEST_EQ(last_used_key.allow_hwcrypto, 1, "hwcrypto is allowed"); + reset_common_data(FOR_CHECK_HASH); TEST_EQ(vb2api_check_hash_get_digest(ctx, digest_result, digest_result_size - 1), diff --git a/tests/vb2_common2_tests.c b/tests/vb2_common2_tests.c index e8c96f78..89a560c4 100644 --- a/tests/vb2_common2_tests.c +++ b/tests/vb2_common2_tests.c @@ -20,6 +20,26 @@ static const uint8_t test_data[] = "This is some test data to sign."; static const uint32_t test_size = sizeof(test_data); +static enum { + HWCRYPTO_OK, + HWCRYPTO_NOTSUPPORTED, + HWCRYPTO_ERROR, +} hwcrypto_state; + +vb2_error_t vb2ex_hwcrypto_rsa_verify_digest(const struct vb2_public_key *key, + const uint8_t *sig, const uint8_t *digest) +{ + switch (hwcrypto_state) { + case HWCRYPTO_OK: + return VB2_SUCCESS; + case HWCRYPTO_NOTSUPPORTED: + return VB2_ERROR_EX_HWCRYPTO_UNSUPPORTED; + case HWCRYPTO_ERROR: + return VB2_ERROR_RSA_VERIFY_DIGEST; + } +} + + static void test_unpack_key(const struct vb2_packed_key *key1) { struct vb2_public_key pubk; @@ -133,6 +153,32 @@ static void test_verify_data(const struct vb2_packed_key *key1, TEST_NEQ(vb2_verify_data(test_data, test_size, sig2, &pubk, &wb), 0, "vb2_verify_data() wrong sig"); + pubk.allow_hwcrypto = 1; + + hwcrypto_state = HWCRYPTO_OK; + memcpy(sig2, sig, sig_total_size); + vb2_signature_data_mutable(sig2)[0] ^= 0x5A; + TEST_EQ(vb2_verify_data(test_data, test_size, sig2, &pubk, &wb), + 0, "vb2_verify_data() hwcrypto ok"); + + hwcrypto_state = HWCRYPTO_ERROR; + memcpy(sig2, sig, sig_total_size); + TEST_NEQ(vb2_verify_data(test_data, test_size, sig2, &pubk, &wb), + 0, "vb2_verify_data() hwcrypto error"); + + hwcrypto_state = HWCRYPTO_NOTSUPPORTED; + memcpy(sig2, sig, sig_total_size); + TEST_EQ(vb2_verify_data(test_data, test_size, sig2, &pubk, &wb), + 0, "vb2_verify_data() hwcrypto fallback ok"); + + memcpy(sig2, sig, sig_total_size); + sig2->sig_size -= 16; + TEST_NEQ(vb2_verify_data(test_data, test_size, sig2, &pubk, &wb), + 0, "vb2_verify_data() hwcrypto fallback error"); + + pubk.allow_hwcrypto = 0; + + free(sig2); } -- cgit v1.2.1