Date: Wed, 29 Nov 2017 14:14:15 -0800 From: Linus Torvalds <torvalds@...ux-foundation.org> To: Kees Cook <keescook@...omium.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 1:17 PM, Kees Cook <keescook@...omium.org> wrote: > > So, what we have now is that the permission verification already > happens at and around the existing request_module() calls. Usually, yes. I liked the "request_module_cap()" interface partly because that made the net/core/dev_ioctl.c ones more explicit, and maybe it could be convenient if we make other places do similar things. I was hoping some other users could be converted, but grepping around, there's no obvious cases. There is tcp_cong.c and tcp_ulp.c, but they want some extra locking in between the checking.. > It still sounds like you'd like to see an explicit change, similar to > the proposed request_module_cap(), that identifies the privilege > expectations on a per-call-site basis. How about this plan: Yes. I'd be perfectly happy to have a long-range plan where the existing "request_module()" ends up requiring more capabilities. I just don't think it's a good first step, exactly because *if* it's a first step, it basically has to be disabled by default. And once you disable it by default, and it becomes purely opt-in, that means that nothing will change for most cases. Some embedded people that do their own thing (ie Android) might change, but normal distributions probably won't. Yes, Android may be 99% of the users, and yes, the embedded world in general needs to be secure, but I'd still like this to be something that helps _everybody_. So: > 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. > 2) Convert known unprivileged callers to use request_module_cap(0, ...) 0 is CAP_CHOWN, so it would have to be -1. 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 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". 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. Anyway, I'm not saying that is necessarily something we should do, but I do suspect that we could adapt to modern systems without having to have tons of magic settings, and try to be as strict as possible without breaking them. Because I dislike "system tuning" in general. I hate knobs that do kernel performance tuning - we try very hard to just DTRT wrt sizing hashes etc instead of expecting the system admin to set flags. And I think we can try to avoid some system tuning in this area too. 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. 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. That mainly leaves the protocol ones we need to look out for, I suspect. > 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. Linus
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.