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

Groups > linux.kernel > #1685113 > unrolled thread

[PATCH v2] xattr: Enable security.capability in user namespaces

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

  [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <"Stefan Bergerstefanb"@linux.vnet.ibm.com> - 2017-07-11 17:10 +0200
    Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-11 19:20 +0200
      Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-12 02:20 +0200
        Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-12 02:50 +0200
    Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-12 05:50 +0200
      Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-12 13:40 +0200
        Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-12 19:40 +0200
    Re: [PATCH v2] xattr: Enable security.capability in user  namespaces James Morris <jmorris@namei.org> - 2017-07-12 10:10 +0200
    Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-12 15:40 +0200
      Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-12 19:10 +0200
        Re: [PATCH v2] xattr: Enable security.capability in user  namespaces James Morris <jmorris@namei.org> - 2017-07-13 00:30 +0200
          Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-13 02:50 +0200
            Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-13 03:10 +0200
        Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-13 01:30 +0200
          Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-13 02:50 +0200
            Re: [PATCH v2] xattr: Enable security.capability in user namespaces Theodore Ts'o <tytso@mit.edu> - 2017-07-13 03:20 +0200
              Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-13 04:40 +0200
              Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-13 14:20 +0200
                Re: [PATCH v2] xattr: Enable security.capability in user namespaces Theodore Ts'o <tytso@mit.edu> - 2017-07-13 18:50 +0200
                  Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-13 19:10 +0200
                    Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-13 19:50 +0200
                      Re: [PATCH v2] xattr: Enable security.capability in user namespaces Theodore Ts'o <tytso@mit.edu> - 2017-07-13 21:20 +0200
                        Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-13 21:50 +0200
                      Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-13 23:20 +0200
                    Re: [PATCH v2] xattr: Enable security.capability in user  namespaces James Morris <jmorris@namei.org> - 2017-07-18 09:10 +0200
                  Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-13 19:30 +0200
                    Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-13 19:40 +0200
                      Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-13 20:00 +0200
                        Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-13 21:50 +0200
                          Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-13 23:30 +0200
                        Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-14 02:50 +0200
                          Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-14 13:40 +0200
                            Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-14 14:20 +0200
                              Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-14 14:50 +0200
                            Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-14 15:40 +0200
                              Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-14 17:30 +0200
                                Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-14 19:40 +0200
                                  Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-14 20:30 +0200
                                  Re: [PATCH v2] xattr: Enable security.capability in user namespaces Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-07-14 20:50 +0200
                                    Re: [PATCH v2] xattr: Enable security.capability in user namespaces James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-07-14 21:00 +0200
                                      Re: [PATCH v2] xattr: Enable security.capability in user namespaces Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-07-14 22:10 +0200
                                        Re: [PATCH v2] xattr: Enable security.capability in user namespaces James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-07-14 22:50 +0200
                                          Re: [PATCH v2] xattr: Enable security.capability in user namespaces Theodore Ts'o <tytso@mit.edu> - 2017-07-14 23:40 +0200
                                            Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-15 01:40 +0200
                                          Re: [PATCH v2] xattr: Enable security.capability in user namespaces Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-07-15 01:40 +0200
                                          Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-15 02:10 +0200
                                    Re: [PATCH v2] xattr: Enable security.capability in user namespaces Theodore Ts'o <tytso@mit.edu> - 2017-07-14 21:30 +0200
                                      Re: [PATCH v2] xattr: Enable security.capability in user namespaces Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-07-14 21:50 +0200
                                  Re: [PATCH v2] xattr: Enable security.capability in user namespaces Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-07-14 21:30 +0200
                                    Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-15 02:20 +0200
                                    Re: [PATCH v2] xattr: Enable security.capability in user namespaces Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-07-16 13:30 +0200
                                Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-14 21:00 +0200
                                  Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-14 21:30 +0200
                      Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-13 23:30 +0200
                  Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-13 23:20 +0200
          Re: [PATCH v2] xattr: Enable security.capability in user namespaces "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-13 02:50 +0200
    Re: [PATCH v2] xattr: Enable security.capability in user namespaces Vivek Goyal <vgoyal@redhat.com> - 2017-07-12 20:00 +0200
      Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-12 21:20 +0200
    Re: [PATCH v2] xattr: Enable security.capability in user namespaces ebiederm@xmission.com (Eric W. Biederman) - 2017-07-15 01:50 +0200
      Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-15 23:30 +0200
    Re: [PATCH v2] xattr: Enable security.capability in user namespaces Vivek Goyal <vgoyal@redhat.com> - 2017-07-17 21:00 +0200
      Re: [PATCH v2] xattr: Enable security.capability in user namespaces Stefan Berger <stefanb@linux.vnet.ibm.com> - 2017-07-17 23:00 +0200

Page 3 of 4 — ← Prev page 1 2 [3] 4  Next page →

#1687636

On Fri, 2017-07-14 at 11:52 -0700, James Bottomley wrote:
> On Fri, 2017-07-14 at 14:48 -0400, Mimi Zohar wrote:
> > The concern is with a shared filesystems.  In that case, for IMA it
> > would make sense to support a native and a namespace xattr.  If due
> > to xattr space limitations we have to limit the number of xattrs,
> > then we should limit it to two - a native and a namespace version,
> > with a "uid=" tag - first namespace gets permission to write the
> > namespace xattr.  Again, like in the layered case, if the namespace
> > xattr doesn't exist, fall back to using the native xattr.
> 
> Just on this point: if we're really concerned about the need on shared
> filesystems to have multiple IMA signatures per file, might it not make
> sense simply to support multiple signatures within the security.ima
> xattr? The rules for writing signature updates within user namespaces
> would be somewhat complex (say only able to replace a signature for
> which you demonstrate you possess the key) but it would lead to an
> implementation which would work for traditional shared filesystems
> (like NFS) as well as containerised bind mounts.

Writing security.ima requires being root with CAP_SYS_ADMIN
privileges.  I wouldn't want to give root within the namespace
permission to over write or just extend the native security.ima.

Mimi

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

#1687664

On Fri, 2017-07-14 at 16:03 -0400, Mimi Zohar wrote:
> On Fri, 2017-07-14 at 11:52 -0700, James Bottomley wrote:
> > 
> > On Fri, 2017-07-14 at 14:48 -0400, Mimi Zohar wrote:
> > > 
> > > The concern is with a shared filesystems.  In that case, for IMA
> > > it would make sense to support a native and a namespace xattr.
> > >  If due to xattr space limitations we have to limit the number of
> > > xattrs, then we should limit it to two - a native and a namespace
> > > version, with a "uid=" tag - first namespace gets permission to
> > > write the namespace xattr.  Again, like in the layered case, if
> > > the namespace xattr doesn't exist, fall back to using the native
> > > xattr.
> > 
> > Just on this point: if we're really concerned about the need on
> > shared filesystems to have multiple IMA signatures per file, might
> > it not make sense simply to support multiple signatures within the
> > security.ima xattr? The rules for writing signature updates within
> > user namespaces would be somewhat complex (say only able to replace
> > a signature for which you demonstrate you possess the key) but it
> > would lead to an implementation which would work for traditional
> > shared filesystems (like NFS) as well as containerised bind mounts.
> 
> Writing security.ima requires being root with CAP_SYS_ADMIN
> privileges.  I wouldn't want to give root within the namespace
> permission to over write or just extend the native security.ima.

but why?  That's partly the point of all of this: some security.
attributes can't be written by container root without some supervision
(the capability ones are the hugely problematic ones from this point of
view), but for some there's no reason they shouldn't be.  What would be
the reason that root in a container shouldn't be able to write the ima
xattr the same as host root could on its filesystem?

James

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

#1687676

On Fri, Jul 14, 2017 at 01:39:59PM -0700, James Bottomley wrote:
> but why?  That's partly the point of all of this: some security.
> attributes can't be written by container root without some supervision
> (the capability ones are the hugely problematic ones from this point of
> view), but for some there's no reason they shouldn't be.  What would be
> the reason that root in a container shouldn't be able to write the ima
> xattr the same as host root could on its filesystem?

So I'm happy to say, "Ix-nay on nested containerization; that way lies
insanity".  But my understanding is that there will be people who want
to run containers in containers in containers in containers...  and
this is what scares me.

What if someone in the Nth layer of containerization wants to allow
the container root in the (N+1)th layer to set file capabilities that
will not be honored in the Nth layer of containerization?

Again I think that this is insane, and I'm happy for the answer to be,
"No, that's not supported".  That the "Host container" can have
capabilities that it won't honor, but will be honored by all
subcontainers, but that same thing can't be done between a
subsubsub-container and its child subsubsubsub-container.

Are we OK with that?  Because how we would encode this in the xattr
seems to be to be hopelessly not scalable.

					- Ted

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

#1687722

Theodore Ts'o <tytso@mit.edu> writes:

> On Fri, Jul 14, 2017 at 01:39:59PM -0700, James Bottomley wrote:
>> but why?  That's partly the point of all of this: some security.
>> attributes can't be written by container root without some supervision
>> (the capability ones are the hugely problematic ones from this point of
>> view), but for some there's no reason they shouldn't be.  What would be
>> the reason that root in a container shouldn't be able to write the ima
>> xattr the same as host root could on its filesystem?
>
> So I'm happy to say, "Ix-nay on nested containerization; that way lies
> insanity".  But my understanding is that there will be people who want
> to run containers in containers in containers in containers...  and
> this is what scares me.

I am happy to say we need to bound the space we take in an inode.
So a design that needs more space the more containers you have is
suspicious.

I am not fond of decisions that don't allow nesting of containers.  That
just paints us into a corner.  I am in favor of things that require
little or bounded space.

Generally I will frown at a decision that won't allow nesting, because
nesting of containers happens naturally

> What if someone in the Nth layer of containerization wants to allow
> the container root in the (N+1)th layer to set file capabilities that
> will not be honored in the Nth layer of containerization?

That works perfectly well with the design we have today.  And it only
needs a single security.capability attribute.  The actual design is
associated with the security.capability attribute (either in the
attribute or in the most recent iteration in the attribute name *scowl*)
is to have the uid (from the filesystems point of view) of the root user
of a user namespace.  Running that executable will give you those
capabilities if the uid matches the root user in your user namespace (or
a parent user namespace).

As anyone can who can modify a file can remove a security.capable
attribute just like anyone who can modify a file can remove the setuid
bit this works fine is is sufficient.  Though perhaps a little different.

> Again I think that this is insane, and I'm happy for the answer to be,
> "No, that's not supported".  That the "Host container" can have
> capabilities that it won't honor, but will be honored by all
> subcontainers, but that same thing can't be done between a
> subsubsub-container and its child subsubsubsub-container.
>
> Are we OK with that?  Because how we would encode this in the xattr
> seems to be to be hopelessly not scalable.

That really isn't an issue right now.

The real question right now is what to do with security.ima and
security.evm.  As it was proposed that we share a common code base for
them.  Right now it looks to me like the semantics are sufficiently
different that it doesn't make sense to share code between the two
implementations.  At which point all reason for storing any of this in
the xattr name goes away.  So we just have a single xattr.

Right now I am very much in favor of security xattrs continuing to have
well known names.  That easily limits how much space in the inode you
can have, and it makes thinking about things easier.  It doesn't
preclude having acls in your xattr.  That is exactly how posix
acls are implemented.   But I am not going to build generic support for
them, and I really don't expect they will be needed. 

Eric

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

#1687724

On Fri, 2017-07-14 at 13:39 -0700, James Bottomley wrote:
> On Fri, 2017-07-14 at 16:03 -0400, Mimi Zohar wrote:
> > On Fri, 2017-07-14 at 11:52 -0700, James Bottomley wrote:
> > > 
> > > On Fri, 2017-07-14 at 14:48 -0400, Mimi Zohar wrote:
> > > > 
> > > > The concern is with a shared filesystems.  In that case, for IMA
> > > > it would make sense to support a native and a namespace xattr.
> > > >  If due to xattr space limitations we have to limit the number of
> > > > xattrs, then we should limit it to two - a native and a namespace
> > > > version, with a "uid=" tag - first namespace gets permission to
> > > > write the namespace xattr.  Again, like in the layered case, if
> > > > the namespace xattr doesn't exist, fall back to using the native
> > > > xattr.
> > > 
> > > Just on this point: if we're really concerned about the need on
> > > shared filesystems to have multiple IMA signatures per file, might
> > > it not make sense simply to support multiple signatures within the
> > > security.ima xattr? The rules for writing signature updates within
> > > user namespaces would be somewhat complex (say only able to replace
> > > a signature for which you demonstrate you possess the key) but it
> > > would lead to an implementation which would work for traditional
> > > shared filesystems (like NFS) as well as containerised bind mounts.
> > 
> > Writing security.ima requires being root with CAP_SYS_ADMIN
> > privileges.  I wouldn't want to give root within the namespace
> > permission to over write or just extend the native security.ima.
> 
> but why?  That's partly the point of all of this: some security.
> attributes can't be written by container root without some supervision
> (the capability ones are the hugely problematic ones from this point of
> view), but for some there's no reason they shouldn't be.  What would be
> the reason that root in a container shouldn't be able to write the ima
> xattr the same as host root could on its filesystem?

Let's describe the different scenarios of a shared filesystem (not
layered).
1. The kernel updating the file hash as the file changes, written as
the security.ima xattr.
2. Root vs root in the namespace explicitly writing the security.ima
xattr. 
 
