[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 29 Nov 2017 16:44:45 -0800
From: Kees Cook <keescook@...omium.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Djalal Harouni <tixxdz@...il.com>, Jessica Yu <jeyu@...nel.org>,
LSM List <linux-security-module@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: Re: [PATCH v5 next 5/5] net: modules: use
request_module_cap() to load 'netdev-%s' modules
On Wed, Nov 29, 2017 at 2:14 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Wed, Nov 29, 2017 at 1:17 PM, Kees Cook <keescook@...omium.org> wrote:
>> 1) Add request_module_cap(required_cap, module_name_prefix, fmt, fmt_args...)
>>
>> 2) Convert known privileged-but-not-CAP_SYS_MODULE request_module()
>> callers to request_module_cap(the_needed_cap, prefix, ...)
>
> Yes. The upside seems to be very limited here, but at least it makes
> the users that use CAP_NET_ADMIN instead of CAP_SYS_MODULE able to
> specify so.
Yeah, agreed; it's limited in scope.
>> 2) Convert known unprivileged callers to use request_module_cap(0, ...)
>
> 0 is CAP_CHOWN, so it would have to be -1.
Oops, yes, think-o.
> And I wouldn't actually want to see that as-is. Not only would I not
> want to see people have that "-1" in random driver subsystems, I'd
> much prefer to have actual helper naming that descibes why something
> is ok
So, if some catch-all is needed, maybe request_module_unpriv(...), but
I think it should be possible to identify SOME kind of defining
privilege to check.
> Because as mentioned, I think there are valid permission reasons that
> are _not_ about capabilities that make you able to load a module.
>
> If you can open a character device node, then "misc_open()" will do
>
> request_module("char-major-%d-%d", MISC_MAJOR, minor);
>
> and there is nothing about capabilities in the CAP_SYS_MODULE sense
> about the user. But the user _did_ have the privileges to open that
> character device file.
>
> That's why I suggested something like request_module_dev(): it's not
> at all the same thing as request_module_cap(-1, ...), saying "I don't
> need/have a capability". It's saying something else entirely, it's
> basically saying "I have the right based on device permissions".
>
> And something like request_module_dev() might even have real semantic
> meaning, exactly because it says "this module request comes from
> trying to open a device node".
I just want to make sure I'm visualizing this correctly. Using the
misc_open() example, you're thinking something like:
request_module_dev(file, "char-major-%d-%d", MISC_MAJOR, minor);
which would then do a verification that file->f_cred matched
current_cred()? (For misc_open(), this is obviously going to always be
okay, but other cases may be further from the fops open handler...)
> Why would that be? If we know we're on a system where /dev is
> auto-populated through udev, then the device nodes should have been
> created by the drivers, not the other way around. So we might even
> have a rule that notices that automatically, and simply disables
> request_module_dev() entirely.
Right, we have both directions -- hotplug module loading ("I saw this
PCI alias, load something! Oh! It needs some fancy crypto too!") and
on-use module loading. It seems like on-use module loading would get
covered by request_module_dev(), and the hotplug case would always be
privileged. Anyway... exploring this with real code is clearly
warranted.
> I suspect that for a lot of our existing request_module() cases, they
> really are pretty trivial. In most cases, it's probably really about
> whether you have the hardware or not.
Right, and those would all be privileged, etc.
> So for the hardware driver cases, either the hardware enumerates
> itself, or it is presumably set up by the system scripts anyway, and
> CAP_SYS_MODULE is all fine. The "open device node" case is one special
> case, though.
Yeah, going through the call sites and asking "where does the
privilege for loading this module derive from?" for each one will help
shape the resulting API changes.
> That mainly leaves the protocol ones we need to look out for, I suspect.
This is where a lot of the exposure really comes from. socket()
triggers a bunch of stuff, but doesn't have an obvious privilege
associated with it... while it already does the name templates, maybe
add request_module_socket() just to explicitly mark it?
>> 3) Add WARN_RATELIMIT for request_module() calls without
>> CAP_SYS_MODULE to shake out other places where request_module_cap() is
>> needed.
>
> Yes.
>
> And this is where I hope that there really aren't actually all that
> many cases that will warn, and that it's hopefully easy to simply just
> look at a handful of reports and say "ok, that case is obviously
> fine".
>
> And I may be wrong.
>
>> 4) Adapt the original patch series to add the per-process flag that
>> can block privilege elevations.
>
> Yes.
Okay, excellent. Hopefully we haven't scared off Djalal, and hopefully
Jessica doesn't think this is all insane. :)
Thanks!
-Kees
--
Kees Cook
Pixel Security
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
