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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5jJGvdO789bu6TO48TwoW4muqYkFuz3NMEY6JoGX2KEW1A@mail.gmail.com>
Date:	Mon, 28 Mar 2016 14:58:08 -0700
From:	Kees Cook <keescook@...omium.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	James Morris <jmorris@...ei.org>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Kalle Valo <kvalo@...eaurora.org>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	Joe Perches <joe@...ches.com>,
	Guenter Roeck <linux@...ck-us.net>,
	Jiri Slaby <jslaby@...e.com>, Paul Moore <pmoore@...hat.com>,
	Stephen Smalley <sds@...ho.nsa.gov>,
	Mimi Zohar <zohar@...ux.vnet.ibm.com>,
	Casey Schaufler <casey@...aufler-ca.com>,
	Andreas Gruenbacher <agruenba@...hat.com>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Ulf Hansson <ulf.hansson@...aro.org>,
	Vitaly Kuznetsov <vkuznets@...hat.com>,
	linux-security-module <linux-security-module@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 5/5] LSM: LoadPin for kernel file loading restrictions

On Mon, Mar 28, 2016 at 2:38 PM, Andrew Morton
<akpm@...ux-foundation.org> wrote:
> On Mon, 28 Mar 2016 14:14:22 -0700 Kees Cook <keescook@...omium.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@...omium.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