Re: [PATCH v3] capabilities.7, prctl.2: Document ambient capabilities

["Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>]

On 12/04/2015 05:12 PM, Andy Lutomirski wrote:
> On Fri, Dec 4, 2015 at 7:08 AM, Michael Kerrisk (man-pages)
> <mtk.manpages@gmail.com> wrote:
>> Hi Andy,
>>
>> I have applied your patch (below). Thanks for writing it.
>> But I have a question or two and a request.
>>
>> ===
>>
>> In the capabilities(7) page tehre is the longstanding text:
>>
>>        An  application  can use the following call to lock itself, and
>>        all of its descendants, into an environment where the only  way
>>        of  gaining capabilities is by executing a program with associ‐
>>        ated file capabilities:
>>
>>            prctl(PR_SET_SECUREBITS,
>>                    SECBIT_KEEP_CAPS_LOCKED |
>>                    SECBIT_NO_SETUID_FIXUP |
>>                    SECBIT_NO_SETUID_FIXUP_LOCKED |
>>                    SECBIT_NOROOT |
>>                    SECBIT_NOROOT_LOCKED);
>>
>> As far as I can estimate, no changes are needed here to include
>> SECBIT_NO_CAP_AMBIENT_RAISE and SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED
>> in the above prctl() call, but could you confirm please?
> 
> Correct.  I'll probably write up a patch to suggest that doing this is
> a poor idea on a conventional distro, though, and I'll explain why.  I
> suppose than deleting this would be an option, too.

Ping! :-)


>> ===
>>
>> In the message for kernel commit
>> 58319057b7847667f0c9585b9de0e8932b0fdb08
>> you included this text:
>>
>> [[
>> Because capability inheritance is so
>> broken, setting KEEPCAPS, using setresuid to switch to nonroot uids, and
>> then 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.
>> ]]
>>
>> I'm struggling to understand the significance of this text,
>> especially as your man-pages patch makes no mention of this point.
>>
>> The thing is, that text ("Therefore...") implies that there's
>> something special going on beyond the rules already documented
>> elsewhere. I mean, according to the rules aly documented elsewhere
>> in the page:
> 
> Whoops, I forgot to add that to the manpage.
> 
>>
>> (1) If a process with UIDs of 0 sets all its UIDs
>> nonzero, then, the permitted and effective sets are cleared
>> (that's the classical behavior), and because the permitted
>> set is cleared, then so is the ambient set.
>>
>> (2) And if we set SECBIT_NO_SETUID_FIXUP then
>> a UID 0 ==> nonzero transition doesn't clear permitted and
>> effective sets, and then of course the ambient set is not
>> cleared.
>>
>> So, what additional point were you meaning to convey in
>> the commit message? (Maybe it was just cruft in the commit
>> message, but if not, can you explain precisely the arguments
>> for setresuid() that are supposed to generate the special
>> behavior described by the above text.)
> 
> It's case (1b), which is like (1) but with KEEPCAPS set.  The
> permitted set doesn't get cleared, but the ambient set is still
> cleared.
> 
> I'll write a manpage patch.

Ping :-)

(Make these separate patches please.)

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/