Linux Security Modules development
 help / color / mirror / Atom feed
From: geokozey@mailfence.com (Geo Kozey)
To: linux-security-module@vger.kernel.org
Subject: [kernel-hardening] Re: [PATCH v5 next 5/5] net: modules: use request_module_cap() to load 'netdev-%s' modules
Date: Tue, 28 Nov 2017 13:16:59 +0100 (CET)	[thread overview]
Message-ID: <1100603534.56586.1511871419952@ichabod.co-bxl> (raw)
In-Reply-To: <CA+55aFzx2eM+wVrjQEfkE-G8E9OVLcdo6vk0fDMwZNu2sh1oCQ@mail.gmail.com>

> From: Linus Torvalds <torvalds@linux-foundation.org>
> Sent: Mon Nov 27 23:04:58 CET 2017
> To: Djalal Harouni <tixxdz@gmail.com>
> Cc: Kees Cook <keescook@chromium.org>, Andy Lutomirski <luto@kernel.org>, Andrew Morton <akpm@linux-foundation.org>, Luis R. Rodriguez <mcgrof@kernel.org>, James Morris <james.l.morris@oracle.com>, Ben Hutchings <ben.hutchings@codethink.co.uk>, Solar Designer <solar@openwall.com>, Serge Hallyn <serge@hallyn.com>, Jessica Yu <jeyu@kernel.org>, Rusty Russell <rusty@rustcorp.com.au>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, LSM List <linux-security-module@vger.kernel.org>, kernel-hardening at lists.openwall.com <kernel-hardening@lists.openwall.com>, Jonathan Corbet <corbet@lwn.net>, Ingo Molnar <mingo@kernel.org>, David S. Miller <davem@davemloft.net>, Network Development <netdev@vger.kernel.org>, Peter Zijlstra <peterz@infradead.org>
> Subject: [kernel-hardening] Re: [PATCH v5 next 5/5] net: modules: use request_module_cap() to load 'netdev-%s' modules
> 
> 
> On Mon, Nov 27, 2017 at 1:41 PM, Djalal Harouni <tixxdz@gmail.com> wrote:
> >
> > However, we are trying hard to abstract some semantics that are easy
> > to grasp, we are mutating capabilities and seccomp to have an
> > abstracted "yes/no" options for our endusers.
> 
> Yes.
> 
> Sadly, it looks like we actually do have users that just expect to
> load modules dynamically without any capabilities at all.
> 
> So we can't actually disallow it by default at all, which imho makes
> this security option essentially useless.
> 
> A security option that people can't use without breaking their system
> is pointless.
> 
> We saw that with SELinux - people ended up just disabling it for
> _years_, simply because it ended up breaking so much in practice. And
> yes, it got fixed eventually, but at an incredibly high maintenance
> cost of all the crazy rules databases.
> 
> > Alright, but I guess we are stuck, is there something better on how we
> > can do this or describe this ?
> 
> So I wonder if we can perhaps look at the places that actually do
> "requerst_module()", and start filtering them on that basis.
> 
> Some of them will already have checked for capabilities.
> 
> Others clearly expect to juist work even _without_ capabilities (ie
> the bluetoothd case).
> 
> So the whole "let's add a global config option" model is broken. There
> is no possible global rule. It will break things, which in turn mean
> that people won't turn it on (and we can't turn it on by default),
> which in turn makes this pointless.
> 
> In other words, I really think that anything that just adds a mode
> flag cannot work.
> 
> So instead of having one "modules_autoload_mode" thing, maybe the
> individual requerst_module() cases need to simply be audited.
> 
> Put another way: I think the part of your patch series that does that
> "request_module_cap()" and makes the netdev modules use it is a good
> addition.
> 
> It's the "mode" part I really don't agree with, because apparently we
> really need to default it to permissive.
> 
> So how about instead:
> 
>  - add that "request_module_cap()" and make the networking code that
> already uses CAP_ADMIN_NET use it.
> 
>  - make "request_module()" itself default to being
> "request_module_cap(CAP_SYS_MODULE,..)"
> 
>  - make sure that when the capability check fails, we print an error
> message, and then for the ones that trigger, we will audit them and
> see if it's ok.
> 
> Because that "mode" flag defaulting to off will just mean that the
> default case will remain the existing unsafe one, and that's bad.
> 
> Opt-in really doesn't work. We've done it.
> 
> Global flags for varied behavior really doesn't work. We've done that
> too. Different cases want different behavior, the global flag is just
> fundamentally broken.
> 
>               Linus
> ----------------------------------------

The only reason this is off by default is kernel rule No 1. Most of new security features are off or WARN_ON because of it otherwise bad things happen on mailinglists :) .

It seems to me that you demand an impossible job here. Don't break userspace and don't disable by default. It can't happen due to years of technical debt. I mean everyone prefers default-on but we can choose best alternative instead of maintaining status quo.

Userspace can be configured in a way which is compatible with those changes being on the same as it can be configured to work with selinux. That means on distro level or sysadmin level it's a valuable tool. It's better than nothing and it's better than using some out-of-tree patches instead. Switching one sysctl would make their life easier.

There is demand for kernel hardening stuff and default-off behavior is good compromise between breaking changes and nothing at all. The kernel has very diverse users group with different needs and capabilities so there is no one size fits all. Targeting all of them will often end in nothing useful to anyone.

Yours sincerely

G. K.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2017-11-28 12:16 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-27 17:18 [PATCH v5 next 0/5] Improve Module autoloading infrastructure Djalal Harouni
2017-11-27 17:18 ` [PATCH v5 next 1/5] modules:capabilities: add request_module_cap() Djalal Harouni
2017-11-27 18:48   ` Randy Dunlap
2017-11-27 21:35     ` Djalal Harouni
2017-11-28 19:14   ` Luis R. Rodriguez
2017-11-28 20:11     ` Kees Cook
2017-11-28 21:16       ` Luis R. Rodriguez
2017-11-28 21:33         ` Djalal Harouni
2017-11-28 22:18           ` Luis R. Rodriguez
2017-11-28 22:52             ` Djalal Harouni
2017-11-28 21:39         ` Kees Cook
2017-11-28 22:12           ` Luis R. Rodriguez
2017-11-28 22:18             ` Kees Cook
2017-11-28 22:48               ` Luis R. Rodriguez
2017-11-29  7:49                 ` Michal Kubecek
2017-11-29 13:46           ` Alan Cox
2017-11-29 14:50             ` David Miller
2017-11-29 15:54               ` Theodore Ts'o
2017-11-29 15:58                 ` David Miller
2017-11-29 16:29                   ` Theodore Ts'o
2017-11-29 22:45                   ` Linus Torvalds
2017-11-30  0:06                     ` Kees Cook
2017-11-29 17:28                 ` Serge E. Hallyn
2017-11-30  0:35                   ` Theodore Ts'o
2017-11-30 17:17                     ` Serge E. Hallyn
2017-11-28 20:18     ` Djalal Harouni
2017-11-27 17:18 ` [PATCH v5 next 2/5] modules:capabilities: add cap_kernel_module_request() permission check Djalal Harouni
2017-11-30  2:05   ` Luis R. Rodriguez
2017-11-27 17:18 ` [PATCH v5 next 3/5] modules:capabilities: automatic module loading restriction Djalal Harouni
2017-11-30  1:23   ` Luis R. Rodriguez
2017-11-30 12:22     ` Djalal Harouni
2017-11-27 17:18 ` [PATCH v5 next 4/5] modules:capabilities: add a per-task modules auto-load mode Djalal Harouni
2017-11-27 17:18 ` [PATCH v5 next 5/5] net: modules: use request_module_cap() to load 'netdev-%s' modules Djalal Harouni
2017-11-27 18:44   ` Linus Torvalds
2017-11-27 21:41     ` Djalal Harouni
2017-11-27 22:04       ` Linus Torvalds
2017-11-27 22:59         ` Kees Cook
2017-11-27 23:14           ` Linus Torvalds
2017-11-27 23:19             ` Kees Cook
2017-11-27 23:35               ` Linus Torvalds
2017-11-28  1:23             ` Kees Cook
2017-11-28 12:16         ` Geo Kozey [this message]
2017-11-28 19:32           ` [kernel-hardening] " Theodore Ts'o
2017-11-28 20:08             ` Kees Cook
2017-11-28 20:12               ` Linus Torvalds
2017-11-28 20:20                 ` Kees Cook
2017-11-28 20:33                   ` Linus Torvalds
2017-11-28 21:10                     ` Djalal Harouni
2017-11-28 21:33                     ` Kees Cook
2017-11-28 23:23                       ` Theodore Ts'o
2017-11-28 23:29                         ` Kees Cook
2017-11-28 23:49                           ` Theodore Ts'o
2017-11-29  0:18                             ` Kees Cook
2017-11-29  6:36                               ` Theodore Ts'o
2017-11-29 14:46                             ` Geo Kozey
2017-12-01 15:22                             ` Marcus Meissner
2017-11-28 23:53                         ` Djalal Harouni
2017-11-28 21:51                     ` Geo Kozey
2017-11-28 23:51                       ` Linus Torvalds
2017-11-29  0:17                         ` Linus Torvalds
2017-11-29  0:26                           ` Kees Cook
2017-11-29  0:50                             ` Linus Torvalds
2017-11-29  4:26                               ` Eric W. Biederman
2017-11-29 18:30                               ` Kees Cook
2017-11-29 18:46                                 ` Linus Torvalds
2017-11-29 18:53                                   ` Linus Torvalds
2017-11-29 21:17                                   ` Kees Cook
2017-11-29 22:14                                     ` Linus Torvalds
2017-11-30  0:44                                       ` Kees Cook
2017-11-30  2:08                                         ` Linus Torvalds
2017-11-30  6:51                                       ` Daniel Micay
2017-11-30  8:50                                         ` Djalal Harouni
2017-11-30 14:16                                           ` Theodore Ts'o
2017-11-30 14:51                                             ` Djalal Harouni
2017-12-01  6:39                                           ` Daniel Micay
2017-11-29 15:28                           ` Geo Kozey
2017-11-27 18:41 ` [PATCH v5 next 0/5] Improve Module autoloading infrastructure Linus Torvalds
2017-11-27 19:02   ` Linus Torvalds
2017-11-27 19:12     ` Linus Torvalds
2017-11-27 21:31       ` Djalal Harouni
2017-11-27 19:14   ` David Miller
2017-11-27 22:31     ` James Morris
2017-11-27 23:04       ` Kees Cook
2017-11-27 23:44         ` James Morris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1100603534.56586.1511871419952@ichabod.co-bxl \
    --to=geokozey@mailfence.com \
    --cc=linux-security-module@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox