linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: "Andy Lutomirski" <luto@amacapital.net>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Richard Weinberger" <richard@nod.at>,
	"Robert Święcki" <robert@swiecki.net>,
	"Dmitry Vyukov" <dvyukov@google.com>,
	"David Howells" <dhowells@redhat.com>,
	"Kostya Serebryany" <kcc@google.com>,
	"Alexander Potapenko" <glider@google.com>,
	"Eric Dumazet" <edumazet@google.com>,
	"Sasha Levin" <sasha.levin@oracle.com>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kernel-hardening@lists.openwall.com"
	<kernel-hardening@lists.openwall.com>
Subject: Re: [PATCH 0/2] sysctl: allow CLONE_NEWUSER to be disabled
Date: Tue, 26 Jan 2016 08:37:28 -0800	[thread overview]
Message-ID: <CAGXu5jJzdK1JHkmqsbuaeY9XDJSF97MNTrnGDr66py7+-zixgw@mail.gmail.com> (raw)
In-Reply-To: <87zivtj5tv.fsf@x220.int.ebiederm.org>

On Mon, Jan 25, 2016 at 8:57 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Kees Cook <keescook@chromium.org> writes:
>
>> On Mon, Jan 25, 2016 at 11:33 AM, Eric W. Biederman
>> <ebiederm@xmission.com> wrote:
>>> Kees Cook <keescook@chromium.org> writes:
>>>>
>>>> Well, I don't know about less weird, but it would leave a unneeded
>>>> hole in the permission checks.
>>>
>>> To be clear the current patch has my:
>>>
>>> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>>>
>>> The code is buggy, and poorly thought through.  Your lack of interest in
>>> fixing the bugs in your patch is distressing.
>>
>> I'm not sure where you see me having a "lack of interest". The
>> existing cap-checking sysctls have a corner-case bug, which is
>> orthogonal to this change.
>
> That certainly doesn't sound like you have any plans to change anything
> there.

Again, not sure why you think that. My primary role in kernel
development is fixing or helping coordinate fixing of security issues
and features. I already acknowledged the issue (it is a corner case,
and no one seems to debate that). I'm working based on priorities; I
have a long list of things to do. :)

>>> So broken code, not willing to fix.  No. We are not merging this sysctl.
>>
>> I think you're jumping to conclusions. :)
>
> I think I am the maintainer.

Sure, no debate there. In fact, I'm certain you're the maintainer. :)

> What you are proposing is very much something that is only of interst to
> people who are not using user namespaces.  It is fatally flawed as
> a way to avoid new attack surfaces for people who don't care as the
> sysctl leaves user namespaces enabled by default.  It is fatally flawed
> as remediation to recommend to people to change if a new user namespace
> related but is discovered.  Any running process that happens to be
> created while user namespace creation was enabled will continue to
> exist.  Effectively a reboot will be required as part of a mitigation.
> Many sysadmins will get that wrong.

I disagree. The same kinds of issues exist with any of the *_restrict
sysctls: if you turn them on later, things that happened before are
still going to be a problem. You'll have already leaked a kernel base
address, etc. This would be no different.

I'm open to having this sysctl kill all CLONE_NEWUSERed process trees,
if you think that'll be more useful?

> I can't possibly see your sysctl as proposed achieving it's goals.  A
> person has to be entirely too aware of subtlety and nuance to use it
> effectively.

Again, I disagree. There are plenty of people who want to have user ns
disabled. This gives them the knob to do so.

>> This feature is already implemented by two distros, and likely wanted
>> by others. We cannot ignore that. The sysctl default doesn't change
>> the existing behavior, so this doesn't get in your way at all. Can you
>> please respond to my earlier email where I rebutted each of your
>> arguments against it? Just saying "no" and putting words in my mouth
>> isn't very productive.
>
> Calling people who make mistakes insane is not a rebuttal.  In security

I said this:

>> Any admin that decides to just turn off CLONE_NEWUSER in the middle of
>> still using it is insane. I don't think this breeds any false sense of
>> security as most sysctls are set at boot time.

I was arguing that admins that use the sysctl are not going to be the
admins that are using containers already. I didn't mean it as "making
a mistake is insane" but rather "it would appear that a person using
both would be seeking opposing goals".

> usability matters, and your sysctl has low usability.

Unsurprisingly, we disagree here too. This sysctl serves as an attack
surface reduction tool. I never saw it as a way to evict existing
containers.

> Further you seem to have missed something crucial in your understanding.
> As was explained earlier the sysctl was added to ubuntu to allow early
> adopters to experiment not as a long term way of managing user
> namespaces.

It's not about management: the audience of the sysctl is only those
that are not using user namespaces. Providing attack surface reduction
tools to admins is a net win for Linux security as a whole. We both
want the same thing: a safer Linux environment. There's no debate that
having user ns exposes a larger attack surface than not having it.
Being able to disable it for people not interested in using user ns
means a reduction in their attack surface.

> What sounds like a generally useful feature that would cover your use
> case and many others is a per user limit on the number of user
> namespaces users may create.

That sounds fine to me. Are you thinking of a new RLIMIT, or something
else? I don't need a sysctl, I just want a way to effectively disable
user ns.

-Kees


-- 
Kees Cook
Chrome OS & Brillo Security

  parent reply	other threads:[~2016-01-26 16:37 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-22 22:39 [PATCH 0/2] sysctl: allow CLONE_NEWUSER to be disabled Kees Cook
2016-01-22 22:39 ` [PATCH 1/2] sysctl: expand use of proc_dointvec_minmax_sysadmin Kees Cook
2016-01-23  3:10   ` Eric W. Biederman
2016-01-23 22:25     ` [kernel-hardening] " Jann Horn
2016-01-24  1:20       ` Eric W. Biederman
2016-01-24  1:43         ` Al Viro
2016-01-24  1:56           ` Jann Horn
2016-01-24  6:02             ` Eric W. Biederman
2016-01-24  6:32               ` Jann Horn
2016-01-24  6:44                 ` Eric W. Biederman
2016-01-22 22:39 ` [PATCH 2/2] sysctl: allow CLONE_NEWUSER to be disabled Kees Cook
2016-01-22 22:47   ` Robert Święcki
2016-01-22 22:50     ` Kees Cook
2016-01-22 22:55       ` Robert Święcki
2016-01-22 23:00         ` Kees Cook
2016-01-23  0:44           ` Serge Hallyn
2016-01-23  0:44           ` Serge Hallyn
2016-01-23  0:59           ` [kernel-hardening] " Ben Hutchings
2016-01-24 20:59             ` Kees Cook
2016-01-24 22:20               ` Andy Lutomirski
2016-01-25 18:51                 ` Kees Cook
2016-01-22 22:49 ` [PATCH 0/2] " Richard Weinberger
2016-01-23  3:02 ` Eric W. Biederman
2016-01-24 20:57   ` Kees Cook
2016-01-26  7:38     ` [kernel-hardening] " Serge Hallyn
2016-01-24 22:22   ` Andy Lutomirski
2016-01-25 18:51     ` Kees Cook
2016-01-25 18:53       ` Andy Lutomirski
2016-01-25 18:56         ` Kees Cook
2016-01-25 19:33           ` Eric W. Biederman
2016-01-25 22:34             ` Kees Cook
2016-01-25 23:33               ` Andy Lutomirski
2016-01-26  2:27               ` [kernel-hardening] " Daniel Micay
2016-01-26  4:57               ` Eric W. Biederman
2016-01-26 14:38                 ` Josh Boyer
2016-01-26 14:46                   ` Austin S. Hemmelgarn
2016-01-26 14:56                     ` Josh Boyer
2016-01-26 17:20                       ` [kernel-hardening] " Serge Hallyn
2016-01-26 19:56                         ` Josh Boyer
2016-01-26 20:11                           ` Austin S. Hemmelgarn
2016-01-26 17:15                   ` Serge Hallyn
2016-01-26 18:09                     ` Austin S. Hemmelgarn
2016-01-26 18:27                       ` Andy Lutomirski
2016-01-26 18:45                         ` Austin S. Hemmelgarn
2016-01-26 23:15                         ` Kees Cook
2016-01-26 23:13                     ` Kees Cook
2016-01-27 10:27                       ` Eric W. Biederman
2016-01-27 12:32                         ` Austin S. Hemmelgarn
2016-01-28 14:41                         ` Robert Święcki
2016-01-26 16:37                 ` Kees Cook [this message]
2016-01-28  8:56                 ` Serge E. Hallyn
2016-01-28 12:53                   ` Austin S. Hemmelgarn

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=CAGXu5jJzdK1JHkmqsbuaeY9XDJSF97MNTrnGDr66py7+-zixgw@mail.gmail.com \
    --to=keescook@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=dvyukov@google.com \
    --cc=ebiederm@xmission.com \
    --cc=edumazet@google.com \
    --cc=glider@google.com \
    --cc=kcc@google.com \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=richard@nod.at \
    --cc=robert@swiecki.net \
    --cc=sasha.levin@oracle.com \
    --cc=viro@zeniv.linux.org.uk \
    /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;
as well as URLs for NNTP newsgroup(s).