Change in coreboot[master]: cbfs: Add file data hashing for CONFIG_CBFS_VERIFICATION

[Julius Werner (Code Review)]

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 
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;