Change in coreboot[master]: cbfstool: Support CONFIG_CBFS_VERIFICATION and master hash anchor

[Julius Werner (Code Review)]

Hello Patrick Georgi, Aaron Durbin,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/41121

to review the following change.

Change subject: cbfstool: Support CONFIG_CBFS_VERIFICATION and master hash anchor
......................................................................

cbfstool: Support CONFIG_CBFS_VERIFICATION and master hash anchor

This patch adds support for the new CONFIG_CBFS_VERIFICATION feature to
cbfstool. When CBFS verification is enabled, cbfstool must automatically
add a hash attribute to every CBFS file it adds (with a handful of
exceptions like bootblock and "header" pseudofiles that are never read
by coreboot code itself). It must also automatically update the master
hash that is embedded in the bootblock code. It will automatically find
the master hash by scanning the bootblock for its magic number and use
its presence to auto-detect whether CBFS verification is enabled for an
image (and which hash algorithm to use).

Signed-off-by: Julius Werner 
Change-Id: I61a84add8654f60c683ef213b844a11b145a5cb7
---
M util/cbfstool/Makefile.inc
A util/cbfstool/cbfs_glue.h
M util/cbfstool/cbfs_sections.h
M util/cbfstool/cbfstool.c
4 files changed, 243 insertions(+), 29 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/41121/1

diff --git a/util/cbfstool/Makefile.inc b/util/cbfstool/Makefile.inc
index 000ceb2..e98b52e 100644
--- a/util/cbfstool/Makefile.inc
+++ b/util/cbfstool/Makefile.inc
@@ -23,10 +23,8 @@
 cbfsobj += xdr.o
 cbfsobj += partitioned_file.o
 # COMMONLIB
-cbfsobj += cbfs.o
+cbfsobj += cbfs_private.o
 cbfsobj += fsp_relocate.o
-cbfsobj += mem_pool.o
-cbfsobj += region.o
 # FMAP
 cbfsobj += fmap.o
 cbfsobj += kv_pair.o
@@ -72,10 +70,6 @@
 ifitobj += cbfs-mkstage.o
 ifitobj += cbfs-mkpayload.o
 ifitobj += rmodule.o
-# COMMONLIB
-ifitobj += cbfs.o
-ifitobj += mem_pool.o
-ifitobj += region.o
 # FMAP
 ifitobj += fmap.o
 ifitobj += kv_pair.o
@@ -101,6 +95,7 @@
 TOOLCPPFLAGS ?= -D_DEFAULT_SOURCE # memccpy() from string.h
 TOOLCPPFLAGS += -D_BSD_SOURCE -D_SVID_SOURCE # _DEFAULT_SOURCE for older glibc
 TOOLCPPFLAGS += -D_XOPEN_SOURCE=700 # strdup() from string.h
+TOOLCPPFLAGS += -D_GNU_SOURCE # memmem() from string.h
 TOOLCPPFLAGS += -I$(top)/util/cbfstool/flashmap
 TOOLCPPFLAGS += -I$(top)/util/cbfstool
 TOOLCPPFLAGS += -I$(objutil)/cbfstool
@@ -207,9 +202,7 @@
 # Tolerate lzma sdk warnings
 $(objutil)/cbfstool/LzmaEnc.o: TOOLCFLAGS += -Wno-sign-compare -Wno-cast-qual
 # Tolerate commonlib warnings
-$(objutil)/cbfstool/region.o: TOOLCFLAGS += -Wno-sign-compare -Wno-cast-qual
-$(objutil)/cbfstool/cbfs.o: TOOLCFLAGS += -Wno-sign-compare -Wno-cast-qual
-$(objutil)/cbfstool/mem_pool.o: TOOLCFLAGS += -Wno-sign-compare -Wno-cast-qual
+$(objutil)/cbfstool/cbfs_private.o: TOOLCFLAGS += -Wno-sign-compare
 # Tolerate lz4 warnings
 $(objutil)/cbfstool/lz4.o: TOOLCFLAGS += -Wno-missing-prototypes
 $(objutil)/cbfstool/lz4_wrapper.o: TOOLCFLAGS += -Wno-attributes
diff --git a/util/cbfstool/cbfs_glue.h b/util/cbfstool/cbfs_glue.h
new file mode 100644
index 0000000..11786be
--- /dev/null
+++ b/util/cbfstool/cbfs_glue.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _CBFS_GLUE_H_
+#define _CBFS_GLUE_H_
+
+#include "cbfs_image.h"
+
+#define CBFS_ENABLE_HASHING 1
+
+typedef const struct cbfs_image *cbfs_dev_t;
+
+static inline ssize_t cbfs_dev_read(cbfs_dev_t dev, void *buffer, size_t offset, size_t size)
+{
+	if (buffer_size(&dev->buffer) < offset ||
+	    buffer_size(&dev->buffer) - offset < size)
+		return -1;
+
+	memcpy(buffer, buffer_get(&dev->buffer) + offset, size);
+	return size;
+}
+
+static inline size_t cbfs_dev_size(cbfs_dev_t dev)
+{
+	return buffer_size(&dev->buffer);
+}
+
+#endif	/* _CBFS_GLUE_H_ */
diff --git a/util/cbfstool/cbfs_sections.h b/util/cbfstool/cbfs_sections.h
index 3526f8d..d444cce 100644
--- a/util/cbfstool/cbfs_sections.h
+++ b/util/cbfstool/cbfs_sections.h
@@ -22,6 +22,7 @@

#define SECTION_NAME_FMAP		"FMAP"
 #define SECTION_NAME_PRIMARY_CBFS	"COREBOOT"
+#define SECTION_NAME_BOOTBLOCK		"BOOTBLOCK"

#define SECTION_ANNOTATION_CBFS		"CBFS"

diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index 6015603..bc07b6f 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -31,13 +31,13 @@
 #include "cbfs_sections.h"
 #include "elfparsing.h"
 #include "partitioned_file.h"
+#include <commonlib/bsd/cbfs_private.h>
+#include <commonlib/bsd/master_hash.h>
 #include <commonlib/fsp.h>
 #include <commonlib/endian.h>
 #include <commonlib/helpers.h>
 #include <vboot_host.h>

-#define SECTION_WITH_FIT_TABLE	"BOOTBLOCK"
-
 struct command {
    const char *name;
    const char *optstring;
@@ -104,6 +104,144 @@
    .u64val = -1,
 };

