Messages in this thread | | | Subject | Re: [PATCH v4] Introduce v3 namespaced file capabilities | From | Stefan Berger <> | Date | Sun, 18 Jun 2017 21:13:28 -0400 |
| |
On 06/18/2017 06:14 PM, Serge E. Hallyn wrote: > Quoting Stefan Berger (stefanb@linux.vnet.ibm.com): >> On 06/14/2017 11:05 PM, Serge E. Hallyn wrote: >>> On Wed, Jun 14, 2017 at 08:27:40AM -0400, Stefan Berger wrote: >>>> On 06/13/2017 07:55 PM, Serge E. Hallyn wrote: >>>>> Quoting Stefan Berger (stefanb@linux.vnet.ibm.com): >>>>>> If all extended >>>>>> attributes were to support this model, maybe the 'uid' could be >>>>>> associated with the 'name' of the xattr rather than its 'value' (not >>>>>> sure whether that's possible). >>>>> Right, I missed that in your original email when I saw it this morning. >>>>> It's not what my patch does, but it's an interesting idea. Do you have >>>>> a patch to that effect? We might even be able to generalize that to >>>> No, I don't have a patch. It may not be possible to implement it. >>>> The xattr_handler's take the name of the xattr as input to get(). >>> That may be ok though. Assume the host created a container with >>> 100000 as the uid for root, which created a container with 130000 as >>> uid for root. If root in the nested container tries to read the >>> xattr, the kernel can check for security.foo[130000] first, then >>> security.foo[100000], then security.foo. Or, it can do a listxattr >>> and look for those. Am I overlooking one? >>> >>>> So one could try to encode the mapped uid in the name. However, that >>> I thought that's exactly what you were suggesting in your original >>> email? "security.capability[uid=2000]" >>> >>>> could lead to problems with stale xattrs in a shared filesystem over >>>> time unless one could limit the number of xattrs with the same >>>> prefix, e.g., security.capability*. So I doubt that it would work. >>> Hm. Yeah. But really how many setups are there like that? I.e. if >>> you launch a regular docker or lxd container, the image doesn't do a >>> bind mount of a shared image, it layers something above it or does a >>> copy. What setups do you know of where multiple containers in different >>> user namespaces mount the same filesystem shared and writeable? >> I think I have something now that accomodates userns access to >> security.capability: >> >> https://github.com/stefanberger/linux/commits/xattr_for_userns > Thanks! > >> Encoding of uid is in the attribute name now as follows: >> security.foo@uid=<uid> >> >> 1) The 'plain' security.capability is only r/w accessible from the >> host (init_user_ns). >> 2) When userns reads/writes 'security.capability' it will read/write >> security.capability@uid=<uid> instead, with uid being the uid of >> root , e.g. 1000. >> 3) When listing xattrs for userns the host's security.capability is >> filtered out to avoid read failures iof 'security.capability' if >> security.capability@uid=<uid> is read but not there. (see 1) and 2)) >> 4) security.capability* may all be read from anywhere >> 5) security.capability@uid=<uid> may be read or written directly >> from a userns if <uid> matches the uid of root (current_uid()) > This looks very close to what we want. One exception - we do want > to support root in a user namespace being able to write > security.capability@uid=<x> where <x> is a valid uid mapped in its > namespace. In that case the name should be rewritten to be > security.capability@uid=<y> where y is the unmapped kuid.val.
I'll try to write a patch on top of the existing one.
Can you adapt your test cases. I haven't tried them, but having them would be important.
> > Eric, > > so far my patch hasn't yet hit Linus' tree. Given that, would you > mind taking a look and seeing what you think of this approach? If > we may decide to go this route, we probably should stop my patch > from hitting Linus' tree before we have to continue supporting it. > > Stefan, > > so do you think the general framework could be re-used by IMA? If > we can move the capability-specific code in fs/xattr.c into > an LSM hook in a way that IMA can also use, then this is a definite > win.
I am fairly sure that this would be easily possible and some of the if statements with string comparisons would likely only have to be extended with another comparison.
Regards, Stefan
> > -serge >
| |