Message-ID: <CALCETrXFPbKSU6x2f4BjdFeUoKnCKjOUs=PUMkYwck-nX4zyvw@mail.gmail.com>
Date:	Mon, 2 Nov 2015 11:45:50 -0800
From:	Andy Lutomirski <luto@...capital.net>
To:	Klaus Ethgen <Klaus+lkml@...gen.de>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Richard Weinberger <richard.weinberger@...il.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Christoph Lameter <cl@...ux.com>,
	Andy Lutomirski <luto@...nel.org>,
	Serge Hallyn <serge.hallyn@...ntu.com>,
	Kees Cook <keescook@...omium.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [KERNEL] Re: Kernel 4.3 breaks security in systems using capabilities

On Mon, Nov 2, 2015 at 11:16 AM, Klaus Ethgen <Klaus+lkml@...gen.de> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
> Hi,
>
> Am Mo den  2. Nov 2015 um 19:50 schrieb Andy Lutomirski:
>> >> I read recently about patch 58319057b7847667f0c9585b9de0e8932b0fdb08
>> >> which made it into kernel 4.3 recently. And I have to say that I was
>> >> shocked on how could such a patch that breaks normal use of capabilities
>> >> make it into the kernel.
>> >>
>> >> Usually I have set very own crafted capabilities set to files instead of
>> >> having them SUID root. With that, I have a comparable set of inheritable
>> >> capabilities set for limited users. That allows me to nearly drop all
>> >> SUID binaries and replace it by only giving the processes the
>> >> capabilities they need but only if the users are allowed to act with
>> >> that capabilities. Especially, and that is important, it inhibit any
>> >> leak of rights to any forked process, be it indented or by a security
>> >> problem of the binary.
>> >>
>> >> With the patch above, any process that is spawned by such a program will
>> >> inherit the raised capabilities if it has no own filecapabilities set.
>> >> Even worse, even every user made tool can be target for such
>> >> escalations! That drives the benefits in security of capabilities over
>> >> SUID ad-absurdum.
>>
>> Can you describe what it is that you think changed in 4.3 that breaks
>> anything?  The behavior of inheritable capabilities shouldn't have
>> changed.
>
> Well, the think that changed is that the ambient capabilities can be set
> by any process if the pI and pE are matching for a process. But then,
> that capabilities are inherited via execve to even programs that don't
> have any capability setting. That allows every subprocess to (mis-)use
> it.

That's true.  I don't see why it's a problem, given that the parent
can already use or mis-use it however it wants, and the child won't
get the capability without explicit action by the parent.

>
>> >> Let me add here, that I disagree with Andy Lutomirski about the
>> >> usefulness of capability inheritance in kernels before that patch. They
>> >> was fully usefull to only allow selective capabilities if both, the
>> >> binary and the user was allowed to use it. I never want to have any
>> >> capabilities for processes that I did not allow them to have. Even
>> >> worse, I never want any capabilities allowed for any shell. It is
>> >> horrible to even think about such a possibility!.
>> >>
>> >>> Users with nonzero pA are unlikely to unintentionally leak that
>> >>> capability. If they run programs that try to drop privileges, dropping
>> >>> privileges will still work.
>> >>
>> >> Even that is naiv. There are only few programs out there that do
>> >> actively drop privileges. Most are agnostic about capabilities. But this
>> >> crappy patch introduce a need for _every_ tool to drop all capabilities
>> >> right after start to stay in a secure system.
>> >>
>>
>> I don't know what you mean.  Concrete examples are welcome.
>
> Am Mo den  2. Nov 2015 um 19:52 schrieb Linus Torvalds:
>> >> With the patch above, any process that is spawned by such a program will
>> >> inherit the raised capabilities if it has no own filecapabilities set.
>>
>> Do you actually have a real example of this?
>
> Very simple example:
> ~> getcap =mount
> /bin/mount = cap_dac_override,cap_sys_admin+ei
> ~> ls -l =mount
> - -rwxr-xr-x 1 root root 40000 Sep 17 15:55 /bin/mount
> ~> getpcaps $$
> Capabilities for `5509': = cap_dac_override,cap_net_raw,cap_sys_rawio,cap_sys_admin+i
>
> What allows me to use mount for mounting shares with user flag without
> the need of having a SUID mount. Nice and with a good security level.
> Even if the binary has some flaw that allows to exec some unrelated
> stuff, there is no harm as after execve, all capabilities are set to 0.
> With the alternative approach of having /bin/mount SUID root (as it is
> normally), root rights would spread to unrelated tools in such a case.

That all sounds fine, except that, if you find a security flaw (in
mount of all things -- mount really shouldn't be exposed to untrusted
input in the first place, and if you find a flaw that lets you coerce
mount into execing unrelated things, you can probably also coerce
mount into acting directly on your behalf, but that's irrelevant
here.)

>
> With pA set (what can be even done in mount itself), the security
> problem would be the same as havin mount SUID root.

No, the security problem would be the same as if you compromised
whatever set pA in the first place.  In your example, that process (a)
doesn't actually exist and (b) wouldn't have full root rights anyway.

>
> Well, not really. With the former approach, the capabilities in pI can
> only be used with a specific tool that the admin allowed to inherit.
> With pA, every tool can inherit that.
>
> And please ask if I described something not understandable as my main
> language is not english. I even might be wrong in understanding the pA.
> but I don't think so currently.

If your mount binary used prctl to set CAP_SYS_MOUNT in pA, then its
children would have CAP_SYS_MOUNT.  Similarly, if your mount binary
used, say, dlopen(), then the shared library would have CAP_SYS_MOUNT
regardless of what the kernel did.

So I still don't see the problem.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists