Julius Werner would like Patrick Georgi and Aaron Durbin to review this change.

View Change

cbfs: Add verification for RO CBFS master hash

This patch adds the first stage of the new CONFIG_CBFS_VERIFICATION
feature. It's not useful to end-users in this stage so it cannot be
selected in menuconfig (and should not be used other than for
development) yet. With this patch coreboot can verify the master hash of
the RO CBFS when it starts booting, but it does not verify individual
files yet. Likewise, verifying RW CBFSes with vboot is not yet
supported.

Verification is bootstrapped from a "master hash anchor" structure that
is embedded in the bootblock code and marked by a unique magic number.
This anchor contains both the CBFS master hash and a separate hash for
the FMAP which is required to find the primary CBFS. Both are verified
on first use in the bootblock (and halt the system on failure).

The CONFIG_TOCTOU_SAFETY option is also added for illustrative purposes
to show some paths that need to be different when full protection
against TOCTOU (time-of-check vs. time-of-use) attacks is desired. For
normal verification it is sufficient to check the FMAP and the CBFS
master hash only once in the bootblock -- for TOCTOU verification we do
the same, but we need to be extra careful that we do not re-read the
FMAP or any CBFS metadata in later stages. This is mostly achieved by
depending on the CBFS metadata cache and FMAP cache features, but we
allow for one edge case in case the RW CBFS metadata cache overflows
(which may happen during an RW update and could otherwise no longer be
fixed because mcache size is defined by RO code). This code is added to
demonstrate design intent but won't really matter until RW CBFS
verification can be supported.

Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I8930434de55eb938b042fdada9aa90218c0b5a34
---
A src/commonlib/bsd/include/commonlib/bsd/master_hash.h
M src/commonlib/include/commonlib/cbmem_id.h
M src/include/cbfs.h
M src/include/cbfs_glue.h
A src/include/master_hash.h
A src/lib/Kconfig.cbfs_verification
M src/lib/Makefile.inc
M src/lib/cbfs.c
M src/lib/fmap.c
A src/lib/master_hash.c
M src/lib/program.ld
M src/security/Kconfig
M src/security/vboot/vboot_loader.c
13 files changed, 218 insertions(+), 28 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/41120/1
diff --git a/src/commonlib/bsd/include/commonlib/bsd/master_hash.h b/src/commonlib/bsd/include/commonlib/bsd/master_hash.h
new file mode 100644
index 0000000..f8bad6c
--- /dev/null
+++ b/src/commonlib/bsd/include/commonlib/bsd/master_hash.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0-only */
+
+#ifndef _COMMONLIB_BSD_MASTER_HASH_H_
+#define _COMMONLIB_BSD_MASTER_HASH_H_
+
+#include <stdint.h>
+#include <vb2_sha.h>
+
+/* This structure is embedded somewhere in the (uncompressed) bootblock. */
+struct master_hash_anchor {
+ uint8_t magic[8];
+ struct vb2_hash cbfs_hash;
+ /* NOTE: This is just reserving space. The FMAP hash is placed right after the CBFS
+ hash above, which may be shorter than the compile-time size of struct vb2_hash! */
+ uint8_t reserved_space_for_fmap_hash[VB2_MAX_DIGEST_SIZE];
+} __packed;
+
+/* Always use this function to figure out the actual location of the FMAP hash. It always uses
+ the same algorithm as the CBFS hash. */
+static inline uint8_t *master_hash_anchor_fmap_hash(struct master_hash_anchor *anchor)
+{
+ return anchor->cbfs_hash.raw + vb2_digest_size(anchor->cbfs_hash.algo);
+}
+
+/* Must *not* appear anywhere else in coreboot code (other than the anchor). */
+#define MASTER_HASH_ANCHOR_MAGIC "\xadMstHsh\x15"
+
+#endif /* _COMMONLIB_BSD_MASTER_HASH_H_ */
diff --git a/src/commonlib/include/commonlib/cbmem_id.h b/src/commonlib/include/commonlib/cbmem_id.h
index 7505fb2..710fd3b 100644
--- a/src/commonlib/include/commonlib/cbmem_id.h
+++ b/src/commonlib/include/commonlib/cbmem_id.h
@@ -25,6 +25,7 @@
#define CBMEM_ID_IGD_OPREGION 0x4f444749
#define CBMEM_ID_IMD_ROOT 0xff4017ff
#define CBMEM_ID_IMD_SMALL 0x53a11439
+#define CBMEM_ID_MASTER_HASH 0x6873484D
#define CBMEM_ID_MEMINFO 0x494D454D
#define CBMEM_ID_MMA_DATA 0x4D4D4144
#define CBMEM_ID_MMC_STATUS 0x4d4d4353
diff --git a/src/include/cbfs.h b/src/include/cbfs.h
index 66cbe21..da3b80a 100644
--- a/src/include/cbfs.h
+++ b/src/include/cbfs.h
@@ -7,6 +7,7 @@
#include <commonlib/cbfs.h>
#include <program_loading.h>
#include <types.h>
+#include <vb2_sha.h>

