Re: [PATCH 0/2] sysctl: allow CLONE_NEWUSER to be disabled

From: Andy Lutomirski <luto@amacapital.net>
To: Kees Cook <keescook@chromium.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	"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>,
	"Miklos Szeredi" <mszeredi@suse.cz>,
	"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: Mon, 25 Jan 2016 15:33:39 -0800	[thread overview]
Message-ID: <CALCETrUMtn285cNTZmXUOBzx9Y2tbNGh=HaObX13OUok+-mKOw@mail.gmail.com> (raw)
In-Reply-To: <CAGXu5jLwwCw9wZA8QYNVrZ2+kHJn7G3sXQZchvqFMYj3bN=DpQ@mail.gmail.com>

On Mon, Jan 25, 2016 at 2:34 PM, Kees Cook <keescook@chromium.org> wrote:
> 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.
>
>> So broken code, not willing to fix.  No. We are not merging this sysctl.
>
> I think you're jumping to conclusions. :)
>
> 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.
>
> Andy, given your interest in this feature, and my explanation of the
> CAP_SYSADMIN check, what are your thoughts?
>

I think the sysctl sucks, but I don't have a better idea, so I think
it should go in.  There's clearly demand.

A better change would be to have an option to tighten up what
namespaces can be manipulated in which manner.  If anyone wants to
step up and do that, it sounds useful.  Denying CAP_NET_ADMIN in an
unprivileged user ns would address one piece of attack surface.  There
are probably others.

*However*, I think that trying to protect against a hypothetical
attacker with uid == global root who has procfs access but doesn't
have CAP_SYS_ADMIN isn't important enough to be worth introducing yet
another bad capable() call.

Whoever wants to tilt at the windmill of fixng sysctl permissions can
address all of them, and then maybe this sysctl would be worth
tightening up.

--Andy