Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1391953 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/1] simplified security.nscapability xattr "Serge E. Hallyn" <serge@hallyn.com> - 2016-05-02 06:00 +0200
    Re: [PATCH 1/1] simplified security.nscapability xattr "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> - 2016-05-02 20:40 +0200
    Re: [PATCH 1/1] simplified security.nscapability xattr ebiederm@xmission.com (Eric W. Biederman) - 2016-05-02 23:50 +0200
      Re: [PATCH 1/1] simplified security.nscapability xattr ebiederm@xmission.com (Eric W. Biederman) - 2016-05-03 07:10 +0200
        Re: [PATCH 1/1] simplified security.nscapability xattr "Serge E. Hallyn" <serge@hallyn.com> - 2016-05-10 21:10 +0200
      Re: [PATCH 1/1] simplified security.nscapability xattr "Serge E. Hallyn" <serge@hallyn.com> - 2016-05-03 07:20 +0200
        Re: [PATCH 1/1] simplified security.nscapability xattr ebiederm@xmission.com (Eric W. Biederman) - 2016-05-03 08:10 +0200
          Re: [PATCH 1/1] simplified security.nscapability xattr "Serge E. Hallyn" <serge@hallyn.com> - 2016-05-03 16:30 +0200
            Re: [PATCH 1/1] simplified security.nscapability xattr "Serge E. Hallyn" <serge@hallyn.com> - 2016-05-10 21:10 +0200
          Re: [PATCH 1/1] simplified security.nscapability xattr Jann Horn <jann@thejh.net> - 2016-05-08 01:10 +0200
            Re: [PATCH 1/1] simplified security.nscapability xattr "Serge E. Hallyn" <serge@hallyn.com> - 2016-05-11 23:10 +0200

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

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@hallyn.com> wrote:
> > Quoting Kees Cook (keescook@chromium.org):
> >> On Fri, Apr 22, 2016 at 10:26 AM,  <serge.hallyn@ubuntu.com> wrote:
> >> > From: Serge Hallyn <serge.hallyn@ubuntu.com>
...
> >> 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.

-serge

[toc] | [next] | [standalone]

#1392444

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@hallyn.com> wrote:
>>> Quoting Kees Cook (keescook@chromium.org):
>>>> On Fri, Apr 22, 2016 at 10:26 AM,  <serge.hallyn@ubuntu.com> wrote:
>>>>> From: Serge Hallyn <serge.hallyn@ubuntu.com>
> ...
>>>> 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/

[toc] | [prev] | [next] | [standalone]

#1392528

"Serge E. Hallyn" <serge@hallyn.com> writes:

> 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@hallyn.com> wrote:
>> > Quoting Kees Cook (keescook@chromium.org):
>> >> On Fri, Apr 22, 2016 at 10:26 AM,  <serge.hallyn@ubuntu.com> wrote:
>> >> > From: Serge Hallyn <serge.hallyn@ubuntu.com>
> ...
>> >> 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.

Apologies for not having followed this more closely before.

I don't like either option.  I think we will be in much better shape if
we upgrade the capability xattr.  It seems totally wrong or at least
confusing for a file to have both capability xattrs.

Just using security.capability allows us to confront any weird issues
with mixing both the old semantics and the new semantics.

We had previously discussioned extending the capbility a little and
adding a uid who needed to be the root uid in a user namespace, to be
valid.  Using the owner of the file seems simpler, and even a little
more transparent as this makes the security.capability xattr a limited
form of setuid (which it semantically is).

So I believe the new semantics in general are an improvement.


Given the expected use case let me ask as simple question: Are there any
known cases where the owner of a setcap exectuable is not root?

I expect the pile of setcap exectuables is small enough we can go
through the top distros and look at all of the setcap executlables.


If there is not a need to support setcap executables owned by non-root,
I suspect the right play is to just change the semantics to always treat
the security.capability attribute this way.

If there is a need to support setcap exectualbes owned by non-root,
then the current implementation is most likely unacceptable.  As that
problem case can not work in a container.



