Re: [PATCH man-pages v2] capabilities.7, prctl.2: Document ambient capabilities

Andy Lutomirski
2015-05-19T17:22:12Z

On Tue, May 19, 2015 at 12:56 AM, Michael Kerrisk (man-pages)
<mtk.manpages@gmail.com> wrote:
> Hi Andy,
>
> Thanks for this patch. There are some broken pieces though. Also,
> I have some minor questions about the API design. See below.
>
> On 05/15/2015 08:43 AM, Andy Lutomirski wrote:
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>
>> There was no v1. I'm calling this v2 to keep it in sync with the kernel
>> patch versioning.
>>
>> man2/prctl.2 | 10 ++++++++++
>> man7/capabilities.7 | 32 ++++++++++++++++++++++++++------
>> 2 files changed, 36 insertions(+), 6 deletions(-)
>>
>> diff --git a/man2/prctl.2 b/man2/prctl.2
>> index b352f6283624..5861e3aefe9a 100644
>> --- a/man2/prctl.2
>> +++ b/man2/prctl.2
>> @@ -949,6 +949,16 @@ had been called.
>> For further information on Intel MPX, see the kernel source file
>> .IR Documentation/x86/intel_mpx.txt .
>> .\"
>> +.TP
>> +.BR PR_CAP_AMBIENT " (since Linux 4.2)"
>> +Reads or changes the ambient capability set. If arg2 is PR_CAP_AMBIENT_RAISE,
>> +then the capability specified in arg3 is added to the ambient set. This will
>> +fail, returning EPERM, if the capability is not already both permitted and
>> +inheritable or if the SECBIT_NO_CAP_AMBIENT_RAISE securebit is set. If arg2
>> +is PR_CAP_AMBIENT_LOWER, then the capability specified in arg3 is removed
>> +from the ambient set. If arg2 is PR_CAP_AMBIENT_GET, then
>> +.BR prctl (2)
>> +will return 1 if the capability in arg3 is in the ambient set and 0 if not.
>
> Some API design questions:
>
> 1. We already have prctl() operations that work on some capability sets:
> PR_CAPBSET_READ and PR_CAPBSET_DROP. These don't use arg3; the operation
> is directly encoded in the first argument of prctl(). Just to keep some
> consistency, why not do things the same way for these new operations?

I'm torn. On the one hand, consistency is nice. On the other hand,
prctl is a mess and trying to organize new additions seems like a good
idea.

>
> Also, you could opt for some consistency in the naming, so using "READ"
> rather than "GET", for example. On the other hand, both "READ" and "GET"
> are suboptimal names: this is really a test operation. So, maybe a
> clean break to a good name, PR_CAP_AMBIENT_IS_SET, is best?

I like IS_SET.

>
> Thus:
>
> prctl(PR_CAP_AMBIENT_READ, cap, 0, 0, 0); // or PR_CAP_AMBIENT_IS_SET?
> prctl(PR_CAP_AMBIENT_RAISE, cap, 0, 0, 0);
> prctl(PR_CAP_AMBIENT_LOWER, cap, 0, 0, 0);
>
> 2. In terms of the API design, would it be useful to have a prctl() operation
> that clears the entire ambient set?
>
> prctl(PR_CAP_AMBIENT_ZERO, 0, 0, 0, 0); // or PR_CAP_AMBIENT_EMPTY?

Seems like a good idea. How about _CLEAR?

>
>> .SH RETURN VALUE
>> On success,
>> .BR PR_GET_DUMPABLE ,
>> diff --git a/man7/capabilities.7 b/man7/capabilities.7
>> index d75ec65de05b..dae62f0be3b7 100644
>> --- a/man7/capabilities.7
>> +++ b/man7/capabilities.7
>> @@ -697,13 +697,26 @@ a program whose associated file capabilities grant that capability).
>> .IR Inheritable :
>> This is a set of capabilities preserved across an
>> .BR execve (2).
>> -It provides a mechanism for a process to assign capabilities
>> -to the permitted set of the new program during an
>> -.BR execve (2).
>> +Inheritable capabilities remain inheritable when executing any program,
>> +and inheritable capabilities are added to the permitted set when executing
>> +a program that has the corresponding bits set in the file inheritable set.
>> +When executing programs without file capabilities, ambient capabilities
>
> That last line is incomplete. Something needs adding/removing.

Whoops, will fix.

>
>> .TP
>> .IR Effective :
>> This is the set of capabilities used by the kernel to
>> perform permission checks for the thread.
>> +.TP
>> +.IR Ambient " (since Linux 4.2) :"
>
> Minor knit: s/ :/:/ for next version.

Will fix.

>
>> +This is a set of capabilities that are preserved across an
>> +.BR execve (2)
>> +of a program that does not have file capabilities. The ambient capability
>> +set obeys the invariant that no capability can ever be ambient if it is
>> +not both permitted and inheritable. Ambient capabilities are, with some
>> +exceptions, preserved in the permitted set and added to the effective
>> +set when
>> +.BR execve (2)
>> +is called. The ambient capability set is modified using
>> +.BR prctl (2).
>
> I think it would be helpful to add a couple of sentences here on why the
> ambient set is useful (i.e., explain what deficiencies in the pre-existing
> API are addressed by the addition of this set--a brief piece from your
> 1/2 patch, for example).

Will do.

--Andy