coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: src/drivers/intel/fsp2_0: Added FSP_S component to VBOOT Stage Verifi...
by Julius Werner (Code Review) 15 Nov '21

15 Nov '21

Julius Werner has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/32155 ) Change subject: src/drivers/intel/fsp2_0: Added FSP_S component to VBOOT Stage Verification. FSP_S component will be verified in RAMSTAGE. ...................................................................... Abandoned This direction of development was abandoned and instead the CONFIG_CBFS_VERIFICATION effort is intended to solve this use case. See CB:32159 for original discussion. -- To view, visit https://review.coreboot.org/c/coreboot/+/32155 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifefad96b54388143fecb56f0402c3b627ae6350d Gerrit-Change-Number: 32155 Gerrit-PatchSet: 2 Gerrit-Owner: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-Reviewer: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: abandon

1 0

Change in ...coreboot[master]: src/security/vboot: Added logic to also verify FSP_S component ...
by Amol N Sukerkar (Code Review) 15 Nov '21

15 Nov '21

Amol N Sukerkar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32156 Change subject: src/security/vboot: Added logic to also verify FSP_S component and syntax change for verify_stage_if_required. ...................................................................... src/security/vboot: Added logic to also verify FSP_S component and syntax change for verify_stage_if_required. When VBOOT Stage Verification is enabled, FSP_S component needs to be verified. This logic has been added. TEST=Create a coreboot.rom image by enabling CONFIG_VBOOT and CONFIG_VBOOT_STAGE_VERIFICATION. Verify that the image boots to authenticated payload and graphics is displayed via HDMI and Display Port. Change-Id: I7e2323086ecddc5195d8b55b47cc71f599b5a0b8 Signed-off-by: Sukerkar, Amol N <amol.n.sukerkar(a)intel.com> --- M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M src/security/vboot/vboot_logic_ex.c 3 files changed, 57 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/32156/1 diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index e7bd3f9..036c553 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -372,6 +372,11 @@ default "$(VBOOT_SOURCE)/tests/devkeys/rhash.vbprik2" depends on VBOOT_STAGE_VERIFICATION +config VBOOT_2_1_FSPS_HASH_KEY + string "Coreboot fsps.bin Stage Hashing Key(private)" + default "$(VBOOT_SOURCE)/tests/devkeys/fhash.vbprik2" + depends on VBOOT_STAGE_VERIFICATION + config VBOOT_2_1_PAYLOAD_HASH_KEY string "Coreboot PAYLOAD Stage Hashing Key(private)" default "$(VBOOT_SOURCE)/tests/devkeys/phash.vbprik2" diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc index 1a6ca9f..1c03c78 100644 --- a/src/security/vboot/Makefile.inc +++ b/src/security/vboot/Makefile.inc @@ -338,9 +338,9 @@ rm -f $@.tmp $@.tmp.size ifeq ($(CONFIG_VBOOT_STAGE_VERIFICATION), y) -# Extract RAMStage and PAYLOAD and hash them -# Since we are booting from FW_MAIN_A, RAMStage and payload are -# extracted from FW_MAIN_A. +# Extract RAMStage, fsps.bin and PAYLOAD and hash them +# Since we are booting from FW_MAIN_A, RAMStage, fsps.bin +# and payload are extracted from FW_MAIN_A. $(obj)/ramstage.hash: $(obj)/coreboot.rom @printf " CREATE RAMSTAGE HASH\n" $(CBFSTOOL) $< print -r FW_MAIN_A > $<.tmp @@ -351,6 +351,16 @@ $@.tmp $@ rm -f $<.tmp $@.tmp +$(obj)/fsps.hash: $(obj)/coreboot.rom + @printf " CREATE FSPS.BIN HASH\n" + $(CBFSTOOL) $< extract -n $(call strip_quotes,$(CONFIG_FSP_S_CBFS)) \ + -r FW_MAIN_A -f $@.tmp &> /dev/null + $(FUTILITY) --vb21 sign \ + --type rwsig \ + --prikey "$(CONFIG_VBOOT_2_1_FSPS_HASH_KEY)" \ + $@.tmp $@ + rm -f $<.tmp $@.tmp + $(obj)/payload.hash: $(obj)/coreboot.rom @printf " CREATE PAYLOAD HASH\n" $(CBFSTOOL) $< print -r FW_MAIN_A > $<.tmp @@ -369,7 +379,8 @@ --flags $(CONFIG_VBOOT_KEYBLOCK_PREAMBLE_FLAGS) $(obj)/VBLOCK_%.bin: $(obj)/FW_MAIN_%.bin $(FUTILITY) \ - $(obj)/ramstage.hash $(obj)/payload.hash $(obj)/firmware.kb21 + $(obj)/ramstage.hash $(obj)/fsps.hash $(obj)/payload.hash \ + $(obj)/firmware.kb21 $(FUTILITY) vbutil_firmware \ --vblock21 $@ \ --keyblock "$(top)/$(obj)/firmware.kb21" \ diff --git a/src/security/vboot/vboot_logic_ex.c b/src/security/vboot/vboot_logic_ex.c index 1b526c7..0a071f0 100644 --- a/src/security/vboot/vboot_logic_ex.c +++ b/src/security/vboot/vboot_logic_ex.c @@ -154,6 +154,32 @@ } } +/* get the hash id from component name */ +static void get_hash_id(struct vb2_id *id, const char *name) +{ + /* in POSTCAR stage, safely assume that we are + * in the process of verifying RAMSTAGE */ + if (ENV_POSTCAR) { + const struct vb2_id tmp_id = VB2_ID_RAMSTAGE; + memcpy(id, &tmp_id, sizeof(*id)); + } else if (ENV_RAMSTAGE) { + /* In RAMSTAGE, we verify FSPS and PAYLOAD, + * conditionally, so, get the appropriate ID */ + if (!memcmp(name, CONFIG_CBFS_PREFIX"/payload", + sizeof(name))) { + const struct vb2_id tmp_id = VB2_ID_PAYLOAD; + memcpy(id, &tmp_id, sizeof(*id)); + } else if (!memcmp(name, CONFIG_FSP_S_CBFS, + sizeof(name))) { + const struct vb2_id tmp_id = VB2_ID_FSPS; + memcpy(id, &tmp_id, sizeof(*id)); + } + else + die("Invalid component"); + } else + die("Invalid stage"); +} + /* VB2 context initialization helper function */ static void init_ctx(struct vb2_context *ctx) { @@ -197,11 +223,11 @@ int rv; uint32_t size = 0; - printk(BIOS_INFO, "Phase 4\n"); + printk(BIOS_INFO, "Phase 4\n"); /* init hash */ rv = vb21api_init_hash(ctx, id, &size); - if (rv) + if (rv) return rv; /* extend hash over the body */ @@ -237,28 +263,23 @@ vboot_set_selected_region(region_device_region(&fw_main)); } -/* Veify the stage to be executed */ -static void verify_stage(const struct region_device *rdev) +/* Verify the stage to be executed */ +static void verify_stage(const struct region_device *rdev, + const char *name) { struct vb2_context ctx; struct region_device fw_main; int rv; size_t fsize = 0; void *map = NULL; - const struct vb2_id* id; + struct vb2_id id; /* get region memory map */ fsize = region_device_sz(rdev); map = rdev_mmap(rdev, 0, fsize); if (!map) die("ERROR: Stage Mapping failed"); - /* get the hash id */ - if (ENV_POSTCAR) - id = vb2_hash_id(VB2_HASH_SHA256); - else if (ENV_RAMSTAGE) - id = vb2_hash_id(VB2_HASH_SHA512); - else - die("Invalid hash id"); + get_hash_id(&id, name); /* initialize the vb context and read the NV data */ init_ctx(&ctx); @@ -277,7 +298,7 @@ } /* verify the hash */ - rv = verify_hash(&ctx, map, id); + rv = verify_hash(&ctx, map, &id); if (rv) { printk(BIOS_ERR, "ERROR:0x%x ", rv); die("Stage Verification Failed"); @@ -289,13 +310,14 @@ } /* stage verification if required */ -void verify_stage_if_required(const struct region_device *rdev) +void verify_stage_if_required(const struct region_device *rdev, + const char *name) { if (!rdev) { die("Invalid region device"); } else { if (ENV_POSTCAR || ENV_RAMSTAGE) - verify_stage(rdev); + verify_stage(rdev, name); } } -- To view, visit https://review.coreboot.org/c/coreboot/+/32156 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7e2323086ecddc5195d8b55b47cc71f599b5a0b8 Gerrit-Change-Number: 32156 Gerrit-PatchSet: 1 Gerrit-Owner: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

3 3

Change in ...coreboot[master]: src/security/vboot: Enabled support for dsdt.aml
by Amol N Sukerkar (Code Review) 15 Nov '21

15 Nov '21

Amol N Sukerkar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32157 Change subject: src/security/vboot: Enabled support for dsdt.aml ...................................................................... src/security/vboot: Enabled support for dsdt.aml This change enables vboot Stage Verification support for dsdt.aml ACPI table. BRANCH=none TEST=Create a coreboot.rom image by enabling CONFIG_VBOOT_STAGE_VERIFICATION and CONFIG_VBOOT. Verify that the image boots to authenticated payload and graphics is displayed via HDMI and Display Port. Change-Id: I51a627bc0622da64ce4486f27912753497822ce0 Signed-off-by: Sukerkar, Amol N <amol.n.sukerkar(a)intel.com> --- M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M src/security/vboot/vboot_logic_ex.c 3 files changed, 36 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/32157/1 diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index 036c553..5e60ee7 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -373,10 +373,20 @@ depends on VBOOT_STAGE_VERIFICATION config VBOOT_2_1_FSPS_HASH_KEY - string "Coreboot fsps.bin Stage Hashing Key(private)" + string "Coreboot fsps.bin Hashing Key(private)" default "$(VBOOT_SOURCE)/tests/devkeys/fhash.vbprik2" depends on VBOOT_STAGE_VERIFICATION +config VBOOT_2_1_DSDT_AML_HASH_KEY + string "Coreboot dsdt.aml Hashing Key(private)" + default "$(VBOOT_SOURCE)/tests/devkeys/dhash.vbprik2" + depends on VBOOT_STAGE_VERIFICATION + +config VBOOT_2_1_TPM2_AML_HASH_KEY + string "Coreboot tpm2.aml Hashing Key(private)" + default "$(VBOOT_SOURCE)/tests/devkeys/thash.vbprik2" + depends on VBOOT_STAGE_VERIFICATION + config VBOOT_2_1_PAYLOAD_HASH_KEY string "Coreboot PAYLOAD Stage Hashing Key(private)" default "$(VBOOT_SOURCE)/tests/devkeys/phash.vbprik2" diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc index 1c03c78..b0700a3 100644 --- a/src/security/vboot/Makefile.inc +++ b/src/security/vboot/Makefile.inc @@ -354,13 +354,23 @@ $(obj)/fsps.hash: $(obj)/coreboot.rom @printf " CREATE FSPS.BIN HASH\n" $(CBFSTOOL) $< extract -n $(call strip_quotes,$(CONFIG_FSP_S_CBFS)) \ - -r FW_MAIN_A -f $@.tmp &> /dev/null + -r FW_MAIN_A -f $@.tmp $(FUTILITY) --vb21 sign \ --type rwsig \ --prikey "$(CONFIG_VBOOT_2_1_FSPS_HASH_KEY)" \ $@.tmp $@ rm -f $<.tmp $@.tmp +$(obj)/dsdt.hash: $(obj)/coreboot.rom + @printf " CREATE DSDT.AML HASH\n" + $(CBFSTOOL) $< extract -n $(CONFIG_CBFS_PREFIX)/dsdt.aml \ + -r FW_MAIN_A -f $@.tmp + $(FUTILITY) --vb21 sign \ + --type rwsig \ + --prikey "$(CONFIG_VBOOT_2_1_DSDT_AML_HASH_KEY)" \ + $@.tmp $@ + rm -f $<.tmp $@.tmp + $(obj)/payload.hash: $(obj)/coreboot.rom @printf " CREATE PAYLOAD HASH\n" $(CBFSTOOL) $< print -r FW_MAIN_A > $<.tmp @@ -379,7 +389,8 @@ --flags $(CONFIG_VBOOT_KEYBLOCK_PREAMBLE_FLAGS) $(obj)/VBLOCK_%.bin: $(obj)/FW_MAIN_%.bin $(FUTILITY) \ - $(obj)/ramstage.hash $(obj)/fsps.hash $(obj)/payload.hash \ + $(obj)/ramstage.hash $(obj)/fsps.hash $(obj)/dsdt.hash \ + $(obj)/payload.hash \ $(obj)/firmware.kb21 $(FUTILITY) vbutil_firmware \ --vblock21 $@ \ diff --git a/src/security/vboot/vboot_logic_ex.c b/src/security/vboot/vboot_logic_ex.c index 0a071f0..000d99e 100644 --- a/src/security/vboot/vboot_logic_ex.c +++ b/src/security/vboot/vboot_logic_ex.c @@ -13,7 +13,6 @@ * GNU General Public License for more details. */ -#include <security/tpm/antirollback.h> #include <arch/exception.h> #include <assert.h> #include <bootmode.h> @@ -30,6 +29,8 @@ #include <cbmem.h> #include <rmodule.h> +#include "antirollback.h" + /* For individual stage verification design, only * RW Region A is relevant. So, force CBFS to bot * from RW Region A */ @@ -60,16 +61,6 @@ return; } -int vb2ex_tpm_clear_owner(struct vb2_context *ctx) -{ - uint32_t rv; - printk(BIOS_INFO, "Clearing TPM owner\n"); - rv = tpm_clear_and_reenable(); - if (rv) - return VB2_ERROR_EX_TPM_CLEAR_OWNER; - return VB2_SUCCESS; -} - int vb2ex_read_resource(struct vb2_context *ctx, enum vb2_resource_index index, uint32_t offset, @@ -163,18 +154,22 @@ const struct vb2_id tmp_id = VB2_ID_RAMSTAGE; memcpy(id, &tmp_id, sizeof(*id)); } else if (ENV_RAMSTAGE) { - /* In RAMSTAGE, we verify FSPS and PAYLOAD, - * conditionally, so, get the appropriate ID */ + /* In RAMSTAGE, we verify FSPS, dsdt.aml, tpm2.aml + * and PAYLOAD, conditionally, so, get the appropriate ID + */ if (!memcmp(name, CONFIG_CBFS_PREFIX"/payload", - sizeof(name))) { + sizeof(CONFIG_CBFS_PREFIX"/payload"))) { const struct vb2_id tmp_id = VB2_ID_PAYLOAD; memcpy(id, &tmp_id, sizeof(*id)); } else if (!memcmp(name, CONFIG_FSP_S_CBFS, - sizeof(name))) { + sizeof(CONFIG_FSP_S_CBFS))) { const struct vb2_id tmp_id = VB2_ID_FSPS; memcpy(id, &tmp_id, sizeof(*id)); - } - else + } else if (!memcmp(name, CONFIG_CBFS_PREFIX"/dsdt.aml", + sizeof(CONFIG_CBFS_PREFIX"/dsdt.aml"))) { + const struct vb2_id tmp_id = VB2_ID_DSDT_AML; + memcpy(id, &tmp_id, sizeof(*id)); + } else die("Invalid component"); } else die("Invalid stage"); -- To view, visit https://review.coreboot.org/c/coreboot/+/32157 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I51a627bc0622da64ce4486f27912753497822ce0 Gerrit-Change-Number: 32157 Gerrit-PatchSet: 1 Gerrit-Owner: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

3 3

Change in ...coreboot[master]: src/arch/x86: Enabled support for dsdt.aml
by Amol N Sukerkar (Code Review) 15 Nov '21

15 Nov '21

Amol N Sukerkar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32158 Change subject: src/arch/x86: Enabled support for dsdt.aml ...................................................................... src/arch/x86: Enabled support for dsdt.aml This change enables vboot Stage Verification support for dsdt.aml ACPI table. BRANCH=none TEST=Create a coreboot.rom image by enabling CONFIG_VBOOT_STAGE_VERIFICATION and CONFIG_VBOOT. Verify that the image boots to authenticated payload and graphics is displayed via HDMI and Display Port. Change-Id: I971ed8ed28cdb6355d342793ae6bb3d754939c21 Signed-off-by: Sukerkar, Amol N <amol.n.sukerkar(a)intel.com> --- M src/arch/x86/acpi.c 1 file changed, 33 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/32158/1 diff --git a/src/arch/x86/acpi.c b/src/arch/x86/acpi.c index f08a401..d129261 100644 --- a/src/arch/x86/acpi.c +++ b/src/arch/x86/acpi.c @@ -9,6 +9,7 @@ * Copyright (C) 2015 Timothy Pearson <tpearson(a)raptorengineeringinc.com>, * Raptor Engineering * Copyright (C) 2016-2019 Siemens AG + * Copyright (C) 2019 Intel Corp. * * ACPI FADT, FACS, and DSDT table support added by * Nick Barker <nick.barker9(a)btinternet.com>, and those portions @@ -1110,6 +1111,34 @@ return 0; } +/* This is marked as weak so some verification mechanism can + * use it to verify after loading into DRAM. Primarily + * overriden by VBOOT mechanism. + */ +void __weak verify_stage_if_required(const struct region_device *rdev, + const char * name) +{ + /* no op */ +} + +/* This is wrapper for acpi table to be verified, if required */ +static void verify_acpi_table(const char *name) +{ + const struct region_device *rdev = NULL; + struct prog file = PROG_INIT(PROG_UNKNOWN, name); + + if (prog_locate(&file)) { + printk (BIOS_ERR, "ERROR: Unable to locate %s\n", prog_name(&file)); + return; + } + + rdev = &file.rdev; + + /* verify if required */ + verify_stage_if_required(rdev, prog_name(&file)); +} + + unsigned long write_acpi_tables(unsigned long start) { unsigned long current; @@ -1228,6 +1257,10 @@ current = acpi_align_current(current); acpi_create_fadt(fadt, facs, dsdt); + + /* verify DSDT ACPI table */ + verify_acpi_table(CONFIG_CBFS_PREFIX "/dsdt.aml"); + acpi_add_table(rsdp, fadt); if (slic_file) { -- To view, visit https://review.coreboot.org/c/coreboot/+/32158 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I971ed8ed28cdb6355d342793ae6bb3d754939c21 Gerrit-Change-Number: 32158 Gerrit-PatchSet: 1 Gerrit-Owner: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-MessageType: newchange

3 6

Change in ...coreboot[master]: Documentation/security/vboot: Add logic to verify stage/blob using VB...
by Amol N Sukerkar (Code Review) 15 Nov '21

15 Nov '21

Amol N Sukerkar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32159 Change subject: Documentation/security/vboot: Add logic to verify stage/blob using VBOOT 2.1 library ...................................................................... Documentation/security/vboot: Add logic to verify stage/blob using VBOOT 2.1 library Added documentation to explain the logic that makes use of VBOOT 2.1 library to verify Coreboot stages/blobs. Signed-off-by: Sukerkar, Amol N <amol.n.sukerkar(a)intel.com> Change-Id: I1eb174bb4f4d84eb8f6befdce18421b6b85ccc02 --- M Documentation/security/index.md A Documentation/security/vboot/flash_partition.png A Documentation/security/vboot/vboot_21_logic.png A Documentation/security/vboot/vboot_flow_20.png A Documentation/security/vboot/vboot_flow_21.png A Documentation/security/vboot/verified_boot_21.md 6 files changed, 115 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/32159/1 diff --git a/Documentation/security/index.md b/Documentation/security/index.md index 89db42e..aebfc82 100644 --- a/Documentation/security/index.md +++ b/Documentation/security/index.md @@ -5,3 +5,4 @@ ## Vendor - [Measured Boot](vboot/measured_boot.md) +- [Verified Boot with VBOOT 2.1](vboot/verified_boot_21.md) diff --git a/Documentation/security/vboot/flash_partition.png b/Documentation/security/vboot/flash_partition.png new file mode 100644 index 0000000..91e459b --- /dev/null +++ b/Documentation/security/vboot/flash_partition.png Binary files differ diff --git a/Documentation/security/vboot/vboot_21_logic.png b/Documentation/security/vboot/vboot_21_logic.png new file mode 100644 index 0000000..d32e99e --- /dev/null +++ b/Documentation/security/vboot/vboot_21_logic.png Binary files differ diff --git a/Documentation/security/vboot/vboot_flow_20.png b/Documentation/security/vboot/vboot_flow_20.png new file mode 100644 index 0000000..fce4c5d --- /dev/null +++ b/Documentation/security/vboot/vboot_flow_20.png Binary files differ diff --git a/Documentation/security/vboot/vboot_flow_21.png b/Documentation/security/vboot/vboot_flow_21.png new file mode 100644 index 0000000..24859fe --- /dev/null +++ b/Documentation/security/vboot/vboot_flow_21.png Binary files differ diff --git a/Documentation/security/vboot/verified_boot_21.md b/Documentation/security/vboot/verified_boot_21.md new file mode 100644 index 0000000..15253b3 --- /dev/null +++ b/Documentation/security/vboot/verified_boot_21.md @@ -0,0 +1,114 @@ +# Enabling Intel BootGuard Support in Coreboot + +## Introduction + +One of the primary and key requirement for Intel customers is to enable Secure +Boot in their platform where root of trust resides in the hardware and is +immutable. While the obvious choice when it comes to hardware root of trust is +Intel Bootguard for IA platforms, the mechanism that extends the chain +of trust from hardware to bootloader, to payload/OS loader and eventually to the +OS is currently not implemented in Coreboot mainly due to licensing constraints. +This document describes the mechanism implemented in Coreboot using Google VBOOT +libraries version 2.1 that makes use of Intel BootGuard technology as Root of +Trust and extends the chain of trust to Coreboot which in turn extends it to the +payload. Note: UEFI payload will use secure boot mechanism to verify and launch +OS but that is beyond the scope of this document and will not be covered here. +More details about VBOOT support in Coreboot are available at +https://www.coreboot.org/git-docs/Intel/vboot.html. + + +## Intel BootGuard Technology + +Intel BootGuard is a platform boot integrity +protection technology. It allows initial stage of bootloader to be verified (and +measured in TPM) by a piece of firmware (ACM) which itself is verified by Intel +CPU microcode. A high level summary of the steps required to enable Intel +BootGuard are: 1. Use Intel FSP-T with Coreboot bootloader. It contains the +logic of correctly handling BtGuard enabled state. 2. Integrate ACM +(Authenticated Code module) binary in bootloader image. 3. Generate BtGuard Key +Manifest(BtG KM) and BtGuard Boot PolicyManifest(BtG BPM) and embed them in +bootloader image. a. BtG KM contains the hash of the key used for signing BtG +BPM. BtG KM is signed by the key whose hash is embedded in field-programmable +fuses. b. BtG BPM contains the hash of initial stage of boot loader. It also +stores other policies related to Intel TXT, BtG DMA protection etc. 4. Add +entries for CPU microcode patch, ACM, BtG KM and BtG BPM in FIT table. 5. +Update BootGuard related field-programmable fuses on the test platform. + + +## VBOOT in Coreboot + +The current implementation of VBOOT tool support and logic +in Coreboot is at version 2.0. The architecture, design and usage of this +feature has been described here, +https://www.coreboot.org/git-docs/Intel/vboot.html. + +### VBOOT 2.0 in Coreboot (Currently available) + +As described in the location mentioned above, the VBOOT 2.0 verified boot logic +flow works as follows. Upon boot, verstage attempts to verify the read-write +section A. It gets the public root key from GBB area and verifies the VBLOCK +area in read-write section A. If the verification is successful, then verstage +instructs Coreboot to boot rest of the firmware in read-write section A +(romstage, postcar, ramstage and payload). If the verification fails, then, +VBOOT falls back on read-only area to boot. +The flow chart below shows this flow: + +**VBOOT 2.0 Verification Flow in Coreboot** +![VBOOT_20_Flow_in_Coreboot][VBOOT_Flow_20] + +[VBOOT_Flow_20]: vboot_flow_20.png + +While this design implements verified boot to a certain extent, it does not take +into account a few use-cases and concerns some Intel customers run into. A +couple of them are as listed below, +- Some hardware designs cannot support ‘read-only’ flash region as Root of Trust + and therefore prefer Intel BootGuard technology as RoT for verified boot +mechanism. +- In some use-cases, some of the firmware components may come from different + media, for instance, customer could boot payload from USB thumb drive instead +of SPI flash. In that case, the entire read-write section will not have all the +firmware components. There needs to be a mechanism to verify payload along with +other components. +- In exisiting implementation, all the firmware components in the boot chain are + verified in verstage. This may result in a TOCTOU attack where right after the +verification phase, some firmware stages/components (such as payload, FSP, etc.) +can be swapped with malware. To extend the vboot security model, the mechanism +described below is proposed where the root of trust Bootguard begins by +verifying the IBB and the chain of trust is extended only to the next +stage/component at every verification pass. For instance, once Bootguard +verifies IBB which contains verstage, romstage and postcar), postcar then uses +the VBOOT mechanism to verify ramstage. Once ramstage is loaded into DRAM, +ramstage in turn will verify FSP and payload. + +### Proposed Changes using VBOOT2.1 Libraries + +The proposed changes are described as follows. Upon power on of the device, +Intel Bootguard attempts to verify IBB. IBB in this case, replaces the read-only +portion of the flash map and contains bootblock, verstage, romstage and postcar +stage. If the verification is successful, Intel Bootguard launches IBB and the +system boots until postcar stage. In postcar stage, GBB is extracted and GBB +verified the VBLOCK. Once VBLOCK is verified, postcar stage extracts the +ramstage hash from VBLOCK and verifies the ramstage after it has been loaded +into DRAM. This is done to ensure maximum security. Once ramstage is verified, +ramstage is launched. At this point, ramstage extracts the hash of FSPS, DSDT +ACPI table and payload in that order, and verifies and launches them +sequentially. At any point, if the verification fails, the system boot will +halt. + +**Stage/Blob Verification using VBOOT2.1 Library** +![VBOOT_Stage_Blob_Verification][VBOOT_Flow_21] + +[VBOOT_Flow_21]: vboot_flow_21.png + +### Flash Partition and Code Flow in Coreboot + +**Flash Partition for Verification using VBOOT 2.1 Library** +![VBOOT_Verification_2_1_Flash][Flash_Partition] + +[Flash_Partition]: flash_partition.png + +**Verification Logic using VBOOT 2.1 Library** +![VBOOT_Verification_2_1_Logic][vboot_21_logic] + +[vboot_21_logic]: vboot_21_logic.png + -- To view, visit https://review.coreboot.org/c/coreboot/+/32159 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1eb174bb4f4d84eb8f6befdce18421b6b85ccc02 Gerrit-Change-Number: 32159 Gerrit-PatchSet: 1 Gerrit-Owner: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-MessageType: newchange

5 13

Change in coreboot[master]: cbfs: Add helper functions to look up size and type of a file
by Julius Werner (Code Review) 15 Nov '21

15 Nov '21

Attention is currently required from: Jakub Czapiga, Werner Zeh. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59312 ) Change subject: cbfs: Add helper functions to look up size and type of a file ...................................................................... Patch Set 1: (3 comments) File src/include/cbfs.h: https://review.coreboot.org/c/coreboot/+/59312/comment/1669e976_85b86f6b PS1, Line 134: static inline bool cbfs_file_exists(const char *name); : static inline bool cbfs_ro_file_exists(const char *name); > I wonder, if we actually need these functions. […] Here was the original code that highlighted the need for a helper like this: https://review.coreboot.org/c/coreboot/+/55367/52/src/soc/intel/elkhartlake… Sometimes you may need to check if a file is there without immediately loading it if it is (and like I tried to point out in the comments, we should avoid those situations as much as possible, but sometimes they can't be avoided). https://review.coreboot.org/c/coreboot/+/59312/comment/65465664_dbc082b4 PS1, Line 308: static inline size_t cbfs_get_size(const char *name) > Should not these functions be inaccessible for code running in CAR mode and similar? This might make […] It's the same stack requirement as a normal CBFS file load? Early stages do those as well. The general coreboot minimum stack size is 2K (and I believe in practice all platforms are much larger... for x86 platforms the CAR stack tends to be 8K or more). cbfs_mdata is intentionally limited to 256 bytes since that should hopefully never be an issue. File src/lib/cbfs.c: https://review.coreboot.org/c/coreboot/+/59312/comment/4c9c3d86_07a45654 PS1, Line 528: if ((err = _cbfs_boot_lookup(prog_name(pstage), false, &mdata, &rdev))) > Would not it be cleaner to set err value in a separate line above? Personally I find assign-and-chec […] I'd say it's a personal style choice... it's not forbidden by the coreboot style guide. I find them useful and increasing readability, because the same code can be written in fewer lines (so you can have more meaningful parts of a function on one screen). Also, I think they are much more important for while-loops, and when you allow them in while-loops there's no reason not to use them in if-statements where appropriate as well. (For example, writing a loop like https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/sr… where you want to call a function, use the result to decide whether the loop ends, and then continue working on the result if it doesn't, looks a lot more ugly and harder to follow without assign-and-check.) Also, in my experience, most concerns about this just come from some 90's era conventional wisdom that it was dangerous (accidentally writing = instead of ==), but these days GCC forces you to put extra parenthesis around the assignment anyway so doing it unintentionally is impossible. -- To view, visit https://review.coreboot.org/c/coreboot/+/59312 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8092d8f6e04bdfb4df6c626dc7d42b402fe0a8ba Gerrit-Change-Number: 59312 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Mon, 15 Nov 2021 22:38:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/intel/adlrvp: Use dedicated VBT files for ADL-M
by Patrick Georgi (Code Review) 15 Nov '21

15 Nov '21

Attention is currently required from: Furquan Shaikh, Selma Bensaid, Maulik V Vaghela, Paul Menzel, Balaji Manigandan, Bernardo Perez Priego, Tim Wawrzynczak. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58202 ) Change subject: mb/intel/adlrvp: Use dedicated VBT files for ADL-M ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/58202/comment/2c17829b_36bf4945 PS3, Line 12: VBT files added at chrome-internal:4138272 > These VBT are HW specific, not sure what is the advantage of making them public. […] The advantage is that a single tree can build everything. The project's culture is "open, except ..." and so far I haven't heard of VBTs needing an exception. That said, I don't intend to block this change over it, VBTs are just some thing that nobody ever could argue why they should be put under lock & wrap, but for some reason often are. -- To view, visit https://review.coreboot.org/c/coreboot/+/58202 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibbf3f11c9277f5dcb3e12f9020f54ec843444c3f Gerrit-Change-Number: 58202 Gerrit-PatchSet: 3 Gerrit-Owner: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <patrick(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Mon, 15 Nov 2021 22:25:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi <patrick(a)coreboot.org> Comment-In-Reply-To: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib/prog_loaders, soc/amd/: Make payload_preload use cbfs_preload
by Raul Rangel (Code Review) 15 Nov '21

15 Nov '21

Attention is currently required from: Jason Glenesk, Marshall Dawson, Julius Werner, Rob Barnes, Karthik Ramasubramanian, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58962 ) Change subject: lib/prog_loaders, soc/amd/: Make payload_preload use cbfs_preload ...................................................................... Patch Set 1: (1 comment) File src/lib/prog_loaders.c: https://review.coreboot.org/c/coreboot/+/58962/comment/4f2b0d36_0dd1cae1 PS1, Line 133: return; > nit: not sure why I didn't mention this first time this patch came around, but I think think this `r […] So the initial thought was that the preload methods are all no-ops when not enabled. This makes the calling code easier. See https://source.corp.google.com/chromeos_public/src/third_party/coreboot/src… -- To view, visit https://review.coreboot.org/c/coreboot/+/58962 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idc521b238620ff52b8ba481cd3c10e5c4f1394bd Gerrit-Change-Number: 58962 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 15 Nov 2021 22:20:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: ec/starlabs: Add standardised ITE EC support
by Sean Rhodes (Code Review) 15 Nov '21

15 Nov '21

Attention is currently required from: Andy Pont, Paul Menzel, Angel Pons, Felix Held. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58343 ) Change subject: ec/starlabs: Add standardised ITE EC support ...................................................................... Patch Set 40: (2 comments) File src/ec/starlabs/merlin/Kconfig: https://review.coreboot.org/c/coreboot/+/58343/comment/0ea66016_86b079f0 PS37, Line 22: bool "Adjustable Keyboard Backlight" > If it's not meant to be user-configurable, the prompt (text after `bool`) should be removed. Done File src/ec/starlabs/merlin/ec.c: https://review.coreboot.org/c/coreboot/+/58343/comment/c5845f6a_7b8753be PS19, Line 134: #if CONFIG(EC_STARLABS_FAN) : switch (get_uint_option("fan_mode", 0)) { : case FAN_AGGRESSIVE: : ec_write(ECRAM_FAN_MODE, FAN_AGGRESSIVE); : break; : case FAN_QUIET: : ec_write(ECRAM_FAN_MODE, FAN_QUIET); : break; : default: : ec_write(ECRAM_FAN_MODE, FAN_NORMAL); : break; : } : #endif > Ah, I see. Then, you should do the following: […] That's very cool, thanks :) -- To view, visit https://review.coreboot.org/c/coreboot/+/58343 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8023c26de23c874c84106fda96e64dcfa0c5ba32 Gerrit-Change-Number: 58343 Gerrit-PatchSet: 40 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 15 Nov 2021 22:09:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <admin(a)starlabs.systems> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: ec/starlabs: Add standardised ITE EC support
by Sean Rhodes (Code Review) 15 Nov '21

