Philipp Deppenwiese submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Philipp Deppenwiese: Looks good to me, approved Julius Werner: Looks good to me, approved
security: Add common boot media write protection

Introduce boot media protection settings and use the existing
boot_device_wp_region() function to apply settings on all
platforms that supports it yet.

Also remove the Intel southbridge code, which is now obsolete.
Every platform locks the SPIBAR in a different stage.
For align up with the common mrc cache driver and lock after it has been
written to.

Tested on Supermicro X11SSH-TF. The whole address space is write-protected.

Change-Id: Iceb3ecf0bde5cec562bc62d1d5c79da35305d183
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32704
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Reviewed-by: Julius Werner <jwerner@chromium.org>
---
M src/include/boot_device.h
M src/security/Kconfig
M src/security/Makefile.inc
A src/security/lockdown/Kconfig
A src/security/lockdown/Makefile.inc
A src/security/lockdown/lockdown.c
M src/southbridge/intel/common/Kconfig
M src/southbridge/intel/common/finalize.c
8 files changed, 135 insertions(+), 49 deletions(-)

diff --git a/src/include/boot_device.h b/src/include/boot_device.h
index 4707331..3146462 100644
--- a/src/include/boot_device.h
+++ b/src/include/boot_device.h
@@ -62,4 +62,12 @@
**/
void boot_device_init(void);

