Philipp Deppenwiese has submitted this change. ( https://review.coreboot.org/c/coreboot/+/32705 )
Change subject: security/lockdown: Write-protect WP_RO ......................................................................
security/lockdown: Write-protect WP_RO
Allow to write protect only the WP_RO region in case of enabled VBOOT. One can either lock the boot device in VERSTAGE early if VBOOT is enabled, or late in RAMSTAGE. Both options have their downsides as explained below.
Lock early if you don't trust the code that's stored in the writeable flash partition. This prevents write-protecting the MRC cache, which is written in ramstage. In case the contents of the MRC cache are corrupted this can lead to system instability or trigger unwanted code flows inside the firmware.
Lock late if you trust the code that's stored in the writeable flash partition. This allows write-protecting the MRC cache, but if a vulnerability is found in the code of the writeable partition an attacker might be able to overwrite the whole flash as it hasn't been locked yet.
Change-Id: I72c3e1a0720514b9b85b0433944ab5fb7109b2a2 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Signed-off-by: Christian Walter christian.walter@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32705 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com --- M src/include/boot_device.h M src/security/lockdown/Kconfig M src/security/lockdown/Makefile.inc M src/security/lockdown/lockdown.c M src/security/vboot/vboot_logic.c 5 files changed, 52 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Philipp Deppenwiese: Looks good to me, approved Julius Werner: Looks good to me, approved
diff --git a/src/include/boot_device.h b/src/include/boot_device.h index 3146462..d5237cd4 100644 --- a/src/include/boot_device.h +++ b/src/include/boot_device.h @@ -65,7 +65,7 @@ /* * Restrict read/write access to the bootmedia using platform defined rules. */ -#if CONFIG(BOOTMEDIA_LOCK_NONE) +#if CONFIG(BOOTMEDIA_LOCK_NONE) || (CONFIG(BOOTMEDIA_LOCK_IN_VERSTAGE) && ENV_RAMSTAGE) static inline void boot_device_security_lockdown(void) {} #else void boot_device_security_lockdown(void); diff --git a/src/security/lockdown/Kconfig b/src/security/lockdown/Kconfig index bfdc984..30b5237 100644 --- a/src/security/lockdown/Kconfig +++ b/src/security/lockdown/Kconfig @@ -59,4 +59,26 @@ you won't be able to write to the whole flash chip using the internal controller any more.
+config BOOTMEDIA_LOCK_WPRO_VBOOT_RO + bool "Write-protect WP_RO FMAP region in boot medium" + depends on VBOOT + help + Select this if you want to write-protect the WP_RO region as specified + in the VBOOT FMAP. You will be able to write every region outside + of WP_RO using the internal controller (eg. FW_MAIN_A/FW_MAIN_B). + In case of BOOTMEDIA_LOCK_IN_VERSTAGE the locking will take place + early, preventing locking of facilities used in ramstage, like the + MRC cache. If not using BOOTMEDIA_LOCK_IN_VERSTAGE the chipset lockdown + is either triggered by coreboot (when INTEL_CHIPSET_LOCKDOWN is set) or + has to be triggered later (e.g. by the payload or the OS). + endchoice + +config BOOTMEDIA_LOCK_IN_VERSTAGE + depends on BOOTMEDIA_LOCK_WPRO_VBOOT_RO + bool "Lock boot media down in verstage" + help + Select this if you want to write-protect the WP_RO region as soon as + possible. This option prevents using write protecting facilities in + ramstage, like the MRC cache for example. + Use this option if you don't trust code running after verstage. diff --git a/src/security/lockdown/Makefile.inc b/src/security/lockdown/Makefile.inc index a51a106..6ccc557 100644 --- a/src/security/lockdown/Makefile.inc +++ b/src/security/lockdown/Makefile.inc @@ -2,5 +2,11 @@ ## This file is part of the coreboot project.
ifneq ($(CONFIG_BOOTMEDIA_LOCK_NONE),y) + +ifeq ($(CONFIG_BOOTMEDIA_LOCK_IN_VERSTAGE),y) +verstage-y += lockdown.c +else ramstage-y += lockdown.c endif + +endif diff --git a/src/security/lockdown/lockdown.c b/src/security/lockdown/lockdown.c index a8aad9b..62d0a29 100644 --- a/src/security/lockdown/lockdown.c +++ b/src/security/lockdown/lockdown.c @@ -5,13 +5,15 @@ #include <commonlib/region.h> #include <console/console.h> #include <bootstate.h> +#include <fmap.h>
/* * Enables read- /write protection of the bootmedia. */ void boot_device_security_lockdown(void) { - const struct region_device *rdev; + const struct region_device *rdev = NULL; + struct region_device dev; enum bootdev_prot_type lock_type;
printk(BIOS_DEBUG, "BM-LOCKDOWN: Enabling boot media protection scheme "); @@ -23,19 +25,32 @@ } else if (CONFIG(BOOTMEDIA_LOCK_WHOLE_NO_ACCESS)) { printk(BIOS_DEBUG, "'no access'"); lock_type = CTRLR_RWP; + } else if (CONFIG(BOOTMEDIA_LOCK_WPRO_VBOOT_RO)) { + printk(BIOS_DEBUG, "'WP_RO only'"); + lock_type = CTRLR_WP; } printk(BIOS_DEBUG, "using CTRL...\n"); } else { if (CONFIG(BOOTMEDIA_LOCK_WHOLE_RO)) { printk(BIOS_DEBUG, "'readonly'"); lock_type = MEDIA_WP; + } else if (CONFIG(BOOTMEDIA_LOCK_WPRO_VBOOT_RO)) { + printk(BIOS_DEBUG, "'WP_RO only'"); + lock_type = MEDIA_WP; } printk(BIOS_DEBUG, "using flash chip...\n"); }
- rdev = boot_device_ro(); + if (CONFIG(BOOTMEDIA_LOCK_WPRO_VBOOT_RO)) { + if (fmap_locate_area_as_rdev("WP_RO", &dev) < 0) + printk(BIOS_ERR, "BM-LOCKDOWN: Could not find region 'WP_RO'\n"); + else + rdev = &dev; + } else { + rdev = boot_device_ro(); + }
- if (boot_device_wp_region(rdev, lock_type) >= 0) + if (rdev && boot_device_wp_region(rdev, lock_type) >= 0) printk(BIOS_INFO, "BM-LOCKDOWN: Enabled bootmedia protection\n"); else printk(BIOS_ERR, "BM-LOCKDOWN: Failed to enable bootmedia protection\n"); diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 8e82e40..e1c77b6 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -14,6 +14,7 @@ #include <string.h> #include <timestamp.h> #include <vb2_api.h> +#include <boot_device.h>
#include "antirollback.h"
@@ -296,6 +297,10 @@
timestamp_add_now(TS_START_VBOOT);
+ /* Lockdown SPI flash controller if required */ + if (CONFIG(BOOTMEDIA_LOCK_IN_VERSTAGE)) + boot_device_security_lockdown(); + /* Set up context and work buffer */ ctx = vboot_get_context();