Attention is currently required from: Furquan Shaikh, Aaron Durbin. Hello Furquan Shaikh, Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/52084
to review the following change.
Change subject: cbfs: Add file data hashing for CONFIG_CBFS_VERIFICATION ......................................................................
cbfs: Add file data hashing for CONFIG_CBFS_VERIFICATION
This patch adds file data hashing for CONFIG_CBFS_VERIFICATION. With this, all CBFS accesses using the new CBFS APIs (cbfs_load/_map/_alloc and variants) will be fully verified when verification is enabled. (Note that some use of legacy APIs remains and thus the CBFS_VERIFICATION feature is not fully finished.)
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: Ic9fff279f69cf3b7c38a0dc2ff3c970eaa756aa8 --- M src/commonlib/bsd/cbfs_private.c M src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h M src/lib/cbfs.c 3 files changed, 74 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/52084/1
diff --git a/src/commonlib/bsd/cbfs_private.c b/src/commonlib/bsd/cbfs_private.c index 91f81e0..16bab7c 100644 --- a/src/commonlib/bsd/cbfs_private.c +++ b/src/commonlib/bsd/cbfs_private.c @@ -190,3 +190,26 @@
return NULL; } + +const struct vb2_hash *cbfs_file_hash(const union cbfs_mdata *mdata) +{ + /* Hashes are variable-length attributes, so need to manually check the length. */ + const struct cbfs_file_attr_hash *attr = + cbfs_find_attr(mdata, CBFS_FILE_ATTR_TAG_HASH, 0); + if (!attr) + return NULL; /* no hash */ + const size_t asize = be32toh(attr->len); + + const struct vb2_hash *hash = &attr->hash; + const size_t hsize = vb2_digest_size(hash->algo); + if (!hsize) { + ERROR("Hash algo %u for '%s' unsupported.\n", hash->algo, mdata->h.filename); + return NULL; + } + if (hsize != asize - offsetof(struct cbfs_file_attr_hash, hash.raw)) { + ERROR("Hash attribute size for '%s' (%zu) incorrect for algo %u.\n", + mdata->h.filename, asize, hash->algo); + return NULL; + } + return hash; +} diff --git a/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h b/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h index df13427..ed5c378 100644 --- a/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h +++ b/src/commonlib/bsd/include/commonlib/bsd/cbfs_mdata.h @@ -24,4 +24,7 @@ else caller is responsible for checking the |len| field to avoid reading out-of-bounds. */ const void *cbfs_find_attr(const union cbfs_mdata *mdata, uint32_t attr_tag, size_t size_check);
+/* Returns pointer to CBFS file hash structure in metadata attributes, or NULL if invalid. */ +const struct vb2_hash *cbfs_file_hash(const union cbfs_mdata *mdata); + #endif /* _COMMONLIB_BSD_CBFS_MDATA_H_ */ diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index 9135b2f..23e8f41 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -186,11 +186,27 @@ return true; }
-static size_t cbfs_load_and_decompress(const struct region_device *rdev, - void *buffer, size_t buffer_size, uint32_t compression) +static inline bool cbfs_file_hash_mismatch(const void *buffer, size_t size, + const struct vb2_hash *file_hash) +{ + /* Avoid linking hash functions when verification is disabled. */ + if (!CONFIG(CBFS_VERIFICATION)) + return false; + + /* If there is no file hash, always count that as a mismatch. */ + if (file_hash && vb2_hash_verify(buffer, size, file_hash) == VB2_SUCCESS) + return false; + + printk(BIOS_CRIT, "CBFS file hash mismatch!\n"); + return true; +} + +static size_t cbfs_load_and_decompress(const struct region_device *rdev, void *buffer, + size_t buffer_size, uint32_t compression, + const struct vb2_hash *file_hash) { size_t in_size = region_device_sz(rdev); - size_t out_size; + size_t out_size = 0; void *map;
DEBUG("Decompressing %zu bytes to %p with algo %d\n", in_size, buffer, compression); @@ -201,6 +217,8 @@ return 0; if (rdev_readat(rdev, buffer, 0, in_size) != in_size) return 0; + if (cbfs_file_hash_mismatch(buffer, in_size, file_hash)) + return 0; return in_size;
case CBFS_COMPRESS_LZ4: @@ -213,9 +231,11 @@ if (map == NULL) return 0;
- timestamp_add_now(TS_START_ULZ4F); - out_size = ulz4fn(map, in_size, buffer, buffer_size); - timestamp_add_now(TS_END_ULZ4F); + if (!cbfs_file_hash_mismatch(map, in_size, file_hash)) { + timestamp_add_now(TS_START_ULZ4F); + out_size = ulz4fn(map, in_size, buffer, buffer_size); + timestamp_add_now(TS_END_ULZ4F); + }
rdev_munmap(rdev, map);
@@ -228,10 +248,12 @@ if (map == NULL) return 0;
- /* Note: timestamp not useful for memory-mapped media (x86) */ - timestamp_add_now(TS_START_ULZMA); - out_size = ulzman(map, in_size, buffer, buffer_size); - timestamp_add_now(TS_END_ULZMA); + if (!cbfs_file_hash_mismatch(map, in_size, file_hash)) { + /* Note: timestamp not useful for memory-mapped media (x86) */ + timestamp_add_now(TS_START_ULZMA); + out_size = ulzman(map, in_size, buffer, buffer_size); + timestamp_add_now(TS_END_ULZMA); + }
rdev_munmap(rdev, map);
@@ -318,11 +340,18 @@ if (size_out) *size_out = size;
+ const struct vb2_hash *file_hash = NULL; + if (CONFIG(CBFS_VERIFICATION)) + file_hash = cbfs_file_hash(&mdata); + /* allocator == NULL means do a cbfs_map() */ if (allocator) { loc = allocator(arg, size, &mdata); } else if (compression == CBFS_COMPRESS_NONE) { - return rdev_mmap_full(&rdev); + void *mapping = rdev_mmap_full(&rdev); + if (!mapping || cbfs_file_hash_mismatch(mapping, size, file_hash)) + return NULL; + return mapping; } else if (!CBFS_CACHE_AVAILABLE) { ERROR("Cannot map compressed file %s on x86\n", mdata.h.filename); return NULL; @@ -335,7 +364,7 @@ return NULL; }
- size = cbfs_load_and_decompress(&rdev, loc, size, compression); + size = cbfs_load_and_decompress(&rdev, loc, size, compression, file_hash); if (!size) return NULL;
@@ -384,12 +413,18 @@ prog_set_entry(pstage, prog_start(pstage) + be32toh(sattr->entry_offset), NULL);
+ const struct vb2_hash *file_hash = NULL; + if (CONFIG(CBFS_VERIFICATION)) + file_hash = cbfs_file_hash(&mdata); + /* Hacky way to not load programs over read only media. The stages * that would hit this path initialize themselves. */ if ((ENV_BOOTBLOCK || ENV_SEPARATE_VERSTAGE) && !CONFIG(NO_XIP_EARLY_STAGES) && CONFIG(BOOT_DEVICE_MEMORY_MAPPED)) { void *mapping = rdev_mmap_full(&rdev); rdev_munmap(&rdev, mapping); + if (cbfs_file_hash_mismatch(mapping, region_device_sz(&rdev), file_hash)) + return CB_CBFS_HASH_MISMATCH; if (mapping == prog_start(pstage)) return CB_SUCCESS; } @@ -405,7 +440,7 @@ }
size_t fsize = cbfs_load_and_decompress(&rdev, prog_start(pstage), prog_size(pstage), - compression); + compression, file_hash); if (!fsize) return CB_ERR;