Julius Werner submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
cbfstool: Support CONFIG_CBFS_VERIFICATION and metadata 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 metadata
hash that is embedded in the bootblock code. It will automatically find
the metadata 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 <jwerner@chromium.org>
Change-Id: I61a84add8654f60c683ef213b844a11b145a5cb7
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41121
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
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, 262 insertions(+), 19 deletions(-)

diff --git a/util/cbfstool/Makefile.inc b/util/cbfstool/Makefile.inc
index 4ef224c..7a13a79 100644
--- a/util/cbfstool/Makefile.inc
+++ b/util/cbfstool/Makefile.inc
@@ -23,6 +23,7 @@
cbfsobj += xdr.o
cbfsobj += partitioned_file.o
# COMMONLIB
+cbfsobj += cbfs_private.o
cbfsobj += fsp_relocate.o
# FMAP
cbfsobj += fmap.o
@@ -94,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
@@ -199,6 +201,8 @@
$(objutil)/cbfstool/fmd_scanner.o: TOOLCFLAGS += -Wno-unused-function
# Tolerate lzma sdk warnings
$(objutil)/cbfstool/LzmaEnc.o: TOOLCFLAGS += -Wno-sign-compare -Wno-cast-qual
+# Tolerate commonlib warnings
+$(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 d7b0173..99a4761 100644
--- a/util/cbfstool/cbfs_sections.h
+++ b/util/cbfstool/cbfs_sections.h
@@ -10,6 +10,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 72c01b9..c55838b 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -14,14 +14,14 @@
#include "cbfs_sections.h"
#include "elfparsing.h"
#include "partitioned_file.h"
+#include <commonlib/bsd/cbfs_private.h>
+#include <commonlib/bsd/metadata_hash.h>
#include <commonlib/fsp.h>
#include <commonlib/endian.h>
#include <commonlib/helpers.h>
#include <commonlib/region.h>
#include <vboot_host.h>

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

+/*
+ * This "metadata_hash cache" caches the value and location of the CBFS metadata
+ * hash embedded in the bootblock when CBFS verification is enabled. The first
+ * call to get_mh_cache() searches for the cache by scanning the whole bootblock
+ * for its 8-byte signature, later calls will just return the previously found
+ * information again. If the cbfs_hash.algo member in the result is
+ * VB2_HASH_INVALID, that means no metadata hash was found and this image does
+ * not use CBFS verification.
+ */
+struct mh_cache {
+ const char *region;
+ size_t offset;
+ struct vb2_hash cbfs_hash;
+ bool initialized;
+};
+
+static struct mh_cache *get_mh_cache(void)
+{
+ static struct mh_cache mhc;
+
+ if (mhc.initialized)
+ return &mhc;
+
+ mhc.initialized = true;
+
+ const struct fmap *fmap = partitioned_file_get_fmap(param.image_file);
+ if (!fmap)
+ goto no_metadata_hash;
+
+ /* Find the bootblock. If there is a "BOOTBLOCK" FMAP section, it's
+ there. If not, it's a normal file in the primary CBFS section. */
+ 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_metadata_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_metadata_hash;
+ mhc.region = SECTION_NAME_PRIMARY_CBFS;
+ if (cbfs_image_from_buffer(&cbfs, &buffer, param.headeroffset))
+ goto no_metadata_hash;
+ bootblock = cbfs_get_entry(&cbfs, "bootblock");
+ if (!bootblock || ntohl(bootblock->type) != CBFS_TYPE_BOOTBLOCK)
+ goto no_metadata_hash;
+ offset = (void *)bootblock + ntohl(bootblock->offset) -
+ buffer_get(&cbfs.buffer);
+ size = ntohl(bootblock->len);
+ }
+
+ /* Find and validate the metadata hash anchor inside the bootblock and
+ record its exact byte offset from the start of the FMAP region. */
+ struct metadata_hash_anchor *anchor = memmem(buffer_get(&buffer) + offset,
+ size, METADATA_HASH_ANCHOR_MAGIC, sizeof(anchor->magic));
+ if (anchor) {
+ if (!vb2_digest_size(anchor->cbfs_hash.algo)) {
+ ERROR("Unknown CBFS metadata hash type: %d\n",
+ anchor->cbfs_hash.algo);
+ goto no_metadata_hash;
+ }
+ mhc.cbfs_hash = anchor->cbfs_hash;
+ mhc.offset = (void *)anchor - buffer_get(&buffer);
+ return &mhc;
+ }
+
+no_metadata_hash:
+ mhc.cbfs_hash.algo = VB2_HASH_INVALID;
+ 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 metadata_hash_anchor *anchor = buffer_get(&buffer) + mhc->offset;
+ /* The metadata hash anchor should always still be where we left it. */
+ assert(!memcmp(anchor->magic, METADATA_HASH_ANCHOR_MAGIC,
+ sizeof(anchor->magic)) &&
+ anchor->cbfs_hash.algo == mhc->cbfs_hash.algo);
+ update_and_info("CBFS metadata hash", anchor->cbfs_hash.raw,
+ mhc->cbfs_hash.raw, vb2_digest_size(anchor->cbfs_hash.algo));
+ if (fmap_hash) {
+ update_and_info("FMAP hash",
+ metadata_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;
+
+}
+
+/* This should be called after every time CBFS metadata might have changed. It
+ will recalculate and update the metadata hash in the bootblock if needed. */
+static int maybe_update_metadata_hash(struct cbfs_image *cbfs)
+{
+ if (strcmp(param.region_name, SECTION_NAME_PRIMARY_CBFS))
+ return 0; /* Metadata 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);
+}
+
+/* This should be called after every time the FMAP or the bootblock itself might
+ have changed, and will write the new FMAP hash into the metadata hash anchor
+ in the bootblock if required (usually when the bootblock is first added). */
+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,
@@ -451,13 +607,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_metadata_hash(&image);

done:
free(header);
@@ -564,6 +728,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) {
@@ -594,7 +759,7 @@
return 1;
}

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

done:
free(header);
@@ -692,18 +857,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 metadata
+ 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 metadata 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) {
@@ -767,14 +946,18 @@

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_metadata_hash(&image) || maybe_update_fmap_hash();
+
+error:
+ free(header);
+ buffer_delete(&buffer);
+ return 1;
}

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

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

static int cbfs_create(void)
@@ -1289,6 +1472,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("[METADATA 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;
}

@@ -1387,7 +1597,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)
@@ -1692,7 +1903,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"

To view, visit change 41121. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I61a84add8654f60c683ef213b844a11b145a5cb7
Gerrit-Change-Number: 41121
Gerrit-PatchSet: 30
Gerrit-Owner: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Idwer Vollering <vidwer@gmail.com>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged