vboot2: un-nest data structures

Originally, we designed the vboot data structures so that some of them
had sub-structures.  Then the variable-length data for each of the
structures was at the end.  So:

    struct vb2_keyblock {
      struct vb2_packed_key
      struct vb2_signature
    }
    // Followed by variable-length data for keyblock
    // Followed by variable-length data for packed key
    // Followed by variable-length data for signature

This had the weird side effect that the header and data for the
sub-structs were not contiguous.  That wasn't too bad before, but it
gets more complicated with the new data structures.  Each structure
now can also have a description.  And keyblocks can have a list of
signatures.

Structures also couldn't really know their own size, since a
sub-struct might have a 20-byte header, but then 2K of other data in
between that and the data for the sub-struct itself.

So, un-nest all the data structures.  That is, the keyblock now
contains the offset of the signature struct, rather than the signature
struct itself.  And then all the variable-length data for each struct
immediately follows the struct itself.  So:

    struct vb2_keyblock2 {
      // Offset of packed key
      // Offset of first signature
    }
    // Followed by variable-length data for keyblock
    struct vb2_packed_key
    // Followed by variable-length data for packed key
    struct vb2_signature2
    // Followed by variable-length data for signature (desc, sig data)

Verifying and traversing these objects is much more straightforward.
And each struct can now know its own size.

This first change rearranges the structures.  Descriptions now
immediately follow the fixed size structure headers.

The next change adds better verification of the structures, using the
fixed_size and total_size fields in the common header.

BUG=chromium:423882
BRANCH=none
TEST=VBOOT2=1 make runtests

Change-Id: Ieb9148d6f26c3e59ea542f3a95e59d8019ccee21
Reviewed-on: https://chromium-review.googlesource.com/226824
Tested-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Bill Richardson <wfrichar@chromium.org>
Commit-Queue: Randall Spangler <rspangler@chromium.org>

3 files changed, 17 insertions, 22 deletions

diff --git a/tests/vb2_common2_tests.c b/tests/vb2_common2_tests.c
index f58d2a35..c2d11d99 100644
--- a/tests/vb2_common2_tests.c
+++ b/tests/vb2_common2_tests.c
@@ -104,7 +104,7 @@ static void test_unpack_key2(const VbPublicKey *orig_key)
 	free(key2);
 
 	key2 = vb2_convert_packed_key2(key1, "Test key", &size);
-	key2->c.desc_offset += size;
+	key2->c.fixed_size += size;
 	TEST_EQ(vb2_unpack_key2(&pubk, (uint8_t *)key2, size),
 		VB2_ERROR_INSIDE_DATA_OUTSIDE,
 		"vb2_unpack_key2() buffer too small for desc");
@@ -112,7 +112,6 @@ static void test_unpack_key2(const VbPublicKey *orig_key)
 
 	key2 = vb2_convert_packed_key2(key1, "Test key", &size);
 	key2->c.desc_size = 0;
-	key2->c.desc_offset = 0;
 	TEST_SUCC(vb2_unpack_key2(&pubk, (uint8_t *)key2, size),
 		  "vb2_unpack_key2() no desc");
 	TEST_EQ(strcmp(pubk.desc, ""), 0, "  empty desc string");
diff --git a/tests/vb2_common_tests.c b/tests/vb2_common_tests.c
index 5a8cb202..31fa036d 100644
--- a/tests/vb2_common_tests.c
+++ b/tests/vb2_common_tests.c
@@ -163,9 +163,9 @@ static void test_struct_packing(void)
 	TEST_EQ(EXPECTED_VB2_FW_PREAMBLE2_SIZE,
 		sizeof(struct vb2_fw_preamble2),
 		"sizeof(vb2_fw_preamble2)");
