From aaf394335cc4e287a1ffb6332311559b2b29c41f Mon Sep 17 00:00:00 2001 From: Joel Kitching Date: Wed, 11 Sep 2019 14:15:45 +0800 Subject: vboot: add VB2_ASSERT and VB2_DIE macros Sometimes vboot needs to make assertions to work sanely without always having to return VB2_ERROR_* values. Add VB2_ASSERT and VB2_DIE macros to deal with these cases. Convert existing VbAssert macro to use either VB2_ASSERT or TEST_* macros depending on the case. Implement testing infrastructure to check that aborts are being triggered correctly. The TEST_ASSERT macro should be used. BUG=b:124141368, chromium:1005700 TEST=make clean && make runtests BRANCH=none Change-Id: I298384ba50842a94a311df7f868f807bf2109cff Signed-off-by: Joel Kitching Cq-Depend: chromium:1813277 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1800112 Tested-by: Joel Kitching Reviewed-by: Julius Werner Commit-Queue: Joel Kitching --- firmware/2lib/2stub.c | 7 +++++++ firmware/2lib/include/2api.h | 10 ++++++++++ firmware/2lib/include/2common.h | 24 +++++++++++++++++++--- firmware/lib/include/utility.h | 9 --------- firmware/lib/tpm2_lite/tlcl.c | 10 +++++----- firmware/lib/tpm_lite/tlcl.c | 16 +++++++-------- tests/test_common.c | 33 +++++++++++++++++++++++++++++++ tests/test_common.h | 20 +++++++++++++++++++ tests/tpm_lite/tpmtest_fastenable.c | 7 +++++-- tests/tpm_lite/tpmtest_globallock.c | 7 ++++--- tests/tpm_lite/tpmtest_redefine_unowned.c | 3 ++- tests/tpm_lite/tpmtest_spaceperm.c | 5 +++-- tests/tpm_lite/tpmtest_writelimit.c | 5 ++++- tests/vb2_common_tests.c | 18 +++++++++++++++++ 14 files changed, 140 insertions(+), 34 deletions(-) diff --git a/firmware/2lib/2stub.c b/firmware/2lib/2stub.c index 0818ed8e..00c5d9b2 100644 --- a/firmware/2lib/2stub.c +++ b/firmware/2lib/2stub.c @@ -66,3 +66,10 @@ vb2_error_t vb2ex_tpm_set_mode(enum vb2_tpm_mode mode_val) fprintf(stderr, "%s: function not implemented\n", __func__); return VB2_ERROR_EX_UNIMPLEMENTED; } + +__attribute__((weak)) +void vb2ex_abort(void) +{ + /* Stub simply exits. */ + exit(1); +} diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h index 8a127317..24d85da0 100644 --- a/firmware/2lib/include/2api.h +++ b/firmware/2lib/include/2api.h @@ -751,4 +751,14 @@ vb2_error_t vb2ex_hwcrypto_digest_finalize(uint8_t *digest, */ vb2_error_t vb2ex_tpm_set_mode(enum vb2_tpm_mode mode_val); +/* + * Abort vboot flow due to a failed assertion or broken assumption. + * + * Likely due to caller misusing vboot (e.g. calling API functions + * out-of-order, filling in vb2_context fields inappropriately). + * Implementation should reboot or halt the machine, or fall back to some + * alternative boot flow. Retrying vboot is unlikely to succeed. + */ +void vb2ex_abort(void); + #endif /* VBOOT_REFERENCE_2API_H_ */ diff --git a/firmware/2lib/include/2common.h b/firmware/2lib/include/2common.h index 600dffbe..e15b59fd 100644 --- a/firmware/2lib/include/2common.h +++ b/firmware/2lib/include/2common.h @@ -37,9 +37,27 @@ struct vb2_public_key; #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0])) #endif -/* Platform-dependent debug output macros. */ -#define VB2_DEBUG(format, args...) vb2ex_printf(__func__, format, ## args) -#define VB2_DEBUG_RAW(format, args...) vb2ex_printf(NULL, format, ## args) +/* Platform-dependent debug/assert output macros. */ +#define VB2_DEBUG(format, args...) \ + vb2ex_printf(__func__, format, ## args) + +#define VB2_DEBUG_RAW(format, args...) \ + vb2ex_printf(NULL, format, ## args) + +#define VB2_ASSERT(expr) do { \ + if (!(expr)) { \ + VB2_DEBUG("assertion failed: %s at %s:%d\n", \ + #expr, __FILE__, __LINE__); \ + vb2ex_abort(); \ + for (;;); \ + } \ +} while (0) + +#define VB2_DIE(format, args...) do { \ + VB2_DEBUG(format, ## args); \ + vb2ex_abort(); \ + for (;;); \ +} while (0) /* * Define test_mockable and for mocking functions when compiled for Chrome OS diff --git a/firmware/lib/include/utility.h b/firmware/lib/include/utility.h index cf39f16a..bca49828 100644 --- a/firmware/lib/include/utility.h +++ b/firmware/lib/include/utility.h @@ -13,15 +13,6 @@ #include "2sysincludes.h" #include "vboot_api.h" -#ifdef VBOOT_DEBUG -#define VbAssert(expr) do { if (!(expr)) { \ - VB2_DEBUG("assert fail: %s at %s:%d\n", \ - #expr, __FILE__, __LINE__); \ - exit(1); }} while(0) -#else -#define VbAssert(expr) -#endif - /* * Buffer size required to hold the longest possible output of Uint64ToString() * - that is, Uint64ToString(~0, 2). diff --git a/firmware/lib/tpm2_lite/tlcl.c b/firmware/lib/tpm2_lite/tlcl.c index 37034c99..70cc3e0e 100644 --- a/firmware/lib/tpm2_lite/tlcl.c +++ b/firmware/lib/tpm2_lite/tlcl.c @@ -209,7 +209,7 @@ uint32_t TlclUndefineSpaceEx(const uint8_t* owner_auth, uint32_t rv; /* Authentication support is not implemented. */ - VbAssert(owner_auth == NULL && owner_auth_size == 0); + VB2_ASSERT(owner_auth == NULL && owner_auth_size == 0); /* get the publicInfo of index */ rv = TlclGetPermissions(index, &permissions); @@ -231,7 +231,7 @@ uint32_t TlclDefineSpaceEx(const uint8_t* owner_auth, uint32_t owner_auth_size, struct tpm2_nv_define_space_cmd define_space; /* Authentication support is not implemented. */ - VbAssert(owner_auth == NULL && owner_auth_size == 0); + VB2_ASSERT(owner_auth == NULL && owner_auth_size == 0); /* For backwards-compatibility, if no READ or WRITE permissions are set, * assume readable/writeable with empty auth value. @@ -260,7 +260,7 @@ uint32_t TlclInitNvAuthPolicy(uint32_t pcr_selection_bitmap, void* auth_policy, uint32_t* auth_policy_size) { /* Actual PCR selection isn't implemented. */ - VbAssert(pcr_selection_bitmap == 0); + VB2_ASSERT(pcr_selection_bitmap == 0); *auth_policy_size = 0; return TPM_SUCCESS; } @@ -649,8 +649,8 @@ uint32_t TlclGetVersion(uint32_t* vendor, uint64_t* firmware_version, size_t prop_len = tlcl_vendor_string_parse( prop_value, prop_string + total_size); - VbAssert(prop_len <= 4 && - total_size + prop_len <= sizeof(prop_string)); + VB2_ASSERT(prop_len <= 4 && + total_size + prop_len <= sizeof(prop_string)); total_size += prop_len; if (prop_len < 4) break; diff --git a/firmware/lib/tpm_lite/tlcl.c b/firmware/lib/tpm_lite/tlcl.c index d102f9b8..b5cb44fc 100644 --- a/firmware/lib/tpm_lite/tlcl.c +++ b/firmware/lib/tpm_lite/tlcl.c @@ -183,7 +183,7 @@ static uint32_t StartOIAPSession(struct auth_session* session, session->handle = ReadTpmUint32(&cursor); memcpy(session->nonce_even.nonce, cursor, sizeof(TPM_NONCE)); cursor += sizeof(TPM_NONCE); - VbAssert(cursor - response <= TPM_LARGE_ENOUGH_COMMAND_SIZE); + VB2_ASSERT(cursor - response <= TPM_LARGE_ENOUGH_COMMAND_SIZE); memcpy(session->shared_secret, secret, TPM_AUTH_DATA_LEN); session->valid = 1; @@ -231,7 +231,7 @@ static uint32_t StartOSAPSession( cursor += sizeof(TPM_NONCE); const uint8_t* nonce_even_osap = cursor; cursor += sizeof(TPM_NONCE); - VbAssert(cursor - response <= TPM_LARGE_ENOUGH_COMMAND_SIZE); + VB2_ASSERT(cursor - response <= TPM_LARGE_ENOUGH_COMMAND_SIZE); /* Compute shared secret */ uint8_t hmac_input[2 * sizeof(TPM_NONCE)]; @@ -544,7 +544,7 @@ uint32_t TlclInitNvAuthPolicy(uint32_t pcr_selection_bitmap, select->pcrSelect[0] = (pcr_selection_bitmap >> 0) & 0xff; select->pcrSelect[1] = (pcr_selection_bitmap >> 8) & 0xff; select->pcrSelect[2] = (pcr_selection_bitmap >> 16) & 0xff; - VbAssert((pcr_selection_bitmap & 0xff000000) == 0); + VB2_ASSERT((pcr_selection_bitmap & 0xff000000) == 0); /* Allow all localities except locality 3. Rationale: * @@ -624,7 +624,7 @@ uint32_t TlclWrite(uint32_t index, const void* data, uint32_t length) VB2_DEBUG("TPM: TlclWrite(0x%x, %d)\n", index, length); memcpy(&cmd, &tpm_nv_write_cmd, sizeof(cmd)); - VbAssert(total_length <= TPM_LARGE_ENOUGH_COMMAND_SIZE); + VB2_ASSERT(total_length <= TPM_LARGE_ENOUGH_COMMAND_SIZE); SetTpmCommandSize(cmd.buffer, total_length); ToTpmUint32(cmd.buffer + tpm_nv_write_cmd.index, index); @@ -776,7 +776,7 @@ uint32_t TlclGetPermanentFlags(TPM_PERMANENT_FLAGS* pflags) return result; FromTpmUint32(response + kTpmResponseHeaderLength, &size); /* TODO(crbug.com/379255): This fails. Find out why. - * VbAssert(size == sizeof(TPM_PERMANENT_FLAGS)); + * VB2_ASSERT(size == sizeof(TPM_PERMANENT_FLAGS)); */ memcpy(pflags, response + kTpmResponseHeaderLength + sizeof(size), @@ -795,7 +795,7 @@ uint32_t TlclGetSTClearFlags(TPM_STCLEAR_FLAGS* vflags) FromTpmUint32(response + kTpmResponseHeaderLength, &size); /* Ugly assertion, but the struct is padded up by one byte. */ /* TODO(crbug.com/379255): This fails. Find out why. - * VbAssert(size == 7 && sizeof(TPM_STCLEAR_FLAGS) - 1 == 7); + * VB2_ASSERT(size == 7 && sizeof(TPM_STCLEAR_FLAGS) - 1 == 7); */ memcpy(vflags, response + kTpmResponseHeaderLength + sizeof(size), @@ -968,7 +968,7 @@ uint32_t TlclGetOwnership(uint8_t* owned) return result; FromTpmUint32(response + kTpmResponseHeaderLength, &size); /* TODO(crbug.com/379255): This fails. Find out why. - * VbAssert(size == sizeof(*owned)); + * VB2_ASSERT(size == sizeof(*owned)); */ memcpy(owned, response + kTpmResponseHeaderLength + sizeof(size), @@ -1113,7 +1113,7 @@ uint32_t TlclIFXFieldUpgradeInfo(TPM_IFX_FIELDUPGRADEINFO* info) info->wFieldUpgradeCounter = ReadTpmUint16(&cursor); uint32_t parsed_bytes = cursor - response; - VbAssert(parsed_bytes <= TPM_LARGE_ENOUGH_COMMAND_SIZE); + VB2_ASSERT(parsed_bytes <= TPM_LARGE_ENOUGH_COMMAND_SIZE); if (parsed_bytes > kTpmResponseHeaderLength + sizeof(size) + size) { return TPM_E_INVALID_RESPONSE; } diff --git a/tests/test_common.c b/tests/test_common.c index ddd81052..02cbc165 100644 --- a/tests/test_common.c +++ b/tests/test_common.c @@ -9,10 +9,13 @@ #include #include +#include "2common.h" #include "test_common.h" /* Global test success flag. */ int gTestSuccess = 1; +int gTestAbortArmed = 0; +jmp_buf gTestJmpEnv; int test_eq(int result, int expected, const char *preamble, const char *desc, const char *comment) @@ -173,3 +176,33 @@ int test_false(int result, } return !result; } + +int test_abort(int aborted, + const char *preamble, const char *desc, const char *comment) +{ + if (aborted) { + fprintf(stderr, "%s: %s ... " COL_GREEN "PASSED\n" COL_STOP, + preamble, comment ? comment : desc); + } else { + fprintf(stderr, "%s: %s ... " COL_RED "FAILED\n" COL_STOP, + preamble, comment ? comment : desc); + fprintf(stderr, " Expected ABORT, but did not get it\n"); + gTestSuccess = 0; + } + return aborted; +} + +void vb2ex_abort(void) +{ + /* + * If expecting an abort call, jump back to TEST_ABORT macro. + * Otherwise, force exit to ensure the test fails. + */ + if (gTestAbortArmed) { + longjmp(gTestJmpEnv, 1); + } else { + fprintf(stderr, COL_RED "Unexpected ABORT encountered, " + "exiting\n" COL_STOP); + exit(1); + } +} diff --git a/tests/test_common.h b/tests/test_common.h index 8d98f5cc..17689b71 100644 --- a/tests/test_common.h +++ b/tests/test_common.h @@ -6,6 +6,7 @@ #ifndef VBOOT_REFERENCE_TEST_COMMON_H_ #define VBOOT_REFERENCE_TEST_COMMON_H_ +#include #include /* Used to get a line number as a constant string. Need to stringify it twice */ @@ -13,6 +14,8 @@ #define TOSTRING(x) STRINGIFY(x) extern int gTestSuccess; +extern int gTestAbortArmed; +extern jmp_buf gTestJmpEnv; /* Return 1 if result is equal to expected_result, else return 0. * Also update the global gTestSuccess flag if test fails. */ @@ -117,6 +120,23 @@ int test_succ(int result, #result " == 0", \ comment) +/* Return 1 if vb2ex_abort() was called, else return 0. + * Also update the global gTestSuccess flag if test fails. */ +int test_abort(int aborted, + const char *preamble, const char *desc, const char *comment); + +#define TEST_ABORT(call, comment) do { \ + gTestAbortArmed = 1; \ + int jumped = setjmp(gTestJmpEnv); \ + if (!jumped) \ + call; \ + gTestAbortArmed = 0; \ + test_abort(jumped, \ + __FILE__ ":" TOSTRING(__LINE__), \ + #call " causes abort", \ + comment); \ +} while (0) + /* ANSI Color coding sequences. * * Don't use \e as MSC does not recognize it as a valid escape sequence. diff --git a/tests/tpm_lite/tpmtest_fastenable.c b/tests/tpm_lite/tpmtest_fastenable.c index 72989c1d..a93dfd77 100644 --- a/tests/tpm_lite/tpmtest_fastenable.c +++ b/tests/tpm_lite/tpmtest_fastenable.c @@ -15,6 +15,7 @@ #include #include "host_common.h" +#include "test_common.h" #include "tlcl.h" #include "tlcl_tests.h" @@ -33,12 +34,14 @@ int main(int argc, char** argv) { TPM_CHECK(TlclForceClear()); TPM_CHECK(TlclGetFlags(&disable, &deactivated, NULL)); printf("disable is %d, deactivated is %d\n", disable, deactivated); - VbAssert(disable == 1 && deactivated == 1); + TEST_EQ(disable, 1, "after ForceClear, disable"); + TEST_EQ(deactivated, 1, "after ForceClear, deactivated"); TPM_CHECK(TlclSetEnable()); TPM_CHECK(TlclSetDeactivated(0)); TPM_CHECK(TlclGetFlags(&disable, &deactivated, NULL)); printf("disable is %d, deactivated is %d\n", disable, deactivated); - VbAssert(disable == 0 && deactivated == 0); + TEST_EQ(disable, 0, "after SetEnable, enabled"); + TEST_EQ(deactivated, 0, "after SetDeactivated(0), activated"); } printf("TEST SUCCEEDED\n"); diff --git a/tests/tpm_lite/tpmtest_globallock.c b/tests/tpm_lite/tpmtest_globallock.c index 2e728524..14f16de0 100644 --- a/tests/tpm_lite/tpmtest_globallock.c +++ b/tests/tpm_lite/tpmtest_globallock.c @@ -11,6 +11,7 @@ #include #include "host_common.h" +#include "test_common.h" #include "tlcl.h" #include "tlcl_tests.h" @@ -33,13 +34,13 @@ int main(int argc, char** argv) { TPM_EXPECT(TlclWrite(INDEX0, (uint8_t*) &x, sizeof(x)), TPM_E_AREA_LOCKED); TPM_CHECK(TlclRead(INDEX0, (uint8_t*) &x, sizeof(x))); - VbAssert(x == 0); + TEST_EQ(x, 0, "Read from INDEX0 should give 0"); // Verifies that write to index1 is still possible. x = 2; TPM_CHECK(TlclWrite(INDEX1, (uint8_t*) &x, sizeof(x))); TPM_CHECK(TlclRead(INDEX1, (uint8_t*) &x, sizeof(x))); - VbAssert(x == 2); + TEST_EQ(x, 0, "Read from INDEX1 should give 2"); // Turns off PP. TlclLockPhysicalPresence(); @@ -49,7 +50,7 @@ int main(int argc, char** argv) { TPM_EXPECT(TlclWrite(INDEX1, (uint8_t*) &x, sizeof(x)), TPM_E_BAD_PRESENCE); TPM_CHECK(TlclRead(INDEX1, (uint8_t*) &x, sizeof(x))); - VbAssert(x == 2); + TEST_EQ(x, 2, "Read from INDEX1 should give 2"); printf("TEST SUCCEEDED\n"); exit(0); } diff --git a/tests/tpm_lite/tpmtest_redefine_unowned.c b/tests/tpm_lite/tpmtest_redefine_unowned.c index 522d999f..0f89870d 100644 --- a/tests/tpm_lite/tpmtest_redefine_unowned.c +++ b/tests/tpm_lite/tpmtest_redefine_unowned.c @@ -11,6 +11,7 @@ #include #include "host_common.h" +#include "test_common.h" #include "tlcl.h" #include "tlcl_tests.h" @@ -23,7 +24,7 @@ int main(int argc, char** argv) { TPM_CHECK(TlclSelfTestFull()); TPM_CHECK(TlclAssertPhysicalPresence()); - VbAssert(!TlclIsOwned()); + TEST_FALSE(TlclIsOwned(), "TlclIsOwned should return false"); /* Ensures spaces exist. */ TPM_CHECK(TlclRead(INDEX0, (uint8_t*) &x, sizeof(x))); diff --git a/tests/tpm_lite/tpmtest_spaceperm.c b/tests/tpm_lite/tpmtest_spaceperm.c index c2608e4b..8c9232b2 100644 --- a/tests/tpm_lite/tpmtest_spaceperm.c +++ b/tests/tpm_lite/tpmtest_spaceperm.c @@ -11,6 +11,7 @@ #include #include "host_common.h" +#include "test_common.h" #include "tlcl.h" #include "tlcl_tests.h" @@ -26,10 +27,10 @@ int main(int argc, char** argv) { TPM_CHECK(TlclAssertPhysicalPresence()); TPM_CHECK(TlclGetPermissions(INDEX0, &perm)); - VbAssert((perm & PERMPPGL) == PERMPPGL); + TEST_NEQ(perm & PERMPPGL, 0, "INDEX0: PERMPPGL is not set"); TPM_CHECK(TlclGetPermissions(INDEX1, &perm)); - VbAssert((perm & PERMPP) == PERMPP); + TEST_NEQ(perm & PERMPP, 0, "INDEX1: PERMPP is not set"); printf("TEST SUCCEEDED\n"); exit(0); diff --git a/tests/tpm_lite/tpmtest_writelimit.c b/tests/tpm_lite/tpmtest_writelimit.c index cedcd831..75bae9f1 100644 --- a/tests/tpm_lite/tpmtest_writelimit.c +++ b/tests/tpm_lite/tpmtest_writelimit.c @@ -12,6 +12,7 @@ #include "2common.h" #include "host_common.h" +#include "test_common.h" #include "tlcl.h" #include "tlcl_tests.h" @@ -37,7 +38,9 @@ int main(int argc, char** argv) { sizeof(i))) != TPM_SUCCESS) { switch (result) { case TPM_E_MAXNVWRITES: - VbAssert(i >= TPM_MAX_NV_WRITES_NOOWNER); + TEST_TRUE(i >= TPM_MAX_NV_WRITES_NOOWNER, + "MAXNVWRITES should only occur after " + "MAX_NV_WRITES_NOOWNER reached"); break; default: VB2_DEBUG("unexpected error code %d (0x%x)\n", diff --git a/tests/vb2_common_tests.c b/tests/vb2_common_tests.c index 4a3025e9..2661cce9 100644 --- a/tests/vb2_common_tests.c +++ b/tests/vb2_common_tests.c @@ -274,6 +274,23 @@ static void test_helper_functions(void) } } +/* Helper for test_assert_die() below */ +static int _true_assertion_helper(void) +{ + VB2_ASSERT(2 + 2 == 4); + return 1; +} + +/** + * Test VB2_ASSERT and VB2_DIE macros + */ +static void test_assert_die(void) +{ + TEST_ABORT(VB2_DIE("die"), "DIE should abort"); + TEST_ABORT(VB2_ASSERT(2 + 2 == 5), "ASSERT false should abort"); + TEST_TRUE(_true_assertion_helper(), "ASSERT true should continue"); +} + int main(int argc, char* argv[]) { test_arithmetic(); @@ -283,6 +300,7 @@ int main(int argc, char* argv[]) test_align(); test_workbuf(); test_helper_functions(); + test_assert_die(); return gTestSuccess ? 0 : 255; } -- cgit v1.2.1