Trusted kernel patchset for Secure Boot lockdown

Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

The conclusion we came to at Plumbers was that this patchset was basically
fine but that Linus hated the name "securelevel" more than I hate pickled
herring, so after thinking about this for a few months I've come up with
"Trusted Kernel". This flag indicates that the kernel is, via some
external mechanism, trusted and should behave that way. If firmware has
some way to verify the kernel, it can pass that information on. If userspace
has some way to verify the kernel, it can set the flag itself. However,
userspace should not attempt to use the flag as a means to verify that the
kernel was trusted - untrusted userspace could have set it on an untrusted
kernel, but by the same metric an untrusted kernel could just set it itself.

If people object to this name then I swear to god that I will open a poll
on Phoronix to decide the next attempt and you will like that even less.

[PATCH 01/12] Add support for indicating that the booted kernel is externally trusted

[Matthew Garrett]

Provide a boolean runtime configuration option for restricting userspace's
ability to modify the running kernel. This can be used when some external
validation of the kernel's state has been performed.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 Documentation/kernel-parameters.txt       |   6 ++
 Documentation/security/trusted_kernel.txt |  35 ++++++++++
 include/linux/security.h                  |   8 +++
 security/Kconfig                          |   9 +++
 security/Makefile                         |   1 +
 security/trusted_kernel.c                 | 111 ++++++++++++++++++++++++++++++
 6 files changed, 170 insertions(+)
 create mode 100644 Documentation/security/trusted_kernel.txt
 create mode 100644 security/trusted_kernel.c

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 7116fda..d82ba9e 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3271,6 +3271,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			with respect to transparent hugepages.
 			See Documentation/vm/transhuge.txt for more details.
 
+	trusted_kernel	Indicate that the booted kernel has been verified to
+			be trustworthy and that userspace should be forbidden
+			from modifying it at runtime.
+			See Documentation/security/trusted_kernel.txt for more
+			details.
+
 	tsc=		Disable clocksource stability checks for TSC.
 			Format: <string>
 			[x86] reliable: mark tsc clocksource as reliable, this
diff --git a/Documentation/security/trusted_kernel.txt b/Documentation/security/trusted_kernel.txt
new file mode 100644
index 0000000..538d21d
--- /dev/null
+++ b/Documentation/security/trusted_kernel.txt
@@ -0,0 +1,35 @@
+Linux trusted kernel support
+----------------------------
+
+Various mechanisms exist to ensure that a booted kernel is trusted by the
+user or some external party (UEFI Secure Boot, Intel TXT, embedded platform
+bootloaders). If userspace is able to modify the running kernel then this
+trust can be subverted.
+
+The trusted kernel support modifies certain kernel interfaces such that
+userspace is restricted from performing acts that would allow it to inject
+untrusted code into the kernel. 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.
+
+The trusted kernel feature may be enabled in multiple ways:
+
+1) Platform-specific code may automatically enable it when it detects that
+the system has been booted appropriately
+
+2) The user or bootloader may pass the "trusted_kernel" kernel parameter
+
+3) Userspace may write "1" to the /sys/kernel/security/trusted_kernel
+node. This must be done sufficiently early in the boot process that
+untrusted userspace has no opportunity to modify the kernel.
+
+Once enabled. trusted kernel support may not be disabled without rebooting
+the system.
+
+Note that this is a mechanism for the kernel to determine whether or not
+it is externally trusted. Untrusted userspace can enable this option even
+if the kernel is not trusted, and therefore userspace should not use this
+value as an indication of whether or not the kernel is trustworthy.
diff --git a/include/linux/security.h b/include/linux/security.h
index 5623a7f..3415968 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -3091,6 +3091,14 @@ static inline void security_audit_rule_free(void *lsmrule)
 #endif /* CONFIG_SECURITY */
 #endif /* CONFIG_AUDIT */
 
+#ifdef CONFIG_SECURITY_TRUSTED_KERNEL
+extern bool get_trusted_kernel(void);
+extern int set_trusted_kernel(bool new_trusted_kernel);
+#else
+static inline bool get_trusted_kernel(void) { return 0; }
+static inline int set_trusted_kernel(bool new_trusted_kernel) { return 0; }
+#endif /* CONFIG_TRUSTED_KERNEL */
+
 #ifdef CONFIG_SECURITYFS
 
 extern struct dentry *securityfs_create_file(const char *name, umode_t mode,
diff --git a/security/Kconfig b/security/Kconfig
index beb86b5..c0462c9 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -70,6 +70,15 @@ config SECURITY_PATH
 	  implement pathname based access controls.
 	  If you are unsure how to answer this question, answer N.
 
+config SECURITY_TRUSTED_KERNEL
+        bool "Support for indicating that the kernel is trusted"
+	depends on SECURITY
+	help
+	  This enables support for adding a set of additional kernel security
+	  restrictions at runtime.
+	  See Documentation/security/trusted_kernel.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 a5918e0..72af305 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_TRUSTED_KERNEL)	+= trusted_kernel.o
 obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o
 obj-$(CONFIG_SECURITY_SMACK)		+= smack/built-in.o
 obj-$(CONFIG_AUDIT)			+= lsm_audit.o
diff --git a/security/trusted_kernel.c b/security/trusted_kernel.c
new file mode 100644
index 0000000..2808113
--- /dev/null
+++ b/security/trusted_kernel.c
@@ -0,0 +1,111 @@
+/*
+ *  trusted_kernel.c - support for generic kernel lockdown
+ *
+ *  Copyright Nebula, Inc <matthew.garrett@nebula.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/security.h>
+#include <linux/uaccess.h>
+
+static bool trusted_kernel;
+
+bool get_trusted_kernel(void)
+{
+	return trusted_kernel;
+}
+EXPORT_SYMBOL(get_trusted_kernel);
+
+int set_trusted_kernel(bool new_trusted_kernel)
+{
+	if (trusted_kernel == true && new_trusted_kernel == false)
+		return -EINVAL;
+
+	trusted_kernel = new_trusted_kernel;
+
+	return 0;
+}
+EXPORT_SYMBOL(set_trusted_kernel);
+
+static ssize_t trusted_kernel_read(struct file *filp, char __user *buf,
+				   size_t count, loff_t *ppos)
+{
+	char tmpbuf[2];
+	ssize_t length;
+
+	length = scnprintf(tmpbuf, sizeof(tmpbuf), "%d", trusted_kernel);
+	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
+}
+
+static ssize_t trusted_kernel_write(struct file *file, const char __user *buf,
+				    size_t count, loff_t *ppos)
+{
+	char *page = NULL;
+	ssize_t length;
+	int new_trusted_kernel;
+
+	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_trusted_kernel) != 1)
+		goto out;
+
+	length = set_trusted_kernel(!!new_trusted_kernel);
+	if (length)
+		goto out;
+
+	length = count;
+out:
+	free_page((unsigned long) page);
+	return length;
+}
+
+static const struct file_operations trusted_kernel_fops = {
+	.read	= trusted_kernel_read,
+	.write	= trusted_kernel_write,
+	.llseek	= generic_file_llseek,
+};
+
+static __init int setup_trusted_kernel(void)
+{
+	struct dentry *trusted_kernel_file;
+
+	trusted_kernel_file = securityfs_create_file("trusted_kernel",
+						     S_IWUSR | S_IRUGO,
+						     NULL, NULL,
+						     &trusted_kernel_fops);
+
+	if (IS_ERR(trusted_kernel_file))
+		return PTR_ERR(trusted_kernel_file);
+
+	return 0;
+}
+late_initcall(setup_trusted_kernel);
+
+static int __init enable_trusted_kernel(char *__str)
+{
+	trusted_kernel = true;
+	return 1;
+}
+__setup("trusted_kernel", enable_trusted_kernel);
-- 
1.8.5.3

Re: [PATCH 01/12] Add support for indicating that the booted kernel is externally trusted

[Kees Cook]

On Wed, Feb 26, 2014 at 12:11 PM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> Provide a boolean runtime configuration option for restricting userspace's
> ability to modify the running kernel. This can be used when some external
> validation of the kernel's state has been performed.
>
> Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
> ---
>  Documentation/kernel-parameters.txt       |   6 ++
>  Documentation/security/trusted_kernel.txt |  35 ++++++++++
>  include/linux/security.h                  |   8 +++
>  security/Kconfig                          |   9 +++
>  security/Makefile                         |   1 +
>  security/trusted_kernel.c                 | 111 ++++++++++++++++++++++++++++++
>  6 files changed, 170 insertions(+)
>  create mode 100644 Documentation/security/trusted_kernel.txt
>  create mode 100644 security/trusted_kernel.c
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 7116fda..d82ba9e 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3271,6 +3271,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>                         with respect to transparent hugepages.
>                         See Documentation/vm/transhuge.txt for more details.
>
> +       trusted_kernel  Indicate that the booted kernel has been verified to
> +                       be trustworthy and that userspace should be forbidden
> +                       from modifying it at runtime.
> +                       See Documentation/security/trusted_kernel.txt for more
> +                       details.
> +
>         tsc=            Disable clocksource stability checks for TSC.
>                         Format: <string>
>                         [x86] reliable: mark tsc clocksource as reliable, this
> diff --git a/Documentation/security/trusted_kernel.txt b/Documentation/security/trusted_kernel.txt
> new file mode 100644
> index 0000000..538d21d
> --- /dev/null
> +++ b/Documentation/security/trusted_kernel.txt
> @@ -0,0 +1,35 @@
> +Linux trusted kernel support
> +----------------------------
> +
> +Various mechanisms exist to ensure that a booted kernel is trusted by the
> +user or some external party (UEFI Secure Boot, Intel TXT, embedded platform
> +bootloaders). If userspace is able to modify the running kernel then this
> +trust can be subverted.
> +
> +The trusted kernel support modifies certain kernel interfaces such that
> +userspace is restricted from performing acts that would allow it to inject
> +untrusted code into the kernel. 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.
> +
> +The trusted kernel feature may be enabled in multiple ways:
> +
> +1) Platform-specific code may automatically enable it when it detects that
> +the system has been booted appropriately
> +
> +2) The user or bootloader may pass the "trusted_kernel" kernel parameter
> +
> +3) Userspace may write "1" to the /sys/kernel/security/trusted_kernel
> +node. This must be done sufficiently early in the boot process that
> +untrusted userspace has no opportunity to modify the kernel.
> +
> +Once enabled. trusted kernel support may not be disabled without rebooting
> +the system.
> +
> +Note that this is a mechanism for the kernel to determine whether or not
> +it is externally trusted. Untrusted userspace can enable this option even
> +if the kernel is not trusted, and therefore userspace should not use this
> +value as an indication of whether or not the kernel is trustworthy.
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 5623a7f..3415968 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -3091,6 +3091,14 @@ static inline void security_audit_rule_free(void *lsmrule)
>  #endif /* CONFIG_SECURITY */
>  #endif /* CONFIG_AUDIT */
>
> +#ifdef CONFIG_SECURITY_TRUSTED_KERNEL
> +extern bool get_trusted_kernel(void);
> +extern int set_trusted_kernel(bool new_trusted_kernel);
> +#else
> +static inline bool get_trusted_kernel(void) { return 0; }
> +static inline int set_trusted_kernel(bool new_trusted_kernel) { return 0; }

Should set_trusted_kernel return -ENOTSUPP when the config is off?

> +#endif /* CONFIG_TRUSTED_KERNEL */
> +
>  #ifdef CONFIG_SECURITYFS
>
>  extern struct dentry *securityfs_create_file(const char *name, umode_t mode,
> diff --git a/security/Kconfig b/security/Kconfig
> index beb86b5..c0462c9 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -70,6 +70,15 @@ config SECURITY_PATH
>           implement pathname based access controls.
>           If you are unsure how to answer this question, answer N.
>
> +config SECURITY_TRUSTED_KERNEL
> +        bool "Support for indicating that the kernel is trusted"
> +       depends on SECURITY
> +       help
> +         This enables support for adding a set of additional kernel security
> +         restrictions at runtime.
> +         See Documentation/security/trusted_kernel.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 a5918e0..72af305 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_TRUSTED_KERNEL)  += trusted_kernel.o
>  obj-$(CONFIG_SECURITY_SELINUX)         += selinux/built-in.o
>  obj-$(CONFIG_SECURITY_SMACK)           += smack/built-in.o
>  obj-$(CONFIG_AUDIT)                    += lsm_audit.o
> diff --git a/security/trusted_kernel.c b/security/trusted_kernel.c
> new file mode 100644
> index 0000000..2808113
> --- /dev/null
> +++ b/security/trusted_kernel.c
> @@ -0,0 +1,111 @@
> +/*
> + *  trusted_kernel.c - support for generic kernel lockdown
> + *
> + *  Copyright Nebula, Inc <matthew.garrett@nebula.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/security.h>
> +#include <linux/uaccess.h>
> +
> +static bool trusted_kernel;
> +
> +bool get_trusted_kernel(void)
> +{
> +       return trusted_kernel;
> +}
> +EXPORT_SYMBOL(get_trusted_kernel);
> +
> +int set_trusted_kernel(bool new_trusted_kernel)
> +{
> +       if (trusted_kernel == true && new_trusted_kernel == false)
> +               return -EINVAL;
> +
> +       trusted_kernel = new_trusted_kernel;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(set_trusted_kernel);
> +
> +static ssize_t trusted_kernel_read(struct file *filp, char __user *buf,
> +                                  size_t count, loff_t *ppos)
> +{
> +       char tmpbuf[2];
> +       ssize_t length;
> +
> +       length = scnprintf(tmpbuf, sizeof(tmpbuf), "%d", trusted_kernel);
> +       return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
> +}
> +
> +static ssize_t trusted_kernel_write(struct file *file, const char __user *buf,
> +                                   size_t count, loff_t *ppos)
> +{
> +       char *page = NULL;
> +       ssize_t length;
> +       int new_trusted_kernel;
> +
> +       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_trusted_kernel) != 1)
> +               goto out;
> +
> +       length = set_trusted_kernel(!!new_trusted_kernel);
> +       if (length)
> +               goto out;
> +
> +       length = count;
> +out:
> +       free_page((unsigned long) page);
> +       return length;
> +}
> +
> +static const struct file_operations trusted_kernel_fops = {
> +       .read   = trusted_kernel_read,
> +       .write  = trusted_kernel_write,
> +       .llseek = generic_file_llseek,
> +};
> +
> +static __init int setup_trusted_kernel(void)
> +{
> +       struct dentry *trusted_kernel_file;
> +
> +       trusted_kernel_file = securityfs_create_file("trusted_kernel",
> +                                                    S_IWUSR | S_IRUGO,
> +                                                    NULL, NULL,
> +                                                    &trusted_kernel_fops);
> +
> +       if (IS_ERR(trusted_kernel_file))
> +               return PTR_ERR(trusted_kernel_file);
> +
> +       return 0;
> +}
> +late_initcall(setup_trusted_kernel);
> +
> +static int __init enable_trusted_kernel(char *__str)
> +{
> +       trusted_kernel = true;
> +       return 1;
> +}
> +__setup("trusted_kernel", enable_trusted_kernel);
> --
> 1.8.5.3
>

Thanks for reposting this. I'm looking forward to adding
"trusted_kernel" to the Chrome OS kernel cmdline. :)

Please consider the series:

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 01/12] Add support for indicating that the booted kernel is externally trusted

[Kees Cook]

On Wed, Feb 26, 2014 at 12:11 PM, Matthew Garrett
<matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
> Provide a boolean runtime configuration option for restricting userspace's
> ability to modify the running kernel. This can be used when some external
> validation of the kernel's state has been performed.
>
> Signed-off-by: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
> ---
>  Documentation/kernel-parameters.txt       |   6 ++
>  Documentation/security/trusted_kernel.txt |  35 ++++++++++
>  include/linux/security.h                  |   8 +++
>  security/Kconfig                          |   9 +++
>  security/Makefile                         |   1 +
>  security/trusted_kernel.c                 | 111 ++++++++++++++++++++++++++++++
>  6 files changed, 170 insertions(+)
>  create mode 100644 Documentation/security/trusted_kernel.txt
>  create mode 100644 security/trusted_kernel.c
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 7116fda..d82ba9e 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3271,6 +3271,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>                         with respect to transparent hugepages.
>                         See Documentation/vm/transhuge.txt for more details.
>
> +       trusted_kernel  Indicate that the booted kernel has been verified to
> +                       be trustworthy and that userspace should be forbidden
> +                       from modifying it at runtime.
> +                       See Documentation/security/trusted_kernel.txt for more
> +                       details.
> +
>         tsc=            Disable clocksource stability checks for TSC.
>                         Format: <string>
>                         [x86] reliable: mark tsc clocksource as reliable, this
> diff --git a/Documentation/security/trusted_kernel.txt b/Documentation/security/trusted_kernel.txt
> new file mode 100644
> index 0000000..538d21d
> --- /dev/null
> +++ b/Documentation/security/trusted_kernel.txt
> @@ -0,0 +1,35 @@
> +Linux trusted kernel support
> +----------------------------
> +
> +Various mechanisms exist to ensure that a booted kernel is trusted by the
> +user or some external party (UEFI Secure Boot, Intel TXT, embedded platform
> +bootloaders). If userspace is able to modify the running kernel then this
> +trust can be subverted.
> +
> +The trusted kernel support modifies certain kernel interfaces such that
> +userspace is restricted from performing acts that would allow it to inject
> +untrusted code into the kernel. 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.
> +
> +The trusted kernel feature may be enabled in multiple ways:
> +
> +1) Platform-specific code may automatically enable it when it detects that
> +the system has been booted appropriately
> +
> +2) The user or bootloader may pass the "trusted_kernel" kernel parameter
> +
> +3) Userspace may write "1" to the /sys/kernel/security/trusted_kernel
> +node. This must be done sufficiently early in the boot process that
> +untrusted userspace has no opportunity to modify the kernel.
> +
> +Once enabled. trusted kernel support may not be disabled without rebooting
> +the system.
> +
> +Note that this is a mechanism for the kernel to determine whether or not
> +it is externally trusted. Untrusted userspace can enable this option even
> +if the kernel is not trusted, and therefore userspace should not use this
> +value as an indication of whether or not the kernel is trustworthy.
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 5623a7f..3415968 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -3091,6 +3091,14 @@ static inline void security_audit_rule_free(void *lsmrule)
>  #endif /* CONFIG_SECURITY */
>  #endif /* CONFIG_AUDIT */
>
> +#ifdef CONFIG_SECURITY_TRUSTED_KERNEL
> +extern bool get_trusted_kernel(void);
> +extern int set_trusted_kernel(bool new_trusted_kernel);
> +#else
> +static inline bool get_trusted_kernel(void) { return 0; }
> +static inline int set_trusted_kernel(bool new_trusted_kernel) { return 0; }

Should set_trusted_kernel return -ENOTSUPP when the config is off?

> +#endif /* CONFIG_TRUSTED_KERNEL */
> +
>  #ifdef CONFIG_SECURITYFS
>
>  extern struct dentry *securityfs_create_file(const char *name, umode_t mode,
> diff --git a/security/Kconfig b/security/Kconfig
> index beb86b5..c0462c9 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -70,6 +70,15 @@ config SECURITY_PATH
>           implement pathname based access controls.
>           If you are unsure how to answer this question, answer N.
>
> +config SECURITY_TRUSTED_KERNEL
> +        bool "Support for indicating that the kernel is trusted"
> +       depends on SECURITY
> +       help
> +         This enables support for adding a set of additional kernel security
> +         restrictions at runtime.
> +         See Documentation/security/trusted_kernel.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 a5918e0..72af305 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_TRUSTED_KERNEL)  += trusted_kernel.o
>  obj-$(CONFIG_SECURITY_SELINUX)         += selinux/built-in.o
>  obj-$(CONFIG_SECURITY_SMACK)           += smack/built-in.o
>  obj-$(CONFIG_AUDIT)                    += lsm_audit.o
> diff --git a/security/trusted_kernel.c b/security/trusted_kernel.c
> new file mode 100644
> index 0000000..2808113
> --- /dev/null
> +++ b/security/trusted_kernel.c
> @@ -0,0 +1,111 @@
> +/*
> + *  trusted_kernel.c - support for generic kernel lockdown
> + *
> + *  Copyright Nebula, Inc <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
> + *
> + *  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 bool trusted_kernel;
> +
> +bool get_trusted_kernel(void)
> +{
> +       return trusted_kernel;
> +}
> +EXPORT_SYMBOL(get_trusted_kernel);
> +
> +int set_trusted_kernel(bool new_trusted_kernel)
> +{
> +       if (trusted_kernel == true && new_trusted_kernel == false)
> +               return -EINVAL;
> +
> +       trusted_kernel = new_trusted_kernel;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(set_trusted_kernel);
> +
> +static ssize_t trusted_kernel_read(struct file *filp, char __user *buf,
> +                                  size_t count, loff_t *ppos)
> +{
> +       char tmpbuf[2];
> +       ssize_t length;
> +
> +       length = scnprintf(tmpbuf, sizeof(tmpbuf), "%d", trusted_kernel);
> +       return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
> +}
> +
> +static ssize_t trusted_kernel_write(struct file *file, const char __user *buf,
> +                                   size_t count, loff_t *ppos)
> +{
> +       char *page = NULL;
> +       ssize_t length;
> +       int new_trusted_kernel;
> +
> +       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_trusted_kernel) != 1)
> +               goto out;
> +
> +       length = set_trusted_kernel(!!new_trusted_kernel);
> +       if (length)
> +               goto out;
> +
> +       length = count;
> +out:
> +       free_page((unsigned long) page);
> +       return length;
> +}
> +
> +static const struct file_operations trusted_kernel_fops = {
> +       .read   = trusted_kernel_read,
> +       .write  = trusted_kernel_write,
> +       .llseek = generic_file_llseek,
> +};
> +
> +static __init int setup_trusted_kernel(void)
> +{
> +       struct dentry *trusted_kernel_file;
> +
> +       trusted_kernel_file = securityfs_create_file("trusted_kernel",
> +                                                    S_IWUSR | S_IRUGO,
> +                                                    NULL, NULL,
> +                                                    &trusted_kernel_fops);
> +
> +       if (IS_ERR(trusted_kernel_file))
> +               return PTR_ERR(trusted_kernel_file);
> +
> +       return 0;
> +}
> +late_initcall(setup_trusted_kernel);
> +
> +static int __init enable_trusted_kernel(char *__str)
> +{
> +       trusted_kernel = true;
> +       return 1;
> +}
> +__setup("trusted_kernel", enable_trusted_kernel);
> --
> 1.8.5.3
>

Thanks for reposting this. I'm looking forward to adding
"trusted_kernel" to the Chrome OS kernel cmdline. :)

Please consider the series:

Acked-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 01/12] Add support for indicating that the booted kernel is externally trusted

[Pavel Machek]

On Wed 2014-02-26 15:11:02, Matthew Garrett wrote:
> Provide a boolean runtime configuration option for restricting userspace's
> ability to modify the running kernel. This can be used when some external
> validation of the kernel's state has been performed.

I still don't like the idea, but...

> +Once enabled. trusted kernel support may not be disabled without rebooting
> +the system.

"enabled,"

> @@ -3091,6 +3091,14 @@ static inline void security_audit_rule_free(void *lsmrule)
>  #endif /* CONFIG_SECURITY */
>  #endif /* CONFIG_AUDIT */
>  
> +#ifdef CONFIG_SECURITY_TRUSTED_KERNEL
> +extern bool get_trusted_kernel(void);
> +extern int set_trusted_kernel(bool new_trusted_kernel);
> +#else
> +static inline bool get_trusted_kernel(void) { return 0; }
> +static inline int set_trusted_kernel(bool new_trusted_kernel) { return 0; }
> +#endif /* CONFIG_TRUSTED_KERNEL */

comment does not match ifdef. (And _SECURITY is really superfluous here, maybe shorter
option would be better?)

> +	length = -EINVAL;
> +	if (sscanf(page, "%d", &new_trusted_kernel) != 1)
> +		goto out;
> +
> +	length = set_trusted_kernel(!!new_trusted_kernel);

If someone writes 2 to the sysfs, it would be better to return einval than trying
to second guess him...

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 02/12] Enforce module signatures when trusted kernel is enabled

[Matthew Garrett]

If trusted_kernel is true, require that all modules have valid signatures.

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

diff --git a/kernel/module.c b/kernel/module.c
index d24fcf2..e0220db 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2448,7 +2448,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_trusted_kernel())
 		err = 0;
 
 	return err;
-- 
1.8.5.3

[PATCH 02/12] Enforce module signatures when trusted kernel is enabled

[Matthew Garrett]

If trusted_kernel is true, require that all modules have valid signatures.

Signed-off-by: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
---
 kernel/module.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/module.c b/kernel/module.c
index d24fcf2..e0220db 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2448,7 +2448,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_trusted_kernel())
 		err = 0;
 
 	return err;
-- 
1.8.5.3

[PATCH 03/12] PCI: Lock down BAR access when trusted_kernel is true

[Matthew Garrett]

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

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 drivers/pci/pci-sysfs.c | 9 +++++++++
 drivers/pci/proc.c      | 9 ++++++++-
 drivers/pci/syscall.c   | 3 ++-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 276ef9c..256f373 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -663,6 +663,9 @@ pci_write_config(struct file* filp, struct kobject *kobj,
 	loff_t init_off = off;
 	u8 *data = (u8*) buf;
 
+	if (get_trusted_kernel())
+		return -EPERM;
+
 	if (off > dev->cfg_size)
 		return 0;
 	if (off + count > dev->cfg_size) {
@@ -969,6 +972,9 @@ pci_mmap_resource(struct kobject *kobj, struct bin_attribute *attr,
 	resource_size_t start, end;
 	int i;
 
+	if (get_trusted_kernel())
+		return -EPERM;
+
 	for (i = 0; i < PCI_ROM_RESOURCE; i++)
 		if (res == &pdev->resource[i])
 			break;
@@ -1076,6 +1082,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_trusted_kernel())
+		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 46d1378..990b24b 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_trusted_kernel())
+		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_trusted_kernel())
+		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_trusted_kernel()))
 		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 24750a1..d962710 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_trusted_kernel()))
 		return -EPERM;
 
 	dev = pci_get_bus_and_slot(bus, dfn);
