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 <kitching@google.com>
Cq-Depend: chromium:1813277
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1800112
Tested-by: Joel Kitching <kitching@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Commit-Queue: Joel Kitching <kitching@chromium.org>

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 <stdio.h>
 #include <string.h>
 
+#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 <setjmp.h>
 #include <stdio.h>
 
 /* 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 <stdio.h>
 
 #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 <stdlib.h>
 
 #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 <stdlib.h>
 
 #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 <stdlib.h>
 
 #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;
 }