+/*
+ * Restrict read/write access to the bootmedia using platform defined rules.
+ */
+#if CONFIG(BOOTMEDIA_LOCK_NONE)
+static inline void boot_device_security_lockdown(void) {}
+#else
+void boot_device_security_lockdown(void);
+#endif
#endif /* _BOOT_DEVICE_H_ */
diff --git a/src/security/Kconfig b/src/security/Kconfig
index b967311..65d2def 100644
--- a/src/security/Kconfig
+++ b/src/security/Kconfig
@@ -15,3 +15,4 @@
source "src/security/tpm/Kconfig"
source "src/security/memory/Kconfig"
source "src/security/intel/Kconfig"
+source "src/security/lockdown/Kconfig"
diff --git a/src/security/Makefile.inc b/src/security/Makefile.inc
index fd78438..72b87db 100644
--- a/src/security/Makefile.inc
+++ b/src/security/Makefile.inc
@@ -2,3 +2,4 @@
subdirs-y += tpm
subdirs-y += memory
subdirs-y += intel
+subdirs-y += lockdown
diff --git a/src/security/lockdown/Kconfig b/src/security/lockdown/Kconfig
new file mode 100644
index 0000000..bfdc984
--- /dev/null
+++ b/src/security/lockdown/Kconfig
@@ -0,0 +1,62 @@
+
+
+choice
+ prompt "Boot media protection mechanism"
+ default BOOTMEDIA_LOCK_NONE
+
+config BOOTMEDIA_LOCK_NONE
+ bool "Don't lock boot media sections"
+
+config BOOTMEDIA_LOCK_CONTROLLER
+ bool "Lock boot media using the controller"
+ help
+ Select this if you want the controller to lock specific regions.
+ This only works on some platforms, please check the code or boot log.
+ On Intel platforms for e.g. this will make use of the SPIBAR PRRs.
+
+config BOOTMEDIA_LOCK_CHIP
+ bool "Lock boot media using the chip"
+ help
+ Select this if you want the chip to lock specific regions.
+ This only works on some chips, please check the code or boot log.
+
+endchoice
+
+choice
+ prompt "Boot media protected regions"
+ depends on !BOOTMEDIA_LOCK_NONE
+ default BOOTMEDIA_LOCK_WHOLE_RO
+
+config BOOTMEDIA_LOCK_WHOLE_RO
+ bool "Write-protect the whole boot medium"
+ help
+ Select this if you want to write-protect the whole firmware boot
+ medium.
+
+ The locking will take place during the chipset lockdown.
+ Chipset lockdown is platform specific und might be done unconditionally,
+ when INTEL_CHIPSET_LOCKDOWN is set or has to be triggered later
+ (e.g. by the payload or the OS).
+
+ NOTE: If you trigger the chipset lockdown unconditionally,
+ you won't be able to write to the whole flash chip using the
+ internal controller any more.
+
+config BOOTMEDIA_LOCK_WHOLE_NO_ACCESS
+ depends on BOOTMEDIA_LOCK_CONTROLLER
+ bool "Read- and write-protect the whole boot medium"
+ help
+ Select this if you want to protect the firmware boot medium against
+ all further accesses. On platforms that memory map a part of the
+ boot medium the corresponding region is still readable.
+
+ The locking will take place during the chipset lockdown.
+ Chipset lockdown is platform specific und might be done unconditionally,
+ when INTEL_CHIPSET_LOCKDOWN is set or has to be triggered later
+ (e.g. by the payload or the OS).
+
+ NOTE: If you trigger the chipset lockdown unconditionally,
+ you won't be able to write to the whole flash chip using the
+ internal controller any more.
+
+endchoice
diff --git a/src/security/lockdown/Makefile.inc b/src/security/lockdown/Makefile.inc
new file mode 100644
index 0000000..a51a106
--- /dev/null
+++ b/src/security/lockdown/Makefile.inc
@@ -0,0 +1,6 @@
+## SPDX-License-Identifier: GPL-2.0-or-later
+## This file is part of the coreboot project.
+
+ifneq ($(CONFIG_BOOTMEDIA_LOCK_NONE),y)
+ramstage-y += lockdown.c
+endif
diff --git a/src/security/lockdown/lockdown.c b/src/security/lockdown/lockdown.c
new file mode 100644
index 0000000..a8aad9b
--- /dev/null
+++ b/src/security/lockdown/lockdown.c
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* This file is part of the coreboot project. */
+
+#include <boot_device.h>
+#include <commonlib/region.h>
+#include <console/console.h>
+#include <bootstate.h>
+
+/*
+ * Enables read- /write protection of the bootmedia.
+ */
+void boot_device_security_lockdown(void)
+{
+ const struct region_device *rdev;
+ enum bootdev_prot_type lock_type;
+
+ printk(BIOS_DEBUG, "BM-LOCKDOWN: Enabling boot media protection scheme ");
+
+ if (CONFIG(BOOTMEDIA_LOCK_CONTROLLER)) {
+ if (CONFIG(BOOTMEDIA_LOCK_WHOLE_RO)) {
+ printk(BIOS_DEBUG, "'readonly'");
+ lock_type = CTRLR_WP;
+ } else if (CONFIG(BOOTMEDIA_LOCK_WHOLE_NO_ACCESS)) {
+ printk(BIOS_DEBUG, "'no access'");
+ lock_type = CTRLR_RWP;
+ }
+ printk(BIOS_DEBUG, "using CTRL...\n");
+ } else {
+ if (CONFIG(BOOTMEDIA_LOCK_WHOLE_RO)) {
+ printk(BIOS_DEBUG, "'readonly'");
+ lock_type = MEDIA_WP;
+ }
+ printk(BIOS_DEBUG, "using flash chip...\n");
+ }
+
+ rdev = boot_device_ro();
+
+ if (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");
+}
+
+static void lock(void *unused)
+{
+ boot_device_security_lockdown();
+}
+
+/*
+ * Keep in sync with mrc_cache.c
+ */
+
+#if CONFIG(MRC_WRITE_NV_LATE)
+BOOT_STATE_INIT_ENTRY(BS_OS_RESUME_CHECK, BS_ON_EXIT, lock, NULL);
+#else
+BOOT_STATE_INIT_ENTRY(BS_DEV_RESOURCES, BS_ON_ENTRY, lock, NULL);
+#endif
diff --git a/src/southbridge/intel/common/Kconfig b/src/southbridge/intel/common/Kconfig
index d1b6bf6..9356a2b 100644
--- a/src/southbridge/intel/common/Kconfig
+++ b/src/southbridge/intel/common/Kconfig
@@ -97,42 +97,3 @@
config SOUTHBRIDGE_INTEL_COMMON_WATCHDOG
bool
depends on SOUTHBRIDGE_INTEL_COMMON_PMBASE
-
-if SOUTHBRIDGE_INTEL_COMMON_FINALIZE
-
-choice
- prompt "Flash locking during chipset lockdown"
- default LOCK_SPI_FLASH_NONE
-
-config LOCK_SPI_FLASH_NONE
- bool "Don't lock flash sections"
-
-config LOCK_SPI_FLASH_RO
- bool "Write-protect all flash sections"
- help
- Select this if you want to write-protect the whole firmware flash
- chip. The locking will take place during the chipset lockdown, which
- 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).
-
- NOTE: If you trigger the chipset lockdown unconditionally,
- you won't be able to write to the flash chip using the
- internal programmer any more.
-
-config LOCK_SPI_FLASH_NO_ACCESS
- bool "Write-protect all flash sections and read-protect non-BIOS sections"
- help
- Select this if you want to protect the firmware flash against all
- further accesses (with the exception of the memory mapped BIOS re-
- gion which is always readable). The locking will take place during
- the chipset lockdown, which 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).
-
- NOTE: If you trigger the chipset lockdown unconditionally,
- you won't be able to write to the flash chip using the
- internal programmer any more.
-
-endchoice
-
-endif
diff --git a/src/southbridge/intel/common/finalize.c b/src/southbridge/intel/common/finalize.c
index 4c6cc63..2d66cad 100644
--- a/src/southbridge/intel/common/finalize.c
+++ b/src/southbridge/intel/common/finalize.c
@@ -15,16 +15,6 @@
{
const pci_devfn_t lpc_dev = PCI_DEV(0, 0x1f, 0);

- if (CONFIG(LOCK_SPI_FLASH_RO) ||
- CONFIG(LOCK_SPI_FLASH_NO_ACCESS)) {
- int i;
- u32 lockmask = 1UL << 31;
- if (CONFIG(LOCK_SPI_FLASH_NO_ACCESS))
- lockmask |= 1 << 15;
- for (i = 0; i < 20; i += 4)
- RCBA32(0x3874 + i) = RCBA32(0x3854 + i) | lockmask;
- }
-
/* Lock SPIBAR */
RCBA32_OR(0x3804, (1 << 15));


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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iceb3ecf0bde5cec562bc62d1d5c79da35305d183
Gerrit-Change-Number: 32704
Gerrit-PatchSet: 14
Gerrit-Owner: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter@9elements.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks@eltan.com>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Aaron Durbin <adurbin@chromium.org>
Gerrit-CC: Michael Niewöhner
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged