2lib: Fix struct vb2_hash the way it was meant to be

My goal in CL:1963614 was to write struct vb2_hash such that it can
match the exisiting binary representation of the CBFS hash attribute,
but no longer be dependent on endianness. Unfortunately I screwed up...
if you want to match the binary representation of a big-endian integer
for small numbers, the important byte you're interested in is the *last*
one, not the first.

Thankfully we still have time to fix the issue before this struct is
really used anywhere, so this patch does that and adds a test to double
check I got it right this time.

Also clarify comments about how vboot is allowed to use this struct a
bit to match the indended usage I'm planning in coreboot. In doing that
I realized that you actually don't want to make it easy to sizeof() the
|bytes| portion of the struct (because functions shouldn't rely on that
size anyway, they should only touch what's valid for a given hash
algorithm), so taking that out which also makes it a little more
comfortable to work with the struct.

BRANCH=none
BUG=none
TEST=make runtests

Change-Id: I7e1a19f36d75acb69e5d1bfa79700c9d878f9703
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2019952
diff --git a/firmware/2lib/2sha_utility.c b/firmware/2lib/2sha_utility.c
index 8c6f4b8..378f4c8 100644
--- a/firmware/2lib/2sha_utility.c
+++ b/firmware/2lib/2sha_utility.c
@@ -221,7 +221,7 @@
 					   hash_buf, hash_size);
 	if (rv)
 		return rv;
-	if (memcmp(hash_buf, hash->bytes.raw, hash_size))
+	if (memcmp(hash_buf, hash->raw, hash_size))
 		return VB2_ERROR_SHA_MISMATCH;
 	else
 		return VB2_SUCCESS;
diff --git a/firmware/2lib/include/2sha.h b/firmware/2lib/include/2sha.h
index 32af9f7..95f8580 100644
--- a/firmware/2lib/include/2sha.h
+++ b/firmware/2lib/include/2sha.h
@@ -100,15 +100,17 @@
 /*
  * Serializable data structure that can store any vboot hash. Layout used in
  * CBFS attributes that need to be backwards-compatible -- do not change!
- * When serializing/deserizaling this, you should store/load (offsetof(bytes) +
- * vb2_digest_size(algo)), not the full size of this structure.
+ * When serializing/deserizaling this, you should store/load (offsetof(raw) +
+ * vb2_digest_size(algo)), not the full size of this structure. vboot functions
+ * taking a pointer to this should only access the |raw| array up to
+ * vb2_digest_size(algo) and not assume that the whole structure is accessible.
  */
 struct vb2_hash {
-	/* enum vb2_hash_algorithm. Fixed width for serialization.
-	   Single byte to avoid endianness issues. */
-	uint8_t algo;
-	/* Padding to align and to match existing CBFS attribute. */
+	/* Padding to match existing 4-byte big-endian from CBFS.
+	   Could be reused for other stuff later (e.g. flags or something). */
 	uint8_t reserved[3];
+	/* enum vb2_hash_algorithm. Single byte to avoid endianness issues. */
+	uint8_t algo;
 	/* The actual digest. Can add new types here as required. */
 	union {
 		uint8_t raw[0];
@@ -121,10 +123,10 @@
 #if VB2_SUPPORT_SHA512
 		uint8_t sha512[VB2_SHA512_DIGEST_SIZE];
 #endif
-	} bytes;  /* This has a name so that it's easy to sizeof(). */
+	};
 };
