Message-ID: <20170523074808.GA4562@openwall.com>
Date:   Tue, 23 May 2017 09:48:08 +0200
From:   Solar Designer <solar@...nwall.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Djalal Harouni <tixxdz@...il.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        LSM List <linux-security-module@...r.kernel.org>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>,
        Andy Lutomirski <luto@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Rusty Russell <rusty@...tcorp.com.au>,
        "Serge E. Hallyn" <serge@...lyn.com>, Jessica Yu <jeyu@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        James Morris <james.l.morris@...cle.com>,
        Paul Moore <paul@...l-moore.com>,
        Stephen Smalley <sds@...ho.nsa.gov>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Ingo Molnar <mingo@...nel.org>,
        Linux API <linux-api@...r.kernel.org>,
        Dongsu Park <dpark@...teo.net>,
        Casey Schaufler <casey@...aufler-ca.com>,
        Jonathan Corbet <corbet@....net>,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Zendyani <zendyani@...il.com>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Ben Hutchings <ben.hutchings@...ethink.co.uk>
Subject: Re: [kernel-hardening] [PATCH v4 next 0/3] modules: automatic module loading restrictions

> >>> On Mon, May 22, 2017 at 2:08 PM, Solar Designer <solar@...nwall.com> wrote:
> >>> > On Mon, May 22, 2017 at 01:57:03PM +0200, Djalal Harouni wrote:
> >>> >> *) When modules_autoload_mode is set to (2), automatic module loading is
> >>> >> disabled for all. Once set, this value can not be changed.
> >>> >
> >>> > What purpose does this securelevel-like property ("Once set, this value
> >>> > can not be changed.") serve here?  I think this mode 2 is needed, but
> >>> > without this extra property, which is bypassable by e.g. explicitly
> >>> > loaded kernel modules anyway (and that's OK).

On Mon, May 22, 2017 at 04:07:56PM -0700, Kees Cook wrote:
> I'm on the fence. For modules_disabled and Yama, it was tied to
> CAP_SYS_ADMIN, basically designed to be a at-boot setting that could
> not later be undone by an attacker gaining that privilege, keeping
> them out of either kernel memory or existing user process memory.
> Here, it's CAP_SYS_MODULE... it's hard to imagine the situation where
> a CAP_SYS_MODULE-capable process could write to this sysctl but NOT
> issue direct modprobe requests, but it's _possible_ via crazy symlink
> games to trick capable processes into writing to sysctls. We've seen
> this multiple times before, and it's a way for attackers to turn a
> single privileged write into a privileged exec.

OK, tricking a process via crazy symlink games is finally a potentially
valid reason.  The question then becomes: are there perhaps so many
other important sysctl's, disk files, etc. (which the vulnerable capable
process could similarly be tricked into writing) so that specifically
resetting modules_autoload_mode isn't particularly lucrative?  I think
that the answer to that is usually yes.  Another related question: do we
really want to inconsistently single out a handful of sysctl's for this
kind of extra protection?  I think not.

I agree there are some other settings where being unable to reset them
makes sense, but I think this isn't one of those.

> I might turn the question around, though: why would we want to have it
> changeable at this setting?

Convenience for the sysadmin - being able to correct one's error (e.g.,
wrong order of shell commands), respond to new findings (thought module
autoloading was unneeded after some point, then found out some software
relies on it), change one's mind, reuse a system differently than
originally intended without a forced reboot.

> I'm fine leaving that piece off, either way.

I'm also fine with either decision.  I just thought I'd point out what
looked weird to me.

I think this is an important patch that should get in, but primarily
for modules_autoload_mode=1, which many distros could make the default
(and maybe the kernel eventually should?)

For modules_autoload_mode=2, we already seem to have the equivalent of
modprobe=/bin/true (or does it differ subtly, maybe in return values?),
which I already use at startup on a GPU box like this (preloading
modules so that the OpenCL backends wouldn't need the autoloading):

nvidia-smi
nvidia-modprobe -u -c=0
#modprobe nvidia_uvm
#modprobe fglrx

sysctl -w kernel.modprobe=/bin/true
sysctl -w kernel.hotplug=/bin/true

but it's good to also have this supported more explicitly and more
consistently through modules_autoload_mode=2 while we're at it.  So I
support having this mode as well.  I just question the need to have it
non-resettable.

Alexander

Powered by blists - more mailing lists