Hi Linus,
I've modified my lockdown mode patches to remove the EFI/secure-boot
interaction and to ignore kexec. Would you be willing to accept this?
David
---
Here's a set of patches to institute a "locked-down mode" in the kernel.
If enabled, the lockdown can be triggered either by configuration (in which
case it cannot be lifted) or by command line (lockdown=1). In the latter
case, there's an option to allow it to be lifted by a SysRq key on an
attached keyboard.
Three config options are provided:
(1) CONFIG_LOCK_DOWN_KERNEL makes lockdown mode available.
(2) CONFIG_LOCK_DOWN_MANDATORY builds the kernel with lockdown mode
enabled at compile time and removes the ability to disable it.
(3) CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ will allow a SysRq combination to
lift the lockdown. On x86 this is SysRq+x. The keys must be pressed
on an attached keyboard. Echoing into the trigger file won't work.
Inside the kernel, kernel_is_locked_down() is used to check if the kernel
is in lockdown mode.
For the moment, there is only one mode that locks everything down. Andy
would prefer that there be two modes, one of which allows the kernel to be
read, but not modified and the other which allows neither. Modifying this
later to allow what Andy wants wouldn't be particularly hard as there isn't
much additional functionality that this would enable (ie. /dev/kmem,
/dev/kcore, bpf and perf).
A manual page, kernel_lockdown.7, is proposed, to which people will be
directed by messages in dmesg. This lists the features that are restricted
amongst other things.
The patches are tagged here:
git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
tags/lockdown-20180410
The patches can be found here also:
http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=lockdown
---
David Howells (11):
Add the ability to lock down access to the running kernel image
Enforce module signatures if the kernel is locked down
Prohibit PCMCIA CIS storage when the kernel is locked down
Lock down TIOCSSERIAL
Lock down module params that specify hardware parameters (eg. ioport)
x86/mmiotrace: Lock down the testmmiotrace module
Lock down /proc/kcore
Lock down kprobes
bpf: Restrict kernel image access functions when the kernel is locked down
Lock down perf
debugfs: Restrict debugfs when the kernel is locked down
Josh Boyer (2):
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 lift kernel lockdown
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 (7):
Restrict /dev/{mem,kmem,port} when the kernel is locked down
kexec_load: 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/msr: Restrict MSR access when the kernel is locked down
ACPI: Limit access to custom_method when the kernel is locked down
Mimi Zohar (1):
ima: require secure_boot rules in lockdown mode
arch/x86/include/asm/setup.h | 2 +
arch/x86/kernel/ioport.c | 6 +-
arch/x86/kernel/msr.c | 10 +++
arch/x86/kernel/setup.c | 2 +
arch/x86/mm/testmmiotrace.c | 3 +
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 | 2 +
drivers/input/misc/uinput.c | 1
drivers/pci/pci-sysfs.c | 9 +++
drivers/pci/proc.c | 9 +++
drivers/pci/syscall.c | 3 +
drivers/pcmcia/cistpl.c | 3 +
drivers/tty/serial/serial_core.c | 6 ++
drivers/tty/sysrq.c | 19 ++++--
fs/debugfs/file.c | 28 +++++++++
fs/debugfs/inode.c | 30 +++++++++
fs/proc/kcore.c | 2 +
include/linux/input.h | 5 ++
include/linux/kernel.h | 32 ++++++++++
include/linux/sysrq.h | 8 ++-
kernel/bpf/syscall.c | 3 +
kernel/debug/kdb/kdb_main.c | 2 -
kernel/events/core.c | 5 ++
kernel/kexec.c | 7 ++
kernel/kprobes.c | 3 +
kernel/module.c | 56 +++++++++++++-----
kernel/params.c | 26 +++++++-
kernel/power/hibernate.c | 2 -
kernel/power/user.c | 3 +
security/Kconfig | 34 ++++++++++-
security/Makefile | 3 +
security/integrity/ima/ima_policy.c | 39 +++++++++---
security/lock_down.c | 112 +++++++++++++++++++++++++++++++++++
36 files changed, 443 insertions(+), 45 deletions(-)
create mode 100644 security/lock_down.c
From: Kyle McMartin <[email protected]>
Make an option to provide a sysrq key that will lift the kernel lockdown,
thereby allowing the running kernel image to be accessed and modified.
On x86 this is triggered with SysRq+x, but this key may not be available on
all arches, so it is set by setting LOCKDOWN_LIFT_KEY in asm/setup.h.
Since this macro must be defined in an arch to be able to use this facility
for that arch, the Kconfig option is restricted to arches that support it.
Signed-off-by: Kyle McMartin <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---
arch/x86/include/asm/setup.h | 2 ++
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 +-
security/Kconfig | 11 ++++++++++
security/lock_down.c | 47 ++++++++++++++++++++++++++++++++++++++++++
8 files changed, 87 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index ae13bc974416..3108e297d87d 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -9,6 +9,8 @@
#include <linux/linkage.h>
#include <asm/page_types.h>
+#define LOCKDOWN_LIFT_KEY 'x'
+
#ifdef __i386__
#include <linux/pfn.h>
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 96a887f33698..027c730631cc 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -365,6 +365,7 @@ static int uinput_create_device(struct uinput_device *udev)
dev->flush = uinput_dev_flush;
}
+ dev->flags |= INPUTDEV_FLAGS_SYNTHETIC;
dev->event = uinput_dev_event;
input_set_drvdata(udev->dev, udev);
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 6364890575ec..ffeb3aa86cd1 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -487,6 +487,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 */
@@ -530,7 +531,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;
@@ -550,11 +551,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);
@@ -586,7 +591,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);
@@ -667,7 +672,7 @@ static void sysrq_do_reset(struct timer_list *t)
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,
@@ -818,8 +823,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;
}
@@ -1102,7 +1109,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 7c7516eb7d76..38cd0ea72c37 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 8c71874e8485..7de1f08b60a9 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -29,6 +29,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;
@@ -43,8 +45,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 dbb0781a0533..aae9a0f44058 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;
diff --git a/security/Kconfig b/security/Kconfig
index a68e5bdebad5..46967ee77dfd 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -253,6 +253,17 @@ config LOCK_DOWN_MANDATORY
Makes the lockdown non-negotiable. It is always on and cannot be
disabled.
+config ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
+ bool "Allow the kernel lockdown to be lifted by SysRq"
+ depends on LOCK_DOWN_KERNEL
+ depends on !LOCK_DOWN_MANDATORY
+ depends on MAGIC_SYSRQ
+ depends on X86
+ help
+ Allow the lockdown on a kernel to be lifted, by pressing a SysRq key
+ combination on a wired keyboard. On x86, this is SysRq+x.
+
+
source security/selinux/Kconfig
source security/smack/Kconfig
source security/tomoyo/Kconfig
diff --git a/security/lock_down.c b/security/lock_down.c
index f35ffdd096ad..2615669dbf03 100644
--- a/security/lock_down.c
+++ b/security/lock_down.c
@@ -11,9 +11,15 @@
#include <linux/export.h>
#include <linux/sched.h>
+#include <linux/sysrq.h>
+#include <asm/setup.h>
#ifndef CONFIG_LOCK_DOWN_MANDATORY
+#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
+static __read_mostly bool kernel_locked_down;
+#else
static __ro_after_init bool kernel_locked_down;
+#endif
#else
#define kernel_locked_down true
#endif
@@ -63,3 +69,44 @@ bool __kernel_is_locked_down(const char *what, bool first)
return kernel_locked_down;
}
EXPORT_SYMBOL(__kernel_is_locked_down);
+
+#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
+
+/*
+ * Take the kernel out of lockdown mode.
+ */
+static void lift_kernel_lockdown(void)
+{
+ pr_notice("Lifting lockdown\n");
+ kernel_locked_down = false;
+}
+
+/*
+ * Allow lockdown to be lifted by pressing something like SysRq+x (and not by
+ * echoing the appropriate letter into the sysrq-trigger file).
+ */
+static void sysrq_handle_lockdown_lift(int key)
+{
+ if (kernel_locked_down)
+ lift_kernel_lockdown();
+}
+
+static struct sysrq_key_op lockdown_lift_sysrq_op = {
+ .handler = sysrq_handle_lockdown_lift,
+ .help_msg = "unSB(x)",
+ .action_msg = "Disabling Secure Boot restrictions",
+ .enable_mask = SYSRQ_DISABLE_USERSPACE,
+};
+
+static int __init lockdown_lift_sysrq(void)
+{
+ if (kernel_locked_down) {
+ lockdown_lift_sysrq_op.help_msg[5] = LOCKDOWN_LIFT_KEY;
+ register_sysrq_key(LOCKDOWN_LIFT_KEY, &lockdown_lift_sysrq_op);
+ }
+ return 0;
+}
+
+late_initcall(lockdown_lift_sysrq);
+
+#endif /* CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ */
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:
- /dev/mem and similar
- Loading of unauthorised modules
- Fiddling with MSR registers
- Suspend to disk managed by the kernel
- Use of device DMA
Two kernel configuration options are provided:
(*) CONFIG_LOCK_DOWN_KERNEL
This makes lockdown available and applies it to all the points that
need to be locked down if the mode is set. Lockdown mode can be
enabled by providing:
lockdown=1
on the command line.
(*) CONFIG_LOCK_DOWN_MANDATORY
This forces lockdown on at compile time, overriding the command line
option.
init_lockdown() is used as a hook from which lockdown can be managed in
future. It has to be called from arch setup code before things like ACPI
are enabled.
Note that, with the other changes in this series, if lockdown mode is
enabled, the kernel will not be able to use certain drivers as the ability
to manually configure hardware parameters would then be prohibited. This
primarily applies to ISA hardware devices.
Signed-off-by: David Howells <[email protected]>
---
arch/x86/kernel/setup.c | 2 +
include/linux/kernel.h | 32 +++++++++++++++++++++++
security/Kconfig | 23 ++++++++++++++++-
security/Makefile | 3 ++
security/lock_down.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 124 insertions(+), 1 deletion(-)
create mode 100644 security/lock_down.c
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 6285697b6e56..566f0f447053 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -996,6 +996,8 @@ void __init setup_arch(char **cmdline_p)
if (efi_enabled(EFI_BOOT))
efi_init();
+ init_lockdown();
+
dmi_scan_machine();
dmi_memdev_walk();
dmi_set_dump_stack_arch_desc();
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 4ae1dfd9bf05..7d085cca9cee 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -306,6 +306,38 @@ static inline void refcount_error_report(struct pt_regs *regs, const char *err)
{ }
#endif
+#ifdef CONFIG_LOCK_DOWN_KERNEL
+extern void __init init_lockdown(void);
+extern bool __kernel_is_locked_down(const char *what, bool first);
+
+#ifndef CONFIG_LOCK_DOWN_MANDATORY
+#define kernel_is_locked_down(what) \
+ ({ \
+ static bool message_given; \
+ bool locked_down = __kernel_is_locked_down(what, !message_given); \
+ message_given = true; \
+ locked_down; \
+ })
+#else
+#define kernel_is_locked_down(what) \
+ ({ \
+ static bool message_given; \
+ __kernel_is_locked_down(what, !message_given); \
+ message_given = true; \
+ true; \
+ })
+#endif
+#else
+static inline void __init init_lockdown(void)
+{
+}
+static inline bool __kernel_is_locked_down(const char *what, bool first)
+{
+ return false;
+}
+#define kernel_is_locked_down(what) ({ 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/security/Kconfig b/security/Kconfig
index c4302067a3ad..a68e5bdebad5 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -231,6 +231,28 @@ 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. Locking down the kernel turns
+ off various features that might otherwise allow access to the kernel
+ image (eg. setting MSR registers).
+
+ Note, however, that locking down your kernel will prevent some
+ drivers from functioning because allowing manual configuration of
+ hardware parameters is forbidden, lest a device be used to access the
+ kernel by DMA. This mostly applies to ISA devices.
+
+ The kernel lockdown can be triggered by adding lockdown=1 to the
+ kernel command line.
+
+config LOCK_DOWN_MANDATORY
+ bool "Make kernel lockdown mandatory"
+ depends on LOCK_DOWN_KERNEL
+ help
+ Makes the lockdown non-negotiable. It is always on and cannot be
+ disabled.
+
source security/selinux/Kconfig
source security/smack/Kconfig
source security/tomoyo/Kconfig
@@ -278,4 +300,3 @@ config DEFAULT_SECURITY
default "" if DEFAULT_SECURITY_DAC
endmenu
-
diff --git a/security/Makefile b/security/Makefile
index 4d2d3782ddef..507ac8c520ce 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -30,3 +30,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..f35ffdd096ad
--- /dev/null
+++ b/security/lock_down.c
@@ -0,0 +1,65 @@
+/* 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/export.h>
+#include <linux/sched.h>
+
+#ifndef CONFIG_LOCK_DOWN_MANDATORY
+static __ro_after_init bool kernel_locked_down;
+#else
+#define kernel_locked_down true
+#endif
+
+/*
+ * Put the kernel into lock-down mode.
+ */
+static void __init lock_kernel_down(const char *where)
+{
+#ifndef CONFIG_LOCK_DOWN_MANDATORY
+ if (!kernel_locked_down) {
+ kernel_locked_down = true;
+ pr_notice("Kernel is locked down from %s; see man kernel_lockdown.7\n",
+ where);
+ }
+#endif
+}
+
+static int __init lockdown_param(char *ignored)
+{
+ lock_kernel_down("command line");
+ return 0;
+}
+
+early_param("lockdown", lockdown_param);
+
+/*
+ * Lock the kernel down from very early in the arch setup. This must happen
+ * prior to things like ACPI being initialised.
+ */
+void __init init_lockdown(void)
+{
+#ifdef CONFIG_LOCK_DOWN_MANDATORY
+ pr_notice("Kernel is locked down from config; see man kernel_lockdown.7\n");
+#endif
+}
+
+/**
+ * kernel_is_locked_down - Find out if the kernel is locked down
+ * @what: Tag to use in notice generated if lockdown is in effect
+ */
+bool __kernel_is_locked_down(const char *what, bool first)
+{
+ if (what && first && kernel_locked_down)
+ pr_notice("Lockdown: %s: %s is restricted; see man kernel_lockdown.7\n",
+ current->comm, what);
+ return kernel_locked_down;
+}
+EXPORT_SYMBOL(__kernel_is_locked_down);
From: Matthew Garrett <[email protected]>
Allowing users to read and write to core kernel memory makes it possible
for the kernel to be subverted, avoiding module loading restrictions, and
also to steal cryptographic information.
Disallow /dev/mem and /dev/kmem from being opened this when the kernel has
been locked down to prevent this.
Also disallow /dev/port from being opened to prevent raw ioport access and
thus DMA from being used to accomplish the same thing.
Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
---
drivers/char/mem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index ffeb60d3434c..b2fca26e5765 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -784,6 +784,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("/dev/mem,kmem,port"))
+ return -EPERM;
return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
}
If the kernel is locked down, require that all modules have valid
signatures that we can verify or that IMA can validate the file.
I have adjusted the errors generated:
(1) If there's no signature (ENODATA) or we can't check it (ENOPKG,
ENOKEY), then:
(a) If signatures are enforced then EKEYREJECTED is returned.
(b) If IMA will have validated the image, return 0 (okay).
(c) If there's no signature or we can't check it, but the kernel is
locked down then EPERM is returned (this is then consistent with
other lockdown cases).
(2) If the signature is unparseable (EBADMSG, EINVAL), the signature fails
the check (EKEYREJECTED) or a system error occurs (eg. ENOMEM), we
return the error we got.
Note that the X.509 code doesn't check for key expiry as the RTC might not
be valid or might not have been transferred to the kernel's clock yet.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Jiri Bohac <[email protected]>
cc: "Lee, Chun-Yi" <[email protected]>
cc: James Morris <[email protected]>
---
kernel/module.c | 56 ++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 43 insertions(+), 13 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index a6e43a5806a1..9c1709a05037 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -64,6 +64,7 @@
#include <linux/bsearch.h>
#include <linux/dynamic_debug.h>
#include <linux/audit.h>
+#include <linux/ima.h>
#include <uapi/linux/module.h>
#include "module-internal.h"
@@ -2761,10 +2762,12 @@ static inline void kmemleak_load_module(const struct module *mod,
#endif
#ifdef CONFIG_MODULE_SIG
-static int module_sig_check(struct load_info *info, int flags)
+static int module_sig_check(struct load_info *info, int flags,
+ bool can_do_ima_check)
{
- int err = -ENOKEY;
+ int err = -ENODATA;
const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+ const char *reason;
const void *mod = info->hdr;
/*
@@ -2779,19 +2782,46 @@ static int module_sig_check(struct load_info *info, int flags)
err = mod_verify_sig(mod, &info->len);
}
- if (!err) {
+ switch (err) {
+ case 0:
info->sig_ok = true;
return 0;
- }
- /* Not having a signature is only an error if we're strict. */
- if (err == -ENOKEY && !sig_enforce)
- err = 0;
+ /* We don't permit modules to be loaded into trusted kernels
+ * without a valid signature on them, but if we're not
+ * enforcing, certain errors are non-fatal.
+ */
+ case -ENODATA:
+ reason = "Loading of unsigned module";
+ goto decide;
+ case -ENOPKG:
+ reason = "Loading of module with unsupported crypto";
+ goto decide;
+ case -ENOKEY:
+ reason = "Loading of module with unavailable key";
+ decide:
+ if (sig_enforce) {
+ pr_notice("%s is rejected\n", reason);
+ return -EKEYREJECTED;
+ }
- return err;
+ if (can_do_ima_check && is_ima_appraise_enabled())
+ return 0;
+ if (kernel_is_locked_down(reason))
+ return -EPERM;
+ return 0;
+
+ /* All other errors are fatal, including nomem, unparseable
+ * signatures and signature check failures - even if signatures
+ * aren't required.
+ */
+ default:
+ return err;
+ }
}
#else /* !CONFIG_MODULE_SIG */
-static int module_sig_check(struct load_info *info, int flags)
+static int module_sig_check(struct load_info *info, int flags,
+ bool can_do_ima_check)
{
return 0;
}
@@ -3651,13 +3681,13 @@ static int unknown_module_param_cb(char *param, char *val, const char *modname,
/* Allocate and load the module: note that size of section 0 is always
zero, and we rely on this for optional sections. */
static int load_module(struct load_info *info, const char __user *uargs,
- int flags)
+ int flags, bool can_do_ima_check)
{
struct module *mod;
long err;
char *after_dashes;
- err = module_sig_check(info, flags);
+ err = module_sig_check(info, flags, can_do_ima_check);
if (err)
goto free_copy;
@@ -3846,7 +3876,7 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
if (err)
return err;
- return load_module(&info, uargs, 0);
+ return load_module(&info, uargs, 0, false);
}
SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
@@ -3873,7 +3903,7 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
info.hdr = hdr;
info.len = size;
- return load_module(&info, uargs, flags);
+ return load_module(&info, uargs, flags, true);
}
static inline int within(unsigned long addr, void *start, unsigned long size)
From: Matthew Garrett <[email protected]>
The kexec_load() syscall 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_load() in this situation.
This does not affect kexec_file_load() syscall 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]>
Acked-by: Dave Young <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
Reviewed-by: James Morris <[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 aed8fb2564b3..1553ac765e73 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -199,6 +199,13 @@ static inline int kexec_load_check(unsigned long nr_segments,
if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
return -EPERM;
+ /*
+ * kexec can be used to circumvent module loading restrictions, so
+ * prevent loading in that case
+ */
+ if (kernel_is_locked_down("kexec of unsigned images"))
+ return -EPERM;
+
/*
* Verify we have a legal set of flags
* This leaves us room for future extensions.
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]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
Reviewed-by: James Morris <[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 75c959de4b29..959b336d8eca 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("/dev/snapshot"))
+ return -EPERM;
+
lock_system_sleep();
if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
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]>
Reviewed-by: "Lee, Chun-Yi" <[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 e967c1173ba3..a07fbe999eb6 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("ACPI custom methods"))
+ return -EPERM;
+
if (!(*ppos)) {
/* parse the table header to get the table length */
if (count <= sizeof(struct acpi_table_header))
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]>
Reviewed-by: "Lee, Chun-Yi" <[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 849c4fb19b03..6c5ee7e66842 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -527,6 +527,11 @@ void __init acpi_table_upgrade(void)
if (table_nr == 0)
return;
+ if (kernel_is_locked_down("ACPI table override")) {
+ 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);
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 | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
index 102646fedb56..e46c948d7246 100644
--- a/drivers/pcmcia/cistpl.c
+++ b/drivers/pcmcia/cistpl.c
@@ -1578,6 +1578,9 @@ static ssize_t pccard_store_cis(struct file *filp, struct kobject *kobj,
struct pcmcia_socket *s;
int error;
+ if (kernel_is_locked_down("Direct PCMCIA CIS storage"))
+ return -EPERM;
+
s = to_socket(container_of(kobj, struct device, kobj));
if (off)
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 | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index cc9108c2a1fd..2c08c4aa376b 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -108,13 +108,19 @@ 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("Command line-specified device addresses, irqs and dma channels"))
+ return false;
+ return true;
}
static int parse_one(char *param,
@@ -144,8 +150,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(¶ms[i]);
- err = params[i].ops->set(val, ¶ms[i]);
+ if (param_check_unsafe(¶ms[i], doing))
+ err = params[i].ops->set(val, ¶ms[i]);
+ else
+ err = -EPERM;
kernel_param_unlock(params[i].mod);
return err;
}
@@ -553,6 +561,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,
@@ -565,8 +579,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;
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 0466f9f08a91..360f8e4416c4 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -829,6 +829,12 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
new_flags = (__force upf_t)new_info->flags;
old_custom_divisor = uport->custom_divisor;
+ if ((change_port || change_irq) &&
+ kernel_is_locked_down("Using TIOCSSERIAL to change device addresses, irqs and dma channels")) {
+ retval = -EPERM;
+ goto exit;
+ }
+
if (!capable(CAP_SYS_ADMIN)) {
retval = -EPERM;
if (change_irq || change_port ||
The testmmiotrace module shouldn't be permitted when the kernel is locked
down as it can be used to arbitrarily read and write MMIO space.
Suggested-by: Thomas Gleixner <[email protected]>
Signed-off-by: David Howells <[email protected]
cc: Thomas Gleixner <[email protected]>
cc: Steven Rostedt <[email protected]>
cc: Ingo Molnar <[email protected]>
cc: "H. Peter Anvin" <[email protected]>
cc: [email protected]
---
arch/x86/mm/testmmiotrace.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/mm/testmmiotrace.c b/arch/x86/mm/testmmiotrace.c
index f6ae6830b341..bbaad357f5d7 100644
--- a/arch/x86/mm/testmmiotrace.c
+++ b/arch/x86/mm/testmmiotrace.c
@@ -115,6 +115,9 @@ static int __init init(void)
{
unsigned long size = (read_far) ? (8 << 20) : (16 << 10);
+ if (kernel_is_locked_down("MMIO trace testing"))
+ return -EPERM;
+
if (mmio_address == 0) {
pr_err("you have to use the module argument mmio_address.\n");
pr_err("DO NOT LOAD THIS MODULE UNLESS YOU REALLY KNOW WHAT YOU ARE DOING!\n");
Disallow access to /proc/kcore when the kernel is locked down to prevent
access to cryptographic data.
Signed-off-by: David Howells <[email protected]>
Reviewed-by: James Morris <[email protected]>
---
fs/proc/kcore.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index d1e82761de81..cdebdee81719 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -546,6 +546,8 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos)
static int open_kcore(struct inode *inode, struct file *filp)
{
+ if (kernel_is_locked_down("/proc/kcore"))
+ return -EPERM;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
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 modify the workings of hardware . Reject
the option when the kernel is locked down.
Signed-off-by: Josh Boyer <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: Dave Young <[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 7ca41bf023c9..34e4ce7939f4 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;
#ifdef CONFIG_KEXEC
- if (acpi_rsdp)
+ if (acpi_rsdp && !kernel_is_locked_down("ACPI RSDP specification"))
return acpi_rsdp;
#endif
pa = acpi_arch_get_root_pointer();
Disallow the creation of kprobes when the kernel is locked down by
preventing their registration. This prevents kprobes from being used to
access kernel memory, either to make modifications or to steal crypto data.
Reported-by: Alexei Starovoitov <[email protected]>
Signed-off-by: David Howells <[email protected]>
---
kernel/kprobes.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 102160ff5c66..4f5757732553 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1561,6 +1561,9 @@ int register_kprobe(struct kprobe *p)
struct module *probed_mod;
kprobe_opcode_t *addr;
+ if (kernel_is_locked_down("Use of kprobes"))
+ return -EPERM;
+
/* Adjust probe address from symbol */
addr = kprobe_addr(p);
if (IS_ERR(addr))
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.
Completely prohibit the use of BPF when the kernel is locked down.
Suggested-by: Alexei Starovoitov <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
cc: Chun-Yi Lee <[email protected]>
cc: Alexei Starovoitov <[email protected]>
---
kernel/bpf/syscall.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 0244973ee544..7457f2676c6d 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2031,6 +2031,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
return -EPERM;
+ if (kernel_is_locked_down("BPF"))
+ return -EPERM;
+
err = check_uarg_tail_zero(uattr, sizeof(attr), size);
if (err)
return err;
Disallow the use of certain perf facilities that might allow userspace to
access kernel data.
Signed-off-by: David Howells <[email protected]>
---
kernel/events/core.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index fc1c330c6bd6..1922f2e0980a 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10407,6 +10407,11 @@ SYSCALL_DEFINE5(perf_event_open,
return -EINVAL;
}
+ if ((attr.sample_type & PERF_SAMPLE_REGS_INTR) &&
+ kernel_is_locked_down("PERF_SAMPLE_REGS_INTR"))
+ /* REGS_INTR can leak data, lockdown must prevent this */
+ return -EPERM;
+
/* Only privileged users can get physical addresses */
if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) &&
perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
Disallow opening of debugfs files that might be used to muck around when
the kernel is locked down as various drivers give raw access to hardware
through debugfs. Given the effort of auditing all 2000 or so files and
manually fixing each one as necessary, I've chosen to apply a heuristic
instead. The following changes are made:
(1) chmod and chown are disallowed on debugfs objects (though the root dir
can be modified by mount and remount, but I'm not worried about that).
(2) When the kernel is locked down, only files with the following criteria
are permitted to be opened:
- The file must have mode 00444
- The file must not have ioctl methods
- The file must not have mmap
(3) When the kernel is locked down, files may only be opened for reading.
Normal device interaction should be done through configfs, sysfs or a
miscdev, not debugfs.
Note that this makes it unnecessary to specifically lock down show_dsts(),
show_devs() and show_call() in the asus-wmi driver.
I would actually prefer to lock down all files by default and have the
the files unlocked by the creator. This is tricky to manage correctly,
though, as there are 19 creation functions and ~1600 call sites (some of
them in loops scanning tables).
Signed-off-by: David Howells <[email protected]>
cc: Andy Shevchenko <[email protected]>
cc: [email protected]
cc: [email protected]
cc: Matthew Garrett <[email protected]>
cc: Thomas Gleixner <[email protected]>
---
fs/debugfs/file.c | 28 ++++++++++++++++++++++++++++
fs/debugfs/inode.c | 30 ++++++++++++++++++++++++++++--
2 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 1f99678ff5d3..51cb894c21f2 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -136,6 +136,25 @@ void debugfs_file_put(struct dentry *dentry)
}
EXPORT_SYMBOL_GPL(debugfs_file_put);
+/*
+ * Only permit access to world-readable files when the kernel is locked down.
+ * We also need to exclude any file that has ways to write or alter it as root
+ * can bypass the permissions check.
+ */
+static bool debugfs_is_locked_down(struct inode *inode,
+ struct file *filp,
+ const struct file_operations *real_fops)
+{
+ if ((inode->i_mode & 07777) == 0444 &&
+ !(filp->f_mode & FMODE_WRITE) &&
+ !real_fops->unlocked_ioctl &&
+ !real_fops->compat_ioctl &&
+ !real_fops->mmap)
+ return false;
+
+ return kernel_is_locked_down("debugfs");
+}
+
static int open_proxy_open(struct inode *inode, struct file *filp)
{
struct dentry *dentry = F_DENTRY(filp);
@@ -147,6 +166,11 @@ static int open_proxy_open(struct inode *inode, struct file *filp)
return r == -EIO ? -ENOENT : r;
real_fops = debugfs_real_fops(filp);
+
+ r = -EPERM;
+ if (debugfs_is_locked_down(inode, filp, real_fops))
+ goto out;
+
real_fops = fops_get(real_fops);
if (!real_fops) {
/* Huh? Module did not clean up after itself at exit? */
@@ -272,6 +296,10 @@ static int full_proxy_open(struct inode *inode, struct file *filp)
return r == -EIO ? -ENOENT : r;
real_fops = debugfs_real_fops(filp);
+ r = -EPERM;
+ if (debugfs_is_locked_down(inode, filp, real_fops))
+ goto out;
+
real_fops = fops_get(real_fops);
if (!real_fops) {
/* Huh? Module did not cleanup after itself at exit? */
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 13b01351dd1c..4daec17b8215 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -32,6 +32,31 @@ static struct vfsmount *debugfs_mount;
static int debugfs_mount_count;
static bool debugfs_registered;
+/*
+ * Don't allow access attributes to be changed whilst the kernel is locked down
+ * so that we can use the file mode as part of a heuristic to determine whether
+ * to lock down individual files.
+ */
+static int debugfs_setattr(struct dentry *dentry, struct iattr *ia)
+{
+ if ((ia->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)) &&
+ kernel_is_locked_down("debugfs"))
+ return -EPERM;
+ return simple_setattr(dentry, ia);
+}
+
+static const struct inode_operations debugfs_file_inode_operations = {
+ .setattr = debugfs_setattr,
+};
+static const struct inode_operations debugfs_dir_inode_operations = {
+ .lookup = simple_lookup,
+ .setattr = debugfs_setattr,
+};
+static const struct inode_operations debugfs_symlink_inode_operations = {
+ .get_link = simple_get_link,
+ .setattr = debugfs_setattr,
+};
+
static struct inode *debugfs_get_inode(struct super_block *sb)
{
struct inode *inode = new_inode(sb);
@@ -356,6 +381,7 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
inode->i_mode = mode;
inode->i_private = data;
+ inode->i_op = &debugfs_file_inode_operations;
inode->i_fop = proxy_fops;
dentry->d_fsdata = (void *)((unsigned long)real_fops |
DEBUGFS_FSDATA_IS_REAL_FOPS_BIT);
@@ -513,7 +539,7 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
return failed_creating(dentry);
inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
- inode->i_op = &simple_dir_inode_operations;
+ inode->i_op = &debugfs_dir_inode_operations;
inode->i_fop = &simple_dir_operations;
/* directory inodes start off with i_nlink == 2 (for "." entry) */
@@ -608,7 +634,7 @@ struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent,
return failed_creating(dentry);
}
inode->i_mode = S_IFLNK | S_IRWXUGO;
- inode->i_op = &simple_symlink_inode_operations;
+ inode->i_op = &debugfs_symlink_inode_operations;
inode->i_link = link;
d_instantiate(dentry, inode);
return end_creating(dentry);
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]>
Reviewed-by: "Lee, Chun-Yi" <[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 5454cc639a8d..629f158f5a0c 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("Hibernation");
}
/**
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]>
Reviewed-by: "Lee, Chun-Yi" <[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 b38737c83a24..6d71e1e97b20 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("ACPI error injection"))
+ 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)))
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]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: [email protected]
---
arch/x86/kernel/ioport.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 0fe1c8782208..abc702a6ae9c 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -31,7 +31,8 @@ long ksys_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("ioperm")))
return -EPERM;
/*
@@ -126,7 +127,8 @@ 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("iopl"))
return -EPERM;
}
regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) |
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.
MSR accesses are logged for the purposes of building up a whitelist as per
Alan Cox's suggestion.
Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: Kees Cook <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: [email protected]
---
arch/x86/kernel/msr.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
index ef688804f80d..dfb61d358196 100644
--- a/arch/x86/kernel/msr.c
+++ b/arch/x86/kernel/msr.c
@@ -84,6 +84,11 @@ static ssize_t msr_write(struct file *file, const char __user *buf,
int err = 0;
ssize_t bytes = 0;
+ if (kernel_is_locked_down("Direct MSR access")) {
+ pr_info("Direct access to MSR %x\n", reg);
+ return -EPERM;
+ }
+
if (count % 8)
return -EINVAL; /* Invalid chunk size */
@@ -135,6 +140,11 @@ static long msr_ioctl(struct file *file, unsigned int ioc, unsigned long arg)
err = -EFAULT;
break;
}
+ if (kernel_is_locked_down("Direct MSR access")) {
+ pr_info("Direct access to MSR %x\n", regs[1]); /* Display %ecx */
+ err = -EPERM;
+ break;
+ }
err = wrmsr_safe_regs_on_cpu(cpu, regs);
if (err)
break;
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]>
Acked-by: Bjorn Helgaas <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: [email protected]
---
drivers/pci/pci-sysfs.c | 9 +++++++++
drivers/pci/proc.c | 9 ++++++++-
drivers/pci/syscall.c | 3 ++-
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 366d93af051d..1e149ec006a4 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -903,6 +903,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("Direct PCI access"))
+ return -EPERM;
+
if (off > dev->cfg_size)
return 0;
if (off + count > dev->cfg_size) {
@@ -1165,6 +1168,9 @@ static int pci_mmap_resource(struct kobject *kobj, struct bin_attribute *attr,
enum pci_mmap_state mmap_type;
struct resource *res = &pdev->resource[bar];
+ if (kernel_is_locked_down("Direct PCI access"))
+ return -EPERM;
+
if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
return -EINVAL;
@@ -1240,6 +1246,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("Direct PCI access"))
+ 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 1ee8927a0635..469445a9019b 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -117,6 +117,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("Direct PCI access"))
+ return -EPERM;
+
if (pos >= size)
return 0;
if (nbytes >= size)
@@ -196,6 +199,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("Direct PCI access"))
+ return -EPERM;
+
switch (cmd) {
case PCIIOC_CONTROLLER:
ret = pci_domain_nr(dev->bus);
@@ -237,7 +243,8 @@ 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 = 0, res_bit = IORESOURCE_MEM;
- if (!capable(CAP_SYS_RAWIO))
+ if (!capable(CAP_SYS_RAWIO) ||
+ kernel_is_locked_down("Direct PCI access"))
return -EPERM;
if (fpriv->mmap_state == pci_mmap_io) {
diff --git a/drivers/pci/syscall.c b/drivers/pci/syscall.c
index d96626c614f5..b8a08d3166a1 100644
--- a/drivers/pci/syscall.c
+++ b/drivers/pci/syscall.c
@@ -90,7 +90,8 @@ 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("Direct PCI access"))
return -EPERM;
dev = pci_get_domain_bus_and_slot(0, bus, dfn);
From: Mimi Zohar <[email protected]>
Require the "secure_boot" rules, whether or not it is specified
on the boot command line, for both the builtin and custom policies
in secure boot lockdown mode.
Signed-off-by: Mimi Zohar <[email protected]>
Signed-off-by: David Howells <[email protected]>
---
security/integrity/ima/ima_policy.c | 39 ++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index d89bebf85421..da6f55c96a61 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -443,14 +443,21 @@ void ima_update_policy_flag(void)
*/
void __init ima_init_policy(void)
{
- int i, measure_entries, appraise_entries, secure_boot_entries;
+ int i;
+ int measure_entries = 0;
+ int appraise_entries = 0;
+ int secure_boot_entries = 0;
+ bool kernel_locked_down = __kernel_is_locked_down(NULL, false);
/* if !ima_policy set entries = 0 so we load NO default rules */
- measure_entries = ima_policy ? ARRAY_SIZE(dont_measure_rules) : 0;
- appraise_entries = ima_use_appraise_tcb ?
- ARRAY_SIZE(default_appraise_rules) : 0;
- secure_boot_entries = ima_use_secure_boot ?
- ARRAY_SIZE(secure_boot_rules) : 0;
+ if (ima_policy)
+ measure_entries = ARRAY_SIZE(dont_measure_rules);
+
+ if (ima_use_appraise_tcb)
+ appraise_entries = ARRAY_SIZE(default_appraise_rules);
+
+ if (ima_use_secure_boot || kernel_locked_down)
+ secure_boot_entries = ARRAY_SIZE(secure_boot_rules);
for (i = 0; i < measure_entries; i++)
list_add_tail(&dont_measure_rules[i].list, &ima_default_rules);
@@ -471,11 +478,23 @@ void __init ima_init_policy(void)
/*
* Insert the appraise rules requiring file signatures, prior to
- * any other appraise rules.
+ * any other appraise rules. In secure boot lock-down mode, also
+ * require these appraise rules for custom policies.
*/
- for (i = 0; i < secure_boot_entries; i++)
- list_add_tail(&secure_boot_rules[i].list,
- &ima_default_rules);
+ for (i = 0; i < secure_boot_entries; i++) {
+ struct ima_rule_entry *entry;
+
+ /* Include for builtin policies */
+ list_add_tail(&secure_boot_rules[i].list, &ima_default_rules);
+
+ /* Include for custom policies */
+ if (kernel_locked_down) {
+ entry = kmemdup(&secure_boot_rules[i], sizeof(*entry),
+ GFP_KERNEL);
+ if (entry)
+ list_add_tail(&entry->list, &ima_policy_rules);
+ }
+ }
for (i = 0; i < appraise_entries; i++) {
list_add_tail(&default_appraise_rules[i].list,
On Wed, Apr 11, 2018 at 6:24 PM, David Howells <[email protected]> wrote:
> 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:
>
> - /dev/mem and similar
> - Loading of unauthorised modules
> - Fiddling with MSR registers
> - Suspend to disk managed by the kernel
> - Use of device DMA
>
> Two kernel configuration options are provided:
>
> (*) CONFIG_LOCK_DOWN_KERNEL
>
> This makes lockdown available and applies it to all the points that
> need to be locked down if the mode is set. Lockdown mode can be
> enabled by providing:
>
> lockdown=1
>
> on the command line.
>
> (*) CONFIG_LOCK_DOWN_MANDATORY
>
> This forces lockdown on at compile time, overriding the command line
> option.
>
> init_lockdown() is used as a hook from which lockdown can be managed in
> future. It has to be called from arch setup code before things like ACPI
> are enabled.
>
> Note that, with the other changes in this series, if lockdown mode is
> enabled, the kernel will not be able to use certain drivers as the ability
> to manually configure hardware parameters would then be prohibited. This
> primarily applies to ISA hardware devices.
>
> Signed-off-by: David Howells <[email protected]>
> ---
[...]
> diff --git a/security/lock_down.c b/security/lock_down.c
> new file mode 100644
> index 000000000000..f35ffdd096ad
> --- /dev/null
> +++ b/security/lock_down.c
[...]
> +/*
> + * Lock the kernel down from very early in the arch setup. This must happen
> + * prior to things like ACPI being initialised.
> + */
Pedantic nit: I think this comment is wrong now? This function
actually just prints stuff.
> +void __init init_lockdown(void)
> +{
> +#ifdef CONFIG_LOCK_DOWN_MANDATORY
> + pr_notice("Kernel is locked down from config; see man kernel_lockdown.7\n");
> +#endif
> +}
On Wed, Apr 11, 2018 at 6:24 PM, David Howells <[email protected]> wrote:
> From: Kyle McMartin <[email protected]>
>
> Make an option to provide a sysrq key that will lift the kernel lockdown,
> thereby allowing the running kernel image to be accessed and modified.
>
> On x86 this is triggered with SysRq+x, but this key may not be available on
> all arches, so it is set by setting LOCKDOWN_LIFT_KEY in asm/setup.h.
> Since this macro must be defined in an arch to be able to use this facility
> for that arch, the Kconfig option is restricted to arches that support it.
In the current form, this is probably incompatible with USB/IP (which
Debian seems to be shipping as a module by default), right? And
perhaps also with dummy_hcd (if I understand correctly what it's
doing)?
On 04/11/2018 09:26 AM, David Howells wrote:
> 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 | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/params.c b/kernel/params.c
> index cc9108c2a1fd..2c08c4aa376b 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -108,13 +108,19 @@ 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("Command line-specified device addresses, irqs and dma channels"))
s/dma/DMA/
Other patches use PCI instead of pci, CIS instead of cis, MMIO instead of mmio,
BPF instead of bpf, MSR instead of msr, etc...
> + return false;
> + return true;
> }
>
> static int parse_one(char *param,
--
~Randy
On 04/11/2018 09:27 AM, David Howells wrote:
> Signed-off-by: David Howells <[email protected]>
> cc: Andy Shevchenko <[email protected]>
> cc: [email protected]
> cc: [email protected]
> cc: Matthew Garrett <[email protected]>
> cc: Thomas Gleixner <[email protected]>
> ---
meta-comment:
I have been dinged for not spelling "cc:" as "Cc:". I really think that
either way should be acceptable.
--
~Randy
On 04/11/2018 09:24 AM, David Howells wrote:
> ---
>
> arch/x86/kernel/setup.c | 2 +
> include/linux/kernel.h | 32 +++++++++++++++++++++++
> security/Kconfig | 23 ++++++++++++++++-
> security/Makefile | 3 ++
> security/lock_down.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 124 insertions(+), 1 deletion(-)
> create mode 100644 security/lock_down.c
> diff --git a/security/Kconfig b/security/Kconfig
> index c4302067a3ad..a68e5bdebad5 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -231,6 +231,28 @@ 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. Locking down the kernel turns
> + off various features that might otherwise allow access to the kernel
s/turns off/disables/
> + image (eg. setting MSR registers).
e.g.
> +
> + Note, however, that locking down your kernel will prevent some
the kernel
a kernel
> + drivers from functioning because allowing manual configuration of
> + hardware parameters is forbidden, lest a device be used to access the
> + kernel by DMA. This mostly applies to ISA devices.
Is DMA from non-ISA devices OK, or did I miss seeing that patch?
> + The kernel lockdown can be triggered by adding lockdown=1 to the
> + kernel command line.
> diff --git a/security/lock_down.c b/security/lock_down.c
> new file mode 100644
> index 000000000000..f35ffdd096ad
> --- /dev/null
> +++ b/security/lock_down.c
> @@ -0,0 +1,65 @@
> +/* 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.
fsf.org spells that Licence word as License. :)
> + */
--
~Randy
Randy Dunlap <[email protected]> wrote:
> > + drivers from functioning because allowing manual configuration of
> > + hardware parameters is forbidden, lest a device be used to access the
> > + kernel by DMA. This mostly applies to ISA devices.
>
> Is DMA from non-ISA devices OK, or did I miss seeing that patch?
The issue is where you can directly command a device to do DMA, possibly by
using the wrong driver. Ask Alan Cox.
David
On Wed, Apr 11, 2018 at 9:24 AM, David Howells <[email protected]> wrote:
> 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:
>
> - /dev/mem and similar
> - Loading of unauthorised modules
> - Fiddling with MSR registers
> - Suspend to disk managed by the kernel
> - Use of device DMA
So what I stlll absolutely detest about this series is that I think
many of these things should simply be done as separate config options.
For example, if the distro is sure that it doesn't need /dev/mem, then
why the hell is this tied to "lockdown" that then may have to be
disabled because *other* changes may not be acceptable (eg people may
need that device DMA, or whatever).
If that /dev/mem access prevention was just instead done as an even
stricter mode of the existing CONFIG_STRICT_DEVMEM, it could just be
enabled unconditionally.
So none of these patches raise my hackles per se. But what continues
to makes me very very uncomfortable is how this is all tied together.
Why is this one magical mode that then - because it has such a big
impact - has to be enabled/disabled as a single magical mode and with
very odd rules?
I think a lot of people would be happier if this wasn't so incestuous
and mixing together independent things under one name, and one flag.
I think a lot of the secure boot problems were exacerbated by that mixup.
So I would seriously ask that the distros that have been using these
patches look at which parts of lockdown they could make unconditional
(because it doesn't break machines), and which ones need that escape
clause.
Linus
On Wed, Apr 11, 2018 at 1:09 PM, Linus Torvalds
<[email protected]> wrote:
> On Wed, Apr 11, 2018 at 9:24 AM, David Howells <[email protected]> wrote:
>> 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:
>>
>> - /dev/mem and similar
>> - Loading of unauthorised modules
>> - Fiddling with MSR registers
>> - Suspend to disk managed by the kernel
>> - Use of device DMA
>
> So what I stlll absolutely detest about this series is that I think
> many of these things should simply be done as separate config options.
>
> For example, if the distro is sure that it doesn't need /dev/mem, then
> why the hell is this tied to "lockdown" that then may have to be
> disabled because *other* changes may not be acceptable (eg people may
> need that device DMA, or whatever).
>
> If that /dev/mem access prevention was just instead done as an even
> stricter mode of the existing CONFIG_STRICT_DEVMEM, it could just be
> enabled unconditionally.
>
> So none of these patches raise my hackles per se. But what continues
> to makes me very very uncomfortable is how this is all tied together.
>
> Why is this one magical mode that then - because it has such a big
> impact - has to be enabled/disabled as a single magical mode and with
> very odd rules?
>
> I think a lot of people would be happier if this wasn't so incestuous
> and mixing together independent things under one name, and one flag.
>
> I think a lot of the secure boot problems were exacerbated by that mixup.
>
> So I would seriously ask that the distros that have been using these
> patches look at which parts of lockdown they could make unconditional
> (because it doesn't break machines), and which ones need that escape
> clause.
>
Optionally, it might make sense to add separate config options for
each of these pieces which can be unconditionally enabled, and a
separate option for secure boot which selects all of them? As much as
I hate select, it might make sense here. Of course the flip side to
that, is users no longer have one big switch "turn off secure boot"
which turns it all off in case of trouble.
Justin
David Howells <[email protected]> writes:
> From: Matthew Garrett <[email protected]>
>
> The kexec_load() syscall 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_load() in this situation.
>
> This does not affect kexec_file_load() syscall which can check for a
> signature on the image to be booted.
Maybing I am missing it but I am not seeing anything that would require
kexec_file_load be configured such that it checks the loaded kernel.
Without that I don't see the point of disabling kexec_load.
Nacked-by: "Eric W. Biederman" <[email protected]>
Eric
> Signed-off-by: Matthew Garrett <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Acked-by: Dave Young <[email protected]>
> Reviewed-by: "Lee, Chun-Yi" <[email protected]>
> Reviewed-by: James Morris <[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 aed8fb2564b3..1553ac765e73 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -199,6 +199,13 @@ static inline int kexec_load_check(unsigned long nr_segments,
> if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
> return -EPERM;
>
> + /*
> + * kexec can be used to circumvent module loading restrictions, so
> + * prevent loading in that case
> + */
> + if (kernel_is_locked_down("kexec of unsigned images"))
> + return -EPERM;
> +
> /*
> * Verify we have a legal set of flags
> * This leaves us room for future extensions.
On Wed, Apr 11, 2018 at 05:27:16PM +0100, David Howells wrote:
> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs. Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead. The following changes are made:
>
> (1) chmod and chown are disallowed on debugfs objects (though the root dir
> can be modified by mount and remount, but I'm not worried about that).
>
> (2) When the kernel is locked down, only files with the following criteria
> are permitted to be opened:
>
> - The file must have mode 00444
> - The file must not have ioctl methods
> - The file must not have mmap
>
> (3) When the kernel is locked down, files may only be opened for reading.
>
> Normal device interaction should be done through configfs, sysfs or a
> miscdev, not debugfs.
>
> Note that this makes it unnecessary to specifically lock down show_dsts(),
> show_devs() and show_call() in the asus-wmi driver.
>
> I would actually prefer to lock down all files by default and have the
> the files unlocked by the creator. This is tricky to manage correctly,
> though, as there are 19 creation functions and ~1600 call sites (some of
> them in loops scanning tables).
Why not just disable debugfs entirely? This half-hearted way to sorta
lock it down is odd, it is meant to not be there at all, nothing in your
normal system should ever depend on it.
So again just don't allow it to be mounted at all, much simpler and more
obvious as to what is going on.
thanks,
greg k-h
On Wed, Apr 11, 2018 at 08:50:00PM +0200, Miguel Ojeda wrote:
> On Wed, Apr 11, 2018 at 7:37 PM, Randy Dunlap <[email protected]> wrote:
> > On 04/11/2018 09:24 AM, David Howells wrote:
> >
> >> diff --git a/security/lock_down.c b/security/lock_down.c
> >> new file mode 100644
> >> index 000000000000..f35ffdd096ad
> >> --- /dev/null
> >> +++ b/security/lock_down.c
> >> @@ -0,0 +1,65 @@
> >> +/* 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.
> >
> > fsf.org spells that Licence word as License. :)
>
> Given this is a new file, simply use the SPF identifier on top.
checkpatch.pl should now check for this as well.
Eric W. Biederman <[email protected]> wrote:
> Why is mounting debugfs allowed at all? Last I checked (it has been a while)
> the code quality of debugfs was fine for debugging but debugfs was not
> safe to mount on a production system.
>
> Maybe the code quality is better now but for a filesystem that is
> not supposed to be needed for developers letting us mount debugfs
> seems odd.
I agree. But debugfs has been abused and it seems that there are some things
that use it as an interface between a kernel driver and the userspace side.
David
Greg KH <[email protected]> wrote:
> Why not just disable debugfs entirely? This half-hearted way to sorta
> lock it down is odd, it is meant to not be there at all, nothing in your
> normal system should ever depend on it.
>
> So again just don't allow it to be mounted at all, much simpler and more
> obvious as to what is going on.
Yeah, I agree - and then I got complaints because it seems that it's been
abused to allow drivers and userspace components to communicate.
David
On Wed, 2018-04-11 at 14:00 -0500, Eric W. Biederman wrote:
> David Howells <[email protected]> writes:
>
> > From: Matthew Garrett <[email protected]>
> >
> > The kexec_load() syscall 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_load() in this situation.
> >
> > This does not affect kexec_file_load() syscall which can check for a
> > signature on the image to be booted.
>
> Maybing I am missing it but I am not seeing anything that would require
> kexec_file_load be configured such that it checks the loaded kernel.
>
> Without that I don't see the point of disabling kexec_load.
>
> Nacked-by: "Eric W. Biederman" <[email protected]>
The IMA "secure boot" policy requires the kexec image to be signed.
This call to kernel_is_locked_down() could be replaced with a call
to security_kernel_read_file(NULL, READING_KEXEC_IMAGE).
It would be similar to the existing init_module syscall calling
security_kernel_read_file().
Mimi
David Howells <[email protected]> writes:
> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs. Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead. The following changes are made:
>
> (1) chmod and chown are disallowed on debugfs objects (though the root dir
> can be modified by mount and remount, but I'm not worried about that).
>
> (2) When the kernel is locked down, only files with the following criteria
> are permitted to be opened:
>
> - The file must have mode 00444
> - The file must not have ioctl methods
> - The file must not have mmap
>
> (3) When the kernel is locked down, files may only be opened for reading.
>
> Normal device interaction should be done through configfs, sysfs or a
> miscdev, not debugfs.
> Note that this makes it unnecessary to specifically lock down show_dsts(),
> show_devs() and show_call() in the asus-wmi driver.
>
> I would actually prefer to lock down all files by default and have the
> the files unlocked by the creator. This is tricky to manage correctly,
> though, as there are 19 creation functions and ~1600 call sites (some of
> them in loops scanning tables).
Why is mounting debugfs allowed at all? Last I checked (it has been a while)
the code quality of debugfs was fine for debugging but debugfs was not
safe to mount on a production system.
Maybe the code quality is better now but for a filesystem that is
not supposed to be needed for developers letting us mount debugfs
seems odd.
Eric
On Wed, Apr 11, 2018 at 7:37 PM, Randy Dunlap <[email protected]> wrote:
> On 04/11/2018 09:24 AM, David Howells wrote:
>
>> diff --git a/security/lock_down.c b/security/lock_down.c
>> new file mode 100644
>> index 000000000000..f35ffdd096ad
>> --- /dev/null
>> +++ b/security/lock_down.c
>> @@ -0,0 +1,65 @@
>> +/* 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.
>
> fsf.org spells that Licence word as License. :)
Given this is a new file, simply use the SPF identifier on top.
Cheers,
Miguel
On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
> Greg KH <[email protected]> wrote:
>
> > Why not just disable debugfs entirely? This half-hearted way to sorta
> > lock it down is odd, it is meant to not be there at all, nothing in your
> > normal system should ever depend on it.
> >
> > So again just don't allow it to be mounted at all, much simpler and more
> > obvious as to what is going on.
>
> Yeah, I agree - and then I got complaints because it seems that it's been
> abused to allow drivers and userspace components to communicate.
With in-kernel code? Please let me know and I'll go fix it up to not
allow that, as that is not ok.
I do know of some bad examples of out-of-tree code abusing debugfs to do
crazy things (battery level monitoring?), but that's their own fault...
debugfs is for DEBUGGING! For anything you all feel should be "secure",
then just disable it entirely.
thanks,
greg k-h
On April 11, 2018 8:09 PM, Linus Torvalds <[email protected]> wrote:
> On Wed, Apr 11, 2018 at 9:24 AM, David Howells [email protected] wrote:
>
> > 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:
> >
> > - /dev/mem and similar
> > - Loading of unauthorised modules
> > - Fiddling with MSR registers
> > - Suspend to disk managed by the kernel
> > - Use of device DMA
>
> So what I stlll absolutely detest about this series is that I think
>
> many of these things should simply be done as separate config options.
>
> For example, if the distro is sure that it doesn't need /dev/mem, then
>
> why the hell is this tied to "lockdown" that then may have to be
>
> disabled because other changes may not be acceptable (eg people may
>
> need that device DMA, or whatever).
>
> If that /dev/mem access prevention was just instead done as an even
>
> stricter mode of the existing CONFIG_STRICT_DEVMEM, it could just be
>
> enabled unconditionally.
CONFIG_DEVMEM=n
>
> So none of these patches raise my hackles per se. But what continues
>
> to makes me very very uncomfortable is how this is all tied together.
>
> Why is this one magical mode that then - because it has such a big
>
> impact - has to be enabled/disabled as a single magical mode and with
>
> very odd rules?
>
> I think a lot of people would be happier if this wasn't so incestuous
>
> and mixing together independent things under one name, and one flag.
>
> I think a lot of the secure boot problems were exacerbated by that mixup.
>
> So I would seriously ask that the distros that have been using these
>
> patches look at which parts of lockdown they could make unconditional
>
> (because it doesn't break machines), and which ones need that escape
>
> clause.
>
> Linus
>
Jordan
Eric W. Biederman <[email protected]> wrote:
> Maybing I am missing it but I am not seeing anything that would require
> kexec_file_load be configured such that it checks the loaded kernel.
>
> Without that I don't see the point of disabling kexec_load.
I meant to remove this patch too.
David
On Wed, Apr 11, 2018 at 2:05 PM, Jordan Glover
<[email protected]> wrote:
>>
>> If that /dev/mem access prevention was just instead done as an even
>> stricter mode of the existing CONFIG_STRICT_DEVMEM, it could just be
>> enabled unconditionally.
>
> CONFIG_DEVMEM=n
It's actually CONFIG_DEVMEM, CONFIG_DEVKMEM and CONFIG_DEVPORT, it's
just not obvious from the patch.
But the important part is this part:
>> So I would seriously ask that the distros that have been using these
>> patches look at which parts of lockdown they could make unconditional
>> (because it doesn't break machines), and which ones need that escape
>> clause.
.. because I get the feeling that not a lot of people have actually
been testing this, because "turn off secure boot" is such a universal
thing when people boot Linux.
So it's really the whole claim that distributions have been running
for this for the last five years that I wonder about, and how often
people end up being told: "just disable secure boot":.
But if people really don't need DEVMEM/DEVKMEM/DEVPORT, maybe we
should just disable them in the default configs, and consider them
legacy.
I'm just surprised. I suspect a lot of people end up actually using
devmem as a fallback for dmidecode etc. Maybe those people don't boot
with EFI secure mode, but if so that just shows that this whole
"hardening" is just security theater.
Linus
On Wed, Apr 11, 2018 at 1:33 PM, Greg KH <[email protected]> wrote:
> On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
>> Greg KH <[email protected]> wrote:
>>
>> > Why not just disable debugfs entirely? This half-hearted way to sorta
>> > lock it down is odd, it is meant to not be there at all, nothing in your
>> > normal system should ever depend on it.
>> >
>> > So again just don't allow it to be mounted at all, much simpler and more
>> > obvious as to what is going on.
>>
>> Yeah, I agree - and then I got complaints because it seems that it's been
>> abused to allow drivers and userspace components to communicate.
>
> With in-kernel code? Please let me know and I'll go fix it up to not
> allow that, as that is not ok.
>
> I do know of some bad examples of out-of-tree code abusing debugfs to do
> crazy things (battery level monitoring?), but that's their own fault...
>
> debugfs is for DEBUGGING! For anything you all feel should be "secure",
> then just disable it entirely.
>
Debugfs is very, very useful for, ahem, debugging. I really think
this is an example of why we should split lockdown into the read and
write varieties and allow mounting and reading debugfs when only write
is locked down.
On Wed, Apr 11, 2018 at 9:24 AM, David Howells <[email protected]> wrote:
>
> (*) CONFIG_LOCK_DOWN_KERNEL
>
> This makes lockdown available and applies it to all the points that
> need to be locked down if the mode is set. Lockdown mode can be
> enabled by providing:
>
> lockdown=1
By doing this, you are basically committing to making the
protect-kernel-integrity vs protect-kernel-secrecy split be a
second-class citizen if it gets added.
How about lockdown=integrity_and_secrecy or lockdown=2 if you feel
like using numbers?
On Wed, Apr 11, 2018 at 07:54:12PM -0700, Andy Lutomirski wrote:
> On Wed, Apr 11, 2018 at 1:33 PM, Greg KH <[email protected]> wrote:
> > On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
> >> Greg KH <[email protected]> wrote:
> >>
> >> > Why not just disable debugfs entirely? This half-hearted way to sorta
> >> > lock it down is odd, it is meant to not be there at all, nothing in your
> >> > normal system should ever depend on it.
> >> >
> >> > So again just don't allow it to be mounted at all, much simpler and more
> >> > obvious as to what is going on.
> >>
> >> Yeah, I agree - and then I got complaints because it seems that it's been
> >> abused to allow drivers and userspace components to communicate.
> >
> > With in-kernel code? Please let me know and I'll go fix it up to not
> > allow that, as that is not ok.
> >
> > I do know of some bad examples of out-of-tree code abusing debugfs to do
> > crazy things (battery level monitoring?), but that's their own fault...
> >
> > debugfs is for DEBUGGING! For anything you all feel should be "secure",
> > then just disable it entirely.
> >
>
> Debugfs is very, very useful for, ahem, debugging. I really think
> this is an example of why we should split lockdown into the read and
> write varieties and allow mounting and reading debugfs when only write
> is locked down.
Ok, but be sure that there are no "secrets" in those debugging files if
you really buy into the whole "lock down" mess...
Really, it's easier to just disable the whole thing.
greg k-h
On Wed, 2018-04-11 at 16:09 -0400, Mimi Zohar wrote:
> On Wed, 2018-04-11 at 14:00 -0500, Eric W. Biederman wrote:
> > David Howells <[email protected]> writes:
> >
> > > From: Matthew Garrett <[email protected]>
> > >
> > > The kexec_load() syscall 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_load() in this situation.
> > >
> > > This does not affect kexec_file_load() syscall which can check for a
> > > signature on the image to be booted.
> >
> > Maybing I am missing it but I am not seeing anything that would require
> > kexec_file_load be configured such that it checks the loaded kernel.
> >
> > Without that I don't see the point of disabling kexec_load.
> >
> > Nacked-by: "Eric W. Biederman" <[email protected]>
>
> The IMA "secure boot" policy requires the kexec image to be signed.
> This call to kernel_is_locked_down() could be replaced with a call
> to security_kernel_read_file(NULL, READING_KEXEC_IMAGE).
>
> It would be similar to the existing init_module syscall calling
> security_kernel_read_file().
David, enabling the IMA-appraisal "secure boot" policy should probably
not be dependent on lockdown either.
Mimi
On Wed, Apr 11, 2018, 5:38 PM Linus Torvalds
<[email protected]> wrote:
>
> On Wed, Apr 11, 2018 at 2:05 PM, Jordan Glover
> <[email protected]> wrote:
> >>
> >> If that /dev/mem access prevention was just instead done as an even
> >> stricter mode of the existing CONFIG_STRICT_DEVMEM, it could just be
> >> enabled unconditionally.
> >
> > CONFIG_DEVMEM=n
>
> It's actually CONFIG_DEVMEM, CONFIG_DEVKMEM and CONFIG_DEVPORT, it's
> just not obvious from the patch.
>
> But the important part is this part:
>
> >> So I would seriously ask that the distros that have been using these
> >> patches look at which parts of lockdown they could make unconditional
> >> (because it doesn't break machines), and which ones need that escape
> >> clause.
>
> .. because I get the feeling that not a lot of people have actually
> been testing this, because "turn off secure boot" is such a universal
> thing when people boot Linux.
>
> So it's really the whole claim that distributions have been running
> for this for the last five years that I wonder about, and how often
> people end up being told: "just disable secure boot":.
Very rarely in my experience. And the one time that we sent a kernel
to updates-testing that was signed with the test key instead of the
real key, we had a surprisingly high number of reports from users that
it was broken before the update even got synched to mirrors. So we
don't have actual numbers of users running active secure boot with
Fedora, but we do know it is more than we expected. The majority of
people who do run into issues are those running out of tree modules,
who haven't imported any sort of key for local signing. This isn't
like SELinux was at launch where it was so invasive that a large
number of users instinctively turned it off with every installation, I
would guess even people who turned it off in the past, don't even
think about it when they get a new machine and leave it on.
> But if people really don't need DEVMEM/DEVKMEM/DEVPORT, maybe we
> should just disable them in the default configs, and consider them
> legacy.
>
> I'm just surprised. I suspect a lot of people end up actually using
> devmem as a fallback for dmidecode etc. Maybe those people don't boot
> with EFI secure mode, but if so that just shows that this whole
> "hardening" is just security theater.
>
> Linus
On Thu, Apr 12, 2018 at 1:23 AM, Greg KH <[email protected]> wrote:
> On Wed, Apr 11, 2018 at 07:54:12PM -0700, Andy Lutomirski wrote:
>> On Wed, Apr 11, 2018 at 1:33 PM, Greg KH <[email protected]> wrote:
>> > On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
>> >> Greg KH <[email protected]> wrote:
>> >>
>> >> > Why not just disable debugfs entirely? This half-hearted way to sorta
>> >> > lock it down is odd, it is meant to not be there at all, nothing in your
>> >> > normal system should ever depend on it.
>> >> >
>> >> > So again just don't allow it to be mounted at all, much simpler and more
>> >> > obvious as to what is going on.
>> >>
>> >> Yeah, I agree - and then I got complaints because it seems that it's been
>> >> abused to allow drivers and userspace components to communicate.
>> >
>> > With in-kernel code? Please let me know and I'll go fix it up to not
>> > allow that, as that is not ok.
>> >
>> > I do know of some bad examples of out-of-tree code abusing debugfs to do
>> > crazy things (battery level monitoring?), but that's their own fault...
>> >
>> > debugfs is for DEBUGGING! For anything you all feel should be "secure",
>> > then just disable it entirely.
>> >
>>
>> Debugfs is very, very useful for, ahem, debugging. I really think
>> this is an example of why we should split lockdown into the read and
>> write varieties and allow mounting and reading debugfs when only write
>> is locked down.
>
> Ok, but be sure that there are no "secrets" in those debugging files if
> you really buy into the whole "lock down" mess...
>
> Really, it's easier to just disable the whole thing.
>
I mostly agree with your sentiment. I'm saying that, for most uses, I
*don't* buy into the idea that a normal secure-boot-supporting distro
should block debugfs. I sometimes like to ask people who report
problems to send me the contents of such-and-such file in debugfs, and
I think it should keep working. Blocking write access to debugfs is
mostly sensible for a lockdown system, but blocking read only makes
sense if you're worried about straight-up bugs or if you think that
debugfs contains protection-worthy secrets.
What I want to see is:
lockdown=protect_integrity: debugfs is read-only, bpf and perf are
unrestricted, iopl and ioperm are disabled, etc.
lockdown=protect_integrity_and_secrecy: debugfs is gone, bpf and perf
are restricted, plus all the restrictions from
lockdown=protect_integrity
Distros should strongly prefer lockdown=protect_integrity (or
lockdown=off) by default. lockdown=protect_integrity_and_secrecy is
for custom setups, embedded applications, etc.
--Andy
On Thu, Apr 12, 2018 at 6:09 AM, Justin Forbes <[email protected]> wrote:
> On Wed, Apr 11, 2018, 5:38 PM Linus Torvalds
> <[email protected]> wrote:
>>
>> So it's really the whole claim that distributions have been running
>> for this for the last five years that I wonder about, and how often
>> people end up being told: "just disable secure boot":.
>
> Very rarely in my experience.
Good. Do you have a handle on the reasons?
Because I'm assuming it's not /dev/{mem,kmem,port}? Because I'd really
be happier if we just say "those are legacy, don't enable them at all
for modern distros".
That way they'd _stay_ disabled even if somebody cannot handle the
other limitations, like DMA etc.
Linus
On Wed 2018-04-11 17:24:52, David Howells wrote:
> From: Kyle McMartin <[email protected]>
>
> Make an option to provide a sysrq key that will lift the kernel lockdown,
> thereby allowing the running kernel image to be accessed and modified.
>
> On x86 this is triggered with SysRq+x, but this key may not be available on
> all arches, so it is set by setting LOCKDOWN_LIFT_KEY in asm/setup.h.
> Since this macro must be defined in an arch to be able to use this facility
> for that arch, the Kconfig option is restricted to arches that support it.
>
> Signed-off-by: Kyle McMartin <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]
Is that good idea? Magic sysrq was meant for debugging, not for
toggling options like that. Distros are expected to turn it off.
It also works over serial consoles etc, being able to toggle security
options from serial is surprising...
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -487,6 +487,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 */
What about x86-32?
> +static struct sysrq_key_op lockdown_lift_sysrq_op = {
> + .handler = sysrq_handle_lockdown_lift,
> + .help_msg = "unSB(x)",
> + .action_msg = "Disabling Secure Boot restrictions",
> + .enable_mask = SYSRQ_DISABLE_USERSPACE,
> +};
I'd remove secure boot mentions here.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Wed 2018-04-11 17:25:25, David Howells wrote:
> 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.
I'd rather see hibernation fixed than disabled like this.
I believe Jiri Kosina may have some patches for that?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Wed 2018-04-11 17:27:16, David Howells wrote:
> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs. Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead. The following changes are made:
>
> (1) chmod and chown are disallowed on debugfs objects (though the root dir
> can be modified by mount and remount, but I'm not worried about that).
This has nothing to do with the lockdown goals, right? I find chown of
such files quite nice, to allow debugging without doing sudo all the time.
> (2) When the kernel is locked down, only files with the following criteria
> are permitted to be opened:
>
> - The file must have mode 00444
> - The file must not have ioctl methods
> - The file must not have mmap
Dunno. Would not it be nicer to go through the debugfs files and split
them into safe/unsafe varieties?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Pavel Machek <[email protected]> wrote:
> > (1) chmod and chown are disallowed on debugfs objects (though the root dir
> > can be modified by mount and remount, but I'm not worried about that).
>
> This has nothing to do with the lockdown goals, right? I find chown of
> such files quite nice, to allow debugging without doing sudo all the time.
It allows someone to give everyone access to files that should perhaps only be
accessible by root. Besides, if you disable lockdown then you can do this if
you want.
> > (2) When the kernel is locked down, only files with the following criteria
> > are permitted to be opened:
> >
> > - The file must have mode 00444
> > - The file must not have ioctl methods
> > - The file must not have mmap
>
> Dunno. Would not it be nicer to go through the debugfs files and split
> them into safe/unsafe varieties?
Whilst that is a laudable idea, there are at least a couple of thousand files
to analyse, some of them doing weird stuff in drivers that aren't easy to
understand, and some of them with lists of files, some of which might be safe
and some of which aren't safe. Even some reads that look like they ought to
be safe have side effects (such as read-and-reset counters).
Besides, define 'safe'. Is reading a particular reg on a device and returning
the value through a debugfs read safe, for example? What about files where
reading is harmless, but writing needs to be disallowed?
I did try initially passing in a flag to say whether something was safe or
not, abusing an inode flag to store it since debugfs uses a plain inode
struct, but then I realised just how many ways there are to create debugfs
files and it started to get out of hand. My initial idea also included
locking everything down by default and marking things unlocked on an as-needed
basis.
If you have the time to audit all these files, then that would be great.
I lean more towards the lock debugfs down entirely side.
David
Pavel Machek <[email protected]> wrote:
> > 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.
>
> I'd rather see hibernation fixed than disabled like this.
The problem is that you have to store the hibernated kernel image encrypted,
but you can't store the decryption key on disk unencrypted or you've just
wasted the effort.
So the firmware has to unlock the image, asking the user for a password to
unlock the key.
David
On Thu, Apr 19, 2018 at 7:38 AM, David Howells <[email protected]> wrote:
> Pavel Machek <[email protected]> wrote:
>
>> > 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.
>>
>> I'd rather see hibernation fixed than disabled like this.
>
> The problem is that you have to store the hibernated kernel image encrypted,
> but you can't store the decryption key on disk unencrypted or you've just
> wasted the effort.
>
> So the firmware has to unlock the image, asking the user for a password to
> unlock the key.
Why firmware?
Either the boot kernel could figure out how to ask for a password (or
unseal using the TPM) or we could defer this to userspace. The latter
should already work using kexec-jump, no?
On Thu 2018-04-19 15:38:53, David Howells wrote:
> Pavel Machek <[email protected]> wrote:
>
> > > 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.
> >
> > I'd rather see hibernation fixed than disabled like this.
>
> The problem is that you have to store the hibernated kernel image encrypted,
> but you can't store the decryption key on disk unencrypted or you've just
> wasted the effort.
That's not how the crypto needs to work. Talk to Jiri Kosina, ok?
Firmware gives you a key, you keep it secret, use it to sign the
hibernation image on suspend, and verify the signature on resume. Or
something like that.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Thursday, April 26, 2018 9:26:46 AM CEST Pavel Machek wrote:
> On Thu 2018-04-19 15:38:53, David Howells wrote:
> > Pavel Machek <[email protected]> wrote:
> >
> > > > 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.
> > >
> > > I'd rather see hibernation fixed than disabled like this.
> >
> > The problem is that you have to store the hibernated kernel image encrypted,
> > but you can't store the decryption key on disk unencrypted or you've just
> > wasted the effort.
>
> That's not how the crypto needs to work. Talk to Jiri Kosina, ok?
>
> Firmware gives you a key, you keep it secret, use it to sign the
> hibernation image on suspend, and verify the signature on resume. Or
> something like that.
A simplified approach might be to encrypt the image during hibernation
using a user-provided passphrase and then ask for that passphrase during
resume to decrypt the image.
The attacker would then need to know the passphrase to substitute their
own image for the original one successfully.
On Thu, 26 Apr 2018, Pavel Machek wrote:
> That's not how the crypto needs to work. Talk to Jiri Kosina, ok?
Yeah, Joey Lee (adding to CC) implemented it here:
https://lkml.org/lkml/2015/8/11/47
I think there have been more respins, Joey definitely knows more details
and status quo.
The design is specifically tailored for secure-boot environments though.
--
Jiri Kosina
SUSE Labs
On Thu 2018-04-19 15:35:47, David Howells wrote:
> Pavel Machek <[email protected]> wrote:
>
> > > (1) chmod and chown are disallowed on debugfs objects (though the root dir
> > > can be modified by mount and remount, but I'm not worried about that).
> >
> > This has nothing to do with the lockdown goals, right? I find chown of
> > such files quite nice, to allow debugging without doing sudo all the time.
>
> It allows someone to give everyone access to files that should perhaps only be
> accessible by root. Besides, if you disable lockdown then you can do this if
> you want.
As I said this has nothing to do with lockdown, so does not belong in
this series. (And besides, it is bad idea.)
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi experts,
Sorry for I missed this discussion...
On Thu, Apr 26, 2018 at 10:20:29AM +0200, Jiri Kosina wrote:
> On Thu, 26 Apr 2018, Pavel Machek wrote:
>
> > That's not how the crypto needs to work. Talk to Jiri Kosina, ok?
>
> Yeah, Joey Lee (adding to CC) implemented it here:
>
> https://lkml.org/lkml/2015/8/11/47
>
> I think there have been more respins, Joey definitely knows more details
> and status quo.
>
> The design is specifically tailored for secure-boot environments though.
>
I am working on the next version of hibernation encryption and authentication:
https://github.com/joeyli/linux-s4sign/wiki
My plan is:
- Hibernation encryption:
There is a draft patch to encrypt image by ctr(aes). This patch works
with the first version of hibernation verification:
https://github.com/joeyli/linux-s4sign/commit/6a9a0113bb221c036ebd0f6321b7191283fe4929
- Adapt hibernation to key retention service:
- Using the encrypted key to derive encrypt key and auth key to
encrypt and hmac snapshot image. Put the encrypted key in the image
header of snapshot.
- The encrypted key will be encrypted by KMK (kernel master key). Either
trusted key(sealed by TPM) or EFI key (explain in later) can be the KMK.
If there have appropriate UI support in initrd, user key can also be
the KMK.
- Similar with the enrolling EVM key, but more earler:
The systemd and dracut must be changed for enrolling kernel master key
before the swap partition be mounted.
- EFI key:
- A new master key type to key retention service.
- It can be a new option beyond trusted key(TPM) and user key.
- EFI stub generates a random key and stores in EFI boot service
variable:
- This random key in boot variable can be called ERK (EFI Root Key)
- The ERK is secure when secure boot enabled.
- User must aware and enable secure boot by themself if they want.
- ERK can be a secret to encrypt a random number for generate a EFI key
- The EFI key can be used by hibernation encryption/authentication.
- The EFI key can be a master key to generate a encrypted key for EVM.
- Rescue mechanism for ERK:
- The ERK may be regenerated after the old ERK be erased by firmware update
or firmware recovery.
- Current idea is using the public key in first/second trusted keyring
to encrypt the ERK for backup. User can enroll the EFI key with old ERK to
request kernel to re-encrypt the EFI key with new ERK.
Thanks a lot!
Joey Lee