Updates: Based on Andy's feedback, lockdown is now a tristate and can be
made stricter at runtime. The states are "none", "integrity" and
"confidentiality". "none" results in no behavioural change, "integrity"
enables features that prevent untrusted code from being run in ring 0,
and "confidentiality" is a superset of "integrity" that also disables
features that may be used to extract secret information from the kernel
at runtime. I've also modified the bpf patch so that only the calls
documented as giving the ability to read in-kernel data are locked down,
rather than all functionality being disabled - I'm not a bpf expert so
would gladly go for further review here. Long term, it'd be preferable
to be able to tag secrets held by the kernel and grant access to
everything else, but I'm open to further feedback here. And at Greg's
request, debugfs is now largely disabled once the system is locked down.
In the general case, I'd expect distributions to opt for nothing
stricter than "integrity" - "confidentiality" seems more suitable for
more special-case scenarios.
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: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Cc: [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 b08dc50f9f26..67b85939b1bd 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -786,6 +786,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", LOCKDOWN_INTEGRITY))
+ return -EPERM;
return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
}
--
2.21.0.392.gf8f6787159e-goog
From: Dave Young <[email protected]>
Kexec reboot in case secure boot being enabled does not keep the secure
boot mode in new kernel, so later one can load unsigned kernel via legacy
kexec_load. In this state, the system is missing the protections provided
by secure boot.
Adding a patch to fix this by retain the secure_boot flag in original
kernel.
secure_boot flag in boot_params is set in EFI stub, but kexec bypasses the
stub. Fixing this issue by copying secure_boot flag across kexec reboot.
Signed-off-by: Dave Young <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
cc: [email protected]
---
arch/x86/kernel/kexec-bzimage64.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 278cd07228dd..d49554b948fd 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -179,6 +179,7 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
if (efi_enabled(EFI_OLD_MEMMAP))
return 0;
+ params->secure_boot = boot_params.secure_boot;
ei->efi_loader_signature = current_ei->efi_loader_signature;
ei->efi_systab = current_ei->efi_systab;
ei->efi_systab_hi = current_ei->efi_systab_hi;
--
2.21.0.392.gf8f6787159e-goog
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: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
cc: [email protected]
Cc: [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 2d8b60a3c86b..99e13fd13237 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", LOCKDOWN_INTEGRITY))
+ return -EPERM;
+
lock_system_sleep();
if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
--
2.21.0.392.gf8f6787159e-goog
From: Jiri Bohac <[email protected]>
When KEXEC_SIG is not enabled, kernel should not load images through
kexec_file systemcall if the kernel is locked down.
[Modified by David Howells to fit with modifications to the previous patch
and to return -EPERM if the kernel is locked down for consistency with
other lockdowns. Modified by Matthew Garrett to remove the IMA
integration, which will be replaced by integrating with the IMA
architecture policy patches.]
Signed-off-by: Jiri Bohac <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Reviewed-by: Jiri Bohac <[email protected]>
cc: [email protected]
---
kernel/kexec_file.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 67f3a866eabe..a1cc37c8b43b 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -239,6 +239,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
}
ret = 0;
+
+ if (kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY)) {
+ ret = -EPERM;
+ goto out;
+ }
+
break;
/* All other errors are fatal, including nomem, unparseable
--
2.21.0.392.gf8f6787159e-goog
From: Matthew Garrett <[email protected]>
IO port access would permit users to gain access to PCI configuration
registers, which in turn (on a lot of hardware) give access to MMIO
register space. This would potentially permit root to trigger arbitrary
DMA, so lock it down by default.
This also implicitly locks down the KDADDIO, KDDELIO, KDENABIO and
KDDISABIO console ioctls.
Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---
arch/x86/kernel/ioport.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 0fe1c8782208..febbd7eb847c 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", LOCKDOWN_INTEGRITY)))
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", LOCKDOWN_INTEGRITY))
return -EPERM;
}
regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) |
--
2.21.0.392.gf8f6787159e-goog
From: Matthew Garrett <[email protected]>
Writing to MSRs should not be allowed if the kernel is locked down, since
it could lead to execution of arbitrary code in kernel mode. Based on a
patch by Kees Cook.
Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: Kees Cook <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>
cc: [email protected]
---
arch/x86/kernel/msr.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
index 4588414e2561..731be1be52b6 100644
--- a/arch/x86/kernel/msr.c
+++ b/arch/x86/kernel/msr.c
@@ -84,6 +84,9 @@ static ssize_t msr_write(struct file *file, const char __user *buf,
int err = 0;
ssize_t bytes = 0;
+ if (kernel_is_locked_down("Direct MSR access", LOCKDOWN_INTEGRITY))
+ return -EPERM;
+
if (count % 8)
return -EINVAL; /* Invalid chunk size */
@@ -135,6 +138,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",
+ LOCKDOWN_INTEGRITY)) {
+ err = -EPERM;
+ break;
+ }
err = wrmsr_safe_regs_on_cpu(cpu, regs);
if (err)
break;
--
2.21.0.392.gf8f6787159e-goog
From: David Howells <[email protected]>
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]>
Signed-off-by: Matthew Garrett <[email protected]>
cc: Jiri Slaby <[email protected]>
Cc: [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 d4cca5bdaf1c..65b67f0d4386 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -842,6 +842,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", LOCKDOWN_INTEGRITY)) {
+ retval = -EPERM;
+ goto exit;
+ }
+
if (!capable(CAP_SYS_ADMIN)) {
retval = -EPERM;
if (change_irq || change_port ||
--
2.21.0.392.gf8f6787159e-goog
From: David Howells <[email protected]>
There are some bpf functions can be used to read kernel memory:
bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
private keys in kernel memory (e.g. the hibernation image signing key) to
be read by an eBPF program and kernel memory to be altered without
restriction. Disable them if the kernel has been locked down in
confidentiality mode.
Suggested-by: Alexei Starovoitov <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
cc: [email protected]
cc: Chun-Yi Lee <[email protected]>
cc: Alexei Starovoitov <[email protected]>
Cc: Daniel Borkmann <[email protected]>
---
kernel/trace/bpf_trace.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 8b068adb9da1..9e8eda605b5e 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -137,6 +137,9 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
{
int ret;
+ if (kernel_is_locked_down("BPF", LOCKDOWN_CONFIDENTIALITY))
+ return -EINVAL;
+
ret = probe_kernel_read(dst, unsafe_ptr, size);
if (unlikely(ret < 0))
memset(dst, 0, size);
@@ -156,6 +159,8 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
u32, size)
{
+ if (kernel_is_locked_down("BPF", LOCKDOWN_CONFIDENTIALITY))
+ return -EINVAL;
/*
* Ensure we're in user context which is safe for the helper to
* run. This helper has no business in a kthread.
@@ -207,6 +212,9 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
char buf[64];
int i;
+ if (kernel_is_locked_down("BPF", LOCKDOWN_CONFIDENTIALITY))
+ return -EINVAL;
+
/*
* bpf_check()->check_func_arg()->check_stack_boundary()
* guarantees that fmt points to bpf program stack,
@@ -535,6 +543,9 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
{
int ret;
+ if (kernel_is_locked_down("BPF", LOCKDOWN_CONFIDENTIALITY))
+ return -EINVAL;
+
/*
* The strncpy_from_unsafe() call will likely not fill the entire
* buffer, but that's okay in this circumstance as we're probing
--
2.21.0.392.gf8f6787159e-goog
From: David Howells <[email protected]>
Provide a single call to allow kernel code to determine whether the system
should be locked down, thereby disallowing various accesses that might
allow the running kernel image to be changed including the loading of
modules that aren't validly signed with a key we recognise, fiddling with
MSR registers and disallowing hibernation.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
Documentation/ABI/testing/lockdown | 19 +++
.../admin-guide/kernel-parameters.txt | 9 ++
include/linux/kernel.h | 28 ++++
include/linux/security.h | 9 +-
init/main.c | 1 +
security/Kconfig | 39 +++++
security/Makefile | 3 +
security/lock_down.c | 147 ++++++++++++++++++
8 files changed, 254 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/testing/lockdown
create mode 100644 security/lock_down.c
diff --git a/Documentation/ABI/testing/lockdown b/Documentation/ABI/testing/lockdown
new file mode 100644
index 000000000000..5bd51e20917a
--- /dev/null
+++ b/Documentation/ABI/testing/lockdown
@@ -0,0 +1,19 @@
+What: security/lockdown
+Date: March 2019
+Contact: Matthew Garrett <[email protected]>
+Description:
+ If CONFIG_LOCK_DOWN_KERNEL is enabled, the kernel can be
+ moved to a more locked down state at runtime by writing to
+ this attribute. Valid values are:
+
+ integrity:
+ The kernel will disable functionality that allows
+ userland to modify the running kernel image, other
+ than through the loading or execution of appropriately
+ signed objects.
+
+ confidentiality:
+ The kernel will disable all functionality disabled by
+ the integrity mode, but additionally will disable
+ features that potentially permit userland to obtain
+ confidential information stored within the kernel.
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 91c0251fdb86..594d268d92ba 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2213,6 +2213,15 @@
lockd.nlm_udpport=M [NFS] Assign UDP port.
Format: <integer>
+ lockdown= [SECURITY]
+ { integrity | confidentiality }
+ Enable the kernel lockdown feature. If set to
+ integrity, kernel features that allow userland to
+ modify the running kernel are disabled. If set to
+ confidentiality, kernel features that allow userland
+ to extract confidential information from the kernel
+ are also disabled.
+
locktorture.nreaders_stress= [KNL]
Set the number of locking read-acquisition kthreads.
Defaults to being automatically set based on the
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8f0e68e250a7..30cf695719d5 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -340,6 +340,34 @@ static inline void refcount_error_report(struct pt_regs *regs, const char *err)
{ }
#endif
+enum lockdown_level {
+ LOCKDOWN_NONE,
+ LOCKDOWN_INTEGRITY,
+ LOCKDOWN_CONFIDENTIALITY,
+ LOCKDOWN_MAX,
+};
+
+#ifdef CONFIG_LOCK_DOWN_KERNEL
+extern bool __kernel_is_locked_down(const char *what,
+ enum lockdown_level level,
+ bool first);
+#else
+static inline bool __kernel_is_locked_down(const char *what,
+ enum lockdown_level level,
+ bool first)
+{
+ return false;
+}
+#endif
+
+#define kernel_is_locked_down(what, level) \
+ ({ \
+ static bool message_given; \
+ bool locked_down = __kernel_is_locked_down(what, level, !message_given); \
+ message_given = true; \
+ locked_down; \
+ })
+
/* Internal, do not use. */
int __must_check _kstrtoul(const char *s, unsigned int base, unsigned long *res);
int __must_check _kstrtol(const char *s, unsigned int base, long *res);
diff --git a/include/linux/security.h b/include/linux/security.h
index 13537a49ae97..b290946341a4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1798,5 +1798,12 @@ static inline void security_bpf_prog_free(struct bpf_prog_aux *aux)
#endif /* CONFIG_SECURITY */
#endif /* CONFIG_BPF_SYSCALL */
-#endif /* ! __LINUX_SECURITY_H */
+#ifdef CONFIG_LOCK_DOWN_KERNEL
+extern void __init init_lockdown(void);
+#else
+static inline void __init init_lockdown(void)
+{
+}
+#endif
+#endif /* ! __LINUX_SECURITY_H */
diff --git a/init/main.c b/init/main.c
index e2e80ca3165a..4c6cca9681c7 100644
--- a/init/main.c
+++ b/init/main.c
@@ -555,6 +555,7 @@ asmlinkage __visible void __init start_kernel(void)
boot_cpu_init();
page_address_init();
pr_notice("%s", linux_banner);
+ init_lockdown();
setup_arch(&command_line);
/*
* Set up the the initial canary and entropy after arch
diff --git a/security/Kconfig b/security/Kconfig
index 1d6463fb1450..593ff231eac6 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -229,6 +229,45 @@ 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. If lockdown support is enabled
+ and activated, the kernel will impose additional restrictions
+ intended to prevent uid 0 from being able to modify the running
+ kernel. This may break userland applications that rely on low-level
+ access to hardware.
+
+choice
+ prompt "Kernel default lockdown mode"
+ default LOCK_DOWN_KERNEL_FORCE_NONE
+ depends on LOCK_DOWN_KERNEL
+ help
+ The kernel can be configured to default to differing levels of
+ lockdown.
+
+config LOCK_DOWN_KERNEL_FORCE_NONE
+ bool "None"
+ help
+ No lockdown functionality is enabled by default. Lockdown may be
+ enabled via the kernel commandline or /sys/kernel/security/lockdown.
+
+config LOCK_DOWN_KERNEL_FORCE_INTEGRITY
+ bool "Integrity"
+ help
+ The kernel runs in integrity mode by default. Features that allow
+ the kernel to be modified at runtime are disabled.
+
+config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
+ bool "Confidentiality"
+ help
+ The kernel runs in confidentiality mode by default. Features that
+ allow the kernel to be modified at runtime or that permit userland
+ code to read confidential material held inside the kernel are
+ disabled.
+
+endchoice
+
source "security/selinux/Kconfig"
source "security/smack/Kconfig"
source "security/tomoyo/Kconfig"
diff --git a/security/Makefile b/security/Makefile
index c598b904938f..5ff090149c88 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -32,3 +32,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..0f9ef4c30aa8
--- /dev/null
+++ b/security/lock_down.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Lock down the kernel
+ *
+ * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells ([email protected])
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/security.h>
+#include <linux/export.h>
+
+static enum lockdown_level kernel_locked_down;
+
+char *lockdown_levels[LOCKDOWN_MAX] = {"none", "integrity", "confidentiality"};
+
+/*
+ * Put the kernel into lock-down mode.
+ */
+static int lock_kernel_down(const char *where, enum lockdown_level level)
+{
+ if (kernel_locked_down >= level)
+ return -EPERM;
+
+ kernel_locked_down = level;
+ pr_notice("Kernel is locked down from %s; see man kernel_lockdown.7\n",
+ where);
+ return 0;
+}
+
+static int __init lockdown_param(char *level)
+{
+ if (!level)
+ return -EINVAL;
+
+ if (strcmp(level, "integrity") == 0)
+ lock_kernel_down("command line", LOCKDOWN_INTEGRITY);
+ else if (strcmp(level, "confidentiality") == 0)
+ lock_kernel_down("command line", LOCKDOWN_CONFIDENTIALITY);
+ else
+ return -EINVAL;
+
+ return 0;
+}
+
+early_param("lockdown", lockdown_param);
+
+/*
+ * This must be called before arch setup code in order to ensure that the
+ * appropriate default can be applied without being overridden by the command
+ * line option.
+ */
+void __init init_lockdown(void)
+{
+#if defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY)
+ lock_kernel_down("Kernel configuration", LOCKDOWN_INTEGRITY);
+#elif defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY)
+ lock_kernel_down("Kernel configuration", LOCKDOWN_CONFIDENTIALITY);
+#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, enum lockdown_level level,
+ bool first)
+{
+ if ((kernel_locked_down >= level) && what && first)
+ pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
+ what);
+ return (kernel_locked_down >= level);
+}
+EXPORT_SYMBOL(__kernel_is_locked_down);
+
+static ssize_t lockdown_read(struct file *filp, char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ char temp[80];
+ int i, offset=0;
+
+ for (i = LOCKDOWN_NONE; i < LOCKDOWN_MAX; i++) {
+ if (lockdown_levels[i]) {
+ const char *label = lockdown_levels[i];
+
+ if (kernel_locked_down == i)
+ offset += sprintf(temp+offset, "[%s] ", label);
+ else
+ offset += sprintf(temp+offset, "%s ", label);
+ }
+ }
+
+ /* Convert the last space to a newline if needed. */
+ if (offset > 0)
+ temp[offset-1] = '\n';
+
+ return simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
+}
+
+static ssize_t lockdown_write(struct file *file, const char __user *buf,
+ size_t n, loff_t *ppos)
+{
+ char *state;
+ int i, len, err = 0;
+
+ state = memdup_user_nul(buf, n);
+ if (IS_ERR(state))
+ return PTR_ERR(state);
+
+ len = strlen(state);
+ if (state[len-1] == '\n') {
+ state[len-1] = '\0';
+ len--;
+ }
+
+ for (i = 0; i < LOCKDOWN_MAX; i++) {
+ const char *label = lockdown_levels[i];
+
+ if (label && len == strlen(label) && !strncmp(state, label, len))
+ err = lock_kernel_down("securityfs", i);
+ }
+
+ kfree(state);
+ return err ? err : n;
+}
+
+static const struct file_operations lockdown_ops = {
+ .read = lockdown_read,
+ .write = lockdown_write,
+};
+
+static int __init lockdown_secfs_init(void)
+{
+ struct dentry *dentry;
+
+ dentry = securityfs_create_file("lockdown", 0660, NULL, NULL,
+ &lockdown_ops);
+ if (IS_ERR(dentry))
+ return PTR_ERR(dentry);
+
+ return 0;
+}
+
+core_initcall(lockdown_secfs_init);
--
2.21.0.392.gf8f6787159e-goog
From: David Howells <[email protected]>
Print the content of current->comm in messages generated by lockdown to
indicate a restriction that was hit. This makes it a bit easier to find
out what caused the message.
The message now patterned something like:
Lockdown: <comm>: <what> is restricted; see man kernel_lockdown.7
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
include/linux/ima.h | 9 ++++++
kernel/kexec_file.c | 7 +++-
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_main.c | 2 +-
security/integrity/ima/ima_policy.c | 50 +++++++++++++++++++++++++++++
security/lock_down.c | 4 +--
6 files changed, 70 insertions(+), 4 deletions(-)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index b5e16b8c50b7..05921227d700 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -127,4 +127,13 @@ static inline int ima_inode_removexattr(struct dentry *dentry,
return 0;
}
#endif /* CONFIG_IMA_APPRAISE */
+
+#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING)
+extern bool ima_appraise_signature(enum kernel_read_file_id func);
+#else
+static inline bool ima_appraise_kexec_signature(enum kernel_read_file_id func)
+{
+ return false;
+}
+#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
#endif /* _LINUX_IMA_H */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index a1cc37c8b43b..7599039623a7 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -240,7 +240,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
ret = 0;
- if (kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY)) {
+ /* If IMA is guaranteed to appraise a signature on the kexec
+ * image, permit it even if the kernel is otherwise locked
+ * down.
+ */
+ if (!ima_appraise_signature(READING_KEXEC_IMAGE) &&
+ kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY)) {
ret = -EPERM;
goto out;
}
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index cc12f3449a72..fe03cc6f1ca4 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -115,6 +115,8 @@ struct ima_kexec_hdr {
u64 count;
};
+extern const int read_idmap[];
+
#ifdef CONFIG_HAVE_IMA_KEXEC
void ima_load_kexec_buffer(void);
#else
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 4ffac4f5c647..106f06dee9d1 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -442,7 +442,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
return 0;
}
-static const int read_idmap[READING_MAX_ID] = {
+const int read_idmap[READING_MAX_ID] = {
[READING_FIRMWARE] = FIRMWARE_CHECK,
[READING_FIRMWARE_PREALLOC_BUFFER] = FIRMWARE_CHECK,
[READING_MODULE] = MODULE_CHECK,
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 122797023bdb..f8f1cdb74a4f 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1341,3 +1341,53 @@ int ima_policy_show(struct seq_file *m, void *v)
return 0;
}
#endif /* CONFIG_IMA_READ_POLICY */
+
+#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING)
+/*
+ * ima_appraise_signature: whether IMA will appraise a given function using
+ * an IMA digital signature. This is restricted to cases where the kernel
+ * has a set of built-in trusted keys in order to avoid an attacker simply
+ * loading additional keys.
+ */
+bool ima_appraise_signature(enum kernel_read_file_id id)
+{
+ struct ima_rule_entry *entry;
+ bool found = false;
+ enum ima_hooks func;
+
+ if (id >= READING_MAX_ID)
+ return false;
+
+ func = read_idmap[id] ?: FILE_CHECK;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(entry, ima_rules, list) {
+ if (entry->action != APPRAISE)
+ continue;
+
+ /*
+ * A generic entry will match, but otherwise require that it
+ * match the func we're looking for
+ */
+ if (entry->func && entry->func != func)
+ continue;
+
+ /*
+ * We require this to be a digital signature, not a raw IMA
+ * hash.
+ */
+ if (entry->flags & IMA_DIGSIG_REQUIRED)
+ found = true;
+
+ /*
+ * We've found a rule that matches, so break now even if it
+ * didn't require a digital signature - a later rule that does
+ * won't override it, so would be a false positive.
+ */
+ break;
+ }
+
+ rcu_read_unlock();
+ return found;
+}
+#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
diff --git a/security/lock_down.c b/security/lock_down.c
index 0f9ef4c30aa8..6bcffd0bb200 100644
--- a/security/lock_down.c
+++ b/security/lock_down.c
@@ -70,8 +70,8 @@ bool __kernel_is_locked_down(const char *what, enum lockdown_level level,
bool first)
{
if ((kernel_locked_down >= level) && what && first)
- pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
- what);
+ pr_notice("Lockdown: %s: %s is restricted; see man kernel_lockdown.7\n",
+ current->comm, what);
return (kernel_locked_down >= level);
}
EXPORT_SYMBOL(__kernel_is_locked_down);
--
2.21.0.392.gf8f6787159e-goog
From: Matthew Garrett <[email protected]>
debugfs has not been meaningfully audited in terms of ensuring that
userland cannot trample over the kernel. At Greg's request, disable
access to it entirely when the kernel is locked down. This is done at
open() time rather than init time as the kernel lockdown status may be
made stricter at runtime.
Signed-off-by: Matthew Garrett <[email protected]>
Cc: [email protected]
---
fs/debugfs/file.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 4fce1da7db23..9ae12ef29ba0 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -142,6 +142,9 @@ static int open_proxy_open(struct inode *inode, struct file *filp)
const struct file_operations *real_fops = NULL;
int r;
+ if (kernel_is_locked_down("debugfs", LOCKDOWN_INTEGRITY))
+ return -EPERM;
+
r = debugfs_file_get(dentry);
if (r)
return r == -EIO ? -ENOENT : r;
@@ -267,6 +270,9 @@ static int full_proxy_open(struct inode *inode, struct file *filp)
struct file_operations *proxy_fops = NULL;
int r;
+ if (kernel_is_locked_down("debugfs", LOCKDOWN_INTEGRITY))
+ return -EPERM;
+
r = debugfs_file_get(dentry);
if (r)
return r == -EIO ? -ENOENT : r;
--
2.21.0.392.gf8f6787159e-goog
From: David Howells <[email protected]>
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]>
Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/params.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index ce89f757e6da..da1297f7cc26 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_notice("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", LOCKDOWN_INTEGRITY))
+ 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;
--
2.21.0.392.gf8f6787159e-goog
From: Jiri Bohac <[email protected]>
This is a preparatory patch for kexec_file_load() lockdown. A locked down
kernel needs to prevent unsigned kernel images from being loaded with
kexec_file_load(). Currently, the only way to force the signature
verification is compiling with KEXEC_VERIFY_SIG. This prevents loading
usigned images even when the kernel is not locked down at runtime.
This patch splits KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE.
Analogous to the MODULE_SIG and MODULE_SIG_FORCE for modules, KEXEC_SIG
turns on the signature verification but allows unsigned images to be
loaded. KEXEC_SIG_FORCE disallows images without a valid signature.
[Modified by David Howells such that:
(1) verify_pefile_signature() differentiates between no-signature and
sig-didn't-match in its returned errors.
(2) kexec fails with EKEYREJECTED and logs an appropriate message if
signature checking is enforced and an signature is not found, uses
unsupported crypto or has no matching key.
(3) kexec fails with EKEYREJECTED if there is a signature for which we
have a key, but signature doesn't match - even if in non-forcing mode.
(4) kexec fails with EBADMSG or some other error if there is a signature
which cannot be parsed - even if in non-forcing mode.
(5) kexec fails with ELIBBAD if the PE file cannot be parsed to extract
the signature - even if in non-forcing mode.
]
Signed-off-by: Jiri Bohac <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Reviewed-by: Jiri Bohac <[email protected]>
cc: [email protected]
---
arch/x86/Kconfig | 20 ++++++++---
crypto/asymmetric_keys/verify_pefile.c | 4 ++-
include/linux/kexec.h | 4 +--
kernel/kexec_file.c | 48 ++++++++++++++++++++++----
4 files changed, 61 insertions(+), 15 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4b4a7f32b68e..735d04a4b18f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2016,20 +2016,30 @@ config KEXEC_FILE
config ARCH_HAS_KEXEC_PURGATORY
def_bool KEXEC_FILE
-config KEXEC_VERIFY_SIG
+config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
depends on KEXEC_FILE
---help---
- This option makes kernel signature verification mandatory for
- the kexec_file_load() syscall.
- In addition to that option, you need to enable signature
+ This option makes the kexec_file_load() syscall check for a valid
+ signature of the kernel image. The image can still be loaded without
+ a valid signature unless you also enable KEXEC_SIG_FORCE, though if
+ there's a signature that we can check, then it must be valid.
+
+ In addition to this option, you need to enable signature
verification for the corresponding kernel image type being
loaded in order for this to work.
+config KEXEC_SIG_FORCE
+ bool "Require a valid signature in kexec_file_load() syscall"
+ depends on KEXEC_SIG
+ ---help---
+ This option makes kernel signature verification mandatory for
+ the kexec_file_load() syscall.
+
config KEXEC_BZIMAGE_VERIFY_SIG
bool "Enable bzImage signature verification support"
- depends on KEXEC_VERIFY_SIG
+ depends on KEXEC_SIG
depends on SIGNED_PE_FILE_VERIFICATION
select SYSTEM_TRUSTED_KEYRING
---help---
diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
index d178650fd524..4473cea1e877 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -100,7 +100,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
if (!ddir->certs.virtual_address || !ddir->certs.size) {
pr_debug("Unsigned PE binary\n");
- return -EKEYREJECTED;
+ return -ENODATA;
}
chkaddr(ctx->header_size, ddir->certs.virtual_address,
@@ -408,6 +408,8 @@ static int pefile_digest_pe(const void *pebuf, unsigned int pelen,
* (*) 0 if at least one signature chain intersects with the keys in the trust
* keyring, or:
*
+ * (*) -ENODATA if there is no signature present.
+ *
* (*) -ENOPKG if a suitable crypto module couldn't be found for a check on a
* chain.
*
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index b9b1bc5f9669..58b27c7bdc2b 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -125,7 +125,7 @@ typedef void *(kexec_load_t)(struct kimage *image, char *kernel_buf,
unsigned long cmdline_len);
typedef int (kexec_cleanup_t)(void *loader_data);
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
typedef int (kexec_verify_sig_t)(const char *kernel_buf,
unsigned long kernel_len);
#endif
@@ -134,7 +134,7 @@ struct kexec_file_ops {
kexec_probe_t *probe;
kexec_load_t *load;
kexec_cleanup_t *cleanup;
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
kexec_verify_sig_t *verify_sig;
#endif
};
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f1d0e00a3971..67f3a866eabe 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -90,7 +90,7 @@ int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
return kexec_image_post_load_cleanup_default(image);
}
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
static int kexec_image_verify_sig_default(struct kimage *image, void *buf,
unsigned long buf_len)
{
@@ -188,7 +188,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
const char __user *cmdline_ptr,
unsigned long cmdline_len, unsigned flags)
{
- int ret = 0;
+ const char *reason;
+ int ret;
void *ldata;
loff_t size;
@@ -207,15 +208,48 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
if (ret)
goto out;
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
image->kernel_buf_len);
- if (ret) {
- pr_debug("kernel signature verification failed.\n");
+#else
+ ret = -ENODATA;
+#endif
+
+ switch (ret) {
+ case 0:
+ break;
+
+ /* Certain verification errors are non-fatal if we're not
+ * checking errors, provided we aren't mandating that there
+ * must be a valid signature.
+ */
+ case -ENODATA:
+ reason = "kexec of unsigned image";
+ goto decide;
+ case -ENOPKG:
+ reason = "kexec of image with unsupported crypto";
+ goto decide;
+ case -ENOKEY:
+ reason = "kexec of image with unavailable key";
+ decide:
+ if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
+ pr_notice("%s rejected\n", reason);
+ ret = -EKEYREJECTED;
+ goto out;
+ }
+
+ ret = 0;
+ break;
+
+ /* All other errors are fatal, including nomem, unparseable
+ * signatures and signature check failures - even if signatures
+ * aren't required.
+ */
+ default:
+ pr_notice("kernel signature verification failed (%d).\n", ret);
goto out;
}
- pr_debug("kernel signature verification successful.\n");
-#endif
+
/* It is possible that there no initramfs is being loaded */
if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
--
2.21.0.392.gf8f6787159e-goog
From: David Howells <[email protected]>
Disallow the creation of kprobes when the kernel is locked down in
confidentiality mode by preventing their registration. This prevents
kprobes from being used to access kernel memory to steal crypto data.
Reported-by: Alexei Starovoitov <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Cc: Naveen N. Rao <[email protected]>
Cc: Anil S Keshavamurthy <[email protected]>
Cc: [email protected]
Cc: Masami Hiramatsu <[email protected]>
---
kernel/kprobes.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index f4ddfdd2d07e..b9781bd2db8c 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1552,6 +1552,9 @@ int register_kprobe(struct kprobe *p)
struct module *probed_mod;
kprobe_opcode_t *addr;
+ if (kernel_is_locked_down("Use of kprobes", LOCKDOWN_CONFIDENTIALITY))
+ return -EPERM;
+
/* Adjust probe address from symbol */
addr = kprobe_addr(p);
if (IS_ERR(addr))
--
2.21.0.392.gf8f6787159e-goog
From: David Howells <[email protected]>
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. This is
a runtime check rather than buildtime in order to allow configurations
where the same kernel may be run in both locked down or permissive modes
depending on local policy.
Suggested-by: Thomas Gleixner <[email protected]>
Signed-off-by: David Howells <[email protected]
Signed-off-by: Matthew Garrett <[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..9e8ad665f354 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", LOCKDOWN_INTEGRITY))
+ 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");
--
2.21.0.392.gf8f6787159e-goog
From: David Howells <[email protected]>
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]>
Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/pcmcia/cistpl.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
index ac0672b8dfca..9e23300a55e5 100644
--- a/drivers/pcmcia/cistpl.c
+++ b/drivers/pcmcia/cistpl.c
@@ -1578,6 +1578,10 @@ 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",
+ LOCKDOWN_INTEGRITY))
+ return -EPERM;
+
s = to_socket(container_of(kobj, struct device, kobj));
if (off)
--
2.21.0.392.gf8f6787159e-goog
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]>
Signed-off-by: Matthew Garrett <[email protected]>
cc: Dave Young <[email protected]>
cc: [email protected]
---
drivers/acpi/osl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index f29e427d0d1d..cd5bba7b8eb3 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -194,7 +194,8 @@ 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",
+ LOCKDOWN_INTEGRITY))
return acpi_rsdp;
#endif
pa = acpi_arch_get_root_pointer();
--
2.21.0.392.gf8f6787159e-goog
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 lockdown is enabled, 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]>
Signed-off-by: Matthew Garrett <[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 48eabb6c2d4f..0dc561210c86 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -531,6 +531,11 @@ void __init acpi_table_upgrade(void)
if (table_nr == 0)
return;
+ if (kernel_is_locked_down("ACPI table override", LOCKDOWN_INTEGRITY)) {
+ 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);
--
2.21.0.392.gf8f6787159e-goog
From: David Howells <[email protected]>
Disallow access to /proc/kcore when the kernel is locked down to prevent
access to cryptographic data. This is limited to lockdown
confidentiality mode and is still permitted in integrity mode.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[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 bbcc185062bb..1c556a453569 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -518,6 +518,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", LOCKDOWN_CONFIDENTIALITY))
+ return -EPERM;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
--
2.21.0.392.gf8f6787159e-goog
From: Matthew Garrett <[email protected]>
custom_method effectively allows arbitrary access to system memory, making
it possible for an attacker to circumvent restrictions on module loading.
Disable it if the kernel is locked down.
Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
---
drivers/acpi/custom_method.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index 4451877f83b6..37de3cd84493 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", LOCKDOWN_INTEGRITY))
+ return -EPERM;
+
if (!(*ppos)) {
/* parse the table header to get the table length */
if (count <= sizeof(struct acpi_table_header))
--
2.21.0.392.gf8f6787159e-goog
From: David Howells <[email protected]>
Disallow the use of certain perf facilities that might allow userspace to
access kernel data.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---
kernel/events/core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3cd13a30f732..6ad3d83c091c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10461,6 +10461,12 @@ SYSCALL_DEFINE5(perf_event_open,
return -EINVAL;
}
+ if ((attr.sample_type & PERF_SAMPLE_REGS_INTR) &&
+ kernel_is_locked_down("PERF_SAMPLE_REGS_INTR",
+ LOCKDOWN_CONFIDENTIALITY))
+ /* 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))
--
2.21.0.392.gf8f6787159e-goog
From: David Howells <[email protected]>
If the kernel is locked down, require that all modules have valid
signatures that we can verify.
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 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.
[Modified by Matthew Garrett to remove the IMA integration. This will
be replaced with integration with the IMA architecture policy
patchset.]
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Cc: Jessica Yu <[email protected]>
---
kernel/module.c | 39 ++++++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 2ad1b5239910..deea9d2763f8 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2767,8 +2767,9 @@ static inline void kmemleak_load_module(const struct module *mod,
#ifdef CONFIG_MODULE_SIG
static int module_sig_check(struct load_info *info, int flags)
{
- int err = -ENOKEY;
+ int err = -ENODATA;
const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+ const char *reason;
const void *mod = info->hdr;
/*
@@ -2783,16 +2784,40 @@ static int module_sig_check(struct load_info *info, int flags)
err = mod_verify_sig(mod, info);
}
- 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 && !is_module_sig_enforced())
- 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 (is_module_sig_enforced()) {
+ pr_notice("%s is rejected\n", reason);
+ return -EKEYREJECTED;
+ }
- return err;
+ if (kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY))
+ 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)
--
2.21.0.392.gf8f6787159e-goog
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: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Acked-by: Dave Young <[email protected]>
cc: [email protected]
---
kernel/kexec.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 68559808fdfa..57047acc9a36 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -207,6 +207,14 @@ static inline int kexec_load_check(unsigned long nr_segments,
if (result < 0)
return result;
+ /*
+ * kexec can be used to circumvent module loading restrictions, so
+ * prevent loading in that case
+ */
+ if (kernel_is_locked_down("kexec of unsigned images",
+ LOCKDOWN_INTEGRITY))
+ return -EPERM;
+
/*
* Verify we have a legal set of flags
* This leaves us room for future extensions.
--
2.21.0.392.gf8f6787159e-goog
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: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Acked-by: Bjorn Helgaas <[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 9ecfe13157c0..59d02088945e 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -905,6 +905,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", LOCKDOWN_INTEGRITY))
+ return -EPERM;
+
if (off > dev->cfg_size)
return 0;
if (off + count > dev->cfg_size) {
@@ -1167,6 +1170,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", LOCKDOWN_INTEGRITY))
+ return -EPERM;
+
if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
return -EINVAL;
@@ -1242,6 +1248,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", LOCKDOWN_INTEGRITY))
+ 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 6fa1627ce08d..85769f222b6d 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", LOCKDOWN_INTEGRITY))
+ 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", LOCKDOWN_INTEGRITY))
+ 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", LOCKDOWN_INTEGRITY))
return -EPERM;
if (fpriv->mmap_state == pci_mmap_io) {
diff --git a/drivers/pci/syscall.c b/drivers/pci/syscall.c
index d96626c614f5..0669cb09e792 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", LOCKDOWN_INTEGRITY))
return -EPERM;
dev = pci_get_domain_bus_and_slot(0, bus, dfn);
--
2.21.0.392.gf8f6787159e-goog
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]>
Signed-off-by: Matthew Garrett <[email protected]>
Cc: [email protected]
Cc: [email protected]
cc: [email protected]
---
kernel/power/hibernate.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index abef759de7c8..928b198cfa26 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -70,7 +70,8 @@ static const struct platform_hibernation_ops *hibernation_ops;
bool hibernation_available(void)
{
- return (nohibernate == 0);
+ return nohibernate == 0 && !kernel_is_locked_down("Hibernation",
+ LOCKDOWN_INTEGRITY);
}
/**
--
2.21.0.392.gf8f6787159e-goog
On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
<[email protected]> wrote:
>
> From: David Howells <[email protected]>
>
> There are some bpf functions can be used to read kernel memory:
> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
> private keys in kernel memory (e.g. the hibernation image signing key) to
> be read by an eBPF program and kernel memory to be altered without
> restriction. Disable them if the kernel has been locked down in
> confidentiality mode.
>
:)
This is yet another reason to get the new improved bpf_probe_user_read
stuff landed!
On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
<[email protected]> wrote:
>
> From: Matthew Garrett <[email protected]>
>
> debugfs has not been meaningfully audited in terms of ensuring that
> userland cannot trample over the kernel. At Greg's request, disable
> access to it entirely when the kernel is locked down. This is done at
> open() time rather than init time as the kernel lockdown status may be
> made stricter at runtime.
Ugh. Some of those files are very useful. Could this perhaps still
allow O_RDONLY if we're in INTEGRITY mode?
On Tue, Mar 26, 2019 at 12:20 PM Andy Lutomirski <[email protected]> wrote:
> Ugh. Some of those files are very useful. Could this perhaps still
> allow O_RDONLY if we're in INTEGRITY mode?
The previous implementation did, but Greg wanted it to go away entirely.
On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
<[email protected]> wrote:
>
> From: Matthew Garrett <[email protected]>
>
> Any hardware that can potentially generate DMA has to be locked down in
> order to avoid it being possible for an attacker to modify kernel code,
> allowing them to circumvent disabled module loading or module signing.
> Default to paranoid - in future we can potentially relax this for
> sufficiently IOMMU-isolated devices.
Does this break vfio?
--Andy
On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
<[email protected]> wrote:
>
> From: Matthew Garrett <[email protected]>
>
> IO port access would permit users to gain access to PCI configuration
> registers, which in turn (on a lot of hardware) give access to MMIO
> register space. This would potentially permit root to trigger arbitrary
> DMA, so lock it down by default.
>
> This also implicitly locks down the KDADDIO, KDDELIO, KDENABIO and
> KDDISABIO console ioctls.
>
Reviewed-by: Andy Lutomirski <[email protected]>
On Tue, 26 Mar 2019 13:55:39 -0700
Andy Lutomirski <[email protected]> wrote:
> On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
> <[email protected]> wrote:
> >
> > From: Matthew Garrett <[email protected]>
> >
> > Any hardware that can potentially generate DMA has to be locked down in
> > order to avoid it being possible for an attacker to modify kernel code,
> > allowing them to circumvent disabled module loading or module signing.
> > Default to paranoid - in future we can potentially relax this for
> > sufficiently IOMMU-isolated devices.
>
> Does this break vfio?
No, vfio provides its own interface to pci config space. Thanks,
Alex
On Tue, Mar 26, 2019 at 12:20:24PM -0700, Andy Lutomirski wrote:
> On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
> <[email protected]> wrote:
> >
> > From: Matthew Garrett <[email protected]>
> >
> > debugfs has not been meaningfully audited in terms of ensuring that
> > userland cannot trample over the kernel. At Greg's request, disable
> > access to it entirely when the kernel is locked down. This is done at
> > open() time rather than init time as the kernel lockdown status may be
> > made stricter at runtime.
>
> Ugh. Some of those files are very useful. Could this perhaps still
> allow O_RDONLY if we're in INTEGRITY mode?
Useful for what? Debugging, sure, but for "normal operation", no kernel
functionality should ever require debugfs. If it does, that's a bug and
should be fixed.
thanks,
greg k-h
On Tue, Mar 26, 2019 at 11:27:41AM -0700, Matthew Garrett wrote:
> From: Matthew Garrett <[email protected]>
>
> debugfs has not been meaningfully audited in terms of ensuring that
> userland cannot trample over the kernel. At Greg's request, disable
> access to it entirely when the kernel is locked down. This is done at
> open() time rather than init time as the kernel lockdown status may be
> made stricter at runtime.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> Cc: [email protected]
> ---
> fs/debugfs/file.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index 4fce1da7db23..9ae12ef29ba0 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -142,6 +142,9 @@ static int open_proxy_open(struct inode *inode, struct file *filp)
> const struct file_operations *real_fops = NULL;
> int r;
>
> + if (kernel_is_locked_down("debugfs", LOCKDOWN_INTEGRITY))
> + return -EPERM;
Why allow all this, why not just abort the registering of the filesystem
with the vfs core so it can't even be mounted?
thanks,
greg k-h
On Tue, Mar 26, 2019 at 5:31 PM Greg KH <[email protected]> wrote:
> On Tue, Mar 26, 2019 at 11:27:41AM -0700, Matthew Garrett wrote:
> > From: Matthew Garrett <[email protected]>
> >
> > debugfs has not been meaningfully audited in terms of ensuring that
> > userland cannot trample over the kernel. At Greg's request, disable
> > access to it entirely when the kernel is locked down. This is done at
> > open() time rather than init time as the kernel lockdown status may be
> > made stricter at runtime.
(snip)
> Why allow all this, why not just abort the registering of the filesystem
> with the vfs core so it can't even be mounted?
As mentioned in the commit message, because the lockdown state can be
made stricter at runtime - blocking at mount time would be
inconsistent if the machine is locked down afterwards. We could
potentially assert that it's the admin's responsibility to ensure that
debugfs isn't mounted at the point of policy being made stricter?
On Tue, Mar 26, 2019 at 07:06:36PM -0700, Matthew Garrett wrote:
> On Tue, Mar 26, 2019 at 5:31 PM Greg KH <[email protected]> wrote:
> > On Tue, Mar 26, 2019 at 11:27:41AM -0700, Matthew Garrett wrote:
> > > From: Matthew Garrett <[email protected]>
> > >
> > > debugfs has not been meaningfully audited in terms of ensuring that
> > > userland cannot trample over the kernel. At Greg's request, disable
> > > access to it entirely when the kernel is locked down. This is done at
> > > open() time rather than init time as the kernel lockdown status may be
> > > made stricter at runtime.
>
> (snip)
>
> > Why allow all this, why not just abort the registering of the filesystem
> > with the vfs core so it can't even be mounted?
>
> As mentioned in the commit message, because the lockdown state can be
> made stricter at runtime - blocking at mount time would be
> inconsistent if the machine is locked down afterwards. We could
> potentially assert that it's the admin's responsibility to ensure that
> debugfs isn't mounted at the point of policy being made stricter?
Ugh, I can not read, sorry, neverind.
The patch is fine as-is.
greg k-h
On Tue, Mar 26, 2019 at 5:31 PM Greg KH <[email protected]> wrote:
>
> On Tue, Mar 26, 2019 at 12:20:24PM -0700, Andy Lutomirski wrote:
> > On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
> > <[email protected]> wrote:
> > >
> > > From: Matthew Garrett <[email protected]>
> > >
> > > debugfs has not been meaningfully audited in terms of ensuring that
> > > userland cannot trample over the kernel. At Greg's request, disable
> > > access to it entirely when the kernel is locked down. This is done at
> > > open() time rather than init time as the kernel lockdown status may be
> > > made stricter at runtime.
> >
> > Ugh. Some of those files are very useful. Could this perhaps still
> > allow O_RDONLY if we're in INTEGRITY mode?
>
> Useful for what? Debugging, sure, but for "normal operation", no kernel
> functionality should ever require debugfs. If it does, that's a bug and
> should be fixed.
>
I semi-regularly read files in debugfs to diagnose things, and I think
it would be good for this to work on distro kernels.
On Tue, Mar 26, 2019 at 09:29:14PM -0700, Andy Lutomirski wrote:
> On Tue, Mar 26, 2019 at 5:31 PM Greg KH <[email protected]> wrote:
> >
> > On Tue, Mar 26, 2019 at 12:20:24PM -0700, Andy Lutomirski wrote:
> > > On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
> > > <[email protected]> wrote:
> > > >
> > > > From: Matthew Garrett <[email protected]>
> > > >
> > > > debugfs has not been meaningfully audited in terms of ensuring that
> > > > userland cannot trample over the kernel. At Greg's request, disable
> > > > access to it entirely when the kernel is locked down. This is done at
> > > > open() time rather than init time as the kernel lockdown status may be
> > > > made stricter at runtime.
> > >
> > > Ugh. Some of those files are very useful. Could this perhaps still
> > > allow O_RDONLY if we're in INTEGRITY mode?
> >
> > Useful for what? Debugging, sure, but for "normal operation", no kernel
> > functionality should ever require debugfs. If it does, that's a bug and
> > should be fixed.
> >
>
> I semi-regularly read files in debugfs to diagnose things, and I think
> it would be good for this to work on distro kernels.
Doing that for debugging is wonderful. People who want this type of
"lock down" are trading potential security for diagnositic ability.
good luck!
greg k-h
> On Mar 26, 2019, at 10:06 PM, Greg KH <[email protected]> wrote:
>
>> On Tue, Mar 26, 2019 at 09:29:14PM -0700, Andy Lutomirski wrote:
>>> On Tue, Mar 26, 2019 at 5:31 PM Greg KH <[email protected]> wrote:
>>>
>>>> On Tue, Mar 26, 2019 at 12:20:24PM -0700, Andy Lutomirski wrote:
>>>> On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
>>>> <[email protected]> wrote:
>>>>>
>>>>> From: Matthew Garrett <[email protected]>
>>>>>
>>>>> debugfs has not been meaningfully audited in terms of ensuring that
>>>>> userland cannot trample over the kernel. At Greg's request, disable
>>>>> access to it entirely when the kernel is locked down. This is done at
>>>>> open() time rather than init time as the kernel lockdown status may be
>>>>> made stricter at runtime.
>>>>
>>>> Ugh. Some of those files are very useful. Could this perhaps still
>>>> allow O_RDONLY if we're in INTEGRITY mode?
>>>
>>> Useful for what? Debugging, sure, but for "normal operation", no kernel
>>> functionality should ever require debugfs. If it does, that's a bug and
>>> should be fixed.
>>>
>>
>> I semi-regularly read files in debugfs to diagnose things, and I think
>> it would be good for this to work on distro kernels.
>
> Doing that for debugging is wonderful. People who want this type of
> "lock down" are trading potential security for diagnositic ability.
>
I think you may be missing the point of splitting lockdown to separate integrity and confidentiality. Can you actually think of a case where *reading* a debugfs file can take over a kernel?
On Tue, Mar 26, 2019 at 10:29:41PM -0700, Andy Lutomirski wrote:
>
>
> > On Mar 26, 2019, at 10:06 PM, Greg KH <[email protected]> wrote:
> >
> >> On Tue, Mar 26, 2019 at 09:29:14PM -0700, Andy Lutomirski wrote:
> >>> On Tue, Mar 26, 2019 at 5:31 PM Greg KH <[email protected]> wrote:
> >>>
> >>>> On Tue, Mar 26, 2019 at 12:20:24PM -0700, Andy Lutomirski wrote:
> >>>> On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
> >>>> <[email protected]> wrote:
> >>>>>
> >>>>> From: Matthew Garrett <[email protected]>
> >>>>>
> >>>>> debugfs has not been meaningfully audited in terms of ensuring that
> >>>>> userland cannot trample over the kernel. At Greg's request, disable
> >>>>> access to it entirely when the kernel is locked down. This is done at
> >>>>> open() time rather than init time as the kernel lockdown status may be
> >>>>> made stricter at runtime.
> >>>>
> >>>> Ugh. Some of those files are very useful. Could this perhaps still
> >>>> allow O_RDONLY if we're in INTEGRITY mode?
> >>>
> >>> Useful for what? Debugging, sure, but for "normal operation", no kernel
> >>> functionality should ever require debugfs. If it does, that's a bug and
> >>> should be fixed.
> >>>
> >>
> >> I semi-regularly read files in debugfs to diagnose things, and I think
> >> it would be good for this to work on distro kernels.
> >
> > Doing that for debugging is wonderful. People who want this type of
> > "lock down" are trading potential security for diagnositic ability.
> >
>
> I think you may be missing the point of splitting lockdown to separate integrity and confidentiality. Can you actually think of a case where *reading* a debugfs file can take over a kernel?
Reading a debugfs file can expose loads of things that can help take
over a kernel, or at least make it easier. Pointer addresses, internal
system state, loads of other fun things. And before 4.14 or so, it was
pretty trivial to use it to oops the kernel as well (not an issue here
anymore, but people are right to be nervous).
Personally, I think these are all just "confidentiality" type things,
but who really knows given the wild-west nature of debugfs (which is as
designed). And given that I think this patch series just crazy anyway,
I really don't care :)
thanks,
greg k-h
On Tue, 26 Mar 2019 11:27:35 -0700
Matthew Garrett <[email protected]> wrote:
> From: David Howells <[email protected]>
>
> 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. This is
> a runtime check rather than buildtime in order to allow configurations
> where the same kernel may be run in both locked down or permissive modes
> depending on local policy.
>
Acked-by: Steven Rostedt (VMware) <[email protected]>
I'm curious. Should there be a mode to lockdown the tracefs directory
too? As that can expose addresses.
-- Steve
> Suggested-by: Thomas Gleixner <[email protected]>
> Signed-off-by: David Howells <[email protected]
> Signed-off-by: Matthew Garrett <[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..9e8ad665f354 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", LOCKDOWN_INTEGRITY))
> + 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");
On Wed, 27 Mar 2019, Greg KH wrote:
> Personally, I think these are all just "confidentiality" type things,
> but who really knows given the wild-west nature of debugfs (which is as
> designed). And given that I think this patch series just crazy anyway,
> I really don't care :)
Why do you think it's crazy?
--
James Morris
<[email protected]>
On Wed, Mar 27, 2019 at 8:57 AM Steven Rostedt <[email protected]> wrote:
>
> On Tue, 26 Mar 2019 11:27:35 -0700
> Matthew Garrett <[email protected]> wrote:
>
> > From: David Howells <[email protected]>
> >
> > 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. This is
> > a runtime check rather than buildtime in order to allow configurations
> > where the same kernel may be run in both locked down or permissive modes
> > depending on local policy.
> >
>
> Acked-by: Steven Rostedt (VMware) <[email protected]>
>
> I'm curious. Should there be a mode to lockdown the tracefs directory
> too? As that can expose addresses.
That sounds like a reasonable thing to do in the confidentiality mode,
I don't think it'd be necessary in the integrity mode.
On Tue, Mar 26, 2019 at 10:33 PM Greg KH <[email protected]> wrote:
>
> On Tue, Mar 26, 2019 at 10:29:41PM -0700, Andy Lutomirski wrote:
> >
> >
> > > On Mar 26, 2019, at 10:06 PM, Greg KH <[email protected]> wrote:
> > >
> > >> On Tue, Mar 26, 2019 at 09:29:14PM -0700, Andy Lutomirski wrote:
> > >>> On Tue, Mar 26, 2019 at 5:31 PM Greg KH <[email protected]> wrote:
> > >>>
> > >>>> On Tue, Mar 26, 2019 at 12:20:24PM -0700, Andy Lutomirski wrote:
> > >>>> On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
> > >>>> <[email protected]> wrote:
> > >>>>>
> > >>>>> From: Matthew Garrett <[email protected]>
> > >>>>>
> > >>>>> debugfs has not been meaningfully audited in terms of ensuring that
> > >>>>> userland cannot trample over the kernel. At Greg's request, disable
> > >>>>> access to it entirely when the kernel is locked down. This is done at
> > >>>>> open() time rather than init time as the kernel lockdown status may be
> > >>>>> made stricter at runtime.
> > >>>>
> > >>>> Ugh. Some of those files are very useful. Could this perhaps still
> > >>>> allow O_RDONLY if we're in INTEGRITY mode?
> > >>>
> > >>> Useful for what? Debugging, sure, but for "normal operation", no kernel
> > >>> functionality should ever require debugfs. If it does, that's a bug and
> > >>> should be fixed.
> > >>>
> > >>
> > >> I semi-regularly read files in debugfs to diagnose things, and I think
> > >> it would be good for this to work on distro kernels.
> > >
> > > Doing that for debugging is wonderful. People who want this type of
> > > "lock down" are trading potential security for diagnositic ability.
> > >
> >
> > I think you may be missing the point of splitting lockdown to separate integrity and confidentiality. Can you actually think of a case where *reading* a debugfs file can take over a kernel?
>
> Reading a debugfs file can expose loads of things that can help take
> over a kernel, or at least make it easier. Pointer addresses, internal
> system state, loads of other fun things. And before 4.14 or so, it was
> pretty trivial to use it to oops the kernel as well (not an issue here
> anymore, but people are right to be nervous).
>
> Personally, I think these are all just "confidentiality" type things,
> but who really knows given the wild-west nature of debugfs (which is as
> designed). And given that I think this patch series just crazy anyway,
> I really don't care :)
>
As far as I'm concerned, preventing root from crashing the system
should not be a design goal of lockdown at all. And I think that the
"integrity" mode should be as non-annoying as possible, so I think we
should allow reading from debugfs.
On Wed, Mar 27, 2019 at 10:40 AM Andy Lutomirski <[email protected]> wrote:
> As far as I'm concerned, preventing root from crashing the system
> should not be a design goal of lockdown at all. And I think that the
> "integrity" mode should be as non-annoying as possible, so I think we
> should allow reading from debugfs.
I have no horse in this game - I'm happy to bring back the previous
approach for integrity mode and block reads entirely in
confidentiality mode, but I'd rather not spend another release cycle
arguing about it.
On Wed, Mar 27, 2019 at 10:42:18AM -0700, Matthew Garrett wrote:
> On Wed, Mar 27, 2019 at 10:40 AM Andy Lutomirski <[email protected]> wrote:
> > As far as I'm concerned, preventing root from crashing the system
> > should not be a design goal of lockdown at all. And I think that the
> > "integrity" mode should be as non-annoying as possible, so I think we
> > should allow reading from debugfs.
>
> I have no horse in this game - I'm happy to bring back the previous
> approach for integrity mode and block reads entirely in
> confidentiality mode, but I'd rather not spend another release cycle
> arguing about it.
I really do not care either way about any of this :)
greg k-h
On Wed, Mar 27, 2019 at 10:39:53AM -0700, Andy Lutomirski wrote:
> On Tue, Mar 26, 2019 at 10:33 PM Greg KH <[email protected]> wrote:
> >
> > On Tue, Mar 26, 2019 at 10:29:41PM -0700, Andy Lutomirski wrote:
> > >
> > >
> > > > On Mar 26, 2019, at 10:06 PM, Greg KH <[email protected]> wrote:
> > > >
> > > >> On Tue, Mar 26, 2019 at 09:29:14PM -0700, Andy Lutomirski wrote:
> > > >>> On Tue, Mar 26, 2019 at 5:31 PM Greg KH <[email protected]> wrote:
> > > >>>
> > > >>>> On Tue, Mar 26, 2019 at 12:20:24PM -0700, Andy Lutomirski wrote:
> > > >>>> On Tue, Mar 26, 2019 at 11:28 AM Matthew Garrett
> > > >>>> <[email protected]> wrote:
> > > >>>>>
> > > >>>>> From: Matthew Garrett <[email protected]>
> > > >>>>>
> > > >>>>> debugfs has not been meaningfully audited in terms of ensuring that
> > > >>>>> userland cannot trample over the kernel. At Greg's request, disable
> > > >>>>> access to it entirely when the kernel is locked down. This is done at
> > > >>>>> open() time rather than init time as the kernel lockdown status may be
> > > >>>>> made stricter at runtime.
> > > >>>>
> > > >>>> Ugh. Some of those files are very useful. Could this perhaps still
> > > >>>> allow O_RDONLY if we're in INTEGRITY mode?
> > > >>>
> > > >>> Useful for what? Debugging, sure, but for "normal operation", no kernel
> > > >>> functionality should ever require debugfs. If it does, that's a bug and
> > > >>> should be fixed.
> > > >>>
> > > >>
> > > >> I semi-regularly read files in debugfs to diagnose things, and I think
> > > >> it would be good for this to work on distro kernels.
> > > >
> > > > Doing that for debugging is wonderful. People who want this type of
> > > > "lock down" are trading potential security for diagnositic ability.
> > > >
> > >
> > > I think you may be missing the point of splitting lockdown to separate integrity and confidentiality. Can you actually think of a case where *reading* a debugfs file can take over a kernel?
> >
> > Reading a debugfs file can expose loads of things that can help take
> > over a kernel, or at least make it easier. Pointer addresses, internal
> > system state, loads of other fun things. And before 4.14 or so, it was
> > pretty trivial to use it to oops the kernel as well (not an issue here
> > anymore, but people are right to be nervous).
> >
> > Personally, I think these are all just "confidentiality" type things,
> > but who really knows given the wild-west nature of debugfs (which is as
> > designed). And given that I think this patch series just crazy anyway,
> > I really don't care :)
> >
>
> As far as I'm concerned, preventing root from crashing the system
> should not be a design goal of lockdown at all. And I think that the
> "integrity" mode should be as non-annoying as possible, so I think we
> should allow reading from debugfs.
Sorry, the "crash the system" is not the issue here. The issue is if
everyone can "ensure" that "everything" in debugfs is "safe" for this
mode of "lock down". Given that no one has any idea of what really is
in debugfs, and to try to compare that with the design goals of what
"lock down" really is trying to achive, I think the goal of just giving
up and restricting access is fine if that makes people feel better about
this whole thing.
If this is locked down, it is going to cause distros more pain in
debugging user's issues, but that's their choice, not mine :)
thanks,
greg k-h
On 03/26/19 at 11:27am, Matthew Garrett wrote:
> From: Jiri Bohac <[email protected]>
>
> This is a preparatory patch for kexec_file_load() lockdown. A locked down
> kernel needs to prevent unsigned kernel images from being loaded with
> kexec_file_load(). Currently, the only way to force the signature
> verification is compiling with KEXEC_VERIFY_SIG. This prevents loading
> usigned images even when the kernel is not locked down at runtime.
>
> This patch splits KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE.
> Analogous to the MODULE_SIG and MODULE_SIG_FORCE for modules, KEXEC_SIG
> turns on the signature verification but allows unsigned images to be
> loaded. KEXEC_SIG_FORCE disallows images without a valid signature.
>
> [Modified by David Howells such that:
>
> (1) verify_pefile_signature() differentiates between no-signature and
> sig-didn't-match in its returned errors.
>
> (2) kexec fails with EKEYREJECTED and logs an appropriate message if
> signature checking is enforced and an signature is not found, uses
> unsupported crypto or has no matching key.
>
> (3) kexec fails with EKEYREJECTED if there is a signature for which we
> have a key, but signature doesn't match - even if in non-forcing mode.
>
> (4) kexec fails with EBADMSG or some other error if there is a signature
> which cannot be parsed - even if in non-forcing mode.
>
> (5) kexec fails with ELIBBAD if the PE file cannot be parsed to extract
> the signature - even if in non-forcing mode.
>
> ]
>
> Signed-off-by: Jiri Bohac <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> Reviewed-by: Jiri Bohac <[email protected]>
> cc: [email protected]
> ---
> arch/x86/Kconfig | 20 ++++++++---
> crypto/asymmetric_keys/verify_pefile.c | 4 ++-
> include/linux/kexec.h | 4 +--
> kernel/kexec_file.c | 48 ++++++++++++++++++++++----
> 4 files changed, 61 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 4b4a7f32b68e..735d04a4b18f 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2016,20 +2016,30 @@ config KEXEC_FILE
> config ARCH_HAS_KEXEC_PURGATORY
> def_bool KEXEC_FILE
>
> -config KEXEC_VERIFY_SIG
> +config KEXEC_SIG
> bool "Verify kernel signature during kexec_file_load() syscall"
> depends on KEXEC_FILE
> ---help---
> - This option makes kernel signature verification mandatory for
> - the kexec_file_load() syscall.
>
> - In addition to that option, you need to enable signature
> + This option makes the kexec_file_load() syscall check for a valid
> + signature of the kernel image. The image can still be loaded without
> + a valid signature unless you also enable KEXEC_SIG_FORCE, though if
> + there's a signature that we can check, then it must be valid.
> +
> + In addition to this option, you need to enable signature
> verification for the corresponding kernel image type being
> loaded in order for this to work.
>
> +config KEXEC_SIG_FORCE
> + bool "Require a valid signature in kexec_file_load() syscall"
> + depends on KEXEC_SIG
> + ---help---
> + This option makes kernel signature verification mandatory for
> + the kexec_file_load() syscall.
> +
> config KEXEC_BZIMAGE_VERIFY_SIG
> bool "Enable bzImage signature verification support"
> - depends on KEXEC_VERIFY_SIG
> + depends on KEXEC_SIG
> depends on SIGNED_PE_FILE_VERIFICATION
> select SYSTEM_TRUSTED_KEYRING
> ---help---
> diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
> index d178650fd524..4473cea1e877 100644
> --- a/crypto/asymmetric_keys/verify_pefile.c
> +++ b/crypto/asymmetric_keys/verify_pefile.c
> @@ -100,7 +100,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
>
> if (!ddir->certs.virtual_address || !ddir->certs.size) {
> pr_debug("Unsigned PE binary\n");
> - return -EKEYREJECTED;
> + return -ENODATA;
> }
>
> chkaddr(ctx->header_size, ddir->certs.virtual_address,
> @@ -408,6 +408,8 @@ static int pefile_digest_pe(const void *pebuf, unsigned int pelen,
> * (*) 0 if at least one signature chain intersects with the keys in the trust
> * keyring, or:
> *
> + * (*) -ENODATA if there is no signature present.
> + *
> * (*) -ENOPKG if a suitable crypto module couldn't be found for a check on a
> * chain.
> *
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index b9b1bc5f9669..58b27c7bdc2b 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -125,7 +125,7 @@ typedef void *(kexec_load_t)(struct kimage *image, char *kernel_buf,
> unsigned long cmdline_len);
> typedef int (kexec_cleanup_t)(void *loader_data);
>
> -#ifdef CONFIG_KEXEC_VERIFY_SIG
> +#ifdef CONFIG_KEXEC_SIG
> typedef int (kexec_verify_sig_t)(const char *kernel_buf,
> unsigned long kernel_len);
> #endif
> @@ -134,7 +134,7 @@ struct kexec_file_ops {
> kexec_probe_t *probe;
> kexec_load_t *load;
> kexec_cleanup_t *cleanup;
> -#ifdef CONFIG_KEXEC_VERIFY_SIG
> +#ifdef CONFIG_KEXEC_SIG
> kexec_verify_sig_t *verify_sig;
> #endif
> };
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index f1d0e00a3971..67f3a866eabe 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -90,7 +90,7 @@ int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
> return kexec_image_post_load_cleanup_default(image);
> }
>
> -#ifdef CONFIG_KEXEC_VERIFY_SIG
> +#ifdef CONFIG_KEXEC_SIG
> static int kexec_image_verify_sig_default(struct kimage *image, void *buf,
> unsigned long buf_len)
> {
> @@ -188,7 +188,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> const char __user *cmdline_ptr,
> unsigned long cmdline_len, unsigned flags)
> {
> - int ret = 0;
> + const char *reason;
> + int ret;
> void *ldata;
> loff_t size;
>
> @@ -207,15 +208,48 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> if (ret)
> goto out;
>
> -#ifdef CONFIG_KEXEC_VERIFY_SIG
> +#ifdef CONFIG_KEXEC_SIG
> ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
> image->kernel_buf_len);
> - if (ret) {
> - pr_debug("kernel signature verification failed.\n");
> +#else
> + ret = -ENODATA;
> +#endif
> +
> + switch (ret) {
> + case 0:
> + break;
> +
> + /* Certain verification errors are non-fatal if we're not
> + * checking errors, provided we aren't mandating that there
> + * must be a valid signature.
> + */
> + case -ENODATA:
> + reason = "kexec of unsigned image";
> + goto decide;
> + case -ENOPKG:
> + reason = "kexec of image with unsupported crypto";
> + goto decide;
> + case -ENOKEY:
> + reason = "kexec of image with unavailable key";
> + decide:
> + if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
> + pr_notice("%s rejected\n", reason);
> + ret = -EKEYREJECTED;
Force use -EKEYREJECTED is odd, why not just use original "ret"?
> + goto out;
> + }
> +
> + ret = 0;
> + break;
> +
> + /* All other errors are fatal, including nomem, unparseable
> + * signatures and signature check failures - even if signatures
> + * aren't required.
> + */
> + default:
> + pr_notice("kernel signature verification failed (%d).\n", ret);
> goto out;
> }
> - pr_debug("kernel signature verification successful.\n");
> -#endif
> +
> /* It is possible that there no initramfs is being loaded */
> if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
> ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
> --
> 2.21.0.392.gf8f6787159e-goog
>
>
> _______________________________________________
> kexec mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec
Thanks
Dave
On 03/26/19 at 11:27am, Matthew Garrett wrote:
> From: Jiri Bohac <[email protected]>
>
> When KEXEC_SIG is not enabled, kernel should not load images through
> kexec_file systemcall if the kernel is locked down.
>
> [Modified by David Howells to fit with modifications to the previous patch
> and to return -EPERM if the kernel is locked down for consistency with
> other lockdowns. Modified by Matthew Garrett to remove the IMA
> integration, which will be replaced by integrating with the IMA
> architecture policy patches.]
>
> Signed-off-by: Jiri Bohac <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> Reviewed-by: Jiri Bohac <[email protected]>
> cc: [email protected]
> ---
> kernel/kexec_file.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 67f3a866eabe..a1cc37c8b43b 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -239,6 +239,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> }
>
> ret = 0;
> +
> + if (kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY)) {
> + ret = -EPERM;
> + goto out;
> + }
> +
Checking here is late, it would be good to move the check to earlier
code around below code:
/* We only trust the superuser with rebooting the system. */
if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
return -EPERM;
> break;
>
> /* All other errors are fatal, including nomem, unparseable
> --
> 2.21.0.392.gf8f6787159e-goog
>
>
> _______________________________________________
> kexec mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec
Thanks
Dave
On Thu, Jun 20, 2019 at 11:34 PM Dave Young <[email protected]> wrote:
> Force use -EKEYREJECTED is odd, why not just use original "ret"?
Fair question. Jiri, any feelings here?
On Fri, Jun 21, 2019 at 1:13 PM Matthew Garrett <[email protected]> wrote:
>
> On Thu, Jun 20, 2019 at 11:34 PM Dave Young <[email protected]> wrote:
> > Force use -EKEYREJECTED is odd, why not just use original "ret"?
>
> Fair question. Jiri, any feelings here?
Actually, looks like this change was made by Dave Howells.
On Thu, Jun 20, 2019 at 11:43 PM Dave Young <[email protected]> wrote:
>
> On 03/26/19 at 11:27am, Matthew Garrett wrote:
> > From: Jiri Bohac <[email protected]>
> >
> > When KEXEC_SIG is not enabled, kernel should not load images through
> > kexec_file systemcall if the kernel is locked down.
> >
> > [Modified by David Howells to fit with modifications to the previous patch
> > and to return -EPERM if the kernel is locked down for consistency with
> > other lockdowns. Modified by Matthew Garrett to remove the IMA
> > integration, which will be replaced by integrating with the IMA
> > architecture policy patches.]
> >
> > Signed-off-by: Jiri Bohac <[email protected]>
> > Signed-off-by: David Howells <[email protected]>
> > Signed-off-by: Matthew Garrett <[email protected]>
> > Reviewed-by: Jiri Bohac <[email protected]>
> > cc: [email protected]
> > ---
> > kernel/kexec_file.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > index 67f3a866eabe..a1cc37c8b43b 100644
> > --- a/kernel/kexec_file.c
> > +++ b/kernel/kexec_file.c
> > @@ -239,6 +239,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> > }
> >
> > ret = 0;
> > +
> > + if (kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY)) {
> > + ret = -EPERM;
> > + goto out;
> > + }
> > +
>
> Checking here is late, it would be good to move the check to earlier
> code around below code:
> /* We only trust the superuser with rebooting the system. */
> if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
> return -EPERM;
I don't think so - we want it to be possible to load images if they
have a valid signature.
On 06/21/19 at 01:18pm, Matthew Garrett wrote:
> On Thu, Jun 20, 2019 at 11:43 PM Dave Young <[email protected]> wrote:
> >
> > On 03/26/19 at 11:27am, Matthew Garrett wrote:
> > > From: Jiri Bohac <[email protected]>
> > >
> > > When KEXEC_SIG is not enabled, kernel should not load images through
> > > kexec_file systemcall if the kernel is locked down.
> > >
> > > [Modified by David Howells to fit with modifications to the previous patch
> > > and to return -EPERM if the kernel is locked down for consistency with
> > > other lockdowns. Modified by Matthew Garrett to remove the IMA
> > > integration, which will be replaced by integrating with the IMA
> > > architecture policy patches.]
> > >
> > > Signed-off-by: Jiri Bohac <[email protected]>
> > > Signed-off-by: David Howells <[email protected]>
> > > Signed-off-by: Matthew Garrett <[email protected]>
> > > Reviewed-by: Jiri Bohac <[email protected]>
> > > cc: [email protected]
> > > ---
> > > kernel/kexec_file.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > > index 67f3a866eabe..a1cc37c8b43b 100644
> > > --- a/kernel/kexec_file.c
> > > +++ b/kernel/kexec_file.c
> > > @@ -239,6 +239,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> > > }
> > >
> > > ret = 0;
> > > +
> > > + if (kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY)) {
> > > + ret = -EPERM;
> > > + goto out;
> > > + }
> > > +
> >
> > Checking here is late, it would be good to move the check to earlier
> > code around below code:
> > /* We only trust the superuser with rebooting the system. */
> > if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
> > return -EPERM;
>
> I don't think so - we want it to be possible to load images if they
> have a valid signature.
I know it works like this way because of the previous patch. But from
the patch log "When KEXEC_SIG is not enabled, kernel should not load
images", it is simple to check it early for !IS_ENABLED(CONFIG_KEXEC_SIG) &&
kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY) instead of depending
on the late code to verify signature. In that way, easier to
understand the logic, no?
Thanks
Dave
On Sun, Jun 23, 2019 at 6:52 PM Dave Young <[email protected]> wrote:
>
> On 06/21/19 at 01:18pm, Matthew Garrett wrote:
> > I don't think so - we want it to be possible to load images if they
> > have a valid signature.
>
> I know it works like this way because of the previous patch. But from
> the patch log "When KEXEC_SIG is not enabled, kernel should not load
> images", it is simple to check it early for !IS_ENABLED(CONFIG_KEXEC_SIG) &&
> kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY) instead of depending
> on the late code to verify signature. In that way, easier to
> understand the logic, no?
But that combination doesn't enforce signature validation? We can't
depend on !IS_ENABLED(CONFIG_KEXEC_SIG_FORCE) because then it'll
enforce signature validation even if lockdown is disabled.
Hi Matthew,
On Mon, 2019-06-24 at 14:06 -0700, Matthew Garrett wrote:
> On Sun, Jun 23, 2019 at 6:52 PM Dave Young <[email protected]> wrote:
> >
> > On 06/21/19 at 01:18pm, Matthew Garrett wrote:
> > > I don't think so - we want it to be possible to load images if they
> > > have a valid signature.
> >
> > I know it works like this way because of the previous patch. But from
> > the patch log "When KEXEC_SIG is not enabled, kernel should not load
> > images", it is simple to check it early for !IS_ENABLED(CONFIG_KEXEC_SIG) &&
> > kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY) instead of depending
> > on the late code to verify signature. In that way, easier to
> > understand the logic, no?
>
> But that combination doesn't enforce signature validation? We can't
> depend on !IS_ENABLED(CONFIG_KEXEC_SIG_FORCE) because then it'll
> enforce signature validation even if lockdown is disabled.
I agree with Dave. There should be a stub lockdown function to
prevent enforcing lockdown when it isn't enabled.
Mimi
On Mon, Jun 24, 2019 at 2:27 PM Mimi Zohar <[email protected]> wrote:
> I agree with Dave. There should be a stub lockdown function to
> prevent enforcing lockdown when it isn't enabled.
Sorry, when what isn't enabled? If no LSMs are enforcing lockdown then
the check will return 0. The goal here is for distributions to be able
to ship a kernel that has CONFIG_KEXEC_SIG=y, CONFIG_KEXEC_SIG_FORCE=n
and at runtime be able to enforce a policy that requires signatures on
kexec payloads.
On Mon, 2019-06-24 at 17:02 -0700, Matthew Garrett wrote:
> On Mon, Jun 24, 2019 at 2:27 PM Mimi Zohar <[email protected]> wrote:
>
> > I agree with Dave. There should be a stub lockdown function to
> > prevent enforcing lockdown when it isn't enabled.
>
> Sorry, when what isn't enabled? If no LSMs are enforcing lockdown then
> the check will return 0. The goal here is for distributions to be able
> to ship a kernel that has CONFIG_KEXEC_SIG=y, CONFIG_KEXEC_SIG_FORCE=n
> and at runtime be able to enforce a policy that requires signatures on
> kexec payloads.
Never mind, the call can't be moved earlier.
On 06/24/19 at 02:06pm, Matthew Garrett wrote:
> On Sun, Jun 23, 2019 at 6:52 PM Dave Young <[email protected]> wrote:
> >
> > On 06/21/19 at 01:18pm, Matthew Garrett wrote:
> > > I don't think so - we want it to be possible to load images if they
> > > have a valid signature.
> >
> > I know it works like this way because of the previous patch. But from
> > the patch log "When KEXEC_SIG is not enabled, kernel should not load
> > images", it is simple to check it early for !IS_ENABLED(CONFIG_KEXEC_SIG) &&
> > kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY) instead of depending
> > on the late code to verify signature. In that way, easier to
> > understand the logic, no?
>
> But that combination doesn't enforce signature validation? We can't
> depend on !IS_ENABLED(CONFIG_KEXEC_SIG_FORCE) because then it'll
> enforce signature validation even if lockdown is disabled.
Ok, got your point. still something could be improved though, in the switch
chunk, the errno, reason and IS_ENABLED(CONFIG_KEXEC_SIG_FORCE) etc is
not necessary for this -EPERM case.
/* add some comment to describe the behavior */
if (ret && security_is_locked_down(LOCKDOWN_KEXEC)) {
ret = -EPERM;
goto out;
}
Thanks
Dave