[PATCH 00/12] One more attempt at useful kernel lockdown

[PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

Some use cases require the ability to ensure that anything running in ring 0
is trusted code. We have support for signing the kernel and kernel modules,
but there's still a range of exported kernel interfaces that make it easy to
modify the running kernel. Previous attempts to implement a generic interface
to restrict this have included a new capability (breaks existing userspace)
and tying it to a requirement for signed modules (breaks assumptions in
certain situations where userspace is already running with restricted
privileges).

So, this is my final attempt at providing the functionality I'm interested
in without inherently tying it to Secure Boot. There's strong parallels
between the functionality that I'm interested in and the BSD securelevel
interface, so here's a trivial implementation. 

[PATCH 01/12] Add BSD-style securelevel support

[Matthew Garrett]

Provide a coarse-grained runtime configuration option for restricting
userspace's ability to modify the running kernel.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 Documentation/security/securelevel.txt |  23 +++++++
 include/linux/security.h               |   8 +++
 security/Kconfig                       |   8 +++
 security/Makefile                      |   1 +
 security/securelevel.c                 | 116 +++++++++++++++++++++++++++++++++
 5 files changed, 156 insertions(+)
 create mode 100644 Documentation/security/securelevel.txt
 create mode 100644 security/securelevel.c

diff --git a/Documentation/security/securelevel.txt b/Documentation/security/securelevel.txt
new file mode 100644
index 0000000..a1355a0
--- /dev/null
+++ b/Documentation/security/securelevel.txt
@@ -0,0 +1,23 @@
+Linux securelevel interface
+---------------------------
+
+The Linux securelevel interface (inspired by the BSD securelevel interface)
+is a runtime mechanism for configuring coarse-grained kernel-level security
+restrictions. It provides a runtime configuration variable at
+/sys/kernel/security/securelevel which can be written to by root. The
+following values are supported:
+
+-1: Permanently insecure mode. This level is equivalent to level 0, but once
+    set cannot be changed.
+
+0:  Insecure mode (default). This level imposes no additional kernel
+    restrictions.
+
+1:  Secure mode. If set, userspace will be unable to perform direct access
+    to PCI devices, port IO access, access system memory directly via
+    /dev/mem and /dev/kmem, perform kexec_load(), use the userspace
+    software suspend mechanism, insert new ACPI code at runtime via the
+    custom_method interface or modify CPU MSRs (on x86). Certain drivers
+    may also limit additional interfaces.
+
+Once the securelevel value is increased, it may not be decreased.
diff --git a/include/linux/security.h b/include/linux/security.h
index 9d37e2b..e8b69e0 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -3081,6 +3081,14 @@ static inline void security_audit_rule_free(void *lsmrule)
 #endif /* CONFIG_SECURITY */
 #endif /* CONFIG_AUDIT */
 
+#ifdef CONFIG_SECURITY_SECURELEVEL
+extern int get_securelevel(void);
+extern int set_securelevel(int new_securelevel);
+#else
+static inline int get_securelevel(void) { return 0; }
+static inline int set_securelevel(int new_securelevel) { return 0; }
+#endif /* CONFIG_SECURELEVEL */
+
 #ifdef CONFIG_SECURITYFS
 
 extern struct dentry *securityfs_create_file(const char *name, umode_t mode,
diff --git a/security/Kconfig b/security/Kconfig
index e9c6ac7..3605d24 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -70,6 +70,14 @@ config SECURITY_PATH
 	  implement pathname based access controls.
 	  If you are unsure how to answer this question, answer N.
 
+config SECURITY_SECURELEVEL
+        bool "Securelevel kernel restriction interface"
+	depends on SECURITY
+	help
+	  This enables support for adding a set of additional kernel security
+	  restrictions at runtime. See Documentation/security/securelevel.txt
+	  for further information.
+
 config INTEL_TXT
 	bool "Enable Intel(R) Trusted Execution Technology (Intel(R) TXT)"
 	depends on HAVE_INTEL_TXT
diff --git a/security/Makefile b/security/Makefile
index c26c81e..d6c98ab 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_MMU)			+= min_addr.o
 # Object file lists
 obj-$(CONFIG_SECURITY)			+= security.o capability.o
 obj-$(CONFIG_SECURITYFS)		+= inode.o
+obj-$(CONFIG_SECURITY_SECURELEVEL)	+= securelevel.o
 # Must precede capability.o in order to stack properly.
 obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o
 obj-$(CONFIG_SECURITY_SMACK)		+= smack/built-in.o
diff --git a/security/securelevel.c b/security/securelevel.c
new file mode 100644
index 0000000..7128430
--- /dev/null
+++ b/security/securelevel.c
@@ -0,0 +1,116 @@
+/*
+ *  securelevel.c - support for generic kernel lockdown
+ *
+ *  Copyright Nebula, Inc <matthew.garrett@nebula.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/security.h>
+#include <linux/uaccess.h>
+
+static int securelevel;
+
+static DEFINE_SPINLOCK(securelevel_lock);
+
+#define MAX_SECURELEVEL 1
+
+int get_securelevel(void)
+{
+	return securelevel;
+}
+EXPORT_SYMBOL(get_securelevel);
+
+int set_securelevel(int new_securelevel)
+{
+	int ret = 0;
+
+	spin_lock(&securelevel_lock);
+
+	if ((securelevel == -1) || (new_securelevel < securelevel) ||
+	    (new_securelevel > MAX_SECURELEVEL)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	securelevel = new_securelevel;
+out:
+	spin_unlock(&securelevel_lock);
+	return ret;
+}
+EXPORT_SYMBOL(set_securelevel);
+
+static ssize_t securelevel_read(struct file *filp, char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	char tmpbuf[12];
+	ssize_t length;
+
+	length = scnprintf(tmpbuf, sizeof(tmpbuf), "%d", securelevel);
+	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
+}
+
+static ssize_t securelevel_write(struct file *file, const char __user *buf,
+				 size_t count, loff_t *ppos)
+{
+	char *page = NULL;
+	ssize_t length;
+	int new_securelevel;
+
+	length = -ENOMEM;
+	if (count >= PAGE_SIZE)
+		goto out;
+
+	length = -EINVAL;
+	if (*ppos != 0)
+		goto out;
+
+	length = -ENOMEM;
+	page = (char *)get_zeroed_page(GFP_KERNEL);
+	if (!page)
+		goto out;
+
+	length = -EFAULT;
+	if (copy_from_user(page, buf, count))
+		goto out;
+
+	length = -EINVAL;
+	if (sscanf(page, "%d", &new_securelevel) != 1)
+		goto out;
+
+	length = set_securelevel(new_securelevel);
+	if (length)
+		goto out;
+
+	length = count;
+out:
+	free_page((unsigned long) page);
+	return length;
+}
+
+static const struct file_operations securelevel_fops = {
+	.read 	= securelevel_read,
+	.write 	= securelevel_write,
+	.llseek	= generic_file_llseek,
+};
+
+static __init int setup_securelevel(void)
+{
+	struct dentry *securelevel_file;
+
+	securelevel_file = securityfs_create_file("securelevel",
+						  S_IWUSR | S_IRUGO,
+						  NULL, NULL,
+						  &securelevel_fops);
+
+	if (IS_ERR(securelevel_file))
+		return PTR_ERR(securelevel_file);
+
+	return 0;
+}
+late_initcall(setup_securelevel);
-- 
1.8.3.1

[PATCH 02/12] Enforce module signatures when securelevel is greater than 0

[Matthew Garrett]

If securelevel has been set to 1 or greater, require that all modules have
valid signatures.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 kernel/module.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/module.c b/kernel/module.c
index dc58274..ab126d7 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2487,7 +2487,7 @@ static int module_sig_check(struct load_info *info)
 	if (err < 0 && fips_enabled)
 		panic("Module verification failed with error %d in FIPS mode\n",
 		      err);
-	if (err == -ENOKEY && !sig_enforce)
+	if ((err == -ENOKEY && !sig_enforce) && (get_securelevel() <= 0))
 		err = 0;
 
 	return err;
-- 
1.8.3.1

[PATCH 03/12] PCI: Lock down BAR access when securelevel is enabled

[Matthew Garrett]

Any hardware that can potentially generate DMA has to be locked down from
userspace in order to avoid it being possible for an attacker to modify
kernel code. This should be prevented if securelevel has been set. Default
to paranoid - in future we can potentially relax this for sufficiently
IOMMU-isolated devices.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 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 7128cfd..0a3fca2 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -634,6 +634,9 @@ pci_write_config(struct file* filp, struct kobject *kobj,
 	loff_t init_off = off;
 	u8 *data = (u8*) buf;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	if (off > dev->cfg_size)
 		return 0;
 	if (off + count > dev->cfg_size) {
@@ -940,6 +943,9 @@ pci_mmap_resource(struct kobject *kobj, struct bin_attribute *attr,
 	resource_size_t start, end;
 	int i;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	for (i = 0; i < PCI_ROM_RESOURCE; i++)
 		if (res == &pdev->resource[i])
 			break;
@@ -1047,6 +1053,9 @@ pci_write_resource_io(struct file *filp, struct kobject *kobj,
 		      struct bin_attribute *attr, char *buf,
 		      loff_t off, size_t count)
 {
+	if (get_securelevel() > 0)
+		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 cdc7836..45c59e9 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -11,6 +11,7 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/capability.h>
+#include <linux/security.h>
 #include <asm/uaccess.h>
 #include <asm/byteorder.h>
 #include "pci.h"
@@ -117,6 +118,9 @@ proc_bus_pci_write(struct file *file, const char __user *buf, size_t nbytes, lof
 	int size = dev->cfg_size;
 	int cnt;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	if (pos >= size)
 		return 0;
 	if (nbytes >= size)
@@ -196,6 +200,9 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd,
 #endif /* HAVE_PCI_MMAP */
 	int ret = 0;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	switch (cmd) {
 	case PCIIOC_CONTROLLER:
 		ret = pci_domain_nr(dev->bus);
@@ -234,7 +241,7 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma)
 	struct pci_filp_private *fpriv = file->private_data;
 	int i, ret;
 
-	if (!capable(CAP_SYS_RAWIO))
+	if (!capable(CAP_SYS_RAWIO) || (get_securelevel() > 0))
 		return -EPERM;
 
 	/* Make sure the caller is mapping a real resource for this device */
diff --git a/drivers/pci/syscall.c b/drivers/pci/syscall.c
index e1c1ec5..d922577 100644
--- a/drivers/pci/syscall.c
+++ b/drivers/pci/syscall.c
@@ -10,6 +10,7 @@
 #include <linux/errno.h>
 #include <linux/pci.h>
 #include <linux/syscalls.h>
+#include <linux/security.h>
 #include <asm/uaccess.h>
 #include "pci.h"
 
@@ -92,7 +93,7 @@ SYSCALL_DEFINE5(pciconfig_write, unsigned long, bus, unsigned long, dfn,
 	u32 dword;
 	int err = 0;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!capable(CAP_SYS_ADMIN) || (get_securelevel() > 0))
 		return -EPERM;
 
 	dev = pci_get_bus_and_slot(bus, dfn);
-- 
1.8.3.1

[PATCH 04/12] x86: Lock down IO port access when securelevel is enabled

[Matthew Garrett]

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 when securelevel is set.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 arch/x86/kernel/ioport.c | 5 +++--
 drivers/char/mem.c       | 7 +++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 4ddaf66..2a9ccd4 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -15,6 +15,7 @@
 #include <linux/thread_info.h>
 #include <linux/syscalls.h>
 #include <linux/bitmap.h>
+#include <linux/security.h>
 #include <asm/syscalls.h>
 
 /*
@@ -28,7 +29,7 @@ asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
 
 	if ((from + num <= from) || (from + num > IO_BITMAP_BITS))
 		return -EINVAL;
-	if (turn_on && !capable(CAP_SYS_RAWIO))
+	if (turn_on && (!capable(CAP_SYS_RAWIO) || (get_securelevel() > 0)))
 		return -EPERM;
 
 	/*
@@ -103,7 +104,7 @@ SYSCALL_DEFINE1(iopl, unsigned int, level)
 		return -EINVAL;
 	/* Trying to gain more privileges? */
 	if (level > old) {
-		if (!capable(CAP_SYS_RAWIO))
+		if (!capable(CAP_SYS_RAWIO) || (get_securelevel() > 0))
 			return -EPERM;
 	}
 	regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) | (level << 12);
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index f895a8c..741f536 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -28,6 +28,7 @@
 #include <linux/export.h>
 #include <linux/io.h>
 #include <linux/aio.h>
+#include <linux/security.h>
 
 #include <asm/uaccess.h>
 
@@ -545,6 +546,9 @@ static ssize_t read_port(struct file *file, char __user *buf,
 	unsigned long i = *ppos;
 	char __user *tmp = buf;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	if (!access_ok(VERIFY_WRITE, buf, count))
 		return -EFAULT;
 	while (count-- > 0 && i < 65536) {
@@ -563,6 +567,9 @@ static ssize_t write_port(struct file *file, const char __user *buf,
 	unsigned long i = *ppos;
 	const char __user *tmp = buf;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	if (!access_ok(VERIFY_READ, buf, count))
 		return -EFAULT;
 	while (count-- > 0 && i < 65536) {
-- 
1.8.3.1

[PATCH 05/12] Restrict /dev/mem and /dev/kmem when securelevel is set.

[Matthew Garrett]

Allowing users to write to address space provides mechanisms that may permit
modification of the kernel at runtime. Prevent this if securelevel has been
set.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 drivers/char/mem.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 741f536..91d1cff 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -159,6 +159,9 @@ static ssize_t write_mem(struct file *file, const char __user *buf,
 	unsigned long copied;
 	void *ptr;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	if (!valid_phys_addr_range(p, count))
 		return -EFAULT;
 
@@ -497,6 +500,9 @@ static ssize_t write_kmem(struct file *file, const char __user *buf,
 	char *kbuf; /* k-addr because vwrite() takes vmlist_lock rwlock */
 	int err = 0;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	if (p < (unsigned long) high_memory) {
 		unsigned long to_write = min_t(unsigned long, count,
 					       (unsigned long)high_memory - p);
-- 
1.8.3.1

[PATCH 06/12] acpi: Limit access to custom_method if securelevel is set

[Matthew Garrett]

custom_method effectively allows arbitrary access to system memory, making
it possible for an attacker to modify the kernel at runtime. Prevent this
if securelevel has been set.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 drivers/acpi/custom_method.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index 12b62f2..8b881db 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -7,6 +7,7 @@
 #include <linux/kernel.h>
 #include <linux/uaccess.h>
 #include <linux/debugfs.h>
+#include <linux/security.h>
 #include <acpi/acpi_drivers.h>
 
 #include "internal.h"
@@ -29,6 +30,9 @@ static ssize_t cm_write(struct file *file, const char __user * user_buf,
 	struct acpi_table_header table;
 	acpi_status status;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	if (!(*ppos)) {
 		/* parse the table header to get the table length */
 		if (count <= sizeof(struct acpi_table_header))
-- 
1.8.3.1

[PATCH 07/12] acpi: Ignore acpi_rsdp kernel parameter when securelevel is set

[Matthew Garrett]

From: Josh Boyer <jwboyer@redhat.com>

This option allows userspace to pass the RSDP address to the kernel, which
makes it possible for a user to execute arbitrary code in the kernel.
Disable this when securelevel is set.

Signed-off-by: Josh Boyer <jwboyer@redhat.com>
---
 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 e5f416c..f6d8977 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -45,6 +45,7 @@
 #include <linux/list.h>
 #include <linux/jiffies.h>
 #include <linux/semaphore.h>
+#include <linux/security.h>
 
 #include <asm/io.h>
 #include <asm/uaccess.h>
@@ -249,7 +250,7 @@ early_param("acpi_rsdp", setup_acpi_rsdp);
 acpi_physical_address __init acpi_os_get_root_pointer(void)
 {
 #ifdef CONFIG_KEXEC
-	if (acpi_rsdp)
+	if (acpi_rsdp && (get_securelevel <= 0))
 		return acpi_rsdp;
 #endif
 
-- 
1.8.3.1

[PATCH 08/12] kexec: Disable at runtime if securelevel has been set.

[Matthew Garrett]

kexec permits the loading and execution of arbitrary code in ring 0, which
permits the modification of the running kernel. Prevent this if securelevel
has been set.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 kernel/kexec.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index 59f7b55..98db2a1 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -32,6 +32,7 @@
 #include <linux/vmalloc.h>
 #include <linux/swap.h>
 #include <linux/syscore_ops.h>
+#include <linux/security.h>
 
 #include <asm/page.h>
 #include <asm/uaccess.h>
@@ -942,6 +943,9 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments,
 	if (!capable(CAP_SYS_BOOT))
 		return -EPERM;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	/*
 	 * Verify we have a legal set of flags
 	 * This leaves us room for future extensions.
-- 
1.8.3.1

[PATCH 09/12] uswsusp: Disable when securelevel is set

[Matthew Garrett]

uswsusp allows a user process to dump and then restore kernel state, which
makes it possible to modify the running kernel. Disable this if securelevel
has been set.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 kernel/power/user.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/power/user.c b/kernel/power/user.c
index 4ed81e7..520a9c4 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -24,6 +24,7 @@
 #include <linux/console.h>
 #include <linux/cpu.h>
 #include <linux/freezer.h>
+#include <linux/security.h>
 
 #include <asm/uaccess.h>
 
@@ -48,6 +49,9 @@ static int snapshot_open(struct inode *inode, struct file *filp)
 	struct snapshot_data *data;
 	int error;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	lock_system_sleep();
 
 	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
-- 
1.8.3.1

[PATCH 10/12] x86: Restrict MSR access when securelevel is set

[Matthew Garrett]

Permitting write access to MSRs allows userspace to modify the running
kernel. Prevent this if securelevel has been set. Based on a patch by Kees
Cook.

Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 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 88458fa..6b6cb4f 100644
--- a/arch/x86/kernel/msr.c
+++ b/arch/x86/kernel/msr.c
@@ -37,6 +37,7 @@
 #include <linux/notifier.h>
 #include <linux/uaccess.h>
 #include <linux/gfp.h>
+#include <linux/security.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>
@@ -103,6 +104,9 @@ static ssize_t msr_write(struct file *file, const char __user *buf,
 	int err = 0;
 	ssize_t bytes = 0;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	if (count % 8)
 		return -EINVAL;	/* Invalid chunk size */
 
@@ -150,6 +154,10 @@ static long msr_ioctl(struct file *file, unsigned int ioc, unsigned long arg)
 			err = -EBADF;
 			break;
 		}
+		if (get_securelevel() > 0) {
+			err = -EPERM;
+			break;
+		}
 		if (copy_from_user(&regs, uregs, sizeof regs)) {
 			err = -EFAULT;
 			break;
-- 
1.8.3.1

[PATCH 11/12] asus-wmi: Restrict debugfs interface when securelevel is set

[Matthew Garrett]

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

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 drivers/platform/x86/asus-wmi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 19c313b..1a9847b 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -45,6 +45,7 @@
 #include <linux/seq_file.h>
 #include <linux/platform_device.h>
 #include <linux/thermal.h>
+#include <linux/security.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <acpi/video.h>
@@ -1618,6 +1619,9 @@ static int show_dsts(struct seq_file *m, void *data)
 	int err;
 	u32 retval = -1;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	err = asus_wmi_get_devstate(asus, asus->debug.dev_id, &retval);
 
 	if (err < 0)
@@ -1634,6 +1638,9 @@ static int show_devs(struct seq_file *m, void *data)
 	int err;
 	u32 retval = -1;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	err = asus_wmi_set_devstate(asus->debug.dev_id, asus->debug.ctrl_param,
 				    &retval);
 
@@ -1658,6 +1665,9 @@ static int show_call(struct seq_file *m, void *data)
 	union acpi_object *obj;
 	acpi_status status;
 
+	if (get_securelevel() > 0)
+		return -EPERM;
+
 	status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID,
 				     1, asus->debug.method_id,
 				     &input, &output);
-- 
1.8.3.1

[PATCH 12/12] Add option to automatically set securelevel when in Secure Boot mode

[Matthew Garrett]

UEFI Secure Boot provides a mechanism for ensuring that the firmware will
only load signed bootloaders and kernels. Certain use cases may also
require that the kernel prevent userspace from inserting untrusted kernel
code at runtime. Add a configuration option that enforces this automatically
when enabled.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 Documentation/x86/zero-page.txt       |  2 ++
 arch/x86/Kconfig                      | 13 +++++++++++++
 arch/x86/boot/compressed/eboot.c      | 36 +++++++++++++++++++++++++++++++++++
 arch/x86/include/uapi/asm/bootparam.h |  3 ++-
 arch/x86/kernel/setup.c               |  7 +++++++
 5 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/Documentation/x86/zero-page.txt b/Documentation/x86/zero-page.txt
index 199f453..ec38acf 100644
--- a/Documentation/x86/zero-page.txt
+++ b/Documentation/x86/zero-page.txt
@@ -30,6 +30,8 @@ Offset	Proto	Name		Meaning
 1E9/001	ALL	eddbuf_entries	Number of entries in eddbuf (below)
 1EA/001	ALL	edd_mbr_sig_buf_entries	Number of entries in edd_mbr_sig_buffer
 				(below)
+1EB/001	ALL     kbd_status      Numlock is enabled
+1EC/001	ALL     secure_boot	Secure boot is enabled in the firmware
 1EF/001	ALL	sentinel	Used to detect broken bootloaders
 290/040	ALL	edd_mbr_sig_buffer EDD MBR signatures
 2D0/A00	ALL	e820_map	E820 memory map table
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 30c40f0..6c2b0fd 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1595,6 +1595,19 @@ config EFI_STUB
 
 	  See Documentation/x86/efi-stub.txt for more information.
 
+config EFI_SECURE_BOOT_SECURELEVEL
+        def_bool n
+	depends on SECURITY_SECURELEVEL
+	depends on EFI
+	prompt "Automatically set securelevel when UEFI Secure Boot is enabled"
+	---help---
+	  UEFI Secure Boot provides a mechanism for ensuring that the
+	  firmware will only load signed bootloaders and kernels. Certain
+	  use cases may also require that the kernel restrict any userspace
+	  mechanism that could insert untrusted code into the kernel.
+	  Say Y here to automatically enable securelevel enforcement
+	  when a system boots with UEFI Secure Boot enabled.
+
 config SECCOMP
 	def_bool y
 	prompt "Enable seccomp to safely compute untrusted bytecode"
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index b7388a4..53bfe4f 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -12,6 +12,7 @@
 #include <asm/efi.h>
 #include <asm/setup.h>
 #include <asm/desc.h>
+#include <asm/bootparam_utils.h>
 
 #undef memcpy			/* Use memcpy from misc.c */
 
@@ -861,6 +862,37 @@ fail:
 	return status;
 }
 
+static int get_secure_boot(void)
+{
+	u8 sb, setup;
+	unsigned long datasize = sizeof(sb);
+	efi_guid_t var_guid = EFI_GLOBAL_VARIABLE_GUID;
+	efi_status_t status;
+
+	status = efi_call_phys5(sys_table->runtime->get_variable,
+				L"SecureBoot", &var_guid, NULL, &datasize, &sb);
+
+	if (status != EFI_SUCCESS)
+		return 0;
+
+	if (sb == 0)
+		return 0;
+
+
+	status = efi_call_phys5(sys_table->runtime->get_variable,
+				L"SetupMode", &var_guid, NULL, &datasize,
+				&setup);
+
+	if (status != EFI_SUCCESS)
+		return 0;
+
+	if (setup == 1)
+		return 0;
+
+	return 1;
+}
+
+
 /*
  * Because the x86 boot code expects to be passed a boot_params we
  * need to create one ourselves (usually the bootloader would create
@@ -1169,6 +1201,10 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table,
 	if (sys_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
 		goto fail;
 
+	sanitize_boot_params(boot_params);
+
+	boot_params->secure_boot = get_secure_boot();
+
 	setup_graphics(boot_params);
 
 	setup_efi_pci(boot_params);
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index c15ddaf..85d7685 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -131,7 +131,8 @@ struct boot_params {
 	__u8  eddbuf_entries;				/* 0x1e9 */
 	__u8  edd_mbr_sig_buf_entries;			/* 0x1ea */
 	__u8  kbd_status;				/* 0x1eb */
-	__u8  _pad5[3];					/* 0x1ec */
+	__u8  secure_boot;				/* 0x1ec */
+	__u8  _pad5[2];					/* 0x1ed */
 	/*
 	 * The sentinel is set to a nonzero value (0xff) in header.S.
 	 *
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f0de629..35ecc60 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -50,6 +50,7 @@
 #include <linux/init_ohci1394_dma.h>
 #include <linux/kvm_para.h>
 #include <linux/dma-contiguous.h>
+#include <linux/security.h>
 
 #include <linux/errno.h>
 #include <linux/kernel.h>
@@ -1126,6 +1127,12 @@ void __init setup_arch(char **cmdline_p)
 
 	io_delay_init();
 
+#ifdef CONFIG_EFI_SECURE_BOOT_SECURELEVEL
+	if (boot_params.secure_boot) {
+		set_securelevel(1);
+	}
+#endif
+
 	/*
 	 * Parse the ACPI tables for possible boot-time SMP configuration.
 	 */
-- 
1.8.3.1

Re: [PATCH 01/12] Add BSD-style securelevel support

[H. Peter Anvin]

On 09/09/2013 08:49 AM, Matthew Garrett wrote:
> Provide a coarse-grained runtime configuration option for restricting
> userspace's ability to modify the running kernel.
> 
> Signed-off-by: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
> ---
>  Documentation/security/securelevel.txt |  23 +++++++
>  include/linux/security.h               |   8 +++
>  security/Kconfig                       |   8 +++
>  security/Makefile                      |   1 +
>  security/securelevel.c                 | 116 +++++++++++++++++++++++++++++++++
>  5 files changed, 156 insertions(+)
>  create mode 100644 Documentation/security/securelevel.txt
>  create mode 100644 security/securelevel.c
> 
> diff --git a/Documentation/security/securelevel.txt b/Documentation/security/securelevel.txt
> new file mode 100644
> index 0000000..a1355a0
> --- /dev/null
> +++ b/Documentation/security/securelevel.txt
> @@ -0,0 +1,23 @@
> +Linux securelevel interface
> +---------------------------
> +
> +The Linux securelevel interface (inspired by the BSD securelevel interface)
> +is a runtime mechanism for configuring coarse-grained kernel-level security
> +restrictions. It provides a runtime configuration variable at
> +/sys/kernel/security/securelevel which can be written to by root. The
> +following values are supported:
> +
> +-1: Permanently insecure mode. This level is equivalent to level 0, but once
> +    set cannot be changed.
> +
> +0:  Insecure mode (default). This level imposes no additional kernel
> +    restrictions.
> +
> +1:  Secure mode. If set, userspace will be unable to perform direct access
> +    to PCI devices, port IO access, access system memory directly via
> +    /dev/mem and /dev/kmem, perform kexec_load(), use the userspace
> +    software suspend mechanism, insert new ACPI code at runtime via the
> +    custom_method interface or modify CPU MSRs (on x86). Certain drivers
> +    may also limit additional interfaces.
> +

This will break or have to be redefined once you have signed kexec.

	-hpa

Re: [PATCH 01/12] Add BSD-style securelevel support

[Matthew Garrett]

On Mon, 2013-09-09 at 09:27 -0700, H. Peter Anvin wrote:

> This will break or have to be redefined once you have signed kexec.

Yeah. I wasn't really sure how to define it based on an implementation
that isn't there yet - saying "kexec_load() of untrusted binaries"
implies that there's some way to do it for trusted binaries.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 01/12] Add BSD-style securelevel support

[H. Peter Anvin]

On 09/09/2013 09:30 AM, Matthew Garrett wrote:
> On Mon, 2013-09-09 at 09:27 -0700, H. Peter Anvin wrote:
> 
>> This will break or have to be redefined once you have signed kexec.
> 
> Yeah. I wasn't really sure how to define it based on an implementation
> that isn't there yet - saying "kexec_load() of untrusted binaries"
> implies that there's some way to do it for trusted binaries.
> 

However, this is the fundamental problem with securelevel: it assumes
there is not only a strict ordering between things to be secured, but
also that specific rings will remain the meaningful levels forever.

Neither of this tend to be true long time... which leads one back to
capabilities.

	-hpa

Re: [PATCH 01/12] Add BSD-style securelevel support

[Matthew Garrett]

On Mon, 2013-09-09 at 09:42 -0700, H. Peter Anvin wrote:

> Neither of this tend to be true long time... which leads one back to
> capabilities.

We can't use capabilities. Doing so breaks existing userspace.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 01/12] Add BSD-style securelevel support

[H. Peter Anvin]

On 09/09/2013 09:44 AM, Matthew Garrett wrote:
> On Mon, 2013-09-09 at 09:42 -0700, H. Peter Anvin wrote:
> 
>> Neither of this tend to be true long time... which leads one back to
>> capabilities.
> 
> We can't use capabilities. Doing so breaks existing userspace.
> 

Capabilities don't have to mean "POSIX capabilities"... although the
POSIX capability system in Linux really is a massive fail which it would
be nice to find some kind of fix for.

	-hpa

Re: [PATCH 01/12] Add BSD-style securelevel support

[Matthew Garrett]

On Mon, 2013-09-09 at 09:51 -0700, H. Peter Anvin wrote:
> On 09/09/2013 09:44 AM, Matthew Garrett wrote:
> > On Mon, 2013-09-09 at 09:42 -0700, H. Peter Anvin wrote:
> > 
> >> Neither of this tend to be true long time... which leads one back to
> >> capabilities.
> > 
> > We can't use capabilities. Doing so breaks existing userspace.
> > 
> 
> Capabilities don't have to mean "POSIX capabilities"... although the
> POSIX capability system in Linux really is a massive fail which it would
> be nice to find some kind of fix for.

Designing a worthwhile capabilities interface certainly seems like a
great thing for someone to spend a few years on, but I'm not going to be
happy if it's the only way to solve this problem.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Valdis.Kletnieks-PjAqaU27lzQ]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

On Mon, 09 Sep 2013 11:49:34 -0400, Matthew Garrett said:

> So, this is my final attempt at providing the functionality I'm interested
> in without inherently tying it to Secure Boot. There's strong parallels
> between the functionality that I'm interested in and the BSD securelevel
> interface, so here's a trivial implementation.

Although all the individual patches look like sane and reasonable things
to do, I'm not at all convinced that sticking them all under control of one
flag is really the right way to do it.  In particular, there probably needs
to be some re-thinking of the kexec, signed-module, and secure-boot stuff,
as it's still a moving target.

> So, this is my final attempt at providing the functionality I'm interested
> in without inherently tying it to Secure Boot.

You may as well bite the bullet on this one, and tie it together.  Without
Secure Boot, by the time your code runs it's already too late.  That's the
whole point of Secure Boot, after all.

[-- Attachment #2: Type: application/pgp-signature, Size: 865 bytes --]

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Mon, 2013-09-09 at 13:18 -0400, Valdis.Kletnieks@vt.edu wrote:

> You may as well bite the bullet on this one, and tie it together.  Without
> Secure Boot, by the time your code runs it's already too late.  That's the
> whole point of Secure Boot, after all.

It's already been made clear that nobody's interested in merging a
solution that's specific to Secure Boot. I can add a command line option
to set a default, and then anyone using an attesting bootloader
(TPM/TXT) can verify the state.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[David Lang]

On Mon, 9 Sep 2013, Valdis.Kletnieks@vt.edu wrote:

> On Mon, 09 Sep 2013 11:49:34 -0400, Matthew Garrett said:
>
>> So, this is my final attempt at providing the functionality I'm interested
>> in without inherently tying it to Secure Boot. There's strong parallels
>> between the functionality that I'm interested in and the BSD securelevel
>> interface, so here's a trivial implementation.
>
> Although all the individual patches look like sane and reasonable things
> to do, I'm not at all convinced that sticking them all under control of one
> flag is really the right way to do it.  In particular, there probably needs
> to be some re-thinking of the kexec, signed-module, and secure-boot stuff,
> as it's still a moving target.

Given that we know that people want signed binaries without blocking kexec, you 
should have '1' just enforce module signing and '2' (or higher) implement a full 
lockdown including kexec.

Or, eliminate the -1  permanently insecure option and make this a bitmask, if 
someone wants to enable every possible lockdown, have them set it to "all 1's", 
define the bits only as you need them.

right now
1 lock down modules
2 lock down kexec

etc

you may also want to have a 'disable module loading after this point' in the 
future.

David Lang

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Mon, 2013-09-09 at 11:25 -0700, David Lang wrote:

> Given that we know that people want signed binaries without blocking kexec, you 
> should have '1' just enforce module signing and '2' (or higher) implement a full 
> lockdown including kexec.

There's already a kernel option for that.
-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 01/12] Add BSD-style securelevel support

[Matthew Garrett]

On Mon, 2013-09-09 at 09:27 -0700, H. Peter Anvin wrote:
> On 09/09/2013 08:49 AM, Matthew Garrett wrote:

> > +1:  Secure mode. If set, userspace will be unable to perform direct access
> > +    to PCI devices, port IO access, access system memory directly via
> > +    /dev/mem and /dev/kmem, perform kexec_load(), use the userspace
> > +    software suspend mechanism, insert new ACPI code at runtime via the
> > +    custom_method interface or modify CPU MSRs (on x86). Certain drivers
> > +    may also limit additional interfaces.
> > +
> 
> This will break or have to be redefined once you have signed kexec.

So, thinking about this, how about defining it as:

1:  Secure mode. If set, userspace will be prevented from performing any
operation that would permit the insertion of untrusted code into the
running kernel. At present this includes direct access to PCI devices,
port IO access,  direct system memory access via /dev/mem and /dev/kmem,
kexec_load(), the userspace software suspend mechanism, insertion of new
ACPI code at runtime via the custom_method interface or modification of
CPU MSRs (on x86). Certain drivers may also limit additional interfaces.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[David Lang]

On Mon, 9 Sep 2013, Matthew Garrett wrote:

> On Mon, 2013-09-09 at 11:25 -0700, David Lang wrote:
>
>> Given that we know that people want signed binaries without blocking kexec, you
>> should have '1' just enforce module signing and '2' (or higher) implement a full
>> lockdown including kexec.
>
> There's already a kernel option for that.

So, if there is an existing kernel option for this, why do we need a new one?

David Lang

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Mon, 2013-09-09 at 11:40 -0700, David Lang wrote:
> On Mon, 9 Sep 2013, Matthew Garrett wrote:
> 
> > On Mon, 2013-09-09 at 11:25 -0700, David Lang wrote:
> >
> >> Given that we know that people want signed binaries without blocking kexec, you
> >> should have '1' just enforce module signing and '2' (or higher) implement a full
> >> lockdown including kexec.
> >
> > There's already a kernel option for that.
> 
> So, if there is an existing kernel option for this, why do we need a new one?

There's an existing kernel option for "I want to enforce module
signatures but I don't care about anything else". There isn't for "I
want to prevent userspace from modifying my running kernel".

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[David Lang]

On Mon, 9 Sep 2013, Matthew Garrett wrote:

> On Mon, 2013-09-09 at 11:40 -0700, David Lang wrote:
>> On Mon, 9 Sep 2013, Matthew Garrett wrote:
>>
>>> On Mon, 2013-09-09 at 11:25 -0700, David Lang wrote:
>>>
>>>> Given that we know that people want signed binaries without blocking kexec, you
>>>> should have '1' just enforce module signing and '2' (or higher) implement a full
>>>> lockdown including kexec.
>>>
>>> There's already a kernel option for that.
>>
>> So, if there is an existing kernel option for this, why do we need a new one?
>
> There's an existing kernel option for "I want to enforce module
> signatures but I don't care about anything else". There isn't for "I
> want to prevent userspace from modifying my running kernel".

So is there a way to unify these different things rather than creating yet 
another different knob?

David Lang

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Valdis.Kletnieks-PjAqaU27lzQ]

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

On Mon, 09 Sep 2013 11:25:38 -0700, David Lang said:

> Given that we know that people want signed binaries without blocking kexec, you
> should have '1' just enforce module signing and '2' (or higher) implement a full
> lockdown including kexec.

> Or, eliminate the -1  permanently insecure option and make this a bitmask, if
> someone wants to enable every possible lockdown, have them set it to "all 1's",
> define the bits only as you need them.

This strikes me as much more workable than one big sledgehammer.

[-- Attachment #2: Type: application/pgp-signature, Size: 865 bytes --]

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Mon, 2013-09-09 at 11:53 -0700, David Lang wrote:

> So is there a way to unify these different things rather than creating yet 
> another different knob?

We haven't found one that people consider generally acceptable.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Mon, 2013-09-09 at 15:01 -0400, Valdis.Kletnieks@vt.edu wrote:
> On Mon, 09 Sep 2013 11:25:38 -0700, David Lang said:
> 
> > Given that we know that people want signed binaries without blocking kexec, you
> > should have '1' just enforce module signing and '2' (or higher) implement a full
> > lockdown including kexec.
> 
> > Or, eliminate the -1  permanently insecure option and make this a bitmask, if
> > someone wants to enable every possible lockdown, have them set it to "all 1's",
> > define the bits only as you need them.
> 
> This strikes me as much more workable than one big sledgehammer.

Which combinations are you envisioning as being useful?

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[H. Peter Anvin]

On 09/09/2013 12:01 PM, Valdis.Kletnieks-PjAqaU27lzQ@public.gmane.org wrote:
> On Mon, 09 Sep 2013 11:25:38 -0700, David Lang said:
> 
>> Given that we know that people want signed binaries without
>> blocking kexec, you should have '1' just enforce module signing
>> and '2' (or higher) implement a full lockdown including kexec.
> 
>> Or, eliminate the -1  permanently insecure option and make this a
>> bitmask, if someone wants to enable every possible lockdown, have
>> them set it to "all 1's", define the bits only as you need them.
> 
> This strikes me as much more workable than one big sledgehammer.
> 

I.e. capabilities ;)

	-hpa

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Josh Boyer]

On Mon, Sep 9, 2013 at 3:41 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 09/09/2013 12:01 PM, Valdis.Kletnieks@vt.edu wrote:
>> On Mon, 09 Sep 2013 11:25:38 -0700, David Lang said:
>>
>>> Given that we know that people want signed binaries without
>>> blocking kexec, you should have '1' just enforce module signing
>>> and '2' (or higher) implement a full lockdown including kexec.
>>
>>> Or, eliminate the -1  permanently insecure option and make this a
>>> bitmask, if someone wants to enable every possible lockdown, have
>>> them set it to "all 1's", define the bits only as you need them.
>>
>> This strikes me as much more workable than one big sledgehammer.
>>
>
> I.e. capabilities ;)

Circles.  All I see here are circles.

Having lived an entire release with a capabilities based mechanism for
this in Fedora, please no.

And if you are talking about non-POSIX capabilities as you mentioned
earlier, that seems to be no different than having securelevel being a
bitmask of, well, levels.  I don't have much opinion on securelevel
being a big hammer or a bitmask of finer grained things, but I do
think it's a more manageable way forward.  Calling the implementation
"capabilities" seems to just be unnecessarily confusing.

josh

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[H. Peter Anvin]

>>
>> I.e. capabilities ;)
> 
> Circles.  All I see here are circles.
> 
> Having lived an entire release with a capabilities based mechanism for
> this in Fedora, please no.
> 
> And if you are talking about non-POSIX capabilities as you mentioned
> earlier, that seems to be no different than having securelevel being a
> bitmask of, well, levels.  I don't have much opinion on securelevel
> being a big hammer or a bitmask of finer grained things, but I do
> think it's a more manageable way forward.  Calling the implementation
> "capabilities" seems to just be unnecessarily confusing.
> 

This is the term "capability" in the general sense, not the POSIX
implementation thereof.

	-hpa

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Josh Boyer]

On Mon, Sep 9, 2013 at 3:56 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>>
>>> I.e. capabilities ;)
>>
>> Circles.  All I see here are circles.
>>
>> Having lived an entire release with a capabilities based mechanism for
>> this in Fedora, please no.
>>
>> And if you are talking about non-POSIX capabilities as you mentioned
>> earlier, that seems to be no different than having securelevel being a
>> bitmask of, well, levels.  I don't have much opinion on securelevel
>> being a big hammer or a bitmask of finer grained things, but I do
>> think it's a more manageable way forward.  Calling the implementation
>> "capabilities" seems to just be unnecessarily confusing.
>>
>
> This is the term "capability" in the general sense, not the POSIX
> implementation thereof.

See the whole last paragraph.  Particularly the last sentence.

josh

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[David Lang]

On Mon, 9 Sep 2013, Matthew Garrett wrote:

> On Mon, 2013-09-09 at 11:53 -0700, David Lang wrote:
>
>> So is there a way to unify these different things rather than creating yet
>> another different knob?
>
> We haven't found one that people consider generally acceptable.

At least you should be able to unify the implementation, even if you don't unify 
the user visible knob

If you do this with a capabilities approach, then you can implement the 'only 
load signed modules' bit and then have that bit call the existing kernel code, 
or change that existing kernel code to internally set the capabilities bit.

I see you looking for two key things

1. a sledgehammer to say "I want to be as secure as possible"

2. no way to unset the setting (barring kernel bugs/vulnerabilities)

implementing this as capabilities will still let you meet your goals, just have 
your tools that lock things down set the value to -1 (all 1's)

As long as a bit cannot be unset, only set, it still satisfies your second 
requirement to not be able to back out.

And for people who want a partial lockdown, the various capabilities bits allow 
them to get the amount of lockdown that they want.

but if you are really trying to lock down a system, there are things that you 
want to do that will break normal users (things like disabling all module 
loading, disabling mounting of filesystems, disable the connection of new USB 
devices, etc)

These are good tools to have available, but since using them will break some 
normal use cases, you really do want to be able to enable some things without 
enabling others.

What you are seeing in this discussion is that even for the set of things that 
you consider 'essential' to lock down, other people see valid cases to not lock 
them down.

If your tools only set 'known' or 'allocated' bits, you run the risk of not 
locking things down as tightly as you could, but if new bits are only allocated 
for new lockdown features, this is not a regression since you are no worse off 
than you would have been with an old kernel.


Since we are not talking POSIX capabilities here, we are talking 'secure the 
system from even root' capabilities, there is no other system that we need to 
copy. The BSD securelevel approach is a simple big hammer approach, and it's 
easy to emulate with capabilities.

David Lang

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[H. Peter Anvin]

On 09/09/2013 12:58 PM, Josh Boyer wrote:
>>
>> This is the term "capability" in the general sense, not the POSIX
>> implementation thereof.
> 
> See the whole last paragraph.  Particularly the last sentence.
> 

Yes.  I disagree with not being able to use standard terminology.

	-hpa

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Mon, 2013-09-09 at 12:59 -0700, David Lang wrote:

> At least you should be able to unify the implementation, even if you don't unify 
> the user visible knob

Well sure, I could take this integer and merge another integer into it,
but now you have the same value being modified by two different
user-visible interfaces which aren't guaranteed to have the same
semantics.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[David Lang]

On Mon, 9 Sep 2013, Josh Boyer wrote:

> On Mon, Sep 9, 2013 at 3:41 PM, H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org> wrote:
>> On 09/09/2013 12:01 PM, Valdis.Kletnieks-PjAqaU27lzQ@public.gmane.org wrote:
>>> On Mon, 09 Sep 2013 11:25:38 -0700, David Lang said:
>>>
>>>> Given that we know that people want signed binaries without
>>>> blocking kexec, you should have '1' just enforce module signing
>>>> and '2' (or higher) implement a full lockdown including kexec.
>>>
>>>> Or, eliminate the -1  permanently insecure option and make this a
>>>> bitmask, if someone wants to enable every possible lockdown, have
>>>> them set it to "all 1's", define the bits only as you need them.
>>>
>>> This strikes me as much more workable than one big sledgehammer.
>>>
>>
>> I.e. capabilities ;)
>
> Circles.  All I see here are circles.

the thing is that these are not circles. they are separate orthoginal things 
that you may or may not want to allow.

If this was a simple set of circles, then this could be defined as a vector 
instead of bitmap, the further you go the more secure you are.

But there are always going to be cases where you want to keep most of the 
lockdown, but relax just one specific aspect of it, so you want it to be a 
bitmap instead nested circles.

Now, I know that some people are going to argue that if you relax one portion 
you have a hole in your secureity so it's not worth having any security at all. 
but I've been doing security for banks for the last 16 years and I can say that 
security is not a binary thing, just because there is one hole it isn't 
worthless to close another one. Attackers are not omnificent, they don't know 
everything there is to know about your system. If you block some holes you will 
make those attaches worthless. some holes you need to leave open to avoid 
breaking the business, and you either mitigate the risk in other ways (as 
discussed in this thread, by only loading modules from media that was tested to 
be intact, even if they aren't signed), or you accept that risk and factor the 
cost of cleanup into your cusiness plans.

David Lang

> Having lived an entire release with a capabilities based mechanism for
> this in Fedora, please no.
>
> And if you are talking about non-POSIX capabilities as you mentioned
> earlier, that seems to be no different than having securelevel being a
> bitmask of, well, levels.  I don't have much opinion on securelevel
> being a big hammer or a bitmask of finer grained things, but I do
> think it's a more manageable way forward.  Calling the implementation
> "capabilities" seems to just be unnecessarily confusing.
>
> josh
>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Josh Boyer]

On Mon, Sep 9, 2013 at 4:10 PM, David Lang <david-gFPdbfVZQbY@public.gmane.org> wrote:
> On Mon, 9 Sep 2013, Josh Boyer wrote:
>
>> On Mon, Sep 9, 2013 at 3:41 PM, H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>> On 09/09/2013 12:01 PM, Valdis.Kletnieks-PjAqaU27lzQ@public.gmane.org wrote:
>>>>
>>>> On Mon, 09 Sep 2013 11:25:38 -0700, David Lang said:
>>>>
>>>>> Given that we know that people want signed binaries without
>>>>> blocking kexec, you should have '1' just enforce module signing
>>>>> and '2' (or higher) implement a full lockdown including kexec.
>>>>
>>>>
>>>>> Or, eliminate the -1  permanently insecure option and make this a
>>>>> bitmask, if someone wants to enable every possible lockdown, have
>>>>> them set it to "all 1's", define the bits only as you need them.
>>>>
>>>>
>>>> This strikes me as much more workable than one big sledgehammer.
>>>>
>>>
>>> I.e. capabilities ;)
>>
>>
>> Circles.  All I see here are circles.
>
>
> the thing is that these are not circles. they are separate orthoginal things
> that you may or may not want to allow.
>
> If this was a simple set of circles, then this could be defined as a vector
> instead of bitmap, the further you go the more secure you are.

I didn't mean your recommendation of using a bitmask.  I understood
your proposal and I don't even disagree with it really.  I was
replying to something else.

josh

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[David Lang]

On Mon, 9 Sep 2013, Matthew Garrett wrote:

> On Mon, 2013-09-09 at 12:59 -0700, David Lang wrote:
>
>> At least you should be able to unify the implementation, even if you don't unify
>> the user visible knob
>
> Well sure, I could take this integer and merge another integer into it,
> but now you have the same value being modified by two different
> user-visible interfaces which aren't guaranteed to have the same
> semantics.

It's not that you merge integers, it's that the knob that currently sets the 
signed module only loading but not anything else would have it's implementation 
changed so that instead of doing whatever it currently does, it would instead 
make an internal call to set the "require signed modules" bit, and that one 
place would implement the lockdown.

David Lang

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Mon, 2013-09-09 at 13:15 -0700, David Lang wrote:
> On Mon, 9 Sep 2013, Matthew Garrett wrote:
> 
> > On Mon, 2013-09-09 at 12:59 -0700, David Lang wrote:
> >
> >> At least you should be able to unify the implementation, even if you don't unify
> >> the user visible knob
> >
> > Well sure, I could take this integer and merge another integer into it,
> > but now you have the same value being modified by two different
> > user-visible interfaces which aren't guaranteed to have the same
> > semantics.
> 
> It's not that you merge integers, it's that the knob that currently sets the 
> signed module only loading but not anything else would have it's implementation 
> changed so that instead of doing whatever it currently does, it would instead 
> make an internal call to set the "require signed modules" bit, and that one 
> place would implement the lockdown.

Thanks.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Mimi Zohar]

On Mon, 2013-09-09 at 11:49 -0400, Matthew Garrett wrote:
> Some use cases require the ability to ensure that anything running in ring 0
> is trusted code. We have support for signing the kernel and kernel modules,
> but there's still a range of exported kernel interfaces that make it easy to
> modify the running kernel. Previous attempts to implement a generic interface
> to restrict this have included a new capability (breaks existing userspace)
> and tying it to a requirement for signed modules (breaks assumptions in
> certain situations where userspace is already running with restricted
> privileges).
> 
> So, this is my final attempt at providing the functionality I'm interested
> in without inherently tying it to Secure Boot. There's strong parallels
> between the functionality that I'm interested in and the BSD securelevel
> interface, so here's a trivial implementation. 

Thank you for not tying this functionality to UEFI secure boot.  There
are, and will continue to be, many systems out there that don't support
UEFI secure boot, yet would be interested in this functionality.

As David Lang aptly said, "... security is not a binary thing, just
because there is one hole, it isn't worthless to close another one."

Mimi

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Mon, 2013-09-09 at 11:25 -0700, David Lang wrote:

> 1 lock down modules
> 2 lock down kexec

Having thought about this, the answer is no. It presents exactly the
same problem as capabilities do - the set can never be meaningfully
extended. If an application sets only the bits it knows about, and if a
new security-sensitive feature is added to the kernel, the feature will
be left enabled and the system will be insecure. Alternatively, if an
application sets all the bits regardless of whether it knows them or
not, it may enable a lockdown feature that it otherwise required.

The only way this is useful is if all the bits are semantically
equivalent, and in that case there's no point in having anything other
than a single bit. Users who want a more fine-grained interface should
use one of the existing mechanisms for doing so - leave the kernel open
and impose the security policy from userspace using either capabilities
or selinux.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[David Lang]

On Mon, 9 Sep 2013, Matthew Garrett wrote:

> On Mon, 2013-09-09 at 11:25 -0700, David Lang wrote:
>
>> 1 lock down modules
>> 2 lock down kexec
>
> Having thought about this, the answer is no. It presents exactly the
> same problem as capabilities do - the set can never be meaningfully
> extended. If an application sets only the bits it knows about, and if a
> new security-sensitive feature is added to the kernel, the feature will
> be left enabled and the system will be insecure. Alternatively, if an
> application sets all the bits regardless of whether it knows them or
> not, it may enable a lockdown feature that it otherwise required.

In this case you are no less secure than you were before the feature was added, 
you just can't take advantage of the new feature without updating userspace.

That's a very common situation

> The only way this is useful is if all the bits are semantically
> equivalent, and in that case there's no point in having anything other
> than a single bit. Users who want a more fine-grained interface should
> use one of the existing mechanisms for doing so - leave the kernel open
> and impose the security policy from userspace using either capabilities
> or selinux.

so if you only have a single bit, how do you deal with the case where that bit 
locks down something that's required? (your reason for not just setting all bits 
in the first approach)

your arguments don't seem self consistent.


why should there only be one way to lock down a system? there are lots of 
different use cases.

If I'm building a kiosk PC (or voting machine), I want to disable a lot of 
things that I could not get away with disabling on a generic laptop. Are we 
going to have Securelevel, ReallySecurelevel, ReallyReallySecurelevel, etc? or 
can we accept that security is not binary and allow users to disable features 
in a more granualar way?

And if SELinux can do the job, what is the reason for creating this new option?

David Lang

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Kees Cook]

On Mon, Sep 9, 2013 at 4:19 PM, David Lang <david-gFPdbfVZQbY@public.gmane.org> wrote:
> On Mon, 9 Sep 2013, Matthew Garrett wrote:
>
>> On Mon, 2013-09-09 at 11:25 -0700, David Lang wrote:
>>
>>> 1 lock down modules
>>> 2 lock down kexec
>>
>>
>> Having thought about this, the answer is no. It presents exactly the
>> same problem as capabilities do - the set can never be meaningfully
>> extended. If an application sets only the bits it knows about, and if a
>> new security-sensitive feature is added to the kernel, the feature will
>> be left enabled and the system will be insecure. Alternatively, if an
>> application sets all the bits regardless of whether it knows them or
>> not, it may enable a lockdown feature that it otherwise required.
>
>
> In this case you are no less secure than you were before the feature was
> added, you just can't take advantage of the new feature without updating
> userspace.
>
> That's a very common situation
>
>
>> The only way this is useful is if all the bits are semantically
>> equivalent, and in that case there's no point in having anything other
>> than a single bit. Users who want a more fine-grained interface should
>> use one of the existing mechanisms for doing so - leave the kernel open
>> and impose the security policy from userspace using either capabilities
>> or selinux.
>
>
> so if you only have a single bit, how do you deal with the case where that
> bit locks down something that's required? (your reason for not just setting
> all bits in the first approach)
>
> your arguments don't seem self consistent.
>
>
> why should there only be one way to lock down a system? there are lots of
> different use cases.
>
> If I'm building a kiosk PC (or voting machine), I want to disable a lot of
> things that I could not get away with disabling on a generic laptop. Are we
> going to have Securelevel, ReallySecurelevel, ReallyReallySecurelevel, etc?
> or can we accept that security is not binary and allow users to disable
> features in a more granualar way?
>
> And if SELinux can do the job, what is the reason for creating this new
> option?

Not everyone uses SELinux. :) Also, it's rarely controlled the things
we want to control here.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[James Bottomley]

On Mon, 2013-09-09 at 16:20 -0700, Kees Cook wrote:
> On Mon, Sep 9, 2013 at 4:19 PM, David Lang <david-gFPdbfVZQbY@public.gmane.org> wrote:
> > On Mon, 9 Sep 2013, Matthew Garrett wrote:
> >
> >> On Mon, 2013-09-09 at 11:25 -0700, David Lang wrote:
> >>
> >>> 1 lock down modules
> >>> 2 lock down kexec
> >>
> >>
> >> Having thought about this, the answer is no. It presents exactly the
> >> same problem as capabilities do - the set can never be meaningfully
> >> extended. If an application sets only the bits it knows about, and if a
> >> new security-sensitive feature is added to the kernel, the feature will
> >> be left enabled and the system will be insecure. Alternatively, if an
> >> application sets all the bits regardless of whether it knows them or
> >> not, it may enable a lockdown feature that it otherwise required.
> >
> >
> > In this case you are no less secure than you were before the feature was
> > added, you just can't take advantage of the new feature without updating
> > userspace.
> >
> > That's a very common situation
> >
> >
> >> The only way this is useful is if all the bits are semantically
> >> equivalent, and in that case there's no point in having anything other
> >> than a single bit. Users who want a more fine-grained interface should
> >> use one of the existing mechanisms for doing so - leave the kernel open
> >> and impose the security policy from userspace using either capabilities
> >> or selinux.
> >
> >
> > so if you only have a single bit, how do you deal with the case where that
> > bit locks down something that's required? (your reason for not just setting
> > all bits in the first approach)
> >
> > your arguments don't seem self consistent.
> >
> >
> > why should there only be one way to lock down a system? there are lots of
> > different use cases.
> >
> > If I'm building a kiosk PC (or voting machine), I want to disable a lot of
> > things that I could not get away with disabling on a generic laptop. Are we
> > going to have Securelevel, ReallySecurelevel, ReallyReallySecurelevel, etc?
> > or can we accept that security is not binary and allow users to disable
> > features in a more granualar way?
> >
> > And if SELinux can do the job, what is the reason for creating this new
> > option?
> 
> Not everyone uses SELinux. :) Also, it's rarely controlled the things
> we want to control here.

It comes on by default (or its equivalent: AppArmour) in almost every
shipping distro.

The problem with Selinux/AppArmour isn't that they're not used by a lot
of people, it's that they're hugely complex and the first time they deny
access to something the user wants to do they get disabled (after the
user grubbed around a bit to find out if he could fix whatever the
problem was, concluded it was too difficult and turned them off).

I have noticed that on every upgrade, AppArmour gets turned back on and
I turn it off again when it causes some problem or other.  However, as
of OpenSUSE 12.3, I haven't actually turned it off yet (and I've been
running with it for months) so perhaps the level of distro
sophistication at configuring these things has finally reached the point
where we can make them useful?

James

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Kees Cook]

On Mon, Sep 9, 2013 at 4:30 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Mon, 2013-09-09 at 16:20 -0700, Kees Cook wrote:
>> On Mon, Sep 9, 2013 at 4:19 PM, David Lang <david@lang.hm> wrote:
>> > And if SELinux can do the job, what is the reason for creating this new
>> > option?
>>
>> Not everyone uses SELinux. :) Also, it's rarely controlled the things
>> we want to control here.
>
> It comes on by default (or its equivalent: AppArmour) in almost every
> shipping distro.

Right, if "LSM" was meant here, yeah, I do use an LSM. But they, as a
class of security policy in the kernel, handle isolation of entirely
different things. The goal of "no way to mess with ring-0" isn't
really related to the goals of the LSM in general, or specific MACs in
particular.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Mon, 2013-09-09 at 16:19 -0700, David Lang wrote:
> On Mon, 9 Sep 2013, Matthew Garrett wrote:
> > Having thought about this, the answer is no. It presents exactly the
> > same problem as capabilities do - the set can never be meaningfully
> > extended. If an application sets only the bits it knows about, and if a
> > new security-sensitive feature is added to the kernel, the feature will
> > be left enabled and the system will be insecure. Alternatively, if an
> > application sets all the bits regardless of whether it knows them or
> > not, it may enable a lockdown feature that it otherwise required.
> 
> In this case you are no less secure than you were before the feature was added, 
> you just can't take advantage of the new feature without updating userspace.

No. Say someone adds an additional lockdown bit to forbid raw access to
mounted block devices. The "Turn everything off" approach now means that
I won't be able to perform raw access to mounted block devices, even if
that's something that my use case relies on.

> > The only way this is useful is if all the bits are semantically
> > equivalent, and in that case there's no point in having anything other
> > than a single bit. Users who want a more fine-grained interface should
> > use one of the existing mechanisms for doing so - leave the kernel open
> > and impose the security policy from userspace using either capabilities
> > or selinux.
> 
> so if you only have a single bit, how do you deal with the case where that bit 
> locks down something that's required? (your reason for not just setting all bits 
> in the first approach)

Because that bit is well-defined, and if anything is added to it that
doesn't match that definition then it's a bug.

> your arguments don't seem self consistent.

You don't seem to have been paying attention to the past 12 months of
discussion.

> If I'm building a kiosk PC (or voting machine), I want to disable a lot of 
> things that I could not get away with disabling on a generic laptop. Are we 
> going to have Securelevel, ReallySecurelevel, ReallyReallySecurelevel, etc? or 
> can we accept that security is not binary and allow users to disable features 
> in a more granualar way?

Anything more granular means that you trust your userspace, and if you
trust your userspace then you can already set up a granular policy using
the existing tools for that job. So just use the existing tools.

> And if SELinux can do the job, what is the reason for creating this new option?

Because you can't embed an unmodifiable selinux policy in the kernel.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[David Lang]

On Tue, 10 Sep 2013, Matthew Garrett wrote:

> On Mon, 2013-09-09 at 16:19 -0700, David Lang wrote:
>> On Mon, 9 Sep 2013, Matthew Garrett wrote:
>>> Having thought about this, the answer is no. It presents exactly the
>>> same problem as capabilities do - the set can never be meaningfully
>>> extended. If an application sets only the bits it knows about, and if a
>>> new security-sensitive feature is added to the kernel, the feature will
>>> be left enabled and the system will be insecure. Alternatively, if an
>>> application sets all the bits regardless of whether it knows them or
>>> not, it may enable a lockdown feature that it otherwise required.
>>
>> In this case you are no less secure than you were before the feature was added,
>> you just can't take advantage of the new feature without updating userspace.
>
> No. Say someone adds an additional lockdown bit to forbid raw access to
> mounted block devices. The "Turn everything off" approach now means that
> I won't be able to perform raw access to mounted block devices, even if
> that's something that my use case relies on.

I was meaning that if you only turn off features that you know about, the 
addition of a new thing that can be disabled doesn't make you any worse off than 
you were.

>>> The only way this is useful is if all the bits are semantically
>>> equivalent, and in that case there's no point in having anything other
>>> than a single bit. Users who want a more fine-grained interface should
>>> use one of the existing mechanisms for doing so - leave the kernel open
>>> and impose the security policy from userspace using either capabilities
>>> or selinux.
>>
>> so if you only have a single bit, how do you deal with the case where that bit
>> locks down something that's required? (your reason for not just setting all bits
>> in the first approach)
>
> Because that bit is well-defined, and if anything is added to it that
> doesn't match that definition then it's a bug.

it may be well defined, but that doesn't mean that it actually matches what the 
system owner wants to do.

The idea that the programmer can possibly anticipate all possible needs and 
provide a switch for exactly that need is just wrong. Users will have needs that 
you never thought of. The best systems are the ones where the creators look at 
what users are doing and react with "I never imagined doing that"

defining the "one true way" of operating is just wrong.

>> your arguments don't seem self consistent.
>
> You don't seem to have been paying attention to the past 12 months of
> discussion.
>
>> If I'm building a kiosk PC (or voting machine), I want to disable a lot of
>> things that I could not get away with disabling on a generic laptop. Are we
>> going to have Securelevel, ReallySecurelevel, ReallyReallySecurelevel, etc? or
>> can we accept that security is not binary and allow users to disable features
>> in a more granualar way?
>
> Anything more granular means that you trust your userspace, and if you
> trust your userspace then you can already set up a granular policy using
> the existing tools for that job. So just use the existing tools.

If you can't trust your userspace, how do you know that the userspace has set 
the big hammer flag in the first place? if you can trust it to throw that 
switch, you can trust it to throw multiple smaller switches.

>> And if SELinux can do the job, what is the reason for creating this new option?
>
> Because you can't embed an unmodifiable selinux policy in the kernel.

Why not, have the policy set in an initramfs that's part of the kernel and have 
part of that policy be to block all access to the selinux controls.

David Lang

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Mon, 2013-09-09 at 19:44 -0700, David Lang wrote:
> On Tue, 10 Sep 2013, Matthew Garrett wrote:
> > No. Say someone adds an additional lockdown bit to forbid raw access to
> > mounted block devices. The "Turn everything off" approach now means that
> > I won't be able to perform raw access to mounted block devices, even if
> > that's something that my use case relies on.
> 
> I was meaning that if you only turn off features that you know about, the 
> addition of a new thing that can be disabled doesn't make you any worse off than 
> you were.

Someone adds a new "install_evil()" syscall and adds a disable bit. If I
don't disable it, I'm now vulnerable. Please pay attention to earlier
discussion.

> >> so if you only have a single bit, how do you deal with the case where that bit
> >> locks down something that's required? (your reason for not just setting all bits
> >> in the first approach)
> >
> > Because that bit is well-defined, and if anything is added to it that
> > doesn't match that definition then it's a bug.
> 
> it may be well defined, but that doesn't mean that it actually matches what the 
> system owner wants to do.

If it doesn't match what the system owner wants to do, the system owner
doesn't set it. The system owner uses a more appropriate security
mechanism instead.

> The idea that the programmer can possibly anticipate all possible needs and 
> provide a switch for exactly that need is just wrong. Users will have needs that 
> you never thought of. The best systems are the ones where the creators look at 
> what users are doing and react with "I never imagined doing that"

Describe the security case for disabling PCI BAR access but permitting
i/o port access.

> > Anything more granular means that you trust your userspace, and if you
> > trust your userspace then you can already set up a granular policy using
> > the existing tools for that job. So just use the existing tools.
> 
> If you can't trust your userspace, how do you know that the userspace has set 
> the big hammer flag in the first place? if you can trust it to throw that 
> switch, you can trust it to throw multiple smaller switches.

Hence the final patch in the series, and hence also the suggestion for
exposing it as a command line option that can be set by the bootloader
during an attested boot.

> >> And if SELinux can do the job, what is the reason for creating this new option?
> >
> > Because you can't embed an unmodifiable selinux policy in the kernel.
> 
> Why not, have the policy set in an initramfs that's part of the kernel and have 
> part of that policy be to block all access to the selinux controls.

Because then someone disables selinux on the kernel command line.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[David Lang]

On Tue, 10 Sep 2013, Matthew Garrett wrote:

> On Mon, 2013-09-09 at 19:44 -0700, David Lang wrote:
>> On Tue, 10 Sep 2013, Matthew Garrett wrote:
>>> No. Say someone adds an additional lockdown bit to forbid raw access to
>>> mounted block devices. The "Turn everything off" approach now means that
>>> I won't be able to perform raw access to mounted block devices, even if
>>> that's something that my use case relies on.
>>
>> I was meaning that if you only turn off features that you know about, the
>> addition of a new thing that can be disabled doesn't make you any worse off than
>> you were.
>
> Someone adds a new "install_evil()" syscall and adds a disable bit. If I
> don't disable it, I'm now vulnerable. Please pay attention to earlier
> discussion.

so instead they add install_evil() and don't have it be disabled by your big 
switch.

or do you think the existance of this switch will give you veto power over any 
new system calls unless they include the ability for them to be disabled if they 
don't match your security model.

>>>> so if you only have a single bit, how do you deal with the case where that bit
>>>> locks down something that's required? (your reason for not just setting all bits
>>>> in the first approach)
>>>
>>> Because that bit is well-defined, and if anything is added to it that
>>> doesn't match that definition then it's a bug.
>>
>> it may be well defined, but that doesn't mean that it actually matches what the
>> system owner wants to do.
>
> If it doesn't match what the system owner wants to do, the system owner
> doesn't set it. The system owner uses a more appropriate security
> mechanism instead.

SELinux is a wonderful example of how making a system that users cannot change 
doesn't improve security, it just gets disabled instead.

>> The idea that the programmer can possibly anticipate all possible needs and
>> provide a switch for exactly that need is just wrong. Users will have needs that
>> you never thought of. The best systems are the ones where the creators look at
>> what users are doing and react with "I never imagined doing that"
>
> Describe the security case for disabling PCI BAR access but permitting
> i/o port access.

Simple, I have some device that lives on those I/O ports that I want to be able 
to use.

>>> Anything more granular means that you trust your userspace, and if you
>>> trust your userspace then you can already set up a granular policy using
>>> the existing tools for that job. So just use the existing tools.
>>
>> If you can't trust your userspace, how do you know that the userspace has set
>> the big hammer flag in the first place? if you can trust it to throw that
>> switch, you can trust it to throw multiple smaller switches.
>
> Hence the final patch in the series, and hence also the suggestion for
> exposing it as a command line option that can be set by the bootloader
> during an attested boot.
>
>>>> And if SELinux can do the job, what is the reason for creating this new option?
>>>
>>> Because you can't embed an unmodifiable selinux policy in the kernel.
>>
>> Why not, have the policy set in an initramfs that's part of the kernel and have
>> part of that policy be to block all access to the selinux controls.
>
> Because then someone disables selinux on the kernel command line.

If they can modify the command line, they can remove your command line switch to 
turn this on.

If you really care about this, why are you using a bootloader that lets you 
modify the kernel command line in the first place?

In any case, even if you make it impossible to change from the command line, you 
won't prevent people from changing your system (unless you take control 
completely away from them with TPM or similar)

remember that the system integrity checking of the original Tivo was defeated by 
someone dong a binary patch to bypass a routine that they didn't understand that 
took a lot of time, so they did a binary patch to the bios to speed up boot and 
discovered that what they did was disable the system integrity checking.

users who own the systems are going to modify them and bypass any restrictions 
you want to impose.

users who don't own the systems can be defeated by simply not offering them the 
option to change the settings in the first place.

David Lang

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Mon, 2013-09-09 at 20:09 -0700, David Lang wrote:
> On Tue, 10 Sep 2013, Matthew Garrett wrote:

> > Someone adds a new "install_evil()" syscall and adds a disable bit. If I
> > don't disable it, I'm now vulnerable. Please pay attention to earlier
> > discussion.
> 
> so instead they add install_evil() and don't have it be disabled by your big 
> switch.

And that's a bug, so we fix it.

> > Describe the security case for disabling PCI BAR access but permitting
> > i/o port access.
> 
> Simple, I have some device that lives on those I/O ports that I want to be able 
> to use.

That's a great argument for permitting i/o port access. But in that
case, why are you disabling BAR access? You can use the i/o port access
to reprogram the device in question into a range you can modify, which
allows you to avoid the BAR restriction.

> > Because then someone disables selinux on the kernel command line.
> 
> If they can modify the command line, they can remove your command line switch to 
> turn this on.

Not in the secure boot case.

> If you really care about this, why are you using a bootloader that lets you 
> modify the kernel command line in the first place?

And then the user just modifies the configuration file instead.

> In any case, even if you make it impossible to change from the command line, you 
> won't prevent people from changing your system (unless you take control 
> completely away from them with TPM or similar)

That's fine. People are free to modify their own systems.

> remember that the system integrity checking of the original Tivo was defeated by 
> someone dong a binary patch to bypass a routine that they didn't understand that 
> took a lot of time, so they did a binary patch to the bios to speed up boot and 
> discovered that what they did was disable the system integrity checking.

That's why modern systems require signed firmware updates.

> users who own the systems are going to modify them and bypass any restrictions 
> you want to impose.

The idea isn't to produce something that's impossible for the owner of a
system to disable. The idea is to produce something that can't be used
to circumvent the security policy that the system owner has chosen.
Could you please give me the benefit of the doubt and assume that I'm
not completely unaware of how computers work?

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Henrique de Moraes Holschuh]

On Tue, 10 Sep 2013, Matthew Garrett wrote:
> That's why modern systems require signed firmware updates.

Linux doesn't.  Is someone working on adding signature support to the
runtime firmware loader?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Matthew Garrett]

On Tue, 2013-09-10 at 14:23 -0300, Henrique de Moraes Holschuh wrote:
> On Tue, 10 Sep 2013, Matthew Garrett wrote:
> > That's why modern systems require signed firmware updates.
> 
> Linux doesn't.  Is someone working on adding signature support to the
> runtime firmware loader?

It'd be simple to do so, but so far the model appears to be that devices
that expect signed firmware enforce that themselves.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[H. Peter Anvin]

On 09/10/2013 11:26 AM, Matthew Garrett wrote:
> On Tue, 2013-09-10 at 14:23 -0300, Henrique de Moraes Holschuh wrote:
>> On Tue, 10 Sep 2013, Matthew Garrett wrote:
>>> That's why modern systems require signed firmware updates.
>>
>> Linux doesn't.  Is someone working on adding signature support to the
>> runtime firmware loader?
> 
> It'd be simple to do so, but so far the model appears to be that devices
> that expect signed firmware enforce that themselves.
> 

Most devices do absolutely no verification on the firmware, and simply
trust the driver.

So signing firmware is probably critical.

	-hpa

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Kees Cook]

On Tue, Sep 10, 2013 at 11:26 AM, Matthew Garrett
<matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, 2013-09-10 at 14:23 -0300, Henrique de Moraes Holschuh wrote:
>> On Tue, 10 Sep 2013, Matthew Garrett wrote:
>> > That's why modern systems require signed firmware updates.
>>
>> Linux doesn't.  Is someone working on adding signature support to the
>> runtime firmware loader?

I feel like there was maybe confusion here between "boot loader"
firmware (PC-BIOS, UEFI, etc), and device (maybe "component" is a
better term to distinguish this?) firmware (network cards, hard
drives, etc). Boot loader firmware has been moving rapidly toward
verified updates. This is true in many many shipping systems. It is
much less true for component firmware.

> It'd be simple to do so, but so far the model appears to be that devices
> that expect signed firmware enforce that themselves.

Yeah, the unfortunately reality is that for full sanity, it is
components themselves that need to be doing this signature validation.
That said, adding signature (or similar "origin" verification) to the
kernel is a good first step to move the trust from uid-0 up to ring-0.
I've had this on my TODO list for a while now. It remains a potential
hole, but since a solution doesn't exist today, it's outside of what
Matthew's patch series does. I would, however, expect that in the
future when component firmware loading includes origin verification,
it would become required when running with the "lock down the world"
setting.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[gregkh]

On Tue, Sep 10, 2013 at 11:29:45AM -0700, H. Peter Anvin wrote:
> On 09/10/2013 11:26 AM, Matthew Garrett wrote:
> > On Tue, 2013-09-10 at 14:23 -0300, Henrique de Moraes Holschuh wrote:
> >> On Tue, 10 Sep 2013, Matthew Garrett wrote:
> >>> That's why modern systems require signed firmware updates.
> >>
> >> Linux doesn't.  Is someone working on adding signature support to the
> >> runtime firmware loader?
> > 
> > It'd be simple to do so, but so far the model appears to be that devices
> > that expect signed firmware enforce that themselves.
> > 
> 
> Most devices do absolutely no verification on the firmware, and simply
> trust the driver.
> 
> So signing firmware is probably critical.

How are you going to "validate" that the firmware is correct, given
that it's just a "blob" living in the linux-firmware tree.  If you sign
it, what is that saying?

I'm with Matthew here, any device that needs/wants this, has their own
built-in checking, nothing the kernel should do here.

Especially given that no other os does this :)

thanks,

greg k-h

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Kees Cook]

On Tue, Sep 10, 2013 at 11:51 AM, gregkh@linuxfoundation.org
<gregkh@linuxfoundation.org> wrote:
> On Tue, Sep 10, 2013 at 11:29:45AM -0700, H. Peter Anvin wrote:
>> On 09/10/2013 11:26 AM, Matthew Garrett wrote:
>> > On Tue, 2013-09-10 at 14:23 -0300, Henrique de Moraes Holschuh wrote:
>> >> On Tue, 10 Sep 2013, Matthew Garrett wrote:
>> >>> That's why modern systems require signed firmware updates.
>> >>
>> >> Linux doesn't.  Is someone working on adding signature support to the
>> >> runtime firmware loader?
>> >
>> > It'd be simple to do so, but so far the model appears to be that devices
>> > that expect signed firmware enforce that themselves.
>> >
>>
>> Most devices do absolutely no verification on the firmware, and simply
>> trust the driver.
>>
>> So signing firmware is probably critical.
>
> How are you going to "validate" that the firmware is correct, given
> that it's just a "blob" living in the linux-firmware tree.  If you sign
> it, what is that saying?

In theory these blobs are traceable to a manufacturer. It's not really
an indication that it's "safe" more than it's an indication that it
hasn't been changed. But I haven't chased this very hard yet because
of below...

> I'm with Matthew here, any device that needs/wants this, has their own
> built-in checking, nothing the kernel should do here.
>
> Especially given that no other os does this :)

Yeah, it's impossible to handle since the way components do firmware
updates is frequently exposed to userspace anyway. 3G modems that do
firmware updates over the AT-command set, harddrives doing firmware
updates over SCSI-generic commands, etc. Creating this barrier in the
kernel is not a good solution; the component makers need to be doing
the enforcement. :(

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[David Lang]

On Tue, 10 Sep 2013, Kees Cook wrote:

> Subject: Re: [PATCH 00/12] One more attempt at useful kernel lockdown
> 
> On Tue, Sep 10, 2013 at 11:51 AM, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org
> <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
>> On Tue, Sep 10, 2013 at 11:29:45AM -0700, H. Peter Anvin wrote:
>>> On 09/10/2013 11:26 AM, Matthew Garrett wrote:
>>>> On Tue, 2013-09-10 at 14:23 -0300, Henrique de Moraes Holschuh wrote:
>>>>> On Tue, 10 Sep 2013, Matthew Garrett wrote:
>>>>>> That's why modern systems require signed firmware updates.
>>>>>
>>>>> Linux doesn't.  Is someone working on adding signature support to the
>>>>> runtime firmware loader?
>>>>
>>>> It'd be simple to do so, but so far the model appears to be that devices
>>>> that expect signed firmware enforce that themselves.
>>>>
>>>
>>> Most devices do absolutely no verification on the firmware, and simply
>>> trust the driver.
>>>
>>> So signing firmware is probably critical.
>>
>> How are you going to "validate" that the firmware is correct, given
>> that it's just a "blob" living in the linux-firmware tree.  If you sign
>> it, what is that saying?
>
> In theory these blobs are traceable to a manufacturer. It's not really
> an indication that it's "safe" more than it's an indication that it
> hasn't been changed. But I haven't chased this very hard yet because
> of below...

well, not if you are trying to defend against root breaking in to the machine.

David Lang

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[H. Peter Anvin]

On 09/10/2013 12:17 PM, David Lang wrote:
>>
>> In theory these blobs are traceable to a manufacturer. It's not really
>> an indication that it's "safe" more than it's an indication that it
>> hasn't been changed. But I haven't chased this very hard yet because
>> of below...
> 
> well, not if you are trying to defend against root breaking in to the
> machine.
> 

And we have at least some drivers where we even have the firmware in the
Linux kernel tree, and thus aren't opaque blobs at all.

I suspect we'll need, at some point, a way for vendors that aren't
already doing signatures on their firmware in a device-specific way to
do so in a kernel-supported way.  The easiest (in terms of getting
vendors to play along, not necessarily technically) might be a PGP
signature (either inline or standalone) and have the public key as part
of the driver?

	-hpa

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Mimi Zohar]

On Tue, 2013-09-10 at 12:44 -0700, H. Peter Anvin wrote:
> On 09/10/2013 12:17 PM, David Lang wrote:
> >>
> >> In theory these blobs are traceable to a manufacturer. It's not really
> >> an indication that it's "safe" more than it's an indication that it
> >> hasn't been changed. But I haven't chased this very hard yet because
> >> of below...
> > 
> > well, not if you are trying to defend against root breaking in to the
> > machine.
> > 
> 
> And we have at least some drivers where we even have the firmware in the
> Linux kernel tree, and thus aren't opaque blobs at all.
> 
> I suspect we'll need, at some point, a way for vendors that aren't
> already doing signatures on their firmware in a device-specific way to
> do so in a kernel-supported way.  The easiest (in terms of getting
> vendors to play along, not necessarily technically) might be a PGP
> signature (either inline or standalone) and have the public key as part
> of the driver?

Why invent yet another method of verifying the integrity of a file based
on a signature?  Why not use the existing method for appraising files?
Just create a new integrity hook at the appropriate place.

Mimi

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[H. Peter Anvin]

On 09/10/2013 04:43 PM, Mimi Zohar wrote:
> 
> Why invent yet another method of verifying the integrity of a file based
> on a signature?  Why not use the existing method for appraising files?
> Just create a new integrity hook at the appropriate place.
> 

What would the deliverables be from the hardware vendor and what tools
would you expect them to need on their end?

	-hpa

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[Mimi Zohar]

On Tue, 2013-09-10 at 16:48 -0700, H. Peter Anvin wrote:
> On 09/10/2013 04:43 PM, Mimi Zohar wrote:
> > 
> > Why invent yet another method of verifying the integrity of a file based
> > on a signature?  Why not use the existing method for appraising files?
> > Just create a new integrity hook at the appropriate place.
> > 
> 
> What would the deliverables be from the hardware vendor and what tools
> would you expect them to need on their end?

The package installer needs to not only install files, but file metadata
as well.  Elena Reshetova (Intel) has already added rpm hooks to write
security xattrs.  The next step, yet to be done, is to include and write
the signatures as part of the rpm install process.

Mimi

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[H. Peter Anvin]

On 09/10/2013 04:55 PM, Mimi Zohar wrote:
>>
>> What would the deliverables be from the hardware vendor and what tools
>> would you expect them to need on their end?
> 
> The package installer needs to not only install files, but file metadata
> as well.  Elena Reshetova (Intel) has already added rpm hooks to write
> security xattrs.  The next step, yet to be done, is to include and write
> the signatures as part of the rpm install process.
> 

That's a total non-option.

There needs to be something that can be done even on a Windows box by a
largely untrained release engineer if we're going to have a prayer of
getting this supported.

So, there is your answer why not.

	-hpa

Re: [PATCH 00/12] One more attempt at useful kernel lockdown

[joeyli]

於 二，2013-09-10 於 18:26 +0000，Matthew Garrett 提到：
> On Tue, 2013-09-10 at 14:23 -0300, Henrique de Moraes Holschuh wrote:
> > On Tue, 10 Sep 2013, Matthew Garrett wrote:
> > > That's why modern systems require signed firmware updates.
> > 
> > Linux doesn't.  Is someone working on adding signature support to the
> > runtime firmware loader?
> 
> It'd be simple to do so, but so far the model appears to be that devices
> that expect signed firmware enforce that themselves.
> 
> -- 
> Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
> NrybXǧv^)޺{.n+{y^nrz\x1ah&\x1eGh\x03(階ݢj"\x1a^[mzޖfh~m

Takashi has a implementation of firmware check:

[PATCH RFC v2 0/4] Add firmware signature file check
https://lkml.org/lkml/2012/11/8/343


Thanks
Joey Lee

Re: [PATCH 07/12] acpi: Ignore acpi_rsdp kernel parameter when securelevel is set

[Josh Boyer]

On Mon, Sep 9, 2013 at 11:49 AM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> From: Josh Boyer <jwboyer@redhat.com>
>
> This option allows userspace to pass the RSDP address to the kernel, which
> makes it possible for a user to execute arbitrary code in the kernel.
> Disable this when securelevel is set.
>
> Signed-off-by: Josh Boyer <jwboyer@redhat.com>

Dredging up an old thread in the hopes that Matthew runs sed and resubmits...

Also, FWIW, I didn't write this.  It was derived from previous
versions of something I did write, but there's really no evidence of
anything I wrote left, so it should probably be From: you.

> ---
>  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 e5f416c..f6d8977 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -45,6 +45,7 @@
>  #include <linux/list.h>
>  #include <linux/jiffies.h>
>  #include <linux/semaphore.h>
> +#include <linux/security.h>
>
>  #include <asm/io.h>
>  #include <asm/uaccess.h>
> @@ -249,7 +250,7 @@ early_param("acpi_rsdp", setup_acpi_rsdp);
>  acpi_physical_address __init acpi_os_get_root_pointer(void)
>  {
>  #ifdef CONFIG_KEXEC
> -       if (acpi_rsdp)
> +       if (acpi_rsdp && (get_securelevel <= 0))

This is missing some ( ).  That means you're comparing the
get_securelevel function pointer to 0.  Pretty sure bad things will
happen.

josh