Message-ID: <CALCETrUwi-_q2GzYquKG4u5K=Zm+9X57C4vStSPFRAtwBHi9uA@mail.gmail.com>
Date:	Wed, 27 May 2015 15:44:44 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Serge Hallyn <serge@...lyn.com>
Cc:	Andy Lutomirski <luto@...nel.org>,
	Serge Hallyn <serge.hallyn@...ntu.com>,
	Andrew Morton <akpm@...uxfoundation.org>,
	Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
	"Ted Ts'o" <tytso@....edu>, "Andrew G. Morgan" <morgan@...nel.org>,
	Linux API <linux-api@...r.kernel.org>,
	Mimi Zohar <zohar@...ux.vnet.ibm.com>,
	Michael Kerrisk <mtk.manpages@...il.com>,
	Austin S Hemmelgarn <ahferroin7@...il.com>,
	linux-security-module <linux-security-module@...r.kernel.org>,
	Aaron Jones <aaronmdjones@...il.com>,
	Serge Hallyn <serge.hallyn@...onical.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Markku Savela <msa@...h.iki.fi>,
	Kees Cook <keescook@...omium.org>,
	Jonathan Corbet <corbet@....net>,
	Christoph Lameter <cl@...ux.com>
Subject: Re: [PATCH v2 1/2] capabilities: Ambient capabilities

On Sat, May 23, 2015 at 12:37 PM, Serge Hallyn <serge@...lyn.com> wrote:
> Thanks very much, Andy.  Comments and ack below.
>
> Quoting Andy Lutomirski (luto@...nel.org):
>> ===== The problem =====
>>
>> Capability inheritance is basically useless.
>>
>> If you aren't root and you execute an ordinary binary, fI is zero,
>> so your capabilities have no effect whatsoever on pP'.  This means
>> that you can't usefully execute a helper process or a shell command
>> with elevated capabilities if you aren't root.
>>
>> On current kernels, you can sort of work around this by setting fI
>> to the full set for most or all non-setuid executable files.  This
>> causes pP' = pI for nonroot, and inheritance works.  No one does
>> this because it's a PITA and it isn't even supported on most
>> filesystems.
>>
>> If you try this, you'll discover that every nonroot program ends up
>> with secure exec rules, breaking many things.
>
> PI would have worked great if most programs wanting privilege were
> self-contained and compiled.  Shell scripts and lots of fork+execing
> make pI [much less useful] [completely useless].  See also golang's
> predisposition to fork+exec.

Agreed.

>
>> This is a problem that has bitten many people who have tried to use
>> capabilities for anything useful.
>>
>> ===== The proposed change =====
>>
>> This patch adds a fifth capability mask called the ambient mask
>> (pA).  pA does what pI should have done.
>
> Or at least what most people want it to do.
>
>> pA obeys the invariant that no bit can ever be set in pA if it is
>> not set in both pP and pI.  Dropping a bit from pP or pI drops that
>> bit from pA.  This ensures that existing programs that try to drop
>> capabilities still do so, with a complication.  Because capability
>> inheritance is so broken, setting KEEPCAPS, using setresuid to
>
> Sorry, did you mean "... setting KEEPCAPS and then either using
> setresuid to a nonroot uid or calling execve ..." ?

So, I actually meant:

Because capability inheritance is so broken, setting KEEPCAPS, using
setresuid to switch to nonroot uids, and then calling execve
effectively drops capabilities.

Fixed.

>
>> switch to nonroot uids, or calling execve effectively drops
>> capabilities.  Therefore, setresuid from root to nonroot
>> conditionally clears pA unless SECBIT_NO_SETUID_FIXUP is set.
>> Processes that don't like this can re-add bits to pA afterwards.
>>
>> The capability evolution rules are changed:
>>
>>   pA' = (file caps or setuid or setgid ? 0 : pA)
>>   pP' = (X & fP) | (pI & fI) | pA'
>>   pI' = pI
>>   pE' = (fE ? pP' : pA')
>>   X is unchanged
>>
>> If you are nonroot but you have a capability, you can add it to pA.
>> If you do so, your children get that capability in pA, pP, and pE.
>> For example, you can set pA = CAP_NET_BIND_SERVICE, and your
>> children can automatically bind low-numbered ports.  Hallelujah!
>>
>> Unprivileged users can create user namespaces, map themselves to a
>> nonzero uid, and create both privileged (relative to their
>> namespace) and unprivileged process trees.  This is currently more
>> or less impossible.  Hallelujah!
>>
>> You cannot use pA to try to subvert a setuid, setgid, or file-capped
>> program: if you execute any such program, pA gets cleared and the
>> resulting evolution rules are unchanged by this patch.
>
> Christoph, just to be sure, is this ^ going to suffice for you?
>
> Seems like it should since any program which is setuid-root, i.e.
> passwd, isn't likely to be designed to exec other programs.
>
>> 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.
>>
>> It's worth noting that the degree of paranoia in this patch could
>> possibly be reduced without causing serious problems.  Specifically,
>> if we allowed pA to persist across executing non-pA-aware setuid
>> binaries and across setresuid, then, naively, the only capabilities
>> that could leak as a result would be the capabilities in pA, and any
>> attacker *already* has those capabilities.  This would make me
>> nervous, though -- setuid binaries that tried to privilege-separate
>> might fail to do so, and putting CAP_DAC_READ_SEARCH or
>> CAP_DAC_OVERRIDE into pA could have unexpected side effects.
>> (Whether these unexpected side effects would be exploitable is an
>> open question.)  I've therefore taken the more paranoid route.
>
> I'm definitely open to revisiting this question after the current
> patch has been in use for awhile.

Noted in the changelog for v3.

>
>> An alternative would be to require PR_SET_NO_NEW_PRIVS before
>> setting ambient capabilities.  I think that this would be annoying
>> and would make granting otherwise unprivileged users minor ambient
>> capabilities (CAP_NET_BIND_SERVICE or CAP_NET_RAW for example) much
>> less useful than it is with this patch.
>>
>> ===== Footnotes =====
>>
>> [1] Files that are missing the "security.capability" xattr or that
>> have unrecognized values for that xattr end up with has_cap set to
>> false.  The code that does that appears to be complicated for no
>> good reason.
>>
>> [2] The libcap capability mask parsers and formatters are
>> dangerously misleading and the documentation is flat-out wrong.  fE
>> is *not* a mask; it's a single bit.  This has probably confused
>> every single person who has tried to use file capabilities.
>>
>> [3] Linux very confusingly processes both the script and the
>> interpreter if applicable, for reasons that elude me.  The results
>> from thinking about a script's file capabilities and/or setuid bits
>> are mostly discarded.
>
> Not quite sure what you mean here - setuid and fX are ignored on
> shell scripts, but I think you're saying something less simple is
> going on?

I think that's the effect.  What I mean is that, when running a
script, prepare_binprm and hence cap_bprm_set_creds is called twice:
once from do_execveat_common and once from load_script.

Thanks,
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