-- 
1.8.5.3

[PATCH 04/12] x86: Lock down IO port access when trusted_kernel is true

[Matthew Garrett]

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

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

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 4ddaf66..3121541 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_trusted_kernel())))
 		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_trusted_kernel()))
 			return -EPERM;
 	}
 	regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) | (level << 12);
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 92c5937..15331a8 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -27,6 +27,7 @@
 #include <linux/export.h>
 #include <linux/io.h>
 #include <linux/aio.h>
+#include <linux/security.h>
 
 #include <asm/uaccess.h>
 
@@ -544,6 +545,9 @@ static ssize_t read_port(struct file *file, char __user *buf,
 	unsigned long i = *ppos;
 	char __user *tmp = buf;
 
+	if (get_trusted_kernel())
+		return -EPERM;
+
 	if (!access_ok(VERIFY_WRITE, buf, count))
 		return -EFAULT;
 	while (count-- > 0 && i < 65536) {
@@ -562,6 +566,9 @@ static ssize_t write_port(struct file *file, const char __user *buf,
 	unsigned long i = *ppos;
 	const char __user *tmp = buf;
 
+	if (get_trusted_kernel())
+		return -EPERM;
+
 	if (!access_ok(VERIFY_READ, buf, count))
 		return -EFAULT;
 	while (count-- > 0 && i < 65536) {
-- 
1.8.5.3

[PATCH 05/12] Restrict /dev/mem and /dev/kmem when trusted_kernel is true

[Matthew Garrett]

Allowing users to write to address space provides mechanisms that may permit
modification of the kernel at runtime. Prevent this if trusted_kernel is
true.

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

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 15331a8..2cecab9 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -158,6 +158,9 @@ static ssize_t write_mem(struct file *file, const char __user *buf,
 	unsigned long copied;
 	void *ptr;
 
+	if (get_trusted_kernel())
+		return -EPERM;
+
 	if (!valid_phys_addr_range(p, count))
 		return -EFAULT;
 
@@ -496,6 +499,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_trusted_kernel())
+		return -EPERM;
+
 	if (p < (unsigned long) high_memory) {
 		unsigned long to_write = min_t(unsigned long, count,
 					       (unsigned long)high_memory - p);
-- 
1.8.5.3

[PATCH 06/12] acpi: Limit access to custom_method if trusted_kernel is true

[Matthew Garrett]

custom_method effectively allows arbitrary access to system memory, making
it possible for an attacker to modify the kernel at runtime. Prevent this
if trusted_kernel is true.

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

diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index c68e724..774bb7b 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -8,6 +8,7 @@
 #include <linux/uaccess.h>
 #include <linux/debugfs.h>
 #include <linux/acpi.h>
+#include <linux/security.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_trusted_kernel())
+		return -EPERM;
+
 	if (!(*ppos)) {
 		/* parse the table header to get the table length */
 		if (count <= sizeof(struct acpi_table_header))
-- 
1.8.5.3

[PATCH 07/12] acpi: Ignore acpi_rsdp kernel parameter when trusted_kernel is true.

[Matthew Garrett]

From: Josh Boyer <jwboyer@redhat.com>

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

Signed-off-by: Josh Boyer <jwboyer@redhat.com>
---
 drivers/acpi/osl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index fc1aa79..087a351 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -44,6 +44,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>
@@ -244,7 +245,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_trusted_kernel())
 		return acpi_rsdp;
 #endif
 
-- 
1.8.5.3

[PATCH 08/12] kexec: Disable at runtime if trusted_kernel is true

[Matthew Garrett]

kexec permits the loading and execution of arbitrary code in ring 0, which
permits the modification of the running kernel. Prevent this if
 trusted_kernel is true.

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

diff --git a/kernel/kexec.c b/kernel/kexec.c
index 60bafbe..281ddd0 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>
@@ -946,6 +947,9 @@ SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments,
 	if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
 		return -EPERM;
 
+	if (get_trusted_kernel())
+		return -EPERM;
+
 	/*
 	 * Verify we have a legal set of flags
 	 * This leaves us room for future extensions.
-- 
1.8.5.3

[PATCH 09/12] uswsusp: Disable when trusted_kernel is true

[Matthew Garrett]

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

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

diff --git a/kernel/power/user.c b/kernel/power/user.c
index 98d3575..2d7a5d3 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>
 
@@ -49,6 +50,9 @@ static int snapshot_open(struct inode *inode, struct file *filp)
 	struct snapshot_data *data;
 	int error;
 
+	if (get_trusted_kernel())
+		return -EPERM;
+
 	lock_system_sleep();
 
 	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
-- 
1.8.5.3

[PATCH 09/12] uswsusp: Disable when trusted_kernel is true

[Matthew Garrett]

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

Signed-off-by: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
---
 kernel/power/user.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/power/user.c b/kernel/power/user.c
index 98d3575..2d7a5d3 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>
 
@@ -49,6 +50,9 @@ static int snapshot_open(struct inode *inode, struct file *filp)
 	struct snapshot_data *data;
 	int error;
 
+	if (get_trusted_kernel())
+		return -EPERM;
+
 	lock_system_sleep();
 
 	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
-- 
1.8.5.3

Re: [PATCH 09/12] uswsusp: Disable when trusted_kernel is true

[Pavel Machek]

On Wed 2014-02-26 15:11:10, Matthew Garrett wrote:
> uswsusp allows a user process to dump and then restore kernel state, which
> makes it possible to modify the running kernel. Disable this if
>  trusted_kernel is true.
> 
> Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>

It can also enter S3 etc...

And dumping the kernel state should not be a problem, right? Only restoring is....

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 10/12] x86: Restrict MSR access when trusted_kernel is true

[Matthew Garrett]

Permitting write access to MSRs allows userspace to modify the running
kernel. Prevent this if trusted_kernel is true. Based on a patch by Kees
Cook.

Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 arch/x86/kernel/msr.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
index 05266b5..1bd5e92 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_trusted_kernel())
+		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_trusted_kernel()) {
+			err = -EPERM;
+			break;
+		}
 		if (copy_from_user(&regs, uregs, sizeof regs)) {
 			err = -EFAULT;
 			break;
-- 
1.8.5.3

[PATCH 11/12] asus-wmi: Restrict debugfs interface when trusted_kernel is true

[Matthew Garrett]

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

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

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index c5e082f..4c1000b 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -46,6 +46,7 @@
 #include <linux/platform_device.h>
 #include <linux/thermal.h>
 #include <linux/acpi.h>
+#include <linux/security.h>
 #include <acpi/video.h>
 
 #include "asus-wmi.h"
@@ -1595,6 +1596,9 @@ static int show_dsts(struct seq_file *m, void *data)
 	int err;
 	u32 retval = -1;
 
+	if (get_trusted_kernel())
+		return -EPERM;
+
 	err = asus_wmi_get_devstate(asus, asus->debug.dev_id, &retval);
 
 	if (err < 0)
@@ -1611,6 +1615,9 @@ static int show_devs(struct seq_file *m, void *data)
 	int err;
 	u32 retval = -1;
 
+	if (get_trusted_kernel())
+		return -EPERM;
+
 	err = asus_wmi_set_devstate(asus->debug.dev_id, asus->debug.ctrl_param,
 				    &retval);
 
@@ -1635,6 +1642,9 @@ static int show_call(struct seq_file *m, void *data)
 	union acpi_object *obj;
 	acpi_status status;
 
+	if (get_trusted_kernel())
+		return -EPERM;
+
 	status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID,
 				     1, asus->debug.method_id,
 				     &input, &output);
-- 
1.8.5.3

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

[Matthew Garrett]

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

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 Documentation/x86/zero-page.txt       |  2 ++
 arch/x86/Kconfig                      | 13 +++++++++++++
 arch/x86/boot/compressed/eboot.c      | 36 +++++++++++++++++++++++++++++++++++
 arch/x86/include/uapi/asm/bootparam.h |  3 ++-
 arch/x86/kernel/setup.c               |  6 ++++++
 5 files changed, 59 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 0af5250..3856dfb 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1585,6 +1585,19 @@ config EFI_STUB
 
 	  See Documentation/efi-stub.txt for more information.
 
+config EFI_SECURE_BOOT_TRUSTED_KERNEL
+        def_bool n
+	depends on SECURITY_TRUSTED_KERNEL
+	depends on EFI
+	prompt "Automatically set trusted_kernel with UEFI Secure Boot"
+	---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 trusted_kernel 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 a7677ba..4836551 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 */
 
@@ -421,6 +422,37 @@ void setup_graphics(struct boot_params *boot_params)
 }
 
 
+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
@@ -760,6 +792,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 225b098..90dbfb7 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -133,7 +133,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 06853e6..875258e 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>
@@ -1143,6 +1144,11 @@ void __init setup_arch(char **cmdline_p)
 
 	io_delay_init();
 
+#ifdef CONFIG_EFI_SECURE_BOOT_TRUSTED_KERNEL
+	if (boot_params.secure_boot)
+		set_trusted_kernel(true);
+#endif
+
 	/*
 	 * Parse the ACPI tables for possible boot-time SMP configuration.
 	 */
-- 
1.8.5.3

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

[Matthew Garrett]

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

Signed-off-by: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
---
 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               |  6 ++++++
 5 files changed, 59 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 0af5250..3856dfb 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1585,6 +1585,19 @@ config EFI_STUB
 
 	  See Documentation/efi-stub.txt for more information.
 
+config EFI_SECURE_BOOT_TRUSTED_KERNEL
+        def_bool n
+	depends on SECURITY_TRUSTED_KERNEL
+	depends on EFI
+	prompt "Automatically set trusted_kernel with UEFI Secure Boot"
+	---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 trusted_kernel 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 a7677ba..4836551 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 */
 
@@ -421,6 +422,37 @@ void setup_graphics(struct boot_params *boot_params)
 }
 
 
+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
@@ -760,6 +792,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 225b098..90dbfb7 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -133,7 +133,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 06853e6..875258e 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>
@@ -1143,6 +1144,11 @@ void __init setup_arch(char **cmdline_p)
 
 	io_delay_init();
 
+#ifdef CONFIG_EFI_SECURE_BOOT_TRUSTED_KERNEL
+	if (boot_params.secure_boot)
+		set_trusted_kernel(true);
+#endif
+
 	/*
 	 * Parse the ACPI tables for possible boot-time SMP configuration.
 	 */
-- 
1.8.5.3

Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

[One Thousand Gnomes]

On Wed, 26 Feb 2014 15:11:13 -0500
Matthew Garrett <matthew.garrett@nebula.com> wrote:

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

I think you have a load more cases to attempt to paper over before you
even pretend to achieve that goal. Firewire for example. Also it only
remotely begins to work if you also force CAP_SYS_RAWIO off globally as
you need to force off things like raw command issuing to various
controllers (especially as some of that code is written on the basis that
'its RAWIO, screw making it secure and doing all the checks we could
bother with'.

RAWIO also disables things like CPU msr access - which is also quite
adequate for subverting a kernel.

Another issue that needs addressing is firmware. Quite a few of our
request_firmware cases load device firmware which is not signed into DMA
capable hardware. Probably also worth checking what the
architectural guarantees on bogus microcode updates is. Maybe we need
firmware signing for such cases to match the mod signing ?

I'm trying to think what else. Possibly disabling it on Pentium-M with
the rep movs erratum (Y19) as it's quite possible to set up suitable
adjacent page sets in user space via the graphics.

Alan

Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

[One Thousand Gnomes]

On Wed, 26 Feb 2014 15:11:13 -0500
Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:

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

I think you have a load more cases to attempt to paper over before you
even pretend to achieve that goal. Firewire for example. Also it only
remotely begins to work if you also force CAP_SYS_RAWIO off globally as
you need to force off things like raw command issuing to various
controllers (especially as some of that code is written on the basis that
'its RAWIO, screw making it secure and doing all the checks we could
bother with'.

RAWIO also disables things like CPU msr access - which is also quite
adequate for subverting a kernel.

Another issue that needs addressing is firmware. Quite a few of our
request_firmware cases load device firmware which is not signed into DMA
capable hardware. Probably also worth checking what the
architectural guarantees on bogus microcode updates is. Maybe we need
firmware signing for such cases to match the mod signing ?

I'm trying to think what else. Possibly disabling it on Pentium-M with
the rep movs erratum (Y19) as it's quite possible to set up suitable
adjacent page sets in user space via the graphics.

Alan

Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

[H. Peter Anvin]

On 02/26/2014 02:41 PM, One Thousand Gnomes wrote:
> On Wed, 26 Feb 2014 15:11:13 -0500
> Matthew Garrett <matthew.garrett@nebula.com> wrote:
> 
>> 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.
> 
> I think you have a load more cases to attempt to paper over before you
> even pretend to achieve that goal. Firewire for example. Also it only
> remotely begins to work if you also force CAP_SYS_RAWIO off globally as
> you need to force off things like raw command issuing to various
> controllers (especially as some of that code is written on the basis that
> 'its RAWIO, screw making it secure and doing all the checks we could
> bother with'.
> 
> RAWIO also disables things like CPU msr access - which is also quite
> adequate for subverting a kernel.
> 
> Another issue that needs addressing is firmware. Quite a few of our
> request_firmware cases load device firmware which is not signed into DMA
> capable hardware. Probably also worth checking what the
> architectural guarantees on bogus microcode updates is. Maybe we need
> firmware signing for such cases to match the mod signing ?
> 
> I'm trying to think what else. Possibly disabling it on Pentium-M with
> the rep movs erratum (Y19) as it's quite possible to set up suitable
> adjacent page sets in user space via the graphics.
> 

I have been arguing for a long time that this should disable RAWIO.  The
argument was that apparently some SCSI controllers started requiring
RAWIO to do things like update firmware... which is arguably equally
problematic, but either way is (a) clearly wrong and (b) started a long,
never-ending discussion...

	-hpa

Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2011 bytes --]

On Wed, 2014-02-26 at 22:41 +0000, One Thousand Gnomes wrote:

> I think you have a load more cases to attempt to paper over before you
> even pretend to achieve that goal. Firewire for example. Also it only
> remotely begins to work if you also force CAP_SYS_RAWIO off globally as
> you need to force off things like raw command issuing to various
> controllers (especially as some of that code is written on the basis that
> 'its RAWIO, screw making it secure and doing all the checks we could
> bother with'.

Physical presence is required to do anything meaningful with firewire,
and UEFI secure boot isn't intended to protect against that. Which
controllers will trigger arbitrary DMA in response to raw commands?

> RAWIO also disables things like CPU msr access - which is also quite
> adequate for subverting a kernel.

Patch 7.

> Another issue that needs addressing is firmware. Quite a few of our
> request_firmware cases load device firmware which is not signed into DMA
> capable hardware. Probably also worth checking what the
> architectural guarantees on bogus microcode updates is. Maybe we need
> firmware signing for such cases to match the mod signing ?

Vendors keep telling me that they're validating firmware for new
hardware, and I keep tending not to believe them. Meh. The big problem
with firmware signatures is that we don't necessarily have the right to
distribute modified versions of the firmware, so we'd need detached
signature support. I'm certainly not against this.

> I'm trying to think what else. Possibly disabling it on Pentium-M with
> the rep movs erratum (Y19) as it's quite possible to set up suitable
> adjacent page sets in user space via the graphics.

Quirking this out when the hardware makes it impossible to provide any
guarantees seems reasonable.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

[Matthew Garrett]

On Wed, 2014-02-26 at 22:41 +0000, One Thousand Gnomes wrote:

> I think you have a load more cases to attempt to paper over before you
> even pretend to achieve that goal. Firewire for example. Also it only
> remotely begins to work if you also force CAP_SYS_RAWIO off globally as
> you need to force off things like raw command issuing to various
> controllers (especially as some of that code is written on the basis that
> 'its RAWIO, screw making it secure and doing all the checks we could
> bother with'.

Physical presence is required to do anything meaningful with firewire,
and UEFI secure boot isn't intended to protect against that. Which
controllers will trigger arbitrary DMA in response to raw commands?

> RAWIO also disables things like CPU msr access - which is also quite
> adequate for subverting a kernel.

Patch 7.

> Another issue that needs addressing is firmware. Quite a few of our
> request_firmware cases load device firmware which is not signed into DMA
> capable hardware. Probably also worth checking what the
> architectural guarantees on bogus microcode updates is. Maybe we need
> firmware signing for such cases to match the mod signing ?

Vendors keep telling me that they're validating firmware for new
hardware, and I keep tending not to believe them. Meh. The big problem
with firmware signatures is that we don't necessarily have the right to
distribute modified versions of the firmware, so we'd need detached
signature support. I'm certainly not against this.

> I'm trying to think what else. Possibly disabling it on Pentium-M with
> the rep movs erratum (Y19) as it's quite possible to set up suitable
> adjacent page sets in user space via the graphics.

Quirking this out when the hardware makes it impossible to provide any
guarantees seems reasonable.

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

Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

[Kees Cook]

On Wed, Feb 26, 2014 at 2:48 PM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Wed, 2014-02-26 at 22:41 +0000, One Thousand Gnomes wrote:
>> Another issue that needs addressing is firmware. Quite a few of our
>> request_firmware cases load device firmware which is not signed into DMA
>> capable hardware. Probably also worth checking what the
>> architectural guarantees on bogus microcode updates is. Maybe we need
>> firmware signing for such cases to match the mod signing ?
>
> Vendors keep telling me that they're validating firmware for new
> hardware, and I keep tending not to believe them. Meh. The big problem
> with firmware signatures is that we don't necessarily have the right to
> distribute modified versions of the firmware, so we'd need detached
> signature support. I'm certainly not against this.

I have been working on a patch series for this. It will have LSM hooks
for validating firmware origin (via fd) and contents (via blob),
similar to the changes I made for validating module origins. It just
need to finish testing, and I'll post the series. If you want to check
it out in its current state, it's here:

http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=fw-restrict

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode

[Kees Cook]

On Wed, Feb 26, 2014 at 2:48 PM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Wed, 2014-02-26 at 22:41 +0000, One Thousand Gnomes wrote:
>> Another issue that needs addressing is firmware. Quite a few of our
>> request_firmware cases load device firmware which is not signed into DMA
>> capable hardware. Probably also worth checking what the
>> architectural guarantees on bogus microcode updates is. Maybe we need
>> firmware signing for such cases to match the mod signing ?
>
> Vendors keep telling me that they're validating firmware for new
> hardware, and I keep tending not to believe them. Meh. The big problem
> with firmware signatures is that we don't necessarily have the right to
> distribute modified versions of the firmware, so we'd need detached
> signature support. I'm certainly not against this.

I have been working on a patch series for this. It will have LSM hooks
for validating firmware origin (via fd) and contents (via blob),
similar to the changes I made for validating module origins. It just
need to finish testing, and I'll post the series. If you want to check
it out in its current state, it's here:

http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=fw-restrict

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[Kees Cook]

On Wed, Feb 26, 2014 at 12:11 PM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> The conclusion we came to at Plumbers was that this patchset was basically
> fine but that Linus hated the name "securelevel" more than I hate pickled
> herring, so after thinking about this for a few months I've come up with
> "Trusted Kernel". This flag indicates that the kernel is, via some
> external mechanism, trusted and should behave that way. If firmware has
> some way to verify the kernel, it can pass that information on. If userspace
> has some way to verify the kernel, it can set the flag itself. However,
> userspace should not attempt to use the flag as a means to verify that the
> kernel was trusted - untrusted userspace could have set it on an untrusted
> kernel, but by the same metric an untrusted kernel could just set it itself.
>
> If people object to this name then I swear to god that I will open a poll
> on Phoronix to decide the next attempt and you will like that even less.

For the Chrome OS use-case, it might be better described as "untrusted
userspace", but that seems unfriendly. :) The "trusted kernel" name
seems fine to me.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> > kernel was trusted - untrusted userspace could have set it on an untrusted
> > kernel, but by the same metric an untrusted kernel could just set it itself.
> >
> > If people object to this name then I swear to god that I will open a poll
> > on Phoronix to decide the next attempt and you will like that even less.

Go on open the poll -  I dare you. But don't be shocked if it ends up
being called "Eric" or "Icanhazsigs" 8)

> For the Chrome OS use-case, it might be better described as "untrusted
> userspace", but that seems unfriendly. :) The "trusted kernel" name
> seems fine to me.

Trusted is rather misleading. It's not trusted, it's *measured*.

It's the same bits you had when you made it, and when you booted it
before. Whether you trust them is a different and quite unrelated
question. You may have reasons to do either.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> > kernel was trusted - untrusted userspace could have set it on an untrusted
> > kernel, but by the same metric an untrusted kernel could just set it itself.
> >
> > If people object to this name then I swear to god that I will open a poll
> > on Phoronix to decide the next attempt and you will like that even less.

Go on open the poll -  I dare you. But don't be shocked if it ends up
being called "Eric" or "Icanhazsigs" 8)

> For the Chrome OS use-case, it might be better described as "untrusted
> userspace", but that seems unfriendly. :) The "trusted kernel" name
> seems fine to me.

Trusted is rather misleading. It's not trusted, it's *measured*.

It's the same bits you had when you made it, and when you booted it
before. Whether you trust them is a different and quite unrelated
question. You may have reasons to do either.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[Alon Ziv]

One Thousand Gnomes <gnomes@...> writes:
> Trusted is rather misleading. It's not trusted, it's *measured*.
> 
> It's the same bits you had when you made it, and when you booted it
> before. Whether you trust them is a different and quite unrelated
> question. You may have reasons to do either.

I believe mjg's point is that the patch implement's the kernel's view
of its trust requirements, and is thus independent of whether it is
measured or not.

Frankly, from my experience, the term "trust" is one of the most
confusing ones in the security field... In general, unlike the intuitive
definition, "trust" is a _negative_ feature.
To say it differently: a "Trusted Kernel" is "a kernel that believes
someone trusts it". It does _not_ mean the kernel is actually
_trustworthy_. A "measured" kernel is one way for it to be trustworthy -
but you cannot trust the kernel to tell you if it is measured. (This is
a classic case of the Epimenides paradox: if the kernel is untrustworhy,
it will always claim to be trusted.)

-Alon

Re: Trusted kernel patchset for Secure Boot lockdown

[Florian Weimer]

* One Thousand Gnomes:

>> For the Chrome OS use-case, it might be better described as "untrusted
>> userspace", but that seems unfriendly. :) The "trusted kernel" name
>> seems fine to me.
>
> Trusted is rather misleading. It's not trusted, it's *measured*.

I don't think anyone is doing any measurement.  In particular, the
kernel does not know anything about the history of the boot process
and cannot provide any form of attestation.  This is why this feature
is not very useful to end users.

Re: Trusted kernel patchset for Secure Boot lockdown

[Florian Weimer]

* One Thousand Gnomes:

>> For the Chrome OS use-case, it might be better described as "untrusted
>> userspace", but that seems unfriendly. :) The "trusted kernel" name
>> seems fine to me.
>
> Trusted is rather misleading. It's not trusted, it's *measured*.

I don't think anyone is doing any measurement.  In particular, the
kernel does not know anything about the history of the boot process
and cannot provide any form of attestation.  This is why this feature
is not very useful to end users.

Re: Trusted kernel patchset for Secure Boot lockdown

[Josh Boyer]

On Wed, Feb 26, 2014 at 3:11 PM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> The conclusion we came to at Plumbers was that this patchset was basically
> fine but that Linus hated the name "securelevel" more than I hate pickled
> herring, so after thinking about this for a few months I've come up with
> "Trusted Kernel". This flag indicates that the kernel is, via some
> external mechanism, trusted and should behave that way. If firmware has
> some way to verify the kernel, it can pass that information on. If userspace
> has some way to verify the kernel, it can set the flag itself. However,
> userspace should not attempt to use the flag as a means to verify that the
> kernel was trusted - untrusted userspace could have set it on an untrusted
> kernel, but by the same metric an untrusted kernel could just set it itself.

FWIW, I've been running a kernel using this patchset in place of the
patchset Fedora typically carries for this purpose for a bit.  Things
appear to be working as expected and the protections remain the same.

It would be really nice to get this set of patches in so some of the
other patches that depend on them can start being pushed as well.

josh

Re: Trusted kernel patchset for Secure Boot lockdown

[Josh Boyer]

On Wed, Feb 26, 2014 at 3:11 PM, Matthew Garrett
<matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
> The conclusion we came to at Plumbers was that this patchset was basically
> fine but that Linus hated the name "securelevel" more than I hate pickled
> herring, so after thinking about this for a few months I've come up with
> "Trusted Kernel". This flag indicates that the kernel is, via some
> external mechanism, trusted and should behave that way. If firmware has
> some way to verify the kernel, it can pass that information on. If userspace
> has some way to verify the kernel, it can set the flag itself. However,
> userspace should not attempt to use the flag as a means to verify that the
> kernel was trusted - untrusted userspace could have set it on an untrusted
> kernel, but by the same metric an untrusted kernel could just set it itself.

FWIW, I've been running a kernel using this patchset in place of the
patchset Fedora typically carries for this purpose for a bit.  Things
appear to be working as expected and the protections remain the same.

It would be really nice to get this set of patches in so some of the
other patches that depend on them can start being pushed as well.

josh

Re: Trusted kernel patchset for Secure Boot lockdown

[Greg KH]

On Thu, Feb 27, 2014 at 01:04:34PM -0500, Josh Boyer wrote:
> On Wed, Feb 26, 2014 at 3:11 PM, Matthew Garrett
> <matthew.garrett@nebula.com> wrote:
> > The conclusion we came to at Plumbers was that this patchset was basically
> > fine but that Linus hated the name "securelevel" more than I hate pickled
> > herring, so after thinking about this for a few months I've come up with
> > "Trusted Kernel". This flag indicates that the kernel is, via some
> > external mechanism, trusted and should behave that way. If firmware has
> > some way to verify the kernel, it can pass that information on. If userspace
> > has some way to verify the kernel, it can set the flag itself. However,
> > userspace should not attempt to use the flag as a means to verify that the
> > kernel was trusted - untrusted userspace could have set it on an untrusted
> > kernel, but by the same metric an untrusted kernel could just set it itself.
> 
> FWIW, I've been running a kernel using this patchset in place of the
> patchset Fedora typically carries for this purpose for a bit.  Things
> appear to be working as expected and the protections remain the same.
> 
> It would be really nice to get this set of patches in so some of the
> other patches that depend on them can start being pushed as well.

What other patches depend on this series?  Why aren't they also in this
series?

thanks,

greg k-h

Re: Trusted kernel patchset for Secure Boot lockdown

[Josh Boyer]

On Thu, Feb 27, 2014 at 2:07 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Feb 27, 2014 at 01:04:34PM -0500, Josh Boyer wrote:
>> On Wed, Feb 26, 2014 at 3:11 PM, Matthew Garrett
>> <matthew.garrett@nebula.com> wrote:
>> > The conclusion we came to at Plumbers was that this patchset was basically
>> > fine but that Linus hated the name "securelevel" more than I hate pickled
>> > herring, so after thinking about this for a few months I've come up with
>> > "Trusted Kernel". This flag indicates that the kernel is, via some
>> > external mechanism, trusted and should behave that way. If firmware has
>> > some way to verify the kernel, it can pass that information on. If userspace
>> > has some way to verify the kernel, it can set the flag itself. However,
>> > userspace should not attempt to use the flag as a means to verify that the
>> > kernel was trusted - untrusted userspace could have set it on an untrusted
>> > kernel, but by the same metric an untrusted kernel could just set it itself.
>>
>> FWIW, I've been running a kernel using this patchset in place of the
>> patchset Fedora typically carries for this purpose for a bit.  Things
>> appear to be working as expected and the protections remain the same.
>>
>> It would be really nice to get this set of patches in so some of the
>> other patches that depend on them can start being pushed as well.
>
> What other patches depend on this series?  Why aren't they also in this
> series?

The patches we have to import certificates from the UEFI db and dbx
vars, and MokListRT and apply them to signed module verification.
Looking at them closely, there are pieces that could be sent now as
they are slightly orthogonal to what this patchset is doing, which is
probably why they aren't in this patchset to begin with.  I'll have to
figure out which of those actually depend on anything in Matthew's
series.

josh

Re: Trusted kernel patchset for Secure Boot lockdown

[Josh Boyer]

On Thu, Feb 27, 2014 at 2:07 PM, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> On Thu, Feb 27, 2014 at 01:04:34PM -0500, Josh Boyer wrote:
>> On Wed, Feb 26, 2014 at 3:11 PM, Matthew Garrett
>> <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
>> > The conclusion we came to at Plumbers was that this patchset was basically
>> > fine but that Linus hated the name "securelevel" more than I hate pickled
>> > herring, so after thinking about this for a few months I've come up with
>> > "Trusted Kernel". This flag indicates that the kernel is, via some
>> > external mechanism, trusted and should behave that way. If firmware has
>> > some way to verify the kernel, it can pass that information on. If userspace
>> > has some way to verify the kernel, it can set the flag itself. However,
>> > userspace should not attempt to use the flag as a means to verify that the
>> > kernel was trusted - untrusted userspace could have set it on an untrusted
>> > kernel, but by the same metric an untrusted kernel could just set it itself.
>>
>> FWIW, I've been running a kernel using this patchset in place of the
>> patchset Fedora typically carries for this purpose for a bit.  Things
>> appear to be working as expected and the protections remain the same.
>>
>> It would be really nice to get this set of patches in so some of the
>> other patches that depend on them can start being pushed as well.
>
> What other patches depend on this series?  Why aren't they also in this
> series?

The patches we have to import certificates from the UEFI db and dbx
vars, and MokListRT and apply them to signed module verification.
Looking at them closely, there are pieces that could be sent now as
they are slightly orthogonal to what this patchset is doing, which is
probably why they aren't in this patchset to begin with.  I'll have to
figure out which of those actually depend on anything in Matthew's
series.

josh

Re: Trusted kernel patchset for Secure Boot lockdown

[Josh Boyer]

On Thu, Feb 27, 2014 at 2:11 PM, Josh Boyer <jwboyer@fedoraproject.org> wrote:
> On Thu, Feb 27, 2014 at 2:07 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> On Thu, Feb 27, 2014 at 01:04:34PM -0500, Josh Boyer wrote:
>>> On Wed, Feb 26, 2014 at 3:11 PM, Matthew Garrett
>>> <matthew.garrett@nebula.com> wrote:
>>> > The conclusion we came to at Plumbers was that this patchset was basically
>>> > fine but that Linus hated the name "securelevel" more than I hate pickled
>>> > herring, so after thinking about this for a few months I've come up with
>>> > "Trusted Kernel". This flag indicates that the kernel is, via some
>>> > external mechanism, trusted and should behave that way. If firmware has
>>> > some way to verify the kernel, it can pass that information on. If userspace
>>> > has some way to verify the kernel, it can set the flag itself. However,
>>> > userspace should not attempt to use the flag as a means to verify that the
>>> > kernel was trusted - untrusted userspace could have set it on an untrusted
>>> > kernel, but by the same metric an untrusted kernel could just set it itself.
>>>
>>> FWIW, I've been running a kernel using this patchset in place of the
>>> patchset Fedora typically carries for this purpose for a bit.  Things
>>> appear to be working as expected and the protections remain the same.
>>>
>>> It would be really nice to get this set of patches in so some of the
>>> other patches that depend on them can start being pushed as well.
>>
>> What other patches depend on this series?  Why aren't they also in this
>> series?
>
> The patches we have to import certificates from the UEFI db and dbx
> vars, and MokListRT and apply them to signed module verification.
> Looking at them closely, there are pieces that could be sent now as
> they are slightly orthogonal to what this patchset is doing, which is
> probably why they aren't in this patchset to begin with.  I'll have to
> figure out which of those actually depend on anything in Matthew's
> series.

OK, I figured it out.  I have a patch that adds an EFI_SECURE_BOOT
x86_efi_facility bit, and that is used in the later patches where
applicable.  The patch that adds it depends on patch 12 in Matthew's
series.

There are a few patches that are mostly stand-alone and I'll try and
get those sent out soon.  They're a mix of things from David Howells
and myself, and should probably go through the security tree.

josh

Re: Trusted kernel patchset for Secure Boot lockdown

[James Morris]

On Thu, 27 Feb 2014, Josh Boyer wrote:

> On Wed, Feb 26, 2014 at 3:11 PM, Matthew Garrett
> <matthew.garrett@nebula.com> wrote:
> > The conclusion we came to at Plumbers was that this patchset was basically
> > fine but that Linus hated the name "securelevel" more than I hate pickled
> > herring, so after thinking about this for a few months I've come up with
> > "Trusted Kernel". This flag indicates that the kernel is, via some
> > external mechanism, trusted and should behave that way. If firmware has
> > some way to verify the kernel, it can pass that information on. If userspace
> > has some way to verify the kernel, it can set the flag itself. However,
> > userspace should not attempt to use the flag as a means to verify that the
> > kernel was trusted - untrusted userspace could have set it on an untrusted
> > kernel, but by the same metric an untrusted kernel could just set it itself.
> 
> FWIW, I've been running a kernel using this patchset in place of the
> patchset Fedora typically carries for this purpose for a bit.  Things
> appear to be working as expected and the protections remain the same.
> 
> It would be really nice to get this set of patches in so some of the
> other patches that depend on them can start being pushed as well.

Ok, which tree should take this?  I'm happy to, although most of it is 
outside security/ .



-- 
James Morris
<jmorris@namei.org>

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 540 bytes --]

On Fri, 2014-02-28 at 14:03 +1100, James Morris wrote:

> Ok, which tree should take this?  I'm happy to, although most of it is 
> outside security/ .

Security might make the most sense - I don't think any of the additional
restrictions (beyond kexec, and I think we've hashed that argument out
now) are terribly controversial.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-02-28 at 14:03 +1100, James Morris wrote:

> Ok, which tree should take this?  I'm happy to, although most of it is 
> outside security/ .

Security might make the most sense - I don't think any of the additional
restrictions (beyond kexec, and I think we've hashed that argument out
now) are terribly controversial.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 425 bytes --]

On Fri, 2014-02-28 at 14:03 +1100, James Morris wrote:

> Ok, which tree should take this?  I'm happy to, although most of it is 
> outside security/ .

Should I be looking for someone else to take them instead? :)

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-02-28 at 14:03 +1100, James Morris wrote:

> Ok, which tree should take this?  I'm happy to, although most of it is 
> outside security/ .

Should I be looking for someone else to take them instead? :)

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

Re: Trusted kernel patchset for Secure Boot lockdown

[Kees Cook]

On Wed, Mar 12, 2014 at 10:01 PM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Fri, 2014-02-28 at 14:03 +1100, James Morris wrote:
>
>> Ok, which tree should take this?  I'm happy to, although most of it is
>> outside security/ .
>
> Should I be looking for someone else to take them instead? :)

Andrew, is this series[1] something you'd be okay taking? It touches
many different areas, so you might be best for it.

-Kees

[1] https://lkml.org/lkml/2014/2/26/811

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[Kees Cook]

On Wed, Mar 12, 2014 at 10:01 PM, Matthew Garrett
<matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, 2014-02-28 at 14:03 +1100, James Morris wrote:
>
>> Ok, which tree should take this?  I'm happy to, although most of it is
>> outside security/ .
>
> Should I be looking for someone else to take them instead? :)

Andrew, is this series[1] something you'd be okay taking? It touches
many different areas, so you might be best for it.

-Kees

[1] https://lkml.org/lkml/2014/2/26/811

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[James Morris]

On Wed, 12 Mar 2014, Kees Cook wrote:

> On Wed, Mar 12, 2014 at 10:01 PM, Matthew Garrett
> <matthew.garrett@nebula.com> wrote:
> > On Fri, 2014-02-28 at 14:03 +1100, James Morris wrote:
> >
> >> Ok, which tree should take this?  I'm happy to, although most of it is
> >> outside security/ .
> >
> > Should I be looking for someone else to take them instead? :)
> 
> Andrew, is this series[1] something you'd be okay taking? It touches
> many different areas, so you might be best for it.

I'll take it, but there's unanswered review feedback (your response to the 
first question), and Alan raised some doubts about the patches which I'm 
not sure have been resolved.



-- 
James Morris
<jmorris@namei.org>

Re: Trusted kernel patchset for Secure Boot lockdown

[James Morris]

On Wed, 12 Mar 2014, Kees Cook wrote:

> On Wed, Mar 12, 2014 at 10:01 PM, Matthew Garrett
> <matthew.garrett@nebula.com> wrote:
> > On Fri, 2014-02-28 at 14:03 +1100, James Morris wrote:
> >
> >> Ok, which tree should take this?  I'm happy to, although most of it is
> >> outside security/ .
> >
> > Should I be looking for someone else to take them instead? :)
> 
> Andrew, is this series[1] something you'd be okay taking? It touches
> many different areas, so you might be best for it.

I'll take it, but there's unanswered review feedback (your response to the 
first question), and Alan raised some doubts about the patches which I'm 
not sure have been resolved.



-- 
James Morris
<jmorris@namei.org>

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Thu, 13 Mar 2014 20:33:06 +1100 (EST)
James Morris <jmorris@namei.org> wrote:

> On Wed, 12 Mar 2014, Kees Cook wrote:
> 
> > On Wed, Mar 12, 2014 at 10:01 PM, Matthew Garrett
> > <matthew.garrett@nebula.com> wrote:
> > > On Fri, 2014-02-28 at 14:03 +1100, James Morris wrote:
> > >
> > >> Ok, which tree should take this?  I'm happy to, although most of it is
> > >> outside security/ .
> > >
> > > Should I be looking for someone else to take them instead? :)
> > 
> > Andrew, is this series[1] something you'd be okay taking? It touches
> > many different areas, so you might be best for it.
> 
> I'll take it, but there's unanswered review feedback (your response to the 
> first question), and Alan raised some doubts about the patches which I'm 
> not sure have been resolved.

I have a series of doubts about their completeness which didn't get any
answer at all, and one on the misleading use of the term "secure" as
opposed to "measured" 8)

I don't think it's reasonable to have a policy of refusing them until
they cover all cases. It's not like it can be dropped into staging and
refined.

So other than the usual moan about people naming things "security" being
like putting "i-" and "e-" on the front of stuff to make it sound cool
when it isn't what it says I'm fine 8)

I would prefer it did the revocation of CAP_SYS_RAWIO or at least
documented the absolute requirement.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Thu, 13 Mar 2014 20:33:06 +1100 (EST)
James Morris <jmorris@namei.org> wrote:

> On Wed, 12 Mar 2014, Kees Cook wrote:
> 
> > On Wed, Mar 12, 2014 at 10:01 PM, Matthew Garrett
> > <matthew.garrett@nebula.com> wrote:
> > > On Fri, 2014-02-28 at 14:03 +1100, James Morris wrote:
> > >
> > >> Ok, which tree should take this?  I'm happy to, although most of it is
> > >> outside security/ .
> > >
> > > Should I be looking for someone else to take them instead? :)
> > 
> > Andrew, is this series[1] something you'd be okay taking? It touches
> > many different areas, so you might be best for it.
> 
> I'll take it, but there's unanswered review feedback (your response to the 
> first question), and Alan raised some doubts about the patches which I'm 
> not sure have been resolved.

I have a series of doubts about their completeness which didn't get any
answer at all, and one on the misleading use of the term "secure" as
opposed to "measured" 8)

I don't think it's reasonable to have a policy of refusing them until
they cover all cases. It's not like it can be dropped into staging and
refined.

So other than the usual moan about people naming things "security" being
like putting "i-" and "e-" on the front of stuff to make it sound cool
when it isn't what it says I'm fine 8)

I would prefer it did the revocation of CAP_SYS_RAWIO or at least
documented the absolute requirement.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[H. Peter Anvin]

On 03/13/2014 03:12 AM, One Thousand Gnomes wrote:
> 
> I would prefer it did the revocation of CAP_SYS_RAWIO or at least
> documented the absolute requirement.
> 

Seconded.  This has been my opinion, raised over and over and over again.

	-hpa

Re: Trusted kernel patchset for Secure Boot lockdown

[H. Peter Anvin]

On 03/13/2014 03:12 AM, One Thousand Gnomes wrote:
> 
> I would prefer it did the revocation of CAP_SYS_RAWIO or at least
> documented the absolute requirement.
> 

Seconded.  This has been my opinion, raised over and over and over again.

	-hpa

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1688 bytes --]

On Thu, 2014-03-13 at 20:33 +1100, James Morris wrote:

> I'll take it, but there's unanswered review feedback (your response to the 
> first question), and Alan raised some doubts about the patches which I'm 
> not sure have been resolved.

The remaining opens seem to be CAP_SYS_RAWIO and firmware signing?
Ironically, disabling CAP_SYS_RAWIO disables firmware loading…

The problem with CAP_SYS_RAWIO is that its semantics were never
sufficiently well documented, and as a result it's a mixture of "This is
incredibly dangerous" and "We replaced a check for uid 0 with whichever
capability seemed to have the most appropriate name". I've gone through
all the uses of CAP_SYS_RAWIO and added additional checks to the generic
ones that seem appropriate. There's a couple of old drivers that use it
to gate access to features that potentially allow arbitrary DMA and it
might be worth cleaning those up, but the only general case I haven't
modified is the ability to send arbitrary SCSI commands from userspace.
My understanding is that endpoints aren't going to be able to DMA to
arbitrary addresses, so that doesn't seem like a problem.

On the other hand, disabling CAP_SYS_RAWIO *definitely* breaks expected
functionality - firmware loading and the fibmap ioctl are probably the
most obvious. And changing the use of CAP_SYS_RAWIO potentially breaks
userspace expectations, so we're kind of stuck there.

As for signed firmware, I'm looking forward to Kees' work on that.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Thu, 2014-03-13 at 20:33 +1100, James Morris wrote:

> I'll take it, but there's unanswered review feedback (your response to the 
> first question), and Alan raised some doubts about the patches which I'm 
> not sure have been resolved.

The remaining opens seem to be CAP_SYS_RAWIO and firmware signing?
Ironically, disabling CAP_SYS_RAWIO disables firmware loading…

The problem with CAP_SYS_RAWIO is that its semantics were never
sufficiently well documented, and as a result it's a mixture of "This is
incredibly dangerous" and "We replaced a check for uid 0 with whichever
capability seemed to have the most appropriate name". I've gone through
all the uses of CAP_SYS_RAWIO and added additional checks to the generic
ones that seem appropriate. There's a couple of old drivers that use it
to gate access to features that potentially allow arbitrary DMA and it
might be worth cleaning those up, but the only general case I haven't
modified is the ability to send arbitrary SCSI commands from userspace.
My understanding is that endpoints aren't going to be able to DMA to
arbitrary addresses, so that doesn't seem like a problem.

On the other hand, disabling CAP_SYS_RAWIO *definitely* breaks expected
functionality - firmware loading and the fibmap ioctl are probably the
most obvious. And changing the use of CAP_SYS_RAWIO potentially breaks
userspace expectations, so we're kind of stuck there.

As for signed firmware, I'm looking forward to Kees' work on that.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Thu, 13 Mar 2014 15:59:24 +0000
Matthew Garrett <matthew.garrett@nebula.com> wrote:

> On Thu, 2014-03-13 at 20:33 +1100, James Morris wrote:
> 
> > I'll take it, but there's unanswered review feedback (your response to the 
> > first question), and Alan raised some doubts about the patches which I'm 
> > not sure have been resolved.
> 
> The remaining opens seem to be CAP_SYS_RAWIO and firmware signing?
> Ironically, disabling CAP_SYS_RAWIO disables firmware loading…
> 
> The problem with CAP_SYS_RAWIO is that its semantics were never
> sufficiently well documented, and as a result it's a mixture of "This is
> incredibly dangerous" and "We replaced a check for uid 0 with whichever
> capability seemed to have the most appropriate name". I've gone through
> all the uses of CAP_SYS_RAWIO and added additional checks to the generic
> ones that seem appropriate. There's a couple of old drivers that use it
> to gate access to features that potentially allow arbitrary DMA and it
> might be worth cleaning those up, but the only general case I haven't
> modified is the ability to send arbitrary SCSI commands from userspace.
> My understanding is that endpoints aren't going to be able to DMA to
> arbitrary addresses, so that doesn't seem like a problem.
> 
> On the other hand, disabling CAP_SYS_RAWIO *definitely* breaks expected
> functionality - firmware loading and the fibmap ioctl are probably the
> most obvious. And changing the use of CAP_SYS_RAWIO potentially breaks
> userspace expectations, so we're kind of stuck there.

If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
trivially and in a fashion well known and documented.

So if that isn't sufficient then we need to sort CAP_foo out first.

You've missed a few others too - mem= (especially with exactmap) for
example.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Thu, 13 Mar 2014 15:59:24 +0000
Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:

> On Thu, 2014-03-13 at 20:33 +1100, James Morris wrote:
> 
> > I'll take it, but there's unanswered review feedback (your response to the 
> > first question), and Alan raised some doubts about the patches which I'm 
> > not sure have been resolved.
> 
> The remaining opens seem to be CAP_SYS_RAWIO and firmware signing?
> Ironically, disabling CAP_SYS_RAWIO disables firmware loading…
> 
> The problem with CAP_SYS_RAWIO is that its semantics were never
> sufficiently well documented, and as a result it's a mixture of "This is
> incredibly dangerous" and "We replaced a check for uid 0 with whichever
> capability seemed to have the most appropriate name". I've gone through
> all the uses of CAP_SYS_RAWIO and added additional checks to the generic
> ones that seem appropriate. There's a couple of old drivers that use it
> to gate access to features that potentially allow arbitrary DMA and it
> might be worth cleaning those up, but the only general case I haven't
> modified is the ability to send arbitrary SCSI commands from userspace.
> My understanding is that endpoints aren't going to be able to DMA to
> arbitrary addresses, so that doesn't seem like a problem.
> 
> On the other hand, disabling CAP_SYS_RAWIO *definitely* breaks expected
> functionality - firmware loading and the fibmap ioctl are probably the
> most obvious. And changing the use of CAP_SYS_RAWIO potentially breaks
> userspace expectations, so we're kind of stuck there.

If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
trivially and in a fashion well known and documented.

So if that isn't sufficient then we need to sort CAP_foo out first.

You've missed a few others too - mem= (especially with exactmap) for
example.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[H. Peter Anvin]

On 03/13/2014 02:24 PM, One Thousand Gnomes wrote:
> 
> If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
> trivially and in a fashion well known and documented.
> 

... and once we eliminate CAP_SYS_RAWIO a bunch of the patches become
redundant.

	-hpa

Re: Trusted kernel patchset for Secure Boot lockdown

[H. Peter Anvin]

On 03/13/2014 02:24 PM, One Thousand Gnomes wrote:
> 
> If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
> trivially and in a fashion well known and documented.
> 

... and once we eliminate CAP_SYS_RAWIO a bunch of the patches become
redundant.

	-hpa

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 717 bytes --]

On Thu, 2014-03-13 at 14:28 -0700, H. Peter Anvin wrote:
> On 03/13/2014 02:24 PM, One Thousand Gnomes wrote:
> > 
> > If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
> > trivially and in a fashion well known and documented.
> > 
> 
> ... and once we eliminate CAP_SYS_RAWIO a bunch of the patches become
> redundant.

We can only drop CAP_SYS_RAWIO if we change a bunch of the existing
CAP_SYS_RAWIO checks to something else. How do we do that without
breaking existing userspace?

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Thu, 2014-03-13 at 14:28 -0700, H. Peter Anvin wrote:
> On 03/13/2014 02:24 PM, One Thousand Gnomes wrote:
> > 
> > If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
> > trivially and in a fashion well known and documented.
> > 
> 
> ... and once we eliminate CAP_SYS_RAWIO a bunch of the patches become
> redundant.

We can only drop CAP_SYS_RAWIO if we change a bunch of the existing
CAP_SYS_RAWIO checks to something else. How do we do that without
breaking existing userspace?

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

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 666 bytes --]

On Thu, 2014-03-13 at 21:24 +0000, One Thousand Gnomes wrote:

> If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
> trivially and in a fashion well known and documented.

How?

> You've missed a few others too - mem= (especially with exactmap) for
> example.

/dev/mem access is restricted, so what would this buy you? The potential
to have the kernel hand over a region belonging to hardware in response
to a userspace allocation?

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Thu, 2014-03-13 at 21:24 +0000, One Thousand Gnomes wrote:

> If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
> trivially and in a fashion well known and documented.

How?

> You've missed a few others too - mem= (especially with exactmap) for
> example.

/dev/mem access is restricted, so what would this buy you? The potential
to have the kernel hand over a region belonging to hardware in response
to a userspace allocation?

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

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Thu, 13 Mar 2014 21:30:48 +0000
Matthew Garrett <matthew.garrett@nebula.com> wrote:

> On Thu, 2014-03-13 at 21:24 +0000, One Thousand Gnomes wrote:
> 
> > If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
> > trivially and in a fashion well known and documented.
> 
> How?

You want a list... there are load of places all over the kernel that have
assumptions that RAWIO = safe from the boringly mundane like MSR access
to the more obscure driver "this is RAWIO trust the user" cases of which
there are plenty.

If you are going to do something "secure" then do it *right* otherwise
you are not actually implementing anything, just making fake security
noises.

SELinux required user space changes, fiddle with CAP_SYS_RAWIO will need
userspace changes IFF you want to run 'secure' mode. That I think is
acceptable.

You can even avoid the userspace issues with a small amount of checking.
If you don't want to touch capability sets then make the default
behaviour for capable(x) in fact be

	capable(x & ~secure_forbidden)

for a measured kernel and add a 

	capable_always()

for the cases you want to not break.

As for mem= and exactmap, it has nothing to do with /dev/mem and
everything to do with giving the kernel a memory map where some of the
space it thinks is RAM is in fact devices, rom, space etc. If the kernel
is given a false memory map it will misbehave. Exploitably - well given
the kind of things people have achieved in the past - quite possibly.

If you are not prepared to do the job right, then I don't think it
belongs upstream. Let's do it right, and if we have to tweak a few bits
of userspace to make them work in measured mode (but without breaking
anything in normal modes) then it's worth doing the job properly.

I don't think we need to break any userspace for "normal" mode to do
this. Userspace in measured mode is going to change anyway. It already
has just for things like module signing.

(As an aside you may also then want to think about whether you allow
measured userspace elements that secure_forbidden is considered to be 0
for so you can sign userspace apps that are allowed to do RAWIO)

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Thu, 13 Mar 2014 21:30:48 +0000
Matthew Garrett <matthew.garrett@nebula.com> wrote:

> On Thu, 2014-03-13 at 21:24 +0000, One Thousand Gnomes wrote:
> 
> > If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
> > trivially and in a fashion well known and documented.
> 
> How?

You want a list... there are load of places all over the kernel that have
assumptions that RAWIO = safe from the boringly mundane like MSR access
to the more obscure driver "this is RAWIO trust the user" cases of which
there are plenty.

If you are going to do something "secure" then do it *right* otherwise
you are not actually implementing anything, just making fake security
noises.

SELinux required user space changes, fiddle with CAP_SYS_RAWIO will need
userspace changes IFF you want to run 'secure' mode. That I think is
acceptable.

You can even avoid the userspace issues with a small amount of checking.
If you don't want to touch capability sets then make the default
behaviour for capable(x) in fact be

	capable(x & ~secure_forbidden)

for a measured kernel and add a 

	capable_always()

for the cases you want to not break.

As for mem= and exactmap, it has nothing to do with /dev/mem and
everything to do with giving the kernel a memory map where some of the
space it thinks is RAM is in fact devices, rom, space etc. If the kernel
is given a false memory map it will misbehave. Exploitably - well given
the kind of things people have achieved in the past - quite possibly.

If you are not prepared to do the job right, then I don't think it
belongs upstream. Let's do it right, and if we have to tweak a few bits
of userspace to make them work in measured mode (but without breaking
anything in normal modes) then it's worth doing the job properly.

I don't think we need to break any userspace for "normal" mode to do
this. Userspace in measured mode is going to change anyway. It already
has just for things like module signing.

(As an aside you may also then want to think about whether you allow
measured userspace elements that secure_forbidden is considered to be 0
for so you can sign userspace apps that are allowed to do RAWIO)

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4264 bytes --]

On Thu, 2014-03-13 at 23:21 +0000, One Thousand Gnomes wrote:
> On Thu, 13 Mar 2014 21:30:48 +0000
> Matthew Garrett <matthew.garrett@nebula.com> wrote:
> 
> > On Thu, 2014-03-13 at 21:24 +0000, One Thousand Gnomes wrote:
> > 
> > > If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
> > > trivially and in a fashion well known and documented.
> > 
> > How?
> 
> You want a list... there are load of places all over the kernel that have
> assumptions that RAWIO = safe from the boringly mundane like MSR access
> to the more obscure driver "this is RAWIO trust the user" cases of which
> there are plenty.

Have you actually looked at these patches? I've looked at every case of
RAWIO in the kernel. For cases that are hardware specific and tied to
fairly old hardware, I've ignored them. For cases which provide an
obvious mechanism for exploitation, I've added an additional check. For
cases where I can't see a reasonable mechanism for executing arbitrary
code in the kernel, I've done nothing.

If you have specific examples of processes with CAP_SYS_RAWIO being able
to execute arbitrary code in the kernel even with this patchset applied,
please, give them.

>You can even avoid the userspace issues with a small amount of
>checking. If you don't want to touch capability sets then make the
>default behaviour for capable(x) in fact be
>
>        capable(x & ~secure_forbidden)
>
>for a measured kernel and add a 
>
>        capable_always()
>
>for the cases you want to not break.

We could do that, but now the behaviour of the patchset is far less
obvious. capable(CAP_SYS_RAWIO) now means something different to every
other use of capable(), and we still need get_trusted_kernel() calls for
cases where the checks have nothing to do with processes and so
capabilities can't be used. It still involves auditing every use of
CAP_SYS_RAWIO. In fact, in some cases we need to *add* CAP_SYS_RAWIO
checks - which, again, breaks userspace.

> As for mem= and exactmap, it has nothing to do with /dev/mem and
> everything to do with giving the kernel a memory map where some of the
> space it thinks is RAM is in fact devices, rom, space etc. If the kernel
> is given a false memory map it will misbehave. Exploitably - well given
> the kind of things people have achieved in the past - quite possibly.

Sure. That's a worthwhile thing to fix, and it's something that dropping
CAP_SYS_RAWIO would do nothing to help you with.

> If you are not prepared to do the job right, then I don't think it
> belongs upstream. Let's do it right, and if we have to tweak a few bits
> of userspace to make them work in measured mode (but without breaking
> anything in normal modes) then it's worth doing the job properly.

We can do this without unnecessarily breaking any userspace. We just
can't do it by fiddling with capabilities.

> I don't think we need to break any userspace for "normal" mode to do
> this. Userspace in measured mode is going to change anyway. It already
> has just for things like module signing.

This has been discussed at length. Nobody who's actually spent time
working on the problem wants to use capabilities. CAP_SYS_RAWIO is not
semantically identical to the trusted kernel bit. Trying to make them
semantically identical will break existing userspace.

> (As an aside you may also then want to think about whether you allow
> measured userspace elements that secure_forbidden is considered to be 0
> for so you can sign userspace apps that are allowed to do RAWIO)

I'd be amazed if any of the applications that need RAWIO have had any
kind of meaningful security audit, with the possible exception of X (and
then we'd need to add support for signed X modules and sign all the
DDXes and seriously just no). I've no objection to someone doing that
work (and Vivek did a pile of it when looking at implementing kexec via
signed userspace), but I don't see any real use cases - pretty much
everyone using bits of RAWIO that are gated in the trusted kernel case
should be using a real kernel interface instead.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Thu, 2014-03-13 at 23:21 +0000, One Thousand Gnomes wrote:
> On Thu, 13 Mar 2014 21:30:48 +0000
> Matthew Garrett <matthew.garrett@nebula.com> wrote:
> 
> > On Thu, 2014-03-13 at 21:24 +0000, One Thousand Gnomes wrote:
> > 
> > > If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
> > > trivially and in a fashion well known and documented.
> > 
> > How?
> 
> You want a list... there are load of places all over the kernel that have
> assumptions that RAWIO = safe from the boringly mundane like MSR access
> to the more obscure driver "this is RAWIO trust the user" cases of which
> there are plenty.

Have you actually looked at these patches? I've looked at every case of
RAWIO in the kernel. For cases that are hardware specific and tied to
fairly old hardware, I've ignored them. For cases which provide an
obvious mechanism for exploitation, I've added an additional check. For
cases where I can't see a reasonable mechanism for executing arbitrary
code in the kernel, I've done nothing.

If you have specific examples of processes with CAP_SYS_RAWIO being able
to execute arbitrary code in the kernel even with this patchset applied,
please, give them.

>You can even avoid the userspace issues with a small amount of
>checking. If you don't want to touch capability sets then make the
>default behaviour for capable(x) in fact be
>
>        capable(x & ~secure_forbidden)
>
>for a measured kernel and add a 
>
>        capable_always()
>
>for the cases you want to not break.

We could do that, but now the behaviour of the patchset is far less
obvious. capable(CAP_SYS_RAWIO) now means something different to every
other use of capable(), and we still need get_trusted_kernel() calls for
cases where the checks have nothing to do with processes and so
capabilities can't be used. It still involves auditing every use of
CAP_SYS_RAWIO. In fact, in some cases we need to *add* CAP_SYS_RAWIO
checks - which, again, breaks userspace.

> As for mem= and exactmap, it has nothing to do with /dev/mem and
> everything to do with giving the kernel a memory map where some of the
> space it thinks is RAM is in fact devices, rom, space etc. If the kernel
> is given a false memory map it will misbehave. Exploitably - well given
> the kind of things people have achieved in the past - quite possibly.

Sure. That's a worthwhile thing to fix, and it's something that dropping
CAP_SYS_RAWIO would do nothing to help you with.

> If you are not prepared to do the job right, then I don't think it
> belongs upstream. Let's do it right, and if we have to tweak a few bits
> of userspace to make them work in measured mode (but without breaking
> anything in normal modes) then it's worth doing the job properly.

We can do this without unnecessarily breaking any userspace. We just
can't do it by fiddling with capabilities.

> I don't think we need to break any userspace for "normal" mode to do
> this. Userspace in measured mode is going to change anyway. It already
> has just for things like module signing.

This has been discussed at length. Nobody who's actually spent time
working on the problem wants to use capabilities. CAP_SYS_RAWIO is not
semantically identical to the trusted kernel bit. Trying to make them
semantically identical will break existing userspace.

> (As an aside you may also then want to think about whether you allow
> measured userspace elements that secure_forbidden is considered to be 0
> for so you can sign userspace apps that are allowed to do RAWIO)

I'd be amazed if any of the applications that need RAWIO have had any
kind of meaningful security audit, with the possible exception of X (and
then we'd need to add support for signed X modules and sign all the
DDXes and seriously just no). I've no objection to someone doing that
work (and Vivek did a pile of it when looking at implementing kexec via
signed userspace), but I don't see any real use cases - pretty much
everyone using bits of RAWIO that are gated in the trusted kernel case
should be using a real kernel interface instead.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> Have you actually looked at these patches? I've looked at every case of
> RAWIO in the kernel. For cases that are hardware specific and tied to
> fairly old hardware, I've ignored them. For cases which provide an

Yes I have - and it's not exactly localised: it modifies stuff all over
the tree when it shouldn't need to. It's a security policy. If it leaks
policy all over the kernel then the implementation model is *wrong*.

> >default behaviour for capable(x) in fact be
> >
> >        capable(x & ~secure_forbidden)
> >
> >for a measured kernel and add a 
> >
> >        capable_always()
> >
> >for the cases you want to not break.
> 
> We could do that, but now the behaviour of the patchset is far less
> obvious. capable(CAP_SYS_RAWIO) now means something different to every
> other use of capable(), and we still need get_trusted_kernel() calls for
> cases where the checks have nothing to do with processes and so
> capabilities can't be used. 

You don't need get_measured_kernel() calls because thats hidden within
capable/capable_always. And if the interaction is a complicated as you
imply that again says to me the model is wrong. Complicated multi-way
security interactions lead to complicated exploits leveraging the
disconnects.

But see further down...

> > As for mem= and exactmap, it has nothing to do with /dev/mem and
> > everything to do with giving the kernel a memory map where some of the
> > space it thinks is RAM is in fact devices, rom, space etc. If the kernel
> > is given a false memory map it will misbehave. Exploitably - well given
> > the kind of things people have achieved in the past - quite possibly.
> 
> Sure. That's a worthwhile thing to fix, and it's something that dropping
> CAP_SYS_RAWIO would do nothing to help you with.

I didn't mention it with regards to CAP_SYS_RAWIO, just as a general
example of stuff not dealt with. Again that one really suggests that the
right approach is to whitelist allowed boot options not blacklist bad
ones. It's the usual story with security, people fight bitterly not to do
whitelisting, then spend two years and more effort fixing holes than
doing whitelisting then eventually have to whitelist anyway. 

(or maybe I'm just bitter and cynical)
 
> We can do this without unnecessarily breaking any userspace. We just
> can't do it by fiddling with capabilities.

It should still be a security model nto spreading measured_kernel()
checks about. Now if that means

	capable(CAP_SYS_RAWIO)

should become security-> interface hooks that pass something meaningful
to the security layer then I'd rather we did the job properly in the first
place and put the policy in one spot not all over the kernel.

The question then becomes what do we need to pass beyond "CAP_SYS_RAWIO"
so the policy can be in the right place - even for example imposed by
SELinux rules.
	

> > I don't think we need to break any userspace for "normal" mode to do
> > this. Userspace in measured mode is going to change anyway. It already
> > has just for things like module signing.
> 
> This has been discussed at length. Nobody who's actually spent time
> working on the problem wants to use capabilities. CAP_SYS_RAWIO is not
> semantically identical to the trusted kernel bit. Trying to make them
> semantically identical will break existing userspace.

I never said it was. I said that getting rid of CAP_SYS_RAWIO and then
dealing with *just* the exceptions to this is a lot cleaner, and likely
to be more secure.

Right now the 'measured' patches spew all over the place. That shouldn't
be needed.

In addition several of the cases that you point out could be fixed by
replacing the CAP_SYS_RAWIO using stuff with a proper interface should
probably just be *fixed* to provide that.

> > (As an aside you may also then want to think about whether you allow
> > measured userspace elements that secure_forbidden is considered to be 0
> > for so you can sign userspace apps that are allowed to do RAWIO)
> 
> I'd be amazed if any of the applications that need RAWIO have had any
> kind of meaningful security audit

How re-assuring ;)

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> Have you actually looked at these patches? I've looked at every case of
> RAWIO in the kernel. For cases that are hardware specific and tied to
> fairly old hardware, I've ignored them. For cases which provide an

Yes I have - and it's not exactly localised: it modifies stuff all over
the tree when it shouldn't need to. It's a security policy. If it leaks
policy all over the kernel then the implementation model is *wrong*.

> >default behaviour for capable(x) in fact be
> >
> >        capable(x & ~secure_forbidden)
> >
> >for a measured kernel and add a 
> >
> >        capable_always()
> >
> >for the cases you want to not break.
> 
> We could do that, but now the behaviour of the patchset is far less
> obvious. capable(CAP_SYS_RAWIO) now means something different to every
> other use of capable(), and we still need get_trusted_kernel() calls for
> cases where the checks have nothing to do with processes and so
> capabilities can't be used. 

You don't need get_measured_kernel() calls because thats hidden within
capable/capable_always. And if the interaction is a complicated as you
imply that again says to me the model is wrong. Complicated multi-way
security interactions lead to complicated exploits leveraging the
disconnects.

But see further down...

> > As for mem= and exactmap, it has nothing to do with /dev/mem and
> > everything to do with giving the kernel a memory map where some of the
> > space it thinks is RAM is in fact devices, rom, space etc. If the kernel
> > is given a false memory map it will misbehave. Exploitably - well given
> > the kind of things people have achieved in the past - quite possibly.
> 
> Sure. That's a worthwhile thing to fix, and it's something that dropping
> CAP_SYS_RAWIO would do nothing to help you with.

I didn't mention it with regards to CAP_SYS_RAWIO, just as a general
example of stuff not dealt with. Again that one really suggests that the
right approach is to whitelist allowed boot options not blacklist bad
ones. It's the usual story with security, people fight bitterly not to do
whitelisting, then spend two years and more effort fixing holes than
doing whitelisting then eventually have to whitelist anyway. 

(or maybe I'm just bitter and cynical)
 
> We can do this without unnecessarily breaking any userspace. We just
> can't do it by fiddling with capabilities.

It should still be a security model nto spreading measured_kernel()
checks about. Now if that means

	capable(CAP_SYS_RAWIO)

should become security-> interface hooks that pass something meaningful
to the security layer then I'd rather we did the job properly in the first
place and put the policy in one spot not all over the kernel.

The question then becomes what do we need to pass beyond "CAP_SYS_RAWIO"
so the policy can be in the right place - even for example imposed by
SELinux rules.
	

> > I don't think we need to break any userspace for "normal" mode to do
> > this. Userspace in measured mode is going to change anyway. It already
> > has just for things like module signing.
> 
> This has been discussed at length. Nobody who's actually spent time
> working on the problem wants to use capabilities. CAP_SYS_RAWIO is not
> semantically identical to the trusted kernel bit. Trying to make them
> semantically identical will break existing userspace.

I never said it was. I said that getting rid of CAP_SYS_RAWIO and then
dealing with *just* the exceptions to this is a lot cleaner, and likely
to be more secure.

Right now the 'measured' patches spew all over the place. That shouldn't
be needed.

In addition several of the cases that you point out could be fixed by
replacing the CAP_SYS_RAWIO using stuff with a proper interface should
probably just be *fixed* to provide that.

> > (As an aside you may also then want to think about whether you allow
> > measured userspace elements that secure_forbidden is considered to be 0
> > for so you can sign userspace apps that are allowed to do RAWIO)
> 
> I'd be amazed if any of the applications that need RAWIO have had any
> kind of meaningful security audit

How re-assuring ;)

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4395 bytes --]

On Fri, 2014-03-14 at 12:22 +0000, One Thousand Gnomes wrote:
> > Have you actually looked at these patches? I've looked at every case of
> > RAWIO in the kernel. For cases that are hardware specific and tied to
> > fairly old hardware, I've ignored them. For cases which provide an
> 
> Yes I have - and it's not exactly localised: it modifies stuff all over
> the tree when it shouldn't need to. It's a security policy. If it leaks
> policy all over the kernel then the implementation model is *wrong*.

But you keep talking about MSRs despite there being a patch that limits
access to MSRs. If you have specific examples of privilege escalations
that are possible even with these patches then please, mention them.

> > >default behaviour for capable(x) in fact be
> > >
> > >        capable(x & ~secure_forbidden)
> > >
> > >for a measured kernel and add a 
> > >
> > >        capable_always()
> > >
> > >for the cases you want to not break.
> > 
> > We could do that, but now the behaviour of the patchset is far less
> > obvious. capable(CAP_SYS_RAWIO) now means something different to every
> > other use of capable(), and we still need get_trusted_kernel() calls for
> > cases where the checks have nothing to do with processes and so
> > capabilities can't be used. 
> 
> You don't need get_measured_kernel() calls because thats hidden within
> capable/capable_always. And if the interaction is a complicated as you
> imply that again says to me the model is wrong. Complicated multi-way
> security interactions lead to complicated exploits leveraging the
> disconnects.

How is capable() going to be any use when deciding whether or not to
interpret some kernel command line options?

> > We can do this without unnecessarily breaking any userspace. We just
> > can't do it by fiddling with capabilities.
> 
> It should still be a security model nto spreading measured_kernel()
> checks about. Now if that means
> 
> 	capable(CAP_SYS_RAWIO)
> 
> should become security-> interface hooks that pass something meaningful
> to the security layer then I'd rather we did the job properly in the first
> place and put the policy in one spot not all over the kernel.

We still need a mechanism to differentiate between cases where
CAP_SYS_RAWIO should be forbidden and cases where it should be
permitted, which means that we need to add additional policy. We can
modify capable(), but that means that capable() no longer does what you
expect it to - it's not a transparent interface, and that's likely to
result in people accidentally misusing it.

> The question then becomes what do we need to pass beyond "CAP_SYS_RAWIO"
> so the policy can be in the right place - even for example imposed by
> SELinux rules.

It needs to be possible for this policy to be imposed without userspace
being involved, so using selinux would mean baking it into the kernel
(along with an additional implementation for apparmor, and presumably
one for tomoyo as well).

> 
> > > I don't think we need to break any userspace for "normal" mode to do
> > > this. Userspace in measured mode is going to change anyway. It already
> > > has just for things like module signing.
> > 
> > This has been discussed at length. Nobody who's actually spent time
> > working on the problem wants to use capabilities. CAP_SYS_RAWIO is not
> > semantically identical to the trusted kernel bit. Trying to make them
> > semantically identical will break existing userspace.
> 
> I never said it was. I said that getting rid of CAP_SYS_RAWIO and then
> dealing with *just* the exceptions to this is a lot cleaner, and likely
> to be more secure.

And will give us the choice between complicating a simple interface or
breaking userspace. If the maintainer believes that's a better approach
then I can rewrite all of this again, but so far you seem to be in a
minority on this front. 

> In addition several of the cases that you point out could be fixed by
> replacing the CAP_SYS_RAWIO using stuff with a proper interface should
> probably just be *fixed* to provide that.

...thus breaking userspace outside this use case. I mean, sure, I'm
absolutely fine with deleting /dev/mem entirely. I don't see Linus
agreeing.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-03-14 at 12:22 +0000, One Thousand Gnomes wrote:
> > Have you actually looked at these patches? I've looked at every case of
> > RAWIO in the kernel. For cases that are hardware specific and tied to
> > fairly old hardware, I've ignored them. For cases which provide an
> 
> Yes I have - and it's not exactly localised: it modifies stuff all over
> the tree when it shouldn't need to. It's a security policy. If it leaks
> policy all over the kernel then the implementation model is *wrong*.

But you keep talking about MSRs despite there being a patch that limits
access to MSRs. If you have specific examples of privilege escalations
that are possible even with these patches then please, mention them.

> > >default behaviour for capable(x) in fact be
> > >
> > >        capable(x & ~secure_forbidden)
> > >
> > >for a measured kernel and add a 
> > >
> > >        capable_always()
> > >
> > >for the cases you want to not break.
> > 
> > We could do that, but now the behaviour of the patchset is far less
> > obvious. capable(CAP_SYS_RAWIO) now means something different to every
> > other use of capable(), and we still need get_trusted_kernel() calls for
> > cases where the checks have nothing to do with processes and so
> > capabilities can't be used. 
> 
> You don't need get_measured_kernel() calls because thats hidden within
> capable/capable_always. And if the interaction is a complicated as you
> imply that again says to me the model is wrong. Complicated multi-way
> security interactions lead to complicated exploits leveraging the
> disconnects.

How is capable() going to be any use when deciding whether or not to
interpret some kernel command line options?

> > We can do this without unnecessarily breaking any userspace. We just
> > can't do it by fiddling with capabilities.
> 
> It should still be a security model nto spreading measured_kernel()
> checks about. Now if that means
> 
> 	capable(CAP_SYS_RAWIO)
> 
> should become security-> interface hooks that pass something meaningful
> to the security layer then I'd rather we did the job properly in the first
> place and put the policy in one spot not all over the kernel.

We still need a mechanism to differentiate between cases where
CAP_SYS_RAWIO should be forbidden and cases where it should be
permitted, which means that we need to add additional policy. We can
modify capable(), but that means that capable() no longer does what you
expect it to - it's not a transparent interface, and that's likely to
result in people accidentally misusing it.

> The question then becomes what do we need to pass beyond "CAP_SYS_RAWIO"
> so the policy can be in the right place - even for example imposed by
> SELinux rules.

It needs to be possible for this policy to be imposed without userspace
being involved, so using selinux would mean baking it into the kernel
(along with an additional implementation for apparmor, and presumably
one for tomoyo as well).

> 
> > > I don't think we need to break any userspace for "normal" mode to do
> > > this. Userspace in measured mode is going to change anyway. It already
> > > has just for things like module signing.
> > 
> > This has been discussed at length. Nobody who's actually spent time
> > working on the problem wants to use capabilities. CAP_SYS_RAWIO is not
> > semantically identical to the trusted kernel bit. Trying to make them
> > semantically identical will break existing userspace.
> 
> I never said it was. I said that getting rid of CAP_SYS_RAWIO and then
> dealing with *just* the exceptions to this is a lot cleaner, and likely
> to be more secure.

And will give us the choice between complicating a simple interface or
breaking userspace. If the maintainer believes that's a better approach
then I can rewrite all of this again, but so far you seem to be in a
minority on this front. 

> In addition several of the cases that you point out could be fixed by
> replacing the CAP_SYS_RAWIO using stuff with a proper interface should
> probably just be *fixed* to provide that.

...thus breaking userspace outside this use case. I mean, sure, I'm
absolutely fine with deleting /dev/mem entirely. I don't see Linus
agreeing.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[Kees Cook]

On Fri, Mar 14, 2014 at 5:51 AM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Fri, 2014-03-14 at 12:22 +0000, One Thousand Gnomes wrote:
>> > Have you actually looked at these patches? I've looked at every case of
>> > RAWIO in the kernel. For cases that are hardware specific and tied to
>> > fairly old hardware, I've ignored them. For cases which provide an
>>
>> Yes I have - and it's not exactly localised: it modifies stuff all over
>> the tree when it shouldn't need to. It's a security policy. If it leaks
>> policy all over the kernel then the implementation model is *wrong*.
>
> But you keep talking about MSRs despite there being a patch that limits
> access to MSRs. If you have specific examples of privilege escalations
> that are possible even with these patches then please, mention them.
>
>> > >default behaviour for capable(x) in fact be
>> > >
>> > >        capable(x & ~secure_forbidden)
>> > >
>> > >for a measured kernel and add a
>> > >
>> > >        capable_always()
>> > >
>> > >for the cases you want to not break.
>> >
>> > We could do that, but now the behaviour of the patchset is far less
>> > obvious. capable(CAP_SYS_RAWIO) now means something different to every
>> > other use of capable(), and we still need get_trusted_kernel() calls for
>> > cases where the checks have nothing to do with processes and so
>> > capabilities can't be used.

And this ignores the fact that being able to query the measured state
is potentially valuable to additional security policies (e.g. module
loading, firmware loading). Hiding the measured logic in capabilities
makes no sense -- they're orthogonal restrictions.

>>
>> You don't need get_measured_kernel() calls because thats hidden within
>> capable/capable_always. And if the interaction is a complicated as you
>> imply that again says to me the model is wrong. Complicated multi-way
>> security interactions lead to complicated exploits leveraging the
>> disconnects.
>
> How is capable() going to be any use when deciding whether or not to
> interpret some kernel command line options?

The command line problem here is a total red herring. If you've got a
measured kernel, you have a measured command line. (If not, you don't
have a measured kernel.) Dealing with the command line has nothing to
do with enforcing the ring0/uid0 boundary which is what this patch
series does.

>> > We can do this without unnecessarily breaking any userspace. We just
>> > can't do it by fiddling with capabilities.
>>
>> It should still be a security model nto spreading measured_kernel()
>> checks about. Now if that means
>>
>>       capable(CAP_SYS_RAWIO)
>>
>> should become security-> interface hooks that pass something meaningful
>> to the security layer then I'd rather we did the job properly in the first
>> place and put the policy in one spot not all over the kernel.
>
> We still need a mechanism to differentiate between cases where
> CAP_SYS_RAWIO should be forbidden and cases where it should be
> permitted, which means that we need to add additional policy. We can
> modify capable(), but that means that capable() no longer does what you
> expect it to - it's not a transparent interface, and that's likely to
> result in people accidentally misusing it.
>
>> The question then becomes what do we need to pass beyond "CAP_SYS_RAWIO"
>> so the policy can be in the right place - even for example imposed by
>> SELinux rules.
>
> It needs to be possible for this policy to be imposed without userspace
> being involved, so using selinux would mean baking it into the kernel
> (along with an additional implementation for apparmor, and presumably
> one for tomoyo as well).
>
>>
>> > > I don't think we need to break any userspace for "normal" mode to do
>> > > this. Userspace in measured mode is going to change anyway. It already
>> > > has just for things like module signing.
>> >
>> > This has been discussed at length. Nobody who's actually spent time
>> > working on the problem wants to use capabilities. CAP_SYS_RAWIO is not
>> > semantically identical to the trusted kernel bit. Trying to make them
>> > semantically identical will break existing userspace.
>>
>> I never said it was. I said that getting rid of CAP_SYS_RAWIO and then
>> dealing with *just* the exceptions to this is a lot cleaner, and likely
>> to be more secure.
>
> And will give us the choice between complicating a simple interface or
> breaking userspace. If the maintainer believes that's a better approach
> then I can rewrite all of this again, but so far you seem to be in a
> minority on this front.
>
>> In addition several of the cases that you point out could be fixed by
>> replacing the CAP_SYS_RAWIO using stuff with a proper interface should
>> probably just be *fixed* to provide that.
>
> ...thus breaking userspace outside this use case. I mean, sure, I'm
> absolutely fine with deleting /dev/mem entirely. I don't see Linus
> agreeing.

Right. As mentioned, we've been over all of this before. We cannot
change the meaning of capabilities (nor expand their coverage to
existing interfaces) without breaking userspace. Since we cannot break
userspace, we must create a different policy system that covers this
new thing we want to do and keeps the new policy distinctly separate.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[Kees Cook]

On Fri, Mar 14, 2014 at 5:51 AM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Fri, 2014-03-14 at 12:22 +0000, One Thousand Gnomes wrote:
>> > Have you actually looked at these patches? I've looked at every case of
>> > RAWIO in the kernel. For cases that are hardware specific and tied to
>> > fairly old hardware, I've ignored them. For cases which provide an
>>
>> Yes I have - and it's not exactly localised: it modifies stuff all over
>> the tree when it shouldn't need to. It's a security policy. If it leaks
>> policy all over the kernel then the implementation model is *wrong*.
>
> But you keep talking about MSRs despite there being a patch that limits
> access to MSRs. If you have specific examples of privilege escalations
> that are possible even with these patches then please, mention them.
>
>> > >default behaviour for capable(x) in fact be
>> > >
>> > >        capable(x & ~secure_forbidden)
>> > >
>> > >for a measured kernel and add a
>> > >
>> > >        capable_always()
>> > >
>> > >for the cases you want to not break.
>> >
>> > We could do that, but now the behaviour of the patchset is far less
>> > obvious. capable(CAP_SYS_RAWIO) now means something different to every
>> > other use of capable(), and we still need get_trusted_kernel() calls for
>> > cases where the checks have nothing to do with processes and so
>> > capabilities can't be used.

And this ignores the fact that being able to query the measured state
is potentially valuable to additional security policies (e.g. module
loading, firmware loading). Hiding the measured logic in capabilities
makes no sense -- they're orthogonal restrictions.

>>
>> You don't need get_measured_kernel() calls because thats hidden within
>> capable/capable_always. And if the interaction is a complicated as you
>> imply that again says to me the model is wrong. Complicated multi-way
>> security interactions lead to complicated exploits leveraging the
>> disconnects.
>
> How is capable() going to be any use when deciding whether or not to
> interpret some kernel command line options?

The command line problem here is a total red herring. If you've got a
measured kernel, you have a measured command line. (If not, you don't
have a measured kernel.) Dealing with the command line has nothing to
do with enforcing the ring0/uid0 boundary which is what this patch
series does.

>> > We can do this without unnecessarily breaking any userspace. We just
>> > can't do it by fiddling with capabilities.
>>
>> It should still be a security model nto spreading measured_kernel()
>> checks about. Now if that means
>>
>>       capable(CAP_SYS_RAWIO)
>>
>> should become security-> interface hooks that pass something meaningful
>> to the security layer then I'd rather we did the job properly in the first
>> place and put the policy in one spot not all over the kernel.
>
> We still need a mechanism to differentiate between cases where
> CAP_SYS_RAWIO should be forbidden and cases where it should be
> permitted, which means that we need to add additional policy. We can
> modify capable(), but that means that capable() no longer does what you
> expect it to - it's not a transparent interface, and that's likely to
> result in people accidentally misusing it.
>
>> The question then becomes what do we need to pass beyond "CAP_SYS_RAWIO"
>> so the policy can be in the right place - even for example imposed by
>> SELinux rules.
>
> It needs to be possible for this policy to be imposed without userspace
> being involved, so using selinux would mean baking it into the kernel
> (along with an additional implementation for apparmor, and presumably
> one for tomoyo as well).
>
>>
>> > > I don't think we need to break any userspace for "normal" mode to do
>> > > this. Userspace in measured mode is going to change anyway. It already
>> > > has just for things like module signing.
>> >
>> > This has been discussed at length. Nobody who's actually spent time
>> > working on the problem wants to use capabilities. CAP_SYS_RAWIO is not
>> > semantically identical to the trusted kernel bit. Trying to make them
>> > semantically identical will break existing userspace.
>>
>> I never said it was. I said that getting rid of CAP_SYS_RAWIO and then
>> dealing with *just* the exceptions to this is a lot cleaner, and likely
>> to be more secure.
>
> And will give us the choice between complicating a simple interface or
> breaking userspace. If the maintainer believes that's a better approach
> then I can rewrite all of this again, but so far you seem to be in a
> minority on this front.
>
>> In addition several of the cases that you point out could be fixed by
>> replacing the CAP_SYS_RAWIO using stuff with a proper interface should
>> probably just be *fixed* to provide that.
>
> ...thus breaking userspace outside this use case. I mean, sure, I'm
> absolutely fine with deleting /dev/mem entirely. I don't see Linus
> agreeing.

Right. As mentioned, we've been over all of this before. We cannot
change the meaning of capabilities (nor expand their coverage to
existing interfaces) without breaking userspace. Since we cannot break
userspace, we must create a different policy system that covers this
new thing we want to do and keeps the new policy distinctly separate.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 874 bytes --]

On Fri, 2014-03-14 at 08:23 -0700, Kees Cook wrote:

> The command line problem here is a total red herring. If you've got a
> measured kernel, you have a measured command line. (If not, you don't
> have a measured kernel.) Dealing with the command line has nothing to
> do with enforcing the ring0/uid0 boundary which is what this patch
> series does.

That's why I used trusted rather than measured. The Secure Boot trust
model assumes that the user is able to modify the command line (it's
basically impossible to deploy generically otherwise), so we need to
filter out command line options that allow a user to elevate themselves
into the kernel at boot time.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-03-14 at 08:23 -0700, Kees Cook wrote:

> The command line problem here is a total red herring. If you've got a
> measured kernel, you have a measured command line. (If not, you don't
> have a measured kernel.) Dealing with the command line has nothing to
> do with enforcing the ring0/uid0 boundary which is what this patch
> series does.

That's why I used trusted rather than measured. The Secure Boot trust
model assumes that the user is able to modify the command line (it's
basically impossible to deploy generically otherwise), so we need to
filter out command line options that allow a user to elevate themselves
into the kernel at boot time.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[Kees Cook]

On Fri, Mar 14, 2014 at 8:46 AM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Fri, 2014-03-14 at 08:23 -0700, Kees Cook wrote:
>
>> The command line problem here is a total red herring. If you've got a
>> measured kernel, you have a measured command line. (If not, you don't
>> have a measured kernel.) Dealing with the command line has nothing to
>> do with enforcing the ring0/uid0 boundary which is what this patch
>> series does.
>
> That's why I used trusted rather than measured. The Secure Boot trust
> model assumes that the user is able to modify the command line (it's
> basically impossible to deploy generically otherwise), so we need to
> filter out command line options that allow a user to elevate themselves
> into the kernel at boot time.

All the more reason to ignore command line at this point. For Chrome
OS, it's part of our boot state, so we don't care about it. For
generic Secure Boot, we can add checks for dangerous stuff as we go
forward. That's why I like this interface -- we can add to it as we
identify bad stuff, and it stay separate from other semantics.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[Kees Cook]

On Fri, Mar 14, 2014 at 8:46 AM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Fri, 2014-03-14 at 08:23 -0700, Kees Cook wrote:
>
>> The command line problem here is a total red herring. If you've got a
>> measured kernel, you have a measured command line. (If not, you don't
>> have a measured kernel.) Dealing with the command line has nothing to
>> do with enforcing the ring0/uid0 boundary which is what this patch
>> series does.
>
> That's why I used trusted rather than measured. The Secure Boot trust
> model assumes that the user is able to modify the command line (it's
> basically impossible to deploy generically otherwise), so we need to
> filter out command line options that allow a user to elevate themselves
> into the kernel at boot time.

All the more reason to ignore command line at this point. For Chrome
OS, it's part of our boot state, so we don't care about it. For
generic Secure Boot, we can add checks for dangerous stuff as we go
forward. That's why I like this interface -- we can add to it as we
identify bad stuff, and it stay separate from other semantics.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 919 bytes --]

On Fri, 2014-03-14 at 08:54 -0700, Kees Cook wrote:

> All the more reason to ignore command line at this point. For Chrome
> OS, it's part of our boot state, so we don't care about it. For
> generic Secure Boot, we can add checks for dangerous stuff as we go
> forward. That's why I like this interface -- we can add to it as we
> identify bad stuff, and it stay separate from other semantics.

Sure, it's just another reason not to want to use a capability-based
interface - not all the policy we want to impose is related to
processes, so capabilities really don't make sense. The current patchset
adds a restriction to the acpi_rsdp argument, and I've no objection to
adding one to limit the use of mem=.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-03-14 at 08:54 -0700, Kees Cook wrote:

> All the more reason to ignore command line at this point. For Chrome
> OS, it's part of our boot state, so we don't care about it. For
> generic Secure Boot, we can add checks for dangerous stuff as we go
> forward. That's why I like this interface -- we can add to it as we
> identify bad stuff, and it stay separate from other semantics.

Sure, it's just another reason not to want to use a capability-based
interface - not all the policy we want to impose is related to
processes, so capabilities really don't make sense. The current patchset
adds a restriction to the acpi_rsdp argument, and I've no objection to
adding one to limit the use of mem=.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> The command line problem here is a total red herring. If you've got a
> measured kernel, you have a measured command line. (If not, you don't

That would be the sensible approach, but it has some quite drastic
ramifications.

> have a measured kernel.) Dealing with the command line has nothing to
> do with enforcing the ring0/uid0 boundary which is what this patch
> series does.

It has a huge relevance. You are signing blocks of code and loading them
into your kernel. In case it's escaped your notice those blocks of code
have behaviour which is considerably customisable by passing module
arguments to them. In many cases they don't actually check those
arguments. If I can set module paramters I already 0wn your box so many
different ways its not even funny.

> Right. As mentioned, we've been over all of this before. We cannot
> change the meaning of capabilities (nor expand their coverage to
> existing interfaces) without breaking userspace. Since we cannot break
> userspace, we must create a different policy system that covers this
> new thing we want to do and keeps the new policy distinctly separate.

So you have everything checked twice all over the kernel and you
guarantee that people will get it wrong. I don't see the point of putting
a broken implementation in the kernel. If you want to do security do it
*right* and do it once. You think every single person who adds a driver
is going to muddle through two interfaces (and in future no doubt more if
we don't fix it properly) and get it right each time ?

The 'breaking userspace' argument is bullshit already. The whole *point*
of the measured kernel is to break userspace. You are deliberately
breaking a pile of functionality, much of it undersirable in order to
achieve your measured system.

The only "don't break" case that matters is if the measured stuff isn't
in use. In those cases we don't need to break anything.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> The command line problem here is a total red herring. If you've got a
> measured kernel, you have a measured command line. (If not, you don't

That would be the sensible approach, but it has some quite drastic
ramifications.

> have a measured kernel.) Dealing with the command line has nothing to
> do with enforcing the ring0/uid0 boundary which is what this patch
> series does.

It has a huge relevance. You are signing blocks of code and loading them
into your kernel. In case it's escaped your notice those blocks of code
have behaviour which is considerably customisable by passing module
arguments to them. In many cases they don't actually check those
arguments. If I can set module paramters I already 0wn your box so many
different ways its not even funny.

> Right. As mentioned, we've been over all of this before. We cannot
> change the meaning of capabilities (nor expand their coverage to
> existing interfaces) without breaking userspace. Since we cannot break
> userspace, we must create a different policy system that covers this
> new thing we want to do and keeps the new policy distinctly separate.

So you have everything checked twice all over the kernel and you
guarantee that people will get it wrong. I don't see the point of putting
a broken implementation in the kernel. If you want to do security do it
*right* and do it once. You think every single person who adds a driver
is going to muddle through two interfaces (and in future no doubt more if
we don't fix it properly) and get it right each time ?

The 'breaking userspace' argument is bullshit already. The whole *point*
of the measured kernel is to break userspace. You are deliberately
breaking a pile of functionality, much of it undersirable in order to
achieve your measured system.

The only "don't break" case that matters is if the measured stuff isn't
in use. In those cases we don't need to break anything.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> But you keep talking about MSRs despite there being a patch that limits
> access to MSRs. If you have specific examples of privilege escalations
> that are possible even with these patches then please, mention them.

I mentioned MSRs once, and then you kept going on about it. 

Your patches are all over the tree, for what should be one piece of
policy. They rely on not missing a single case, which is also bad
security engineering. We know that, as an industry we've spent fifty
years repeatedly sticking our fingers in our ears, refusing to listen and
then learning it again and again the hard way.

> How is capable() going to be any use when deciding whether or not to
> interpret some kernel command line options?

I have no idea, why are you asking such an odd question ?

You need to whitelist module parameters and boot options that are safe in
your measured kernel environment. Nothing to do with RAWIO or capable(),
just that we have thousands of such options many of which are quite
exploitable because they assume the loader is trustable (which btw is
already *broken* and on many systems - eg Fedora - allows you to break
from DAC override into RAWIO).

That one actually wants cleaning up anyway but its hard to see the right
way to do it.

> We still need a mechanism to differentiate between cases where
> CAP_SYS_RAWIO should be forbidden and cases where it should be
> permitted, which means that we need to add additional policy. We can
> modify capable(), but that means that capable() no longer does what you
> expect it to - it's not a transparent interface, and that's likely to
> result in people accidentally misusing it.

capable() is not a good interface. If we are going to patch half
the capable() calls in the kernel wouldn't it be a good time to actually
do something about it properly ?

At the very least we should turn capabilities/needs into two lists and
make them a 2D grid so we break the tie between the userspace
'capability' and 'what this allows you to do in this specific
instance'.

For example instead of spewing magic trusted kernel checks all over the
place we could do a global seach/replace of most of the capable calls with

	is_permitted(x)

where x is a value indicating what we wish to do (action) not what right
we need (policy). All the real mess comes from the fact that capable() is
implementing security *policy* in drivers all over the kernel.

You can then set up a map of capability to X values, and more importantly
you can use that map to have more complete, and descriptive versions of
'x' than capabilities.

You can now ask the rather more useful question

	am I allowed to do 'X'

and the answer may well depend upon selinux, trust models and what have
you as well as securelevels and other concepts anyone wishes to play with
(things like remote attestation for example)

A simple minded adjustment to cap_capable would then be to make
cap_is_permitted what cap_capable is now but with

	cap = security_required_capability[x];
	if (cap == -1)
		return 0;

Prior to adding anything or considered measured kernels that would be a
1:1 mapping. Nothing breaks, nothing exciting happens.

But you can now add new permissions and map them about. So you can mark
some of the RAW_IO ones with new names and in non trusted kernels they
get given a map value of CAP_SYS_RAWIO, or in trusted cases -1.

At that point we've got

- an automated conversion for the non-measured case, so we don't screw up
  a long change set.

- no breakages of userspace in the non measured case (and we can test at
  this point quite happily)

- a single interface, because driver authors can't be expected to
  understand two differently broken interacting sets of checks (plus
  since we just got rid of capable they can't forget to update their code
  or copy old now broken but happily compiling code).

- a way to fix the existing capable() mess

- a way to support measured kernels by adding new permissions without
  them breaking non measured userspace cases, without breaking the
  capability model seen from userspace.

  So for example we can add

	RIGHT_LOAD_SIGNED_FIRMWARE
	RIGHT_LOAD_UNSIGNED_FIRMWARE

  etc and we can tie them to things other than CAP_SYS_RAWIO in untrusted
  mode. Indeed it wouldn't be hard to make the table two dimensional

	cap = security_required_capability[measured_kernel()][x];

  and just mark the 'generic' RIGHT_RAW_IO_ACCESS as -1 in trusted mode
  but CAP_SYS_RAWIO if not, while RIGHT_LOAD_SIGNED_FIRMWARE is
  CAP_SYS_RAWIO in both cases.

- a whitelist type behaviour. If the default for RIGHT_RAW_IO_ACCESS is
  -1 in trusted mode then only the cases someone goes back and explicitly
  refines will get enabled. Anyone adding a driver or blindly copying the
  old behaviours either breaks or if they blindly convert gets the safe
  behaviour.

- a way to allow security modules to make smarter decisions because they
  all have the ability to implement policy for such events, and
  because we can provide a meaningful way to split them further as
  needed

- a way to make the question of trust namespace specific (because caps
  are and the cap_capable logic has NS visibility)

Why do we care about that, because if you want you can boot a measured
file system which contains some services you consider part of the
measured environment, and put the unmeasured universe in another
namespace.

> > The question then becomes what do we need to pass beyond "CAP_SYS_RAWIO"
> > so the policy can be in the right place - even for example imposed by
> > SELinux rules.
> 
> It needs to be possible for this policy to be imposed without userspace
> being involved, so using selinux would mean baking it into the kernel
> (along with an additional implementation for apparmor, and presumably
> one for tomoyo as well).

That's clearly false. You can load signed modules so you can load signed
policies. Assuming you don't have an initial measured file system you
need a kernel policy initially that protects you. If you have a measured
file system you don't even need that, nor do you need to revoke RAWIO in
kernel, you can do it before you switch into "measured and running
un-measured code" mode - ie about the point you pivot root out of the
initrd that loaded the measured policy.

> > In addition several of the cases that you point out could be fixed by
> > replacing the CAP_SYS_RAWIO using stuff with a proper interface should
> > probably just be *fixed* to provide that.
> 
> ...thus breaking userspace outside this use case. I mean, sure, I'm
> absolutely fine with deleting /dev/mem entirely. I don't see Linus
> agreeing.

That's not what I meant. We have a lot of interfaces where we use
CAP_SYS_RAWIO rather than having a structured interface as opposed to a
'yeah whatever, poke the registers' interface.

For a lot of environments btw you don't actually need /dev/mem. It's not
really got a lot of uses any more and plenty of people really restrict or
remove it from systems. I don't think very many people would be sad
if /dev/mem became a legacy interface most people left turned off.

> That's why I used trusted rather than measured. The Secure Boot trust
> model assumes that the user is able to modify the command line (it's
> basically impossible to deploy generically otherwise), so we need to
> filter out command line options that allow a user to elevate themselves
> into the kernel at boot time.

This is the kind of head up arsehole thinking that has security people
banging their heads against the wall wondering why programmers are
incapable of learning from history.

You do not want to "filter out" options. You'll never get them all, there
are rather a lot of them, and many apparently harmless ones are not at
all harmless. The number of drivers where you can pass the maximum number
of interfaces which then do

	blah = kzalloc(sizeof(stuff) * unchecked_number)

is rather large for example. The number of devices that allow you force
arbitary MMIO values is not exactly small either.

You want to whitelist the ones you actually need, and have done careful
inspections of. The number of kernel options actually used by 99.99% of
the universe in normal operational state or when doing basic poking at
stuff or verbose booting is minimal. It also btw means you can have a
measured command line (all trusted), or a mixed command line (measured
bits trusted implicitly, rest not).

It's not too hard. Most are module_parm, so we just need
module_parm_is_safe(x) and a single centralised check for the general
case.

If we are going to do a measured kernel then it needs to be done right,
it needs to be properly abstracted so we don't add another one for
Android, another one for Chrome, another one for ARM trustzone, another
one for Intel SGX, and so on.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> But you keep talking about MSRs despite there being a patch that limits
> access to MSRs. If you have specific examples of privilege escalations
> that are possible even with these patches then please, mention them.

I mentioned MSRs once, and then you kept going on about it. 

Your patches are all over the tree, for what should be one piece of
policy. They rely on not missing a single case, which is also bad
security engineering. We know that, as an industry we've spent fifty
years repeatedly sticking our fingers in our ears, refusing to listen and
then learning it again and again the hard way.

> How is capable() going to be any use when deciding whether or not to
> interpret some kernel command line options?

I have no idea, why are you asking such an odd question ?

You need to whitelist module parameters and boot options that are safe in
your measured kernel environment. Nothing to do with RAWIO or capable(),
just that we have thousands of such options many of which are quite
exploitable because they assume the loader is trustable (which btw is
already *broken* and on many systems - eg Fedora - allows you to break
from DAC override into RAWIO).

That one actually wants cleaning up anyway but its hard to see the right
way to do it.

> We still need a mechanism to differentiate between cases where
> CAP_SYS_RAWIO should be forbidden and cases where it should be
> permitted, which means that we need to add additional policy. We can
> modify capable(), but that means that capable() no longer does what you
> expect it to - it's not a transparent interface, and that's likely to
> result in people accidentally misusing it.

capable() is not a good interface. If we are going to patch half
the capable() calls in the kernel wouldn't it be a good time to actually
do something about it properly ?

At the very least we should turn capabilities/needs into two lists and
make them a 2D grid so we break the tie between the userspace
'capability' and 'what this allows you to do in this specific
instance'.

For example instead of spewing magic trusted kernel checks all over the
place we could do a global seach/replace of most of the capable calls with

	is_permitted(x)

where x is a value indicating what we wish to do (action) not what right
we need (policy). All the real mess comes from the fact that capable() is
implementing security *policy* in drivers all over the kernel.

You can then set up a map of capability to X values, and more importantly
you can use that map to have more complete, and descriptive versions of
'x' than capabilities.

You can now ask the rather more useful question

	am I allowed to do 'X'

and the answer may well depend upon selinux, trust models and what have
you as well as securelevels and other concepts anyone wishes to play with
(things like remote attestation for example)

A simple minded adjustment to cap_capable would then be to make
cap_is_permitted what cap_capable is now but with

	cap = security_required_capability[x];
	if (cap == -1)
		return 0;

Prior to adding anything or considered measured kernels that would be a
1:1 mapping. Nothing breaks, nothing exciting happens.

But you can now add new permissions and map them about. So you can mark
some of the RAW_IO ones with new names and in non trusted kernels they
get given a map value of CAP_SYS_RAWIO, or in trusted cases -1.

At that point we've got

- an automated conversion for the non-measured case, so we don't screw up
  a long change set.

- no breakages of userspace in the non measured case (and we can test at
  this point quite happily)

- a single interface, because driver authors can't be expected to
  understand two differently broken interacting sets of checks (plus
  since we just got rid of capable they can't forget to update their code
  or copy old now broken but happily compiling code).

- a way to fix the existing capable() mess

- a way to support measured kernels by adding new permissions without
  them breaking non measured userspace cases, without breaking the
  capability model seen from userspace.

  So for example we can add

	RIGHT_LOAD_SIGNED_FIRMWARE
	RIGHT_LOAD_UNSIGNED_FIRMWARE

  etc and we can tie them to things other than CAP_SYS_RAWIO in untrusted
  mode. Indeed it wouldn't be hard to make the table two dimensional

	cap = security_required_capability[measured_kernel()][x];

  and just mark the 'generic' RIGHT_RAW_IO_ACCESS as -1 in trusted mode
  but CAP_SYS_RAWIO if not, while RIGHT_LOAD_SIGNED_FIRMWARE is
  CAP_SYS_RAWIO in both cases.

- a whitelist type behaviour. If the default for RIGHT_RAW_IO_ACCESS is
  -1 in trusted mode then only the cases someone goes back and explicitly
  refines will get enabled. Anyone adding a driver or blindly copying the
  old behaviours either breaks or if they blindly convert gets the safe
  behaviour.

- a way to allow security modules to make smarter decisions because they
  all have the ability to implement policy for such events, and
  because we can provide a meaningful way to split them further as
  needed

- a way to make the question of trust namespace specific (because caps
  are and the cap_capable logic has NS visibility)

Why do we care about that, because if you want you can boot a measured
file system which contains some services you consider part of the
measured environment, and put the unmeasured universe in another
namespace.

> > The question then becomes what do we need to pass beyond "CAP_SYS_RAWIO"
> > so the policy can be in the right place - even for example imposed by
> > SELinux rules.
> 
> It needs to be possible for this policy to be imposed without userspace
> being involved, so using selinux would mean baking it into the kernel
> (along with an additional implementation for apparmor, and presumably
> one for tomoyo as well).

That's clearly false. You can load signed modules so you can load signed
policies. Assuming you don't have an initial measured file system you
need a kernel policy initially that protects you. If you have a measured
file system you don't even need that, nor do you need to revoke RAWIO in
kernel, you can do it before you switch into "measured and running
un-measured code" mode - ie about the point you pivot root out of the
initrd that loaded the measured policy.

> > In addition several of the cases that you point out could be fixed by
> > replacing the CAP_SYS_RAWIO using stuff with a proper interface should
> > probably just be *fixed* to provide that.
> 
> ...thus breaking userspace outside this use case. I mean, sure, I'm
> absolutely fine with deleting /dev/mem entirely. I don't see Linus
> agreeing.

That's not what I meant. We have a lot of interfaces where we use
CAP_SYS_RAWIO rather than having a structured interface as opposed to a
'yeah whatever, poke the registers' interface.

For a lot of environments btw you don't actually need /dev/mem. It's not
really got a lot of uses any more and plenty of people really restrict or
remove it from systems. I don't think very many people would be sad
if /dev/mem became a legacy interface most people left turned off.

> That's why I used trusted rather than measured. The Secure Boot trust
> model assumes that the user is able to modify the command line (it's
> basically impossible to deploy generically otherwise), so we need to
> filter out command line options that allow a user to elevate themselves
> into the kernel at boot time.

This is the kind of head up arsehole thinking that has security people
banging their heads against the wall wondering why programmers are
incapable of learning from history.

You do not want to "filter out" options. You'll never get them all, there
are rather a lot of them, and many apparently harmless ones are not at
all harmless. The number of drivers where you can pass the maximum number
of interfaces which then do

	blah = kzalloc(sizeof(stuff) * unchecked_number)

is rather large for example. The number of devices that allow you force
arbitary MMIO values is not exactly small either.

You want to whitelist the ones you actually need, and have done careful
inspections of. The number of kernel options actually used by 99.99% of
the universe in normal operational state or when doing basic poking at
stuff or verbose booting is minimal. It also btw means you can have a
measured command line (all trusted), or a mixed command line (measured
bits trusted implicitly, rest not).

It's not too hard. Most are module_parm, so we just need
module_parm_is_safe(x) and a single centralised check for the general
case.

If we are going to do a measured kernel then it needs to be done right,
it needs to be properly abstracted so we don't add another one for
Android, another one for Chrome, another one for ARM trustzone, another
one for Intel SGX, and so on.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4335 bytes --]

On Fri, 2014-03-14 at 17:06 +0000, One Thousand Gnomes wrote:
> > But you keep talking about MSRs despite there being a patch that limits
> > access to MSRs. If you have specific examples of privilege escalations
> > that are possible even with these patches then please, mention them.
> 
> I mentioned MSRs once, and then you kept going on about it. 

You've twice mentioned MSRs as a case where CAP_SYS_RAWIO allows you to
elevate privileges, despite that being a case that's explicitly covered
by this patchset. Now, do you have an actual example of CAP_SYS_RAWIO
allowing you to elevate privileges even with this patchset applied (and
without requiring the presence of obsolete hardware, which is a separate
conversation)?

> Your patches are all over the tree, for what should be one piece of
> policy. They rely on not missing a single case, which is also bad
> security engineering. We know that, as an industry we've spent fifty
> years repeatedly sticking our fingers in our ears, refusing to listen and
> then learning it again and again the hard way.

I have a set of patches that appear acceptable to the security
maintainer. I've rewritten them multiple times in response to various
levels of bikeshedding. They solve a real problem and are being shipped
by multiple distributions.

What the rest of your mail is asking me to do is to rewrite capabilities
support entirely. Now, yes, I agree that capabilities are an awful
interface that's approximately impossible to use correctly and which
adds little security even if you do. But I reject the idea that
rewriting fundamental security infrastructure should be a prerequisite
for adding this functionality.

> > It needs to be possible for this policy to be imposed without userspace
> > being involved, so using selinux would mean baking it into the kernel
> > (along with an additional implementation for apparmor, and presumably
> > one for tomoyo as well).
> 
> That's clearly false. You can load signed modules so you can load signed
> policies. Assuming you don't have an initial measured file system you
> need a kernel policy initially that protects you. If you have a measured
> file system you don't even need that, nor do you need to revoke RAWIO in
> kernel, you can do it before you switch into "measured and running
> un-measured code" mode - ie about the point you pivot root out of the
> initrd that loaded the measured policy.

We can't rely on userspace choosing to load that policy. We don't have a
measured filesystem. The initrd is not signed (and nor can it be). The
kernel itself *must* impose policy.

> > ...thus breaking userspace outside this use case. I mean, sure, I'm
> > absolutely fine with deleting /dev/mem entirely. I don't see Linus
> > agreeing.
> 
> That's not what I meant. We have a lot of interfaces where we use
> CAP_SYS_RAWIO rather than having a structured interface as opposed to a
> 'yeah whatever, poke the registers' interface.
> 
> For a lot of environments btw you don't actually need /dev/mem. It's not
> really got a lot of uses any more and plenty of people really restrict or
> remove it from systems. I don't think very many people would be sad
> if /dev/mem became a legacy interface most people left turned off.

Sure! Except then A Large Vendor's custom IPMI userspace stops working,
even on systems without Secure Boot, and then said Large Vendor wants to
have conference calls at times that are convenient for Japan and then
this is an issue that's going to cause $1 billion in lost sales and come
on, you've been there. You know that this isn't going to work. 

> If we are going to do a measured kernel then it needs to be done right,
> it needs to be properly abstracted so we don't add another one for
> Android, another one for Chrome, another one for ARM trustzone, another
> one for Intel SGX, and so on.

The fact that you keep saying measured really does make me suspect that
you misunderstand the problem. There's no measurement involved, there's
simply an assertion that the firmware (which you're forced to trust)
chose, via some policy you may be unaware of, to trust the booted
kernel.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-03-14 at 17:06 +0000, One Thousand Gnomes wrote:
> > But you keep talking about MSRs despite there being a patch that limits
> > access to MSRs. If you have specific examples of privilege escalations
> > that are possible even with these patches then please, mention them.
> 
> I mentioned MSRs once, and then you kept going on about it. 

You've twice mentioned MSRs as a case where CAP_SYS_RAWIO allows you to
elevate privileges, despite that being a case that's explicitly covered
by this patchset. Now, do you have an actual example of CAP_SYS_RAWIO
allowing you to elevate privileges even with this patchset applied (and
without requiring the presence of obsolete hardware, which is a separate
conversation)?

> Your patches are all over the tree, for what should be one piece of
> policy. They rely on not missing a single case, which is also bad
> security engineering. We know that, as an industry we've spent fifty
> years repeatedly sticking our fingers in our ears, refusing to listen and
> then learning it again and again the hard way.

I have a set of patches that appear acceptable to the security
maintainer. I've rewritten them multiple times in response to various
levels of bikeshedding. They solve a real problem and are being shipped
by multiple distributions.

What the rest of your mail is asking me to do is to rewrite capabilities
support entirely. Now, yes, I agree that capabilities are an awful
interface that's approximately impossible to use correctly and which
adds little security even if you do. But I reject the idea that
rewriting fundamental security infrastructure should be a prerequisite
for adding this functionality.

> > It needs to be possible for this policy to be imposed without userspace
> > being involved, so using selinux would mean baking it into the kernel
> > (along with an additional implementation for apparmor, and presumably
> > one for tomoyo as well).
> 
> That's clearly false. You can load signed modules so you can load signed
> policies. Assuming you don't have an initial measured file system you
> need a kernel policy initially that protects you. If you have a measured
> file system you don't even need that, nor do you need to revoke RAWIO in
> kernel, you can do it before you switch into "measured and running
> un-measured code" mode - ie about the point you pivot root out of the
> initrd that loaded the measured policy.

We can't rely on userspace choosing to load that policy. We don't have a
measured filesystem. The initrd is not signed (and nor can it be). The
kernel itself *must* impose policy.

> > ...thus breaking userspace outside this use case. I mean, sure, I'm
> > absolutely fine with deleting /dev/mem entirely. I don't see Linus
> > agreeing.
> 
> That's not what I meant. We have a lot of interfaces where we use
> CAP_SYS_RAWIO rather than having a structured interface as opposed to a
> 'yeah whatever, poke the registers' interface.
> 
> For a lot of environments btw you don't actually need /dev/mem. It's not
> really got a lot of uses any more and plenty of people really restrict or
> remove it from systems. I don't think very many people would be sad
> if /dev/mem became a legacy interface most people left turned off.

Sure! Except then A Large Vendor's custom IPMI userspace stops working,
even on systems without Secure Boot, and then said Large Vendor wants to
have conference calls at times that are convenient for Japan and then
this is an issue that's going to cause $1 billion in lost sales and come
on, you've been there. You know that this isn't going to work. 

> If we are going to do a measured kernel then it needs to be done right,
> it needs to be properly abstracted so we don't add another one for
> Android, another one for Chrome, another one for ARM trustzone, another
> one for Intel SGX, and so on.

The fact that you keep saying measured really does make me suspect that
you misunderstand the problem. There's no measurement involved, there's
simply an assertion that the firmware (which you're forced to trust)
chose, via some policy you may be unaware of, to trust the booted
kernel.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 749 bytes --]

On Fri, 2014-03-14 at 14:11 -0400, Matthew Garrett wrote:

> The fact that you keep saying measured really does make me suspect that
> you misunderstand the problem. There's no measurement involved, there's
> simply an assertion that the firmware (which you're forced to trust)
> chose, via some policy you may be unaware of, to trust the booted
> kernel.

As an example, imagine a platform with the bootloader and kernel on
read-only media. The platform can assert that the kernel is trusted even
if there's no measurement of the kernel.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-03-14 at 14:11 -0400, Matthew Garrett wrote:

> The fact that you keep saying measured really does make me suspect that
> you misunderstand the problem. There's no measurement involved, there's
> simply an assertion that the firmware (which you're forced to trust)
> chose, via some policy you may be unaware of, to trust the booted
> kernel.

As an example, imagine a platform with the bootloader and kernel on
read-only media. The platform can assert that the kernel is trusted even
if there's no measurement of the kernel.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[David Lang]

On Fri, 14 Mar 2014, Matthew Garrett wrote:

> On Fri, 2014-03-14 at 14:11 -0400, Matthew Garrett wrote:
>
>> The fact that you keep saying measured really does make me suspect that
>> you misunderstand the problem. There's no measurement involved, there's
>> simply an assertion that the firmware (which you're forced to trust)
>> chose, via some policy you may be unaware of, to trust the booted
>> kernel.
>
> As an example, imagine a platform with the bootloader and kernel on
> read-only media. The platform can assert that the kernel is trusted even
> if there's no measurement of the kernel.

Trusted by who?

Alan is saying measured because then if it matches what the owner of that device 
intends it's trusted, but just because you trust it doesn't mean that I trust 
it, and it doesn't mean that the russian government should trust it, etc.

There just isn't one value of trust.

David Lang

Re: Trusted kernel patchset for Secure Boot lockdown

[David Lang]

On Fri, 14 Mar 2014, Matthew Garrett wrote:

> On Fri, 2014-03-14 at 14:11 -0400, Matthew Garrett wrote:
>
>> The fact that you keep saying measured really does make me suspect that
>> you misunderstand the problem. There's no measurement involved, there's
>> simply an assertion that the firmware (which you're forced to trust)
>> chose, via some policy you may be unaware of, to trust the booted
>> kernel.
>
> As an example, imagine a platform with the bootloader and kernel on
> read-only media. The platform can assert that the kernel is trusted even
> if there's no measurement of the kernel.

Trusted by who?

Alan is saying measured because then if it matches what the owner of that device 
intends it's trusted, but just because you trust it doesn't mean that I trust 
it, and it doesn't mean that the russian government should trust it, etc.

There just isn't one value of trust.

David Lang

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1067 bytes --]

On Fri, 2014-03-14 at 13:37 -0700, David Lang wrote:
> On Fri, 14 Mar 2014, Matthew Garrett wrote:
> > As an example, imagine a platform with the bootloader and kernel on
> > read-only media. The platform can assert that the kernel is trusted even
> > if there's no measurement of the kernel.
> 
> Trusted by who?

The platform. If you don't trust the platform's ability to make that
decision then that's something that informs your own behaviour, not the
platform's.

> Alan is saying measured because then if it matches what the owner of that device 
> intends it's trusted, but just because you trust it doesn't mean that I trust 
> it, and it doesn't mean that the russian government should trust it, etc.

"Measured" has a specific meaning. If you trust a file based on its
source rather than some property of the file itself, you're not
measuring it.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-03-14 at 13:37 -0700, David Lang wrote:
> On Fri, 14 Mar 2014, Matthew Garrett wrote:
> > As an example, imagine a platform with the bootloader and kernel on
> > read-only media. The platform can assert that the kernel is trusted even
> > if there's no measurement of the kernel.
> 
> Trusted by who?

The platform. If you don't trust the platform's ability to make that
decision then that's something that informs your own behaviour, not the
platform's.

> Alan is saying measured because then if it matches what the owner of that device 
> intends it's trusted, but just because you trust it doesn't mean that I trust 
> it, and it doesn't mean that the russian government should trust it, etc.

"Measured" has a specific meaning. If you trust a file based on its
source rather than some property of the file itself, you're not
measuring it.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Fri, 14 Mar 2014 19:24:55 +0000
Matthew Garrett <matthew.garrett@nebula.com> wrote:

> On Fri, 2014-03-14 at 14:11 -0400, Matthew Garrett wrote:
> 
> > The fact that you keep saying measured really does make me suspect that
> > you misunderstand the problem. There's no measurement involved, there's
> > simply an assertion that the firmware (which you're forced to trust)
> > chose, via some policy you may be unaware of, to trust the booted
> > kernel.
> 
> As an example, imagine a platform with the bootloader and kernel on
> read-only media. The platform can assert that the kernel is trusted even
> if there's no measurement of the kernel.

Only if you have a secure signed path through the controller firmware and
physical security of the hardware. If not I can reprogram your BIOS, your
GPU firmware, your USB stick or your CD-ROM controller to lie.

Anything must either be measurable or tamperproof from within the system
itself (or both). So a physically write protected ROM bootloader loading
a kernel and initrd from that same physically protected ROM is secure,
but your average CD-ROM drive is not.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Fri, 14 Mar 2014 19:24:55 +0000
Matthew Garrett <matthew.garrett@nebula.com> wrote:

> On Fri, 2014-03-14 at 14:11 -0400, Matthew Garrett wrote:
> 
> > The fact that you keep saying measured really does make me suspect that
> > you misunderstand the problem. There's no measurement involved, there's
> > simply an assertion that the firmware (which you're forced to trust)
> > chose, via some policy you may be unaware of, to trust the booted
> > kernel.
> 
> As an example, imagine a platform with the bootloader and kernel on
> read-only media. The platform can assert that the kernel is trusted even
> if there's no measurement of the kernel.

Only if you have a secure signed path through the controller firmware and
physical security of the hardware. If not I can reprogram your BIOS, your
GPU firmware, your USB stick or your CD-ROM controller to lie.

Anything must either be measurable or tamperproof from within the system
itself (or both). So a physically write protected ROM bootloader loading
a kernel and initrd from that same physically protected ROM is secure,
but your average CD-ROM drive is not.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1151 bytes --]

On Fri, 2014-03-14 at 21:58 +0000, One Thousand Gnomes wrote:
> On Fri, 14 Mar 2014 19:24:55 +0000
> Matthew Garrett <matthew.garrett@nebula.com> wrote:
> > As an example, imagine a platform with the bootloader and kernel on
> > read-only media. The platform can assert that the kernel is trusted even
> > if there's no measurement of the kernel.
> 
> Only if you have a secure signed path through the controller firmware and
> physical security of the hardware. If not I can reprogram your BIOS, your
> GPU firmware, your USB stick or your CD-ROM controller to lie.

Sure, and then the trust that the firmware placed in the kernel would be
misplaced. You can subvert Secure Boot with an SPI flasher, just as you
can subvert selinux with a firewire dongle. Those attacks are outside
the threat model. If you're in a situation where you have to care about
threats outside that threat model then you need to choose a more
appropriate solution.
-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-03-14 at 21:58 +0000, One Thousand Gnomes wrote:
> On Fri, 14 Mar 2014 19:24:55 +0000
> Matthew Garrett <matthew.garrett@nebula.com> wrote:
> > As an example, imagine a platform with the bootloader and kernel on
> > read-only media. The platform can assert that the kernel is trusted even
> > if there's no measurement of the kernel.
> 
> Only if you have a secure signed path through the controller firmware and
> physical security of the hardware. If not I can reprogram your BIOS, your
> GPU firmware, your USB stick or your CD-ROM controller to lie.

Sure, and then the trust that the firmware placed in the kernel would be
misplaced. You can subvert Secure Boot with an SPI flasher, just as you
can subvert selinux with a firewire dongle. Those attacks are outside
the threat model. If you're in a situation where you have to care about
threats outside that threat model then you need to choose a more
appropriate solution.
-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> I have a set of patches that appear acceptable to the security
> maintainer. I've rewritten them multiple times in response to various
> levels of bikeshedding. They solve a real problem and are being shipped
> by multiple distributions.

And ? I've seen some of the other "extra" stuff distributions ship. Lots
of it is hilariously bad. That really isn't a good argument for the code
- for the feature yes.

> What the rest of your mail is asking me to do is to rewrite capabilities
> support entirely. Now, yes, I agree that capabilities are an awful
> interface that's approximately impossible to use correctly and which
> adds little security even if you do. But I reject the idea that
> rewriting fundamental security infrastructure should be a prerequisite
> for adding this functionality.

Actually it's nothing of the sort. It's a proposal to do it. It's 90% a
regexp task. I'm offering to go and do that bit. I am quite happy
to do the capability crapfest fixing. It wants doing *anyway*.

But if I got do that work, does it solve your problem ? I think it does
but I'd like to be sure.

> > > It needs to be possible for this policy to be imposed without userspace
> > > being involved, so using selinux would mean baking it into the kernel
> > > (along with an additional implementation for apparmor, and presumably
> > > one for tomoyo as well).
> > 
> > That's clearly false. You can load signed modules so you can load signed
> > policies. Assuming you don't have an initial measured file system you
> > need a kernel policy initially that protects you. If you have a measured
> > file system you don't even need that, nor do you need to revoke RAWIO in
> > kernel, you can do it before you switch into "measured and running
> > un-measured code" mode - ie about the point you pivot root out of the
> > initrd that loaded the measured policy.
> 
> We can't rely on userspace choosing to load that policy. We don't have a
> measured filesystem. The initrd is not signed (and nor can it be). The
> kernel itself *must* impose policy.

In your particularly implementation maybe you've got a weak setup where
you don't measure down to your initrd. That's a *flaw* in your
implementation. Don't inflict your limitations on others or on the
future. EFI is only one (and not a very strong one at that) implementation
of a 'secure' boot chain. A lot of other systems can not only propogate
measurement and security assertions into their initrd they can propogate
them into their rootfs (yes upgrades are .. exciting, but these kinds of
users will live with that pain).

Even in EFI you can make your kernel or loader check the initrd signature
and the rootfs signature if you want.

> Sure! Except then A Large Vendor's custom IPMI userspace stops working,
> even on systems without Secure Boot, and then said Large Vendor wants to
> have conference calls at times that are convenient for Japan and then
> this is an issue that's going to cause $1 billion in lost sales and come
> on, you've been there. You know that this isn't going to work. 

That's not an upstream problem (but with my work hat on I understand
your position). Anyway they'll be running someones enterprise product
which will have legacy enabled out of the wazoo so their customers 15 year
old "we lost the code" blob that nobody can even remember what it does
still works 8)

> > If we are going to do a measured kernel then it needs to be done right,
> > it needs to be properly abstracted so we don't add another one for
> > Android, another one for Chrome, another one for ARM trustzone, another
> > one for Intel SGX, and so on.
> 
> The fact that you keep saying measured really does make me suspect that
> you misunderstand the problem. There's no measurement involved, there's
> simply an assertion that the firmware (which you're forced to trust)
> chose, via some policy you may be unaware of, to trust the booted
> kernel.

You are currently using some of those interfaces for measuring to produce
a notionally 'trusted' initial loaded environment.

Correct me if I am wrong but your starting point is "I have a chain of
measurement as far as the kernel I load". Without that I can just go into
grub and 0wn you.

There are multiple things you want to do in measured environments,
stopping people sneakily loading new kernels is one of them. If you want
to talk about "secure" that implies a whole load more than ring 0 hacks.
It implies having a model where not only has J Random smart hacker gone
over the code and hacked all the call points, but that someone has done
some kind of formal review of it, used good security practices like
whitelisting and thought very hard about the rest of the exposed surfaces.

Otherwise its security theatre, it's marketing fluff. We've got lots of
marketing security fluff in the kernel, we don't need more of it.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> I have a set of patches that appear acceptable to the security
> maintainer. I've rewritten them multiple times in response to various
> levels of bikeshedding. They solve a real problem and are being shipped
> by multiple distributions.

And ? I've seen some of the other "extra" stuff distributions ship. Lots
of it is hilariously bad. That really isn't a good argument for the code
- for the feature yes.

> What the rest of your mail is asking me to do is to rewrite capabilities
> support entirely. Now, yes, I agree that capabilities are an awful
> interface that's approximately impossible to use correctly and which
> adds little security even if you do. But I reject the idea that
> rewriting fundamental security infrastructure should be a prerequisite
> for adding this functionality.

Actually it's nothing of the sort. It's a proposal to do it. It's 90% a
regexp task. I'm offering to go and do that bit. I am quite happy
to do the capability crapfest fixing. It wants doing *anyway*.

But if I got do that work, does it solve your problem ? I think it does
but I'd like to be sure.

> > > It needs to be possible for this policy to be imposed without userspace
> > > being involved, so using selinux would mean baking it into the kernel
> > > (along with an additional implementation for apparmor, and presumably
> > > one for tomoyo as well).
> > 
> > That's clearly false. You can load signed modules so you can load signed
> > policies. Assuming you don't have an initial measured file system you
> > need a kernel policy initially that protects you. If you have a measured
> > file system you don't even need that, nor do you need to revoke RAWIO in
> > kernel, you can do it before you switch into "measured and running
> > un-measured code" mode - ie about the point you pivot root out of the
> > initrd that loaded the measured policy.
> 
> We can't rely on userspace choosing to load that policy. We don't have a
> measured filesystem. The initrd is not signed (and nor can it be). The
> kernel itself *must* impose policy.

In your particularly implementation maybe you've got a weak setup where
you don't measure down to your initrd. That's a *flaw* in your
implementation. Don't inflict your limitations on others or on the
future. EFI is only one (and not a very strong one at that) implementation
of a 'secure' boot chain. A lot of other systems can not only propogate
measurement and security assertions into their initrd they can propogate
them into their rootfs (yes upgrades are .. exciting, but these kinds of
users will live with that pain).

Even in EFI you can make your kernel or loader check the initrd signature
and the rootfs signature if you want.

> Sure! Except then A Large Vendor's custom IPMI userspace stops working,
> even on systems without Secure Boot, and then said Large Vendor wants to
> have conference calls at times that are convenient for Japan and then
> this is an issue that's going to cause $1 billion in lost sales and come
> on, you've been there. You know that this isn't going to work. 

That's not an upstream problem (but with my work hat on I understand
your position). Anyway they'll be running someones enterprise product
which will have legacy enabled out of the wazoo so their customers 15 year
old "we lost the code" blob that nobody can even remember what it does
still works 8)

> > If we are going to do a measured kernel then it needs to be done right,
> > it needs to be properly abstracted so we don't add another one for
> > Android, another one for Chrome, another one for ARM trustzone, another
> > one for Intel SGX, and so on.
> 
> The fact that you keep saying measured really does make me suspect that
> you misunderstand the problem. There's no measurement involved, there's
> simply an assertion that the firmware (which you're forced to trust)
> chose, via some policy you may be unaware of, to trust the booted
> kernel.

You are currently using some of those interfaces for measuring to produce
a notionally 'trusted' initial loaded environment.

Correct me if I am wrong but your starting point is "I have a chain of
measurement as far as the kernel I load". Without that I can just go into
grub and 0wn you.

There are multiple things you want to do in measured environments,
stopping people sneakily loading new kernels is one of them. If you want
to talk about "secure" that implies a whole load more than ring 0 hacks.
It implies having a model where not only has J Random smart hacker gone
over the code and hacked all the call points, but that someone has done
some kind of formal review of it, used good security practices like
whitelisting and thought very hard about the rest of the exposed surfaces.

Otherwise its security theatre, it's marketing fluff. We've got lots of
marketing security fluff in the kernel, we don't need more of it.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2016 bytes --]

On Fri, 2014-03-14 at 21:48 +0000, One Thousand Gnomes wrote:

> In your particularly implementation maybe you've got a weak setup where
> you don't measure down to your initrd. That's a *flaw* in your
> implementation. Don't inflict your limitations on others or on the
> future. EFI is only one (and not a very strong one at that) implementation
> of a 'secure' boot chain. A lot of other systems can not only propogate
> measurement and security assertions into their initrd they can propogate
> them into their rootfs (yes upgrades are .. exciting, but these kinds of
> users will live with that pain).

Signed userspace is not a requirement, and therefore any solution that
relies on a signed initrd is inadequate. There are use cases that
require verification of the initrd and other levels. This isn't one of
them.

> Even in EFI you can make your kernel or loader check the initrd signature
> and the rootfs signature if you want.

Except the initramfs gets built at kernel install time.
 
> > The fact that you keep saying measured really does make me suspect that
> > you misunderstand the problem. There's no measurement involved, there's
> > simply an assertion that the firmware (which you're forced to trust)
> > chose, via some policy you may be unaware of, to trust the booted
> > kernel.
> 
> You are currently using some of those interfaces for measuring to produce
> a notionally 'trusted' initial loaded environment.
> 
> Correct me if I am wrong but your starting point is "I have a chain of
> measurement as far as the kernel I load". Without that I can just go into
> grub and 0wn you.

In my use case. But not all implementations will be measuring things -
they can assert that the kernel is trustworthy through some other
mechanism. This genuinely is about trust, not measurement.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-03-14 at 21:48 +0000, One Thousand Gnomes wrote:

> In your particularly implementation maybe you've got a weak setup where
> you don't measure down to your initrd. That's a *flaw* in your
> implementation. Don't inflict your limitations on others or on the
> future. EFI is only one (and not a very strong one at that) implementation
> of a 'secure' boot chain. A lot of other systems can not only propogate
> measurement and security assertions into their initrd they can propogate
> them into their rootfs (yes upgrades are .. exciting, but these kinds of
> users will live with that pain).

Signed userspace is not a requirement, and therefore any solution that
relies on a signed initrd is inadequate. There are use cases that
require verification of the initrd and other levels. This isn't one of
them.

> Even in EFI you can make your kernel or loader check the initrd signature
> and the rootfs signature if you want.

Except the initramfs gets built at kernel install time.
 
> > The fact that you keep saying measured really does make me suspect that
> > you misunderstand the problem. There's no measurement involved, there's
> > simply an assertion that the firmware (which you're forced to trust)
> > chose, via some policy you may be unaware of, to trust the booted
> > kernel.
> 
> You are currently using some of those interfaces for measuring to produce
> a notionally 'trusted' initial loaded environment.
> 
> Correct me if I am wrong but your starting point is "I have a chain of
> measurement as far as the kernel I load". Without that I can just go into
> grub and 0wn you.

In my use case. But not all implementations will be measuring things -
they can assert that the kernel is trustworthy through some other
mechanism. This genuinely is about trust, not measurement.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Fri, 14 Mar 2014 21:56:33 +0000
Matthew Garrett <matthew.garrett@nebula.com> wrote:

> On Fri, 2014-03-14 at 21:48 +0000, One Thousand Gnomes wrote:
> 
> > In your particularly implementation maybe you've got a weak setup where
> > you don't measure down to your initrd. That's a *flaw* in your
> > implementation. Don't inflict your limitations on others or on the
> > future. EFI is only one (and not a very strong one at that) implementation
> > of a 'secure' boot chain. A lot of other systems can not only propogate
> > measurement and security assertions into their initrd they can propogate
> > them into their rootfs (yes upgrades are .. exciting, but these kinds of
> > users will live with that pain).
> 
> Signed userspace is not a requirement, and therefore any solution that
> relies on a signed initrd is inadequate. There are use cases that
> require verification of the initrd and other levels. This isn't one of
> them.

The job of the kernel is to solve the general problem. There are lots of
people who happen to care about verification beyond the kernel so it
shouldn't be ignored. And they can do do things like load trusted SELinux
rulesets even if you can't support it in your environment.

> > Even in EFI you can make your kernel or loader check the initrd signature
> > and the rootfs signature if you want.
> 
> Except the initramfs gets built at kernel install time.

Implementation detail for your use case.

> > Correct me if I am wrong but your starting point is "I have a chain of
> > measurement as far as the kernel I load". Without that I can just go into
> > grub and 0wn you.
> 
> In my use case. But not all implementations will be measuring things -
> they can assert that the kernel is trustworthy through some other
> mechanism. This genuinely is about trust, not measurement.

The assertion you attempt to achieve is I believe

"No ring 0 code is executed directly or indirectly that is not measured"

Some of your measuring is EFI boot, some is module signing and then you
must impose a security model as well.

It's a "measurement" problem if you ask what the rule is - yes ? Getting
there is not just a measurement problem but your intended result is a
measurement based rule ?

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Fri, 14 Mar 2014 21:56:33 +0000
Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:

> On Fri, 2014-03-14 at 21:48 +0000, One Thousand Gnomes wrote:
> 
> > In your particularly implementation maybe you've got a weak setup where
> > you don't measure down to your initrd. That's a *flaw* in your
> > implementation. Don't inflict your limitations on others or on the
> > future. EFI is only one (and not a very strong one at that) implementation
> > of a 'secure' boot chain. A lot of other systems can not only propogate
> > measurement and security assertions into their initrd they can propogate
> > them into their rootfs (yes upgrades are .. exciting, but these kinds of
> > users will live with that pain).
> 
> Signed userspace is not a requirement, and therefore any solution that
> relies on a signed initrd is inadequate. There are use cases that
> require verification of the initrd and other levels. This isn't one of
> them.

The job of the kernel is to solve the general problem. There are lots of
people who happen to care about verification beyond the kernel so it
shouldn't be ignored. And they can do do things like load trusted SELinux
rulesets even if you can't support it in your environment.

> > Even in EFI you can make your kernel or loader check the initrd signature
> > and the rootfs signature if you want.
> 
> Except the initramfs gets built at kernel install time.

Implementation detail for your use case.

> > Correct me if I am wrong but your starting point is "I have a chain of
> > measurement as far as the kernel I load". Without that I can just go into
> > grub and 0wn you.
> 
> In my use case. But not all implementations will be measuring things -
> they can assert that the kernel is trustworthy through some other
> mechanism. This genuinely is about trust, not measurement.

The assertion you attempt to achieve is I believe

"No ring 0 code is executed directly or indirectly that is not measured"

Some of your measuring is EFI boot, some is module signing and then you
must impose a security model as well.

It's a "measurement" problem if you ask what the rule is - yes ? Getting
there is not just a measurement problem but your intended result is a
measurement based rule ?

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2183 bytes --]

On Fri, 2014-03-14 at 22:08 +0000, One Thousand Gnomes wrote:
> On Fri, 14 Mar 2014 21:56:33 +0000
> Matthew Garrett <matthew.garrett@nebula.com> wrote:
> > Signed userspace is not a requirement, and therefore any solution that
> > relies on a signed initrd is inadequate. There are use cases that
> > require verification of the initrd and other levels. This isn't one of
> > them.
> 
> The job of the kernel is to solve the general problem. There are lots of
> people who happen to care about verification beyond the kernel so it
> shouldn't be ignored. And they can do do things like load trusted SELinux
> rulesets even if you can't support it in your environment.

The general problem includes having to support this even without an
selinux policy.

> > > Even in EFI you can make your kernel or loader check the initrd signature
> > > and the rootfs signature if you want.
> > 
> > Except the initramfs gets built at kernel install time.
> 
> Implementation detail for your use case.

And one that's not going to change, so the general problem includes not
relying on a signed initramfs.

> > > Correct me if I am wrong but your starting point is "I have a chain of
> > > measurement as far as the kernel I load". Without that I can just go into
> > > grub and 0wn you.
> > 
> > In my use case. But not all implementations will be measuring things -
> > they can assert that the kernel is trustworthy through some other
> > mechanism. This genuinely is about trust, not measurement.
> 
> The assertion you attempt to achieve is I believe
> 
> "No ring 0 code is executed directly or indirectly that is not measured"

No. As I keep pointing out, not all code is measured. The firmware is
not required to measure itself. A particular implementation may skip
measuring the kernel because it can attest to its trustworthyness in
some other way. ChromeOS will load unmeasured kernel modules provided it
can attest to the trustworthyness of the filesystem containing them.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-03-14 at 22:08 +0000, One Thousand Gnomes wrote:
> On Fri, 14 Mar 2014 21:56:33 +0000
> Matthew Garrett <matthew.garrett@nebula.com> wrote:
> > Signed userspace is not a requirement, and therefore any solution that
> > relies on a signed initrd is inadequate. There are use cases that
> > require verification of the initrd and other levels. This isn't one of
> > them.
> 
> The job of the kernel is to solve the general problem. There are lots of
> people who happen to care about verification beyond the kernel so it
> shouldn't be ignored. And they can do do things like load trusted SELinux
> rulesets even if you can't support it in your environment.

The general problem includes having to support this even without an
selinux policy.

> > > Even in EFI you can make your kernel or loader check the initrd signature
> > > and the rootfs signature if you want.
> > 
> > Except the initramfs gets built at kernel install time.
> 
> Implementation detail for your use case.

And one that's not going to change, so the general problem includes not
relying on a signed initramfs.

> > > Correct me if I am wrong but your starting point is "I have a chain of
> > > measurement as far as the kernel I load". Without that I can just go into
> > > grub and 0wn you.
> > 
> > In my use case. But not all implementations will be measuring things -
> > they can assert that the kernel is trustworthy through some other
> > mechanism. This genuinely is about trust, not measurement.
> 
> The assertion you attempt to achieve is I believe
> 
> "No ring 0 code is executed directly or indirectly that is not measured"

No. As I keep pointing out, not all code is measured. The firmware is
not required to measure itself. A particular implementation may skip
measuring the kernel because it can attest to its trustworthyness in
some other way. ChromeOS will load unmeasured kernel modules provided it
can attest to the trustworthyness of the filesystem containing them.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Fri, 14 Mar 2014 22:15:45 +0000
Matthew Garrett <matthew.garrett@nebula.com> wrote:

> On Fri, 2014-03-14 at 22:08 +0000, One Thousand Gnomes wrote:
> > On Fri, 14 Mar 2014 21:56:33 +0000
> > Matthew Garrett <matthew.garrett@nebula.com> wrote:
> > > Signed userspace is not a requirement, and therefore any solution that
> > > relies on a signed initrd is inadequate. There are use cases that
> > > require verification of the initrd and other levels. This isn't one of
> > > them.
> > 
> > The job of the kernel is to solve the general problem. There are lots of
> > people who happen to care about verification beyond the kernel so it
> > shouldn't be ignored. And they can do do things like load trusted SELinux
> > rulesets even if you can't support it in your environment.
> 
> The general problem includes having to support this even without an
> selinux policy.

Yes. No dispute about that. But equally the general solution should allow
for it.

> And one that's not going to change, so the general problem includes not
> relying on a signed initramfs.

Likewise

> some other way. ChromeOS will load unmeasured kernel modules provided it
> can attest to the trustworthyness of the filesystem containing them.

See "How to Bypass Verified Boot Security in Chromium OS" 8)

And it attests the trustworthiness of the filesystem by measuring it. If
you have a measurement of object X that states it is unchanged then you
have a valid measurement of any subset of object X for which the same
assertion is proven. In this case since you know all the bits in the root
fs are as before, so you know all the bits in the module are as before

And how do you know all the bits in the root fs are as before, because you
have a set of measurements (hashes) on partition 12. At the end of the
day you end up with a chain of measurements from a trusted thing you deep
immutable. If your chain has gaps you have holes (see above).

So ChromeOS loads *measured* kernel modules. It just did the measuring
differently to the signed module code.

Alan


 

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

On Fri, 14 Mar 2014 22:15:45 +0000
Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:

> On Fri, 2014-03-14 at 22:08 +0000, One Thousand Gnomes wrote:
> > On Fri, 14 Mar 2014 21:56:33 +0000
> > Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
> > > Signed userspace is not a requirement, and therefore any solution that
> > > relies on a signed initrd is inadequate. There are use cases that
> > > require verification of the initrd and other levels. This isn't one of
> > > them.
> > 
> > The job of the kernel is to solve the general problem. There are lots of
> > people who happen to care about verification beyond the kernel so it
> > shouldn't be ignored. And they can do do things like load trusted SELinux
> > rulesets even if you can't support it in your environment.
> 
> The general problem includes having to support this even without an
> selinux policy.

Yes. No dispute about that. But equally the general solution should allow
for it.

> And one that's not going to change, so the general problem includes not
> relying on a signed initramfs.

Likewise

> some other way. ChromeOS will load unmeasured kernel modules provided it
> can attest to the trustworthyness of the filesystem containing them.

See "How to Bypass Verified Boot Security in Chromium OS" 8)

And it attests the trustworthiness of the filesystem by measuring it. If
you have a measurement of object X that states it is unchanged then you
have a valid measurement of any subset of object X for which the same
assertion is proven. In this case since you know all the bits in the root
fs are as before, so you know all the bits in the module are as before

And how do you know all the bits in the root fs are as before, because you
have a set of measurements (hashes) on partition 12. At the end of the
day you end up with a chain of measurements from a trusted thing you deep
immutable. If your chain has gaps you have holes (see above).

So ChromeOS loads *measured* kernel modules. It just did the measuring
differently to the signed module code.

Alan


 

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1434 bytes --]

On Fri, 2014-03-14 at 22:31 +0000, One Thousand Gnomes wrote:
> On Fri, 14 Mar 2014 22:15:45 +0000
> Matthew Garrett <matthew.garrett@nebula.com> wrote:
> > The general problem includes having to support this even without an
> > selinux policy.
> 
> Yes. No dispute about that. But equally the general solution should allow
> for it.

Well, sure. The current implementation doesn't conflict with selinux in
any way.

> > some other way. ChromeOS will load unmeasured kernel modules provided it
> > can attest to the trustworthyness of the filesystem containing them.
> 
> See "How to Bypass Verified Boot Security in Chromium OS" 8)
> 
> And it attests the trustworthiness of the filesystem by measuring it. If
> you have a measurement of object X that states it is unchanged then you
> have a valid measurement of any subset of object X for which the same
> assertion is proven. In this case since you know all the bits in the root
> fs are as before, so you know all the bits in the module are as before

You may attest to the trustworthiness of a filesystem by measuring it,
but you may also attest to it via some other means - for instance, it's
read-only and stored on media that requires physical presence to
modify. 

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Fri, 2014-03-14 at 22:31 +0000, One Thousand Gnomes wrote:
> On Fri, 14 Mar 2014 22:15:45 +0000
> Matthew Garrett <matthew.garrett@nebula.com> wrote:
> > The general problem includes having to support this even without an
> > selinux policy.
> 
> Yes. No dispute about that. But equally the general solution should allow
> for it.

Well, sure. The current implementation doesn't conflict with selinux in
any way.

> > some other way. ChromeOS will load unmeasured kernel modules provided it
> > can attest to the trustworthyness of the filesystem containing them.
> 
> See "How to Bypass Verified Boot Security in Chromium OS" 8)
> 
> And it attests the trustworthiness of the filesystem by measuring it. If
> you have a measurement of object X that states it is unchanged then you
> have a valid measurement of any subset of object X for which the same
> assertion is proven. In this case since you know all the bits in the root
> fs are as before, so you know all the bits in the module are as before

You may attest to the trustworthiness of a filesystem by measuring it,
but you may also attest to it via some other means - for instance, it's
read-only and stored on media that requires physical presence to
modify. 

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

Re: Trusted kernel patchset for Secure Boot lockdown

[Kees Cook]

On Fri, Mar 14, 2014 at 3:31 PM, One Thousand Gnomes
<gnomes@lxorguk.ukuu.org.uk> wrote:
> On Fri, 14 Mar 2014 22:15:45 +0000
> Matthew Garrett <matthew.garrett@nebula.com> wrote:
>
>> On Fri, 2014-03-14 at 22:08 +0000, One Thousand Gnomes wrote:
>> > On Fri, 14 Mar 2014 21:56:33 +0000
>> > Matthew Garrett <matthew.garrett@nebula.com> wrote:
>> > > Signed userspace is not a requirement, and therefore any solution that
>> > > relies on a signed initrd is inadequate. There are use cases that
>> > > require verification of the initrd and other levels. This isn't one of
>> > > them.
>> >
>> > The job of the kernel is to solve the general problem. There are lots of
>> > people who happen to care about verification beyond the kernel so it
>> > shouldn't be ignored. And they can do do things like load trusted SELinux
>> > rulesets even if you can't support it in your environment.
>>
>> The general problem includes having to support this even without an
>> selinux policy.
>
> Yes. No dispute about that. But equally the general solution should allow
> for it.
>
>> And one that's not going to change, so the general problem includes not
>> relying on a signed initramfs.
>
> Likewise
>
>> some other way. ChromeOS will load unmeasured kernel modules provided it
>> can attest to the trustworthyness of the filesystem containing them.
>
> See "How to Bypass Verified Boot Security in Chromium OS" 8)

That method a) is intentionally available (system owner can disable
firmware RO and install their own keys), and b) requires physical
presence.

> So ChromeOS loads *measured* kernel modules. It just did the measuring
> differently to the signed module code.

Right, using dm-verity. However, the read-only media case is still
valid (and is why I am still trying to get the module restrictions LSM
accepted, and why I'm modelling my firmware restrictions on the same
principle).

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[Kees Cook]

On Fri, Mar 14, 2014 at 3:31 PM, One Thousand Gnomes
<gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org> wrote:
> On Fri, 14 Mar 2014 22:15:45 +0000
> Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
>
>> On Fri, 2014-03-14 at 22:08 +0000, One Thousand Gnomes wrote:
>> > On Fri, 14 Mar 2014 21:56:33 +0000
>> > Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org> wrote:
>> > > Signed userspace is not a requirement, and therefore any solution that
>> > > relies on a signed initrd is inadequate. There are use cases that
>> > > require verification of the initrd and other levels. This isn't one of
>> > > them.
>> >
>> > The job of the kernel is to solve the general problem. There are lots of
>> > people who happen to care about verification beyond the kernel so it
>> > shouldn't be ignored. And they can do do things like load trusted SELinux
>> > rulesets even if you can't support it in your environment.
>>
>> The general problem includes having to support this even without an
>> selinux policy.
>
> Yes. No dispute about that. But equally the general solution should allow
> for it.
>
>> And one that's not going to change, so the general problem includes not
>> relying on a signed initramfs.
>
> Likewise
>
>> some other way. ChromeOS will load unmeasured kernel modules provided it
>> can attest to the trustworthyness of the filesystem containing them.
>
> See "How to Bypass Verified Boot Security in Chromium OS" 8)

That method a) is intentionally available (system owner can disable
firmware RO and install their own keys), and b) requires physical
presence.

> So ChromeOS loads *measured* kernel modules. It just did the measuring
> differently to the signed module code.

