2013-09-09 15:55:26

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 00/12] One more attempt at useful kernel lockdown

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.


2013-09-09 15:52:28

by Matthew Garrett

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

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 <[email protected]>
---
drivers/char/mem.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 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

2013-09-09 15:52:26

by Matthew Garrett

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

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 <[email protected]>
---
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

2013-09-09 15:52:25

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 01/12] Add BSD-style securelevel support

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

Signed-off-by: Matthew Garrett <[email protected]>
---
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 <[email protected]>
+ *
+ * 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

2013-09-09 15:52:23

by Matthew Garrett

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

From: Josh Boyer <[email protected]>

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

Signed-off-by: Josh Boyer <[email protected]>
---
drivers/acpi/osl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 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

2013-09-09 15:52:21

by Matthew Garrett

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

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 <[email protected]>
---
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

2013-09-09 15:52:19

by Matthew Garrett

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

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 <[email protected]>
---
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

2013-09-09 15:52:15

by Matthew Garrett

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

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 <[email protected]>
---
drivers/pci/pci-sysfs.c | 9 +++++++++
drivers/pci/proc.c | 9 ++++++++-
drivers/pci/syscall.c | 3 ++-
3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 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

2013-09-09 15:54:30

by Matthew Garrett

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

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 <[email protected]>
---
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

2013-09-09 15:55:24

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 02/12] Enforce module signatures when securelevel is greater than 0

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

Signed-off-by: Matthew Garrett <[email protected]>
---
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

2013-09-09 16:00:56

by Matthew Garrett

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

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 <[email protected]>
---
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

2013-09-09 16:01:08

by Matthew Garrett

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

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 <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
arch/x86/kernel/msr.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
index 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

2013-09-09 16:01:18

by Matthew Garrett

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

UEFI Secure Boot provides a mechanism for ensuring that the firmware will
only load signed bootloaders and kernels. Certain use cases may also
require that 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 <[email protected]>
---
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

2013-09-09 16:27:26

by H. Peter Anvin

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

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 <[email protected]>
> ---
> 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

2013-09-09 16:30:15

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 16:43:13

by H. Peter Anvin

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

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

2013-09-09 16:44:49

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 16:52:15

by H. Peter Anvin

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

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

2013-09-09 16:55:25

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 17:19:53

by Valdis Klētnieks

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

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.


Attachments:
(No filename) (865.00 B)

2013-09-09 17:24:23

by Matthew Garrett

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

On Mon, 2013-09-09 at 13:18 -0400, [email protected] 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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 18:27:15

by David Lang

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

On Mon, 9 Sep 2013, [email protected] 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

2013-09-09 18:28:45

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 18:31:13

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 18:40:58

by David Lang

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

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

2013-09-09 18:42:46

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 18:53:47

by David Lang

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

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

2013-09-09 19:01:49

by Valdis Klētnieks

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

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.


Attachments:
(No filename) (865.00 B)

2013-09-09 19:07:08

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 19:08:23

by Matthew Garrett

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

On Mon, 2013-09-09 at 15:01 -0400, [email protected] 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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 19:42:40

by H. Peter Anvin

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

On 09/09/2013 12:01 PM, [email protected] 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

2013-09-09 19:52:50

by Josh Boyer

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

On Mon, Sep 9, 2013 at 3:41 PM, H. Peter Anvin <[email protected]> wrote:
> On 09/09/2013 12:01 PM, [email protected] 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

2013-09-09 19:56:44

by H. Peter Anvin

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

>>
>> 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

2013-09-09 19:58:19

by Josh Boyer

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

On Mon, Sep 9, 2013 at 3:56 PM, H. Peter Anvin <[email protected]> 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

2013-09-09 20:00:21

by David Lang

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

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

2013-09-09 20:03:12

by H. Peter Anvin

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

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

2013-09-09 20:06:47

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 20:10:36

by David Lang

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

On Mon, 9 Sep 2013, Josh Boyer wrote:

> On Mon, Sep 9, 2013 at 3:41 PM, H. Peter Anvin <[email protected]> wrote:
>> On 09/09/2013 12:01 PM, [email protected] 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
>

2013-09-09 20:13:59

by Josh Boyer

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

On Mon, Sep 9, 2013 at 4:10 PM, David Lang <[email protected]> wrote:
> On Mon, 9 Sep 2013, Josh Boyer wrote:
>
>> On Mon, Sep 9, 2013 at 3:41 PM, H. Peter Anvin <[email protected]> wrote:
>>>
>>> On 09/09/2013 12:01 PM, [email protected] 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

2013-09-09 20:16:04

by David Lang

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

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

2013-09-09 20:17:50

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 20:30:57

by Mimi Zohar

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

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

2013-09-09 23:02:55

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 23:19:37

by David Lang

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

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

2013-09-09 23:20:56

by Kees Cook

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

On Mon, Sep 9, 2013 at 4:19 PM, David Lang <[email protected]> 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

2013-09-09 23:30:42

by James Bottomley

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

On Mon, 2013-09-09 at 16:20 -0700, Kees Cook wrote:
> On Mon, Sep 9, 2013 at 4:19 PM, David Lang <[email protected]> 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

2013-09-09 23:34:11

by Kees Cook

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

On Mon, Sep 9, 2013 at 4:30 PM, James Bottomley
<[email protected]> wrote:
> On Mon, 2013-09-09 at 16:20 -0700, Kees Cook wrote:
>> On Mon, Sep 9, 2013 at 4:19 PM, David Lang <[email protected]> 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

2013-09-10 00:53:22

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-10 02:45:05

by David Lang

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

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

2013-09-10 02:55:23

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-10 03:09:17

by David Lang

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

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

2013-09-10 03:53:40

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

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

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

2013-09-10 18:26:20

by Matthew Garrett

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

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 <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-10 18:30:16

by H. Peter Anvin

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

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

2013-09-10 18:48:10

by Kees Cook

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

On Tue, Sep 10, 2013 at 11:26 AM, Matthew Garrett
<[email protected]> 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

2013-09-10 18:51:52

by Greg Kroah-Hartman

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

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

2013-09-10 18:55:53

by Kees Cook

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

On Tue, Sep 10, 2013 at 11:51 AM, [email protected]
<[email protected]> 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

2013-09-10 19:18:32

by David Lang

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

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, [email protected]
> <[email protected]> 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

2013-09-10 19:44:56

by H. Peter Anvin

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

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

2013-09-10 23:43:30

by Mimi Zohar

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

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

2013-09-10 23:49:01

by H. Peter Anvin

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

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

2013-09-10 23:55:36

by Mimi Zohar

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

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

2013-09-10 23:58:27

by H. Peter Anvin

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

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

2013-09-11 09:32:21

by joeyli

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

於 二,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 <[email protected]>
> NrybXǧv^)޺{.n+{y^nrzh&Gh(階ݢj"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