Re: [PATCH 1/1] simplified security.nscapability xattr

[Michael Kerrisk (man-pages)]

On 05/02/2016 05:54 AM, Serge E. Hallyn wrote:
> On Tue, Apr 26, 2016 at 03:39:54PM -0700, Kees Cook wrote:
>> On Tue, Apr 26, 2016 at 3:26 PM, Serge E. Hallyn <serge@xxxxxxxxxx> wrote:
>>> Quoting Kees Cook (keescook@xxxxxxxxxxxx):
>>>> On Fri, Apr 22, 2016 at 10:26 AM,  <serge.hallyn@xxxxxxxxxx> wrote:
>>>>> From: Serge Hallyn <serge.hallyn@xxxxxxxxxx>
> ...
>>>> This looks like userspace must knowingly be aware that it is in a
>>>> namespace and to DTRT instead of it being translated by the kernel
>>>> when setxattr is called under !init_user_ns?
>>>
>>> Yes - my libcap2 patch checks /proc/self/uid_map to decide that.  If that
>>> shows you are in init_user_ns then it uses security.capability, otherwise
>>> it uses security.nscapability.
>>>
>>> I've occasionally considered having the xattr code do the quiet
>>> substitution if need be.
>>>
>>> In fact, much of this structure comes from when I was still trying to
>>> do multiple values per xattr.  Given what we're doing here, we could
>>> keep the xattr contents exactly the same, just changing the name.
>>> So userspace could just get and set security.capability;  if you are
>>> in a non-init user_ns, if security.capability is set then you cannot
>>> set it;  if security.capability is not set, then the kernel writes
>>> security.nscapability instead and returns success.
>>>
>>> I don't like magic, but this might be just straightforward enough
>>> to not be offensive.  Thoughts?
>>
>> Yeah, I think it might be better to have the magic in this case, since
>> it seems weird to just reject setxattr if a tool didn't realize it was
>> in a namespace. I'm not sure -- it is also nice to have an explicit
>> API here.
>>
>> I would defer to Eric or Michael on that. I keep going back and forth,
>> though I suspect it's probably best to do what you already have
>> (explicit API).
> 
> Michael, Eric, what do you think?  The choice we're making here is
> whether we should
> 
> 1. Keep a nice simple separate pair of xattrs, the pre-existing
> security.capability which can only be written from init_user_ns,
> and the new (in this patch) security.nscapability which you can
> write to any file where you are privileged wrt the file.
> 
> 2. Make security.capability somewhat 'magic' - if someone in a
> non-initial user ns tries to write it and has privilege wrt the
> file, then the kernel silently writes security.nscapability instead.
> 
> The biggest drawback of (1) would be any tar-like program trying
> to restore a file which had security.capability, needing to know
> to detect its userns and write the security.nscapability instead.
> The drawback of (2) is ~\o/~ magic.

I have only (minor) thoughts from the interface perspective.
(1) Sounds the source of possibly unpleasant surprises.
(2) Is a little surprising, but less so if it's well documented,
and it saves us the surprises of (1). So, (2) sounds better.

Cheers,

Michael


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