/***********************************************
* Perform CBFS operations on the boot device. *
@@ -65,4 +66,13 @@
*/
const struct cbfs_boot_device *cbfs_get_boot_device(bool force_ro);

+/*
+ * Builds the mcache (if |cbd->mcache| is set) and verifies the master hash (if
+ * |master_hash| is not NULL). If CB_CBFS_CACHE_FULL is returned, the mcache is
+ * incomplete but still valid and the master hash was still verified. Should be
+ * called once per *boot* (not once per stage) before the first CBFS access.
+ */
+cb_err_t cbfs_init_boot_device(const struct cbfs_boot_device *cbd,
+ struct vb2_hash *master_hash);
+
#endif
diff --git a/src/include/cbfs_glue.h b/src/include/cbfs_glue.h
index 7c40714..7836e72 100644
--- a/src/include/cbfs_glue.h
+++ b/src/include/cbfs_glue.h
@@ -6,7 +6,7 @@
#include <commonlib/region.h>
#include <console/console.h>

-#define CBFS_ENABLE_HASHING 0
+#define CBFS_ENABLE_HASHING CONFIG(CBFS_VERIFICATION)

#define ERROR(...) printk(BIOS_ERR, "CBFS ERROR: " __VA_ARGS__)
#define LOG(...) printk(BIOS_ERR, "CBFS: " __VA_ARGS__)
diff --git a/src/include/master_hash.h b/src/include/master_hash.h
new file mode 100644
index 0000000..2ce1521
--- /dev/null
+++ b/src/include/master_hash.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+#ifndef _MASTER_HASH_H_
+#define _MASTER_HASH_H_
+
+#include <commonlib/bsd/master_hash.h>
+
+vb2_error_t master_hash_verify_fmap(const void *fmap_base, size_t fmap_size);
+
+#if CONFIG(CBFS_VERIFICATION)
+struct vb2_hash *master_hash_get(void);
+#else
+static inline struct vb2_hash *master_hash_get(void) { return NULL; }
+#endif
+
+#endif
diff --git a/src/lib/Kconfig.cbfs_verification b/src/lib/Kconfig.cbfs_verification
new file mode 100644
index 0000000..c11f1af
--- /dev/null
+++ b/src/lib/Kconfig.cbfs_verification
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0-or-later
+#
+# This file is part of the coreboot project.
+#
+# This file is sourced from src/security/Kconfig for menuconfig convenience.
+
+#menu "CBFS verification" # TODO: enable once it works
+
+config CBFS_VERIFICATION
+ bool # TODO: make user selectable once it works
+ depends on !COMPRESS_BOOTBLOCK # TODO: figure out decompressor anchor
+ help
+ Work in progress. Do not use (yet).
+
+config TOCTOU_SAFETY
+ bool
+ depends on CBFS_VERIFICATION
+ depends on !NO_FMAP_CACHE
+ depends on !NO_CBFS_MCACHE
+ help
+ Work in progress. Not actually TOCTOU safe yet. Do not use.
+
+ Design idea here is that mcache overflows in this mode are only legal
+ for the RW CBFS, because it's relatively easy to retrieve the RW
+ master hash from persistent vboot context at any time, but the RO
+ master hash is lost after the bootblock is unloaded. This avoids the
+ need to carry yet another piece forward through the stages. Mcache
+ overflows are mostly a concern for RW updates (if an update adds more
+ files than originally planned for), for the RO section it should
+ always be possible to dimension the mcache correctly beforehand, so
+ this should be an acceptable limitation.
+
+config CBFS_HASH_ALGO
+ int
+ default 1 if CBFS_HASH_SHA1
+ default 2 if CBFS_HASH_SHA256
+ default 3 if CBFS_HASH_SHA512
+
+choice
+ prompt "--> hash type"
+ depends on CBFS_VERIFICATION
+ default CBFS_HASH_SHA256
+
+config CBFS_HASH_SHA1
+ bool "SHA-1"
+
+config CBFS_HASH_SHA256
+ bool "SHA-256"
+
+config CBFS_HASH_SHA512
+ bool "SHA-512"
+
+endchoice
+
+#endmenu
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index 085f6b2..a031c01a 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -37,6 +37,7 @@
bootblock-y += cbfs.c
bootblock-$(CONFIG_GENERIC_GPIO_LIB) += gpio.c
bootblock-y += libgcc.c
+bootblock-$(CONFIG_CBFS_VERIFICATION) += master_hash.c
bootblock-$(CONFIG_GENERIC_UDELAY) += timer.c