15 Nov '21

Attention is currently required from: Sean Rhodes, Andy Pont, Paul Menzel, Felix Held. Hello build bot (Jenkins), Patrick Georgi, Angel Pons, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/58343 to look at the new patch set (#41). Change subject: ec/starlabs: Add standardised ITE EC support ...................................................................... ec/starlabs: Add standardised ITE EC support Add EC support that supports different Q Events and EC memory. Created from the ITE IT5570E and IT8987E datasheets, all using data port 0x4e. Tested with Ubuntu 20.04.3 and Windows 10 on: * StarBook Mk V (TGL + IT5570E): * ITE Firmware 1.00 * Merlin Firmware 1.00 * LabTop Mk IV (CML + IT8987E): * ITE Firmware 1.04 * LabTop Mk III (KBL + IT8987E): * ITE Firmware 3.12 Signed-off-by: Sean Rhodes <sean(a)starlabs.systems> Change-Id: I8023c26de23c874c84106fda96e64dcfa0c5ba32 --- A src/ec/starlabs/merlin/Kconfig A src/ec/starlabs/merlin/Makefile.inc A src/ec/starlabs/merlin/acpi/ac.asl A src/ec/starlabs/merlin/acpi/battery.asl A src/ec/starlabs/merlin/acpi/cmos.asl A src/ec/starlabs/merlin/acpi/ec.asl A src/ec/starlabs/merlin/acpi/hid.asl A src/ec/starlabs/merlin/acpi/keyboard.asl A src/ec/starlabs/merlin/acpi/lid.asl A src/ec/starlabs/merlin/acpi/suspend.asl A src/ec/starlabs/merlin/acpi/typec.asl A src/ec/starlabs/merlin/acpi/ubtc.asl A src/ec/starlabs/merlin/ec.c A src/ec/starlabs/merlin/ec.h A src/ec/starlabs/merlin/variants/apl/ecdefs.h A src/ec/starlabs/merlin/variants/apl/emem.asl A src/ec/starlabs/merlin/variants/apl/events.asl A src/ec/starlabs/merlin/variants/cml/ecdefs.h A src/ec/starlabs/merlin/variants/cml/emem.asl A src/ec/starlabs/merlin/variants/cml/events.asl A src/ec/starlabs/merlin/variants/glk/ecdefs.h A src/ec/starlabs/merlin/variants/glk/emem.asl A src/ec/starlabs/merlin/variants/glk/events.asl A src/ec/starlabs/merlin/variants/kbl/ecdefs.h A src/ec/starlabs/merlin/variants/kbl/emem.asl A src/ec/starlabs/merlin/variants/kbl/events.asl A src/ec/starlabs/merlin/variants/merlin/ecdefs.h A src/ec/starlabs/merlin/variants/merlin/emem.asl A src/ec/starlabs/merlin/variants/merlin/events.asl A src/ec/starlabs/merlin/variants/tgl/ecdefs.h A src/ec/starlabs/merlin/variants/tgl/emem.asl A src/ec/starlabs/merlin/variants/tgl/events.asl 32 files changed, 3,703 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/58343/41 -- To view, visit https://review.coreboot.org/c/coreboot/+/58343 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8023c26de23c874c84106fda96e64dcfa0c5ba32 Gerrit-Change-Number: 58343 Gerrit-PatchSet: 41 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Attention: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021