-_Static_assert(sizeof(((struct vb2_hash *)0)->bytes) <= VB2_MAX_DIGEST_SIZE,
-	       "Must update VB2_MAX_DIGEST_SIZE for new digests!");
+_Static_assert(sizeof(struct vb2_hash) - offsetof(struct vb2_hash, raw)
+	<= VB2_MAX_DIGEST_SIZE, "Update VB2_MAX_DIGEST_SIZE for new digests!");
 _Static_assert(VB2_HASH_ALG_COUNT <= UINT8_MAX, "vb2_hash.algo overflow!");
 
 /**
@@ -270,7 +272,7 @@
 					     struct vb2_hash *hash)
 {
 	hash->algo = algo;
-	return vb2_digest_buffer(buf, size, algo, hash->bytes.raw,
+	return vb2_digest_buffer(buf, size, algo, hash->raw,
 				 vb2_digest_size(algo));
 }
 
diff --git a/tests/vb2_sha_api_tests.c b/tests/vb2_sha_api_tests.c
index 6a23dce..cb58f7b 100644
--- a/tests/vb2_sha_api_tests.c
+++ b/tests/vb2_sha_api_tests.c
@@ -54,13 +54,30 @@
 	return mock_finalize_rv;
 }
 
+static void vb2_hash_cbfs_compatibility_test(void)
+{
+	/* 'algo' used to be represented as a 4-byte big-endian in CBFS. Confirm
+	   that the new representation is binary compatible for small values. */
+	union {
+		struct vb2_hash hash;
+		struct {
+			uint32_t be32;
+			uint8_t bytes[0];
+		};
+	} test = {0};
+
+	test.be32 = htobe32(0xa5);
+	TEST_EQ(test.hash.algo, 0xa5, "vb2_hash algo compatible to CBFS attr");
+	TEST_PTR_EQ(&test.hash.raw, &test.bytes, "  digest offset matches");
+}
+
 static void vb2_hash_calculate_tests(void)
 {
 	reset_common_data();
 	TEST_SUCC(vb2_hash_calculate(&mock_buffer, sizeof(mock_buffer),
 				     VB2_HASH_SHA1, &mock_hash),
 		  "hash_calculate success");
-	TEST_SUCC(memcmp(mock_hash.bytes.sha1, mock_sha1, sizeof(mock_sha1)),
+	TEST_SUCC(memcmp(mock_hash.sha1, mock_sha1, sizeof(mock_sha1)),
 		  "  got the right hash");
 	TEST_EQ(mock_hash.algo, VB2_HASH_SHA1, "  set algo correctly");
 
@@ -87,19 +104,19 @@
 {
 	reset_common_data();
 
-	memcpy(mock_hash.bytes.sha1, mock_sha1, sizeof(mock_sha1));
+	memcpy(mock_hash.sha1, mock_sha1, sizeof(mock_sha1));
 	mock_hash.algo = VB2_HASH_SHA1;
 	TEST_SUCC(vb2_hash_verify(mock_buffer, sizeof(mock_buffer),
 				  &mock_hash), "hash_verify success");
 
-	memcpy(mock_hash.bytes.sha1, mock_sha1, sizeof(mock_sha1));
+	memcpy(mock_hash.sha1, mock_sha1, sizeof(mock_sha1));
 	mock_hash.algo = VB2_HASH_SHA256;
 	TEST_EQ(vb2_hash_verify(mock_buffer, sizeof(mock_buffer),
 				&mock_hash), VB2_ERROR_MOCK,
 		"hash_verify wrong algo");
 
-	memcpy(mock_hash.bytes.sha1, mock_sha1, sizeof(mock_sha1));
-	mock_hash.bytes.sha1[5] = 0xfe;
+	memcpy(mock_hash.sha1, mock_sha1, sizeof(mock_sha1));
+	mock_hash.sha1[5] = 0xfe;
 	mock_hash.algo = VB2_HASH_SHA1;
 	TEST_EQ(vb2_hash_verify(mock_buffer, sizeof(mock_buffer),
 				&mock_hash), VB2_ERROR_SHA_MISMATCH,
@@ -108,9 +125,11 @@
 
 int main(int argc, char *argv[])
 {
-	TEST_EQ(sizeof(mock_hash.bytes), VB2_SHA512_DIGEST_SIZE,
+	TEST_EQ(sizeof(mock_hash),
+		offsetof(struct vb2_hash, raw) + VB2_SHA512_DIGEST_SIZE,
 		"tests run with all SHA algorithms enabled");
 
+	vb2_hash_cbfs_compatibility_test();
 	vb2_hash_calculate_tests();
 	vb2_hash_verify_tests();