bootblock-$(CONFIG_COLLECT_TIMESTAMPS) += timestamp.c
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
index 201e705..992d104 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -11,6 +11,7 @@
#include <endian.h>
#include <fmap.h>
#include <lib.h>
+#include <master_hash.h>
#include <security/tpm/tspi/crtm.h>
#include <security/vboot/vboot_common.h>
#include <stdlib.h>
@@ -30,8 +31,20 @@
if (!CONFIG(NO_CBFS_MCACHE))
err = cbfs_mcache_lookup(cbd->mcache, cbd->mcache_size,
name, mdata, &data_offset);
- if (err == CB_CBFS_CACHE_FULL)
- err = cbfs_lookup(&cbd->rdev, name, mdata, &data_offset, NULL);
+ if (err == CB_CBFS_CACHE_FULL) {
+ struct vb2_hash *master_hash = NULL;
+ if (CONFIG(TOCTOU_SAFETY)) {
+ if (cbd != vboot_get_cbfs_boot_device()) {
+ printk(BIOS_ERR,
+ "ERROR: RO mcache overflow for %s breaks TOCTOU safety!\n",
+ name);
+ return CB_CBFS_CACHE_FULL;
+ }
+ /* TODO: set master_hash to RW master hash here. */
+ }
+ err = cbfs_lookup(&cbd->rdev, name, mdata, &data_offset,
+ master_hash);
+ }

if (CONFIG(VBOOT_ENABLE_CBFS_FALLBACK) && !force_ro &&
err == CB_CBFS_NOT_FOUND) {
@@ -308,6 +321,26 @@
return 0;
}

+cb_err_t cbfs_init_boot_device(const struct cbfs_boot_device *cbd,
+ struct vb2_hash *master_hash)
+{
+ /* If we have an mcache, mcache_build() will also check master hash. */
+ if (!CONFIG(NO_CBFS_MCACHE) && cbd->mcache_size > 0)
+ return cbfs_mcache_build(&cbd->rdev, cbd->mcache,
+ cbd->mcache_size, master_hash);
+
+ /* No mcache and no verification means we have nothing special to do. */
+ if (!CONFIG(CBFS_VERIFICATION) || !master_hash)
+ return CB_SUCCESS;
+
+ /* Verification only: use cbfs_walk() without a walker() function to
+ just run through the CBFS once, will return NOT_FOUND by default. */
+ cb_err_t err = cbfs_walk(&cbd->rdev, NULL, NULL, master_hash, 0);
+ if (err == CB_CBFS_NOT_FOUND)
+ err = CB_SUCCESS;
+ return err;
+}
+
const struct cbfs_boot_device *cbfs_get_boot_device(bool force_ro)
{
static struct cbfs_boot_device ro;
@@ -328,7 +361,7 @@
return &ro;

if (fmap_locate_area_as_rdev("COREBOOT", &ro.rdev))
- return NULL;
+ die("Cannot locate primary CBFS");

if (!CONFIG(NO_CBFS_MCACHE)) {
const struct cbmem_entry *entry;
@@ -341,12 +374,14 @@
ro.mcache_size = REGION_SIZE(cbfs_mcache) *
(100 - CONFIG_CBFS_MCACHE_RW_PERCENTAGE) / 100;
}
- if (ENV_BOOTBLOCK) {
- cb_err_t err = cbfs_mcache_build(&ro.rdev, ro.mcache,
- ro.mcache_size, NULL);
- if (err && err != CB_CBFS_CACHE_FULL)
- die("Failed to build RO mcache");
- }
+ }
+
+ if (ENV_BOOTBLOCK) {
+ cb_err_t err = cbfs_init_boot_device(&ro, master_hash_get());
+ if (err == CB_CBFS_HASH_MISMATCH)
+ die("RO CBFS master hash verification failure");
+ else if (err && err != CB_CBFS_CACHE_FULL)
+ die("RO CBFS initialization error: %d", err);
}

return &ro;
diff --git a/src/lib/fmap.c b/src/lib/fmap.c
index ecd23f6..197ad0e 100644
--- a/src/lib/fmap.c
+++ b/src/lib/fmap.c
@@ -5,6 +5,7 @@
#include <cbmem.h>
#include <console/console.h>
#include <fmap.h>
+#include <master_hash.h>
#include <stddef.h>
#include <string.h>
#include <symbols.h>
@@ -28,9 +29,20 @@
return FMAP_OFFSET;
}

-static int check_signature(const struct fmap *fmap)
+static int verify_fmap(const struct fmap *fmap)
{
- return memcmp(fmap->signature, FMAP_SIGNATURE, sizeof(fmap->signature));
+ if (memcmp(fmap->signature, FMAP_SIGNATURE, sizeof(fmap->signature)))
+ return -1;
+
+ static bool done = false;
+ if (!CONFIG(CBFS_VERIFICATION) || !ENV_BOOTBLOCK || done)
+ return 0; /* Only need to check hash in first stage. */
+
+ if (master_hash_verify_fmap(fmap, FMAP_SIZE) != VB2_SUCCESS)
+ return -1;
+
+ done = true;
+ return 0;
}

static void report(const struct fmap *fmap)
@@ -61,10 +73,12 @@
/* NOTE: This assumes that for all platforms running this code,
the bootblock is the first stage and the bootblock will make
at least one FMAP access (usually from finding CBFS). */
- if (!check_signature(fmap))
+ if (!verify_fmap(fmap))
goto register_cache;

printk(BIOS_ERR, "ERROR: FMAP cache corrupted?!\n");
+ if (CONFIG(TOCTOU_SAFETY))
+ die("TOCTOU safety relies on FMAP cache");
}

/* In case we fail below, make sure the cache is invalid. */
@@ -78,7 +92,7 @@
/* memlayout statically guarantees that the FMAP_CACHE is big enough. */
if (rdev_readat(boot_rdev, fmap, FMAP_OFFSET, FMAP_SIZE) != FMAP_SIZE)
return;
- if (check_signature(fmap))
+ if (verify_fmap(fmap))
return;
report(fmap);

@@ -109,8 +123,9 @@
if (fmap == NULL)
return -1;

