On Mon, Aug 29, 2016 at 01:30:44PM -0700, Linn Crosetto wrote: > On Sat, Aug 27, 2016 at 10:22:52PM +0100, Ben Hutchings wrote: > > Control: tag -1 pending > > > > Applied, but: > > > > > > > > --- a/drivers/firmware/efi/libstub/fdt.c > > > +++ b/drivers/firmware/efi/libstub/fdt.c > > > @@ -139,6 +139,13 @@ efi_status_t update_fdt(efi_system_table_t > > > *sys_table, void *orig_fdt, > > > return efi_status; > > > } > > > } > > > + > > > + fdt_val32 = efi_get_secureboot(sys_table); > > > > Shouldn't there be a cpu_to_fdt32() conversion here? > > Yes. When setting securelevel the check to see if secure_boot is > greater-than-zero behaves the same, but it should be byte-swapped to preserve > the value returned by efi_get_secureboot(). I tested and validated the > following change: > > @@ -140,7 +140,7 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt, > } > } > > - fdt_val32 = efi_get_secureboot(sys_table); > + fdt_val32 = cpu_to_fdt32(efi_get_secureboot(sys_table)); > status = fdt_setprop(fdt, node, "linux,uefi-secure-boot", > &fdt_val32, sizeof(fdt_val32)); > if (status) Attaching version 2 of the patches with the change.
From 5ea781b342c1df2e4de72a5d2a6b34b34d02875c Mon Sep 17 00:00:00 2001
From: Linn Crosetto <linn@hpe.com>
Date: Mon, 22 Feb 2016 12:54:37 -0700
Subject: [PATCH v2 1/2] arm64/efi: Disable secure boot if shim is in insecure
mode
Port to arm64 a patch originally written by Josh Boyer for the x86 EFI
stub.
A user can manually tell the shim boot loader to disable validation of
images it loads. When a user does this, it creates a UEFI variable called
MokSBState that does not have the runtime attribute set. Given that the
user explicitly disabled validation, we can honor that and not enable
secure boot mode if that variable is set.
Signed-off-by: Linn Crosetto <linn@hpe.com>
Cc: Josh Boyer <jwboyer@fedoraproject.org>
---
drivers/firmware/efi/libstub/arm-stub.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 993aa56..d1cb612 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -26,11 +26,14 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)
'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0 };
static efi_char16_t const sm_var_name[] = {
'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0 };
+ static efi_char16_t const mk_var_name[] = {
+ 'M', 'o', 'k', 'S', 'B', 'S', 't', 'a', 't', 'e', 0 };
efi_guid_t var_guid = EFI_GLOBAL_VARIABLE_GUID;
efi_get_variable_t *f_getvar = sys_table_arg->runtime->get_variable;
u8 val;
unsigned long size = sizeof(val);
+ u32 attr;
efi_status_t status;
status = f_getvar((efi_char16_t *)sb_var_name, (efi_guid_t *)&var_guid,
@@ -51,6 +54,22 @@ static int efi_get_secureboot(efi_system_table_t *sys_table_arg)
if (val == 1)
return 0;
+ /* See if a user has put shim into insecure_mode. If so, and the variable
+ * doesn't have the runtime attribute set, we might as well honor that.
+ */
+ var_guid = EFI_SHIM_LOCK_GUID;
+ status = f_getvar((efi_char16_t *)mk_var_name, (efi_guid_t *)&var_guid,
+ &attr, &size, &val);
+
+ /* If it fails, we don't care why. Default to secure */
+ if (status != EFI_SUCCESS)
+ return 1;
+
+ if (!(attr & EFI_VARIABLE_RUNTIME_ACCESS)) {
+ if (val == 1)
+ return 0;
+ }
+
return 1;
out_efi_err:
--
2.6.4
From 970ddb1eb849083673c1d3e8604cb7bd66e99e44 Mon Sep 17 00:00:00 2001
From: Linn Crosetto <linn@hpe.com>
Date: Tue, 30 Aug 2016 11:54:38 -0600
Subject: [PATCH v2 2/2] arm64: add kernel config option to set securelevel when in Secure Boot mode
Add a kernel configuration option to enable securelevel, to restrict
userspace's ability to modify the running kernel when UEFI Secure Boot is
enabled. Based on the x86 patch by Matthew Garrett.
Determine the state of Secure Boot in the EFI stub and pass this to the
kernel using the FDT.
Signed-off-by: Linn Crosetto <linn@hpe.com>
---
v2:
- Add cpu_to_fdt32() when setting Secure Boot flag in FDT (Ben Hutchings)
arch/arm64/Kconfig | 13 +++++++++++++
drivers/firmware/efi/arm-init.c | 7 +++++++
drivers/firmware/efi/efi.c | 3 ++-
drivers/firmware/efi/libstub/arm-stub.c | 2 +-
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/fdt.c | 7 +++++++
include/linux/efi.h | 1 +
7 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5a0a691..78895f3 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -953,6 +953,19 @@ config EFI
allow the kernel to be booted as an EFI application. This
is only useful on systems that have UEFI firmware.
+config EFI_SECURE_BOOT_SECURELEVEL
+ def_bool n
+ depends on SECURITY_SECURELEVEL
+ depends on EFI
+ prompt "Automatically set securelevel when UEFI Secure Boot is enabled"
+ ---help---
+ UEFI Secure Boot provides a mechanism for ensuring that the
+ firmware will only load signed bootloaders and kernels. Certain
+ use cases may also require that the kernel restrict any userspace
+ mechanism that could insert untrusted code into the kernel.
+ Say Y here to automatically enable securelevel enforcement
+ when a system boots with UEFI Secure Boot enabled.
+
config DMI
bool "Enable support for SMBIOS (DMI) tables"
depends on EFI
diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index c49d50e..df57f6a 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -21,6 +21,7 @@
#include <linux/of_fdt.h>
#include <linux/platform_device.h>
#include <linux/screen_info.h>
+#include <linux/security.h>
#include <asm/efi.h>
@@ -243,6 +244,12 @@ void __init efi_init(void)
"Unexpected EFI_MEMORY_DESCRIPTOR version %ld",
efi.memmap.desc_version);
+#ifdef CONFIG_EFI_SECURE_BOOT_SECURELEVEL
+ if (params.secure_boot > 0) {
+ set_securelevel(1);
+ }
+#endif
+
if (uefi_init() < 0)
return;
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 05509f3..7e3582e 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -482,7 +482,8 @@ static __initdata struct {
UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
UEFI_PARAM("MemMap Desc. Size", "linux,uefi-mmap-desc-size", desc_size),
- UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
+ UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver),
+ UEFI_PARAM("Secure Boot Enabled", "linux,uefi-secure-boot", secure_boot)
};
struct param_info {
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index d1cb612..61ad972 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -20,7 +20,7 @@
bool __nokaslr;
-static int efi_get_secureboot(efi_system_table_t *sys_table_arg)
+int efi_get_secureboot(efi_system_table_t *sys_table_arg)
{
static efi_char16_t const sb_var_name[] = {
'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0 };
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index ee49cd2..97a329a 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -62,4 +62,5 @@ efi_status_t efi_random_alloc(efi_system_table_t *sys_table_arg,
efi_status_t check_platform_features(efi_system_table_t *sys_table_arg);
+int efi_get_secureboot(efi_system_table_t *sys_table_arg);
#endif
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index e58abfa..01d4515 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -139,6 +139,13 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
return efi_status;
}
}
+
+ fdt_val32 = cpu_to_fdt32(efi_get_secureboot(sys_table));
+ status = fdt_setprop(fdt, node, "linux,uefi-secure-boot",
+ &fdt_val32, sizeof(fdt_val32));
+ if (status)
+ goto fdt_set_fail;
+
return EFI_SUCCESS;
fdt_set_fail:
diff --git a/include/linux/efi.h b/include/linux/efi.h
index e6a8fe9..4b9a8aa 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -745,6 +745,7 @@ struct efi_fdt_params {
u32 mmap_size;
u32 desc_size;
u32 desc_ver;
+ u32 secure_boot;
};
typedef struct {
--
2.6.4
Attachment:
signature.asc
Description: Digital signature