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

[Serge E. Hallyn]

Quoting Serge E. Hallyn (serge@xxxxxxxxxx):
> Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx):
> > "Serge E. Hallyn" <serge@xxxxxxxxxx> writes:
> > 
> > > Quoting Andrew G. Morgan (morgan@xxxxxxxxxx):
> > >> 
> > >> 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