Right, using dm-verity. However, the read-only media case is still
valid (and is why I am still trying to get the module restrictions LSM
accepted, and why I'm modelling my firmware restrictions on the same
principle).

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[Theodore Ts'o]

On Fri, Mar 14, 2014 at 10:08:40PM +0000, One Thousand Gnomes wrote:
> > Signed userspace is not a requirement, and therefore any solution that
> > relies on a signed initrd is inadequate. There are use cases that
> > require verification of the initrd and other levels. This isn't one of
> > them.
> 
> The job of the kernel is to solve the general problem. There are lots of
> people who happen to care about verification beyond the kernel so it
> shouldn't be ignored. And they can do do things like load trusted SELinux
> rulesets even if you can't support it in your environment.

This is really a question about goals and trust models.  Alan is
arguing that we should support trust models and goals which go far
beyond the goals and trust model of the UEFI Forum.

Matthew is, I think, trying to make the argument that his patches
fulfill the goals that are needed so we can boot Linux distribution
kernels on UEFI secure boot machines without worrying about Microsoft
deciding to revoke keys so that Red Hat or SuSE will no longer be able
to be bootable on desktops that are certified for Windows 8.  And
while we might want to improve the framework to support other trust
models later on, supporting distro kernels on Windows 8 certified PC's
is important enough that we should let these patches into mainline.

Is that a fair summary of the two viewpoints?

Personally, I think that we are fortunate that Windows 8 has been
enough of a train wreck that huge numbers of users have been taking
Windows 8 systems and upgrading them to Windows 7, and hence the need
for Distro kernels that can boot on fully locked down Windows 8 PC's.
But at some point, whether it is a few years or a decade later (if
Windows 7 lives on as long as XP :-), Windows 7 will be EOL'ed, and
even before that, UEFI secure boot will be enabled by default.

Right now, even though Lenovo laptops are shipping with Windows
8. UEFI secure boot is not made mandatory (although it is on enough to
brick the laptop when it runs into bugs wwith the UEFI BIOS code,
sigh).  But sooner or later, UEFI secure boot will be on by default,
and then if Linux distros don't have kernels where the installer can
be run without needing to twiddle BIOS settings, it might make it
harder for the "Year of the Desktop" to come about.

So as far as the narrow question of whether we should accept these
patches, I think it's a good thing.  Personally, I'm always going to
be disabling UEFI secure boot (even if it doesn't brick my laptop),
because for me, the security guarantees it provides isn't worth it.
But there will be people who want to be able to install Linux on
Windows 8 certified PC's without tweaking BIOS settings, so merging
the UEFI secure boot is a good thing, so long as those of use who
don't want to have anything to do with UEFI secure boot can disable
it.

Regards,

					- Ted

Re: Trusted kernel patchset for Secure Boot lockdown

[Theodore Ts'o]

On Fri, Mar 14, 2014 at 10:08:40PM +0000, One Thousand Gnomes wrote:
> > Signed userspace is not a requirement, and therefore any solution that
> > relies on a signed initrd is inadequate. There are use cases that
> > require verification of the initrd and other levels. This isn't one of
> > them.
> 
> The job of the kernel is to solve the general problem. There are lots of
> people who happen to care about verification beyond the kernel so it
> shouldn't be ignored. And they can do do things like load trusted SELinux
> rulesets even if you can't support it in your environment.

This is really a question about goals and trust models.  Alan is
arguing that we should support trust models and goals which go far
beyond the goals and trust model of the UEFI Forum.

Matthew is, I think, trying to make the argument that his patches
fulfill the goals that are needed so we can boot Linux distribution
kernels on UEFI secure boot machines without worrying about Microsoft
deciding to revoke keys so that Red Hat or SuSE will no longer be able
to be bootable on desktops that are certified for Windows 8.  And
while we might want to improve the framework to support other trust
models later on, supporting distro kernels on Windows 8 certified PC's
is important enough that we should let these patches into mainline.

Is that a fair summary of the two viewpoints?

Personally, I think that we are fortunate that Windows 8 has been
enough of a train wreck that huge numbers of users have been taking
Windows 8 systems and upgrading them to Windows 7, and hence the need
for Distro kernels that can boot on fully locked down Windows 8 PC's.
But at some point, whether it is a few years or a decade later (if
Windows 7 lives on as long as XP :-), Windows 7 will be EOL'ed, and
even before that, UEFI secure boot will be enabled by default.

Right now, even though Lenovo laptops are shipping with Windows
8. UEFI secure boot is not made mandatory (although it is on enough to
brick the laptop when it runs into bugs wwith the UEFI BIOS code,
sigh).  But sooner or later, UEFI secure boot will be on by default,
and then if Linux distros don't have kernels where the installer can
be run without needing to twiddle BIOS settings, it might make it
harder for the "Year of the Desktop" to come about.

So as far as the narrow question of whether we should accept these
patches, I think it's a good thing.  Personally, I'm always going to
be disabling UEFI secure boot (even if it doesn't brick my laptop),
because for me, the security guarantees it provides isn't worth it.
But there will be people who want to be able to install Linux on
Windows 8 certified PC's without tweaking BIOS settings, so merging
the UEFI secure boot is a good thing, so long as those of use who
don't want to have anything to do with UEFI secure boot can disable
it.

Regards,

					- Ted

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> So as far as the narrow question of whether we should accept these
> patches, I think it's a good thing.  Personally, I'm always going to
> be disabling UEFI secure boot (even if it doesn't brick my laptop),
> because for me, the security guarantees it provides isn't worth it.
> But there will be people who want to be able to install Linux on
> Windows 8 certified PC's without tweaking BIOS settings, so merging
> the UEFI secure boot is a good thing, so long as those of use who
> don't want to have anything to do with UEFI secure boot can disable
> it.

I definitely think we want the feature and there are a lot of non UEFI
reasons for this (eg running trusted_kernel() virtual namespaces). I have
three specific issues

1. The implementation is a mess in part because it propogates more policy
all over the place that should be separated. Root cause capable() mixes
policy and activity. Fix suggested in my previous emails (and offer to do
the work)

2. It's likely to lead to more bugs and errors because of the way it has
been done and it doesn't break old code that gets added without
considering the issue. It fails insecure which is bad. Fixed by doing
what I suggested (and offered to do)

3. For things like module options we should be white not blacklisting
'bad' ones.

I've offered to go and fix up the capability stuff - I'm just waiting for
Matthew to actually confirm the question I specifically asked him - does
this solve that bit of his problem. If it does great, I'll go and sort
the capability bits out so we can keep the policy in the right place and
we don't have the kernel festooned with && !trusted_kernel() everywhere.

There is a question of completeness but its very clear we get there with
a combination of two things - whitelisting so we catch stuff we missed
rather than leave holes, and just accepting the reality that it'll take a
few kernels once its upstream until we get them all.

I care about security, we should do the job properly. We have a
further magnitude shift in security needs coming that's going to be at
least equivalent to the scale of shift between the old 'university,
everyone is nice' internet and today.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> So as far as the narrow question of whether we should accept these
> patches, I think it's a good thing.  Personally, I'm always going to
> be disabling UEFI secure boot (even if it doesn't brick my laptop),
> because for me, the security guarantees it provides isn't worth it.
> But there will be people who want to be able to install Linux on
> Windows 8 certified PC's without tweaking BIOS settings, so merging
> the UEFI secure boot is a good thing, so long as those of use who
> don't want to have anything to do with UEFI secure boot can disable
> it.

I definitely think we want the feature and there are a lot of non UEFI
reasons for this (eg running trusted_kernel() virtual namespaces). I have
three specific issues

1. The implementation is a mess in part because it propogates more policy
all over the place that should be separated. Root cause capable() mixes
policy and activity. Fix suggested in my previous emails (and offer to do
the work)

2. It's likely to lead to more bugs and errors because of the way it has
been done and it doesn't break old code that gets added without
considering the issue. It fails insecure which is bad. Fixed by doing
what I suggested (and offered to do)

3. For things like module options we should be white not blacklisting
'bad' ones.

I've offered to go and fix up the capability stuff - I'm just waiting for
Matthew to actually confirm the question I specifically asked him - does
this solve that bit of his problem. If it does great, I'll go and sort
the capability bits out so we can keep the policy in the right place and
we don't have the kernel festooned with && !trusted_kernel() everywhere.

There is a question of completeness but its very clear we get there with
a combination of two things - whitelisting so we catch stuff we missed
rather than leave holes, and just accepting the reality that it'll take a
few kernels once its upstream until we get them all.

I care about security, we should do the job properly. We have a
further magnitude shift in security needs coming that's going to be at
least equivalent to the scale of shift between the old 'university,
everyone is nice' internet and today.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[Florian Weimer]

* Theodore Ts'o:

> Right now, even though Lenovo laptops are shipping with Windows
> 8. UEFI secure boot is not made mandatory (although it is on enough to
> brick the laptop when it runs into bugs wwith the UEFI BIOS code,
> sigh).  But sooner or later, UEFI secure boot will be on by default,
> and then if Linux distros don't have kernels where the installer can
> be run without needing to twiddle BIOS settings, it might make it
> harder for the "Year of the Desktop" to come about.

Windows 8 logo devices already enable Secure Boot by default.

One aspect which makes all this really tricky is that Microsoft is
watching what we're doing and will keep raising the bar, probably not
with the intent to lock us out completely, but sufficiently high to
make things quite annoying.  For example, any certificate-signing
certificate in the boot process needs to be an EV CA certificate,
which comes with fairly stringent requirements that are quite costly
to implement.

So any restrictions we implement as a good-will gesture will
eventually come back to haunt us.

Re: Trusted kernel patchset for Secure Boot lockdown

[Florian Weimer]

* Theodore Ts'o:

> Right now, even though Lenovo laptops are shipping with Windows
> 8. UEFI secure boot is not made mandatory (although it is on enough to
> brick the laptop when it runs into bugs wwith the UEFI BIOS code,
> sigh).  But sooner or later, UEFI secure boot will be on by default,
> and then if Linux distros don't have kernels where the installer can
> be run without needing to twiddle BIOS settings, it might make it
> harder for the "Year of the Desktop" to come about.

Windows 8 logo devices already enable Secure Boot by default.

One aspect which makes all this really tricky is that Microsoft is
watching what we're doing and will keep raising the bar, probably not
with the intent to lock us out completely, but sufficiently high to
make things quite annoying.  For example, any certificate-signing
certificate in the boot process needs to be an EV CA certificate,
which comes with fairly stringent requirements that are quite costly
to implement.

So any restrictions we implement as a good-will gesture will
eventually come back to haunt us.

Re: Trusted kernel patchset for Secure Boot lockdown

[Kees Cook]

On Fri, Mar 14, 2014 at 4:18 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Fri, Mar 14, 2014 at 10:08:40PM +0000, One Thousand Gnomes wrote:
>> > Signed userspace is not a requirement, and therefore any solution that
>> > relies on a signed initrd is inadequate. There are use cases that
>> > require verification of the initrd and other levels. This isn't one of
>> > them.
>>
>> The job of the kernel is to solve the general problem. There are lots of
>> people who happen to care about verification beyond the kernel so it
>> shouldn't be ignored. And they can do do things like load trusted SELinux
>> rulesets even if you can't support it in your environment.
>
> This is really a question about goals and trust models.  Alan is
> arguing that we should support trust models and goals which go far
> beyond the goals and trust model of the UEFI Forum.
>
> Matthew is, I think, trying to make the argument that his patches
> fulfill the goals that are needed so we can boot Linux distribution
> kernels on UEFI secure boot machines without worrying about Microsoft
> deciding to revoke keys so that Red Hat or SuSE will no longer be able
> to be bootable on desktops that are certified for Windows 8.  And
> while we might want to improve the framework to support other trust
> models later on, supporting distro kernels on Windows 8 certified PC's
> is important enough that we should let these patches into mainline.
>
> Is that a fair summary of the two viewpoints?

UEFI is a red herring in both cases. This isn't about UEFI, it just
happens that one of the things that can assert "trusted_kernel" is the
UEFI Secure Boot path. Chrome OS would assert "trusted_kernel" by
passing it on the kernel command line (since it, along with firmware,
kernel, and root fs are effectively measured). A
boot-my-router-from-CD system can assert similarly because the kernel
is on RO media.

Based on my view of the conversation, it's about whether or not
capable() can be made to do these checks. The proposal about creating
the action/privilege map is interesting, and I'm all for doing it just
to make things easy to reason about for capabilities, but it still
seems separate from this work.

There still needs to be a global boolean for the "trusted kernel"
state. It would be used, for example, on the module param
whitelisting, which has nothing to do with capabilities. It would be
used to change driver behavior (e.g. disabling DMA or other badness
that isn't trusted), which has nothing to do with capabilities. The
ability to check this state is going to be needed going into the
future as we uncover more dangerous interfaces.

Since the capability work is separate, when it happens, that same
regex work could replace some of the is_trusted_kernel checks with new
action tests, but we still need the same infrastructure and
identification of dangerous interfaces.

Capabilities can be seen as related to this patch set, but it cannot
be seen as a blocker. This logic is needed today, it's implemented,
and it clearly marks where the known problems are. If an overhaul of
capabilities happens, it can happen separately.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[Kees Cook]

On Fri, Mar 14, 2014 at 4:18 PM, Theodore Ts'o <tytso-3s7WtUTddSA@public.gmane.org> wrote:
> On Fri, Mar 14, 2014 at 10:08:40PM +0000, One Thousand Gnomes wrote:
>> > Signed userspace is not a requirement, and therefore any solution that
>> > relies on a signed initrd is inadequate. There are use cases that
>> > require verification of the initrd and other levels. This isn't one of
>> > them.
>>
>> The job of the kernel is to solve the general problem. There are lots of
>> people who happen to care about verification beyond the kernel so it
>> shouldn't be ignored. And they can do do things like load trusted SELinux
>> rulesets even if you can't support it in your environment.
>
> This is really a question about goals and trust models.  Alan is
> arguing that we should support trust models and goals which go far
> beyond the goals and trust model of the UEFI Forum.
>
> Matthew is, I think, trying to make the argument that his patches
> fulfill the goals that are needed so we can boot Linux distribution
> kernels on UEFI secure boot machines without worrying about Microsoft
> deciding to revoke keys so that Red Hat or SuSE will no longer be able
> to be bootable on desktops that are certified for Windows 8.  And
> while we might want to improve the framework to support other trust
> models later on, supporting distro kernels on Windows 8 certified PC's
> is important enough that we should let these patches into mainline.
>
> Is that a fair summary of the two viewpoints?

UEFI is a red herring in both cases. This isn't about UEFI, it just
happens that one of the things that can assert "trusted_kernel" is the
UEFI Secure Boot path. Chrome OS would assert "trusted_kernel" by
passing it on the kernel command line (since it, along with firmware,
kernel, and root fs are effectively measured). A
boot-my-router-from-CD system can assert similarly because the kernel
is on RO media.

Based on my view of the conversation, it's about whether or not
capable() can be made to do these checks. The proposal about creating
the action/privilege map is interesting, and I'm all for doing it just
to make things easy to reason about for capabilities, but it still
seems separate from this work.

There still needs to be a global boolean for the "trusted kernel"
state. It would be used, for example, on the module param
whitelisting, which has nothing to do with capabilities. It would be
used to change driver behavior (e.g. disabling DMA or other badness
that isn't trusted), which has nothing to do with capabilities. The
ability to check this state is going to be needed going into the
future as we uncover more dangerous interfaces.

Since the capability work is separate, when it happens, that same
regex work could replace some of the is_trusted_kernel checks with new
action tests, but we still need the same infrastructure and
identification of dangerous interfaces.

Capabilities can be seen as related to this patch set, but it cannot
be seen as a blocker. This logic is needed today, it's implemented,
and it clearly marks where the known problems are. If an overhaul of
capabilities happens, it can happen separately.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> Capabilities can be seen as related to this patch set, but it cannot
> be seen as a blocker. This logic is needed today, it's implemented,
> and it clearly marks where the known problems are. If an overhaul of
> capabilities happens, it can happen separately.

A working version is needed today - but the implementation assumes things
like the idea you can blacklist a few boot parameters and all is good. The
reality is the reverse. So this at the moment is security theatre at
least for the EFI and PC case.

At the minimum you need to do module and kernel command line parameter
whitelisting/signing.

It's also rather odd to suggest the solution is to splatter changes all
over the kernel causing mass churn and maintainer headache... so that we
can do the whole mass churn again fixing what was added.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> Capabilities can be seen as related to this patch set, but it cannot
> be seen as a blocker. This logic is needed today, it's implemented,
> and it clearly marks where the known problems are. If an overhaul of
> capabilities happens, it can happen separately.

A working version is needed today - but the implementation assumes things
like the idea you can blacklist a few boot parameters and all is good. The
reality is the reverse. So this at the moment is security theatre at
least for the EFI and PC case.

At the minimum you need to do module and kernel command line parameter
whitelisting/signing.

It's also rather odd to suggest the solution is to splatter changes all
over the kernel causing mass churn and maintainer headache... so that we
can do the whole mass churn again fixing what was added.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[tytso]

On Wed, Mar 19, 2014 at 01:16:15PM -0700, Kees Cook wrote:
> UEFI is a red herring in both cases. This isn't about UEFI, it just
> happens that one of the things that can assert "trusted_kernel" is the
> UEFI Secure Boot path. Chrome OS would assert "trusted_kernel" by
> passing it on the kernel command line (since it, along with firmware,
> kernel, and root fs are effectively measured). A
> boot-my-router-from-CD system can assert similarly because the kernel
> is on RO media.

I disagree; it's highly likely, if not certain that Windows booting
under UEFI secure boot is going to be able to do some of the things
that people are proposing that we have to prohibit in the name of
security.  That's because presumably Windows won't be willing to make
certain usability tradeoffs, and since they control the signing certs,
even in the unlikely case that people can leverage these "holes" to
enable a boot sector virus, it seems unlikely that Windows will revoke
its own cert.

The security goals for Windows' secure boot is quite a bit weaker than
what ChromeOS is trying to promise; this is why I claim the real
argument is over what the goals are for "trusted boot".

Cheers,

					- Ted

Re: Trusted kernel patchset for Secure Boot lockdown

[tytso]

On Wed, Mar 19, 2014 at 01:16:15PM -0700, Kees Cook wrote:
> UEFI is a red herring in both cases. This isn't about UEFI, it just
> happens that one of the things that can assert "trusted_kernel" is the
> UEFI Secure Boot path. Chrome OS would assert "trusted_kernel" by
> passing it on the kernel command line (since it, along with firmware,
> kernel, and root fs are effectively measured). A
> boot-my-router-from-CD system can assert similarly because the kernel
> is on RO media.

I disagree; it's highly likely, if not certain that Windows booting
under UEFI secure boot is going to be able to do some of the things
that people are proposing that we have to prohibit in the name of
security.  That's because presumably Windows won't be willing to make
certain usability tradeoffs, and since they control the signing certs,
even in the unlikely case that people can leverage these "holes" to
enable a boot sector virus, it seems unlikely that Windows will revoke
its own cert.

The security goals for Windows' secure boot is quite a bit weaker than
what ChromeOS is trying to promise; this is why I claim the real
argument is over what the goals are for "trusted boot".

Cheers,

					- Ted

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1315 bytes --]

On Thu, 2014-03-20 at 10:55 -0400, tytso@mit.edu wrote:

> I disagree; it's highly likely, if not certain that Windows booting
> under UEFI secure boot is going to be able to do some of the things
> that people are proposing that we have to prohibit in the name of
> security.  That's because presumably Windows won't be willing to make
> certain usability tradeoffs, and since they control the signing certs,
> even in the unlikely case that people can leverage these "holes" to
> enable a boot sector virus, it seems unlikely that Windows will revoke
> its own cert.

I don't think any of the functionality we're disabling (with the
arguable exception of kexec, which, again, there is a plan to handle) is
useful on modern systems. And, seriously, if this forces vendors to
write actual kernel drivers rather than run an io port banging IPMI
driver in userspace, that's a *good* thing.

Whether Microsoft would actually follow through on blacklisting their
own signatures is obviously an unknown - they've told us they would, but
commercial concerns etc who knows. They *will* blacklist our signatures.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Thu, 2014-03-20 at 10:55 -0400, tytso@mit.edu wrote:

> I disagree; it's highly likely, if not certain that Windows booting
> under UEFI secure boot is going to be able to do some of the things
> that people are proposing that we have to prohibit in the name of
> security.  That's because presumably Windows won't be willing to make
> certain usability tradeoffs, and since they control the signing certs,
> even in the unlikely case that people can leverage these "holes" to
> enable a boot sector virus, it seems unlikely that Windows will revoke
> its own cert.

I don't think any of the functionality we're disabling (with the
arguable exception of kexec, which, again, there is a plan to handle) is
useful on modern systems. And, seriously, if this forces vendors to
write actual kernel drivers rather than run an io port banging IPMI
driver in userspace, that's a *good* thing.

Whether Microsoft would actually follow through on blacklisting their
own signatures is obviously an unknown - they've told us they would, but
commercial concerns etc who knows. They *will* blacklist our signatures.

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

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> Whether Microsoft would actually follow through on blacklisting their
> own signatures is obviously an unknown - they've told us they would, but
> commercial concerns etc who knows. They *will* blacklist our signatures.

I think that becomes an irrelevant debate. It's going to end up being
argued in a court by lawyers some day and its not software problem. One
day some bright spark from MS will decide to do things like enforce
patent disputes this way or commercial pressures will lead them to
try and find some other excuse to do it.

It's never going to be "secure", so they'll always be able to find an
excuse.

The functionality you have to disable is for the most part quite boring
for desktop users. Server may see it differently because you cripple a
lot of debugging work. OTOH many of them probably want to turn it on for
production boxes.

The main thing you lose are lots of module options, the ability to force
addresses for things like the serial port console (otherwise I can force
an address and root the kernel that way), mem=, custom ACPI tables and so
on.

It's the stuff that lets you get a box with crapware as firmware working
that's really hit.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> Whether Microsoft would actually follow through on blacklisting their
> own signatures is obviously an unknown - they've told us they would, but
> commercial concerns etc who knows. They *will* blacklist our signatures.

I think that becomes an irrelevant debate. It's going to end up being
argued in a court by lawyers some day and its not software problem. One
day some bright spark from MS will decide to do things like enforce
patent disputes this way or commercial pressures will lead them to
try and find some other excuse to do it.

It's never going to be "secure", so they'll always be able to find an
excuse.

The functionality you have to disable is for the most part quite boring
for desktop users. Server may see it differently because you cripple a
lot of debugging work. OTOH many of them probably want to turn it on for
production boxes.

The main thing you lose are lots of module options, the ability to force
addresses for things like the serial port console (otherwise I can force
an address and root the kernel that way), mem=, custom ACPI tables and so
on.

It's the stuff that lets you get a box with crapware as firmware working
that's really hit.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> On the other hand, disabling CAP_SYS_RAWIO *definitely* breaks expected
> functionality - firmware loading and the fibmap ioctl are probably the
> most obvious. And changing the use of CAP_SYS_RAWIO potentially breaks
> userspace expectations, so we're kind of stuck there.

Actually I know how to describe the problem better.

Whitelist v Blacklist.

Going around adding extra cases for CAP_SYS_RAWIO is a fails insecure
model. Going around adding CAP_SYS_RAWIO || CAP_SYS_RAWIO_SEC is a 'fails
secure' case.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[One Thousand Gnomes]

> On the other hand, disabling CAP_SYS_RAWIO *definitely* breaks expected
> functionality - firmware loading and the fibmap ioctl are probably the
> most obvious. And changing the use of CAP_SYS_RAWIO potentially breaks
> userspace expectations, so we're kind of stuck there.

Actually I know how to describe the problem better.

Whitelist v Blacklist.

Going around adding extra cases for CAP_SYS_RAWIO is a fails insecure
model. Going around adding CAP_SYS_RAWIO || CAP_SYS_RAWIO_SEC is a 'fails
secure' case.

Alan

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 903 bytes --]

On Thu, 2014-03-13 at 21:26 +0000, One Thousand Gnomes wrote:
> > On the other hand, disabling CAP_SYS_RAWIO *definitely* breaks expected
> > functionality - firmware loading and the fibmap ioctl are probably the
> > most obvious. And changing the use of CAP_SYS_RAWIO potentially breaks
> > userspace expectations, so we're kind of stuck there.
> 
> Actually I know how to describe the problem better.
> 
> Whitelist v Blacklist.
> 
> Going around adding extra cases for CAP_SYS_RAWIO is a fails insecure
> model. Going around adding CAP_SYS_RAWIO || CAP_SYS_RAWIO_SEC is a 'fails
> secure' case.

We've already been through this. We can't add new capabilities. It
breaks existing userspace.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Thu, 2014-03-13 at 21:26 +0000, One Thousand Gnomes wrote:
> > On the other hand, disabling CAP_SYS_RAWIO *definitely* breaks expected
> > functionality - firmware loading and the fibmap ioctl are probably the
> > most obvious. And changing the use of CAP_SYS_RAWIO potentially breaks
> > userspace expectations, so we're kind of stuck there.
> 
> Actually I know how to describe the problem better.
> 
> Whitelist v Blacklist.
> 
> Going around adding extra cases for CAP_SYS_RAWIO is a fails insecure
> model. Going around adding CAP_SYS_RAWIO || CAP_SYS_RAWIO_SEC is a 'fails
> secure' case.

We've already been through this. We can't add new capabilities. It
breaks existing userspace.

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

Trusted kernel patchset

[Matthew Garrett]

This is a slightly cleaned up version of the patchset posted last year
(https://lkml.org/lkml/2014/2/26/554). I've made a couple of minor changes
based on feedback, but otherwise this is pretty much the same. Some things
I *haven't* done:

1) Disabled CAP_SYS_RAWIO

Disabling CAP_SYS_RAWIO has the fun side effect of disabling things like
CPU microcode loading. Given that the microcode is already signed and the
CPU validates that, this isn't helpful. There's just too many cases where
CAP_SYS_RAWIO is required for features that are outside the scope of ensuring
that the kernel can't be tampered with. In addition, there are features that
don't require CAP_SYS_RAWIO which should be blocked.

2) Disabled CAP_SYS_RAWIO but whitelisted specific CAP_SYS_RAWIO features

This was Alan's suggestion - change capable() to check whether the capability
requested was blacklisted, and then add a capable_always() that ignored the
blacklist. In this scenario, /dev/mem would still use capable(CAP_SYS_RAWIO)
and would be denied, but the microcode loader would use
capable_always(CAP_SYS_RAWIO) and would be permitted. This changes certain
behavioural expectations (eg, having CAP_SYS_RAWIO would no longer be
sufficient to pass capable(CAP_SYS_RAWIO)) and would still hit the problem
of features that should be blocked but don't currently require CAP_SYS_RAWIO.
I think this is fundamentally more confusing than the approach I've
implemented.

3) Done one of the above and added new CAP_SYS_RAWIO checks

This would handle the case of features that should be blocked but which don't
currently require CAP_SYS_RAWIO, but would break any userspace that has
dropped privileges and expects to be able to use these features even in the
case that this feature isn't enabled. I don't think it's beneficial.

4) Used the word "measured"

Nothing is being measured.

A patchset basically equivalent to this is already used by most major Linux
distributions, so it would be nice to either get this merged or have feedback
from a relevant maintainer as to how they'd like it to be implemented instead.

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

[PATCH 01/12] Add support for indicating that the booted kernel is externally trusted

[Matthew Garrett]

Provide a boolean runtime configuration option for restricting userspace's
ability to modify the running kernel. This can be used when some external
validation of the kernel's state has been performed.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
---
 Documentation/kernel-parameters.txt       |   6 ++
 Documentation/security/trusted_kernel.txt |  35 ++++++++++
 include/linux/security.h                  |  10 +++
 security/Kconfig                          |   9 +++
 security/Makefile                         |   1 +
 security/trusted_kernel.c                 | 111 ++++++++++++++++++++++++++++++
 6 files changed, 172 insertions(+)
 create mode 100644 Documentation/security/trusted_kernel.txt
 create mode 100644 security/trusted_kernel.c

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index bfcb1a6..2d2aa946 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3647,6 +3647,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			with respect to transparent hugepages.
 			See Documentation/vm/transhuge.txt for more details.
 
+	trusted_kernel	Indicate that the booted kernel has been verified to
+			be trustworthy and that userspace should be forbidden
+			from modifying it at runtime.
+			See Documentation/security/trusted_kernel.txt for more
+			details.
+
 	tsc=		Disable clocksource stability checks for TSC.
 			Format: <string>
 			[x86] reliable: mark tsc clocksource as reliable, this
diff --git a/Documentation/security/trusted_kernel.txt b/Documentation/security/trusted_kernel.txt
new file mode 100644
index 0000000..95300bb
--- /dev/null
+++ b/Documentation/security/trusted_kernel.txt
@@ -0,0 +1,35 @@
+Linux trusted kernel support
+----------------------------
+
+Various mechanisms exist to ensure that a booted kernel is trusted by the
+user or some external party (UEFI Secure Boot, Intel TXT, embedded platform
+bootloaders). If userspace is able to modify the running kernel then this
+trust can be subverted.
+
+The trusted kernel support modifies certain kernel interfaces such that
+userspace is restricted from performing acts that would allow it to inject
+untrusted code into the kernel. 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.
+
+The trusted kernel feature may be enabled in multiple ways:
+
+1) Platform-specific code may automatically enable it when it detects that
+the system has been booted appropriately
+
+2) The user or bootloader may pass the "trusted_kernel" kernel parameter
+
+3) Userspace may write "1" to the /sys/kernel/security/trusted_kernel
+node. This must be done sufficiently early in the boot process that
+untrusted userspace has no opportunity to modify the kernel.
+
+Once enabled, trusted kernel support may not be disabled without rebooting
+the system.
+
+Note that this is a mechanism for the kernel to determine whether or not
+it is externally trusted. Untrusted userspace can enable this option even
+if the kernel is not trusted, and therefore userspace should not use this
+value as an indication of whether or not the kernel is trustworthy.
diff --git a/include/linux/security.h b/include/linux/security.h
index a1b7dbd..87bbbc0 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -3174,6 +3174,16 @@ static inline void security_audit_rule_free(void *lsmrule)
 #endif /* CONFIG_SECURITY */
 #endif /* CONFIG_AUDIT */
 
+#ifdef CONFIG_SECURITY_TRUSTED_KERNEL
+extern bool get_trusted_kernel(void);
+extern int set_trusted_kernel(bool new_trusted_kernel);
+#else
+static inline bool get_trusted_kernel(void) { return 0; }
+static inline int set_trusted_kernel(bool new_trusted_kernel) {
+	return -ENOTSUPP;
+}
+#endif /* CONFIG_SECURITY_TRUSTED_KERNEL */
+
 #ifdef CONFIG_SECURITYFS
 
 extern struct dentry *securityfs_create_file(const char *name, umode_t mode,
diff --git a/security/Kconfig b/security/Kconfig
index beb86b5..c0462c9 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -70,6 +70,15 @@ config SECURITY_PATH
 	  implement pathname based access controls.
 	  If you are unsure how to answer this question, answer N.
 
+config SECURITY_TRUSTED_KERNEL
+        bool "Support for indicating that the kernel is trusted"
+	depends on SECURITY
+	help
+	  This enables support for adding a set of additional kernel security
+	  restrictions at runtime.
+	  See Documentation/security/trusted_kernel.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 05f1c93..0d5d689 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_TRUSTED_KERNEL)	+= trusted_kernel.o
 obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/
 obj-$(CONFIG_SECURITY_SMACK)		+= smack/
 obj-$(CONFIG_AUDIT)			+= lsm_audit.o
diff --git a/security/trusted_kernel.c b/security/trusted_kernel.c
new file mode 100644
index 0000000..2808113
--- /dev/null
+++ b/security/trusted_kernel.c
@@ -0,0 +1,111 @@
+/*
+ *  trusted_kernel.c - support for generic kernel lockdown
+ *
+ *  Copyright Nebula, Inc <matthew.garrett@nebula.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/security.h>
+#include <linux/uaccess.h>
+
+static bool trusted_kernel;
+
+bool get_trusted_kernel(void)
+{
+	return trusted_kernel;
+}
+EXPORT_SYMBOL(get_trusted_kernel);
+
+int set_trusted_kernel(bool new_trusted_kernel)
+{
+	if (trusted_kernel == true && new_trusted_kernel == false)
+		return -EINVAL;
+
+	trusted_kernel = new_trusted_kernel;
+
+	return 0;
+}
+EXPORT_SYMBOL(set_trusted_kernel);
+
+static ssize_t trusted_kernel_read(struct file *filp, char __user *buf,
+				   size_t count, loff_t *ppos)
+{
+	char tmpbuf[2];
+	ssize_t length;
+
+	length = scnprintf(tmpbuf, sizeof(tmpbuf), "%d", trusted_kernel);
+	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
+}
+
+static ssize_t trusted_kernel_write(struct file *file, const char __user *buf,
+				    size_t count, loff_t *ppos)
+{
+	char *page = NULL;
+	ssize_t length;
+	int new_trusted_kernel;
+
+	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_trusted_kernel) != 1)
+		goto out;
+
+	length = set_trusted_kernel(!!new_trusted_kernel);
+	if (length)
+		goto out;
+
+	length = count;
+out:
+	free_page((unsigned long) page);
+	return length;
+}
+
+static const struct file_operations trusted_kernel_fops = {
+	.read	= trusted_kernel_read,
+	.write	= trusted_kernel_write,
+	.llseek	= generic_file_llseek,
+};
+
+static __init int setup_trusted_kernel(void)
+{
+	struct dentry *trusted_kernel_file;
+
+	trusted_kernel_file = securityfs_create_file("trusted_kernel",
+						     S_IWUSR | S_IRUGO,
+						     NULL, NULL,
+						     &trusted_kernel_fops);
+
+	if (IS_ERR(trusted_kernel_file))
+		return PTR_ERR(trusted_kernel_file);
+
+	return 0;
+}
+late_initcall(setup_trusted_kernel);
+
+static int __init enable_trusted_kernel(char *__str)
+{
+	trusted_kernel = true;
+	return 1;
+}
+__setup("trusted_kernel", enable_trusted_kernel);
-- 
2.1.0