My guess is that we can just reinterpret the current security.capable to
only be valid when root owns the file in the initial user namespace.  At
which point backwards compatibility becomes trivial as the
security.capable does not change, just the rules for setting it, and
interpreting it.


We should also ensure that the gid of the file is mapped into the user
namespace where the uid is the root of the user namespace.  So that we
are effectively testing capable_wrtuid_and_gid on execute as well a
read/write of the the xattr.

Eric

[toc] | [prev] | [next] | [standalone]

#1393035

"Andrew G. Morgan" <morgan@kernel.org> writes:

> On 2 May 2016 6:04 p.m., "Eric W. Biederman" <ebiederm@xmission.com>
> wrote:
>>
>> "Serge E. Hallyn" <serge@hallyn.com> writes:
>>
>> > 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@hallyn.com> wrote:
>> >> > Quoting Kees Cook (keescook@chromium.org):
>> >> >> On Fri, Apr 22, 2016 at 10:26 AM, <serge.hallyn@ubuntu.com>
> wrote:
>> >> >> > From: Serge Hallyn <serge.hallyn@ubuntu.com>
>> > ...
>> >> >> 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.
>>
>> Apologies for not having followed this more closely before.
>>
>> I don't like either option. I think we will be in much better shape
> if
>> we upgrade the capability xattr. It seems totally wrong or at least
>> confusing for a file to have both capability xattrs.
>>
>> Just using security.capability allows us to confront any weird
> issues
>> with mixing both the old semantics and the new semantics.
>>
>> We had previously discussioned extending the capbility a little and
>> adding a uid who needed to be the root uid in a user namespace, to
> be
>> valid. Using the owner of the file seems simpler, and even a little
>> more transparent as this makes the security.capability xattr a
> limited
>> form of setuid (which it semantically is).
>>
>> So I believe the new semantics in general are an improvement.
>>
>>
>> Given the expected use case let me ask as simple question: Are there
> any
>> known cases where the owner of a setcap exectuable is not root?
>>
>> I expect the pile of setcap exectuables is small enough we can go
>> through the top distros and look at all of the setcap executlables.
>>
>>
>> If there is not a need to support setcap executables owned by
> non-root,
>> I suspect the right play is to just change the semantics to always
> treat
>> the security.capability attribute this way.
>>
>
> I guess I'm confused how we have strayed so far that this isn't an
> obvious requirement. Uid=0 as being the root of privilege was the
> basic problem that capabilities were designed to change.

uid==0 as the owner of a file is slightly different from uid==0 of a
running process.  Last I checked if it is installed as part of a
distribution the programs are owned by root by default.

> Uid is an acl concept. Capabilities are supposed to be independent of
> that.

I don't have a clue what you mean.  Posix capabilities on executables
are part of discretionary access control.  Whatever their rules posix
capabilities are just watered down versions of the permissions of
a setuid root exectuable.  I don't think anyone has ever actually run a
system with setuid root exectuables not being special.  If you are
thinking of what any other system call capabilities unix calls those are
file descriptors.

I don't think it is necessarily wrong that files that hold exectuable
programs need to be owned by a user that is trusted to install files
system wide.  So far that user in my limited sampling that user is
always root.  Given that installing a program like that is fundamentally
a very privileged role we may not be able to break it up successfully,
so root as the owner of program files seems to make a lot of sense.

How would you design a system wide program installer in a root less
system?

Does anyone know any linux based system that uses file capabilities on
executables without setting the executable to be owned by root?  One
example would be all that it takes to shut down this marvelous
simplification that I see.


I strongly suspect the reality is that all that exists are watered down
setuid root exectuables.  If that is indeed the case we can safely let
file caps only be valid if root owns the file.  That would be convinient
at it is much simpler to understand and implement and audit.


I really don't care either way except that I like simpler code, and I
like not breaking userspace.

Eric

[toc] | [prev] | [next] | [standalone]

#1398425

Quoting Eric W. Biederman (ebiederm@xmission.com):
> "Andrew G. Morgan" <morgan@kernel.org> writes:
> 
> > On 2 May 2016 6:04 p.m., "Eric W. Biederman" <ebiederm@xmission.com>
> > wrote:
> >>
> >> "Serge E. Hallyn" <serge@hallyn.com> writes:
> >>
> >> > 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@hallyn.com> wrote:
> >> >> > Quoting Kees Cook (keescook@chromium.org):
> >> >> >> On Fri, Apr 22, 2016 at 10:26 AM, <serge.hallyn@ubuntu.com>
> > wrote:
> >> >> >> > From: Serge Hallyn <serge.hallyn@ubuntu.com>
> >> > ...
> >> >> >> 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.
> >>
> >> Apologies for not having followed this more closely before.
> >>
> >> I don't like either option. I think we will be in much better shape
> > if
> >> we upgrade the capability xattr. It seems totally wrong or at least
> >> confusing for a file to have both capability xattrs.
> >>
> >> Just using security.capability allows us to confront any weird
> > issues
> >> with mixing both the old semantics and the new semantics.
> >>
> >> We had previously discussioned extending the capbility a little and
> >> adding a uid who needed to be the root uid in a user namespace, to
> > be
> >> valid. Using the owner of the file seems simpler, and even a little
> >> more transparent as this makes the security.capability xattr a
> > limited
> >> form of setuid (which it semantically is).
> >>
> >> So I believe the new semantics in general are an improvement.
> >>
> >>
> >> Given the expected use case let me ask as simple question: Are there
> > any
> >> known cases where the owner of a setcap exectuable is not root?
> >>
> >> I expect the pile of setcap exectuables is small enough we can go
> >> through the top distros and look at all of the setcap executlables.
> >>
> >>
> >> If there is not a need to support setcap executables owned by
> > non-root,
> >> I suspect the right play is to just change the semantics to always
> > treat
> >> the security.capability attribute this way.
> >>
> >
> > I guess I'm confused how we have strayed so far that this isn't an
> > obvious requirement. Uid=0 as being the root of privilege was the
> > basic problem that capabilities were designed to change.
> 
> uid==0 as the owner of a file is slightly different from uid==0 of a
> running process.  Last I checked if it is installed as part of a
> distribution the programs are owned by root by default.

Note that this does mean that a user namespace without a mapping for a
uid 0 cannot use file capabilities.

But I'm not sure there is a way around that.  Even if we store the
userns identifier in an xattr instead of using the file owner, we still
need to uniquely identify the namespace somehow, and as Jann pointed
out using the namespace creator uid is non-ideal as it means all namespaces
even with disjoint uid mappings can mess with each other.

> > Uid is an acl concept. Capabilities are supposed to be independent of
> > that.
> 
> I don't have a clue what you mean.  Posix capabilities on executables
> are part of discretionary access control.  Whatever their rules posix
> capabilities are just watered down versions of the permissions of
> a setuid root exectuable.  I don't think anyone has ever actually run a
> system with setuid root exectuables not being special.  If you are

I actually suspect Andrew does, and I've done it though only as an
experiment.  (Oh - but I may mean something different than what you
mean, see below)

> thinking of what any other system call capabilities unix calls those are
> file descriptors.
> 
> I don't think it is necessarily wrong that files that hold exectuable
> programs need to be owned by a user that is trusted to install files
> system wide.  So far that user in my limited sampling that user is
> always root.  Given that installing a program like that is fundamentally
> a very privileged role we may not be able to break it up successfully,
> so root as the owner of program files seems to make a lot of sense.
> 
> How would you design a system wide program installer in a root less
> system?

The root id is still special, in the same sense as it is in plan 9 - it
is the uid which "owns" the hardware.  In linux setuid always still
works - it just can be configured to not raise/drop privileges on
setuid.  (You know all this, but someone reading along may have
forgotten)

The question then is what it should take to set and to use a file
capability.

So what we have right now is afaics a technical roadblock - in that I
really can't see a way to safely allow filecaps in a userns without a
uid 0 mapping.  It's unfortunate, and it's not a goal, it's an implementation
detail.  I personally think it is a huge improvement over what we have.
The question is does doing it this way now prevent us from doing it
the right way later if we can find a right way?

> Does anyone know any linux based system that uses file capabilities on
> executables without setting the executable to be owned by root?  One
> example would be all that it takes to shut down this marvelous
> simplification that I see.
> 
> 
> I strongly suspect the reality is that all that exists are watered down
> setuid root exectuables.  If that is indeed the case we can safely let
> file caps only be valid if root owns the file.  That would be convinient
> at it is much simpler to understand and implement and audit.
> 
> 
> I really don't care either way except that I like simpler code, and I
> like not breaking userspace.
> 
> Eric

[toc] | [prev] | [next] | [standalone]

#1393039

Quoting Andrew G. Morgan (morgan@kernel.org):
> On 2 May 2016 6:04 p.m., "Eric W. Biederman" <ebiederm@xmission.com> wrote:
> >
> > "Serge E. Hallyn" <serge@hallyn.com> writes:
> >
> > > 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@hallyn.com>
> wrote:
> > >> > Quoting Kees Cook (keescook@chromium.org):
> > >> >> On Fri, Apr 22, 2016 at 10:26 AM,  <serge.hallyn@ubuntu.com> wrote:
> > >> >> > From: Serge Hallyn <serge.hallyn@ubuntu.com>
> > > ...
> > >> >> 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.
> >
> > Apologies for not having followed this more closely before.
> >
> > I don't like either option.  I think we will be in much better shape if
> > we upgrade the capability xattr.  It seems totally wrong or at least
> > confusing for a file to have both capability xattrs.
> >
> > Just using security.capability allows us to confront any weird issues
> > with mixing both the old semantics and the new semantics.
> >
> > We had previously discussioned extending the capbility a little and
> > adding a uid who needed to be the root uid in a user namespace, to be
> > valid.  Using the owner of the file seems simpler, and even a little
> > more transparent as this makes the security.capability xattr a limited
> > form of setuid (which it semantically is).
> >
> > So I believe the new semantics in general are an improvement.
> >
> >
> > Given the expected use case let me ask as simple question: Are there any
> > known cases where the owner of a setcap exectuable is not root?
> >
> > I expect the pile of setcap exectuables is small enough we can go
> > through the top distros and look at all of the setcap executlables.
> >
> >
> > If there is not a need to support setcap executables owned by non-root,
> > I suspect the right play is to just change the semantics to always treat
> > the security.capability attribute this way.
> >
> 
> I guess I'm confused how we have strayed so far that this isn't an obvious
> requirement. Uid=0 as being the root of privilege was the basic problem
> that capabilities were designed to change.

The task executing the file can be any uid mapped into the namespace.  The
file only has to be owned by the root of the user_ns.  Which I agree is
unfortunate.  We can work around it by putting the root uid into the xattr
itself (which still isn't orthogonal but allows the file to at least by
owned by non-root), but the problem then is that a task needs to know its
global root k_uid just to write the xattr.

> Uid is an acl concept. Capabilities are supposed to be independent of that.
> 
> If we want to support NS file capabilities I would look at replacing the
> xattr syscall with a dedicated file capabilities modification syscall. Then

That was one ofthe possibilities I'd mentioned in my earlier proposal,
fwiw.  The problem is if we want tar to still work unmodified then
simple xattr operations still have to work.

Maybe there's workable semantics there though.  Worth thinking about.

> namespace partitioning and file capabilities can be managed in the kernel
> with respect to the prevailing namespace, and not by some hybrid userspace
> kernel convention.
> 
> Cheers
> 
> Andrew
> 
> > If there is a need to support setcap exectualbes owned by non-root,
> > then the current implementation is most likely unacceptable.  As that
> > problem case can not work in a container.
> >
> >
> >
> > My guess is that we can just reinterpret the current security.capable to
> > only be valid when root owns the file in the initial user namespace.  At
> > which point backwards compatibility becomes trivial as the
> > security.capable does not change, just the rules for setting it, and
> > interpreting it.
> >
> >
> > We should also ensure that the gid of the file is mapped into the user
> > namespace where the uid is the root of the user namespace.  So that we
> > are effectively testing capable_wrtuid_and_gid on execute as well a
> > read/write of the the xattr.
> >
> > Eric

[toc] | [prev] | [next] | [standalone]

#1393075

"Serge E. Hallyn" <serge@hallyn.com> writes:

> Quoting Andrew G. Morgan (morgan@kernel.org):
>> 
>> I guess I'm confused how we have strayed so far that this isn't an obvious
>> requirement. Uid=0 as being the root of privilege was the basic problem
>> that capabilities were designed to change.
>
> The task executing the file can be any uid mapped into the namespace.  The
> file only has to be owned by the root of the user_ns.  Which I agree is
> unfortunate.  We can work around it by putting the root uid into the xattr
> itself (which still isn't orthogonal but allows the file to at least by
> owned by non-root), but the problem then is that a task needs to know its
> global root k_uid just to write the xattr.

The root kuid is just make_kuids(user_ns, 0) so it is easy to find.

It might be a hair better to use the userns->owner instead of the root
uid.  That would allow user namespaces without a mapped root to still
use file capabilities.

>> Uid is an acl concept. Capabilities are supposed to be independent of that.
>> 
>> If we want to support NS file capabilities I would look at replacing the
>> xattr syscall with a dedicated file capabilities modification syscall. Then
>
> That was one ofthe possibilities I'd mentioned in my earlier proposal,
> fwiw.  The problem is if we want tar to still work unmodified then
> simple xattr operations still have to work.
>
> Maybe there's workable semantics there though.  Worth thinking about.

If the problem is compatibilty please look at
posix_acl_fix_xattr_from_user.  With something similar for the
security.capability attribute we can perform whatever transformation
makes sense.  I admit adding 4 bytes is a bit of a pain in that context
but not a big one.

Eric

[toc] | [prev] | [next] | [standalone]

#1393457

Quoting Eric W. Biederman (ebiederm@xmission.com):
> "Serge E. Hallyn" <serge@hallyn.com> writes:
> 
> > Quoting Andrew G. Morgan (morgan@kernel.org):
> >> 
> >> I guess I'm confused how we have strayed so far that this isn't an obvious
> >> requirement. Uid=0 as being the root of privilege was the basic problem
> >> that capabilities were designed to change.
> >
> > The task executing the file can be any uid mapped into the namespace.  The
> > file only has to be owned by the root of the user_ns.  Which I agree is
> > unfortunate.  We can work around it by putting the root uid into the xattr
> > itself (which still isn't orthogonal but allows the file to at least by
> > owned by non-root), but the problem then is that a task needs to know its
> > global root k_uid just to write the xattr.
> 
> The root kuid is just make_kuids(user_ns, 0) so it is easy to find.
>
> It might be a hair better to use the userns->owner instead of the root
> uid.  That would allow user namespaces without a mapped root to still
> use file capabilities.

That's all fine if the kernel does it for us magically.  Which is what we're
talking about below.  Above I was talking about userspace putting it into
the xattr.

> >> Uid is an acl concept. Capabilities are supposed to be independent of that.
> >> 
> >> If we want to support NS file capabilities I would look at replacing the
> >> xattr syscall with a dedicated file capabilities modification syscall. Then
> >
> > That was one ofthe possibilities I'd mentioned in my earlier proposal,
> > fwiw.  The problem is if we want tar to still work unmodified then
> > simple xattr operations still have to work.
> >
> > Maybe there's workable semantics there though.  Worth thinking about.
> 
> If the problem is compatibilty please look at
> posix_acl_fix_xattr_from_user.  With something similar for the

All right.  Excellent.  I simply didn't think something like that would
be acceptable.  I tend to think of xattrs as just out of band file contents,
but generally under user control.  I guess that's not right.

> security.capability attribute we can perform whatever transformation
> makes sense.  I admit adding 4 bytes is a bit of a pain in that context
> but not a big one.

If we can do all the magic in the kernel behind the scenes, then I
absolutely do not mind adding a new security.capability version with 4
more bytes.  Userspace can just write the old xattr format with the new
version number, kernel fills in the userns owner kuid.  It's what I
originally wanted to do, but didn't think was acceptable.

Sounds great!

-serge

[toc] | [prev] | [next] | [standalone]

#1398431

Quoting Serge E. Hallyn (serge@hallyn.com):
> Quoting Eric W. Biederman (ebiederm@xmission.com):
> > "Serge E. Hallyn" <serge@hallyn.com> writes:
> > 
> > > Quoting Andrew G. Morgan (morgan@kernel.org):
> > >> 
> > >> I guess I'm confused how we have strayed so far that this isn't an obvious
> > >> requirement. Uid=0 as being the root of privilege was the basic problem
> > >> that capabilities were designed to change.
> > >
> > > The task executing the file can be any uid mapped into the namespace.  The
> > > file only has to be owned by the root of the user_ns.  Which I agree is
> > > unfortunate.  We can work around it by putting the root uid into the xattr
> > > itself (which still isn't orthogonal but allows the file to at least by
> > > owned by non-root), but the problem then is that a task needs to know its
> > > global root k_uid just to write the xattr.
> > 
> > The root kuid is just make_kuids(user_ns, 0) so it is easy to find.
> >
> > It might be a hair better to use the userns->owner instead of the root
> > uid.  That would allow user namespaces without a mapped root to still
> > use file capabilities.
> 
> That's all fine if the kernel does it for us magically.  Which is what we're
> talking about below.  Above I was talking about userspace putting it into
> the xattr.
> 
> > >> Uid is an acl concept. Capabilities are supposed to be independent of that.
> > >> 
> > >> If we want to support NS file capabilities I would look at replacing the
> > >> xattr syscall with a dedicated file capabilities modification syscall. Then
> > >
> > > That was one ofthe possibilities I'd mentioned in my earlier proposal,
> > > fwiw.  The problem is if we want tar to still work unmodified then
> > > simple xattr operations still have to work.
> > >
> > > Maybe there's workable semantics there though.  Worth thinking about.
> > 
> > If the problem is compatibilty please look at
> > posix_acl_fix_xattr_from_user.  With something similar for the
> 
> All right.  Excellent.  I simply didn't think something like that would
> be acceptable.  I tend to think of xattrs as just out of band file contents,
> but generally under user control.  I guess that's not right.
> 
> > security.capability attribute we can perform whatever transformation
> > makes sense.  I admit adding 4 bytes is a bit of a pain in that context
> > but not a big one.
> 
> If we can do all the magic in the kernel behind the scenes, then I
> absolutely do not mind adding a new security.capability version with 4
> more bytes.  Userspace can just write the old xattr format with the new
> version number, kernel fills in the userns owner kuid.  It's what I
> originally wanted to do, but didn't think was acceptable.
> 
> Sounds great!

So I'm still mulling this over and still undecided as to whether we want to

1. leave the xattr as is and use a new pair of syscalls for setting/unsetting
filecaps.  This would truly let us hide the implementation detail of the
file having to be owned by root (apart from returning a perhaps-unexpected
EPERM when file isn't owned by uid 0, and documenting that as something that
can be changed later)

2. hide the magic in get/setxattr of security.capability.  And if we do
that, then whether to hide the security.nscapability (or newer-version
security.capbility if that's what we do).  probably not hide it...

-serge

[toc] | [prev] | [next] | [standalone]

#1396368

[Multipart message — attachments visible in raw view] — view raw

On Tue, May 03, 2016 at 12:54:40AM -0500, Eric W. Biederman wrote:
> "Serge E. Hallyn" <serge@hallyn.com> writes:
> 
> > Quoting Andrew G. Morgan (morgan@kernel.org):
> >> 
> >> I guess I'm confused how we have strayed so far that this isn't an obvious
> >> requirement. Uid=0 as being the root of privilege was the basic problem
> >> that capabilities were designed to change.
> >
> > The task executing the file can be any uid mapped into the namespace.  The
> > file only has to be owned by the root of the user_ns.  Which I agree is
> > unfortunate.  We can work around it by putting the root uid into the xattr
> > itself (which still isn't orthogonal but allows the file to at least by
> > owned by non-root), but the problem then is that a task needs to know its
> > global root k_uid just to write the xattr.
> 
> The root kuid is just make_kuids(user_ns, 0) so it is easy to find.
> 
> It might be a hair better to use the userns->owner instead of the root
> uid.  That would allow user namespaces without a mapped root to still
> use file capabilities.

Please don't do that. A user might want to create multiple containers with
isolated security properties, and in that case, it would be bad if binaries
that are capable in one container are also automatically valid in the user's
other containers.
Also, this would mean that in an owner!=root scenario, container root can't
setcap executables and needs to ask the administrator of the surrounding system
to do it.
(Of course, this could be worked around using a dummy userns layer between the
init ns and the container, but I don't like seeing new reasons for such a hack.)

[toc] | [prev] | [next] | [standalone]

#1399514

Quoting Jann Horn (jann@thejh.net):
> On Tue, May 03, 2016 at 12:54:40AM -0500, Eric W. Biederman wrote:
> > "Serge E. Hallyn" <serge@hallyn.com> writes:
> > 
> > > Quoting Andrew G. Morgan (morgan@kernel.org):
> > >> 
> > >> I guess I'm confused how we have strayed so far that this isn't an obvious
> > >> requirement. Uid=0 as being the root of privilege was the basic problem
> > >> that capabilities were designed to change.
> > >
> > > The task executing the file can be any uid mapped into the namespace.  The
> > > file only has to be owned by the root of the user_ns.  Which I agree is
> > > unfortunate.  We can work around it by putting the root uid into the xattr
> > > itself (which still isn't orthogonal but allows the file to at least by
> > > owned by non-root), but the problem then is that a task needs to know its
> > > global root k_uid just to write the xattr.
> > 
> > The root kuid is just make_kuids(user_ns, 0) so it is easy to find.
> > 
> > It might be a hair better to use the userns->owner instead of the root
> > uid.  That would allow user namespaces without a mapped root to still
> > use file capabilities.
> 
> Please don't do that. A user might want to create multiple containers with
> isolated security properties, and in that case, it would be bad if binaries
> that are capable in one container are also automatically valid in the user's
> other containers.

But no, if the namespaces both created by uid 1000 have disjoint uid mappings,
say 100000-165535 and 200000-265536, then a file capability on a file owned
by 200000 would not be active when the exec()ing task has only 100000-165535
mapped.

If the uid mappings are not completely disjoint, then you cannot assume that
the user wanted the mappings to be disjoint.  In particular, while setting up
a rootfs for a container I'll frequently use small ad-hoc namespaces to chown
files.  For instance, my intended final container mapping may be 65536 uids
starting at 100000, but to chown a file to uid 5 in the container I may create
a ns with kuid 1000 as uid 0 and 100005 as uid 1.  Here the root uid doing the
writing is not even mapped into the final container namespace.

So a new approach might be to (a) note the kuid which created the
namespace in the xattr (magically written by the kernel at xattr write
time), then say that for the file capability to take effect, two things
must hold:

1. the kuid noted in the xattr must match the kuid which created the calling
task's user_ns (or any ancestor creator)

2. the file uid must map into the calling task's namespace

To write the filecap,

1. the task must be privileged over the uid which owns the file (in the
sense of capable_wrt_inode_uidgid)

2. the task must be privileged over his own user_ns

As Eric said, this should address Andrew Morgan's concern about requiring that
the file be owned by uid 0 in the namespace.

There's a problem though.  The above suffices to prevent an unprivileged user
in a user_ns from unsharing a user_ns to write a file capability and exploit
that capability in the ns where he is unprivileged.  With one exception, which
is the case where the unprivileged user is mapped to the same kuid which
created the namespace.  So if uid 1000 on the host creates a namespace
where uid 1000 maps to 1000 in the namespace, then 1000 in the namespace
can create a new user_ns, write the xattr, and exploit it from the
parent namespace.  This is not an uncommon case.  I'm not sure what to do about
it.

> Also, this would mean that in an owner!=root scenario, container root can't
> setcap executables and needs to ask the administrator of the surrounding system
> to do it.
> (Of course, this could be worked around using a dummy userns layer between the
> init ns and the container, but I don't like seeing new reasons for such a hack.)

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web