lkml.org 
[lkml]   [2015]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 1/2] capabilities: Ambient capabilities
On Sat, May 23, 2015 at 12:37 PM, Serge Hallyn <serge@hallyn.com> wrote:
> Thanks very much, Andy. Comments and ack below.
>
> Quoting Andy Lutomirski (luto@kernel.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


\
 
 \ /
  Last update: 2015-05-28 01:21    [W:0.490 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site