summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulius Werner <jwerner@chromium.org>2020-01-23 14:52:59 -0800
committerCommit Bot <commit-bot@chromium.org>2020-01-28 02:32:08 +0000
commit0e97e25e85f0499e23b09a31a2c7116759f191d5 (patch)
treed990e6cc56eeab048a96de48cefdacc10e0c13b3
parentf57ad98c29072624bf0977ab972201595efd2b38 (diff)
downloadvboot-0e97e25e85f0499e23b09a31a2c7116759f191d5.tar.gz
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
-rw-r--r--firmware/2lib/2sha_utility.c2
-rw-r--r--firmware/2lib/include/2sha.h22
-rw-r--r--tests/vb2_sha_api_tests.c31
3 files changed, 38 insertions, 17 deletions
diff --git a/firmware/2lib/2sha_utility.c b/firmware/2lib/2sha_utility.c
index 8c6f4b80..378f4c8a 100644
--- a/firmware/2lib/2sha_utility.c
+++ b/firmware/2lib/2sha_utility.c
@@ -221,7 +221,7 @@ vb2_error_t vb2_hash_verify(const void *buf, uint32_t size,
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 32af9f74..95f85804 100644
--- a/firmware/2lib/include/2sha.h
+++ b/firmware/2lib/include/2sha.h
@@ -100,15 +100,17 @@ struct vb2_digest_context {
/*
* 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 @@ struct vb2_hash {
#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 @@ static inline vb2_error_t vb2_hash_calculate(const void *buf, uint32_t size,
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 6a23dcee..cb58f7bc 100644
--- a/tests/vb2_sha_api_tests.c
+++ b/tests/vb2_sha_api_tests.c
@@ -54,13 +54,30 @@ vb2_error_t vb2_digest_finalize(struct vb2_digest_context *dc, uint8_t *digest,
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 @@ static void vb2_hash_verify_tests(void)
{
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 @@ static void vb2_hash_verify_tests(void)
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();