Change in coreboot[master]: cbfstool: Add support for platform "fixups" when modifying bootblock - coreboot-gerrit

6 May 2020

Hello Patrick Georgi, Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/41122
to review the following change.
Change subject: cbfstool: Add support for platform "fixups" when modifying bootblock
......................................................................
cbfstool: Add support for platform "fixups" when modifying bootblock
To support the new CONFIG_CBFS_VERIFICATION feature, cbfstool needs to
update the master hash embedded in the bootblock code every time it adds
or removes a CBFS file. This can lead to problems on certain platforms
where the bootblock needs to be specially wrapped in some
platform-specific data structure so that the platform's masked ROM can
recognize it. If that data structure contains any form of hash or
signature of the bootblock code that is checked on every boot, it will
no longer match if cbfstool modifies it after the fact.
In general, we should always try to disable these kinds of features
where possible (they're not super useful anyway). But for platforms
where the hardware simply doesn't allow that, this patch introduces the
concept of "platform fixups" to cbfstool. Whenever cbfstool finds a
master hash anchor in a CBFS image, it will run all built-in "fixup
probe" functions on that bootblock to check if it can recognize it as
the wrapper format for a platform known to have such an issue. If so, it
will register a corresponding fixup function that will run whenever it
tries to write back modified data to that bootblock. The function can
then modify any platform-specific headers as necessary.
As first supported platform, this patch adds a fixup for Qualcomm
platforms (specifically the header format used by sc7180), which
recalculates the bootblock body hash originally added by
util/qualcomm/createxbl.py.
(Note that this feature is not intended to support platform-specific
signature schemes like BootGuard directly in cbfstool. For anything that
requires an actual secret key, it should be okay if the user needs to
run a platform-specific signing tool on the final CBFS image before
flashing. This feature is intended for the normal unsigned case (which
on some platforms may be implemented as signing with a well-known key)
so that on a board that is not "locked down" in any way the normal use
case of manipulating an image with cbfstool and then directly flashing
the output file stays working with CONFIG_CBFS_VERIFICATION.)
Signed-off-by: Julius Werner jwerner@chromium.org
Change-Id: I02a83a40f1d0009e6f9561ae5d2d9f37a510549a
---
M util/cbfstool/Makefile.inc
M util/cbfstool/cbfs.h
M util/cbfstool/cbfstool.c
M util/cbfstool/elf.h
A util/cbfstool/platform_fixups.c
5 files changed, 131 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/41122/1

diff --git a/util/cbfstool/Makefile.inc b/util/cbfstool/Makefile.inc
index e98b52e..1a5882f 100644
--- a/util/cbfstool/Makefile.inc
+++ b/util/cbfstool/Makefile.inc
@@ -22,6 +22,7 @@
 cbfsobj += rmodule.o
 cbfsobj += xdr.o
 cbfsobj += partitioned_file.o
+cbfsobj += platform_fixups.o
 # COMMONLIB
 cbfsobj += cbfs_private.o
 cbfsobj += fsp_relocate.o
diff --git a/util/cbfstool/cbfs.h b/util/cbfstool/cbfs.h
index 10b478c..0199d9c 100644
--- a/util/cbfstool/cbfs.h
+++ b/util/cbfstool/cbfs.h
@@ -78,4 +78,9 @@
 void xdr_get_seg(struct cbfs_payload_segment *out,
    	struct cbfs_payload_segment *in);
+/* platform_fixups.c */
+typedef int (*platform_fixup_func)(struct buffer *buffer, size_t offset);
+platform_fixup_func platform_fixups_probe(struct buffer *buffer, size_t offset,
+					  const char *region_name);
+
 #endif
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index bc07b6f..0b0fea7 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -108,6 +108,7 @@
    const char *region;
    size_t offset;
    struct vb2_hash cbfs_hash;
+	platform_fixup_func fixup;
 };
static struct mh_cache *get_mh_cache(void)
@@ -157,6 +158,8 @@
    	}
    	mhc.cbfs_hash = anchor->cbfs_hash;
    	mhc.offset = (void *)anchor - buffer_get(&buffer);
+		mhc.fixup = platform_fixups_probe(&buffer, mhc.offset,
+						  mhc.region);
    	return &mhc;
    }
@@ -198,6 +201,8 @@
    			master_hash_anchor_fmap_hash(anchor), fmap_hash,
    			vb2_digest_size(anchor->cbfs_hash.algo));
    }
+	if (mhc->fixup && mhc->fixup(&buffer, mhc->offset) != 0)
+		return -1;
    if (!partitioned_file_write_region(param.image_file, &buffer))
    	return -1;
    return 0;
diff --git a/util/cbfstool/elf.h b/util/cbfstool/elf.h
index 11cee4f..9ba55a6 100644
--- a/util/cbfstool/elf.h
+++ b/util/cbfstool/elf.h
@@ -574,6 +574,8 @@
 #define PF_W		(1 << 1)	/* Segment is writable */
 #define PF_R		(1 << 2)	/* Segment is readable */
 #define PF_MASKOS	0x0ff00000	/* OS-specific */
+#define PF_QC_SG_MASK	0x07000000	/* Qualcomm "segment" types */
+#define PF_QC_SG_HASH	0x02000000	/* Qualcomm hash segment */
 #define PF_MASKPROC	0xf0000000	/* Processor-specific */
/* Legal values for note segment descriptor types for core files. */
diff --git a/util/cbfstool/platform_fixups.c b/util/cbfstool/platform_fixups.c
new file mode 100644
index 0000000..f893a4d
--- /dev/null
+++ b/util/cbfstool/platform_fixups.c
@@ -0,0 +1,118 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include "cbfs.h"
+#include "cbfs_sections.h"
+#include "elfparsing.h"
+
+/*
+ * NOTE: This currently only implements support for MBN version 6 (as used by sc7180). Support
+ * for other MBN versions could probably be added but may require more parsing to tell them
+ * apart, and minor modifications (e.g. different hash algorithm). Add later as needed.
+ */
+static void *qualcomm_find_hash(struct buffer *in, size_t bb_offset, struct vb2_hash *real_hash)
+{
+	size_t search_end_size = MIN(0, buffer_size(in) - 32 * KiB);
+	struct buffer elf;
+	buffer_clone(&elf, in);
+
+	/* To identify a Qualcomm image, first we find the GPT header... */
+	while (buffer_size(&elf) > search_end_size &&
+	       !buffer_check_magic(&elf, "EFI PART", 8))
+		buffer_seek(&elf, 512);
+
+	/* ...then shortly afterwards there's an ELF header... */
+	while (buffer_size(&elf) > search_end_size &&
+	       !buffer_check_magic(&elf, ELFMAG, 4))
+		buffer_seek(&elf, 512);
+
+	if (buffer_size(&elf) <= search_end_size)
+		return NULL;	/* Doesn't seem to be a Qualcomm image. */
+
+	struct parsed_elf pelf;
+	if (parse_elf(&elf, &pelf, ELF_PARSE_PHDR))
+		return NULL;	/* Not an ELF -- guess not a Qualcomm MBN after all? */
+
+	/* Qualcomm stores an array of SHA-384 hashes in a special ELF segment. One special one
+	   to start with, and then one for each segment in order. */
+	void *bb_hash = NULL;
+	void *hashtable = NULL;
+	int i;
+	int bb_segment = -1;
+	for (i = 0; i < pelf.ehdr.e_phnum; i++) {
+		Elf64_Phdr *ph = &pelf.phdr[i];
+		if ((ph->p_flags & PF_QC_SG_MASK) == PF_QC_SG_HASH) {
+			if ((int)ph->p_filesz !=
+			    (pelf.ehdr.e_phnum + 1) * VB2_SHA384_DIGEST_SIZE) {
+				ERROR("fixups: Qualcomm hash segment has wrong size!\n");
+				goto destroy_elf;
+			} /* Found the table with the hashes -- store its address. */
+			hashtable = buffer_get(&elf) + ph->p_offset;
+		} else if (bb_segment < 0 && ph->p_offset + ph->p_filesz < buffer_size(&elf) &&
+			   buffer_offset(&elf) + ph->p_offset <= bb_offset &&
+			   buffer_offset(&elf) + ph->p_offset + ph->p_filesz > bb_offset) {
+			bb_segment = i;	/* Found the bootblock segment -- store its index. */
+		}
+	}
+	if (!hashtable)	/* ELF but no special QC hash segment -- guess not QC after all? */
+		goto destroy_elf;
+	if (bb_segment < 0) {	/* Can assume it's QC if we found the special segment. */
+		ERROR("fixups: Cannot find bootblock code in Qualcomm MBN!\n");
+		goto destroy_elf;
+	}
+
+	/* Pass out the actual hash of the current bootblock segment in |real_hash|. */
+	if (vb2_hash_calculate(buffer_get(&elf) + pelf.phdr[bb_segment].p_offset,
+			       pelf.phdr[bb_segment].p_filesz, VB2_HASH_SHA384, real_hash)) {
+		ERROR("fixups: vboot digest error\n");
+		goto destroy_elf;
+	} /* Return pointer to where the bootblock hash needs to go in Qualcomm's table. */
+	bb_hash = hashtable + (bb_segment + 1) * VB2_SHA384_DIGEST_SIZE;
+
+destroy_elf:
+	parsed_elf_destroy(&pelf);
+	return bb_hash;
+}
+
+static bool qualcomm_probe(struct buffer *buffer, size_t offset)
+{
+	struct vb2_hash real_hash;
+	void *table_hash = qualcomm_find_hash(buffer, offset, &real_hash);
+	if (!table_hash)
+		return false;
+
+	if (memcmp(real_hash.raw, table_hash, VB2_SHA384_DIGEST_SIZE)) {
+		ERROR("fixups: Identified Qualcomm MBN, but existing hash doesn't match!\n");
+		return false;
+	}
+
+	return true;
+}
+
+static int qualcomm_fixup(struct buffer *buffer, size_t offset)
+{
+	struct vb2_hash real_hash;
+	void *table_hash = qualcomm_find_hash(buffer, offset, &real_hash);
+	if (!table_hash) {
+		ERROR("fixups: Cannot find Qualcomm MBN headers anymore!\n");
+		return -1;
+	}
+
+	memcpy(table_hash, real_hash.raw, VB2_SHA384_DIGEST_SIZE);
+	INFO("fixups: Updated Qualcomm MBN header bootblock hash.\n");
+	return 0;
+}
+
+platform_fixup_func platform_fixups_probe(struct buffer *buffer, size_t offset,
+					  const char *region_name)
+{
+	if (!strcmp(region_name, SECTION_NAME_BOOTBLOCK)) {
+		if (qualcomm_probe(buffer, offset))
+			return qualcomm_fixup;
+	} else if (!strcmp(region_name, SECTION_NAME_PRIMARY_CBFS)) {
+		/* TODO: add fixups for primary CBFS bootblock platforms, if needed */
+	} else {
+		ERROR("%s called for unexpected FMAP region %s!\n", __func__, region_name);
+	}
+
+	return NULL;
+}
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/41122
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I02a83a40f1d0009e6f9561ae5d2d9f37a510549a
Gerrit-Change-Number: 41122
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner jwerner@chromium.org
Gerrit-Reviewer: Aaron Durbin adurbin@chromium.org
Gerrit-Reviewer: Martin Roth martinroth@google.com
Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com
Gerrit-MessageType: newchange



    