LinuxLists.cc - [PATCH] LSM: ModPin LSM for module loading restrictions

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On Fri, 20 Sep 2013, Kees Cook wrote:

> This LSM enforces that modules 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 or unchanging filesystem to enforce module loading restrictions
> without needing to sign the modules individually.
>
> Signed-off-by: Kees Cook <[email protected]>

Are you using this for ChromeOS?

> ---
> security/Kconfig | 6 ++
> security/Makefile | 2 +
> security/modpin/Kconfig | 9 +++
> security/modpin/Makefile | 1 +
> security/modpin/modpin.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 215 insertions(+)
> create mode 100644 security/modpin/Kconfig
> create mode 100644 security/modpin/Makefile
> create mode 100644 security/modpin/modpin.c
>
> diff --git a/security/Kconfig b/security/Kconfig
> index e9c6ac7..80172fd 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -121,6 +121,7 @@ source security/selinux/Kconfig
> source security/smack/Kconfig
> source security/tomoyo/Kconfig
> source security/apparmor/Kconfig
> +source security/modpin/Kconfig
> source security/yama/Kconfig
>
> source security/integrity/Kconfig
> @@ -131,6 +132,7 @@ choice
> default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
> default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
> default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
> + default DEFAULT_SECURITY_MODPIN if SECURITY_MODPIN
> default DEFAULT_SECURITY_YAMA if SECURITY_YAMA
> default DEFAULT_SECURITY_DAC
>
> @@ -150,6 +152,9 @@ choice
> config DEFAULT_SECURITY_APPARMOR
> bool "AppArmor" if SECURITY_APPARMOR=y
>
> + config DEFAULT_SECURITY_MODPIN
> + bool "ModPin" if SECURITY_MODPIN=y
> +
> config DEFAULT_SECURITY_YAMA
> bool "Yama" if SECURITY_YAMA=y
>
> @@ -164,6 +169,7 @@ config DEFAULT_SECURITY
> default "smack" if DEFAULT_SECURITY_SMACK
> default "tomoyo" if DEFAULT_SECURITY_TOMOYO
> default "apparmor" if DEFAULT_SECURITY_APPARMOR
> + default "modpin" if DEFAULT_SECURITY_MODPIN
> default "yama" if DEFAULT_SECURITY_YAMA
> default "" if DEFAULT_SECURITY_DAC
>
> diff --git a/security/Makefile b/security/Makefile
> index c26c81e..73d0a05 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -7,6 +7,7 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> subdir-$(CONFIG_SECURITY_SMACK) += smack
> subdir-$(CONFIG_SECURITY_TOMOYO) += tomoyo
> subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
> +subdir-$(CONFIG_SECURITY_MODPIN) += modpin
> subdir-$(CONFIG_SECURITY_YAMA) += yama
>
> # always enable default capabilities
> @@ -22,6 +23,7 @@ obj-$(CONFIG_SECURITY_SMACK) += smack/built-in.o
> obj-$(CONFIG_AUDIT) += lsm_audit.o
> obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/built-in.o
> obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/built-in.o
> +obj-$(CONFIG_SECURITY_MODPIN) += modpin/built-in.o
> obj-$(CONFIG_SECURITY_YAMA) += yama/built-in.o
> obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
>
> diff --git a/security/modpin/Kconfig b/security/modpin/Kconfig
> new file mode 100644
> index 0000000..5be9dd5
> --- /dev/null
> +++ b/security/modpin/Kconfig
> @@ -0,0 +1,9 @@
> +config SECURITY_MODPIN
> + bool "Module filesystem origin pinning"
> + depends on SECURITY
> + help
> + Module loading will be pinned to the first filesystem used for
> + loading. Any modules 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/modpin/Makefile b/security/modpin/Makefile
> new file mode 100644
> index 0000000..9080b29
> --- /dev/null
> +++ b/security/modpin/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_SECURITY_MODPIN) += modpin.o
> diff --git a/security/modpin/modpin.c b/security/modpin/modpin.c
> new file mode 100644
> index 0000000..107b4d8
> --- /dev/null
> +++ b/security/modpin/modpin.c
> @@ -0,0 +1,197 @@
> +/*
> + * Module Pinning Security Module
> + *
> + * Copyright 2011-2013 Google Inc.
> + *
> + * Authors:
> + * Kees Cook <[email protected]>
> + *
> + * 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) "ModPin LSM: " fmt
> +
> +#include <linux/module.h>
> +#include <linux/security.h>
> +#include <linux/sched.h>
> +#include <linux/fs.h>
> +#include <linux/fs_struct.h>
> +#include <linux/mount.h>
> +#include <linux/path.h>
> +#include <linux/root_dev.h>
> +
> +static void report_load_module(struct path *path, char *operation)
> +{
> + char *alloced = NULL;
> + char *pathname; /* Pointer to either static string or "alloced". */
> +
> + if (!path)
> + pathname = "<unknown>";
> + else {
> + /* We will allow 11 spaces for ' (deleted)' to be appended */
> + alloced = pathname = kmalloc(PATH_MAX+11, GFP_KERNEL);
> + if (!pathname)
> + pathname = "<no_memory>";
> + else {
> + pathname = d_path(path, pathname, PATH_MAX+11);
> + if (IS_ERR(pathname))
> + pathname = "<too_long>";
> + }
> + }
> +
> + pr_notice("init_module %s module=%s pid=%d\n",
> + operation, pathname, task_pid_nr(current));
> +
> + kfree(alloced);
> +}
> +
> +static int modpin_enforced = 1;
> +static struct dentry *pinned_root;
> +static DEFINE_SPINLOCK(pinned_root_spinlock);
> +
> +#ifdef CONFIG_SYSCTL
> +static int zero;
> +static int one = 1;
> +
> +static struct ctl_path modpin_sysctl_path[] = {
> + { .procname = "kernel", },
> + { .procname = "modpin", },
> + { }
> +};
> +
> +static struct ctl_table modpin_sysctl_table[] = {
> + {
> + .procname = "enforced",
> + .data = &modpin_enforced,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &zero,
> + .extra2 = &one,
> + },
> + { }
> +};
> +
> +/*
> + * Check if the root device is read-only (e.g. dm-verity is enabled).
> + * This must be called after early kernel init, since only then is the
> + * rootdev available.
> + */
> +static bool rootdev_readonly(void)
> +{
> + bool rc;
> + struct block_device *bdev;
> + const fmode_t mode = FMODE_WRITE;
> +
> + bdev = blkdev_get_by_dev(ROOT_DEV, mode, NULL);
> + if (IS_ERR(bdev)) {
> + /* In this weird case, assume it is read-only. */
> + pr_info("dev(%u,%u): FMODE_WRITE disallowed?!\n",
> + MAJOR(ROOT_DEV), MINOR(ROOT_DEV));
> + return true;
> + }
> +
> + rc = bdev_read_only(bdev);
> + blkdev_put(bdev, mode);
> +
> + pr_info("dev(%u,%u): %s\n", MAJOR(ROOT_DEV), MINOR(ROOT_DEV),
> + rc ? "read-only" : "writable");
> +
> + return rc;
> +}
> +
> +static void check_pinning_enforcement(void)
> +{
> + /*
> + * If module pinning is not being enforced, allow sysctl to change
> + * modes for testing.
> + */
> + if (!rootdev_readonly()) {
> + if (!register_sysctl_paths(modpin_sysctl_path,
> + modpin_sysctl_table))
> + pr_notice("sysctl registration failed!\n");
> + else
> + pr_info("module pinning can be disabled.\n");
> + } else
> + pr_info("module pinning engaged.\n");
> +}
> +#else
> +static void check_pinning_enforcement(void) { }
> +#endif
> +
> +
> +static int modpin_load_module(struct file *file)
> +{
> + struct dentry *module_root;
> +
> + if (!file) {
> + if (!modpin_enforced) {
> + report_load_module(NULL, "old-api-pinning-ignored");
> + return 0;
> + }
> +
> + report_load_module(NULL, "old-api-denied");
> + return -EPERM;
> + }
> +
> + module_root = file->f_path.mnt->mnt_root;
> +
> + /* First loaded module defines the root for all others. */
> + spin_lock(&pinned_root_spinlock);
> + if (!pinned_root) {
> + pinned_root = dget(module_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();
> + report_load_module(&file->f_path, "pinned");
> + return 0;
> + }
> + spin_unlock(&pinned_root_spinlock);
> +
> + if (module_root != pinned_root) {
> + if (unlikely(!modpin_enforced)) {
> + report_load_module(&file->f_path, "pinning-ignored");
> + return 0;
> + }
> +
> + report_load_module(&file->f_path, "denied");
> + return -EPERM;
> + }
> +
> + return 0;
> +}
> +
> +static struct security_operations modpin_ops = {
> + .name = "modpin",
> + .kernel_module_from_file = modpin_load_module,
> +};
> +
> +static int __init modpin_init(void)
> +{
> + int error;
> +
> + error = register_security(&modpin_ops);
> +
> + if (error)
> + panic("Could not register ModPin security module");
> +
> + pr_info("ready to pin.\n");
> +
> + return error;
> +}
> +security_initcall(modpin_init);
> +
> +module_param(modpin_enforced, int, S_IRUSR);
> +MODULE_PARM_DESC(modpin_enforced, "Module pinning enforced (default: true)");
> --
> 1.7.9.5
>
>
> --
> Kees Cook
> Chrome OS Security
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

--
James Morris
<[email protected]>

2013-09-24 01:27:14

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On Tue, 24 Sep 2013, James Morris wrote:

> On Fri, 20 Sep 2013, Kees Cook wrote:
>
> > This LSM enforces that modules 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 or unchanging filesystem to enforce module loading restrictions
> > without needing to sign the modules individually.
> >
> > Signed-off-by: Kees Cook <[email protected]>
>
> Are you using this for ChromeOS?

Also, you should CC Rusty on this.

>
>
> > ---
> > security/Kconfig | 6 ++
> > security/Makefile | 2 +
> > security/modpin/Kconfig | 9 +++
> > security/modpin/Makefile | 1 +
> > security/modpin/modpin.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 215 insertions(+)
> > create mode 100644 security/modpin/Kconfig
> > create mode 100644 security/modpin/Makefile
> > create mode 100644 security/modpin/modpin.c
> >
> > diff --git a/security/Kconfig b/security/Kconfig
> > index e9c6ac7..80172fd 100644
> > --- a/security/Kconfig
> > +++ b/security/Kconfig
> > @@ -121,6 +121,7 @@ source security/selinux/Kconfig
> > source security/smack/Kconfig
> > source security/tomoyo/Kconfig
> > source security/apparmor/Kconfig
> > +source security/modpin/Kconfig
> > source security/yama/Kconfig
> >
> > source security/integrity/Kconfig
> > @@ -131,6 +132,7 @@ choice
> > default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
> > default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
> > default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
> > + default DEFAULT_SECURITY_MODPIN if SECURITY_MODPIN
> > default DEFAULT_SECURITY_YAMA if SECURITY_YAMA
> > default DEFAULT_SECURITY_DAC
> >
> > @@ -150,6 +152,9 @@ choice
> > config DEFAULT_SECURITY_APPARMOR
> > bool "AppArmor" if SECURITY_APPARMOR=y
> >
> > + config DEFAULT_SECURITY_MODPIN
> > + bool "ModPin" if SECURITY_MODPIN=y
> > +
> > config DEFAULT_SECURITY_YAMA
> > bool "Yama" if SECURITY_YAMA=y
> >
> > @@ -164,6 +169,7 @@ config DEFAULT_SECURITY
> > default "smack" if DEFAULT_SECURITY_SMACK
> > default "tomoyo" if DEFAULT_SECURITY_TOMOYO
> > default "apparmor" if DEFAULT_SECURITY_APPARMOR
> > + default "modpin" if DEFAULT_SECURITY_MODPIN
> > default "yama" if DEFAULT_SECURITY_YAMA
> > default "" if DEFAULT_SECURITY_DAC
> >
> > diff --git a/security/Makefile b/security/Makefile
> > index c26c81e..73d0a05 100644
> > --- a/security/Makefile
> > +++ b/security/Makefile
> > @@ -7,6 +7,7 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> > subdir-$(CONFIG_SECURITY_SMACK) += smack
> > subdir-$(CONFIG_SECURITY_TOMOYO) += tomoyo
> > subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
> > +subdir-$(CONFIG_SECURITY_MODPIN) += modpin
> > subdir-$(CONFIG_SECURITY_YAMA) += yama
> >
> > # always enable default capabilities
> > @@ -22,6 +23,7 @@ obj-$(CONFIG_SECURITY_SMACK) += smack/built-in.o
> > obj-$(CONFIG_AUDIT) += lsm_audit.o
> > obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/built-in.o
> > obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/built-in.o
> > +obj-$(CONFIG_SECURITY_MODPIN) += modpin/built-in.o
> > obj-$(CONFIG_SECURITY_YAMA) += yama/built-in.o
> > obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
> >
> > diff --git a/security/modpin/Kconfig b/security/modpin/Kconfig
> > new file mode 100644
> > index 0000000..5be9dd5
> > --- /dev/null
> > +++ b/security/modpin/Kconfig
> > @@ -0,0 +1,9 @@
> > +config SECURITY_MODPIN
> > + bool "Module filesystem origin pinning"
> > + depends on SECURITY
> > + help
> > + Module loading will be pinned to the first filesystem used for
> > + loading. Any modules 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/modpin/Makefile b/security/modpin/Makefile
> > new file mode 100644
> > index 0000000..9080b29
> > --- /dev/null
> > +++ b/security/modpin/Makefile
> > @@ -0,0 +1 @@
> > +obj-$(CONFIG_SECURITY_MODPIN) += modpin.o
> > diff --git a/security/modpin/modpin.c b/security/modpin/modpin.c
> > new file mode 100644
> > index 0000000..107b4d8
> > --- /dev/null
> > +++ b/security/modpin/modpin.c
> > @@ -0,0 +1,197 @@
> > +/*
> > + * Module Pinning Security Module
> > + *
> > + * Copyright 2011-2013 Google Inc.
> > + *
> > + * Authors:
> > + * Kees Cook <[email protected]>
> > + *
> > + * 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) "ModPin LSM: " fmt
> > +
> > +#include <linux/module.h>
> > +#include <linux/security.h>
> > +#include <linux/sched.h>
> > +#include <linux/fs.h>
> > +#include <linux/fs_struct.h>
> > +#include <linux/mount.h>
> > +#include <linux/path.h>
> > +#include <linux/root_dev.h>
> > +
> > +static void report_load_module(struct path *path, char *operation)
> > +{
> > + char *alloced = NULL;
> > + char *pathname; /* Pointer to either static string or "alloced". */
> > +
> > + if (!path)
> > + pathname = "<unknown>";
> > + else {
> > + /* We will allow 11 spaces for ' (deleted)' to be appended */
> > + alloced = pathname = kmalloc(PATH_MAX+11, GFP_KERNEL);
> > + if (!pathname)
> > + pathname = "<no_memory>";
> > + else {
> > + pathname = d_path(path, pathname, PATH_MAX+11);
> > + if (IS_ERR(pathname))
> > + pathname = "<too_long>";
> > + }
> > + }
> > +
> > + pr_notice("init_module %s module=%s pid=%d\n",
> > + operation, pathname, task_pid_nr(current));
> > +
> > + kfree(alloced);
> > +}
> > +
> > +static int modpin_enforced = 1;
> > +static struct dentry *pinned_root;
> > +static DEFINE_SPINLOCK(pinned_root_spinlock);
> > +
> > +#ifdef CONFIG_SYSCTL
> > +static int zero;
> > +static int one = 1;
> > +
> > +static struct ctl_path modpin_sysctl_path[] = {
> > + { .procname = "kernel", },
> > + { .procname = "modpin", },
> > + { }
> > +};
> > +
> > +static struct ctl_table modpin_sysctl_table[] = {
> > + {
> > + .procname = "enforced",
> > + .data = &modpin_enforced,
> > + .maxlen = sizeof(int),
> > + .mode = 0644,
> > + .proc_handler = proc_dointvec_minmax,
> > + .extra1 = &zero,
> > + .extra2 = &one,
> > + },
> > + { }
> > +};
> > +
> > +/*
> > + * Check if the root device is read-only (e.g. dm-verity is enabled).
> > + * This must be called after early kernel init, since only then is the
> > + * rootdev available.
> > + */
> > +static bool rootdev_readonly(void)
> > +{
> > + bool rc;
> > + struct block_device *bdev;
> > + const fmode_t mode = FMODE_WRITE;
> > +
> > + bdev = blkdev_get_by_dev(ROOT_DEV, mode, NULL);
> > + if (IS_ERR(bdev)) {
> > + /* In this weird case, assume it is read-only. */
> > + pr_info("dev(%u,%u): FMODE_WRITE disallowed?!\n",
> > + MAJOR(ROOT_DEV), MINOR(ROOT_DEV));
> > + return true;
> > + }
> > +
> > + rc = bdev_read_only(bdev);
> > + blkdev_put(bdev, mode);
> > +
> > + pr_info("dev(%u,%u): %s\n", MAJOR(ROOT_DEV), MINOR(ROOT_DEV),
> > + rc ? "read-only" : "writable");
> > +
> > + return rc;
> > +}
> > +
> > +static void check_pinning_enforcement(void)
> > +{
> > + /*
> > + * If module pinning is not being enforced, allow sysctl to change
> > + * modes for testing.
> > + */
> > + if (!rootdev_readonly()) {
> > + if (!register_sysctl_paths(modpin_sysctl_path,
> > + modpin_sysctl_table))
> > + pr_notice("sysctl registration failed!\n");
> > + else
> > + pr_info("module pinning can be disabled.\n");
> > + } else
> > + pr_info("module pinning engaged.\n");
> > +}
> > +#else
> > +static void check_pinning_enforcement(void) { }
> > +#endif
> > +
> > +
> > +static int modpin_load_module(struct file *file)
> > +{
> > + struct dentry *module_root;
> > +
> > + if (!file) {
> > + if (!modpin_enforced) {
> > + report_load_module(NULL, "old-api-pinning-ignored");
> > + return 0;
> > + }
> > +
> > + report_load_module(NULL, "old-api-denied");
> > + return -EPERM;
> > + }
> > +
> > + module_root = file->f_path.mnt->mnt_root;
> > +
> > + /* First loaded module defines the root for all others. */
> > + spin_lock(&pinned_root_spinlock);
> > + if (!pinned_root) {
> > + pinned_root = dget(module_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();
> > + report_load_module(&file->f_path, "pinned");
> > + return 0;
> > + }
> > + spin_unlock(&pinned_root_spinlock);
> > +
> > + if (module_root != pinned_root) {
> > + if (unlikely(!modpin_enforced)) {
> > + report_load_module(&file->f_path, "pinning-ignored");
> > + return 0;
> > + }
> > +
> > + report_load_module(&file->f_path, "denied");
> > + return -EPERM;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct security_operations modpin_ops = {
> > + .name = "modpin",
> > + .kernel_module_from_file = modpin_load_module,
> > +};
> > +
> > +static int __init modpin_init(void)
> > +{
> > + int error;
> > +
> > + error = register_security(&modpin_ops);
> > +
> > + if (error)
> > + panic("Could not register ModPin security module");
> > +
> > + pr_info("ready to pin.\n");
> > +
> > + return error;
> > +}
> > +security_initcall(modpin_init);
> > +
> > +module_param(modpin_enforced, int, S_IRUSR);
> > +MODULE_PARM_DESC(modpin_enforced, "Module pinning enforced (default: true)");
> > --
> > 1.7.9.5
> >
> >
> > --
> > Kees Cook
> > Chrome OS Security
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>
> --
> James Morris
> <[email protected]>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

--
James Morris
<[email protected]>

2013-09-24 01:45:40

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

[+rusty]

On Mon, Sep 23, 2013 at 6:28 PM, James Morris <[email protected]> wrote:
> On Tue, 24 Sep 2013, James Morris wrote:
>
>> On Fri, 20 Sep 2013, Kees Cook wrote:
>>
>> > This LSM enforces that modules 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 or unchanging filesystem to enforce module loading restrictions
>> > without needing to sign the modules individually.
>> >
>> > Signed-off-by: Kees Cook <[email protected]>
>>
>> Are you using this for ChromeOS?

Yes. Chrome OS uses a read-only root filesystem that is backed by
dm-verity. This lets us pin all module loading to that filesystem
without needing per-module signatures.

> Also, you should CC Rusty on this.

Done! :)

-Kees

>
>
>>
>>
>> > ---
>> > security/Kconfig | 6 ++
>> > security/Makefile | 2 +
>> > security/modpin/Kconfig | 9 +++
>> > security/modpin/Makefile | 1 +
>> > security/modpin/modpin.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++
>> > 5 files changed, 215 insertions(+)
>> > create mode 100644 security/modpin/Kconfig
>> > create mode 100644 security/modpin/Makefile
>> > create mode 100644 security/modpin/modpin.c
>> >
>> > diff --git a/security/Kconfig b/security/Kconfig
>> > index e9c6ac7..80172fd 100644
>> > --- a/security/Kconfig
>> > +++ b/security/Kconfig
>> > @@ -121,6 +121,7 @@ source security/selinux/Kconfig
>> > source security/smack/Kconfig
>> > source security/tomoyo/Kconfig
>> > source security/apparmor/Kconfig
>> > +source security/modpin/Kconfig
>> > source security/yama/Kconfig
>> >
>> > source security/integrity/Kconfig
>> > @@ -131,6 +132,7 @@ choice
>> > default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
>> > default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
>> > default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
>> > + default DEFAULT_SECURITY_MODPIN if SECURITY_MODPIN
>> > default DEFAULT_SECURITY_YAMA if SECURITY_YAMA
>> > default DEFAULT_SECURITY_DAC
>> >
>> > @@ -150,6 +152,9 @@ choice
>> > config DEFAULT_SECURITY_APPARMOR
>> > bool "AppArmor" if SECURITY_APPARMOR=y
>> >
>> > + config DEFAULT_SECURITY_MODPIN
>> > + bool "ModPin" if SECURITY_MODPIN=y
>> > +
>> > config DEFAULT_SECURITY_YAMA
>> > bool "Yama" if SECURITY_YAMA=y
>> >
>> > @@ -164,6 +169,7 @@ config DEFAULT_SECURITY
>> > default "smack" if DEFAULT_SECURITY_SMACK
>> > default "tomoyo" if DEFAULT_SECURITY_TOMOYO
>> > default "apparmor" if DEFAULT_SECURITY_APPARMOR
>> > + default "modpin" if DEFAULT_SECURITY_MODPIN
>> > default "yama" if DEFAULT_SECURITY_YAMA
>> > default "" if DEFAULT_SECURITY_DAC
>> >
>> > diff --git a/security/Makefile b/security/Makefile
>> > index c26c81e..73d0a05 100644
>> > --- a/security/Makefile
>> > +++ b/security/Makefile
>> > @@ -7,6 +7,7 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux
>> > subdir-$(CONFIG_SECURITY_SMACK) += smack
>> > subdir-$(CONFIG_SECURITY_TOMOYO) += tomoyo
>> > subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
>> > +subdir-$(CONFIG_SECURITY_MODPIN) += modpin
>> > subdir-$(CONFIG_SECURITY_YAMA) += yama
>> >
>> > # always enable default capabilities
>> > @@ -22,6 +23,7 @@ obj-$(CONFIG_SECURITY_SMACK) += smack/built-in.o
>> > obj-$(CONFIG_AUDIT) += lsm_audit.o
>> > obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/built-in.o
>> > obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/built-in.o
>> > +obj-$(CONFIG_SECURITY_MODPIN) += modpin/built-in.o
>> > obj-$(CONFIG_SECURITY_YAMA) += yama/built-in.o
>> > obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
>> >
>> > diff --git a/security/modpin/Kconfig b/security/modpin/Kconfig
>> > new file mode 100644
>> > index 0000000..5be9dd5
>> > --- /dev/null
>> > +++ b/security/modpin/Kconfig
>> > @@ -0,0 +1,9 @@
>> > +config SECURITY_MODPIN
>> > + bool "Module filesystem origin pinning"
>> > + depends on SECURITY
>> > + help
>> > + Module loading will be pinned to the first filesystem used for
>> > + loading. Any modules 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/modpin/Makefile b/security/modpin/Makefile
>> > new file mode 100644
>> > index 0000000..9080b29
>> > --- /dev/null
>> > +++ b/security/modpin/Makefile
>> > @@ -0,0 +1 @@
>> > +obj-$(CONFIG_SECURITY_MODPIN) += modpin.o
>> > diff --git a/security/modpin/modpin.c b/security/modpin/modpin.c
>> > new file mode 100644
>> > index 0000000..107b4d8
>> > --- /dev/null
>> > +++ b/security/modpin/modpin.c
>> > @@ -0,0 +1,197 @@
>> > +/*
>> > + * Module Pinning Security Module
>> > + *
>> > + * Copyright 2011-2013 Google Inc.
>> > + *
>> > + * Authors:
>> > + * Kees Cook <[email protected]>
>> > + *
>> > + * 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) "ModPin LSM: " fmt
>> > +
>> > +#include <linux/module.h>
>> > +#include <linux/security.h>
>> > +#include <linux/sched.h>
>> > +#include <linux/fs.h>
>> > +#include <linux/fs_struct.h>
>> > +#include <linux/mount.h>
>> > +#include <linux/path.h>
>> > +#include <linux/root_dev.h>
>> > +
>> > +static void report_load_module(struct path *path, char *operation)
>> > +{
>> > + char *alloced = NULL;
>> > + char *pathname; /* Pointer to either static string or "alloced". */
>> > +
>> > + if (!path)
>> > + pathname = "<unknown>";
>> > + else {
>> > + /* We will allow 11 spaces for ' (deleted)' to be appended */
>> > + alloced = pathname = kmalloc(PATH_MAX+11, GFP_KERNEL);
>> > + if (!pathname)
>> > + pathname = "<no_memory>";
>> > + else {
>> > + pathname = d_path(path, pathname, PATH_MAX+11);
>> > + if (IS_ERR(pathname))
>> > + pathname = "<too_long>";
>> > + }
>> > + }
>> > +
>> > + pr_notice("init_module %s module=%s pid=%d\n",
>> > + operation, pathname, task_pid_nr(current));
>> > +
>> > + kfree(alloced);
>> > +}
>> > +
>> > +static int modpin_enforced = 1;
>> > +static struct dentry *pinned_root;
>> > +static DEFINE_SPINLOCK(pinned_root_spinlock);
>> > +
>> > +#ifdef CONFIG_SYSCTL
>> > +static int zero;
>> > +static int one = 1;
>> > +
>> > +static struct ctl_path modpin_sysctl_path[] = {
>> > + { .procname = "kernel", },
>> > + { .procname = "modpin", },
>> > + { }
>> > +};
>> > +
>> > +static struct ctl_table modpin_sysctl_table[] = {
>> > + {
>> > + .procname = "enforced",
>> > + .data = &modpin_enforced,
>> > + .maxlen = sizeof(int),
>> > + .mode = 0644,
>> > + .proc_handler = proc_dointvec_minmax,
>> > + .extra1 = &zero,
>> > + .extra2 = &one,
>> > + },
>> > + { }
>> > +};
>> > +
>> > +/*
>> > + * Check if the root device is read-only (e.g. dm-verity is enabled).
>> > + * This must be called after early kernel init, since only then is the
>> > + * rootdev available.
>> > + */
>> > +static bool rootdev_readonly(void)
>> > +{
>> > + bool rc;
>> > + struct block_device *bdev;
>> > + const fmode_t mode = FMODE_WRITE;
>> > +
>> > + bdev = blkdev_get_by_dev(ROOT_DEV, mode, NULL);
>> > + if (IS_ERR(bdev)) {
>> > + /* In this weird case, assume it is read-only. */
>> > + pr_info("dev(%u,%u): FMODE_WRITE disallowed?!\n",
>> > + MAJOR(ROOT_DEV), MINOR(ROOT_DEV));
>> > + return true;
>> > + }
>> > +
>> > + rc = bdev_read_only(bdev);
>> > + blkdev_put(bdev, mode);
>> > +
>> > + pr_info("dev(%u,%u): %s\n", MAJOR(ROOT_DEV), MINOR(ROOT_DEV),
>> > + rc ? "read-only" : "writable");
>> > +
>> > + return rc;
>> > +}
>> > +
>> > +static void check_pinning_enforcement(void)
>> > +{
>> > + /*
>> > + * If module pinning is not being enforced, allow sysctl to change
>> > + * modes for testing.
>> > + */
>> > + if (!rootdev_readonly()) {
>> > + if (!register_sysctl_paths(modpin_sysctl_path,
>> > + modpin_sysctl_table))
>> > + pr_notice("sysctl registration failed!\n");
>> > + else
>> > + pr_info("module pinning can be disabled.\n");
>> > + } else
>> > + pr_info("module pinning engaged.\n");
>> > +}
>> > +#else
>> > +static void check_pinning_enforcement(void) { }
>> > +#endif
>> > +
>> > +
>> > +static int modpin_load_module(struct file *file)
>> > +{
>> > + struct dentry *module_root;
>> > +
>> > + if (!file) {
>> > + if (!modpin_enforced) {
>> > + report_load_module(NULL, "old-api-pinning-ignored");
>> > + return 0;
>> > + }
>> > +
>> > + report_load_module(NULL, "old-api-denied");
>> > + return -EPERM;
>> > + }
>> > +
>> > + module_root = file->f_path.mnt->mnt_root;
>> > +
>> > + /* First loaded module defines the root for all others. */
>> > + spin_lock(&pinned_root_spinlock);
>> > + if (!pinned_root) {
>> > + pinned_root = dget(module_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();
>> > + report_load_module(&file->f_path, "pinned");
>> > + return 0;
>> > + }
>> > + spin_unlock(&pinned_root_spinlock);
>> > +
>> > + if (module_root != pinned_root) {
>> > + if (unlikely(!modpin_enforced)) {
>> > + report_load_module(&file->f_path, "pinning-ignored");
>> > + return 0;
>> > + }
>> > +
>> > + report_load_module(&file->f_path, "denied");
>> > + return -EPERM;
>> > + }
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static struct security_operations modpin_ops = {
>> > + .name = "modpin",
>> > + .kernel_module_from_file = modpin_load_module,
>> > +};
>> > +
>> > +static int __init modpin_init(void)
>> > +{
>> > + int error;
>> > +
>> > + error = register_security(&modpin_ops);
>> > +
>> > + if (error)
>> > + panic("Could not register ModPin security module");
>> > +
>> > + pr_info("ready to pin.\n");
>> > +
>> > + return error;
>> > +}
>> > +security_initcall(modpin_init);
>> > +
>> > +module_param(modpin_enforced, int, S_IRUSR);
>> > +MODULE_PARM_DESC(modpin_enforced, "Module pinning enforced (default: true)");
>> > --
>> > 1.7.9.5
>> >
>> >
>> > --
>> > Kees Cook
>> > Chrome OS Security
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> > the body of a message to [email protected]
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>> > Please read the FAQ at http://www.tux.org/lkml/
>> >
>>
>> --
>> James Morris
>> <[email protected]>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> James Morris
> <[email protected]>

--
Kees Cook
Chrome OS Security

2013-10-03 20:55:36

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On Mon, Sep 23, 2013 at 06:45:35PM -0700, Kees Cook wrote:
> [+rusty]
>
> On Mon, Sep 23, 2013 at 6:28 PM, James Morris <[email protected]> wrote:
> > On Tue, 24 Sep 2013, James Morris wrote:
> >
> >> On Fri, 20 Sep 2013, Kees Cook wrote:
> >>
> >> > This LSM enforces that modules 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 or unchanging filesystem to enforce module loading restrictions
> >> > without needing to sign the modules individually.
> >> >
> >> > Signed-off-by: Kees Cook <[email protected]>
> >>
> >> Are you using this for ChromeOS?
>
> Yes. Chrome OS uses a read-only root filesystem that is backed by
> dm-verity. This lets us pin all module loading to that filesystem
> without needing per-module signatures.
>
> > Also, you should CC Rusty on this.
>
> Done! :)

Ping... any feedback on this? I'd like to get this landed so I can send
further patches that touch this and IMA.

Thanks,

-Kees

>
> -Kees
>
> >
> >
> >>
> >>
> >> > ---
> >> > security/Kconfig | 6 ++
> >> > security/Makefile | 2 +
> >> > security/modpin/Kconfig | 9 +++
> >> > security/modpin/Makefile | 1 +
> >> > security/modpin/modpin.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++
> >> > 5 files changed, 215 insertions(+)
> >> > create mode 100644 security/modpin/Kconfig
> >> > create mode 100644 security/modpin/Makefile
> >> > create mode 100644 security/modpin/modpin.c
> >> >
> >> > diff --git a/security/Kconfig b/security/Kconfig
> >> > index e9c6ac7..80172fd 100644
> >> > --- a/security/Kconfig
> >> > +++ b/security/Kconfig
> >> > @@ -121,6 +121,7 @@ source security/selinux/Kconfig
> >> > source security/smack/Kconfig
> >> > source security/tomoyo/Kconfig
> >> > source security/apparmor/Kconfig
> >> > +source security/modpin/Kconfig
> >> > source security/yama/Kconfig
> >> >
> >> > source security/integrity/Kconfig
> >> > @@ -131,6 +132,7 @@ choice
> >> > default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
> >> > default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
> >> > default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
> >> > + default DEFAULT_SECURITY_MODPIN if SECURITY_MODPIN
> >> > default DEFAULT_SECURITY_YAMA if SECURITY_YAMA
> >> > default DEFAULT_SECURITY_DAC
> >> >
> >> > @@ -150,6 +152,9 @@ choice
> >> > config DEFAULT_SECURITY_APPARMOR
> >> > bool "AppArmor" if SECURITY_APPARMOR=y
> >> >
> >> > + config DEFAULT_SECURITY_MODPIN
> >> > + bool "ModPin" if SECURITY_MODPIN=y
> >> > +
> >> > config DEFAULT_SECURITY_YAMA
> >> > bool "Yama" if SECURITY_YAMA=y
> >> >
> >> > @@ -164,6 +169,7 @@ config DEFAULT_SECURITY
> >> > default "smack" if DEFAULT_SECURITY_SMACK
> >> > default "tomoyo" if DEFAULT_SECURITY_TOMOYO
> >> > default "apparmor" if DEFAULT_SECURITY_APPARMOR
> >> > + default "modpin" if DEFAULT_SECURITY_MODPIN
> >> > default "yama" if DEFAULT_SECURITY_YAMA
> >> > default "" if DEFAULT_SECURITY_DAC
> >> >
> >> > diff --git a/security/Makefile b/security/Makefile
> >> > index c26c81e..73d0a05 100644
> >> > --- a/security/Makefile
> >> > +++ b/security/Makefile
> >> > @@ -7,6 +7,7 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> >> > subdir-$(CONFIG_SECURITY_SMACK) += smack
> >> > subdir-$(CONFIG_SECURITY_TOMOYO) += tomoyo
> >> > subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
> >> > +subdir-$(CONFIG_SECURITY_MODPIN) += modpin
> >> > subdir-$(CONFIG_SECURITY_YAMA) += yama
> >> >
> >> > # always enable default capabilities
> >> > @@ -22,6 +23,7 @@ obj-$(CONFIG_SECURITY_SMACK) += smack/built-in.o
> >> > obj-$(CONFIG_AUDIT) += lsm_audit.o
> >> > obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/built-in.o
> >> > obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/built-in.o
> >> > +obj-$(CONFIG_SECURITY_MODPIN) += modpin/built-in.o
> >> > obj-$(CONFIG_SECURITY_YAMA) += yama/built-in.o
> >> > obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
> >> >
> >> > diff --git a/security/modpin/Kconfig b/security/modpin/Kconfig
> >> > new file mode 100644
> >> > index 0000000..5be9dd5
> >> > --- /dev/null
> >> > +++ b/security/modpin/Kconfig
> >> > @@ -0,0 +1,9 @@
> >> > +config SECURITY_MODPIN
> >> > + bool "Module filesystem origin pinning"
> >> > + depends on SECURITY
> >> > + help
> >> > + Module loading will be pinned to the first filesystem used for
> >> > + loading. Any modules 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/modpin/Makefile b/security/modpin/Makefile
> >> > new file mode 100644
> >> > index 0000000..9080b29
> >> > --- /dev/null
> >> > +++ b/security/modpin/Makefile
> >> > @@ -0,0 +1 @@
> >> > +obj-$(CONFIG_SECURITY_MODPIN) += modpin.o
> >> > diff --git a/security/modpin/modpin.c b/security/modpin/modpin.c
> >> > new file mode 100644
> >> > index 0000000..107b4d8
> >> > --- /dev/null
> >> > +++ b/security/modpin/modpin.c
> >> > @@ -0,0 +1,197 @@
> >> > +/*
> >> > + * Module Pinning Security Module
> >> > + *
> >> > + * Copyright 2011-2013 Google Inc.
> >> > + *
> >> > + * Authors:
> >> > + * Kees Cook <[email protected]>
> >> > + *
> >> > + * 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) "ModPin LSM: " fmt
> >> > +
> >> > +#include <linux/module.h>
> >> > +#include <linux/security.h>
> >> > +#include <linux/sched.h>
> >> > +#include <linux/fs.h>
> >> > +#include <linux/fs_struct.h>
> >> > +#include <linux/mount.h>
> >> > +#include <linux/path.h>
> >> > +#include <linux/root_dev.h>
> >> > +
> >> > +static void report_load_module(struct path *path, char *operation)
> >> > +{
> >> > + char *alloced = NULL;
> >> > + char *pathname; /* Pointer to either static string or "alloced". */
> >> > +
> >> > + if (!path)
> >> > + pathname = "<unknown>";
> >> > + else {
> >> > + /* We will allow 11 spaces for ' (deleted)' to be appended */
> >> > + alloced = pathname = kmalloc(PATH_MAX+11, GFP_KERNEL);
> >> > + if (!pathname)
> >> > + pathname = "<no_memory>";
> >> > + else {
> >> > + pathname = d_path(path, pathname, PATH_MAX+11);
> >> > + if (IS_ERR(pathname))
> >> > + pathname = "<too_long>";
> >> > + }
> >> > + }
> >> > +
> >> > + pr_notice("init_module %s module=%s pid=%d\n",
> >> > + operation, pathname, task_pid_nr(current));
> >> > +
> >> > + kfree(alloced);
> >> > +}
> >> > +
> >> > +static int modpin_enforced = 1;
> >> > +static struct dentry *pinned_root;
> >> > +static DEFINE_SPINLOCK(pinned_root_spinlock);
> >> > +
> >> > +#ifdef CONFIG_SYSCTL
> >> > +static int zero;
> >> > +static int one = 1;
> >> > +
> >> > +static struct ctl_path modpin_sysctl_path[] = {
> >> > + { .procname = "kernel", },
> >> > + { .procname = "modpin", },
> >> > + { }
> >> > +};
> >> > +
> >> > +static struct ctl_table modpin_sysctl_table[] = {
> >> > + {
> >> > + .procname = "enforced",
> >> > + .data = &modpin_enforced,
> >> > + .maxlen = sizeof(int),
> >> > + .mode = 0644,
> >> > + .proc_handler = proc_dointvec_minmax,
> >> > + .extra1 = &zero,
> >> > + .extra2 = &one,
> >> > + },
> >> > + { }
> >> > +};
> >> > +
> >> > +/*
> >> > + * Check if the root device is read-only (e.g. dm-verity is enabled).
> >> > + * This must be called after early kernel init, since only then is the
> >> > + * rootdev available.
> >> > + */
> >> > +static bool rootdev_readonly(void)
> >> > +{
> >> > + bool rc;
> >> > + struct block_device *bdev;
> >> > + const fmode_t mode = FMODE_WRITE;
> >> > +
> >> > + bdev = blkdev_get_by_dev(ROOT_DEV, mode, NULL);
> >> > + if (IS_ERR(bdev)) {
> >> > + /* In this weird case, assume it is read-only. */
> >> > + pr_info("dev(%u,%u): FMODE_WRITE disallowed?!\n",
> >> > + MAJOR(ROOT_DEV), MINOR(ROOT_DEV));
> >> > + return true;
> >> > + }
> >> > +
> >> > + rc = bdev_read_only(bdev);
> >> > + blkdev_put(bdev, mode);
> >> > +
> >> > + pr_info("dev(%u,%u): %s\n", MAJOR(ROOT_DEV), MINOR(ROOT_DEV),
> >> > + rc ? "read-only" : "writable");
> >> > +
> >> > + return rc;
> >> > +}
> >> > +
> >> > +static void check_pinning_enforcement(void)
> >> > +{
> >> > + /*
> >> > + * If module pinning is not being enforced, allow sysctl to change
> >> > + * modes for testing.
> >> > + */
> >> > + if (!rootdev_readonly()) {
> >> > + if (!register_sysctl_paths(modpin_sysctl_path,
> >> > + modpin_sysctl_table))
> >> > + pr_notice("sysctl registration failed!\n");
> >> > + else
> >> > + pr_info("module pinning can be disabled.\n");
> >> > + } else
> >> > + pr_info("module pinning engaged.\n");
> >> > +}
> >> > +#else
> >> > +static void check_pinning_enforcement(void) { }
> >> > +#endif
> >> > +
> >> > +
> >> > +static int modpin_load_module(struct file *file)
> >> > +{
> >> > + struct dentry *module_root;
> >> > +
> >> > + if (!file) {
> >> > + if (!modpin_enforced) {
> >> > + report_load_module(NULL, "old-api-pinning-ignored");
> >> > + return 0;
> >> > + }
> >> > +
> >> > + report_load_module(NULL, "old-api-denied");
> >> > + return -EPERM;
> >> > + }
> >> > +
> >> > + module_root = file->f_path.mnt->mnt_root;
> >> > +
> >> > + /* First loaded module defines the root for all others. */
> >> > + spin_lock(&pinned_root_spinlock);
> >> > + if (!pinned_root) {
> >> > + pinned_root = dget(module_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();
> >> > + report_load_module(&file->f_path, "pinned");
> >> > + return 0;
> >> > + }
> >> > + spin_unlock(&pinned_root_spinlock);
> >> > +
> >> > + if (module_root != pinned_root) {
> >> > + if (unlikely(!modpin_enforced)) {
> >> > + report_load_module(&file->f_path, "pinning-ignored");
> >> > + return 0;
> >> > + }
> >> > +
> >> > + report_load_module(&file->f_path, "denied");
> >> > + return -EPERM;
> >> > + }
> >> > +
> >> > + return 0;
> >> > +}
> >> > +
> >> > +static struct security_operations modpin_ops = {
> >> > + .name = "modpin",
> >> > + .kernel_module_from_file = modpin_load_module,
> >> > +};
> >> > +
> >> > +static int __init modpin_init(void)
> >> > +{
> >> > + int error;
> >> > +
> >> > + error = register_security(&modpin_ops);
> >> > +
> >> > + if (error)
> >> > + panic("Could not register ModPin security module");
> >> > +
> >> > + pr_info("ready to pin.\n");
> >> > +
> >> > + return error;
> >> > +}
> >> > +security_initcall(modpin_init);
> >> > +
> >> > +module_param(modpin_enforced, int, S_IRUSR);
> >> > +MODULE_PARM_DESC(modpin_enforced, "Module pinning enforced (default: true)");
> >> > --
> >> > 1.7.9.5
> >> >
> >> >
> >> > --
> >> > Kees Cook
> >> > Chrome OS Security
> >> > --
> >> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >> > the body of a message to [email protected]
> >> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> > Please read the FAQ at http://www.tux.org/lkml/
> >> >
> >>
> >> --
> >> James Morris
> >> <[email protected]>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> >> the body of a message to [email protected]
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>
> >
> > --
> > James Morris
> > <[email protected]>
>
>
>
> --
> Kees Cook
> Chrome OS Security
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Kees Cook @outflux.net

2013-10-03 21:31:50

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

Kees Cook wrote:
> +static int modpin_load_module(struct file *file)
> +{
> + struct dentry *module_root;
> +
> + if (!file) {
> + if (!modpin_enforced) {
> + report_load_module(NULL, "old-api-pinning-ignored");
> + return 0;
> + }
> +
> + report_load_module(NULL, "old-api-denied");
> + return -EPERM;
> + }
> +
> + module_root = file->f_path.mnt->mnt_root;
> +
> + /* First loaded module defines the root for all others. */
> + spin_lock(&pinned_root_spinlock);
> + if (!pinned_root) {
> + pinned_root = dget(module_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();
> + report_load_module(&file->f_path, "pinned");
> + return 0;
> + }
> + spin_unlock(&pinned_root_spinlock);

Firstly loaded module is usually in initramfs whereas subsequently loaded
modules are usually on a hard disk partition.

This module is not meant for PC servers, is it?

2013-10-03 21:36:38

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On Fri, Oct 04, 2013 at 06:31:42AM +0900, Tetsuo Handa wrote:
> Kees Cook wrote:
> > +static int modpin_load_module(struct file *file)
> > +{
> > + struct dentry *module_root;
> > +
> > + if (!file) {
> > + if (!modpin_enforced) {
> > + report_load_module(NULL, "old-api-pinning-ignored");
> > + return 0;
> > + }
> > +
> > + report_load_module(NULL, "old-api-denied");
> > + return -EPERM;
> > + }
> > +
> > + module_root = file->f_path.mnt->mnt_root;
> > +
> > + /* First loaded module defines the root for all others. */
> > + spin_lock(&pinned_root_spinlock);
> > + if (!pinned_root) {
> > + pinned_root = dget(module_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();
> > + report_load_module(&file->f_path, "pinned");
> > + return 0;
> > + }
> > + spin_unlock(&pinned_root_spinlock);
>
> Firstly loaded module is usually in initramfs whereas subsequently loaded
> modules are usually on a hard disk partition.
>
> This module is not meant for PC servers, is it?

This LSM is what Chrome OS uses for the module pinning logic. We do not use
an initramfs. This LSM could also be used for devices booting entirely from
CDROM or other R/O media.

I'm open to improvements, obviously. I imagine things like delayed
activation, where the initramfs triggers pinning in some way once it is
done loading modules from its filesystem, etc. But since I don't have any
real life examples of this, I'm writing the LSM as it currently is, used
without an initramfs. :)

-Kees

--
Kees Cook @outflux.net

2013-10-16 15:18:44

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

Hi James,

On Mon, Sep 23, 2013 at 06:45:35PM -0700, Kees Cook wrote:
> [+rusty]
>
> On Mon, Sep 23, 2013 at 6:28 PM, James Morris <[email protected]> wrote:
> > On Tue, 24 Sep 2013, James Morris wrote:
> >
> >> On Fri, 20 Sep 2013, Kees Cook wrote:
> >>
> >> > This LSM enforces that modules 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 or unchanging filesystem to enforce module loading restrictions
> >> > without needing to sign the modules individually.
> >> >
> >> > Signed-off-by: Kees Cook <[email protected]>
> >>
> >> Are you using this for ChromeOS?
>
> Yes. Chrome OS uses a read-only root filesystem that is backed by
> dm-verity. This lets us pin all module loading to that filesystem
> without needing per-module signatures.
>
> > Also, you should CC Rusty on this.
>
> Done! :)

Any update on this? It'd be nice to have it in linux-next.

Thanks,

-Kees

--
Kees Cook @outflux.net

2013-10-16 20:47:36

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

Kees Cook wrote:
> Any update on this? It'd be nice to have it in linux-next.

What was the conclusion at LSS about multiple concurrent LSM support?
If we agreed to merge multiple concurrent LSM support, there will be nothing to
prevent this module from merging.

2013-10-16 21:49:07

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On 10/16/2013 1:47 PM, Tetsuo Handa wrote:
> Kees Cook wrote:
>> Any update on this? It'd be nice to have it in linux-next.
> What was the conclusion at LSS about multiple concurrent LSM support?
> If we agreed to merge multiple concurrent LSM support, there will be nothing to
> prevent this module from merging.
>
Yeah.

The conclusion was that it needs to be staged because it's
too much to swallow all at once. I can see that. It's going
to be a lot of work to rearrange and rebase. That's a chunk
of time I don't expect to have for a while. It looks good
to happen, but don't hold supper for me.

2013-10-16 22:43:28

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On Wed, Oct 16, 2013 at 2:42 PM, Casey Schaufler <[email protected]> wrote:
> On 10/16/2013 1:47 PM, Tetsuo Handa wrote:
>> Kees Cook wrote:
>>> Any update on this? It'd be nice to have it in linux-next.
>> What was the conclusion at LSS about multiple concurrent LSM support?
>> If we agreed to merge multiple concurrent LSM support, there will be nothing to
>> prevent this module from merging.
>>
> Yeah.

The discussion at LSS basically centered around the catch-22 of not
being able to stack, and not having anything to stack (since Yama got
an hard-coded exception). So I sent this LSM as one I'd been waiting
for stacking on. Essentially, I'm breaking the catch-22 by sending
this. I'd like it to get into the tree so we don't have a catch-22
about stacking any more. :)

> The conclusion was that it needs to be staged because it's
> too much to swallow all at once. I can see that. It's going
> to be a lot of work to rearrange and rebase. That's a chunk
> of time I don't expect to have for a while. It looks good
> to happen, but don't hold supper for me.

Do you want me to take a stab at it? It sounds like it was desirable
to cut the current series into two halves? The core changes first, and
the userspace interface changes next?

-Kees

--
Kees Cook
Chrome OS Security

2013-10-17 00:37:24

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

Kees Cook wrote:
> So I sent this LSM as one I\'d been waiting
> for stacking on. Essentially, I\'m breaking the catch-22 by sending
> this. I\'d like it to get into the tree so we don\'t have a catch-22
> about stacking any more. :)

I\'m also trying to break the catch-22 by sending KPortReserve.
I would send another one which uses only security_file_alloc/free .

> The core changes first, and the userspace interface changes next?

I welcome that approach, for none of such single function LSM modules
depends on userspace interface changes.

2013-10-17 07:57:34

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

This seems like a regression in terms of separating mechanism and policy.

We have several access control systems available (SELinux, at least) which
can implement this functionality with existing mechanisms using dynamic
policy.

I'm concerned about the long term architectural impact of a proliferation
of arbitrary hard-coded security policies in the kernel. I don't
understand the push in this direction, frankly.

On Fri, 20 Sep 2013, Kees Cook wrote:

> This LSM enforces that modules 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 or unchanging filesystem to enforce module loading restrictions
> without needing to sign the modules individually.
>
> Signed-off-by: Kees Cook <[email protected]>
> ---
> security/Kconfig | 6 ++
> security/Makefile | 2 +
> security/modpin/Kconfig | 9 +++
> security/modpin/Makefile | 1 +
> security/modpin/modpin.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 215 insertions(+)
> create mode 100644 security/modpin/Kconfig
> create mode 100644 security/modpin/Makefile
> create mode 100644 security/modpin/modpin.c
>
> diff --git a/security/Kconfig b/security/Kconfig
> index e9c6ac7..80172fd 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -121,6 +121,7 @@ source security/selinux/Kconfig
> source security/smack/Kconfig
> source security/tomoyo/Kconfig
> source security/apparmor/Kconfig
> +source security/modpin/Kconfig
> source security/yama/Kconfig
>
> source security/integrity/Kconfig
> @@ -131,6 +132,7 @@ choice
> default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
> default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
> default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
> + default DEFAULT_SECURITY_MODPIN if SECURITY_MODPIN
> default DEFAULT_SECURITY_YAMA if SECURITY_YAMA
> default DEFAULT_SECURITY_DAC
>
> @@ -150,6 +152,9 @@ choice
> config DEFAULT_SECURITY_APPARMOR
> bool "AppArmor" if SECURITY_APPARMOR=y
>
> + config DEFAULT_SECURITY_MODPIN
> + bool "ModPin" if SECURITY_MODPIN=y
> +
> config DEFAULT_SECURITY_YAMA
> bool "Yama" if SECURITY_YAMA=y
>
> @@ -164,6 +169,7 @@ config DEFAULT_SECURITY
> default "smack" if DEFAULT_SECURITY_SMACK
> default "tomoyo" if DEFAULT_SECURITY_TOMOYO
> default "apparmor" if DEFAULT_SECURITY_APPARMOR
> + default "modpin" if DEFAULT_SECURITY_MODPIN
> default "yama" if DEFAULT_SECURITY_YAMA
> default "" if DEFAULT_SECURITY_DAC
>
> diff --git a/security/Makefile b/security/Makefile
> index c26c81e..73d0a05 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -7,6 +7,7 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> subdir-$(CONFIG_SECURITY_SMACK) += smack
> subdir-$(CONFIG_SECURITY_TOMOYO) += tomoyo
> subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
> +subdir-$(CONFIG_SECURITY_MODPIN) += modpin
> subdir-$(CONFIG_SECURITY_YAMA) += yama
>
> # always enable default capabilities
> @@ -22,6 +23,7 @@ obj-$(CONFIG_SECURITY_SMACK) += smack/built-in.o
> obj-$(CONFIG_AUDIT) += lsm_audit.o
> obj-$(CONFIG_SECURITY_TOMOYO) += tomoyo/built-in.o
> obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/built-in.o
> +obj-$(CONFIG_SECURITY_MODPIN) += modpin/built-in.o
> obj-$(CONFIG_SECURITY_YAMA) += yama/built-in.o
> obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
>
> diff --git a/security/modpin/Kconfig b/security/modpin/Kconfig
> new file mode 100644
> index 0000000..5be9dd5
> --- /dev/null
> +++ b/security/modpin/Kconfig
> @@ -0,0 +1,9 @@
> +config SECURITY_MODPIN
> + bool "Module filesystem origin pinning"
> + depends on SECURITY
> + help
> + Module loading will be pinned to the first filesystem used for
> + loading. Any modules 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/modpin/Makefile b/security/modpin/Makefile
> new file mode 100644
> index 0000000..9080b29
> --- /dev/null
> +++ b/security/modpin/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_SECURITY_MODPIN) += modpin.o
> diff --git a/security/modpin/modpin.c b/security/modpin/modpin.c
> new file mode 100644
> index 0000000..107b4d8
> --- /dev/null
> +++ b/security/modpin/modpin.c
> @@ -0,0 +1,197 @@
> +/*
> + * Module Pinning Security Module
> + *
> + * Copyright 2011-2013 Google Inc.
> + *
> + * Authors:
> + * Kees Cook <[email protected]>
> + *
> + * 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) "ModPin LSM: " fmt
> +
> +#include <linux/module.h>
> +#include <linux/security.h>
> +#include <linux/sched.h>
> +#include <linux/fs.h>
> +#include <linux/fs_struct.h>
> +#include <linux/mount.h>
> +#include <linux/path.h>
> +#include <linux/root_dev.h>
> +
> +static void report_load_module(struct path *path, char *operation)
> +{
> + char *alloced = NULL;
> + char *pathname; /* Pointer to either static string or "alloced". */
> +
> + if (!path)
> + pathname = "<unknown>";
> + else {
> + /* We will allow 11 spaces for ' (deleted)' to be appended */
> + alloced = pathname = kmalloc(PATH_MAX+11, GFP_KERNEL);
> + if (!pathname)
> + pathname = "<no_memory>";
> + else {
> + pathname = d_path(path, pathname, PATH_MAX+11);
> + if (IS_ERR(pathname))
> + pathname = "<too_long>";
> + }
> + }
> +
> + pr_notice("init_module %s module=%s pid=%d\n",
> + operation, pathname, task_pid_nr(current));
> +
> + kfree(alloced);
> +}
> +
> +static int modpin_enforced = 1;
> +static struct dentry *pinned_root;
> +static DEFINE_SPINLOCK(pinned_root_spinlock);
> +
> +#ifdef CONFIG_SYSCTL
> +static int zero;
> +static int one = 1;
> +
> +static struct ctl_path modpin_sysctl_path[] = {
> + { .procname = "kernel", },
> + { .procname = "modpin", },
> + { }
> +};
> +
> +static struct ctl_table modpin_sysctl_table[] = {
> + {
> + .procname = "enforced",
> + .data = &modpin_enforced,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &zero,
> + .extra2 = &one,
> + },
> + { }
> +};
> +
> +/*
> + * Check if the root device is read-only (e.g. dm-verity is enabled).
> + * This must be called after early kernel init, since only then is the
> + * rootdev available.
> + */
> +static bool rootdev_readonly(void)
> +{
> + bool rc;
> + struct block_device *bdev;
> + const fmode_t mode = FMODE_WRITE;
> +
> + bdev = blkdev_get_by_dev(ROOT_DEV, mode, NULL);
> + if (IS_ERR(bdev)) {
> + /* In this weird case, assume it is read-only. */
> + pr_info("dev(%u,%u): FMODE_WRITE disallowed?!\n",
> + MAJOR(ROOT_DEV), MINOR(ROOT_DEV));
> + return true;
> + }
> +
> + rc = bdev_read_only(bdev);
> + blkdev_put(bdev, mode);
> +
> + pr_info("dev(%u,%u): %s\n", MAJOR(ROOT_DEV), MINOR(ROOT_DEV),
> + rc ? "read-only" : "writable");
> +
> + return rc;
> +}
> +
> +static void check_pinning_enforcement(void)
> +{
> + /*
> + * If module pinning is not being enforced, allow sysctl to change
> + * modes for testing.
> + */
> + if (!rootdev_readonly()) {
> + if (!register_sysctl_paths(modpin_sysctl_path,
> + modpin_sysctl_table))
> + pr_notice("sysctl registration failed!\n");
> + else
> + pr_info("module pinning can be disabled.\n");
> + } else
> + pr_info("module pinning engaged.\n");
> +}
> +#else
> +static void check_pinning_enforcement(void) { }
> +#endif
> +
> +
> +static int modpin_load_module(struct file *file)
> +{
> + struct dentry *module_root;
> +
> + if (!file) {
> + if (!modpin_enforced) {
> + report_load_module(NULL, "old-api-pinning-ignored");
> + return 0;
> + }
> +
> + report_load_module(NULL, "old-api-denied");
> + return -EPERM;
> + }
> +
> + module_root = file->f_path.mnt->mnt_root;
> +
> + /* First loaded module defines the root for all others. */
> + spin_lock(&pinned_root_spinlock);
> + if (!pinned_root) {
> + pinned_root = dget(module_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();
> + report_load_module(&file->f_path, "pinned");
> + return 0;
> + }
> + spin_unlock(&pinned_root_spinlock);
> +
> + if (module_root != pinned_root) {
> + if (unlikely(!modpin_enforced)) {
> + report_load_module(&file->f_path, "pinning-ignored");
> + return 0;
> + }
> +
> + report_load_module(&file->f_path, "denied");
> + return -EPERM;
> + }
> +
> + return 0;
> +}
> +
> +static struct security_operations modpin_ops = {
> + .name = "modpin",
> + .kernel_module_from_file = modpin_load_module,
> +};
> +
> +static int __init modpin_init(void)
> +{
> + int error;
> +
> + error = register_security(&modpin_ops);
> +
> + if (error)
> + panic("Could not register ModPin security module");
> +
> + pr_info("ready to pin.\n");
> +
> + return error;
> +}
> +security_initcall(modpin_init);
> +
> +module_param(modpin_enforced, int, S_IRUSR);
> +MODULE_PARM_DESC(modpin_enforced, "Module pinning enforced (default: true)");
> --
> 1.7.9.5
>
>
> --
> Kees Cook
> Chrome OS Security
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

--
James Morris
<[email protected]>

2013-10-17 11:31:03

by Jarkko Sakkinen

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On Thu, Oct 17, 2013 at 07:02:17PM +1100, James Morris wrote:
> This seems like a regression in terms of separating mechanism and policy.
>
> We have several access control systems available (SELinux, at least) which
> can implement this functionality with existing mechanisms using dynamic
> policy.
>
> I'm concerned about the long term architectural impact of a proliferation
> of arbitrary hard-coded security policies in the kernel. I don't
> understand the push in this direction, frankly.

The biggest risk in LSM stacker is really to become backdoor for very product
dilated kernel changes that are not accepted to the mainline kernel. I think
having LSM stacker would be benefical but barrier should be set very high
for "one-shot" modules.

One big benefit that I see in LSM stacker is not at least directly security
related. It would be perfect integration tool when you want for example
provide Android run-time in an OS that uses AppArmor or SMACK as its security
framework.

/Jarkko

2013-10-17 17:26:51

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On 10/17/2013 1:02 AM, James Morris wrote:
> This seems like a regression in terms of separating mechanism and policy.
>
> We have several access control systems available (SELinux, at least) which
> can implement this functionality with existing mechanisms using dynamic
> policy.

They said the same thing about Smack.

The problem there is that you have to buy into the entirety of
SELinux to implement a small bit of behavior. You have to write
a policy that takes every aspect of system behavior into account
when all you care about is loading restrictions on modules.

If you want all of SELinux you still have to define your problem
in a subject/object model. That may be possible, but in this
case at least it certainly ain't obvious.

> I'm concerned about the long term architectural impact of a proliferation
> of arbitrary hard-coded security policies in the kernel. I don't
> understand the push in this direction, frankly.

The rationale is that lots of people doing little things is
likely to get us relevant security in a reasonable amount of time.
The existing LSMs reflect 20th century technologies and use cases.
They are fine for multi-user timesharing systems. We need to move
forward to support networked gaming, phones, tablets and toasters.

>
> On Fri, 20 Sep 2013, Kees Cook wrote:
>
>> ...
>> --
>> 1.7.9.5
>>
>>
>> --
>> Kees Cook
>> Chrome OS Security
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>

2013-10-17 21:00:26

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On Thu, Oct 17, 2013 at 4:30 AM, Jarkko Sakkinen
<[email protected]> wrote:
> On Thu, Oct 17, 2013 at 07:02:17PM +1100, James Morris wrote:
>> This seems like a regression in terms of separating mechanism and policy.
>>
>> We have several access control systems available (SELinux, at least) which
>> can implement this functionality with existing mechanisms using dynamic
>> policy.
>>
>> I'm concerned about the long term architectural impact of a proliferation
>> of arbitrary hard-coded security policies in the kernel. I don't
>> understand the push in this direction, frankly.
>
> The biggest risk in LSM stacker is really to become backdoor for very product
> dilated kernel changes that are not accepted to the mainline kernel. I think
> having LSM stacker would be benefical but barrier should be set very high
> for "one-shot" modules.
>
> One big benefit that I see in LSM stacker is not at least directly security
> related. It would be perfect integration tool when you want for example
> provide Android run-time in an OS that uses AppArmor or SMACK as its security
> framework.

I think of stacking as a way to help people do quick prototyping of
security changes without getting in the way of their distro's MAC.

-Kees

--
Kees Cook
Chrome OS Security

2013-10-17 21:10:03

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On Thu, Oct 17, 2013 at 10:26 AM, Casey Schaufler
<[email protected]> wrote:
> On 10/17/2013 1:02 AM, James Morris wrote:
>> This seems like a regression in terms of separating mechanism and policy.
>>
>> We have several access control systems available (SELinux, at least) which
>> can implement this functionality with existing mechanisms using dynamic
>> policy.
>
> They said the same thing about Smack.
>
> The problem there is that you have to buy into the entirety of
> SELinux to implement a small bit of behavior. You have to write
> a policy that takes every aspect of system behavior into account
> when all you care about is loading restrictions on modules.
>
> If you want all of SELinux you still have to define your problem
> in a subject/object model. That may be possible, but in this
> case at least it certainly ain't obvious.
>
>> I'm concerned about the long term architectural impact of a proliferation
>> of arbitrary hard-coded security policies in the kernel. I don't
>> understand the push in this direction, frankly.
>
> The rationale is that lots of people doing little things is
> likely to get us relevant security in a reasonable amount of time.
> The existing LSMs reflect 20th century technologies and use cases.
> They are fine for multi-user timesharing systems. We need to move
> forward to support networked gaming, phones, tablets and toasters.

I, obviously, agree with with this. At least with Chrome OS, we've
been taking an architectural approach to security. Our trust model is
based on very different ways of doing things, and this results in
needing to trust things that are not part of the traditional
subject/object model, IMO. We trust the contents of our root
filesystem, so there's no need to do things like module signing, etc.
In many ways, IMA follows similar approaches but at a different layer
(i.e. we use dm-verity).

Chrome OS has been using this LSM for about a year now. It provides
demonstrably improved security since it draws a bright line between
uid-0 and ring-0 on systems that have a verified boot path but use
kernel modules.

-Kees

--
Kees Cook
Chrome OS Security

2013-10-18 02:25:59

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On 10/16/2013 3:43 PM, Kees Cook wrote:
> On Wed, Oct 16, 2013 at 2:42 PM, Casey Schaufler <[email protected]> wrote:
>> On 10/16/2013 1:47 PM, Tetsuo Handa wrote:
>>> Kees Cook wrote:
>>>> Any update on this? It'd be nice to have it in linux-next.
>>> What was the conclusion at LSS about multiple concurrent LSM support?
>>> If we agreed to merge multiple concurrent LSM support, there will be nothing to
>>> prevent this module from merging.
>>>
>> Yeah.
> The discussion at LSS basically centered around the catch-22 of not
> being able to stack, and not having anything to stack (since Yama got
> an hard-coded exception). So I sent this LSM as one I'd been waiting
> for stacking on. Essentially, I'm breaking the catch-22 by sending
> this. I'd like it to get into the tree so we don't have a catch-22
> about stacking any more. :)
>
>> The conclusion was that it needs to be staged because it's
>> too much to swallow all at once. I can see that. It's going
>> to be a lot of work to rearrange and rebase. That's a chunk
>> of time I don't expect to have for a while. It looks good
>> to happen, but don't hold supper for me.
> Do you want me to take a stab at it? It sounds like it was desirable
> to cut the current series into two halves? The core changes first, and
> the userspace interface changes next?

My read on it was a three phased approach:

First, move the cap "module" checks out of the other modules
and directly into security.c. There would be no "default" module.
If another module is loaded, call the hook it defines if the cap
check passes. Add /sys/kernel/security/lsm to make it easy to
find out what module (if any) is active.

Second, allow more than one LSM to get called if so requested.
Call them all, and return the error code of the last failure.
Refuse to load more than one module that uses an exclusive feature;
netlabel, secmark or XFRM.

Finally, put in all the gimmicks to decide who gets which of the
networking facilities.

Yes, If you've got the cycles to work with it I'd be happy for the help.

>
> -Kees
>

2013-10-23 00:01:33

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On Thu, 17 Oct 2013, Casey Schaufler wrote:

> On 10/17/2013 1:02 AM, James Morris wrote:
> > This seems like a regression in terms of separating mechanism and policy.
> >
> > We have several access control systems available (SELinux, at least) which
> > can implement this functionality with existing mechanisms using dynamic
> > policy.
>
> They said the same thing about Smack.
>

Nope. Smack separates mechanism and policy. The argument then was
whether we need more than one such enhanced access control system.

The issue now is that we do have several of them (SELinux, Smack,
AppArmor) which are policy-flexible, whether to regress back to adding
hard-coded security policies into the kernel.

> The problem there is that you have to buy into the entirety of
> SELinux to implement a small bit of behavior. You have to write
> a policy that takes every aspect of system behavior into account
> when all you care about is loading restrictions on modules.

You always need to consider the behavior of the system as a whole when
designing security policies.

It's a major step backwards to hard-code a series of ad-hoc policies in
the kernel. You still need to compose them somehow, and reason about the
security of the system as a whole.

> The rationale is that lots of people doing little things is
> likely to get us relevant security in a reasonable amount of time.
> The existing LSMs reflect 20th century technologies and use cases.
> They are fine for multi-user timesharing systems. We need to move
> forward to support networked gaming, phones, tablets and toasters.

You keep making these grand assertions but never provide any detail, or
any kind of evidence to back them up. Yet there are many, many examples
of how the current LSMs meet all of these needs in the 21st century, such
as Smack being adopted for Tizen, digital television etc.:

http://en.wikipedia.org/wiki/Smack

- James
--
James Morris
<[email protected]>

2013-10-23 01:03:34

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

On 10/22/2013 5:02 PM, James Morris wrote:
> On Thu, 17 Oct 2013, Casey Schaufler wrote:
>
>> On 10/17/2013 1:02 AM, James Morris wrote:
>>> This seems like a regression in terms of separating mechanism and policy.
>>>
>>> We have several access control systems available (SELinux, at least) which
>>> can implement this functionality with existing mechanisms using dynamic
>>> policy.
>> They said the same thing about Smack.
>>
> Nope. Smack separates mechanism and policy. The argument then was
> whether we need more than one such enhanced access control system.
>
> The issue now is that we do have several of them (SELinux, Smack,
> AppArmor) which are policy-flexible, whether to regress back to adding
> hard-coded security policies into the kernel.
>
>> The problem there is that you have to buy into the entirety of
>> SELinux to implement a small bit of behavior. You have to write
>> a policy that takes every aspect of system behavior into account
>> when all you care about is loading restrictions on modules.
> You always need to consider the behavior of the system as a whole when
> designing security policies.

This is right thinking. It's just not common thinking.

> It's a major step backwards to hard-code a series of ad-hoc policies in
> the kernel. You still need to compose them somehow, and reason about the
> security of the system as a whole.

It's not like we're talking about throwing out the legacy monolithic LSMs.
And let us not forget that Fedora is using SELinux and Yama. There's a
perception of value in independent security mechanisms used in combination.

>> The rationale is that lots of people doing little things is
>> likely to get us relevant security in a reasonable amount of time.
>> The existing LSMs reflect 20th century technologies and use cases.
>> They are fine for multi-user timesharing systems. We need to move
>> forward to support networked gaming, phones, tablets and toasters.
> You keep making these grand

Cool! Most people use other, less complimentary adjectives!

> assertions but never provide any detail, or
> any kind of evidence to back them up.

There are simple reasons for that. Between Tizen and the stacking patch
the work on a real Application+Service+Resource security model and the
implementation thereof has gotten insufficient attention. I have been
emphasizing work on enabling technology over work on what needs to be
enabled. Kees' approach to security development on CromeOS is the example
I'll hold up for the wave of the future.

> Yet there are many, many examples
> of how the current LSMs meet all of these needs in the 21st century, such
> as Smack being adopted for Tizen, digital television etc.:

Smack in Tizen is an example of old school distribution packaging.
Tizen is very old school in its approach. It's working out pretty
well, but Tizen is hardly a major advance in the world of OS security.

If you look at Android, their (re/miss/ab)use of the UID mechanism and
reliance on the binder to wedge an application based security policy
on top of existing mechanisms it should be pretty obvious that they
would have been better suited with new security features than with
retrofitting to existing technology.

SEAndroid is taking a refreshing approach by putting the access controls
in the middleware and by doing so enabling an appropriate implementation
on the flask architecture. I would not say that SEAndroid qualifies as an
example of adopting an LSM.

> http://en.wikipedia.org/wiki/Smack
>
>
>
> - James

2013-10-23 02:56:22

by Lucas De Marchi

[permalink] [raw]

Subject: Re: [PATCH] LSM: ModPin LSM for module loading restrictions

Hi Kees,

On Thu, Oct 3, 2013 at 6:36 PM, Kees Cook <[email protected]> wrote:
> On Fri, Oct 04, 2013 at 06:31:42AM +0900, Tetsuo Handa wrote:
>> Kees Cook wrote:
>> > +static int modpin_load_module(struct file *file)
>> > +{
>> > + struct dentry *module_root;
>> > +
>> > + if (!file) {
>> > + if (!modpin_enforced) {
>> > + report_load_module(NULL, "old-api-pinning-ignored");
>> > + return 0;
>> > + }
>> > +
>> > + report_load_module(NULL, "old-api-denied");
>> > + return -EPERM;
>> > + }
>> > +
>> > + module_root = file->f_path.mnt->mnt_root;
>> > +
>> > + /* First loaded module defines the root for all others. */
>> > + spin_lock(&pinned_root_spinlock);
>> > + if (!pinned_root) {
>> > + pinned_root = dget(module_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();
>> > + report_load_module(&file->f_path, "pinned");
>> > + return 0;
>> > + }
>> > + spin_unlock(&pinned_root_spinlock);
>>
>> Firstly loaded module is usually in initramfs whereas subsequently loaded
>> modules are usually on a hard disk partition.
>>
>> This module is not meant for PC servers, is it?
>
> This LSM is what Chrome OS uses for the module pinning logic. We do not use
> an initramfs. This LSM could also be used for devices booting entirely from
> CDROM or other R/O media.
>
> I'm open to improvements, obviously. I imagine things like delayed
> activation, where the initramfs triggers pinning in some way once it is
> done loading modules from its filesystem, etc. But since I don't have any
> real life examples of this, I'm writing the LSM as it currently is, used
> without an initramfs. :)

The way you put now as well as the code are at least different from
what it's written in the commit message. I was expecting that this
modpin would allow module loading only from a trusted partition.
Instead we are allowing to load modules from the same partition of the
first module that has been loaded, if the root is trusted. Why? What's
the relation with the root partition being trusted and the partition
where the modules reside? What's the guarantee that the first module
to load is any more trustable than the others?

Lucas De Marchi

2013-10-26 13:51:35