2017-04-05 20:14:41

by David Howells

[permalink] [raw]
Subject: [PATCH 00/24] Kernel lockdown


These patches provide a facility by which a variety of avenues by which
userspace can feasibly modify the running kernel image can be locked down.
These include:

(*) No unsigned modules and no modules for which can't validate the
signature.

(*) No use of ioperm(), iopl() and no writing to /dev/port.

(*) No writing to /dev/mem or /dev/kmem.

(*) No hibernation.

(*) Restrict PCI BAR access.

(*) Restrict MSR access.

(*) No kexec_load().

(*) Certain ACPI restrictions.

(*) Restrict debugfs interface to ASUS WMI.

The lock-down can be configured to be triggered by the EFI secure boot
status, provided the shim isn't insecure. The lock-down can be lifted by
typing SysRq+x on a keyboard attached to the system.


The patches can be found here also:

http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=efi-lockdown

They are dependent on the hwparam branch, which I posted separately.

David
---
Chun-Yi Lee (2):
kexec_file: Disable at runtime if securelevel has been set
bpf: Restrict kernel image access functions when the kernel is locked down

Dave Young (1):
Copy secure_boot flag in boot params across kexec reboot

David Howells (7):
Add the ability to lock down access to the running kernel image
efi: Lock down the kernel if booted in secure boot mode
Enforce module signatures if the kernel is locked down
scsi: Lock down the eata driver
Prohibit PCMCIA CIS storage when the kernel is locked down
Lock down TIOCSSERIAL
Lock down module params that specify hardware parameters (eg. ioport)

Josh Boyer (3):
efi: Add EFI_SECURE_BOOT bit
hibernate: Disable when the kernel is locked down
acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down

Kyle McMartin (1):
Add a sysrq option to exit secure boot mode

Linn Crosetto (2):
acpi: Disable ACPI table override if the kernel is locked down
acpi: Disable APEI error injection if the kernel is locked down

Matthew Garrett (8):
Restrict /dev/mem and /dev/kmem when the kernel is locked down
kexec: Disable at runtime if the kernel is locked down
uswsusp: Disable when the kernel is locked down
PCI: Lock down BAR access when the kernel is locked down
x86: Lock down IO port access when the kernel is locked down
x86: Restrict MSR access when the kernel is locked down
asus-wmi: Restrict debugfs interface when the kernel is locked down
ACPI: Limit access to custom_method when the kernel is locked down


arch/x86/Kconfig | 22 ++++++++++++++++++++
arch/x86/kernel/ioport.c | 4 ++--
arch/x86/kernel/kexec-bzimage64.c | 1 +
arch/x86/kernel/msr.c | 7 ++++++
arch/x86/kernel/setup.c | 40 ++++++++++++++++++++++++++++++++++++-
drivers/acpi/apei/einj.c | 3 +++
drivers/acpi/custom_method.c | 3 +++
drivers/acpi/osl.c | 2 +-
drivers/acpi/tables.c | 5 +++++
drivers/char/mem.c | 8 +++++++
drivers/input/misc/uinput.c | 1 +
drivers/pci/pci-sysfs.c | 9 ++++++++
drivers/pci/proc.c | 8 ++++++-
drivers/pci/syscall.c | 2 +-
drivers/pcmcia/cistpl.c | 5 +++++
drivers/platform/x86/asus-wmi.c | 9 ++++++++
drivers/scsi/eata.c | 7 ++++++
drivers/tty/serial/serial_core.c | 6 ++++++
drivers/tty/sysrq.c | 19 ++++++++++++------
include/linux/efi.h | 1 +
include/linux/input.h | 5 +++++
include/linux/kernel.h | 9 ++++++++
include/linux/security.h | 11 ++++++++++
include/linux/sysrq.h | 8 ++++++-
kernel/debug/kdb/kdb_main.c | 2 +-
kernel/kexec.c | 7 ++++++
kernel/kexec_file.c | 6 ++++++
kernel/module.c | 2 +-
kernel/params.c | 27 ++++++++++++++++++++-----
kernel/power/hibernate.c | 2 +-
kernel/power/user.c | 3 +++
kernel/trace/bpf_trace.c | 11 ++++++++++
security/Kconfig | 15 ++++++++++++++
security/Makefile | 3 +++
security/lock_down.c | 40 +++++++++++++++++++++++++++++++++++++
35 files changed, 291 insertions(+), 22 deletions(-)
create mode 100644 security/lock_down.c


2017-04-05 20:14:56

by David Howells

[permalink] [raw]
Subject: [PATCH 02/24] Add the ability to lock down access to the running kernel image

Provide a single call to allow kernel code to determine whether the system
should be locked down, thereby disallowing various accesses that might
allow the running kernel image to be changed including the loading of
modules that aren't validly signed with a key we recognise, fiddling with
MSR registers and disallowing hibernation,

Signed-off-by: David Howells <[email protected]>
---

include/linux/kernel.h | 9 +++++++++
include/linux/security.h | 11 +++++++++++
security/Kconfig | 15 +++++++++++++++
security/Makefile | 3 +++
security/lock_down.c | 40 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 78 insertions(+)
create mode 100644 security/lock_down.c

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 4c26dc3a8295..b820a80dc949 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -275,6 +275,15 @@ extern int oops_may_print(void);
void do_exit(long error_code) __noreturn;
void complete_and_exit(struct completion *, long) __noreturn;

+#ifdef CONFIG_LOCK_DOWN_KERNEL
+extern bool kernel_is_locked_down(void);
+#else
+static inline bool kernel_is_locked_down(void)
+{
+ return false;
+}
+#endif
+
/* Internal, do not use. */
int __must_check _kstrtoul(const char *s, unsigned int base, unsigned long *res);
int __must_check _kstrtol(const char *s, unsigned int base, long *res);
diff --git a/include/linux/security.h b/include/linux/security.h
index af675b576645..68bab18ddd57 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1698,5 +1698,16 @@ static inline void free_secdata(void *secdata)
{ }
#endif /* CONFIG_SECURITY */

+#ifdef CONFIG_LOCK_DOWN_KERNEL
+extern void lock_kernel_down(void);
+#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT
+extern void lift_kernel_lockdown(void);
+#endif
+#else
+static inline void lock_kernel_down(void)
+{
+}
+#endif
+
#endif /* ! __LINUX_SECURITY_H */

diff --git a/security/Kconfig b/security/Kconfig
index 3ff1bf91080e..e3830171bdcb 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -198,6 +198,21 @@ config STATIC_USERMODEHELPER_PATH
If you wish for all usermode helper programs to be disabled,
specify an empty string here (i.e. "").

+config LOCK_DOWN_KERNEL
+ bool "Allow the kernel to be 'locked down'"
+ help
+ Allow the kernel to be locked down under certain circumstances, for
+ instance if UEFI secure boot is enabled. Locking down the kernel
+ turns off various features that might otherwise allow access to the
+ kernel image (eg. setting MSR registers).
+
+config ALLOW_LOCKDOWN_LIFT
+ bool
+ help
+ Allow the lockdown on a kernel to be lifted, thereby restoring the
+ ability of userspace to access the kernel image (eg. by SysRq+x under
+ x86).
+
source security/selinux/Kconfig
source security/smack/Kconfig
source security/tomoyo/Kconfig
diff --git a/security/Makefile b/security/Makefile
index f2d71cdb8e19..8c4a43e3d4e0 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -29,3 +29,6 @@ obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
# Object integrity file lists
subdir-$(CONFIG_INTEGRITY) += integrity
obj-$(CONFIG_INTEGRITY) += integrity/
+
+# Allow the kernel to be locked down
+obj-$(CONFIG_LOCK_DOWN_KERNEL) += lock_down.o
diff --git a/security/lock_down.c b/security/lock_down.c
new file mode 100644
index 000000000000..5788c60ff4e1
--- /dev/null
+++ b/security/lock_down.c
@@ -0,0 +1,40 @@
+/* Lock down the kernel
+ *
+ * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells ([email protected])
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/security.h>
+#include <linux/export.h>
+
+static __read_mostly bool kernel_locked_down;
+
+/*
+ * Put the kernel into lock-down mode.
+ */
+void lock_kernel_down(void)
+{
+ kernel_locked_down = true;
+}
+
+/*
+ * Take the kernel out of lockdown mode.
+ */
+void lift_kernel_lockdown(void)
+{
+ kernel_locked_down = false;
+}
+
+/**
+ * kernel_is_locked_down - Find out if the kernel is locked down
+ */
+bool kernel_is_locked_down(void)
+{
+ return kernel_locked_down;
+}
+EXPORT_SYMBOL(kernel_is_locked_down);

2017-04-05 20:15:14

by David Howells

[permalink] [raw]
Subject: [PATCH 03/24] efi: Lock down the kernel if booted in secure boot mode

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 all kernel modules also be signed. Add a configuration option
that to lock down the kernel - which includes requiring validly signed
modules - if the kernel is secure-booted.

Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

arch/x86/Kconfig | 12 ++++++++++++
arch/x86/kernel/setup.c | 8 +++++++-
2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc98d5a294ee..21f39855661d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1817,6 +1817,18 @@ config EFI_MIXED

If unsure, say N.

+config EFI_SECURE_BOOT_LOCK_DOWN
+ def_bool n
+ depends on EFI
+ prompt "Lock down the kernel 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 all kernel modules also be signed and that
+ userspace is prevented from directly changing the running kernel
+ image. Say Y here to automatically lock down the kernel when a
+ system boots with UEFI Secure Boot enabled.
+
config SECCOMP
def_bool y
prompt "Enable seccomp to safely compute untrusted bytecode"
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 396285bddb93..85dfa745c442 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -69,6 +69,7 @@
#include <linux/crash_dump.h>
#include <linux/tboot.h>
#include <linux/jiffies.h>
+#include <linux/security.h>

#include <video/edid.h>

@@ -1185,7 +1186,12 @@ void __init setup_arch(char **cmdline_p)
break;
case efi_secureboot_mode_enabled:
set_bit(EFI_SECURE_BOOT, &efi.flags);
- pr_info("Secure boot enabled\n");
+ if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT_LOCK_DOWN)) {
+ lock_kernel_down();
+ pr_info("Secure boot enabled and kernel locked down\n");
+ } else {
+ pr_info("Secure boot enabled\n");
+ }
break;
default:
pr_info("Secure boot could not be determined\n");

2017-04-05 20:14:52

by David Howells

[permalink] [raw]
Subject: [PATCH 01/24] efi: Add EFI_SECURE_BOOT bit

From: Josh Boyer <[email protected]>

UEFI machines can be booted in Secure Boot mode. Add a EFI_SECURE_BOOT bit
that can be passed to efi_enabled() to find out whether secure boot is
enabled.

This will be used by the SysRq+x handler, registered by the x86 arch, to find
out whether secure boot mode is enabled so that it can be disabled.

Signed-off-by: Josh Boyer <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

arch/x86/kernel/setup.c | 1 +
include/linux/efi.h | 1 +
2 files changed, 2 insertions(+)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 4bf0c8926a1c..396285bddb93 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1184,6 +1184,7 @@ void __init setup_arch(char **cmdline_p)
pr_info("Secure boot disabled\n");
break;
case efi_secureboot_mode_enabled:
+ set_bit(EFI_SECURE_BOOT, &efi.flags);
pr_info("Secure boot enabled\n");
break;
default:
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 94d34e0be24f..6049600e5475 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1069,6 +1069,7 @@ extern int __init efi_setup_pcdp_console(char *);
#define EFI_DBG 8 /* Print additional debug info at runtime */
#define EFI_NX_PE_DATA 9 /* Can runtime data regions be mapped non-executable? */
#define EFI_MEM_ATTR 10 /* Did firmware publish an EFI_MEMORY_ATTRIBUTES table? */
+#define EFI_SECURE_BOOT 11 /* Are we in Secure Boot mode? */

#ifdef CONFIG_EFI
/*

2017-04-05 20:16:01

by David Howells

[permalink] [raw]
Subject: [PATCH 08/24] Copy secure_boot flag in boot params across kexec reboot

From: Dave Young <[email protected]>

Kexec reboot in case secure boot being enabled does not keep the secure
boot mode in new kernel, so later one can load unsigned kernel via legacy
kexec_load. In this state, the system is missing the protections provided
by secure boot.

Adding a patch to fix this by retain the secure_boot flag in original
kernel.

secure_boot flag in boot_params is set in EFI stub, but kexec bypasses the
stub. Fixing this issue by copying secure_boot flag across kexec reboot.

Signed-off-by: Dave Young <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

arch/x86/kernel/kexec-bzimage64.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index d0a814a9d96a..3551bcaa1eaf 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -179,6 +179,7 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
if (efi_enabled(EFI_OLD_MEMMAP))
return 0;

+ params->secure_boot = boot_params.secure_boot;
ei->efi_loader_signature = current_ei->efi_loader_signature;
ei->efi_systab = current_ei->efi_systab;
ei->efi_systab_hi = current_ei->efi_systab_hi;

2017-04-05 20:16:47

by David Howells

[permalink] [raw]
Subject: [PATCH 13/24] x86: Lock down IO port access when the kernel is locked down

From: Matthew Garrett <[email protected]>

IO port access would permit users to gain access to PCI configuration
registers, which in turn (on a lot of hardware) give access to MMIO
register space. This would potentially permit root to trigger arbitrary
DMA, so lock it down by default.

This also implicitly locks down the KDADDIO, KDDELIO, KDENABIO and
KDDISABIO console ioctls.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

arch/x86/kernel/ioport.c | 4 ++--
drivers/char/mem.c | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 9c3cf0944bce..4a613fed94b6 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -30,7 +30,7 @@ asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)

if ((from + num <= from) || (from + num > IO_BITMAP_BITS))
return -EINVAL;
- if (turn_on && !capable(CAP_SYS_RAWIO))
+ if (turn_on && (!capable(CAP_SYS_RAWIO) || kernel_is_locked_down()))
return -EPERM;

/*
@@ -120,7 +120,7 @@ SYSCALL_DEFINE1(iopl, unsigned int, level)
return -EINVAL;
/* Trying to gain more privileges? */
if (level > old) {
- if (!capable(CAP_SYS_RAWIO))
+ if (!capable(CAP_SYS_RAWIO) || kernel_is_locked_down())
return -EPERM;
}
regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) |
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index f8144049bda3..9afebb60550f 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -741,6 +741,8 @@ static loff_t memory_lseek(struct file *file, loff_t offset, int orig)

static int open_port(struct inode *inode, struct file *filp)
{
+ if (kernel_is_locked_down())
+ return -EPERM;
return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
}


2017-04-05 20:15:32

by David Howells

[permalink] [raw]
Subject: [PATCH 05/24] Restrict /dev/mem and /dev/kmem when the kernel is locked down

From: Matthew Garrett <[email protected]>

Allowing users to write to address space makes it possible for the kernel to
be subverted, avoiding module loading restrictions. Prevent this when the
kernel has been locked down.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
---

drivers/char/mem.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 6d9cc2d39d22..f8144049bda3 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -163,6 +163,9 @@ static ssize_t write_mem(struct file *file, const char __user *buf,
if (p != *ppos)
return -EFBIG;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
if (!valid_phys_addr_range(p, count))
return -EFAULT;

@@ -513,6 +516,9 @@ static ssize_t write_kmem(struct file *file, const char __user *buf,
char *kbuf; /* k-addr because vwrite() takes vmlist_lock rwlock */
int err = 0;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
if (p < (unsigned long) high_memory) {
unsigned long to_write = min_t(unsigned long, count,
(unsigned long)high_memory - p);

2017-04-05 20:16:09

by David Howells

[permalink] [raw]
Subject: [PATCH 07/24] kexec: Disable at runtime if the kernel is locked down

From: Matthew Garrett <[email protected]>

kexec permits the loading and execution of arbitrary code in ring 0, which
is something that lock-down is meant to prevent. It makes sense to disable
kexec in this situation.

This does not affect kexec_file_load() which can check for a signature on the
image to be booted.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

kernel/kexec.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index 980936a90ee6..46de8e6b42f4 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -194,6 +194,13 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments,
return -EPERM;

/*
+ * kexec can be used to circumvent module loading restrictions, so
+ * prevent loading in that case
+ */
+ if (kernel_is_locked_down())
+ return -EPERM;
+
+ /*
* Verify we have a legal set of flags
* This leaves us room for future extensions.
*/

2017-04-05 20:15:50

by David Howells

[permalink] [raw]
Subject: [PATCH 06/24] Add a sysrq option to exit secure boot mode

From: Kyle McMartin <[email protected]>

Make sysrq+x exit secure boot mode on x86_64, thereby allowing the running
kernel image to be modified. This lifts the lockdown.

Signed-off-by: Kyle McMartin <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

arch/x86/Kconfig | 10 ++++++++++
arch/x86/kernel/setup.c | 31 +++++++++++++++++++++++++++++++
drivers/input/misc/uinput.c | 1 +
drivers/tty/sysrq.c | 19 +++++++++++++------
include/linux/input.h | 5 +++++
include/linux/sysrq.h | 8 +++++++-
kernel/debug/kdb/kdb_main.c | 2 +-
7 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 21f39855661d..457c04971849 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1829,6 +1829,16 @@ config EFI_SECURE_BOOT_LOCK_DOWN
image. Say Y here to automatically lock down the kernel when a
system boots with UEFI Secure Boot enabled.

+config EFI_ALLOW_SECURE_BOOT_EXIT
+ def_bool n
+ depends on EFI_SECURE_BOOT_LOCK_DOWN && MAGIC_SYSRQ
+ select ALLOW_LOCKDOWN_LIFT
+ prompt "Allow secure boot mode to be exited with SysRq+x on a keyboard"
+ ---help---
+ Allow secure boot mode to be exited and the kernel lockdown lifted by
+ typing SysRq+x on a keyboard attached to the system (not permitted
+ through procfs).
+
config SECCOMP
def_bool y
prompt "Enable seccomp to safely compute untrusted bytecode"
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 85dfa745c442..a415a4817684 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -71,6 +71,11 @@
#include <linux/jiffies.h>
#include <linux/security.h>

+#include <linux/fips.h>
+#include <linux/cred.h>
+#include <linux/sysrq.h>
+#include <linux/init_task.h>
+
#include <video/edid.h>

#include <asm/mtrr.h>
@@ -1330,6 +1335,32 @@ void __init i386_reserve_resources(void)

#endif /* CONFIG_X86_32 */

+#ifdef CONFIG_EFI_ALLOW_SECURE_BOOT_EXIT
+
+static void sysrq_handle_secure_boot(int key)
+{
+ if (!efi_enabled(EFI_SECURE_BOOT))
+ return;
+
+ pr_info("Secure boot disabled\n");
+ lift_kernel_lockdown();
+}
+static struct sysrq_key_op secure_boot_sysrq_op = {
+ .handler = sysrq_handle_secure_boot,
+ .help_msg = "unSB(x)",
+ .action_msg = "Disabling Secure Boot restrictions",
+ .enable_mask = SYSRQ_DISABLE_USERSPACE,
+};
+static int __init secure_boot_sysrq(void)
+{
+ if (efi_enabled(EFI_SECURE_BOOT))
+ register_sysrq_key('x', &secure_boot_sysrq_op);
+ return 0;
+}
+late_initcall(secure_boot_sysrq);
+#endif /*CONFIG_EFI_ALLOW_SECURE_BOOT_EXIT*/
+
+
static struct notifier_block kernel_offset_notifier = {
.notifier_call = dump_kernel_offset
};
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 022be0e22eba..4a054a564636 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -387,6 +387,7 @@ static int uinput_allocate_device(struct uinput_device *udev)
if (!udev->dev)
return -ENOMEM;

+ udev->dev->flags |= INPUTDEV_FLAGS_SYNTHETIC;
udev->dev->event = uinput_dev_event;
input_set_drvdata(udev->dev, udev);

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index c6fc7141d7b2..0c96cf60f1a6 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -481,6 +481,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
/* x: May be registered on mips for TLB dump */
/* x: May be registered on ppc/powerpc for xmon */
/* x: May be registered on sparc64 for global PMU dump */
+ /* x: May be registered on x86_64 for disabling secure boot */
NULL, /* x */
/* y: May be registered on sparc64 for global register dump */
NULL, /* y */
@@ -524,7 +525,7 @@ static void __sysrq_put_key_op(int key, struct sysrq_key_op *op_p)
sysrq_key_table[i] = op_p;
}

-void __handle_sysrq(int key, bool check_mask)
+void __handle_sysrq(int key, unsigned int from)
{
struct sysrq_key_op *op_p;
int orig_log_level;
@@ -544,11 +545,15 @@ void __handle_sysrq(int key, bool check_mask)

op_p = __sysrq_get_key_op(key);
if (op_p) {
+ /* Ban synthetic events from some sysrq functionality */
+ if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) &&
+ op_p->enable_mask & SYSRQ_DISABLE_USERSPACE)
+ printk("This sysrq operation is disabled from userspace.\n");
/*
* Should we check for enabled operations (/proc/sysrq-trigger
* should not) and is the invoked operation enabled?
*/
- if (!check_mask || sysrq_on_mask(op_p->enable_mask)) {
+ if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
pr_cont("%s\n", op_p->action_msg);
console_loglevel = orig_log_level;
op_p->handler(key);
@@ -580,7 +585,7 @@ void __handle_sysrq(int key, bool check_mask)
void handle_sysrq(int key)
{
if (sysrq_on())
- __handle_sysrq(key, true);
+ __handle_sysrq(key, SYSRQ_FROM_KERNEL);
}
EXPORT_SYMBOL(handle_sysrq);

@@ -661,7 +666,7 @@ static void sysrq_do_reset(unsigned long _state)
static void sysrq_handle_reset_request(struct sysrq_state *state)
{
if (state->reset_requested)
- __handle_sysrq(sysrq_xlate[KEY_B], false);
+ __handle_sysrq(sysrq_xlate[KEY_B], SYSRQ_FROM_KERNEL);

if (sysrq_reset_downtime_ms)
mod_timer(&state->keyreset_timer,
@@ -812,8 +817,10 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,

default:
if (sysrq->active && value && value != 2) {
+ int from = sysrq->handle.dev->flags & INPUTDEV_FLAGS_SYNTHETIC ?
+ SYSRQ_FROM_SYNTHETIC : 0;
sysrq->need_reinject = false;
- __handle_sysrq(sysrq_xlate[code], true);
+ __handle_sysrq(sysrq_xlate[code], from);
}
break;
}
@@ -1097,7 +1104,7 @@ static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,

if (get_user(c, buf))
return -EFAULT;
- __handle_sysrq(c, false);
+ __handle_sysrq(c, SYSRQ_FROM_PROC);
}

return count;
diff --git a/include/linux/input.h b/include/linux/input.h
index a65e3b24fb18..8b0357175049 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -42,6 +42,7 @@ struct input_value {
* @phys: physical path to the device in the system hierarchy
* @uniq: unique identification code for the device (if device has it)
* @id: id of the device (struct input_id)
+ * @flags: input device flags (SYNTHETIC, etc.)
* @propbit: bitmap of device properties and quirks
* @evbit: bitmap of types of events supported by the device (EV_KEY,
* EV_REL, etc.)
@@ -124,6 +125,8 @@ struct input_dev {
const char *uniq;
struct input_id id;

+ unsigned int flags;
+
unsigned long propbit[BITS_TO_LONGS(INPUT_PROP_CNT)];

unsigned long evbit[BITS_TO_LONGS(EV_CNT)];
@@ -190,6 +193,8 @@ struct input_dev {
};
#define to_input_dev(d) container_of(d, struct input_dev, dev)

+#define INPUTDEV_FLAGS_SYNTHETIC 0x000000001
+
/*
* Verify that we are in sync with input_device_id mod_devicetable.h #defines
*/
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index 387fa7d05c98..f7c52a9ea394 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -28,6 +28,8 @@
#define SYSRQ_ENABLE_BOOT 0x0080
#define SYSRQ_ENABLE_RTNICE 0x0100

+#define SYSRQ_DISABLE_USERSPACE 0x00010000
+
struct sysrq_key_op {
void (*handler)(int);
char *help_msg;
@@ -42,8 +44,12 @@ struct sysrq_key_op {
* are available -- else NULL's).
*/

+#define SYSRQ_FROM_KERNEL 0x0001
+#define SYSRQ_FROM_PROC 0x0002
+#define SYSRQ_FROM_SYNTHETIC 0x0004
+
void handle_sysrq(int key);
-void __handle_sysrq(int key, bool check_mask);
+void __handle_sysrq(int key, unsigned int from);
int register_sysrq_key(int key, struct sysrq_key_op *op);
int unregister_sysrq_key(int key, struct sysrq_key_op *op);
struct sysrq_key_op *__sysrq_get_key_op(int key);
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index c8146d53ca67..b480cadf9272 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1970,7 +1970,7 @@ static int kdb_sr(int argc, const char **argv)
return KDB_ARGCOUNT;

kdb_trap_printk++;
- __handle_sysrq(*argv[1], check_mask);
+ __handle_sysrq(*argv[1], check_mask ? SYSRQ_FROM_KERNEL : 0);
kdb_trap_printk--;

return 0;

2017-04-05 20:16:25

by David Howells

[permalink] [raw]
Subject: [PATCH 10/24] hibernate: Disable when the kernel is locked down

From: Josh Boyer <[email protected]>

There is currently no way to verify the resume image when returning
from hibernate. This might compromise the signed modules trust model,
so until we can work with signed hibernate images we disable it when the
kernel is locked down.

Signed-off-by: Josh Boyer <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

kernel/power/hibernate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index a8b978c35a6a..50cca5dcb62f 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -70,7 +70,7 @@ static const struct platform_hibernation_ops *hibernation_ops;

bool hibernation_available(void)
{
- return (nohibernate == 0);
+ return nohibernate == 0 && !kernel_is_locked_down();
}

/**

2017-04-05 20:16:59

by David Howells

[permalink] [raw]
Subject: [PATCH 12/24] PCI: Lock down BAR access when the kernel is locked down

From: Matthew Garrett <[email protected]>

Any hardware that can potentially generate DMA has to be locked down in
order to avoid it being possible for an attacker to modify kernel code,
allowing them to circumvent disabled module loading or module signing.
Default to paranoid - in future we can potentially relax this for
sufficiently IOMMU-isolated devices.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

drivers/pci/pci-sysfs.c | 9 +++++++++
drivers/pci/proc.c | 8 +++++++-
drivers/pci/syscall.c | 2 +-
3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 25d010d449a3..f70b3668036f 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -727,6 +727,9 @@ static ssize_t pci_write_config(struct file *filp, struct kobject *kobj,
loff_t init_off = off;
u8 *data = (u8 *) buf;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
if (off > dev->cfg_size)
return 0;
if (off + count > dev->cfg_size) {
@@ -1018,6 +1021,9 @@ static int pci_mmap_resource(struct kobject *kobj, struct bin_attribute *attr,
resource_size_t start, end;
int i;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
for (i = 0; i < PCI_ROM_RESOURCE; i++)
if (res == &pdev->resource[i])
break;
@@ -1117,6 +1123,9 @@ static ssize_t pci_write_resource_io(struct file *filp, struct kobject *kobj,
struct bin_attribute *attr, char *buf,
loff_t off, size_t count)
{
+ if (kernel_is_locked_down())
+ return -EPERM;
+
return pci_resource_io(filp, kobj, attr, buf, off, count, true);
}

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index f82710a8694d..139d6f09ae7b 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -116,6 +116,9 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
int size = dev->cfg_size;
int cnt;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
if (pos >= size)
return 0;
if (nbytes >= size)
@@ -195,6 +198,9 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd,
#endif /* HAVE_PCI_MMAP */
int ret = 0;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
switch (cmd) {
case PCIIOC_CONTROLLER:
ret = pci_domain_nr(dev->bus);
@@ -233,7 +239,7 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma)
struct pci_filp_private *fpriv = file->private_data;
int i, ret, write_combine;

- if (!capable(CAP_SYS_RAWIO))
+ if (!capable(CAP_SYS_RAWIO) || kernel_is_locked_down())
return -EPERM;

/* Make sure the caller is mapping a real resource for this device */
diff --git a/drivers/pci/syscall.c b/drivers/pci/syscall.c
index 9bf993e1f71e..c09524738ceb 100644
--- a/drivers/pci/syscall.c
+++ b/drivers/pci/syscall.c
@@ -92,7 +92,7 @@ SYSCALL_DEFINE5(pciconfig_write, unsigned long, bus, unsigned long, dfn,
u32 dword;
int err = 0;

- if (!capable(CAP_SYS_ADMIN))
+ if (!capable(CAP_SYS_ADMIN) || kernel_is_locked_down())
return -EPERM;

dev = pci_get_bus_and_slot(bus, dfn);

2017-04-05 20:17:07

by David Howells

[permalink] [raw]
Subject: [PATCH 16/24] ACPI: Limit access to custom_method when the kernel is locked down

From: Matthew Garrett <[email protected]>

custom_method effectively allows arbitrary access to system memory, making
it possible for an attacker to circumvent restrictions on module loading.
Disable it if the kernel is locked down.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

drivers/acpi/custom_method.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index c68e72414a67..e4d721c330c0 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -29,6 +29,9 @@ static ssize_t cm_write(struct file *file, const char __user * user_buf,
struct acpi_table_header table;
acpi_status status;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
if (!(*ppos)) {
/* parse the table header to get the table length */
if (count <= sizeof(struct acpi_table_header))

2017-04-05 20:17:04

by David Howells

[permalink] [raw]
Subject: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

From: Matthew Garrett <[email protected]>

We have no way of validating what all of the Asus WMI methods do on a given
machine - and there's a risk that some will allow hardware state to be
manipulated in such a way that arbitrary code can be executed in the
kernel, circumventing module loading restrictions. Prevent that if the
kernel is locked down.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
cc: [email protected]
---

drivers/platform/x86/asus-wmi.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 8fe5890bf539..feef25076813 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1900,6 +1900,9 @@ static int show_dsts(struct seq_file *m, void *data)
int err;
u32 retval = -1;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
err = asus_wmi_get_devstate(asus, asus->debug.dev_id, &retval);

if (err < 0)
@@ -1916,6 +1919,9 @@ static int show_devs(struct seq_file *m, void *data)
int err;
u32 retval = -1;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
err = asus_wmi_set_devstate(asus->debug.dev_id, asus->debug.ctrl_param,
&retval);

@@ -1940,6 +1946,9 @@ static int show_call(struct seq_file *m, void *data)
union acpi_object *obj;
acpi_status status;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID,
1, asus->debug.method_id,
&input, &output);

2017-04-05 20:17:35

by David Howells

[permalink] [raw]
Subject: [PATCH 19/24] acpi: Disable APEI error injection if the kernel is locked down

From: Linn Crosetto <[email protected]>

ACPI provides an error injection mechanism, EINJ, for debugging and testing
the ACPI Platform Error Interface (APEI) and other RAS features. If
supported by the firmware, ACPI specification 5.0 and later provide for a
way to specify a physical memory address to which to inject the error.

Injecting errors through EINJ can produce errors which to the platform are
indistinguishable from real hardware errors. This can have undesirable
side-effects, such as causing the platform to mark hardware as needing
replacement.

While it does not provide a method to load unauthenticated privileged code,
the effect of these errors may persist across reboots and affect trust in
the underlying hardware, so disable error injection through EINJ if
the kernel is locked down.

Signed-off-by: Linn Crosetto <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

drivers/acpi/apei/einj.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index ec50c32ea3da..e082718d01c2 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -518,6 +518,9 @@ static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
int rc;
u64 base_addr, size;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
/* If user manually set "flags", make sure it is legal */
if (flags && (flags &
~(SETWA_FLAGS_APICID|SETWA_FLAGS_MEM|SETWA_FLAGS_PCIE_SBDF)))

2017-04-05 20:17:42

by David Howells

[permalink] [raw]
Subject: [PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down

From: Chun-Yi Lee <[email protected]>

There are some bpf functions can be used to read kernel memory:
bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
private keys in kernel memory (e.g. the hibernation image signing key) to
be read by an eBPF program. Prohibit those functions when the kernel is
locked down.

Signed-off-by: Chun-Yi Lee <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

kernel/trace/bpf_trace.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index cee9802cf3e0..7fde851f207b 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -65,6 +65,11 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
{
int ret;

+ if (kernel_is_locked_down()) {
+ memset(dst, 0, size);
+ return -EPERM;
+ }
+
ret = probe_kernel_read(dst, unsafe_ptr, size);
if (unlikely(ret < 0))
memset(dst, 0, size);
@@ -84,6 +89,9 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
u32, size)
{
+ if (kernel_is_locked_down())
+ return -EPERM;
+
/*
* Ensure we're in user context which is safe for the helper to
* run. This helper has no business in a kthread.
@@ -143,6 +151,9 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
if (fmt[--fmt_size] != 0)
return -EINVAL;

+ if (kernel_is_locked_down())
+ return __trace_printk(1, fmt, 0, 0, 0);
+
/* check format string for allowed specifiers */
for (i = 0; i < fmt_size; i++) {
if ((!isprint(fmt[i]) && !isspace(fmt[i])) || !isascii(fmt[i]))

2017-04-05 20:18:01

by David Howells

[permalink] [raw]
Subject: [PATCH 21/24] scsi: Lock down the eata driver

When the kernel is running in secure boot mode, we lock down the kernel to
prevent userspace from modifying the running kernel image. Whilst this
includes prohibiting access to things like /dev/mem, it must also prevent
access by means of configuring driver modules in such a way as to cause a
device to access or modify the kernel image.

The eata driver takes a single string parameter that contains a slew of
settings, including hardware resource configuration. Prohibit use of the
parameter if the kernel is locked down.

Suggested-by: Alan Cox <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Dario Ballabio <[email protected]>
cc: "James E.J. Bottomley" <[email protected]>
cc: "Martin K. Petersen" <[email protected]>
cc: [email protected]
---

drivers/scsi/eata.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/eata.c b/drivers/scsi/eata.c
index 227dd2c2ec2f..5c036d10c18b 100644
--- a/drivers/scsi/eata.c
+++ b/drivers/scsi/eata.c
@@ -1552,8 +1552,13 @@ static int eata2x_detect(struct scsi_host_template *tpnt)

tpnt->proc_name = "eata2x";

- if (strlen(boot_options))
+ if (strlen(boot_options)) {
+ if (kernel_is_locked_down()) {
+ pr_err("Command line-specified device addresses, irqs and dma channels are not permitted when the kernel is locked down\n");
+ return -EPERM;
+ }
option_setup(boot_options);
+ }

#if defined(MODULE)
/* io_port could have been modified when loading as a module */

2017-04-05 20:18:11

by David Howells

[permalink] [raw]
Subject: [PATCH 23/24] Lock down TIOCSSERIAL

Lock down TIOCSSERIAL as that can be used to change the ioport and irq
settings on a serial port. This only appears to be an issue for the serial
drivers that use the core serial code. All other drivers seem to either
ignore attempts to change port/irq or give an error.

Reported-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Jiri Slaby <[email protected]>
---

drivers/tty/serial/serial_core.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 3fe56894974a..4181b0004de9 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -821,6 +821,12 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
new_flags = new_info->flags;
old_custom_divisor = uport->custom_divisor;

+ if ((change_port || change_irq) && kernel_is_locked_down()) {
+ pr_err("Using TIOCSSERIAL to change device addresses, irqs and dma channels is not permitted when the kernel is locked down\n");
+ retval = -EPERM;
+ goto exit;
+ }
+
if (!capable(CAP_SYS_ADMIN)) {
retval = -EPERM;
if (change_irq || change_port ||

2017-04-05 20:18:26

by David Howells

[permalink] [raw]
Subject: [PATCH 22/24] Prohibit PCMCIA CIS storage when the kernel is locked down

Prohibit replacement of the PCMCIA Card Information Structure when the
kernel is locked down.

Suggested-by: Dominik Brodowski <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

drivers/pcmcia/cistpl.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
index 55ef7d1fd8da..193e4f7b73b1 100644
--- a/drivers/pcmcia/cistpl.c
+++ b/drivers/pcmcia/cistpl.c
@@ -1578,6 +1578,11 @@ static ssize_t pccard_store_cis(struct file *filp, struct kobject *kobj,
struct pcmcia_socket *s;
int error;

+ if (kernel_is_locked_down()) {
+ pr_err("Direct CIS storage isn't permitted when the kernel is locked down\n");
+ return -EPERM;
+ }
+
s = to_socket(container_of(kobj, struct device, kobj));

if (off)

2017-04-05 20:18:37

by David Howells

[permalink] [raw]
Subject: [PATCH 24/24] Lock down module params that specify hardware parameters (eg. ioport)

Provided an annotation for module parameters that specify hardware
parameters (such as io ports, iomem addresses, irqs, dma channels, fixed
dma buffers and other types).

Suggested-by: Alan Cox <[email protected]>
Signed-off-by: David Howells <[email protected]>
---

kernel/params.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index a6d6149c0fe6..04185c5aa929 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -108,13 +108,20 @@ bool parameq(const char *a, const char *b)
return parameqn(a, b, strlen(a)+1);
}

-static void param_check_unsafe(const struct kernel_param *kp)
+static bool param_check_unsafe(const struct kernel_param *kp,
+ const char *doing)
{
if (kp->flags & KERNEL_PARAM_FL_UNSAFE) {
pr_warn("Setting dangerous option %s - tainting kernel\n",
kp->name);
add_taint(TAINT_USER, LOCKDEP_STILL_OK);
}
+
+ if (kp->flags & KERNEL_PARAM_FL_HWPARAM && kernel_is_locked_down()) {
+ pr_err("Command line-specified device addresses, irqs and dma channels are not permitted when the kernel is locked down (%s.%s)\n", doing, kp->name);
+ return false;
+ }
+ return true;
}

static int parse_one(char *param,
@@ -144,8 +151,10 @@ static int parse_one(char *param,
pr_debug("handling %s with %p\n", param,
params[i].ops->set);
kernel_param_lock(params[i].mod);
- param_check_unsafe(&params[i]);
- err = params[i].ops->set(val, &params[i]);
+ if (param_check_unsafe(&params[i], doing))
+ err = params[i].ops->set(val, &params[i]);
+ else
+ err = -EPERM;
kernel_param_unlock(params[i].mod);
return err;
}
@@ -608,6 +617,12 @@ static ssize_t param_attr_show(struct module_attribute *mattr,
return count;
}

+#ifdef CONFIG_MODULES
+#define mod_name(mod) (mod)->name
+#else
+#define mod_name(mod) "unknown"
+#endif
+
/* sysfs always hands a nul-terminated string in buf. We rely on that. */
static ssize_t param_attr_store(struct module_attribute *mattr,
struct module_kobject *mk,
@@ -620,8 +635,10 @@ static ssize_t param_attr_store(struct module_attribute *mattr,
return -EPERM;

kernel_param_lock(mk->mod);
- param_check_unsafe(attribute->param);
- err = attribute->param->ops->set(buf, attribute->param);
+ if (param_check_unsafe(attribute->param, mod_name(mk->mod)))
+ err = attribute->param->ops->set(buf, attribute->param);
+ else
+ err = -EPERM;
kernel_param_unlock(mk->mod);
if (!err)
return len;

2017-04-05 20:19:08

by David Howells

[permalink] [raw]
Subject: [PATCH 18/24] acpi: Disable ACPI table override if the kernel is locked down

From: Linn Crosetto <[email protected]>

>From the kernel documentation (initrd_table_override.txt):

If the ACPI_INITRD_TABLE_OVERRIDE compile option is true, it is possible
to override nearly any ACPI table provided by the BIOS with an
instrumented, modified one.

When securelevel is set, the kernel should disallow any unauthenticated
changes to kernel space. ACPI tables contain code invoked by the kernel,
so do not allow ACPI tables to be overridden if the kernel is locked down.

Signed-off-by: Linn Crosetto <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

drivers/acpi/tables.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 2604189d6cd1..601096d0d849 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -542,6 +542,11 @@ void __init acpi_table_upgrade(void)
if (table_nr == 0)
return;

+ if (kernel_is_locked_down()) {
+ pr_notice("kernel is locked down, ignoring table override\n");
+ return;
+ }
+
acpi_tables_addr =
memblock_find_in_range(0, ACPI_TABLE_UPGRADE_MAX_PHYS,
all_tables_size, PAGE_SIZE);

2017-04-05 20:19:20

by David Howells

[permalink] [raw]
Subject: [PATCH 17/24] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down

From: Josh Boyer <[email protected]>

This option allows userspace to pass the RSDP address to the kernel, which
makes it possible for a user to circumvent any restrictions imposed on
loading modules. Ignore the option when the kernel is locked down.

Signed-off-by: Josh Boyer <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

drivers/acpi/osl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index db78d353bab1..d4d4ba348451 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -192,7 +192,7 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
acpi_physical_address pa = 0;

#ifdef CONFIG_KEXEC
- if (acpi_rsdp)
+ if (acpi_rsdp && !kernel_is_locked_down())
return acpi_rsdp;
#endif


2017-04-05 20:16:40

by David Howells

[permalink] [raw]
Subject: [PATCH 11/24] uswsusp: Disable when the kernel is locked down

From: Matthew Garrett <[email protected]>

uswsusp allows a user process to dump and then restore kernel state, which
makes it possible to modify the running kernel. Disable this if the kernel
is locked down.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

kernel/power/user.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/kernel/power/user.c b/kernel/power/user.c
index 22df9f7ff672..e4b926d329b7 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -52,6 +52,9 @@ static int snapshot_open(struct inode *inode, struct file *filp)
if (!hibernation_available())
return -EPERM;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
lock_system_sleep();

if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {

2017-04-05 20:20:00

by David Howells

[permalink] [raw]
Subject: [PATCH 14/24] x86: Restrict MSR access when the kernel is locked down

From: Matthew Garrett <[email protected]>

Writing to MSRs should not be allowed if the kernel is locked down, since
it could lead to execution of arbitrary code in kernel mode. Based on a
patch by Kees Cook.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: Kees Cook <[email protected]>
cc: [email protected]
---

arch/x86/kernel/msr.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
index ef688804f80d..fbcce028e502 100644
--- a/arch/x86/kernel/msr.c
+++ b/arch/x86/kernel/msr.c
@@ -84,6 +84,9 @@ static ssize_t msr_write(struct file *file, const char __user *buf,
int err = 0;
ssize_t bytes = 0;

+ if (kernel_is_locked_down())
+ return -EPERM;
+
if (count % 8)
return -EINVAL; /* Invalid chunk size */

@@ -131,6 +134,10 @@ static long msr_ioctl(struct file *file, unsigned int ioc, unsigned long arg)
err = -EBADF;
break;
}
+ if (kernel_is_locked_down()) {
+ err = -EPERM;
+ break;
+ }
if (copy_from_user(&regs, uregs, sizeof regs)) {
err = -EFAULT;
break;

2017-04-05 20:20:29

by David Howells

[permalink] [raw]
Subject: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

From: Chun-Yi Lee <[email protected]>

When KEXEC_VERIFY_SIG is not enabled, kernel should not loads image
through kexec_file systemcall if securelevel has been set.

This code was showed in Matthew's patch but not in git:
https://lkml.org/lkml/2015/3/13/778

Cc: Matthew Garrett <[email protected]>
Signed-off-by: Chun-Yi Lee <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---

kernel/kexec_file.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b118735fea9d..f6937eecd1eb 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -268,6 +268,12 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
return -EPERM;

+ /* Don't permit images to be loaded into trusted kernels if we're not
+ * going to verify the signature on them
+ */
+ if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
+ return -EPERM;
+
/* Make sure we have a legal set of flags */
if (flags != (flags & KEXEC_FILE_FLAGS))
return -EINVAL;

2017-04-05 20:21:13

by David Howells

[permalink] [raw]
Subject: [PATCH 04/24] Enforce module signatures if the kernel is locked down

If the kernel is locked down, require that all modules have valid
signatures that we can verify.

Signed-off-by: David Howells <[email protected]>
---

kernel/module.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/module.c b/kernel/module.c
index 7eba6dea4f41..3331f2eb9b93 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2756,7 +2756,7 @@ static int module_sig_check(struct load_info *info, int flags)
}

/* Not having a signature is only an error if we're strict. */
- if (err == -ENOKEY && !sig_enforce)
+ if (err == -ENOKEY && !sig_enforce && !kernel_is_locked_down())
err = 0;

return err;

2017-04-05 23:38:21

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 11/24] uswsusp: Disable when the kernel is locked down

On Wed, Apr 5, 2017 at 10:16 PM, David Howells <[email protected]> wrote:
> From: Matthew Garrett <[email protected]>
>
> uswsusp allows a user process to dump and then restore kernel state, which
> makes it possible to modify the running kernel. Disable this if the kernel
> is locked down.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]

You probably want to disable hibernation altogether in this case.

Thanks,
Rafael

2017-04-06 06:41:26

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH 11/24] uswsusp: Disable when the kernel is locked down

Am Donnerstag, den 06.04.2017, 01:38 +0200 schrieb Rafael J. Wysocki:
> On Wed, Apr 5, 2017 at 10:16 PM, David Howells <[email protected]> wrote:
> >
> > From: Matthew Garrett <[email protected]>
> >
> > uswsusp allows a user process to dump and then restore kernel state, which
> > makes it possible to modify the running kernel. Disable this if the kernel
> > is locked down.
> >
> > Signed-off-by: Matthew Garrett <[email protected]>
> > Signed-off-by: David Howells <[email protected]>
> > cc: [email protected]
>
> You probably want to disable hibernation altogether in this case.

Your swap partition may be located on an NVDIMM or be encrypted.
Isn't this a bit overly drastic?

Regards
Oliver

2017-04-06 06:55:22

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 11/24] uswsusp: Disable when the kernel is locked down

Rafael J. Wysocki <[email protected]> wrote:

> You probably want to disable hibernation altogether in this case.

See patch 10. Does that mean patch 11 is superfluous?

David

2017-04-06 08:27:01

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 01/24] efi: Add EFI_SECURE_BOOT bit

On 5 April 2017 at 21:14, David Howells <[email protected]> wrote:
> From: Josh Boyer <[email protected]>
>
> UEFI machines can be booted in Secure Boot mode. Add a EFI_SECURE_BOOT bit
> that can be passed to efi_enabled() to find out whether secure boot is
> enabled.
>
> This will be used by the SysRq+x handler, registered by the x86 arch, to find
> out whether secure boot mode is enabled so that it can be disabled.
>
> Signed-off-by: Josh Boyer <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]
> ---
>
> arch/x86/kernel/setup.c | 1 +
> include/linux/efi.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 4bf0c8926a1c..396285bddb93 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1184,6 +1184,7 @@ void __init setup_arch(char **cmdline_p)
> pr_info("Secure boot disabled\n");
> break;
> case efi_secureboot_mode_enabled:
> + set_bit(EFI_SECURE_BOOT, &efi.flags);
> pr_info("Secure boot enabled\n");
> break;
> default:

Like I asked when this patch was sent round the last time: is there
any reason for this not to live in generic code?


> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 94d34e0be24f..6049600e5475 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1069,6 +1069,7 @@ extern int __init efi_setup_pcdp_console(char *);
> #define EFI_DBG 8 /* Print additional debug info at runtime */
> #define EFI_NX_PE_DATA 9 /* Can runtime data regions be mapped non-executable? */
> #define EFI_MEM_ATTR 10 /* Did firmware publish an EFI_MEMORY_ATTRIBUTES table? */
> +#define EFI_SECURE_BOOT 11 /* Are we in Secure Boot mode? */
>
> #ifdef CONFIG_EFI
> /*
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2017-04-06 08:48:38

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 01/24] efi: Add EFI_SECURE_BOOT bit

Ard Biesheuvel <[email protected]> wrote:

> > @@ -1184,6 +1184,7 @@ void __init setup_arch(char **cmdline_p)
> > pr_info("Secure boot disabled\n");
> > break;
> > case efi_secureboot_mode_enabled:
> > + set_bit(EFI_SECURE_BOOT, &efi.flags);
> > pr_info("Secure boot enabled\n");
> > break;
> > default:
>
> Like I asked when this patch was sent round the last time: is there
> any reason for this not to live in generic code?

It's interpreting the x86 boot_params at this point. I suppose I could move
the following piece:

if (efi_enabled(EFI_BOOT)) {
switch (boot_params.secure_boot) {
case efi_secureboot_mode_disabled:
pr_info("Secure boot disabled\n");
break;
case efi_secureboot_mode_enabled:
pr_info("Secure boot enabled\n");
break;
default:
pr_info("Secure boot could not be determined\n");
break;
}
}

into generic code and pass in boot_params.secure_boot as an argument (since
that's x86-specific I believe. Any suggestions as to where? The same file as
efi_get_secureboot() would seem to be the wrong place since that gets linked
into the bootwrapper. I could put it into drivers/firmware/efi/secure_boot.c
and make that conditional in the Makefile.

David

2017-04-06 09:28:11

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 11/24] uswsusp: Disable when the kernel is locked down

Oliver Neukum <[email protected]> wrote:

> Your swap partition may be located on an NVDIMM or be encrypted.

An NVDIMM should be considered the same as any other persistent storage.

It may be encrypted, but where's the key stored, how easy is it to retrieve
and does the swapout code know this?

> Isn't this a bit overly drastic?

Perhaps, but if it's on disk and it's not encrypted, then maybe not.

David

2017-04-06 12:29:21

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down

On Wed, Apr 05, 2017 at 09:17:25PM +0100, David Howells wrote:
> From: Chun-Yi Lee <[email protected]>
>
> There are some bpf functions can be used to read kernel memory:
> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
> private keys in kernel memory (e.g. the hibernation image signing key) to
> be read by an eBPF program. Prohibit those functions when the kernel is
> locked down.
>
> Signed-off-by: Chun-Yi Lee <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]
> ---
>
> kernel/trace/bpf_trace.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index cee9802cf3e0..7fde851f207b 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -65,6 +65,11 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
> {
> int ret;
>
> + if (kernel_is_locked_down()) {
> + memset(dst, 0, size);
> + return -EPERM;
> + }

this will obviously break the program. How about disabling loading tracing
programs during the lockdown completely?

Also is there a description of what this lockdown trying to accomplish?
The cover letter is scarce in details.

2017-04-06 12:40:27

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down

On 6 April 2017 at 13:29, Alexei Starovoitov
<[email protected]> wrote:
> On Wed, Apr 05, 2017 at 09:17:25PM +0100, David Howells wrote:
>> From: Chun-Yi Lee <[email protected]>
>>
>> There are some bpf functions can be used to read kernel memory:
>> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
>> private keys in kernel memory (e.g. the hibernation image signing key) to
>> be read by an eBPF program. Prohibit those functions when the kernel is
>> locked down.
>>
>> Signed-off-by: Chun-Yi Lee <[email protected]>
>> Signed-off-by: David Howells <[email protected]>
>> cc: [email protected]
>> ---
>>
>> kernel/trace/bpf_trace.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
>> index cee9802cf3e0..7fde851f207b 100644
>> --- a/kernel/trace/bpf_trace.c
>> +++ b/kernel/trace/bpf_trace.c
>> @@ -65,6 +65,11 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
>> {
>> int ret;
>>
>> + if (kernel_is_locked_down()) {
>> + memset(dst, 0, size);
>> + return -EPERM;
>> + }
>
> this will obviously break the program. How about disabling loading tracing
> programs during the lockdown completely?
>
> Also is there a description of what this lockdown trying to accomplish?
> The cover letter is scarce in details.
>

This is a very good point, and this is actually feedback that was
given (by Alan Cox, iirc) the last time this series was circulated.

This series is a mixed bag of patches that all look like they improve
'security' in one way or the other. But what is lacking is a coherent
view on the threat model, and to what extent all these patches reduce
the vulnerability to such threats. Without that, these patches do very
little beyond giving a false sense of security, imo.

2017-04-06 19:44:04

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 17/24] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down

On Wed, Apr 5, 2017 at 10:16 PM, David Howells <[email protected]> wrote:
> From: Josh Boyer <[email protected]>
>
> This option allows userspace to pass the RSDP address to the kernel, which
> makes it possible for a user to circumvent any restrictions imposed on
> loading modules. Ignore the option when the kernel is locked down.

I'm not really sure here.

What exactly is the mechanism?

Thanks,
Rafael

2017-04-06 20:07:59

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 11/24] uswsusp: Disable when the kernel is locked down

On Thu, Apr 6, 2017 at 8:55 AM, David Howells <[email protected]> wrote:
> Rafael J. Wysocki <[email protected]> wrote:
>
>> You probably want to disable hibernation altogether in this case.
>
> See patch 10. Does that mean patch 11 is superfluous?

Yes, it does.

You can't open /dev/snapshot if hibernation_available() returns false.

Thanks,
Rafael

2017-04-06 20:09:55

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 11/24] uswsusp: Disable when the kernel is locked down

On Thu, Apr 6, 2017 at 10:41 AM, David Howells <[email protected]> wrote:
> Oliver Neukum <[email protected]> wrote:
>
>> Your swap partition may be located on an NVDIMM or be encrypted.
>
> An NVDIMM should be considered the same as any other persistent storage.
>
> It may be encrypted, but where's the key stored, how easy is it to retrieve
> and does the swapout code know this?
>
>> Isn't this a bit overly drastic?
>
> Perhaps, but if it's on disk and it's not encrypted, then maybe not.

Right.

Swap encryption is not mandatory and I'm not sure how the hibernate
code can verify whether or not it is in use.

Thanks,
Rafael

2017-04-06 20:12:47

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 11/24] uswsusp: Disable when the kernel is locked down

On Thu, Apr 6, 2017 at 10:09 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Thu, Apr 6, 2017 at 10:41 AM, David Howells <[email protected]> wrote:
>> Oliver Neukum <[email protected]> wrote:
>>
>>> Your swap partition may be located on an NVDIMM or be encrypted.
>>
>> An NVDIMM should be considered the same as any other persistent storage.
>>
>> It may be encrypted, but where's the key stored, how easy is it to retrieve
>> and does the swapout code know this?
>>
>>> Isn't this a bit overly drastic?
>>
>> Perhaps, but if it's on disk and it's not encrypted, then maybe not.
>
> Right.
>
> Swap encryption is not mandatory and I'm not sure how the hibernate
> code can verify whether or not it is in use.

BTW, SUSE has patches adding secure boot support to the hibernate code
and Jiri promised me to post them last year even. :-)

Thanks,
Rafael

2017-04-06 20:25:50

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH 11/24] uswsusp: Disable when the kernel is locked down

On Thu, 6 Apr 2017, Rafael J. Wysocki wrote:

> >>> Your swap partition may be located on an NVDIMM or be encrypted.
> >>
> >> An NVDIMM should be considered the same as any other persistent storage.
> >>
> >> It may be encrypted, but where's the key stored, how easy is it to retrieve
> >> and does the swapout code know this?
> >>
> >>> Isn't this a bit overly drastic?
> >>
> >> Perhaps, but if it's on disk and it's not encrypted, then maybe not.
> >
> > Right.
> >
> > Swap encryption is not mandatory and I'm not sure how the hibernate
> > code can verify whether or not it is in use.
>
> BTW, SUSE has patches adding secure boot support to the hibernate code
> and Jiri promised me to post them last year even. :-)

Oh, thanks for a friendly ping :) Adding Joey Lee to CC.

--
Jiri Kosina
SUSE Labs

2017-04-07 03:06:05

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

On 04/05/17 at 09:15pm, David Howells wrote:
> From: Chun-Yi Lee <[email protected]>
>
> When KEXEC_VERIFY_SIG is not enabled, kernel should not loads image
> through kexec_file systemcall if securelevel has been set.
>
> This code was showed in Matthew's patch but not in git:
> https://lkml.org/lkml/2015/3/13/778
>
> Cc: Matthew Garrett <[email protected]>
> Signed-off-by: Chun-Yi Lee <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]
> ---
>
> kernel/kexec_file.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index b118735fea9d..f6937eecd1eb 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -268,6 +268,12 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
> return -EPERM;
>
> + /* Don't permit images to be loaded into trusted kernels if we're not
> + * going to verify the signature on them
> + */
> + if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
> + return -EPERM;
> +
> /* Make sure we have a legal set of flags */
> if (flags != (flags & KEXEC_FILE_FLAGS))
> return -EINVAL;
>
>
> _______________________________________________
> kexec mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec

Acked-by: Dave Young <[email protected]>

Thanks
Dave

2017-04-07 03:07:21

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH 07/24] kexec: Disable at runtime if the kernel is locked down

On 04/05/17 at 09:15pm, David Howells wrote:
> From: Matthew Garrett <[email protected]>
>
> kexec permits the loading and execution of arbitrary code in ring 0, which
> is something that lock-down is meant to prevent. It makes sense to disable
> kexec in this situation.
>
> This does not affect kexec_file_load() which can check for a signature on the
> image to be booted.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]
> ---
>
> kernel/kexec.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 980936a90ee6..46de8e6b42f4 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -194,6 +194,13 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments,
> return -EPERM;
>
> /*
> + * kexec can be used to circumvent module loading restrictions, so
> + * prevent loading in that case
> + */
> + if (kernel_is_locked_down())
> + return -EPERM;
> +
> + /*
> * Verify we have a legal set of flags
> * This leaves us room for future extensions.
> */
>
>
> _______________________________________________
> kexec mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec

Acked-by: Dave Young <[email protected]>

Thanks
Dave

2017-04-07 03:49:33

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

On Fri, 2017-04-07 at 11:05 +0800, Dave Young wrote:
> On 04/05/17 at 09:15pm, David Howells wrote:
> > From: Chun-Yi Lee <[email protected]>
> >
> > When KEXEC_VERIFY_SIG is not enabled, kernel should not loads image
> > through kexec_file systemcall if securelevel has been set.
> >
> > This code was showed in Matthew's patch but not in git:
> > https://lkml.org/lkml/2015/3/13/778
> >
> > Cc: Matthew Garrett <[email protected]>
> > Signed-off-by: Chun-Yi Lee <[email protected]>
> > Signed-off-by: David Howells <[email protected]>
> > cc: [email protected]
> > ---
> >
> > kernel/kexec_file.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > index b118735fea9d..f6937eecd1eb 100644
> > --- a/kernel/kexec_file.c
> > +++ b/kernel/kexec_file.c
> > @@ -268,6 +268,12 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
> > return -EPERM;
> >
> > + /* Don't permit images to be loaded into trusted kernels if we're not
> > + * going to verify the signature on them
> > + */
> > + if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
> > + return -EPERM;
> > +
> >

IMA can be used to verify file signatures too, based on the LSM hooks
in  kernel_read_file_from_fd().  CONFIG_KEXEC_VERIFY_SIG should not be
required.

Mimi


> /* Make sure we have a legal set of flags */
> > if (flags != (flags & KEXEC_FILE_FLAGS))
> > return -EINVAL;
> >
> >
> > _______________________________________________
> > kexec mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/kexec
>
> Acked-by: Dave Young <[email protected]>
>
> Thanks
> Dave
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2017-04-07 06:19:55

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

On 04/06/17 at 11:49pm, Mimi Zohar wrote:
> On Fri, 2017-04-07 at 11:05 +0800, Dave Young wrote:
> > On 04/05/17 at 09:15pm, David Howells wrote:
> > > From: Chun-Yi Lee <[email protected]>
> > >
> > > When KEXEC_VERIFY_SIG is not enabled, kernel should not loads image
> > > through kexec_file systemcall if securelevel has been set.
> > >
> > > This code was showed in Matthew's patch but not in git:
> > > https://lkml.org/lkml/2015/3/13/778
> > >
> > > Cc: Matthew Garrett <[email protected]>
> > > Signed-off-by: Chun-Yi Lee <[email protected]>
> > > Signed-off-by: David Howells <[email protected]>
> > > cc: [email protected]
> > > ---
> > >
> > > kernel/kexec_file.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > > index b118735fea9d..f6937eecd1eb 100644
> > > --- a/kernel/kexec_file.c
> > > +++ b/kernel/kexec_file.c
> > > @@ -268,6 +268,12 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > > if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
> > > return -EPERM;
> > >
> > > + /* Don't permit images to be loaded into trusted kernels if we're not
> > > + * going to verify the signature on them
> > > + */
> > > + if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
> > > + return -EPERM;
> > > +
> > >
>
> IMA can be used to verify file signatures too, based on the LSM hooks
> in ?kernel_read_file_from_fd(). ?CONFIG_KEXEC_VERIFY_SIG should not be
> required.

Mimi, I remember we talked somthing before about the two signature
verification. One can change IMA policy in initramfs userspace,
also there are kernel cmdline param to disable IMA, so it can break the
lockdown? Suppose kexec boot with ima disabled cmdline param and then
kexec reboot again..

>
> Mimi
>
>
> > /* Make sure we have a legal set of flags */
> > > if (flags != (flags & KEXEC_FILE_FLAGS))
> > > return -EINVAL;
> > >
> > >
> > > _______________________________________________
> > > kexec mailing list
> > > [email protected]
> > > http://lists.infradead.org/mailman/listinfo/kexec
> >
> > Acked-by: Dave Young <[email protected]>
> >
> > Thanks
> > Dave
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>

2017-04-07 06:31:28

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH 17/24] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down

On 04/06/17 at 09:43pm, Rafael J. Wysocki wrote:
> On Wed, Apr 5, 2017 at 10:16 PM, David Howells <[email protected]> wrote:
> > From: Josh Boyer <[email protected]>
> >
> > This option allows userspace to pass the RSDP address to the kernel, which
> > makes it possible for a user to circumvent any restrictions imposed on
> > loading modules. Ignore the option when the kernel is locked down.
>
> I'm not really sure here.
>
> What exactly is the mechanism?

Actually this acpi_rsdp param is created for EFI kexec reboot in old
days when we had not supported persistent efi vm space across kexec
reboot. At that time kexec reboot runs as noefi mode, it can not find
the acpi root table thus kernel will hang early.

Now kexec can support EFI boot so this param is not necessary for most
user unless they still use efi=old_map.

>
> Thanks,
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2017-04-07 07:05:42

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 17/24] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down

Dave Young <[email protected]> wrote:

> > > This option allows userspace to pass the RSDP address to the kernel, which
> > > makes it possible for a user to circumvent any restrictions imposed on
> > > loading modules. Ignore the option when the kernel is locked down.
> >
> > I'm not really sure here.
> >
> > What exactly is the mechanism?
>
> Actually this acpi_rsdp param is created for EFI kexec reboot in old
> days when we had not supported persistent efi vm space across kexec
> reboot. At that time kexec reboot runs as noefi mode, it can not find
> the acpi root table thus kernel will hang early.
>
> Now kexec can support EFI boot so this param is not necessary for most
> user unless they still use efi=old_map.

Is this patch now unnecessary?

David

2017-04-07 07:08:08

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

Dave Young <[email protected]> wrote:

> > > > + /* Don't permit images to be loaded into trusted kernels if we're not
> > > > + * going to verify the signature on them
> > > > + */
> > > > + if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
> > > > + return -EPERM;
> > > > +
> > > >
> >
> > IMA can be used to verify file signatures too, based on the LSM hooks
> > in  kernel_read_file_from_fd().  CONFIG_KEXEC_VERIFY_SIG should not be
> > required.
>
> Mimi, I remember we talked somthing before about the two signature
> verification. One can change IMA policy in initramfs userspace,
> also there are kernel cmdline param to disable IMA, so it can break the
> lockdown? Suppose kexec boot with ima disabled cmdline param and then
> kexec reboot again..

I guess I should lock down the parameter to disable IMA too.

David

2017-04-07 07:10:11

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

Mimi Zohar <[email protected]> wrote:

> > > + if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
> > > + return -EPERM;
> > > +
> > >
>
> IMA can be used to verify file signatures too, based on the LSM hooks
> in  kernel_read_file_from_fd().  CONFIG_KEXEC_VERIFY_SIG should not be
> required.

Okay, fair enough. I can stick in an OR with an IS_ENABLED on some IMA
symbol. CONFIG_IMA_KEXEC maybe? And also require IMA be enabled?

David

2017-04-07 07:40:24

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH 17/24] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down

On 04/07/17 at 08:05am, David Howells wrote:
> Dave Young <[email protected]> wrote:
>
> > > > This option allows userspace to pass the RSDP address to the kernel, which
> > > > makes it possible for a user to circumvent any restrictions imposed on
> > > > loading modules. Ignore the option when the kernel is locked down.
> > >
> > > I'm not really sure here.
> > >
> > > What exactly is the mechanism?
> >
> > Actually this acpi_rsdp param is created for EFI kexec reboot in old
> > days when we had not supported persistent efi vm space across kexec
> > reboot. At that time kexec reboot runs as noefi mode, it can not find
> > the acpi root table thus kernel will hang early.
> >
> > Now kexec can support EFI boot so this param is not necessary for most
> > user unless they still use efi=old_map.
>
> Is this patch now unnecessary?

I think it is still necessary because the acpi_rsdp kernel param is still
a valid paramater and one can still pass a pointer to be recognized as acpi
root pointer.

Maybe "imposed on loading modules" is not clear which can be dropped.

Thanks
Dave

2017-04-07 07:42:30

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

On 04/07/17 at 08:07am, David Howells wrote:
> Dave Young <[email protected]> wrote:
>
> > > > > + /* Don't permit images to be loaded into trusted kernels if we're not
> > > > > + * going to verify the signature on them
> > > > > + */
> > > > > + if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
> > > > > + return -EPERM;
> > > > > +
> > > > >
> > >
> > > IMA can be used to verify file signatures too, based on the LSM hooks
> > > in ?kernel_read_file_from_fd(). ?CONFIG_KEXEC_VERIFY_SIG should not be
> > > required.
> >
> > Mimi, I remember we talked somthing before about the two signature
> > verification. One can change IMA policy in initramfs userspace,
> > also there are kernel cmdline param to disable IMA, so it can break the
> > lockdown? Suppose kexec boot with ima disabled cmdline param and then
> > kexec reboot again..
>
> I guess I should lock down the parameter to disable IMA too.

That is one thing, user can change IMA policy in initramfs userspace,
I'm not sure if IMA enforce the signed policy now, if no it will be also
a problem.

Thanks
Dave

2017-04-07 07:45:28

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

On Fri, 2017-04-07 at 14:19 +0800, Dave Young wrote:
> On 04/06/17 at 11:49pm, Mimi Zohar wrote:
> > On Fri, 2017-04-07 at 11:05 +0800, Dave Young wrote:
> > > On 04/05/17 at 09:15pm, David Howells wrote:
> > > > From: Chun-Yi Lee <[email protected]>
> > > >
> > > > When KEXEC_VERIFY_SIG is not enabled, kernel should not loads image
> > > > through kexec_file systemcall if securelevel has been set.
> > > >
> > > > This code was showed in Matthew's patch but not in git:
> > > > https://lkml.org/lkml/2015/3/13/778

I specifically checked to make sure that either kexec_file() signature
verification was acceptable and would have commented then, if it had
not been included.

> > > > Cc: Matthew Garrett <[email protected]>
> > > > Signed-off-by: Chun-Yi Lee <[email protected]>
> > > > Signed-off-by: David Howells <[email protected]>
> > > > cc: [email protected]
> > > > ---
> > > >
> > > > kernel/kexec_file.c | 6 ++++++
> > > > 1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > > > index b118735fea9d..f6937eecd1eb 100644
> > > > --- a/kernel/kexec_file.c
> > > > +++ b/kernel/kexec_file.c
> > > > @@ -268,6 +268,12 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > > > if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
> > > > return -EPERM;
> > > >
> > > > + /* Don't permit images to be loaded into trusted kernels if we're not
> > > > + * going to verify the signature on them
> > > > + */
> > > > + if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
> > > > + return -EPERM;
> > > > +
> > > >
> >
> > IMA can be used to verify file signatures too, based on the LSM hooks
> > in  kernel_read_file_from_fd().  CONFIG_KEXEC_VERIFY_SIG should not be
> > required.
>
> Mimi, I remember we talked somthing before about the two signature
> verification. One can change IMA policy in initramfs userspace,
> also there are kernel cmdline param to disable IMA, so it can break the
> lockdown? Suppose kexec boot with ima disabled cmdline param and then
> kexec reboot again..

Right, we discussed that the same method of measuring the kexec image
and initramfs, for extending trusted boot to the OS, could also be
used for verifying the kexec image and initramfs signatures, for
extending secure boot to the OS.  The file hash would be calculated
once for both.

All of your concerns could be addressed with very minor changes to
IMA.  (Continued in response to David.)

> >
> > > /* Make sure we have a legal set of flags */
> > > > if (flags != (flags & KEXEC_FILE_FLAGS))
> > > > return -EINVAL;
> > > >
> > > >
> > > > _______________________________________________
> > > > kexec mailing list
> > > > [email protected]
> > > > http://lists.infradead.org/mailman/listinfo/kexec
> > >
> > > Acked-by: Dave Young <[email protected]>
> > >
> > > Thanks
> > > Dave
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> > > the body of a message to [email protected]
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> >
>

2017-04-07 07:46:46

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

On Fri, 2017-04-07 at 08:09 +0100, David Howells wrote:
> Mimi Zohar <[email protected]> wrote:
>
> > > > + if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
> > > > + return -EPERM;
> > > > +
> > > >
> >
> > IMA can be used to verify file signatures too, based on the LSM hooks
> > in  kernel_read_file_from_fd().  CONFIG_KEXEC_VERIFY_SIG should not be
> > required.
>
> Okay, fair enough. I can stick in an OR with an IS_ENABLED on some IMA
> symbol. CONFIG_IMA_KEXEC maybe? And also require IMA be enabled?

Not quite, since as Dave pointed out, IMA is policy driven.  As a
policy is installed, we could set a flag.

Mimi

2017-04-07 08:02:04

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

On 04/07/17 at 03:45am, Mimi Zohar wrote:
> On Fri, 2017-04-07 at 14:19 +0800, Dave Young wrote:
> > On 04/06/17 at 11:49pm, Mimi Zohar wrote:
> > > On Fri, 2017-04-07 at 11:05 +0800, Dave Young wrote:
> > > > On 04/05/17 at 09:15pm, David Howells wrote:
> > > > > From: Chun-Yi Lee <[email protected]>
> > > > >
> > > > > When KEXEC_VERIFY_SIG is not enabled, kernel should not loads image
> > > > > through kexec_file systemcall if securelevel has been set.
> > > > >
> > > > > This code was showed in Matthew's patch but not in git:
> > > > > https://lkml.org/lkml/2015/3/13/778
>
> I specifically checked to make sure that either kexec_file() signature
> verification was acceptable and would have commented then, if it had
> not been included.
>
> > > > > Cc: Matthew Garrett <[email protected]>
> > > > > Signed-off-by: Chun-Yi Lee <[email protected]>
> > > > > Signed-off-by: David Howells <[email protected]>
> > > > > cc: [email protected]
> > > > > ---
> > > > >
> > > > > kernel/kexec_file.c | 6 ++++++
> > > > > 1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > > > > index b118735fea9d..f6937eecd1eb 100644
> > > > > --- a/kernel/kexec_file.c
> > > > > +++ b/kernel/kexec_file.c
> > > > > @@ -268,6 +268,12 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int, initrd_fd,
> > > > > if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
> > > > > return -EPERM;
> > > > >
> > > > > + /* Don't permit images to be loaded into trusted kernels if we're not
> > > > > + * going to verify the signature on them
> > > > > + */
> > > > > + if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
> > > > > + return -EPERM;
> > > > > +
> > > > >
> > >
> > > IMA can be used to verify file signatures too, based on the LSM hooks
> > > in ?kernel_read_file_from_fd(). ?CONFIG_KEXEC_VERIFY_SIG should not be
> > > required.
> >
> > Mimi, I remember we talked somthing before about the two signature
> > verification. One can change IMA policy in initramfs userspace,
> > also there are kernel cmdline param to disable IMA, so it can break the
> > lockdown? Suppose kexec boot with ima disabled cmdline param and then
> > kexec reboot again..
>
> Right, we discussed that the same method of measuring the kexec image
> and initramfs, for extending trusted boot to the OS, could also be
> used for verifying the kexec image and initramfs signatures, for
> extending secure boot to the OS. ?The file hash would be calculated
> once for both.
>
> All of your concerns could be addressed with very minor changes to
> IMA. ?(Continued in response to David.)

Thanks! As long as IMA can ensure not breaking the lockdown it should be
fine to add an check for either !CONFIG_KEXEC_VERIFY_SIG or !IMA
enforced.

>
> > >
> > > > /* Make sure we have a legal set of flags */
> > > > > if (flags != (flags & KEXEC_FILE_FLAGS))
> > > > > return -EINVAL;
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > kexec mailing list
> > > > > [email protected]
> > > > > http://lists.infradead.org/mailman/listinfo/kexec
> > > >
> > > > Acked-by: Dave Young <[email protected]>
> > > >
> > > > Thanks
> > > > Dave
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> > > > the body of a message to [email protected]
> > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > >
> > >
> >
>

2017-04-07 08:28:34

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

On Fri, 2017-04-07 at 15:41 +0800, Dave Young wrote:
> On 04/07/17 at 08:07am, David Howells wrote:
> > Dave Young <[email protected]> wrote:
> >
> > > > > > + /* Don't permit images to be loaded into trusted kernels if we're not
> > > > > > + * going to verify the signature on them
> > > > > > + */
> > > > > > + if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
> > > > > > + return -EPERM;
> > > > > > +
> > > > > >
> > > >
> > > > IMA can be used to verify file signatures too, based on the LSM hooks
> > > > in  kernel_read_file_from_fd().  CONFIG_KEXEC_VERIFY_SIG should not be
> > > > required.
> > >
> > > Mimi, I remember we talked somthing before about the two signature
> > > verification. One can change IMA policy in initramfs userspace,
> > > also there are kernel cmdline param to disable IMA, so it can break the
> > > lockdown? Suppose kexec boot with ima disabled cmdline param and then
> > > kexec reboot again..
> >
> > I guess I should lock down the parameter to disable IMA too.
>
> That is one thing, user can change IMA policy in initramfs userspace,
> I'm not sure if IMA enforce the signed policy now, if no it will be also
> a problem.

I'm not sure how this relates to the question of whether IMA verifies
the kexec kernel image signature, as the test would not be based on a
Kconfig option, but on a runtime variable.

To answer your question, the rule for requiring the policy to be
signed is:  appraise func=POLICY_CHECK appraise_type=imasig

When the ability to append rules is Kconfig enabled, the builtin
policy requires the new policy or additional rules to be signed.
 Unfortunately, always requiring the policy to be signed, would have
broken userspace.

Mimi

2017-04-07 08:43:05

by Dave Young

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

On 04/07/17 at 04:28am, Mimi Zohar wrote:
> On Fri, 2017-04-07 at 15:41 +0800, Dave Young wrote:
> > On 04/07/17 at 08:07am, David Howells wrote:
> > > Dave Young <[email protected]> wrote:
> > >
> > > > > > > + /* Don't permit images to be loaded into trusted kernels if we're not
> > > > > > > + * going to verify the signature on them
> > > > > > > + */
> > > > > > > + if (!IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG) && kernel_is_locked_down())
> > > > > > > + return -EPERM;
> > > > > > > +
> > > > > > >
> > > > >
> > > > > IMA can be used to verify file signatures too, based on the LSM hooks
> > > > > in ?kernel_read_file_from_fd(). ?CONFIG_KEXEC_VERIFY_SIG should not be
> > > > > required.
> > > >
> > > > Mimi, I remember we talked somthing before about the two signature
> > > > verification. One can change IMA policy in initramfs userspace,
> > > > also there are kernel cmdline param to disable IMA, so it can break the
> > > > lockdown? Suppose kexec boot with ima disabled cmdline param and then
> > > > kexec reboot again..
> > >
> > > I guess I should lock down the parameter to disable IMA too.
> >
> > That is one thing, user can change IMA policy in initramfs userspace,
> > I'm not sure if IMA enforce the signed policy now, if no it will be also
> > a problem.
>
> I'm not sure how this relates to the question of whether IMA verifies
> the kexec kernel image signature, as the test would not be based on a
> Kconfig option, but on a runtime variable.

I assumed one can change the policy to avoid kexec and initramfs check
And we use a global IMA status in the -EPERM check for the lockdown
checking. But if there is some fine grained checking to ensure kernel
signature verification it should be fine.
>
> To answer your question, the rule for requiring the policy to be
> signed is: ?appraise func=POLICY_CHECK appraise_type=imasig
>
> When the ability to append rules is Kconfig enabled, the builtin
> policy requires the new policy or additional rules to be signed.
> ?Unfortunately, always requiring the policy to be signed, would have
> broken userspace.
>
> Mimi
>

Thanks
Dave

2017-04-07 09:17:30

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

Mimi Zohar <[email protected]> wrote:

> > Okay, fair enough. I can stick in an OR with an IS_ENABLED on some IMA
> > symbol. CONFIG_IMA_KEXEC maybe? And also require IMA be enabled?
>
> Not quite, since as Dave pointed out, IMA is policy driven. As a
> policy is installed, we could set a flag.

Does such a flag exist as yet?

David

2017-04-07 10:25:54

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

On Wed, Apr 5, 2017 at 11:16 PM, David Howells <[email protected]> wrote:
> From: Matthew Garrett <[email protected]>
>
> We have no way of validating what all of the Asus WMI methods do on a given
> machine - and there's a risk that some will allow hardware state to be
> manipulated in such a way that arbitrary code can be executed in the
> kernel, circumventing module loading restrictions. Prevent that if the
> kernel is locked down.

> + if (kernel_is_locked_down())
> + return -EPERM;

It looks a bit fragile when responsility of whatever reasons kernel
can't serve become a driver burden.
Can we fix this in debugfs framework instead?

> +
> err = asus_wmi_get_devstate(asus, asus->debug.dev_id, &retval);
>
> if (err < 0)
> @@ -1916,6 +1919,9 @@ static int show_devs(struct seq_file *m, void *data)
> int err;
> u32 retval = -1;
>
> + if (kernel_is_locked_down())
> + return -EPERM;
> +
> err = asus_wmi_set_devstate(asus->debug.dev_id, asus->debug.ctrl_param,
> &retval);
>
> @@ -1940,6 +1946,9 @@ static int show_call(struct seq_file *m, void *data)
> union acpi_object *obj;
> acpi_status status;
>
> + if (kernel_is_locked_down())
> + return -EPERM;
> +
> status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID,
> 1, asus->debug.method_id,
> &input, &output);
>



--
With Best Regards,
Andy Shevchenko

2017-04-07 12:36:44

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

On Fri, 2017-04-07 at 10:17 +0100, David Howells wrote:
> Mimi Zohar <[email protected]> wrote:
>
> > > Okay, fair enough. I can stick in an OR with an IS_ENABLED on some IMA
> > > symbol. CONFIG_IMA_KEXEC maybe? And also require IMA be enabled?
> >
> > Not quite, since as Dave pointed out, IMA is policy driven. As a
> > policy is installed, we could set a flag.
>
> Does such a flag exist as yet?

Not exactly what is needed.  There's a flag named ima_appraise, which
is used internally in IMA. A temporary flag is created, while
validating the rules.

if (default_appraise_rules[i].func == POLICY_CHECK)
temp_ima_appraise |= IMA_APPRAISE_POLICY;

if (!result && (entry->action == UNKNOWN))
result = -EINVAL;
else if (entry->func == MODULE_CHECK)
temp_ima_appraise |= IMA_APPRAISE_MODULES;
else if (entry->func == FIRMWARE_CHECK)
temp_ima_appraise |= IMA_APPRAISE_FIRMWARE;
else if (entry->func == POLICY_CHECK)
temp_ima_appraise |= IMA_APPRAISE_POLICY;

If the entire policy is valid, ima_update_policy_flag() sets the ima_appraise flag.

ima_appraise |= temp_ima_appraise;

>From an IMA perspective, either a file hash or signature are valid,
but for this usage it must be a signature.  So in addition to testing
entry->func, above, entry->flags would need to be tested as well to
detect if IMA_DIGSIG_REQUIRED is set.

Mimi

2017-04-07 12:51:06

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

Andy Shevchenko <[email protected]> wrote:

> > From: Matthew Garrett <[email protected]>
> >
> > We have no way of validating what all of the Asus WMI methods do on a given
> > machine - and there's a risk that some will allow hardware state to be
> > manipulated in such a way that arbitrary code can be executed in the
> > kernel, circumventing module loading restrictions. Prevent that if the
> > kernel is locked down.
>
> > + if (kernel_is_locked_down())
> > + return -EPERM;
>
> It looks a bit fragile when responsility of whatever reasons kernel
> can't serve become a driver burden.
> Can we fix this in debugfs framework instead?

Fix it with debugfs how? We can't offload the decision to userspace.

David

2017-04-07 15:59:36

by Austin S Hemmelgarn

[permalink] [raw]
Subject: Re: [PATCH 00/24] Kernel lockdown

On 2017-04-05 16:14, David Howells wrote:
>
> These patches provide a facility by which a variety of avenues by which
> userspace can feasibly modify the running kernel image can be locked down.
> These include:
>
> (*) No unsigned modules and no modules for which can't validate the
> signature.
>
> (*) No use of ioperm(), iopl() and no writing to /dev/port.
>
> (*) No writing to /dev/mem or /dev/kmem.
>
> (*) No hibernation.
>
> (*) Restrict PCI BAR access.
>
> (*) Restrict MSR access.
>
> (*) No kexec_load().
>
> (*) Certain ACPI restrictions.
>
> (*) Restrict debugfs interface to ASUS WMI.
>
> The lock-down can be configured to be triggered by the EFI secure boot
> status, provided the shim isn't insecure. The lock-down can be lifted by
> typing SysRq+x on a keyboard attached to the system.
This has already been mentioned both in response to previous versions of
this patch set, and by at least 2 people in response to a specific patch
in this posting, but for any kind of proper security analysis, you need
to better clarify your threat model. 'Prevent modification to the
running kernel image' is a decent start on this, but at least some of
the patches don't explain very well _how_ what you're disabling could be
used to modify the running kernel image. Clarifying how something is a
threat will help with verifying that you're correctly blocking the threat.

Furthermore, why is the only way to enable this to boot in UEFI Secure
Boot mode? Almost all of the hardening done here has general utility in
hardening regular systems, and as such I'd suggest adding a command line
option to enable kernel lock-down (which would greatly simplify
testing), and a kconfig option to enforce it at build-time.

In addition to all that, it would be nice to be able to disable all of
the following at build time independent of the kernel lock-down state
* The acpi_rsdp kernel parameter (I could easily see many distros
building kernels with this disabled, it's insanely use-case specific).
* IO port and resource reservation module parameters (this would
actually be easier than having runtime blacklisting, and I could also
easily see this being turned on by default by a number of distros).
* TOICSERIAL (this one is more likely than the above two to break systems).

And these would probably be useful as lockable sysctls that would be
automatically locked disabled when the kernel is locked down:
* ioperm/iopl (these can technically be blocked by seccomp or other
means, but that is non-trivial to do).
* Most of the other ACPI stuff (some of this is useful for
troubleshooting, but is not normally used during regular operation).
* PCI BAR access.

2017-04-07 16:30:04

by Justin Forbes

[permalink] [raw]
Subject: Re: [PATCH 00/24] Kernel lockdown

On Fri, Apr 7, 2017 at 10:59 AM, Austin S. Hemmelgarn
<[email protected]> wrote:
> On 2017-04-05 16:14, David Howells wrote:
>>
>>
>> These patches provide a facility by which a variety of avenues by which
>> userspace can feasibly modify the running kernel image can be locked down.
>> These include:
>>
>> (*) No unsigned modules and no modules for which can't validate the
>> signature.
>>
>> (*) No use of ioperm(), iopl() and no writing to /dev/port.
>>
>> (*) No writing to /dev/mem or /dev/kmem.
>>
>> (*) No hibernation.
>>
>> (*) Restrict PCI BAR access.
>>
>> (*) Restrict MSR access.
>>
>> (*) No kexec_load().
>>
>> (*) Certain ACPI restrictions.
>>
>> (*) Restrict debugfs interface to ASUS WMI.
>>
>> The lock-down can be configured to be triggered by the EFI secure boot
>> status, provided the shim isn't insecure. The lock-down can be lifted by
>> typing SysRq+x on a keyboard attached to the system.
>
> This has already been mentioned both in response to previous versions of
> this patch set, and by at least 2 people in response to a specific patch in
> this posting, but for any kind of proper security analysis, you need to
> better clarify your threat model. 'Prevent modification to the running
> kernel image' is a decent start on this, but at least some of the patches
> don't explain very well _how_ what you're disabling could be used to modify
> the running kernel image. Clarifying how something is a threat will help
> with verifying that you're correctly blocking the threat.

It is more than just preventing modification to the running kernel
image. The idea is that everything is verified, from UEFI through the
bootloader, and into the kernel.

> Furthermore, why is the only way to enable this to boot in UEFI Secure Boot
> mode? Almost all of the hardening done here has general utility in
> hardening regular systems, and as such I'd suggest adding a command line
> option to enable kernel lock-down (which would greatly simplify testing),
> and a kconfig option to enforce it at build-time.

The problem is, if the hand off doesn't happen from a secure firmware,
there is no guarantee the system has not been compromised. UEFI Secure
Boot mode attempts to give that promise, and an appropriate hand off.
That doesn't mean that there is no value in turning some of this on,
it is just of more limited effectiveness.

> In addition to all that, it would be nice to be able to disable all of the
> following at build time independent of the kernel lock-down state
> * The acpi_rsdp kernel parameter (I could easily see many distros building
> kernels with this disabled, it's insanely use-case specific).
> * IO port and resource reservation module parameters (this would actually be
> easier than having runtime blacklisting, and I could also easily see this
> being turned on by default by a number of distros).
> * TOICSERIAL (this one is more likely than the above two to break systems).
>
> And these would probably be useful as lockable sysctls that would be
> automatically locked disabled when the kernel is locked down:
> * ioperm/iopl (these can technically be blocked by seccomp or other means,
> but that is non-trivial to do).
> * Most of the other ACPI stuff (some of this is useful for troubleshooting,
> but is not normally used during regular operation).
> * PCI BAR access.

There are more patches to do some of these things. Baby steps.

2017-04-08 03:28:29

by poma

[permalink] [raw]
Subject: Re: [PATCH 11/24] uswsusp: Disable when the kernel is locked down

On 06.04.2017 22:25, Jiri Kosina wrote:
> On Thu, 6 Apr 2017, Rafael J. Wysocki wrote:
>
>>>>> Your swap partition may be located on an NVDIMM or be encrypted.
>>>>
>>>> An NVDIMM should be considered the same as any other persistent storage.
>>>>
>>>> It may be encrypted, but where's the key stored, how easy is it to retrieve
>>>> and does the swapout code know this?
>>>>
>>>>> Isn't this a bit overly drastic?
>>>>
>>>> Perhaps, but if it's on disk and it's not encrypted, then maybe not.
>>>
>>> Right.
>>>
>>> Swap encryption is not mandatory and I'm not sure how the hibernate
>>> code can verify whether or not it is in use.
>>
>> BTW, SUSE has patches adding secure boot support to the hibernate code
>> and Jiri promised me to post them last year even. :-)
>
> Oh, thanks for a friendly ping :) Adding Joey Lee to CC.
>

Rafael J., are you talking about HIBERNATE_VERIFICATION ?

Ref.
https://github.com/joeyli/linux-s4sign/commits/s4sign-hmac-v2-v4.2-rc8
https://lkml.org/lkml/2015/8/11/47
https://bugzilla.redhat.com/show_bug.cgi?id=1330335

2017-04-09 11:10:18

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

On Fri, Apr 7, 2017 at 3:50 PM, David Howells <[email protected]> wrote:
> Andy Shevchenko <[email protected]> wrote:
>
>> > From: Matthew Garrett <[email protected]>
>> >
>> > We have no way of validating what all of the Asus WMI methods do on a given
>> > machine - and there's a risk that some will allow hardware state to be
>> > manipulated in such a way that arbitrary code can be executed in the
>> > kernel, circumventing module loading restrictions. Prevent that if the
>> > kernel is locked down.
>>
>> > + if (kernel_is_locked_down())
>> > + return -EPERM;
>>
>> It looks a bit fragile when responsility of whatever reasons kernel
>> can't serve become a driver burden.
>> Can we fix this in debugfs framework instead?
>
> Fix it with debugfs how? We can't offload the decision to userspace.

I mean to do at least similar like you have done for module
parameters. So, instead of putting above code to each attribute in
question make a special (marked) attribute instead and debugfs
framework will know how to deal with that.

--
With Best Regards,
Andy Shevchenko

2017-04-10 13:16:57

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

Andy Shevchenko <[email protected]> wrote:

> >> It looks a bit fragile when responsility of whatever reasons kernel
> >> can't serve become a driver burden.
> >> Can we fix this in debugfs framework instead?
> >
> > Fix it with debugfs how? We can't offload the decision to userspace.
>
> I mean to do at least similar like you have done for module
> parameters. So, instead of putting above code to each attribute in
> question make a special (marked) attribute instead and debugfs
> framework will know how to deal with that.

Hmmm... It's tricky in that debugfs doesn't have any of its own structures,
but is entirely built on standard VFS ones, so finding somewhere to store the
information is going to be awkward. One obvious solution is to entirely lock
down debugfs in secure boot more, but that might be a bit drastic.

Note that it's still going to be a driver burden to some extent anyway. The
driver has to tell the core what needs to be restricted.

Further, I guess configfs needs attention also.

David

2017-04-10 13:20:02

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 09/24] kexec_file: Disable at runtime if securelevel has been set

Mimi Zohar <[email protected]> wrote:

> From an IMA perspective, either a file hash or signature are valid,
> but for this usage it must be a signature.

Not necessarily. If IMA can guarantee that a module is the same based on its
hash rather than on a key, I would've thought that should be fine.

David

2017-04-10 23:15:37

by David Howells

[permalink] [raw]
Subject: Why kernel lockdown?

Alexei Starovoitov <[email protected]> wrote:

> Also is there a description of what this lockdown trying to accomplish?

Austin S. Hemmelgarn <[email protected]> wrote:

> ... but for any kind of proper security analysis, you need to better clarify
> your threat model. 'Prevent modification to the running kernel image' is a
> decent start on this, but at least some of the patches don't explain very
> well _how_ what you're disabling could be used to modify the running kernel
> image. Clarifying how something is a threat will help with verifying that
> you're correctly blocking the threat.

The idea is to properly implement UEFI Secure Boot mode such that we can kexec
another operating system (Linux, Windows, etc.) and pass on the Secure Boot
flag with a guarantee that we haven't been compromised.

This assumes the following:

(1) Someone wanting to compromise your machine can't get physical access to
the running hardware. I think pretty much all bets are off if someone
gets their hands on your computer.

(2) Whatever boots our kernel is not itself compromised. If it is, there's
pretty much nothing we can do about it.

(3) Whatever boots our kernel can prove that our kernel is what it says it
is.


Now, (2) has to start with security right from the system booting, and likely
involves the firmware being checked by the hardware. The firmware then has to
validate anything it runs, and the things it runs have to validate anything
they run, etc. up to our kernel being booted.

With regard to (3), take the example of booting Fedora Linux on a UEFI PC in
secure boot mode: the UEFI BIOS boots the Fedora SHIM, which boots Grub, which
boots the kernel. The SHIM is signed by the UEFI signing authority and is
checked against the UEFI key database; Grub and the kernel are signed by a key
built into the SHIM.

[Note that in secure boot mode, Grub loads the kernel image asks the SHIM to
verify it; further, the SHIM will catch anyone trying to boot without
verification and halt the machine.]

[Note that we do verification with cryptographic signatures, but compiled-in
hash whitelists would also work.]

In order to maintain the security guarantee, the kernel then needs to prevent
unauthorised modification of code and data in the running kernel (module
loading is permissible) and also it needs to protect any private keys or other
security data it may hold within the image. We try to do this by the
following means:

(1) Refuse to use any key or hash that UEFI has in its blacklist.

(2) Refuse to load any module that isn't signed/hashed.

(3) Refuse to load any firmware that isn't signed/hashed.

(4) Refuse to kexec any image that isn't signed/hashed.

(5) Refuse to dump a kernel image for software suspend/hibernation if it's
not encrypted. Further, if it is encrypted, the key must be protected.

(6) Refuse to load a dumped kernel image that isn't encrypted with a
protected key.

(7) Refuse to allow userspace direct access to kernel memory (no /dev/mem,
/dev/kmem, /proc/kcore).

(8) Refuse to allow userspace direct, unsupervised access to hardware (no
iopl, /dev/ioports, MSRs, etc.). It might be feasible to open PCI BARs
through dedicated device files for certain functions though (eg. X
servers), but unconstrained DMA must be disallowed.

(9) Refuse to let userspace configure a driver to use a piece of hardware to
muck around with system RAM, possibly by mismatching driver and device.

> 'Prevent modification to the running kernel image' is a decent start on
> this, but at least some of the patches don't explain very well _how_ what
> you're disabling could be used to modify the running kernel image.

To be honest, I don't know in all cases since I'm taking a collection of
patches not all of which are written by me. I can see how most of them work,
but some of the more esoteric things, like ACPI...

> Furthermore, why is the only way to enable this to boot in UEFI Secure Boot
> mode?

There's no reason that this need be the case, but it's the one I'm interested
in. But do note assumption (2) above. I can provide kernel lockdown in other
situations, but can't forward the guarantee if I wasn't given one to begin
with.

David

2017-04-12 13:45:19

by joeyli

[permalink] [raw]
Subject: Re: [PATCH 11/24] uswsusp: Disable when the kernel is locked down

On Sat, Apr 08, 2017 at 05:28:15AM +0200, poma wrote:
> On 06.04.2017 22:25, Jiri Kosina wrote:
> > On Thu, 6 Apr 2017, Rafael J. Wysocki wrote:
> >
> >>>>> Your swap partition may be located on an NVDIMM or be encrypted.
> >>>>
> >>>> An NVDIMM should be considered the same as any other persistent storage.
> >>>>
> >>>> It may be encrypted, but where's the key stored, how easy is it to retrieve
> >>>> and does the swapout code know this?
> >>>>
> >>>>> Isn't this a bit overly drastic?
> >>>>
> >>>> Perhaps, but if it's on disk and it's not encrypted, then maybe not.
> >>>
> >>> Right.
> >>>
> >>> Swap encryption is not mandatory and I'm not sure how the hibernate
> >>> code can verify whether or not it is in use.
> >>
> >> BTW, SUSE has patches adding secure boot support to the hibernate code
> >> and Jiri promised me to post them last year even. :-)
> >
> > Oh, thanks for a friendly ping :) Adding Joey Lee to CC.
> >
>
> Rafael J., are you talking about HIBERNATE_VERIFICATION ?
>
> Ref.
> https://github.com/joeyli/linux-s4sign/commits/s4sign-hmac-v2-v4.2-rc8
> https://lkml.org/lkml/2015/8/11/47
> https://bugzilla.redhat.com/show_bug.cgi?id=1330335
>

I am working on switch to HMAC-SHA512.

On the other hand, some mechanisms keep signing/encryption key in memory.
e.g. dm-crypt or hibernation verification. Kees Cook suggested that we
should add kernel memory reads as a thread model of securelevel to
prevent leaking those keys by /dev/kmem, bpf, kdump or hibernation...
We still need time to implement it.

Thanks a lot!
Joey Lee

2017-04-12 14:58:00

by joeyli

[permalink] [raw]
Subject: Re: [PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down

Hi David,

First, thanks for your help to send out this series.

On Wed, Apr 05, 2017 at 09:17:25PM +0100, David Howells wrote:
> From: Chun-Yi Lee <[email protected]>
>
> There are some bpf functions can be used to read kernel memory:
> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
> private keys in kernel memory (e.g. the hibernation image signing key) to
> be read by an eBPF program. Prohibit those functions when the kernel is
> locked down.
>
> Signed-off-by: Chun-Yi Lee <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]

This patch is used with hibernation signature verification. I suggest
that we can remove this patch from your series because we just lock
down the hibernation function until hibernation verification get
accepted.

On the other hand, we are trying to enhance the bpf verifier to
prevent bpf print reads specific memory addresses that have sensitive
data.

Thanks a lot!
Joey Lee

2017-04-13 08:46:26

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 20/24] bpf: Restrict kernel image access functions when the kernel is locked down

Alexei Starovoitov <[email protected]> wrote:

> this will obviously break the program.

Yeah. But if it allows one to twiddle the kernel image or gain access to
crypto material...

> How about disabling loading tracing programs during the lockdown completely?

Interesting thought. I'm not sure how much would actually need locking down
here. Turning on tracepoints in the kernel and reading out of the trace
buffer, for example, ought to be okay, though if there are any tracepoints
that leak crypto information, they may need locking down also.

Basically, I think it boils down to: if it can be used to modify the kernel
image or read arbitrary data from the kernel image then should probably be
forbidden. There have to be exceptions, though, such as loading authenticated
kernel modules.

David

2017-04-14 18:05:23

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 06/24] Add a sysrq option to exit secure boot mode

On Wed, 5 Apr 2017, David Howells wrote:

> From: Kyle McMartin <[email protected]>
>
> Make sysrq+x exit secure boot mode on x86_64, thereby allowing the running
> kernel image to be modified. This lifts the lockdown.
>
> Signed-off-by: Kyle McMartin <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]

Matt, Ard?

Any opinions on this?

Thanks,

tglx

2017-04-14 18:15:45

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 06/24] Add a sysrq option to exit secure boot mode

On 14 April 2017 at 19:05, Thomas Gleixner <[email protected]> wrote:
> On Wed, 5 Apr 2017, David Howells wrote:
>
>> From: Kyle McMartin <[email protected]>
>>
>> Make sysrq+x exit secure boot mode on x86_64, thereby allowing the running
>> kernel image to be modified. This lifts the lockdown.
>>
>> Signed-off-by: Kyle McMartin <[email protected]>
>> Signed-off-by: David Howells <[email protected]>
>> cc: [email protected]
>
> Matt, Ard?
>
> Any opinions on this?
>

>From an EFI point of view, there is not a lot to see here. I think
having a SysRq to lift lockdown makes sense, although I think we
should avoid 'secure boot' when referring to lockdown because they are
really two different things. As someone else pointed out, you may have
other ways of trusting your kernel, in which case you should be able
to lock it down as well.

That does bring me to another EFI related point: many of these patches
are x86 specific for no good reason. We have been working really hard
over the past couple of years to move EFI plumbing into
drivers/firmware/efi, and things are not intimately related to an
architecture should ideally be implemented there. Looking at the
diffstat of this patch, I don't see why this should be a x86 only
feature.

In general, though, I think this should be two patches, one that
introduces the functionality to restrict some SysRq keys to console
only, and one that adds the 'x' for lockdown lift.

I haven't gotten around to responding to David's general email
regarding the point of all of this. I will do so asap, but it will
need to wait until Tuesday at least.

--
Ard.


On 14 April 2017 at 19:05, Thomas Gleixner <[email protected]> wrote:
> On Wed, 5 Apr 2017, David Howells wrote:
>
>> From: Kyle McMartin <[email protected]>
>>
>> Make sysrq+x exit secure boot mode on x86_64, thereby allowing the running
>> kernel image to be modified. This lifts the lockdown.
>>
>> Signed-off-by: Kyle McMartin <[email protected]>
>> Signed-off-by: David Howells <[email protected]>
>> cc: [email protected]
>
> Matt, Ard?
>
> Any opinions on this?
>
> Thanks,
>
> tglx

2017-04-14 18:28:52

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 13/24] x86: Lock down IO port access when the kernel is locked down

On Wed, 5 Apr 2017, David Howells wrote:
> From: Matthew Garrett <[email protected]>
>
> IO port access would permit users to gain access to PCI configuration
> registers, which in turn (on a lot of hardware) give access to MMIO
> register space. This would potentially permit root to trigger arbitrary
> DMA, so lock it down by default.
>
> This also implicitly locks down the KDADDIO, KDDELIO, KDENABIO and
> KDDISABIO console ioctls.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> Signed-off-by: David Howells <[email protected]>

Reviewed-by: Thomas Gleixner <[email protected]>

2017-04-14 18:30:47

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 14/24] x86: Restrict MSR access when the kernel is locked down

On Wed, 5 Apr 2017, David Howells wrote:

Can you please change the subsys in $subject to 'x86/msr:' ?

> From: Matthew Garrett <[email protected]>
>
> Writing to MSRs should not be allowed if the kernel is locked down, since
> it could lead to execution of arbitrary code in kernel mode. Based on a
> patch by Kees Cook.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Acked-by: Kees Cook <[email protected]>
> cc: [email protected]

Reviewed-by: Thomas Gleixner <[email protected]>

2017-04-14 23:16:32

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 06/24] Add a sysrq option to exit secure boot mode

Ard Biesheuvel <[email protected]> wrote:

> That does bring me to another EFI related point: many of these patches
> are x86 specific for no good reason.

Note that the sysrq one is awkward since the key chosen *is* arch-specific.
SysRq+x can't be arbitrarily assigned to this since some other arches have
their own use for it.

Anyway, the ones that are x86-specific are:

efi: Add EFI_SECURE_BOOT bit
efi: Lock down the kernel if booted in secure boot mode
Add a sysrq option to exit secure boot mode
Copy secure_boot flag in boot params across kexec reboot
x86: Lock down IO port access when the kernel is locked down
x86: Restrict MSR access when the kernel is locked down
asus-wmi: Restrict debugfs interface when the kernel is locked down

The first three are dealt with in the five patches I posted later, including
making the choice of sysrq key an arch override. The bits that can be moved
out to the efi firmware driver have been.

The 4th looks to be x86 bootloader protocol specific.

The remainder look very x86 specific, apart from one piece in the 5th patch
where /dev/port is locked down.

David

2017-04-16 20:46:27

by Matt Fleming

[permalink] [raw]
Subject: Re: [PATCH 06/24] Add a sysrq option to exit secure boot mode

On Fri, 14 Apr, at 08:05:07PM, Thomas Gleixner wrote:
> On Wed, 5 Apr 2017, David Howells wrote:
>
> > From: Kyle McMartin <[email protected]>
> >
> > Make sysrq+x exit secure boot mode on x86_64, thereby allowing the running
> > kernel image to be modified. This lifts the lockdown.
> >
> > Signed-off-by: Kyle McMartin <[email protected]>
> > Signed-off-by: David Howells <[email protected]>
> > cc: [email protected]
>
> Matt, Ard?
>
> Any opinions on this?

Looks OK to me.

2017-04-18 06:06:45

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

On Mon, Apr 10, 2017 at 4:16 PM, David Howells <[email protected]> wrote:
> Andy Shevchenko <[email protected]> wrote:
>
>> >> It looks a bit fragile when responsility of whatever reasons kernel
>> >> can't serve become a driver burden.
>> >> Can we fix this in debugfs framework instead?
>> >
>> > Fix it with debugfs how? We can't offload the decision to userspace.
>>
>> I mean to do at least similar like you have done for module
>> parameters. So, instead of putting above code to each attribute in
>> question make a special (marked) attribute instead and debugfs
>> framework will know how to deal with that.
>
> Hmmm... It's tricky in that debugfs doesn't have any of its own structures,
> but is entirely built on standard VFS ones, so finding somewhere to store the
> information is going to be awkward.

I see.

> One obvious solution is to entirely lock
> down debugfs in secure boot more, but that might be a bit drastic.

But this sounds sane! debugFS for debugging, not for production. If
someone is using secure kernel it means pure production use (otherwise
one may do temporary hacks in kernel).
If one still needs debugfs in secure mode, it sounds to me as
architectural bug in code in question.

>
> Note that it's still going to be a driver burden to some extent anyway. The
> driver has to tell the core what needs to be restricted.
>
> Further, I guess configfs needs attention also.

