Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1365682 > unrolled thread

[PATCH v2 5/5] LSM: LoadPin for kernel file loading restrictions

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 5/5] LSM: LoadPin for kernel file loading restrictions Kees Cook <keescook@chromium.org> - 2016-03-28 23:20 +0200
    Re: [PATCH v2 5/5] LSM: LoadPin for kernel file loading  restrictions Andrew Morton <akpm@linux-foundation.org> - 2016-03-28 23:40 +0200
      Re: [PATCH v2 5/5] LSM: LoadPin for kernel file loading restrictions Kees Cook <keescook@chromium.org> - 2016-03-29 00:00 +0200
      Re: [PATCH v2 5/5] LSM: LoadPin for kernel file loading restrictions Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-03-30 22:30 +0200
    Re: [PATCH v2 5/5] LSM: LoadPin for kernel file loading  restrictions Joe Perches <joe@perches.com> - 2016-03-29 01:30 +0200
    Re: [PATCH v2 5/5] LSM: LoadPin for kernel file loading restrictions Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-03-31 23:30 +0200

#1365682 — [PATCH v2 5/5] LSM: LoadPin for kernel file loading restrictions

This LSM enforces that kernel-loaded files (modules, firmware, etc)
must all come from the same filesystem, with the expectation that
such a filesystem is backed by a read-only device such as dm-verity
or CDROM. This allows systems that have a verified and/or unchangeable
filesystem to enforce module and firmware loading restrictions without
needing to sign the files individually.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 MAINTAINERS                |   6 ++
 include/linux/lsm_hooks.h  |   5 ++
 security/Kconfig           |   1 +
 security/Makefile          |   2 +
 security/loadpin/Kconfig   |  10 +++
 security/loadpin/Makefile  |   1 +
 security/loadpin/loadpin.c | 206 +++++++++++++++++++++++++++++++++++++++++++++
 security/security.c        |   1 +
 8 files changed, 232 insertions(+)
 create mode 100644 security/loadpin/Kconfig
 create mode 100644 security/loadpin/Makefile
 create mode 100644 security/loadpin/loadpin.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f07a174bbc81..06ab115a2066 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9937,6 +9937,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/jj/apparmor-dev.git
 S:	Supported
 F:	security/apparmor/
 
+LOADPIN SECURITY MODULE
+M:	Kees Cook <keescook@chromium.org>
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git lsm/loadpin
+S:	Supported
+F:	security/loadpin/
+
 YAMA SECURITY MODULE
 M:	Kees Cook <keescook@chromium.org>
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git yama/tip
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index cdee11cbcdf1..f3402aab1927 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1893,5 +1893,10 @@ extern void __init yama_add_hooks(void);
 #else
 static inline void __init yama_add_hooks(void) { }
 #endif
+#ifdef CONFIG_SECURITY_LOADPIN
+void __init loadpin_add_hooks(void);
+#else
+static inline void loadpin_add_hooks(void) { };
+#endif
 
 #endif /* ! __LINUX_LSM_HOOKS_H */
diff --git a/security/Kconfig b/security/Kconfig
index e45237897b43..176758cdfa57 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -122,6 +122,7 @@ source security/selinux/Kconfig
 source security/smack/Kconfig
 source security/tomoyo/Kconfig
 source security/apparmor/Kconfig
+source security/loadpin/Kconfig
 source security/yama/Kconfig
 
 source security/integrity/Kconfig
diff --git a/security/Makefile b/security/Makefile
index c9bfbc84ff50..f2d71cdb8e19 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -8,6 +8,7 @@ subdir-$(CONFIG_SECURITY_SMACK)		+= smack
 subdir-$(CONFIG_SECURITY_TOMOYO)        += tomoyo
 subdir-$(CONFIG_SECURITY_APPARMOR)	+= apparmor
 subdir-$(CONFIG_SECURITY_YAMA)		+= yama
+subdir-$(CONFIG_SECURITY_LOADPIN)	+= loadpin
 
 # always enable default capabilities
 obj-y					+= commoncap.o
@@ -22,6 +23,7 @@ obj-$(CONFIG_AUDIT)			+= lsm_audit.o
 obj-$(CONFIG_SECURITY_TOMOYO)		+= tomoyo/
 obj-$(CONFIG_SECURITY_APPARMOR)		+= apparmor/
 obj-$(CONFIG_SECURITY_YAMA)		+= yama/
+obj-$(CONFIG_SECURITY_LOADPIN)		+= loadpin/
 obj-$(CONFIG_CGROUP_DEVICE)		+= device_cgroup.o
 
 # Object integrity file lists
diff --git a/security/loadpin/Kconfig b/security/loadpin/Kconfig
new file mode 100644
index 000000000000..c668ac4eda65
--- /dev/null
+++ b/security/loadpin/Kconfig
@@ -0,0 +1,10 @@
+config SECURITY_LOADPIN
+	bool "Pin load of kernel files (modules, fw, etc) to one filesystem"
+	depends on SECURITY && BLOCK
+	help
+	  Any files read through the kernel file reading interface
+	  (kernel modules, firmware, kexec images, security policy) will
+	  be pinned to the first filesystem used for loading. Any files
+	  that come from other filesystems will be rejected. This is best
+	  used on systems without an initrd that have a root filesystem
+	  backed by a read-only device such as dm-verity or a CDROM.
diff --git a/security/loadpin/Makefile b/security/loadpin/Makefile
new file mode 100644
index 000000000000..c2d77f83037b
--- /dev/null
+++ b/security/loadpin/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SECURITY_LOADPIN) += loadpin.o
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
new file mode 100644
index 000000000000..04aa8ee41610
--- /dev/null
+++ b/security/loadpin/loadpin.c
@@ -0,0 +1,206 @@
+/*
+ * Module and Firmware Pinning Security Module
+ *
+ * Copyright 2011-2016 Google Inc.
+ *
+ * Author: Kees Cook <keescook@chromium.org>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt) "LoadPin: " fmt
+
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/fs_struct.h>
+#include <linux/lsm_hooks.h>
+#include <linux/mount.h>
+#include <linux/path.h>
+#include <linux/sched.h>	/* current */
+#include <linux/string_helpers.h>
+
+static void report_load(const char *origin, struct file *file, char *operation)
+{
+	char *cmdline, *pathname;
+
+	pathname = kstrdup_quotable_file(file);
+	cmdline = kstrdup_quotable_cmdline(current);
+
+	pr_notice("%s %s obj=%s%s%s pid=%d cmdline=%s%s%s\n",
+		  origin, operation,
+		  (pathname && pathname[0] != '<') ? "\"" : "",
+		  pathname,
+		  (pathname && pathname[0] != '<') ? "\"" : "",
+		  task_pid_nr(current),
+		  cmdline ? "\"" : "", cmdline, cmdline ? "\"" : "");
+
+	kfree(cmdline);
+	kfree(pathname);
+}
+
+static int load_pinning = 1;
+static struct super_block *pinned_root;
+static DEFINE_SPINLOCK(pinned_root_spinlock);
+
+#ifdef CONFIG_SYSCTL
+static int zero;
+static int one = 1;
+
+static struct ctl_path loadpin_sysctl_path[] = {
+	{ .procname = "kernel", },
+	{ }
+};
+
+static struct ctl_table loadpin_sysctl_table[] = {
+	{
+		.procname       = "load_pinning",
+		.data           = &load_pinning,
+		.maxlen         = sizeof(int),
+		.mode           = 0644,
+		.proc_handler   = proc_dointvec_minmax,
+		.extra1         = &zero,
+		.extra2         = &one,
+	},
+	{ }
+};
+
+/*
+ * This must be called after early kernel init, since then the rootdev
+ * is available.
+ */
+static void check_pinning_enforcement(struct super_block *mnt_sb)
+{
+	bool ro = false;
+
+	/*
+	 * If load pinning is not enforced via a read-only block
+	 * device, allow sysctl to change modes for testing.
+	 */
+	if (mnt_sb->s_bdev) {
+		ro = bdev_read_only(mnt_sb->s_bdev);
+		pr_info("dev(%u,%u): %s\n",
+			MAJOR(mnt_sb->s_bdev->bd_dev),
+			MINOR(mnt_sb->s_bdev->bd_dev),
+			ro ? "read-only" : "writable");
+	} else
+		pr_info("mnt_sb lacks block device, treating as: writable\n");
+
+	if (!ro) {
+		if (!register_sysctl_paths(loadpin_sysctl_path,
+					   loadpin_sysctl_table))
+			pr_notice("sysctl registration failed!\n");
+		else
+			pr_info("load pinning can be disabled.\n");
+	} else
+		pr_info("load pinning engaged.\n");
+}
+#else
+static void check_pinning_enforcement(struct super_block *mnt_sb)
+{
+	pr_info("load pinning engaged.\n");
+}
+#endif
+
+static void loadpin_sb_free_security(struct super_block *mnt_sb)
+{
+	/*
+	 * When unmounting the filesystem we were using for load
+	 * pinning, we acknowledge the superblock release, but make sure
+	 * no other modules or firmware can be loaded.
+	 */
+	if (!IS_ERR_OR_NULL(pinned_root) && mnt_sb == pinned_root) {
+		pinned_root = ERR_PTR(-EIO);
+		pr_info("umount pinned fs: refusing further loads\n");
+	}
+}
+
+static const char *id_str[READING_MAX_ID] = {
+	[READING_FIRMWARE] = "firmware",
+	[READING_MODULE] = "kernel module",
+	[READING_KEXEC_IMAGE] = "kexec image",
+	[READING_KEXEC_INITRAMFS] = "kexec initramfs",
+	[READING_POLICY] = "security policy",
+};
+
+static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
+{
+	struct super_block *load_root;
+	const char *origin;
+
+	if (id < 0 || id >= READING_MAX_ID)
+		return -EINVAL;
+
+	origin = id_str[id];
+	if (!origin) {
+		origin = "unknown";
+		pr_warn_once("unknown kernel_read_file_id %d\n", id);
+	}
+
+	/* This handles the older init_module API that has a NULL file. */
+	if (!file) {
+		if (!load_pinning) {
+			report_load(origin, NULL, "old-api-pinning-ignored");
+			return 0;
+		}
+
+		report_load(origin, NULL, "old-api-denied");
+		return -EPERM;
+	}
+
+	load_root = file->f_path.mnt->mnt_sb;
+
+	/* First loaded module/firmware defines the root for all others. */
+	spin_lock(&pinned_root_spinlock);
+	/*
+	 * pinned_root is only NULL at startup. Otherwise, it is either
+	 * a valid reference, or an ERR_PTR.
+	 */
+	if (!pinned_root) {
+		pinned_root = load_root;
+		/*
+		 * Unlock now since it's only pinned_root we care about.
+		 * In the worst case, we will (correctly) report pinning
+		 * failures before we have announced that pinning is
+		 * enabled. This would be purely cosmetic.
+		 */
+		spin_unlock(&pinned_root_spinlock);
+		check_pinning_enforcement(pinned_root);
+		report_load(origin, file, "pinned");
+	} else {
+		spin_unlock(&pinned_root_spinlock);
+	}
+
+	if (IS_ERR_OR_NULL(pinned_root) || load_root != pinned_root) {
+		if (unlikely(!load_pinning)) {
+			report_load(origin, file, "pinning-ignored");
+			return 0;
+		}
+
+		report_load(origin, file, "denied");
+		return -EPERM;
+	}
+
+	return 0;
+}
+
+static struct security_hook_list loadpin_hooks[] = {
+	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
+	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
+};
+
+void __init loadpin_add_hooks(void)
+{
+	pr_info("preparing to pin");
+	security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks));
+}
+
+/* Should not be mutable after boot, so not listed in sysfs (perm == 0). */
+module_param(load_pinning, int, 0);
+MODULE_PARM_DESC(load_pinning, "Pin module/firmware loading (default: true)");
diff --git a/security/security.c b/security/security.c
index 3644b0344d29..ce02178c892f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -60,6 +60,7 @@ int __init security_init(void)
 	 */
 	capability_add_hooks();
 	yama_add_hooks();
+	loadpin_add_hooks();
 
 	/*
 	 * Load all the remaining security modules.
-- 
2.6.3

[toc] | [next] | [standalone]

#1365695 — Re: [PATCH v2 5/5] LSM: LoadPin for kernel file loading restrictions

On Mon, 28 Mar 2016 14:14:22 -0700 Kees Cook <keescook@chromium.org> wrote:

> This LSM enforces that kernel-loaded files (modules, firmware, etc)
> must all come from the same filesystem, with the expectation that
> such a filesystem is backed by a read-only device such as dm-verity
> or CDROM. This allows systems that have a verified and/or unchangeable
> filesystem to enforce module and firmware loading restrictions without
> needing to sign the files individually.

Patchset generally looks good to me.  It's regrettable that a load of
stuff was added to lib/ for one obscure LSM but hopefully (doubtfully)
someone else will find a use for some of it.

I'll assume that James is handling all of this.

> --- /dev/null
> +++ b/security/loadpin/loadpin.c
> @@ -0,0 +1,206 @@
> +/*
> + * Module and Firmware Pinning Security Module
> + *
> + * Copyright 2011-2016 Google Inc.
> + *
> + * Author: Kees Cook <keescook@chromium.org>
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#define pr_fmt(fmt) "LoadPin: " fmt
> +
> +#include <linux/module.h>
> +#include <linux/fs.h>
> +#include <linux/fs_struct.h>
> +#include <linux/lsm_hooks.h>
> +#include <linux/mount.h>
> +#include <linux/path.h>
> +#include <linux/sched.h>	/* current */
> +#include <linux/string_helpers.h>
> +
> +static void report_load(const char *origin, struct file *file, char *operation)
> +{
> +	char *cmdline, *pathname;
> +
> +	pathname = kstrdup_quotable_file(file);
> +	cmdline = kstrdup_quotable_cmdline(current);
> +
> +	pr_notice("%s %s obj=%s%s%s pid=%d cmdline=%s%s%s\n",
> +		  origin, operation,
> +		  (pathname && pathname[0] != '<') ? "\"" : "",
> +		  pathname,
> +		  (pathname && pathname[0] != '<') ? "\"" : "",
> +		  task_pid_nr(current),
> +		  cmdline ? "\"" : "", cmdline, cmdline ? "\"" : "");
> +
> +	kfree(cmdline);
> +	kfree(pathname);
> +}
> +
> +static int load_pinning = 1;
> +static struct super_block *pinned_root;
> +static DEFINE_SPINLOCK(pinned_root_spinlock);
> +
> +#ifdef CONFIG_SYSCTL
> +static int zero;
> +static int one = 1;
> +
> +static struct ctl_path loadpin_sysctl_path[] = {
> +	{ .procname = "kernel", },
> +	{ }
> +};
> +
> +static struct ctl_table loadpin_sysctl_table[] = {
> +	{
> +		.procname       = "load_pinning",
> +		.data           = &load_pinning,
> +		.maxlen         = sizeof(int),
> +		.mode           = 0644,
> +		.proc_handler   = proc_dointvec_minmax,
> +		.extra1         = &zero,
> +		.extra2         = &one,
> +	},
> +	{ }
> +};

There should be somewhere to document the new sysctl?

[toc] | [prev] | [next] | [standalone]

#1365698

On Mon, Mar 28, 2016 at 2:38 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Mon, 28 Mar 2016 14:14:22 -0700 Kees Cook <keescook@chromium.org> wrote:
>
>> This LSM enforces that kernel-loaded files (modules, firmware, etc)
>> must all come from the same filesystem, with the expectation that
>> such a filesystem is backed by a read-only device such as dm-verity
>> or CDROM. This allows systems that have a verified and/or unchangeable
>> filesystem to enforce module and firmware loading restrictions without
>> needing to sign the files individually.
>
> Patchset generally looks good to me.  It's regrettable that a load of
> stuff was added to lib/ for one obscure LSM but hopefully (doubtfully)
> someone else will find a use for some of it.
>
> I'll assume that James is handling all of this.
>
>> --- /dev/null
>> +++ b/security/loadpin/loadpin.c
>> @@ -0,0 +1,206 @@
>> +/*
>> + * Module and Firmware Pinning Security Module
>> + *
>> + * Copyright 2011-2016 Google Inc.
>> + *
>> + * Author: Kees Cook <keescook@chromium.org>
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#define pr_fmt(fmt) "LoadPin: " fmt
>> +
>> +#include <linux/module.h>
>> +#include <linux/fs.h>
>> +#include <linux/fs_struct.h>
>> +#include <linux/lsm_hooks.h>
>> +#include <linux/mount.h>
>> +#include <linux/path.h>
>> +#include <linux/sched.h>     /* current */
>> +#include <linux/string_helpers.h>
>> +
>> +static void report_load(const char *origin, struct file *file, char *operation)
>> +{
>> +     char *cmdline, *pathname;
>> +
>> +     pathname = kstrdup_quotable_file(file);
>> +     cmdline = kstrdup_quotable_cmdline(current);
>> +
>> +     pr_notice("%s %s obj=%s%s%s pid=%d cmdline=%s%s%s\n",
>> +               origin, operation,
>> +               (pathname && pathname[0] != '<') ? "\"" : "",
>> +               pathname,
>> +               (pathname && pathname[0] != '<') ? "\"" : "",
>> +               task_pid_nr(current),
>> +               cmdline ? "\"" : "", cmdline, cmdline ? "\"" : "");
>> +
>> +     kfree(cmdline);
>> +     kfree(pathname);
>> +}
>> +
>> +static int load_pinning = 1;
>> +static struct super_block *pinned_root;
>> +static DEFINE_SPINLOCK(pinned_root_spinlock);
>> +
>> +#ifdef CONFIG_SYSCTL
>> +static int zero;
>> +static int one = 1;
>> +
>> +static struct ctl_path loadpin_sysctl_path[] = {
>> +     { .procname = "kernel", },
>> +     { }
>> +};
>> +
>> +static struct ctl_table loadpin_sysctl_table[] = {
>> +     {
>> +             .procname       = "load_pinning",
>> +             .data           = &load_pinning,
>> +             .maxlen         = sizeof(int),
>> +             .mode           = 0644,
>> +             .proc_handler   = proc_dointvec_minmax,
>> +             .extra1         = &zero,
>> +             .extra2         = &one,
>> +     },
>> +     { }
>> +};
>
> There should be somewhere to document the new sysctl?

Good call. I'll send a follow-up with a Documentation patch.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1367593

On Mon, 2016-03-28 at 14:38 -0700, Andrew Morton wrote:
> On Mon, 28 Mar 2016 14:14:22 -0700 Kees Cook <keescook@chromium.org> wrote:
> 
> > This LSM enforces that kernel-loaded files (modules, firmware, etc)
> > must all come from the same filesystem, with the expectation that
> > such a filesystem is backed by a read-only device such as dm-verity
> > or CDROM. This allows systems that have a verified and/or unchangeable
> > filesystem to enforce module and firmware loading restrictions without
> > needing to sign the files individually.
> 
> Patchset generally looks good to me.  It's regrettable that a load of
> stuff was added to lib/ for one obscure LSM but hopefully (doubtfully)
> someone else will find a use for some of it.

I'm planning on adding support for measuring buffers, like the boot
command line, which will need to be string safe.

Mimi

[toc] | [prev] | [next] | [standalone]

#1365716 — Re: [PATCH v2 5/5] LSM: LoadPin for kernel file loading restrictions

On Mon, 2016-03-28 at 14:14 -0700, Kees Cook wrote:
> This LSM enforces that kernel-loaded files (modules, firmware, etc)
> must all come from the same filesystem, with the expectation that
> such a filesystem is backed by a read-only device such as dm-verity
> or CDROM. This allows systems that have a verified and/or unchangeable
> filesystem to enforce module and firmware loading restrictions without
> needing to sign the files individually.

trivia:

> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
[]
> +#define pr_fmt(fmt) "LoadPin: " fmt

Using

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

would be a lot more common.

Is there some reason the logging messages should be
prefixed with "LoadPin: " instead of "loadpin: "?

[toc] | [prev] | [next] | [standalone]

#1368815

On Mon, 2016-03-28 at 14:14 -0700, Kees Cook wrote:

> +static const char *id_str[READING_MAX_ID] = {
> +	[READING_FIRMWARE] = "firmware",
> +	[READING_MODULE] = "kernel module",
> +	[READING_KEXEC_IMAGE] = "kexec image",
> +	[READING_KEXEC_INITRAMFS] = "kexec initramfs",
> +	[READING_POLICY] = "security policy",
> +};
> +
> +static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
> +{
> +	struct super_block *load_root;
> +	const char *origin;
> +
> +	if (id < 0 || id >= READING_MAX_ID)
> +		return -EINVAL;
> +

The kernel_read_file_id enumeration starts at 1.

> +	origin = id_str[id];
> +	if (!origin) {
> +		origin = "unknown";

Right, all the entries in the kernel_read_file_id enumeration are
currently listed in id_str.  Checking origin is needed for when id_str[]
isn't kept in sync with the enumeration.

Looks good!

Mimi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web