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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <555B8B6D.4010608@gmail.com>
Date:	Tue, 19 May 2015 21:13:49 +0200
From:	"Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>
To:	Andy Lutomirski <luto@...capital.net>
CC:	mtk.manpages@...il.com, 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>,
	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>
Subject: Re: [PATCH man-pages v2] capabilities.7, prctl.2: Document ambient
 capabilities

Hi Andy,

On 05/19/2015 07:21 PM, Andy Lutomirski wrote:
> On Tue, May 19, 2015 at 12:56 AM, Michael Kerrisk (man-pages)
> <mtk.manpages@...il.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@...nel.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 

Agreed.

> and trying to organize new additions seems like a good
> idea.

Sure, but what is your organizing principle here? (I don't feel strongly
about it, but it's not clear to me what trumps the (mild) degree of 
consistency that I suggest.)


>>    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.

Okay.

>>    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?

Also good.

[...]

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
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/