--
With Best Regards,
Andy Shevchenko

2017-04-18 14:34:50

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

On Tue, 2017-04-18 at 09:06 +0300, Andy Shevchenko wrote:
> > On Mon, Apr 10, 2017 at 4:16 PM, David Howells <[email protected]> wrote:
> > > > Andy Shevchenko <[email protected]> wrote:
> >
> > > > > It looks a bit fragile when responsility of whatever reasons kernel
> > > > > can't serve become a driver burden.
> > > > > Can we fix this in debugfs framework instead?
> > > >
> > > > Fix it with debugfs how?  We can't offload the decision to userspace.
> > >
> > > I mean to do at least similar like you have done for module
> > > parameters. So, instead of putting above code to each attribute in
> > > question make a special (marked) attribute instead and debugfs
> > > framework will know how to deal with that.
> >
> > Hmmm...  It's tricky in that debugfs doesn't have any of its own structures,
> > but is entirely built on standard VFS ones, so finding somewhere to store the
> > information is going to be awkward.
>
> I see.
>
> >  One obvious solution is to entirely lock
> > down debugfs in secure boot more, but that might be a bit drastic.
>
> But this sounds sane! debugFS for debugging, not for production. If
> someone is using secure kernel it means pure production use (otherwise
> one may do temporary hacks in kernel).
[...]

Production systems need instrumentation to understand performance
issues and any bugs that for whatever reason didn't show up in earlier
testing. A number of interfaces for that have been added under
debugfs:

- tracing (now tracefs, but it's expected to appear under debugfs)
- dynamic_debug
- various ad-hoc statistics

So it's generally not going to be OK to turn off debugfs. There will
probably need to be a distinction between believed-safe and unsafe
directories/files.

Ben.

--
Ben Hutchings
The world is coming to an end. Please log off.


Attachments:
signature.asc (833.00 B)
This is a digitally signed message part

2017-04-18 14:56:20

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

Ben Hutchings <[email protected]> wrote:

> - tracing (now tracefs, but it's expected to appear under debugfs)

Shouldn't this now appear under /sys/kernel/tracing/ ?

David

2017-04-18 15:20:19

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

On Tue, 2017-04-18 at 15:55 +0100, David Howells wrote:
> Ben Hutchings <[email protected]> wrote:
>
> > - tracing (now tracefs, but it's expected to appear under debugfs)
>
> Shouldn't this now appear under /sys/kernel/tracing/ ?

True, but old tracing scripts didn't go away.

Ben.

--
Ben Hutchings
The world is coming to an end. Please log off.


Attachments:
signature.asc (833.00 B)
This is a digitally signed message part

2017-04-18 15:30:43

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

Ben Hutchings <[email protected]> wrote:

> So it's generally not going to be OK to turn off debugfs. There will
> probably need to be a distinction between believed-safe and unsafe
> directories/files.

Any suggestion on how to mark this distinction? I'd prefer not to modify
every read/write op associated with a debugfs file. Modify
DEFINE_DEBUGFS_ATTRIBUTE() maybe? And provide lockable variants of
debugfs_create_u8() and co.?

David

2017-04-18 15:34:59

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

Ben Hutchings <[email protected]> wrote:

> > Shouldn't this now appear under /sys/kernel/tracing/ ?
>
> True, but old tracing scripts didn't go away.

Conversion to a symlink would fix that.

David

2017-04-18 17:39:33

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down

On Tue, 2017-04-18 at 16:30 +0100, David Howells wrote:
> Ben Hutchings <[email protected]> wrote:
>
> > So it's generally not going to be OK to turn off debugfs.  There will
> > probably need to be a distinction between believed-safe and unsafe
> > directories/files.
>
> Any suggestion on how to mark this distinction?

I don't know.

> I'd prefer not to modify every read/write op associated with a
> debugfs file.

I think debugfs should be assumed unsafe by default. So only the
believed-safe parts would need to be changed.

> Modify
> DEFINE_DEBUGFS_ATTRIBUTE() maybe?  And provide lockable variants of
> debugfs_create_u8() and co.?

That could help.

Ben.

--
Ben Hutchings
The world is coming to an end. Please log off.


Attachments:
signature.asc (833.00 B)
This is a digitally signed message part

2017-04-18 17:51:01

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 12/24] PCI: Lock down BAR access when the kernel is locked down

On Wed, Apr 05, 2017 at 09:16:18PM +0100, David Howells wrote:
> From: Matthew Garrett <[email protected]>
>
> Any hardware that can potentially generate DMA has to be locked down in
> order to avoid it being possible for an attacker to modify kernel code,
> allowing them to circumvent disabled module loading or module signing.
> Default to paranoid - in future we can potentially relax this for
> sufficiently IOMMU-isolated devices.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]

Acked-by: Bjorn Helgaas <[email protected]>

> ---
>
> drivers/pci/pci-sysfs.c | 9 +++++++++
> drivers/pci/proc.c | 8 +++++++-
> drivers/pci/syscall.c | 2 +-
> 3 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 25d010d449a3..f70b3668036f 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -727,6 +727,9 @@ static ssize_t pci_write_config(struct file *filp, struct kobject *kobj,
> loff_t init_off = off;
> u8 *data = (u8 *) buf;
>
> + if (kernel_is_locked_down())
> + return -EPERM;
> +
> if (off > dev->cfg_size)
> return 0;
> if (off + count > dev->cfg_size) {
> @@ -1018,6 +1021,9 @@ static int pci_mmap_resource(struct kobject *kobj, struct bin_attribute *attr,
> resource_size_t start, end;
> int i;
>
> + if (kernel_is_locked_down())
> + return -EPERM;
> +
> for (i = 0; i < PCI_ROM_RESOURCE; i++)
> if (res == &pdev->resource[i])
> break;
> @@ -1117,6 +1123,9 @@ static ssize_t pci_write_resource_io(struct file *filp, struct kobject *kobj,
> struct bin_attribute *attr, char *buf,
> loff_t off, size_t count)
> {
> + if (kernel_is_locked_down())
> + return -EPERM;
> +
> return pci_resource_io(filp, kobj, attr, buf, off, count, true);
> }
>
> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> index f82710a8694d..139d6f09ae7b 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -116,6 +116,9 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
> int size = dev->cfg_size;
> int cnt;
>
> + if (kernel_is_locked_down())
> + return -EPERM;
> +
> if (pos >= size)
> return 0;
> if (nbytes >= size)
> @@ -195,6 +198,9 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd,
> #endif /* HAVE_PCI_MMAP */
> int ret = 0;
>
> + if (kernel_is_locked_down())
> + return -EPERM;
> +
> switch (cmd) {
> case PCIIOC_CONTROLLER:
> ret = pci_domain_nr(dev->bus);
> @@ -233,7 +239,7 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma)
> struct pci_filp_private *fpriv = file->private_data;
> int i, ret, write_combine;
>
> - if (!capable(CAP_SYS_RAWIO))
> + if (!capable(CAP_SYS_RAWIO) || kernel_is_locked_down())
> return -EPERM;
>
> /* Make sure the caller is mapping a real resource for this device */
> diff --git a/drivers/pci/syscall.c b/drivers/pci/syscall.c
> index 9bf993e1f71e..c09524738ceb 100644
> --- a/drivers/pci/syscall.c
> +++ b/drivers/pci/syscall.c
> @@ -92,7 +92,7 @@ SYSCALL_DEFINE5(pciconfig_write, unsigned long, bus, unsigned long, dfn,
> u32 dword;
> int err = 0;
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!capable(CAP_SYS_ADMIN) || kernel_is_locked_down())
> return -EPERM;
>
> dev = pci_get_bus_and_slot(bus, dfn);
>