In the first case, neither root nor root in the namespace calculates
and writes the file hash as security.ima.  The kernel itself updates
the file hash.  Since the file hash is the same value, it doesn't need
to be written out as two separate security.ima xattrs.

The second case is where root or root in the namespace explicitly
writes out a file signature as security.ima.  Here different entities
might want to sign the file with different keys.  Up to now, only root
with CAP_SYS_ADMIN can write out security.ima.  That shouldn't change.
Nor should root in the namespace be allowed to change the native's
view, but should be only allowed to change its own view of the xattr.
Allowing root in the namespace to change the native's view isn't safe.

Remember writing security.ima also triggers security.evm to be
updated.  Root in the namespace should never be permitted to cause the
native's security.evm to be updated, only the namespaces's version.

Mimi

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

#1687730

James Bottomley <James.Bottomley@HansenPartnership.com> writes:

> On Fri, 2017-07-14 at 16:03 -0400, Mimi Zohar wrote:
>> On Fri, 2017-07-14 at 11:52 -0700, James Bottomley wrote:
>> > 
>> > On Fri, 2017-07-14 at 14:48 -0400, Mimi Zohar wrote:
>> > > 
>> > > The concern is with a shared filesystems.  In that case, for IMA
>> > > it would make sense to support a native and a namespace xattr.
>> > >  If due to xattr space limitations we have to limit the number of
>> > > xattrs, then we should limit it to two - a native and a namespace
>> > > version, with a "uid=" tag - first namespace gets permission to
>> > > write the namespace xattr.  Again, like in the layered case, if
>> > > the namespace xattr doesn't exist, fall back to using the native
>> > > xattr.
>> > 
>> > Just on this point: if we're really concerned about the need on
>> > shared filesystems to have multiple IMA signatures per file, might
>> > it not make sense simply to support multiple signatures within the
>> > security.ima xattr? The rules for writing signature updates within
>> > user namespaces would be somewhat complex (say only able to replace
>> > a signature for which you demonstrate you possess the key) but it
>> > would lead to an implementation which would work for traditional
>> > shared filesystems (like NFS) as well as containerised bind mounts.
>> 
>> Writing security.ima requires being root with CAP_SYS_ADMIN
>> privileges.  I wouldn't want to give root within the namespace
>> permission to over write or just extend the native security.ima.
>
> but why?  That's partly the point of all of this: some security.
> attributes can't be written by container root without some supervision
> (the capability ones are the hugely problematic ones from this point of
> view), but for some there's no reason they shouldn't be.  What would be
> the reason that root in a container shouldn't be able to write the ima
> xattr the same as host root could on its filesystem?

Mimi said she ``native''.  It competely makes sense for the things that
the container doesn't ``own'' to not be allowed to be written/updated by
the container.

James you are making the case here for root in the container to write
to the ima and evm attributes that are for the container.

So I don't actually see any disagreement here except perhaps for
terminology.

Eric

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

#1687618

On Fri, Jul 14, 2017 at 02:48:10PM -0400, Mimi Zohar wrote:
> 
> If I'm understanding the discussion correctly, this isn't an issue for
> layered copy on write filesystems, as each fs layer could have it's
> own set of xattrs.  The underlying and layered xattrs should be able
> to co-exist.  Use the layered xattr if it exists, but fall back to
> using the underlying xattr if it doesn't.

Note that this assumes that it is possible to "copy up" the xattrs
without necessarily "copying up" all of the data blocks.  This might
be true for some layers, but I don't believe it is currently true for
overlayfs, for example.

					- Ted

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

#1687629

On Fri, 2017-07-14 at 15:29 -0400, Theodore Ts'o wrote:
> On Fri, Jul 14, 2017 at 02:48:10PM -0400, Mimi Zohar wrote:
> > 
> > If I'm understanding the discussion correctly, this isn't an issue for
> > layered copy on write filesystems, as each fs layer could have it's
> > own set of xattrs.  The underlying and layered xattrs should be able
> > to co-exist.  Use the layered xattr if it exists, but fall back to
> > using the underlying xattr if it doesn't.
> 
> Note that this assumes that it is possible to "copy up" the xattrs
> without necessarily "copying up" all of the data blocks.  This might
> be true for some layers, but I don't believe it is currently true for
> overlayfs, for example.

Ok, so for the use case scneario where the container owner is willing
to use the public key distributed with the files, then only those
files that are new or modified in the overlay would need to be signed
with a key local to the overlay.  In the worst case scenario, where
the container owner is only willing to trust their own public key, I
guess we can live with having to copy up the files.

Mimi

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

#1687615

On Fri, 2017-07-14 at 13:17 -0500, Eric W. Biederman wrote:
> "Serge E. Hallyn" <serge@hallyn.com> writes:
> 
> > Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
> >> On 07/14/2017 09:34 AM, Serge E. Hallyn wrote:
> >> >Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
> >> >>On 07/13/2017 08:38 PM, Eric W. Biederman wrote:
> >> >>>Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
> >> >>>
> >> >>>>On 07/13/2017 01:49 PM, Eric W. Biederman wrote:
> >> >>>>
> >> >>>>>My big question right now is can you implement Ted's suggested
> >> >>>>>restriction.  Only one security.foo or secuirty.foo@... attribute ?
> >> >>>>We need to raw-list the xattrs and do the check before writing them. I am fairly sure this can be done.
> >> >>>>
> >> >>>>So now you want to allow security.foo and one security.foo@uid=<> or just a single one security.foo(@[[:print:]]*)?
> >> >>>>
> >> >>>The latter.
> >> >>That case would prevent a container user from overriding the xattr
> >> >>on the host. Is that what we want? For limiting the number of xattrs
> >> >Not really.  If the file is owned by a uid mapped into the container,
> >> >then the container root can chown the file which will clear the file
> >> >capability, after which he can set a new one.  If the file is not
> >> >owned by a uid mapped into the container, then container root could
> >> >not set a filecap anyway.
> >> 
> >> Let's say I installed a container where all files are signed and
> >> thus have security.ima. Now for some reason I want to re-sign some
> >> or all files inside that container. How would I do that ? Would I
> >> need to get rid of security.ima first, possibly by copying each
> >> file, deleting the original file, and renaming the copied file to
> >> the original name, or should I just be able to write out a new
> >> signature, thus creating security.ima@uid=1000 besides the
> >> security.ima ?
> >> 
> >>    Stefan
> >
> > Hi Mimi,
> >
> > what do you think makes most sense for IMA?
> 
> I am going to give my two cents since I have been thinking about this.
> 
> First I think this entire scheme plays hobs with the security.evm
> attribute as security.evm needs to know the names of the xattrs to
> protect.
> 
> I forget which attributes has a hash and what has a message
> athentication code.

security.ima contains either a file hash or a signature.  (file data)
security.evm contains either a signature or an hmac of the security
xattrs and other file metadata. (file meta-data)

The same rules would apply to security.evm, as described in my
response.  Based on it's view of the security xattrs, either the
native or namespace security.evm would be updated.

> If there is an attribute with a simple file hash I think it only make
> sense for the kernel to touch it, and I don't see any sense in having
> multiples.

Only files that are in the IMA-appraisal policy is the file hash
calculated and written out as security.ima.  Depending this policy,
does the security.ima exist.  So if the file is in policy for both the
native and namespace policies, agreed the same hash doesn't need to be
written as two different xattrs.

> If there is an attribute with a message authentication code (roughly a
> signed hash) it makes sense to have that to be tied to the kernel key
> ring that controlls the keys.  (Which probably means a per user
> namespace thing at some point).  But again pretty untouchable otherwise.

Right, the namespace would require it's own EVM key. 

> Which brings us to the semantic question of would it be nice to have
> stacked IMA/EVM on the same file.
> 
> I really don't think we do.  I think allowing multiple keys for
> different part of trusting files is easy enough that we should have no
> need to fight over which keys do which.

We definitely want to support different policies on the native and in
the namespace with different keys and keyrings.

Refer to Mehmet Kaaylap's recent post, which refers to a PoC version
of IMA namespacing - kernsec.org/pipermail/linux-security-module-
archive/2017-July/002286.html.

> Looking at integrity.h I see signature_v2_hdr that has a keyid.  Any use
> case I can think of for distributing a distribution image with ima/evm
> xattrs will need to use asymmetric keys aka public/private keypairs so
> that the originator of the content does not give away their private
> keys.

Agreed.

> Given that usefully we are talking about content that should be
> connected to keys in one way or another I don't believe it even makes
> sense at this point to attempt to use uids for dealing with ima and
> evm content.

We need to resolve the xattr issue in order to namespace IMA-
appraisal. 

Mimi

> Further looking Serge's previous patch is 300 lines of code Setfan's
> patch that provides the possibility of code resuse is 500 lines of code.
> 
> Increasingly it is looking to me that code reuse rather than concept
> reuse is a false economy.  The code does not get smaller.  The semantic
> differences make it problematic.  Possibly to the problematic to the
> point where significant pieces may not be reused.  The format breaks
> assumptions for other parts of the code like security.evm.  The format
> by multiple names instead of a single attribute requires more disk
> access so is less efficient.
> 
> In short I am seeing more code that runs slower and is harder to
> maintain.  Please point out where I am wrong.

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

#1687731

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> On Fri, 2017-07-14 at 13:17 -0500, Eric W. Biederman wrote:
>> "Serge E. Hallyn" <serge@hallyn.com> writes:
>> 
>> > Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
>> >> On 07/14/2017 09:34 AM, Serge E. Hallyn wrote:
>> >> >Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
>> >> >>On 07/13/2017 08:38 PM, Eric W. Biederman wrote:
>> >> >>>Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
>> >> >>>
>> >> >>>>On 07/13/2017 01:49 PM, Eric W. Biederman wrote:
>> >> >>>>
>> >> >>>>>My big question right now is can you implement Ted's suggested
>> >> >>>>>restriction.  Only one security.foo or secuirty.foo@... attribute ?
>> >> >>>>We need to raw-list the xattrs and do the check before writing them. I am fairly sure this can be done.
>> >> >>>>
>> >> >>>>So now you want to allow security.foo and one security.foo@uid=<> or just a single one security.foo(@[[:print:]]*)?
>> >> >>>>
>> >> >>>The latter.
>> >> >>That case would prevent a container user from overriding the xattr
>> >> >>on the host. Is that what we want? For limiting the number of xattrs
>> >> >Not really.  If the file is owned by a uid mapped into the container,
>> >> >then the container root can chown the file which will clear the file
>> >> >capability, after which he can set a new one.  If the file is not
>> >> >owned by a uid mapped into the container, then container root could
>> >> >not set a filecap anyway.
>> >> 
>> >> Let's say I installed a container where all files are signed and
>> >> thus have security.ima. Now for some reason I want to re-sign some
>> >> or all files inside that container. How would I do that ? Would I
>> >> need to get rid of security.ima first, possibly by copying each
>> >> file, deleting the original file, and renaming the copied file to
>> >> the original name, or should I just be able to write out a new
>> >> signature, thus creating security.ima@uid=1000 besides the
>> >> security.ima ?
>> >> 
>> >>    Stefan
>> >
>> > Hi Mimi,
>> >
>> > what do you think makes most sense for IMA?
>> 
>> I am going to give my two cents since I have been thinking about this.
>> 
>> First I think this entire scheme plays hobs with the security.evm
>> attribute as security.evm needs to know the names of the xattrs to
>> protect.
>> 
>> I forget which attributes has a hash and what has a message
>> athentication code.
>
> security.ima contains either a file hash or a signature.  (file data)
> security.evm contains either a signature or an hmac of the security
> xattrs and other file metadata. (file meta-data)
>
> The same rules would apply to security.evm, as described in my
> response.  Based on it's view of the security xattrs, either the
> native or namespace security.evm would be updated.
>
>> If there is an attribute with a simple file hash I think it only make
>> sense for the kernel to touch it, and I don't see any sense in having
>> multiples.
>
> Only files that are in the IMA-appraisal policy is the file hash
> calculated and written out as security.ima.  Depending this policy,
> does the security.ima exist.  So if the file is in policy for both the
> native and namespace policies, agreed the same hash doesn't need to be
> written as two different xattrs.
>
>> If there is an attribute with a message authentication code (roughly a
>> signed hash) it makes sense to have that to be tied to the kernel key
>> ring that controlls the keys.  (Which probably means a per user
>> namespace thing at some point).  But again pretty untouchable otherwise.
>
> Right, the namespace would require it's own EVM key. 
>
>> Which brings us to the semantic question of would it be nice to have
>> stacked IMA/EVM on the same file.
>> 
>> I really don't think we do.  I think allowing multiple keys for
>> different part of trusting files is easy enough that we should have no
>> need to fight over which keys do which.
>
> We definitely want to support different policies on the native and in
> the namespace with different keys and keyrings.
>
> Refer to Mehmet Kaaylap's recent post, which refers to a PoC version
> of IMA namespacing - kernsec.org/pipermail/linux-security-module-
> archive/2017-July/002286.html.
>
>> Looking at integrity.h I see signature_v2_hdr that has a keyid.  Any use
>> case I can think of for distributing a distribution image with ima/evm
>> xattrs will need to use asymmetric keys aka public/private keypairs so
>> that the originator of the content does not give away their private
>> keys.
>
> Agreed.
>
>> Given that usefully we are talking about content that should be
>> connected to keys in one way or another I don't believe it even makes
>> sense at this point to attempt to use uids for dealing with ima and
>> evm content.
>
> We need to resolve the xattr issue in order to namespace IMA-
> appraisal. 


Mimi I have two questions:

a) Is the keyid enough to distinguish the security.ima and security.evm
   xattrs of one container from another container and from native?  Or
   do we have some important security xattrs that are associated with
   keys that don't have a keyid?

b) Can we reasonably live with a limitation that the native and the
   namespace'd policies don't intersect?  Or in the case of an
   interesection the native policy is the only one that is executed?

I submit that if the answer is keyids are always present, and we can
live with the native policy taking precedence over the container policy
then we have a solution to the IMA xattrs.

Eric

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

#1688190

On Fri, 2017-07-14 at 19:02 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > On Fri, 2017-07-14 at 13:17 -0500, Eric W. Biederman wrote:
> >> "Serge E. Hallyn" <serge@hallyn.com> writes:
> >> 
> >> > Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
> >> >> On 07/14/2017 09:34 AM, Serge E. Hallyn wrote:
> >> >> >Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
> >> >> >>On 07/13/2017 08:38 PM, Eric W. Biederman wrote:
> >> >> >>>Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
> >> >> >>>
> >> >> >>>>On 07/13/2017 01:49 PM, Eric W. Biederman wrote:
> >> >> >>>>
> >> >> >>>>>My big question right now is can you implement Ted's suggested
> >> >> >>>>>restriction.  Only one security.foo or secuirty.foo@... attribute ?
> >> >> >>>>We need to raw-list the xattrs and do the check before writing them. I am fairly sure this can be done.
> >> >> >>>>
> >> >> >>>>So now you want to allow security.foo and one security.foo@uid=<> or just a single one security.foo(@[[:print:]]*)?
> >> >> >>>>
> >> >> >>>The latter.
> >> >> >>That case would prevent a container user from overriding the xattr
> >> >> >>on the host. Is that what we want? For limiting the number of xattrs
> >> >> >Not really.  If the file is owned by a uid mapped into the container,
> >> >> >then the container root can chown the file which will clear the file
> >> >> >capability, after which he can set a new one.  If the file is not
> >> >> >owned by a uid mapped into the container, then container root could
> >> >> >not set a filecap anyway.
> >> >> 
> >> >> Let's say I installed a container where all files are signed and
> >> >> thus have security.ima. Now for some reason I want to re-sign some
> >> >> or all files inside that container. How would I do that ? Would I
> >> >> need to get rid of security.ima first, possibly by copying each
> >> >> file, deleting the original file, and renaming the copied file to
> >> >> the original name, or should I just be able to write out a new
> >> >> signature, thus creating security.ima@uid=1000 besides the
> >> >> security.ima ?
> >> >> 
> >> >>    Stefan
> >> >
> >> > Hi Mimi,
> >> >
> >> > what do you think makes most sense for IMA?
> >> 
> >> I am going to give my two cents since I have been thinking about this.
> >> 
> >> First I think this entire scheme plays hobs with the security.evm
> >> attribute as security.evm needs to know the names of the xattrs to
> >> protect.
> >> 
> >> I forget which attributes has a hash and what has a message
> >> athentication code.
> >
> > security.ima contains either a file hash or a signature.  (file data)
> > security.evm contains either a signature or an hmac of the security
> > xattrs and other file metadata. (file meta-data)
> >
> > The same rules would apply to security.evm, as described in my
> > response.  Based on it's view of the security xattrs, either the
> > native or namespace security.evm would be updated.
> >
> >> If there is an attribute with a simple file hash I think it only make
> >> sense for the kernel to touch it, and I don't see any sense in having
> >> multiples.
> >
> > Only files that are in the IMA-appraisal policy is the file hash
> > calculated and written out as security.ima.  Depending this policy,
> > does the security.ima exist.  So if the file is in policy for both the
> > native and namespace policies, agreed the same hash doesn't need to be
> > written as two different xattrs.
> >
> >> If there is an attribute with a message authentication code (roughly a
> >> signed hash) it makes sense to have that to be tied to the kernel key
> >> ring that controlls the keys.  (Which probably means a per user
> >> namespace thing at some point).  But again pretty untouchable otherwise.
> >
> > Right, the namespace would require it's own EVM key. 
> >
> >> Which brings us to the semantic question of would it be nice to have
> >> stacked IMA/EVM on the same file.
> >> 
> >> I really don't think we do.  I think allowing multiple keys for
> >> different part of trusting files is easy enough that we should have no
> >> need to fight over which keys do which.
> >
> > We definitely want to support different policies on the native and in
> > the namespace with different keys and keyrings.
> >
> > Refer to Mehmet Kaaylap's recent post, which refers to a PoC version
> > of IMA namespacing - kernsec.org/pipermail/linux-security-module-
> > archive/2017-July/002286.html.
> >
> >> Looking at integrity.h I see signature_v2_hdr that has a keyid.  Any use
> >> case I can think of for distributing a distribution image with ima/evm
> >> xattrs will need to use asymmetric keys aka public/private keypairs so
> >> that the originator of the content does not give away their private
> >> keys.
> >
> > Agreed.
> >
> >> Given that usefully we are talking about content that should be
> >> connected to keys in one way or another I don't believe it even makes
> >> sense at this point to attempt to use uids for dealing with ima and
> >> evm content.
> >
> > We need to resolve the xattr issue in order to namespace IMA-
> > appraisal. 
> 
> 
> Mimi I have two questions:
> 
> a) Is the keyid enough to distinguish the security.ima and security.evm
>    xattrs of one container from another container and from native?  Or
>    do we have some important security xattrs that are associated with
>    keys that don't have a keyid?
> 
> b) Can we reasonably live with a limitation that the native and the
>    namespace'd policies don't intersect?  Or in the case of an
>    interesection the native policy is the only one that is executed?
> 
> I submit that if the answer is keyids are always present, and we can
> live with the native policy taking precedence over the container policy
> then we have a solution to the IMA xattrs.

IMA-measurement is hierachical, meaning that the measurement policy
determines whether the measurement exists in the native, the
container, or both measurement lists.

One of the main namespacing use cases for IMA-appraisal is the ability
to limit running an executable to a particular container.  So unlike
IMA-measurement, which is hierarchical, the IMA-appraisal namespace
policy takes precedence over the native policy.

Mimi

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

#1687605

Stefan Berger <stefanb@linux.vnet.ibm.com> writes:

> On 07/14/2017 09:34 AM, Serge E. Hallyn wrote:
>> Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
>>> On 07/13/2017 08:38 PM, Eric W. Biederman wrote:
>>>> Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
>>>>
>>>>> On 07/13/2017 01:49 PM, Eric W. Biederman wrote:
>>>>>
>>>>>> My big question right now is can you implement Ted's suggested
>>>>>> restriction.  Only one security.foo or secuirty.foo@... attribute ?
>>>>> We need to raw-list the xattrs and do the check before writing them. I am fairly sure this can be done.
>>>>>
>>>>> So now you want to allow security.foo and one security.foo@uid=<> or just a single one security.foo(@[[:print:]]*)?
>>>>>
>>>> The latter.
>>> That case would prevent a container user from overriding the xattr
>>> on the host. Is that what we want? For limiting the number of xattrs
>> Not really.  If the file is owned by a uid mapped into the container,
>> then the container root can chown the file which will clear the file
>> capability, after which he can set a new one.  If the file is not
>> owned by a uid mapped into the container, then container root could
>> not set a filecap anyway.
>
> Let's say I installed a container where all files are signed and thus have
> security.ima. Now for some reason I want to re-sign some or all files inside
> that container. How would I do that ? Would I need to get rid of security.ima
> first, possibly by copying each file, deleting the original file, and renaming
> the copied file to the original name, or should I just be able to write out a
> new signature, thus creating security.ima@uid=1000 besides the security.ima ?

This gets us into some interesting territory, where the semantics of
these attributes matters.

The implementation of security.capable implements the security killpriv
hooks.  Anyone merely by changing the file can cause the security
capability to go away.  So it makes sense from the security.capable side
that anyone who has the capable_wrt_inode_uidgid(CAP_SETFCAP) will be
able to clear and set security.capable.

The integrity xattrs do not.  Which results in very big semantic
difference between these two kinds of attributes.  I am insufficiently
familiar with the rules for security.ima and security.evm to understand
what those rules should be.

That may be enough that we can not share code between these two cases.

Eric

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

#1687614

On 07/14/2017 01:36 PM, Eric W. Biederman wrote:
> Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
>
>> On 07/14/2017 09:34 AM, Serge E. Hallyn wrote:
>>> Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
>>>> On 07/13/2017 08:38 PM, Eric W. Biederman wrote:
>>>>> Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
>>>>>
>>>>>> On 07/13/2017 01:49 PM, Eric W. Biederman wrote:
>>>>>>
>>>>>>> My big question right now is can you implement Ted's suggested
>>>>>>> restriction.  Only one security.foo or secuirty.foo@... attribute ?
>>>>>> We need to raw-list the xattrs and do the check before writing them. I am fairly sure this can be done.
>>>>>>
>>>>>> So now you want to allow security.foo and one security.foo@uid=<> or just a single one security.foo(@[[:print:]]*)?
>>>>>>
>>>>> The latter.
>>>> That case would prevent a container user from overriding the xattr
>>>> on the host. Is that what we want? For limiting the number of xattrs
>>> Not really.  If the file is owned by a uid mapped into the container,
>>> then the container root can chown the file which will clear the file
>>> capability, after which he can set a new one.  If the file is not
>>> owned by a uid mapped into the container, then container root could
>>> not set a filecap anyway.
>> Let's say I installed a container where all files are signed and thus have
>> security.ima. Now for some reason I want to re-sign some or all files inside
>> that container. How would I do that ? Would I need to get rid of security.ima
>> first, possibly by copying each file, deleting the original file, and renaming
>> the copied file to the original name, or should I just be able to write out a
>> new signature, thus creating security.ima@uid=1000 besides the security.ima ?
> This gets us into some interesting territory, where the semantics of
> these attributes matters.
>
> The implementation of security.capable implements the security killpriv
> hooks.  Anyone merely by changing the file can cause the security
> capability to go away.  So it makes sense from the security.capable side
> that anyone who has the capable_wrt_inode_uidgid(CAP_SETFCAP) will be
> able to clear and set security.capable.
>
> The integrity xattrs do not.  Which results in very big semantic
> difference between these two kinds of attributes.  I am insufficiently
> familiar with the rules for security.ima and security.evm to understand
> what those rules should be.
>
> That may be enough that we can not share code between these two cases.

On the host I can simply overwrite capabilities. I think the same model 
should apply to the virtualized world. The difference still is that 
removing an xattr, if written on the host, may only be possible by copy 
+ file move to original filename.

On IMA, when appending a letter to an executable, the executable doesn't 
run anymore when appraisal is used, but the signature is still there and 
needs to be re-written. Though I think this aspect on how they disappear 
doesn't matter as much if they can simply be overwritten.

Some things could certainly be solved with flags indicating behaviors of 
xattrs for as long as these flags only affect the reading, listing, and 
re-writing of the virtualized xattrs, which is what the patch does. For 
example a flag for security.capability could say that only a single 
'security.capability(@uid=<uid>)?' may exist while security.ima could 
have two.

    Stefan

>
> Eric
>

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

#1686935

Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
> For virtualizing the xattrs on the 'value' side I was looking for
> whether there's something like a 'wrapper' structure around the
> actual value of the xattr so that that wrapper could be extended to
> support different values at different uids and applied to any xattr.
> Unfortunately there's no such 'wrapper'.

I believe my very first implementation did essentially this - it used
the not uncommon structure of (mostly making this up):

struct ns_vfs_cap {
	int magic;
	int ncaps;
	struct ns_vfs_cap_data data[0];
};

with (ncaps * sizeof(ns_vfs_cap_data)) following that.

I didn't like it.

-serge

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

#1686922

Quoting Theodore Ts'o (tytso@mit.edu):
> On Thu, Jul 13, 2017 at 07:11:36AM -0500, Eric W. Biederman wrote:
> > The concise summary:
> > 
> > Today we have the xattr security.capable that holds a set of
> > capabilities that an application gains when executed.  AKA setuid root exec
> > without actually being setuid root.
> > 
> > User namespaces have the concept of capabilities that are not global but
> > are limited to their user namespace.  We do not currently have
> > filesystem support for this concept.
> 
> So correct me if I am wrong; in general, there will only be one
> variant of the form:
> 
>    security.foo@uid=15000
> 
> It's not like there will be:
> 
>    security.foo@uid=1000
>    security.foo@uid=2000
> 
> Except.... if you have an Distribution root directory which is shared
> by many containers, you would need to put the xattrs in the overlay
> inodes.

Is that a problem?  Essentially people who would try to do the
above also want to use 'shiftfs' stackable filesystem, which would
presumably eventually do this for you.

>   Worse, each time you launch a new container, with a new
> subuid allocation, you will have to iterate over all files with
> capabilities and do a copy-up operations on the xattrs in overlayfs.
> So that's actually a bit of a disaster.

Only if you create the container rootfs as a copy.

Note that generally they would want to walk the fs in that case anyway, to chown
the files into the container.  And said chown would clear out any existing file
capabilities (and suid/sgid bits).

On the other hand, unprivileged lxc containers are created by
untarring the distro image straight into the mapped user namespace.
So no chowning is needed, and - once we we have this properly supported -
the filecaps should be automatically written correctly for the container.

> So for distribution overlays, you will need to do things a different
> way, which is to map the distro subdirectory so you know that the
> capability with the global uid 0 should be used for the container
> "root" uid, right?
> 
> So this hack of using security.foo@uid=1000 is *only* useful when the
> subcontainer root wants to create the privileged executable.  You
> still have to do things the other way.
> 
> So can we make perhaps the assertion that *either*:
> 
>    security.foo
> 
> exists, *or*
> 
>    security.foo@uid=BAR
> 
> exists, but never both?  And there BAR is exclusive to only one
> instances?

I think that's fine.

-serge

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

#1686156

Quoting Eric W. Biederman (ebiederm@xmission.com):
> "Serge E. Hallyn" <serge@hallyn.com> writes:
> 
> > Quoting Eric W. Biederman (ebiederm@xmission.com):
> >> Stefan Berger <"Stefan Bergerstefanb"@linux.vnet.ibm.com> writes:
> >> > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> >> > Signed-off-by: Serge Hallyn <serge@hallyn.com>
> >> > Reviewed-by: Serge Hallyn <serge@hallyn.com>
> >> 
> >> It doesn't look like this is coming through Serge so I don't see how
> >> the Signed-off-by tag is legtimate.
> >
> > This is mostly explained by the fact that there have been a *lot* of
> > changes, many of them discussed in private emails.
> >
> >> >From the replies to this it doesn't look like Serge has reviewed this
> >> version either.
> >> 
> >> I am disappointed that all of my concerns about technical feasibility
> >> remain unaddressed.
> >
> > Can you re-state those, or give a link to them?
> 
> Well I only posted about one substantive comment on the last round
> so it should be easy to find that said.

Ok so you are likely referring to http://lkml.org/lkml/2017/6/23/551 ,
thanks.  I had actually read that differently when you sent it, and
thought it was more to do with the suggestion of putting the nsid
tags in the middle of the xattr name versus putting it on the end.
As far as that is concerned, note that no other tags besides uid=
are currently supported, and only security.capability is being
namespaced.

> The big question is how does this intereact with filesystems
> xattr implementations?
> 
> There is the potential that we create many more security xattrs this
> way.  How does that scale?  With more names etc.
> What happens if we have one xattr per uid for 1000+ uids?

Well, that's not the intent here.  The goal is *not* to make one fs image
that satisfies 200k possible uid mappings.  The goal is to reconcile the
support for an unprivileged user to set uid agnostic (within the
container) file capabilities with the uid namespace's design goals -
namely root in a container is privileged over the container but
completely unprivileged wrt the host.

This is in part why in my previous version I only allowed a single
namespaced fscap.  But I don't think that we have to enforce a
single fscap - I think it's fair to tell users "go ahead and shoot
yourself in the foot" performance-wise, if they insist on doing
this.

The goal of now putting the root kuid in the name is not to support
multiple containers, but to have common code supporting
security.capability and security.ima, and maybe a few more.

> How does this interact with filesystems optimization of xattr names?
> For some filesystems they optmize the xattr names, and don't store the
> entire thing.

This I have no idea on.  Stefan, have you looked at this?

> > I'd really like to get to a point where unprivileged containers can start
> > using filecaps - at this point if that means having an extra temporary
> > file format based on my earlier patchset while we hash this out, that
> > actually seems worthwhile.  But it would of course be ideal if we could
> > do the name based caps right in the first place.
> 
> This whole new version has set my review back to square one
> unfortunately.

Well it is a whole new approach and whole new patch, so of course that's
to be expected :(

-serge

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

#1685937

On Tue, Jul 11, 2017 at 11:05:11AM -0400, Stefan Berger wrote:

[..]
> @@ -301,14 +721,39 @@ ssize_t
>  __vfs_getxattr(struct dentry *dentry, struct inode *inode, const char *name,
>  	       void *value, size_t size)
>  {
> -	const struct xattr_handler *handler;
> +	const struct xattr_handler *handler = NULL;
> +	char *newname =  NULL;
> +	int ret, userns_supt_xattr;
> +	struct user_namespace *userns = current_user_ns();
> +
> +	userns_supt_xattr = (xattr_is_userns_supported(name, false) >= 0);
> +

Hi Stephan,

> +	do {
> +		kfree(newname);
> +
> +		newname = xattr_userns_name(name, userns);
					    ^^^
Will name be pointing to a freed string in second iteration of loop. 

> +		if (IS_ERR(newname))
> +			return PTR_ERR(newname);
> +
> +		if (!handler) {
> +			name = newname;

Here we assign name and at the beginning of second iteration we free 
newname.

Also I am not sure why do we do this assignment only if handler is NULL.

BTW, I set cap_sys_admin on a file outside usernamespace and then launched
user namespace (mapping 1000 to 0). And then tried to do getcap on file
and I am not seeing security.capability set by host. Not sure what am I 
doing wrong. getxattr() seems to return -ENODATA. Still debugging it.

Also, have we resovled the question of stacked filesystem like overlayfs.
There we are switching creds to mounter's creds when doing operations on
underlying filesystem. I am concenrned does that mean, we will get and
return security.capability to caller in usernamespace instead of
security.capability@uid=1000. 

Vivek

> +			handler = xattr_resolve_name(inode, &name);
> +			if (IS_ERR(handler)) {
> +				ret = PTR_ERR(handler);
> +				goto out;
> +			}
> +			if (!handler->get) {
> +				ret = -EOPNOTSUPP;
> +				goto out;
> +			}
> +		}
> +		ret = handler->get(handler, dentry, inode, name, value, size);
> +		userns = userns->parent;
> +	} while ((ret == -ENODATA) && userns && userns_supt_xattr);
>  
> -	handler = xattr_resolve_name(inode, &name);
> -	if (IS_ERR(handler))
> -		return PTR_ERR(handler);
> -	if (!handler->get)
> -		return -EOPNOTSUPP;
> -	return handler->get(handler, dentry, inode, name, value, size);
> +out:
> +	kfree(newname);
> +	return ret;
>  }
>  EXPORT_SYMBOL(__vfs_getxattr);
>  

Thanks
Vivek

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

#1685986

On 07/12/2017 01:53 PM, Vivek Goyal wrote:
> On Tue, Jul 11, 2017 at 11:05:11AM -0400, Stefan Berger wrote:
>
> [..]
>> @@ -301,14 +721,39 @@ ssize_t
>>   __vfs_getxattr(struct dentry *dentry, struct inode *inode, const char *name,
>>   	       void *value, size_t size)
>>   {
>> -	const struct xattr_handler *handler;
>> +	const struct xattr_handler *handler = NULL;
>> +	char *newname =  NULL;
>> +	int ret, userns_supt_xattr;
>> +	struct user_namespace *userns = current_user_ns();
>> +
>> +	userns_supt_xattr = (xattr_is_userns_supported(name, false) >= 0);
>> +
> Hi Stephan,
>
>> +	do {
>> +		kfree(newname);
>> +
>> +		newname = xattr_userns_name(name, userns);
> 					    ^^^
> Will name be pointing to a freed string in second iteration of loop.

Fixing for v3.

>
>> +		if (IS_ERR(newname))
>> +			return PTR_ERR(newname);
>> +
>> +		if (!handler) {
>> +			name = newname;
> Here we assign name and at the beginning of second iteration we free
> newname.
>
> Also I am not sure why do we do this assignment only if handler is NULL.

The handler shouldn't change but this optimization isn't helpful. Fixed 
through this patch:

https://github.com/stefanberger/linux/commit/10828401b29a13f8c56f8fad0c0fb2690e4af878


>
> BTW, I set cap_sys_admin on a file outside usernamespace and then launched
> user namespace (mapping 1000 to 0). And then tried to do getcap on file
> and I am not seeing security.capability set by host. Not sure what am I
> doing wrong. getxattr() seems to return -ENODATA. Still debugging it.

This was a regression due to the bug in the loop. I didn't have a test 
case (with runc) for it, now I do.

>
> Also, have we resovled the question of stacked filesystem like overlayfs.
> There we are switching creds to mounter's creds when doing operations on
> underlying filesystem. I am concenrned does that mean, we will get and
> return security.capability to caller in usernamespace instead of
> security.capability@uid=1000.

I would have to test this, otherwise I don't know. I'll try it out with 
Docker.

    Stefan

>
> Vivek
>
>> +			handler = xattr_resolve_name(inode, &name);
>> +			if (IS_ERR(handler)) {
>> +				ret = PTR_ERR(handler);
>> +				goto out;
>> +			}
>> +			if (!handler->get) {
>> +				ret = -EOPNOTSUPP;
>> +				goto out;
>> +			}
>> +		}
>> +		ret = handler->get(handler, dentry, inode, name, value, size);
>> +		userns = userns->parent;
>> +	} while ((ret == -ENODATA) && userns && userns_supt_xattr);
>>   
>> -	handler = xattr_resolve_name(inode, &name);
>> -	if (IS_ERR(handler))
>> -		return PTR_ERR(handler);
>> -	if (!handler->get)
>> -		return -EOPNOTSUPP;
>> -	return handler->get(handler, dentry, inode, name, value, size);
>> +out:
>> +	kfree(newname);
>> +	return ret;
>>   }
>>   EXPORT_SYMBOL(__vfs_getxattr);
>>   
> Thanks
> Vivek
>

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

#1687725

Stefan Berger <"Stefan Bergerstefanb"@linux.vnet.ibm.com> writes:

> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
>
> This patch enables security.capability in user namespaces but also
> takes a more general approach to enabling extended attributes in user
> namespaces.
>
> The following rules describe the approach using security.foo as a
> 'user namespace enabled' extended attribute:
>
> Reading of extended attributes:
>
> 1a) Reading security.foo from a user namespace will read
>     security.foo@uid=<uid> of the parent user namespace instead with uid
>     being the mapping of root in that parent user namespace. An
>     exception is if root is mapped to uid 0 on the host, and in this case
>     we will read security.foo directly.
>     --> reading security.foo will read security.foo@uid=1000 for uid
>         mapping of root to 1000.
>
> 1b) If security.foo@uid=<uid> is not available, the security.foo of the
>     parent namespace is tried to be read. This procedure is repeated up to
>     the init user namespace. This step only applies for reading of extended
>     attributes and provides the same behavior as older system where the
>     host's extended attributes applied to user namespaces.
>
> 2) All security.foo@uid=<uid> with valid uid mapping in the user namespace
>    can be read. The uid within the user namespace will be mapped to the
>    corresponding uid on the host and that uid will be used in the name of
>    the extended attribute.
>    -> reading security.foo@uid=1 will read security.foo@uid=1001 for uid
>       mapping of root to 1000, size of at least 2.
>
>    All security.foo@uid=<uid> can be read (by root) on the host with values
>    of <uid> also being subject to checking for valid mappings.
>
> 3) No other security.foo* can be read.
>
> The same rules for reading apply to writing and removing of user
> namespace enabled extended attributes.
>
> When listing extended attributes of a file, only those are presented
> to the user namespace that have a valid mapping. Besides that, names
> of the extended attributes are adjusted to represent the mapping.
> This means that if root is mapped to uid 1000 on the host, the
> security.foo@uid=1000 will be listed as security.foo in the user
> namespace, security.foo@uid=1001 becomes security.foo@uid=1 and so on.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Serge Hallyn <serge@hallyn.com>
> Reviewed-by: Serge Hallyn <serge@hallyn.com>
> ---
>  fs/xattr.c               | 509 +++++++++++++++++++++++++++++++++++++++++++++--
>  security/commoncap.c     |  36 +++-
>  security/selinux/hooks.c |   9 +-
>  3 files changed, 523 insertions(+), 31 deletions(-)

I am just going to quickly and publicly point out that as designed this
patch breaks evm inode metadata signing.  As evm_config_xattrnames is not
updated.

While not completely insurmountable that seems like a strong limitation of
this design.

Eric

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

#1688013

On 07/14/2017 07:41 PM, Eric W. Biederman wrote:
> Stefan Berger <"Stefan Bergerstefanb"@linux.vnet.ibm.com> writes:
>
>> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>
>> This patch enables security.capability in user namespaces but also
>> takes a more general approach to enabling extended attributes in user
>> namespaces.
>>
>> The following rules describe the approach using security.foo as a
>> 'user namespace enabled' extended attribute:
>>
>> Reading of extended attributes:
>>
>> 1a) Reading security.foo from a user namespace will read
>>      security.foo@uid=<uid> of the parent user namespace instead with uid
>>      being the mapping of root in that parent user namespace. An
>>      exception is if root is mapped to uid 0 on the host, and in this case
>>      we will read security.foo directly.
>>      --> reading security.foo will read security.foo@uid=1000 for uid
>>          mapping of root to 1000.
>>
>> 1b) If security.foo@uid=<uid> is not available, the security.foo of the
>>      parent namespace is tried to be read. This procedure is repeated up to
>>      the init user namespace. This step only applies for reading of extended
>>      attributes and provides the same behavior as older system where the
>>      host's extended attributes applied to user namespaces.
>>
>> 2) All security.foo@uid=<uid> with valid uid mapping in the user namespace
>>     can be read. The uid within the user namespace will be mapped to the
>>     corresponding uid on the host and that uid will be used in the name of
>>     the extended attribute.
>>     -> reading security.foo@uid=1 will read security.foo@uid=1001 for uid
>>        mapping of root to 1000, size of at least 2.
>>
>>     All security.foo@uid=<uid> can be read (by root) on the host with values
>>     of <uid> also being subject to checking for valid mappings.
>>
>> 3) No other security.foo* can be read.
>>
>> The same rules for reading apply to writing and removing of user
>> namespace enabled extended attributes.
>>
>> When listing extended attributes of a file, only those are presented
>> to the user namespace that have a valid mapping. Besides that, names
>> of the extended attributes are adjusted to represent the mapping.
>> This means that if root is mapped to uid 1000 on the host, the
>> security.foo@uid=1000 will be listed as security.foo in the user
>> namespace, security.foo@uid=1001 becomes security.foo@uid=1 and so on.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> Signed-off-by: Serge Hallyn <serge@hallyn.com>
>> Reviewed-by: Serge Hallyn <serge@hallyn.com>
>> ---
>>   fs/xattr.c               | 509 +++++++++++++++++++++++++++++++++++++++++++++--
>>   security/commoncap.c     |  36 +++-
>>   security/selinux/hooks.c |   9 +-
>>   3 files changed, 523 insertions(+), 31 deletions(-)
> I am just going to quickly and publicly point out that as designed this
> patch breaks evm inode metadata signing.  As evm_config_xattrnames is not
> updated.
>
> While not completely insurmountable that seems like a strong limitation of
> this design.

EVM could be converted to get the list of xattrs and prefix-compare it 
against the evm_config_xattrnames to do what it does now.

    Stefan

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

Page 3 of 4 — ← Prev page 1 2 [3] 4  Next page →

Back to top | Article view | linux.kernel

csiph-web