-	TEST_EQ(EXPECTED_VB2_FW_PREAMBLE_HASH_SIZE,
-		sizeof(struct vb2_fw_preamble_hash),
-		"sizeof(vb2_fw_preamble_hash)");
+	TEST_EQ(EXPECTED_VB2_FW_PREAMBLE2_HASH_SIZE,
+		sizeof(struct vb2_fw_preamble2_hash),
+		"sizeof(vb2_fw_preamble2_hash)");
 }
 
 /**
@@ -246,13 +246,14 @@ static void test_helper_functions(void)
 		uint8_t cbuf[sizeof(struct vb2_struct_common) + 128];
 		struct vb2_struct_common *c = (struct vb2_struct_common *)cbuf;
 
-		c->desc_offset = sizeof(*c);
+		c->total_size = sizeof(cbuf);
+		c->fixed_size = sizeof(*c);
 		c->desc_size = 128;
 		cbuf[sizeof(cbuf) - 1] = 0;
 		TEST_SUCC(vb2_verify_common_header(cbuf, sizeof(cbuf), c),
 			  "CommonInside at start");
 
-		c[1].desc_offset = sizeof(*c);
+		c[1].fixed_size = sizeof(*c);
 		c[1].desc_size = 128 - sizeof(*c);
 		TEST_SUCC(vb2_verify_common_header(cbuf, sizeof(cbuf), c + 1),
 			  "CommonInside after start");
@@ -261,11 +262,11 @@ static void test_helper_functions(void)
 			VB2_ERROR_INSIDE_DATA_OUTSIDE,
 			"CommonInside key too big");
 
-		c->desc_offset = sizeof(cbuf);
+		c->fixed_size = sizeof(cbuf);
 		TEST_EQ(vb2_verify_common_header(cbuf, sizeof(cbuf), c),
 			VB2_ERROR_INSIDE_DATA_OUTSIDE,
 			"CommonInside offset too big");
-		c->desc_offset = sizeof(*c);
+		c->fixed_size = sizeof(*c);
 
 		cbuf[sizeof(cbuf) - 1] = 1;
 		TEST_EQ(vb2_verify_common_header(cbuf, sizeof(cbuf), c),
@@ -273,14 +274,8 @@ static void test_helper_functions(void)
 			"CommonInside description not terminated");
 
 		c->desc_size = 0;
-		c->desc_offset = 0;
 		TEST_SUCC(vb2_verify_common_header(cbuf, sizeof(cbuf), c),
 			  "CommonInside no description");
-
-		c->desc_offset = 4;
-		TEST_EQ(vb2_verify_common_header(cbuf, sizeof(cbuf), c),
-			VB2_ERROR_DESC_EMPTY_OFFSET,
-			"CommonInside description empty offset");
 	}
 
 	{
diff --git a/tests/vb2_convert_structs.c b/tests/vb2_convert_structs.c
index fe74f85f..cb29a979 100644
--- a/tests/vb2_convert_structs.c
+++ b/tests/vb2_convert_structs.c
@@ -24,20 +24,21 @@ struct vb2_packed_key2 *vb2_convert_packed_key2(
 	};
 	uint8_t *kbuf;
 
-	/* Calculate description size */
-	k2.c.desc_offset = sizeof(k2);
+	/* Calculate sizes and offsets */
+	k2.c.fixed_size = sizeof(k2);
 	k2.c.desc_size = roundup32(strlen(desc) + 1);
+	k2.key_offset = k2.c.fixed_size + k2.c.desc_size;
+	k2.key_size = key->key_size;
+	k2.c.total_size = k2.key_offset + k2.key_size;
 
 	/* Copy/initialize fields */
-	k2.key_offset = k2.c.desc_offset + k2.c.desc_size;
-	k2.key_size = key->key_size;
 	k2.key_version = key->key_version;
 	k2.sig_algorithm = vb2_crypto_to_signature(key->algorithm);
 	k2.hash_algorithm = vb2_crypto_to_hash(key->algorithm);
 	/* TODO: fill in a non-zero GUID */
 
 	/* Allocate the new buffer */
-	*out_size = k2.key_offset + k2.key_size;
+	*out_size = k2.c.total_size;
 	kbuf = malloc(*out_size);
 	memset(kbuf, 0, *out_size);
 
@@ -45,8 +46,8 @@ struct vb2_packed_key2 *vb2_convert_packed_key2(
 	memcpy(kbuf, &k2, sizeof(k2));
 
 	/* strcpy() is safe because we allocated above based on strlen() */
-	strcpy((char *)(kbuf + k2.c.desc_offset), desc);
-	kbuf[k2.c.desc_offset + k2.c.desc_size - 1] = 0;
+	strcpy((char *)(kbuf + k2.c.fixed_size), desc);
+	kbuf[k2.c.fixed_size + k2.c.desc_size - 1] = 0;
 
 	memcpy(kbuf + k2.key_offset,
 	       (const uint8_t *)key + key->key_offset,