+struct mh_cache {
+	const char *region;
+	size_t offset;
+	struct vb2_hash cbfs_hash;
+};
+
+static struct mh_cache *get_mh_cache(void)
+{
+	static struct mh_cache mhc;
+
+	if (mhc.region)
+		return &mhc;
+
+	const struct fmap *fmap = partitioned_file_get_fmap(param.image_file);
+	if (!fmap)
+		goto no_master_hash;
+
+	size_t offset, size;
+	struct buffer buffer;
+	if (fmap_find_area(fmap, SECTION_NAME_BOOTBLOCK)) {
+		if (!partitioned_file_read_region(&buffer, param.image_file,
+						  SECTION_NAME_BOOTBLOCK))
+			goto no_master_hash;
+		mhc.region = SECTION_NAME_BOOTBLOCK;
+		offset = 0;
+		size = buffer.size;
+	} else {
+		struct cbfs_image cbfs;
+		struct cbfs_file *bootblock;
+		if (!partitioned_file_read_region(&buffer, param.image_file,
+						  SECTION_NAME_PRIMARY_CBFS))
+			goto no_master_hash;
+		mhc.region = SECTION_NAME_PRIMARY_CBFS;
+		if (cbfs_image_from_buffer(&cbfs, &buffer, param.headeroffset))
+			goto no_master_hash;
+		bootblock = cbfs_get_entry(&cbfs, "bootblock");
+		if (!bootblock || ntohl(bootblock->type) != CBFS_TYPE_BOOTBLOCK)
+			goto no_master_hash;
+		offset = (void *)bootblock + ntohl(bootblock->offset) -
+			 buffer_get(&cbfs.buffer);
+		size = ntohl(bootblock->len);
+	}
+
+	struct master_hash_anchor *anchor = memmem(buffer_get(&buffer) + offset,
+			size, MASTER_HASH_ANCHOR_MAGIC, sizeof(anchor->magic));
+	if (anchor) {
+		if (!vb2_digest_size(anchor->cbfs_hash.algo)) {
+			ERROR("Unknown CBFS master hash type: %d\n",
+			      anchor->cbfs_hash.algo);
+			goto no_master_hash;
+		}
+		mhc.cbfs_hash = anchor->cbfs_hash;
+		mhc.offset = (void *)anchor - buffer_get(&buffer);
+		return &mhc;
+	}
+
+no_master_hash:
+	mhc.cbfs_hash.algo = VB2_HASH_INVALID;
+	mhc.region = (void *)-1;
+	return &mhc;
+}
+
+static void update_and_info(const char *name, void *dst, void *src, size_t size)
+{
+	if (!memcmp(dst, src, size))
+		return;
+	char *src_str = bintohex(src, size);
+	char *dst_str = bintohex(dst, size);
+	INFO("Updating %s from %s to %s\n", name, dst_str, src_str);
+	memcpy(dst, src, size);
+	free(src_str);
+	free(dst_str);
+}
+
+static int update_anchor(struct mh_cache *mhc, uint8_t *fmap_hash)
+{
+	struct buffer buffer;
+	if (!partitioned_file_read_region(&buffer, param.image_file,
+					  mhc->region))
+		return -1;
+	struct master_hash_anchor *anchor = buffer_get(&buffer) + mhc->offset;
+	if (memcmp(anchor->magic, MASTER_HASH_ANCHOR_MAGIC,
+		   sizeof(anchor->magic)) ||
+	    anchor->cbfs_hash.algo != mhc->cbfs_hash.algo) {
+		ERROR("Master hash anchor no longer where it used to be!\n");
+		return -1;
+	}
+	update_and_info("CBFS master hash", anchor->cbfs_hash.raw,
+		mhc->cbfs_hash.raw, vb2_digest_size(anchor->cbfs_hash.algo));
+	if (fmap_hash) {
+		update_and_info("FMAP hash",
+				master_hash_anchor_fmap_hash(anchor), fmap_hash,
+				vb2_digest_size(anchor->cbfs_hash.algo));
+	}
+	if (!partitioned_file_write_region(param.image_file, &buffer))
+		return -1;
+	return 0;
+
+}
+
+static int maybe_update_master_hash(struct cbfs_image *cbfs)
+{
+	if (strcmp(param.region_name, SECTION_NAME_PRIMARY_CBFS))
+		return 0;	/* Master hash only embedded in primary CBFS. */
+
+	struct mh_cache *mhc = get_mh_cache();
+	if (mhc->cbfs_hash.algo == VB2_HASH_INVALID)
+		return 0;
+
+	cb_err_t err = cbfs_walk(cbfs, NULL, NULL, &mhc->cbfs_hash,
+				 CBFS_WALK_WRITEBACK_HASH);
+	if (err != CB_CBFS_NOT_FOUND) {
+		ERROR("Unexpected cbfs_walk() error %d\n", err);
+		return -1;
+	}
+
+	return update_anchor(mhc, NULL);
+}
+
+static int maybe_update_fmap_hash(void)
+{
+	if (strcmp(param.region_name, SECTION_NAME_BOOTBLOCK) &&
+	    strcmp(param.region_name, SECTION_NAME_FMAP) &&
+	    param.type != CBFS_TYPE_BOOTBLOCK)
+		return 0;	/* FMAP and bootblock didn't change. */
+
+	struct mh_cache *mhc = get_mh_cache();
+	if (mhc->cbfs_hash.algo == VB2_HASH_INVALID)
+		return 0;
+
+	uint8_t fmap_hash[VB2_MAX_DIGEST_SIZE];
+	const struct fmap *fmap = partitioned_file_get_fmap(param.image_file);
+	if (!fmap || vb2_digest_buffer((const void *)fmap, fmap_size(fmap),
+			mhc->cbfs_hash.algo, fmap_hash, sizeof(fmap_hash)))
+		return -1;
+	return update_anchor(mhc, fmap_hash);
+}
+
 static bool region_is_flashmap(const char *region)
 {
    return partitioned_file_region_check_magic(param.image_file, region,
@@ -253,13 +391,21 @@

header = cbfs_create_file_header(CBFS_TYPE_RAW,
    	buffer.size, name);
+
+	enum vb2_hash_algorithm algo = get_mh_cache()->cbfs_hash.algo;
+	if (algo != VB2_HASH_INVALID)
+		if (cbfs_add_file_hash(header, &buffer, algo)) {
+			ERROR("couldn't add hash for '%s'\n", name);
+			goto done;
+		}
+
    if (cbfs_add_entry(&image, &buffer, offset, header, 0) != 0) {
    	ERROR("Failed to add %llu into ROM image as '%s'.\n",
    				(long long unsigned)u64val, name);
    	goto done;
    }

-	ret = 0;
+	ret = maybe_update_master_hash(&image);

done:
    free(header);
@@ -366,6 +512,7 @@
    h->offset = htonl(offset);
    h->architecture = htonl(CBFS_ARCHITECTURE_UNKNOWN);

+	/* Never add a hash attribute to the master header. */
    header = cbfs_create_file_header(CBFS_TYPE_CBFSHEADER,
    	buffer_size(&buffer), name);
    if (cbfs_add_entry(&image, &buffer, 0, header, 0) != 0) {
@@ -396,7 +543,7 @@
    		return 1;
    }

-	ret = 0;
+	ret = maybe_update_master_hash(&image);

done:
    free(header);
@@ -497,18 +644,32 @@

if (convert && convert(&buffer, &offset, header) != 0) {
    	ERROR("Failed to parse file '%s'.\n", filename);
-		buffer_delete(&buffer);
-		return 1;
+		goto error;
    }

-	if (param.hash != VB2_HASH_INVALID)
-		if (cbfs_add_file_hash(header, &buffer, param.hash) == -1) {
-			ERROR("couldn't add hash for '%s'\n", name);
-			free(header);
-			buffer_delete(&buffer);
-			return 1;
+	/* Bootblock and CBFS header should never have file hashes. When adding
+	   the bootblock it is important that we *don't* look up the master hash
+	   yet (before it is added) or we'll cache an outdated result. */
+	if (type != CBFS_TYPE_BOOTBLOCK && type != CBFS_TYPE_CBFSHEADER) {
+		enum vb2_hash_algorithm mh_algo = get_mh_cache()->cbfs_hash.algo;
+		if (mh_algo != VB2_HASH_INVALID && param.hash != mh_algo) {
+			if (param.hash == VB2_HASH_INVALID) {
+				param.hash = mh_algo;
+			} else {
+				ERROR("Cannot specify hash %s that's different from master hash algorithm %s\n",
+				      vb2_get_hash_algorithm_name(param.hash),
+				      vb2_get_hash_algorithm_name(mh_algo));
+				goto error;
+			}
    	}

+		if (param.hash != VB2_HASH_INVALID &&
+		    cbfs_add_file_hash(header, &buffer, param.hash) == -1) {
+			ERROR("couldn't add hash for '%s'\n", name);
+			goto error;
+		}
+	}
+
    if (param.autogen_attr) {
    	/* Add position attribute if assigned */
    	if (param.baseaddress_assigned || param.stage_xip) {
@@ -573,14 +734,18 @@
    							-offset);
    if (cbfs_add_entry(&image, &buffer, offset, header, len_align) != 0) {
    	ERROR("Failed to add '%s' into ROM image.\n", filename);
-		free(header);
-		buffer_delete(&buffer);
-		return 1;
+		goto error;
    }

free(header);
    buffer_delete(&buffer);
-	return 0;
+
+	return maybe_update_master_hash(&image) || maybe_update_fmap_hash();
+
+error:
+	free(header);
+	buffer_delete(&buffer);
+	return 1;
 }

static int cbfstool_convert_raw(struct buffer *buffer,
@@ -925,7 +1090,7 @@
    	return 1;
    }

-	return 0;
+	return maybe_update_master_hash(&image);
 }

static int cbfs_create(void)
@@ -1096,6 +1261,33 @@
    	cbfs_print_directory(&image);
    }

+	if (verbose) {
+		struct mh_cache *mhc = get_mh_cache();
+		if (mhc->cbfs_hash.algo == VB2_HASH_INVALID)
+			return 0;
+
+		struct vb2_hash real_hash = { .algo = mhc->cbfs_hash.algo };
+		cb_err_t err = cbfs_walk(&image, NULL, NULL, &real_hash,
+					 CBFS_WALK_WRITEBACK_HASH);
+		if (err != CB_CBFS_NOT_FOUND) {
+			ERROR("Unexpected cbfs_walk() error %d\n", err);
+			return 1;
+		}
+		char *hash_str = bintohex(real_hash.raw,
+				vb2_digest_size(real_hash.algo));
+		printf("[MASTER HASH]\t%s:%s",
+		       vb2_get_hash_algorithm_name(real_hash.algo), hash_str);
+		if (!strcmp(param.region_name, SECTION_NAME_PRIMARY_CBFS)) {
+			if (!memcmp(mhc->cbfs_hash.raw, real_hash.raw,
+				    vb2_digest_size(real_hash.algo)))
+				printf(":valid");
+			else
+				printf(":invalid");
+		}
+		printf("\n");
+		free(hash_str);
+	}
+
    return 0;
 }

@@ -1194,7 +1386,8 @@
    memcpy(param.image_region->data + offset, new_content.data,
    						new_content.size);
    buffer_delete(&new_content);
-	return 0;
+
+	return maybe_update_fmap_hash();
 }

static int cbfs_read(void)
@@ -1454,7 +1647,7 @@
    		"Find a place for a file of that size\n"
         " layout [-w]                                                 "
    		"List mutable (or, with -w, readable) image regions\n"
-	     " print [-r image,regions]                                    "
+	     " print [-r image,regions] [-k]                               "
    		"Show the contents of the ROM\n"
         " extract [-r image,regions] [-m ARCH] -n NAME -f FILE [-U]   "
    		"Extracts a file from ROM\n"