- if (check_signature(fmap)) {
- printk(BIOS_DEBUG, "No FMAP found at %zx offset.\n", offset);
+ if (verify_fmap(fmap)) {
+ printk(BIOS_ERR, "FMAP missing or corrupted at offset 0x%zx!\n",
+ offset);
rdev_munmap(boot, fmap);
return -1;
}
diff --git a/src/lib/master_hash.c b/src/lib/master_hash.c
new file mode 100644
index 0000000..a4b5943
--- /dev/null
+++ b/src/lib/master_hash.c
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* This file is part of the coreboot project. */
+
+#include <assert.h>
+#include <cbmem.h>
+#include <master_hash.h>
+#include <symbols.h>
+
+__attribute__((used, section(".master_hash_anchor")))
+struct master_hash_anchor master_hash_anchor = {
+ .magic = MASTER_HASH_ANCHOR_MAGIC,
+ .cbfs_hash = { .algo = CONFIG_CBFS_HASH_ALGO }
+};
+
+struct vb2_hash *master_hash_get(void)
+{
+ return &master_hash_anchor.cbfs_hash;
+}
+
+vb2_error_t master_hash_verify_fmap(const void *fmap_buffer, size_t fmap_size)
+{
+ struct vb2_hash hash = { .algo = master_hash_anchor.cbfs_hash.algo };
+ memcpy(hash.raw, master_hash_anchor_fmap_hash(&master_hash_anchor),
+ vb2_digest_size(hash.algo));
+ return vb2_hash_verify(fmap_buffer, fmap_size, &hash);
+}
diff --git a/src/lib/program.ld b/src/lib/program.ld
index 6f096dc..cb753e8 100644
--- a/src/lib/program.ld
+++ b/src/lib/program.ld
@@ -28,6 +28,7 @@
CONFIG(ARCH_BOOTBLOCK_X86_64))
KEEP(*(.id));
#endif
+ KEEP(*(.master_hash_anchor));
*(.text);
*(.text.*);

diff --git a/src/security/Kconfig b/src/security/Kconfig
index 65d2def..8987369 100644
--- a/src/security/Kconfig
+++ b/src/security/Kconfig
@@ -11,6 +11,10 @@
## GNU General Public License for more details.
##

+# These features are implemented in src/lib/cbfs.c, but they are security
+# features so sort them in here for menuconfig.
+source "src/lib/Kconfig.cbfs_verification"
+
source "src/security/vboot/Kconfig"
source "src/security/tpm/Kconfig"
source "src/security/memory/Kconfig"
diff --git a/src/security/vboot/vboot_loader.c b/src/security/vboot/vboot_loader.c
index e6ac5cf..f3b1c7c 100644
--- a/src/security/vboot/vboot_loader.c
+++ b/src/security/vboot/vboot_loader.c
@@ -25,18 +25,17 @@

int vboot_executed;

-static void build_rw_mcache(void)
+static void after_verstage(void)
{
- if (CONFIG(NO_CBFS_MCACHE))
- return;
+ vboot_executed = 1; /* Mark verstage execution complete. */

const struct cbfs_boot_device *cbd = vboot_get_cbfs_boot_device();
- if (!cbd) /* Don't build RW mcache in recovery mode. */
+ if (!cbd) /* Can't initialize RW CBFS in recovery mode. */
return;
- cb_err_t err = cbfs_mcache_build(&cbd->rdev, cbd->mcache,
- cbd->mcache_size, NULL);
- if (err && err != CB_CBFS_CACHE_FULL)
- die("Failed to build RW mcache."); /* TODO: -> recovery? */
+
+ cb_err_t err = cbfs_init_boot_device(cbd, NULL); /* TODO: RW hash */
+ if (err && err != CB_CBFS_CACHE_FULL) /* TODO: -> recovery? */
+ die("RW CBFS initialization failure: %d", err);
}

void vboot_run_logic(void)
@@ -44,8 +43,7 @@
if (verification_should_run()) {
/* Note: this path is not used for VBOOT_RETURN_FROM_VERSTAGE */
verstage_main();
- vboot_executed = 1;
- build_rw_mcache();
+ after_verstage();
} else if (verstage_should_load()) {
struct cbfsf file;
struct prog verstage =
@@ -72,8 +70,7 @@
if (!CONFIG(VBOOT_RETURN_FROM_VERSTAGE))
return;

- vboot_executed = 1;
- build_rw_mcache();
+ after_verstage();
}
}


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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8930434de55eb938b042fdada9aa90218c0b5a34
Gerrit-Change-Number: 41120
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