Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Eric W. Biederman (ebied...@xmission.com): > "Serge E. Hallyn" writes: > > > Quoting Eric W. Biederman (ebied...@xmission.com): > > >> A child user namespace having capabilities against processes in it's > >> parent seems totally bizarre and pretty dangerous from a capabilities > >> standpoint. > > > > How would it have them against its parent? > > init_user_ns >userns a --- created by kuid 1 Now a mapping needs to be set up (by a task with CAP_SYS_ADMIN in init_user_ns) which allows kuids 1 and 2 to be used by userns a. Otherwise (if no mapping is set up) userns a only has the overlowuid. Realistically only kuids over 10 (let's say) would used. i.e. kuids 100,000-199,999 would map to container uids 0-99,999. > userns b -- created by kuid 2 Now a mapping needs to be set up (by a task with CAP_SYS_ADMIN in userns a) which allows kuids 1 and 2 to be used by userns b. If userns had been mapped with kuids 100,000-199,999 mapping to uids 0-9, then only kuids in that range could be mapped into userns b. > process c in userns b with kuid 1 > > Serge read the first permisison check in common_cap. > Think what happens in the above example. > > For the rest I understand your concern. Ok. Then we can discuss my concern later (after the new year). > Serge please read and look at the patches I have posted to fix > the issues Andy found with the user namespace tree. Especially > the fix to commit_creds. The setns fixes were IMO the most important - and interesting - ones :) Thanks, Andy! > After you have looked at the patches to fix the issues I will > be happy to discuss things further with you. Thanks, -serge -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
"Serge E. Hallyn" writes: > Quoting Eric W. Biederman (ebied...@xmission.com): >> A child user namespace having capabilities against processes in it's >> parent seems totally bizarre and pretty dangerous from a capabilities >> standpoint. > > How would it have them against its parent? init_user_ns userns a --- created by kuid 1 userns b -- created by kuid 2 process c in userns b with kuid 1 Serge read the first permisison check in common_cap. Think what happens in the above example. For the rest I understand your concern. Serge please read and look at the patches I have posted to fix the issues Andy found with the user namespace tree. Especially the fix to commit_creds. After you have looked at the patches to fix the issues I will be happy to discuss things further with you. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Linus Torvalds writes: > On Fri, Dec 14, 2012 at 10:12 AM, Eric W. Biederman > wrote: >> >> That said Serge I think I have lost track of the point of your question. > > .. and I'm a bit unsure what I should do about this all. Including > pulling the pull request that actually can make this all matter. > > Hmm? Any consensus? It looks like we have consensus (baring the color of the shed) of what the code should look like for v3.8. >From the most embarrassingly timed, but most useful review by Andy I have 4 fixes queued up in my development tree. Fixing cap_capable to test for the parent namespace. Fixing setns to require nsown_capable(CAP_SYS_ADMIN) -- Fixing commit_creds to not clear task dumpable unnecessarily. Fixing a typo in the description. What I would like to do is to do is what I would if this was not the middle of the merge window with changes like this. Toss those patches out for last round of review. Possibly toss the last two patches if there are any problems because they are not necessary. Put the patches in my for-next branch and have them sit in linux-next for a day or three. Send you an updated pull request. I am recovering from a cold so I am running slower than I would like this week and would really rather not rush getting these patches out. What I don't want to be is so cautious and careful that you decide to pass on my pull request. The code is harmless with user namespaces disabled. The code has been baking for a long time, some of it for much too long and it is as solid as I think it will get out before being merged. Nor is the code complex Andy managed to dig and figure it all out in about a day. Linus does that work for you? Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Linus Torvalds (torva...@linux-foundation.org): > On Fri, Dec 14, 2012 at 10:12 AM, Eric W. Biederman > wrote: > > > > That said Serge I think I have lost track of the point of your question. > > .. and I'm a bit unsure what I should do about this all. Including > pulling the pull request that actually can make this all matter. Sorry I didn't mean to complicate this. I did ack the patch and we can cull the cc list for continued discussion. In practical terms, the only thing the patch prevent is having two separate tasks each clone a new user ns with the same uid mapping, and having consistent relationships between the same uids between the namespaces. It's worth it to prevent (or while we consider) the case Andy and Eric bring up. > Hmm? Any consensus? Acked-by: Serge Hallyn -serge -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Eric W. Biederman (ebied...@xmission.com): > "Serge E. Hallyn" writes: Note: I acked your patch before and still don't object to it. > > In which case it would be > > > >child_user_ns1 [10-19] > >child_user_ns2 [10-19] > > child_user_ns3 [12-12] > > > > Yes. You have to nest uids. > > >> > with unfortunate mappings - ns1 and ns2 should have had nonoverlapping > >> > ranges, but in any case now uid 1000 in ns1 can exert privilege over > >> > ns3. Again, uids comparisons will succeed for file access anyway, so > >> > ns1 can 0wn ns2 and ns3 other ways. > >> > >> Yes yours is the more realistic scenario. Mine was simplified to be clear. > >> > >> > Heck I'm starting to think the bug is a feature - surely given the > >> > mappings above I meant for ns1 and ns2 to bleed privilege to each > >> > other? > >> > >> The serious problem is that privileges can bleed up. A user in > >> ns3 can wind up owning ns2 or ns1. Which totally defeats the permission > >> model. You have CAP_DAC_OVERRIDE so you don't even need access to files > >> you own, etc, etc. > > > > Would that not require intervention from the init_user_ns? In my > > example above (let's add that ns2 is owned by kuid.uid=1000 in > > init_user_ns), root in child_user_ns2 cannot map kuid.val=0 or > > kuid.val=1000 into ns3 because 0 and 1000 are not in the range > > 10-19. So there is no uid in child_user_ns3 which is able > > to spoof uid=0 in child_user_ns1. > > Right. It does require having the uid of the owner of ns1 or ns2 in > ns3. So you have to explicitly allow it. > > What I don't see is any point in allowing something like that. The point isn't specifically to allow that, but rather to not muddle the kuids with the user namespace *. If I clone two tasks in separate user namespace but give them the same uid mappings, how should the uids between those namespaces be related? The way you're doing it now they will equate for DAC checks but not for privilege checks. It feels ad-hoc and flaky. > A child user namespace having capabilities against processes in it's > parent seems totally bizarre and pretty dangerous from a capabilities > standpoint. How would it have them against its parent? > That said Serge I think I have lost track of the point of your question. Look at it this way. You've introduced kuids which allow "uids" to be disassociated from the user namespace * in kernel. You have the uid mapping rules which enforce some sanity (requiring nesting). If the parent ns has re-used an active uid of its own to give to a child namespace, then any DAC-governed objects (files in particular) will be owned by that uid in the child user ns. I'm not sure - does this also apply to /proc/$$/fd/* access? -serge -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
On Fri, Dec 14, 2012 at 10:43 AM, Linus Torvalds wrote: > On Fri, Dec 14, 2012 at 10:12 AM, Eric W. Biederman > wrote: >> >> That said Serge I think I have lost track of the point of your question. > > .. and I'm a bit unsure what I should do about this all. Including > pulling the pull request that actually can make this all matter. > > Hmm? Any consensus? I think that, if Eric submits a newer version that renames the loop variable for added comprehensibility, I'm okay with it. Changing the semantics to a more expansive version like Serge was talking about later on wouldn't break anything. But I don't think there's any reason to change it. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
On Fri, Dec 14, 2012 at 10:12 AM, Eric W. Biederman wrote: > > That said Serge I think I have lost track of the point of your question. .. and I'm a bit unsure what I should do about this all. Including pulling the pull request that actually can make this all matter. Hmm? Any consensus? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
"Serge E. Hallyn" writes: > Quoting Eric W. Biederman (ebied...@xmission.com): >> "Serge E. Hallyn" writes: >> >> > Quoting Eric W. Biederman (ebied...@xmission.com): >> >> "Serge E. Hallyn" writes: >> >> >> >> > Quoting Eric W. Biederman (ebied...@xmission.com): >> >> >> >> >> >> Andy Lutomirski pointed out that the current behavior of allowing the >> >> >> owner of a user namespace to have all caps when that owner is not in a >> >> >> parent user namespace is wrong. >> >> > >> >> > To make sure I understand right, the issue is when a uid is mapped >> >> > into multiple namespaces. >> >> >> >> Yes. >> >> >> >> i.e. uid 1000 in ns1 may own ns2, but uid 1000 in ns3 does not? >> >> >> >> I am not certain of your example. >> >> >> >> The simple case is: >> >> >> >> init_user_ns: >> >> child_user_ns1 (owned by uid == 0 [in all user namespaces]) >> >>child_user_ns2 (owned by uid == 0 [ in all user namespaces]) >> >> >> >> >> >> root (uid == 0) in child_user_ns2 has all rights over anything in >> >> child_user_ns1. >> > >> > Well that is only if there was no mapping. (since we're comparing >> > kuids, not uid_ts). right? If you didn't map uid 0 in child_user_ns2 >> > to another id in the parent ns, you weren't all *that* serious about >> > isolating the ns. >> > >> > The case I was thinking is >> > >> > init_user_ns: [0-uidmax] >> > child_user_ns1 [10-19] >> > child_user_ns2 [10-19] >> > child_user_ns3 [20-29] > > Wait is my example above possible? Or does child_user_ns3's range need > to be a subset of child_user_ns2's? > > In which case it would be > >child_user_ns1 [10-19] >child_user_ns2 [10-19] > child_user_ns3 [12-12] > Yes. You have to nest uids. >> > with unfortunate mappings - ns1 and ns2 should have had nonoverlapping >> > ranges, but in any case now uid 1000 in ns1 can exert privilege over >> > ns3. Again, uids comparisons will succeed for file access anyway, so >> > ns1 can 0wn ns2 and ns3 other ways. >> >> Yes yours is the more realistic scenario. Mine was simplified to be clear. >> >> > Heck I'm starting to think the bug is a feature - surely given the >> > mappings above I meant for ns1 and ns2 to bleed privilege to each >> > other? >> >> The serious problem is that privileges can bleed up. A user in >> ns3 can wind up owning ns2 or ns1. Which totally defeats the permission >> model. You have CAP_DAC_OVERRIDE so you don't even need access to files >> you own, etc, etc. > > Would that not require intervention from the init_user_ns? In my > example above (let's add that ns2 is owned by kuid.uid=1000 in > init_user_ns), root in child_user_ns2 cannot map kuid.val=0 or > kuid.val=1000 into ns3 because 0 and 1000 are not in the range > 10-19. So there is no uid in child_user_ns3 which is able > to spoof uid=0 in child_user_ns1. Right. It does require having the uid of the owner of ns1 or ns2 in ns3. So you have to explicitly allow it. What I don't see is any point in allowing something like that. After taking a second look I just realized that this is completely unexploitable with the code that is currently merged. As creating a grand child user namespace is competelely impossible. Creating a user namespace is requires capable(CAP_SYS_ADMIN) which is never present in anything but the initial user namespace. That said I think the current semantics of cap_capable are completely fatal to reasoning about user namespaces. A child user namespace having capabilities against processes in it's parent seems totally bizarre and pretty dangerous from a capabilities standpoint. That said Serge I think I have lost track of the point of your question. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Eric W. Biederman (ebied...@xmission.com): > "Serge E. Hallyn" writes: > > > Quoting Eric W. Biederman (ebied...@xmission.com): > >> "Serge E. Hallyn" writes: > >> > >> > Quoting Eric W. Biederman (ebied...@xmission.com): > >> >> > >> >> Andy Lutomirski pointed out that the current behavior of allowing the > >> >> owner of a user namespace to have all caps when that owner is not in a > >> >> parent user namespace is wrong. > >> > > >> > To make sure I understand right, the issue is when a uid is mapped > >> > into multiple namespaces. > >> > >> Yes. > >> > >> i.e. uid 1000 in ns1 may own ns2, but uid 1000 in ns3 does not? > >> > >> I am not certain of your example. > >> > >> The simple case is: > >> > >> init_user_ns: > >> child_user_ns1 (owned by uid == 0 [in all user namespaces]) > >>child_user_ns2 (owned by uid == 0 [ in all user namespaces]) > >> > >> > >> root (uid == 0) in child_user_ns2 has all rights over anything in > >> child_user_ns1. > > > > Well that is only if there was no mapping. (since we're comparing > > kuids, not uid_ts). right? If you didn't map uid 0 in child_user_ns2 > > to another id in the parent ns, you weren't all *that* serious about > > isolating the ns. > > > > The case I was thinking is > > > > init_user_ns: [0-uidmax] > > child_user_ns1 [10-19] > > child_user_ns2 [10-19] > > child_user_ns3 [20-29] Wait is my example above possible? Or does child_user_ns3's range need to be a subset of child_user_ns2's? In which case it would be child_user_ns1 [10-19] child_user_ns2 [10-19] child_user_ns3 [12-12] > > with unfortunate mappings - ns1 and ns2 should have had nonoverlapping > > ranges, but in any case now uid 1000 in ns1 can exert privilege over > > ns3. Again, uids comparisons will succeed for file access anyway, so > > ns1 can 0wn ns2 and ns3 other ways. > > Yes yours is the more realistic scenario. Mine was simplified to be clear. > > > Heck I'm starting to think the bug is a feature - surely given the > > mappings above I meant for ns1 and ns2 to bleed privilege to each > > other? > > The serious problem is that privileges can bleed up. A user in > ns3 can wind up owning ns2 or ns1. Which totally defeats the permission > model. You have CAP_DAC_OVERRIDE so you don't even need access to files > you own, etc, etc. Would that not require intervention from the init_user_ns? In my example above (let's add that ns2 is owned by kuid.uid=1000 in init_user_ns), root in child_user_ns2 cannot map kuid.val=0 or kuid.val=1000 into ns3 because 0 and 1000 are not in the range 10-19. So there is no uid in child_user_ns3 which is able to spoof uid=0 in child_user_ns1. -serge -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
"Serge E. Hallyn" writes:
> Quoting Eric W. Biederman (ebied...@xmission.com):
>> "Serge E. Hallyn" writes:
>>
>> > Quoting Eric W. Biederman (ebied...@xmission.com):
>> >>
>> >> Andy Lutomirski pointed out that the current behavior of allowing the
>> >> owner of a user namespace to have all caps when that owner is not in a
>> >> parent user namespace is wrong.
>> >
>> > To make sure I understand right, the issue is when a uid is mapped
>> > into multiple namespaces.
>>
>> Yes.
>>
>> i.e. uid 1000 in ns1 may own ns2, but uid 1000 in ns3 does not?
>>
>> I am not certain of your example.
>>
>> The simple case is:
>>
>> init_user_ns:
>> child_user_ns1 (owned by uid == 0 [in all user namespaces])
>>child_user_ns2 (owned by uid == 0 [ in all user namespaces])
>>
>>
>> root (uid == 0) in child_user_ns2 has all rights over anything in
>> child_user_ns1.
>
> Well that is only if there was no mapping. (since we're comparing
> kuids, not uid_ts). right? If you didn't map uid 0 in child_user_ns2
> to another id in the parent ns, you weren't all *that* serious about
> isolating the ns.
>
> The case I was thinking is
>
> init_user_ns: [0-uidmax]
> child_user_ns1 [10-19]
> child_user_ns2 [10-19]
> child_user_ns3 [20-29]
>
> with unfortunate mappings - ns1 and ns2 should have had nonoverlapping
> ranges, but in any case now uid 1000 in ns1 can exert privilege over
> ns3. Again, uids comparisons will succeed for file access anyway, so
> ns1 can 0wn ns2 and ns3 other ways.
Yes yours is the more realistic scenario. Mine was simplified to be clear.
> Heck I'm starting to think the bug is a feature - surely given the
> mappings above I meant for ns1 and ns2 to bleed privilege to each
> other?
The serious problem is that privileges can bleed up. A user in
ns3 can wind up owning ns2 or ns1. Which totally defeats the permission
model. You have CAP_DAC_OVERRIDE so you don't even need access to files
you own, etc, etc.
Or the more fun version as root:
/* Drop all privs */
pid = clone(CLONE_NEWUSER);
if (pid == 0) {
/* Have all privs! Bahaha */
}
Which makes dropping capabilies a joke.
And since up to through 3.7 the only user that can create a user namespace
is root that is a very realistic scenario. It doesn't work against the
initial user namespace thankfully, but in v3.7 it works against every
other user namespace.
To keep user namespaces safe we need to maintain the invariant that a
child user namespace can not get capapabilities in it's parent user
namespace.
Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Eric W. Biederman (ebied...@xmission.com): > "Serge E. Hallyn" writes: > > > Quoting Eric W. Biederman (ebied...@xmission.com): > >> > >> Andy Lutomirski pointed out that the current behavior of allowing the > >> owner of a user namespace to have all caps when that owner is not in a > >> parent user namespace is wrong. > > > > To make sure I understand right, the issue is when a uid is mapped > > into multiple namespaces. > > Yes. > > i.e. uid 1000 in ns1 may own ns2, but uid 1000 in ns3 does not? > > I am not certain of your example. > > The simple case is: > > init_user_ns: > child_user_ns1 (owned by uid == 0 [in all user namespaces]) >child_user_ns2 (owned by uid == 0 [ in all user namespaces]) > > > root (uid == 0) in child_user_ns2 has all rights over anything in > child_user_ns1. Well that is only if there was no mapping. (since we're comparing kuids, not uid_ts). right? If you didn't map uid 0 in child_user_ns2 to another id in the parent ns, you weren't all *that* serious about isolating the ns. The case I was thinking is init_user_ns: [0-uidmax] child_user_ns1 [10-19] child_user_ns2 [10-19] child_user_ns3 [20-29] with unfortunate mappings - ns1 and ns2 should have had nonoverlapping ranges, but in any case now uid 1000 in ns1 can exert privilege over ns3. Again, uids comparisons will succeed for file access anyway, so ns1 can 0wn ns2 and ns3 other ways. Heck I'm starting to think the bug is a feature - surely given the mappings above I meant for ns1 and ns2 to bleed privilege to each other? > Thank you for looking. > > Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Eric W. Biederman (ebied...@xmission.com): Serge E. Hallyn se...@hallyn.com writes: Quoting Eric W. Biederman (ebied...@xmission.com): Andy Lutomirski pointed out that the current behavior of allowing the owner of a user namespace to have all caps when that owner is not in a parent user namespace is wrong. To make sure I understand right, the issue is when a uid is mapped into multiple namespaces. Yes. i.e. uid 1000 in ns1 may own ns2, but uid 1000 in ns3 does not? I am not certain of your example. The simple case is: init_user_ns: child_user_ns1 (owned by uid == 0 [in all user namespaces]) child_user_ns2 (owned by uid == 0 [ in all user namespaces]) root (uid == 0) in child_user_ns2 has all rights over anything in child_user_ns1. Well that is only if there was no mapping. (since we're comparing kuids, not uid_ts). right? If you didn't map uid 0 in child_user_ns2 to another id in the parent ns, you weren't all *that* serious about isolating the ns. The case I was thinking is init_user_ns: [0-uidmax] child_user_ns1 [10-19] child_user_ns2 [10-19] child_user_ns3 [20-29] with unfortunate mappings - ns1 and ns2 should have had nonoverlapping ranges, but in any case now uid 1000 in ns1 can exert privilege over ns3. Again, uids comparisons will succeed for file access anyway, so ns1 can 0wn ns2 and ns3 other ways. Heck I'm starting to think the bug is a feature - surely given the mappings above I meant for ns1 and ns2 to bleed privilege to each other? Thank you for looking. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Serge E. Hallyn se...@hallyn.com writes:
Quoting Eric W. Biederman (ebied...@xmission.com):
Serge E. Hallyn se...@hallyn.com writes:
Quoting Eric W. Biederman (ebied...@xmission.com):
Andy Lutomirski pointed out that the current behavior of allowing the
owner of a user namespace to have all caps when that owner is not in a
parent user namespace is wrong.
To make sure I understand right, the issue is when a uid is mapped
into multiple namespaces.
Yes.
i.e. uid 1000 in ns1 may own ns2, but uid 1000 in ns3 does not?
I am not certain of your example.
The simple case is:
init_user_ns:
child_user_ns1 (owned by uid == 0 [in all user namespaces])
child_user_ns2 (owned by uid == 0 [ in all user namespaces])
root (uid == 0) in child_user_ns2 has all rights over anything in
child_user_ns1.
Well that is only if there was no mapping. (since we're comparing
kuids, not uid_ts). right? If you didn't map uid 0 in child_user_ns2
to another id in the parent ns, you weren't all *that* serious about
isolating the ns.
The case I was thinking is
init_user_ns: [0-uidmax]
child_user_ns1 [10-19]
child_user_ns2 [10-19]
child_user_ns3 [20-29]
with unfortunate mappings - ns1 and ns2 should have had nonoverlapping
ranges, but in any case now uid 1000 in ns1 can exert privilege over
ns3. Again, uids comparisons will succeed for file access anyway, so
ns1 can 0wn ns2 and ns3 other ways.
Yes yours is the more realistic scenario. Mine was simplified to be clear.
Heck I'm starting to think the bug is a feature - surely given the
mappings above I meant for ns1 and ns2 to bleed privilege to each
other?
The serious problem is that privileges can bleed up. A user in
ns3 can wind up owning ns2 or ns1. Which totally defeats the permission
model. You have CAP_DAC_OVERRIDE so you don't even need access to files
you own, etc, etc.
Or the more fun version as root:
/* Drop all privs */
pid = clone(CLONE_NEWUSER);
if (pid == 0) {
/* Have all privs! Bahaha */
}
Which makes dropping capabilies a joke.
And since up to through 3.7 the only user that can create a user namespace
is root that is a very realistic scenario. It doesn't work against the
initial user namespace thankfully, but in v3.7 it works against every
other user namespace.
To keep user namespaces safe we need to maintain the invariant that a
child user namespace can not get capapabilities in it's parent user
namespace.
Eric
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Eric W. Biederman (ebied...@xmission.com): Serge E. Hallyn se...@hallyn.com writes: Quoting Eric W. Biederman (ebied...@xmission.com): Serge E. Hallyn se...@hallyn.com writes: Quoting Eric W. Biederman (ebied...@xmission.com): Andy Lutomirski pointed out that the current behavior of allowing the owner of a user namespace to have all caps when that owner is not in a parent user namespace is wrong. To make sure I understand right, the issue is when a uid is mapped into multiple namespaces. Yes. i.e. uid 1000 in ns1 may own ns2, but uid 1000 in ns3 does not? I am not certain of your example. The simple case is: init_user_ns: child_user_ns1 (owned by uid == 0 [in all user namespaces]) child_user_ns2 (owned by uid == 0 [ in all user namespaces]) root (uid == 0) in child_user_ns2 has all rights over anything in child_user_ns1. Well that is only if there was no mapping. (since we're comparing kuids, not uid_ts). right? If you didn't map uid 0 in child_user_ns2 to another id in the parent ns, you weren't all *that* serious about isolating the ns. The case I was thinking is init_user_ns: [0-uidmax] child_user_ns1 [10-19] child_user_ns2 [10-19] child_user_ns3 [20-29] Wait is my example above possible? Or does child_user_ns3's range need to be a subset of child_user_ns2's? In which case it would be child_user_ns1 [10-19] child_user_ns2 [10-19] child_user_ns3 [12-12] with unfortunate mappings - ns1 and ns2 should have had nonoverlapping ranges, but in any case now uid 1000 in ns1 can exert privilege over ns3. Again, uids comparisons will succeed for file access anyway, so ns1 can 0wn ns2 and ns3 other ways. Yes yours is the more realistic scenario. Mine was simplified to be clear. Heck I'm starting to think the bug is a feature - surely given the mappings above I meant for ns1 and ns2 to bleed privilege to each other? The serious problem is that privileges can bleed up. A user in ns3 can wind up owning ns2 or ns1. Which totally defeats the permission model. You have CAP_DAC_OVERRIDE so you don't even need access to files you own, etc, etc. Would that not require intervention from the init_user_ns? In my example above (let's add that ns2 is owned by kuid.uid=1000 in init_user_ns), root in child_user_ns2 cannot map kuid.val=0 or kuid.val=1000 into ns3 because 0 and 1000 are not in the range 10-19. So there is no uid in child_user_ns3 which is able to spoof uid=0 in child_user_ns1. -serge -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Serge E. Hallyn se...@hallyn.com writes: Quoting Eric W. Biederman (ebied...@xmission.com): Serge E. Hallyn se...@hallyn.com writes: Quoting Eric W. Biederman (ebied...@xmission.com): Serge E. Hallyn se...@hallyn.com writes: Quoting Eric W. Biederman (ebied...@xmission.com): Andy Lutomirski pointed out that the current behavior of allowing the owner of a user namespace to have all caps when that owner is not in a parent user namespace is wrong. To make sure I understand right, the issue is when a uid is mapped into multiple namespaces. Yes. i.e. uid 1000 in ns1 may own ns2, but uid 1000 in ns3 does not? I am not certain of your example. The simple case is: init_user_ns: child_user_ns1 (owned by uid == 0 [in all user namespaces]) child_user_ns2 (owned by uid == 0 [ in all user namespaces]) root (uid == 0) in child_user_ns2 has all rights over anything in child_user_ns1. Well that is only if there was no mapping. (since we're comparing kuids, not uid_ts). right? If you didn't map uid 0 in child_user_ns2 to another id in the parent ns, you weren't all *that* serious about isolating the ns. The case I was thinking is init_user_ns: [0-uidmax] child_user_ns1 [10-19] child_user_ns2 [10-19] child_user_ns3 [20-29] Wait is my example above possible? Or does child_user_ns3's range need to be a subset of child_user_ns2's? In which case it would be child_user_ns1 [10-19] child_user_ns2 [10-19] child_user_ns3 [12-12] Yes. You have to nest uids. with unfortunate mappings - ns1 and ns2 should have had nonoverlapping ranges, but in any case now uid 1000 in ns1 can exert privilege over ns3. Again, uids comparisons will succeed for file access anyway, so ns1 can 0wn ns2 and ns3 other ways. Yes yours is the more realistic scenario. Mine was simplified to be clear. Heck I'm starting to think the bug is a feature - surely given the mappings above I meant for ns1 and ns2 to bleed privilege to each other? The serious problem is that privileges can bleed up. A user in ns3 can wind up owning ns2 or ns1. Which totally defeats the permission model. You have CAP_DAC_OVERRIDE so you don't even need access to files you own, etc, etc. Would that not require intervention from the init_user_ns? In my example above (let's add that ns2 is owned by kuid.uid=1000 in init_user_ns), root in child_user_ns2 cannot map kuid.val=0 or kuid.val=1000 into ns3 because 0 and 1000 are not in the range 10-19. So there is no uid in child_user_ns3 which is able to spoof uid=0 in child_user_ns1. Right. It does require having the uid of the owner of ns1 or ns2 in ns3. So you have to explicitly allow it. What I don't see is any point in allowing something like that. After taking a second look I just realized that this is completely unexploitable with the code that is currently merged. As creating a grand child user namespace is competelely impossible. Creating a user namespace is requires capable(CAP_SYS_ADMIN) which is never present in anything but the initial user namespace. That said I think the current semantics of cap_capable are completely fatal to reasoning about user namespaces. A child user namespace having capabilities against processes in it's parent seems totally bizarre and pretty dangerous from a capabilities standpoint. That said Serge I think I have lost track of the point of your question. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
On Fri, Dec 14, 2012 at 10:12 AM, Eric W. Biederman ebied...@xmission.com wrote: That said Serge I think I have lost track of the point of your question. .. and I'm a bit unsure what I should do about this all. Including pulling the pull request that actually can make this all matter. Hmm? Any consensus? Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
On Fri, Dec 14, 2012 at 10:43 AM, Linus Torvalds torva...@linux-foundation.org wrote: On Fri, Dec 14, 2012 at 10:12 AM, Eric W. Biederman ebied...@xmission.com wrote: That said Serge I think I have lost track of the point of your question. .. and I'm a bit unsure what I should do about this all. Including pulling the pull request that actually can make this all matter. Hmm? Any consensus? I think that, if Eric submits a newer version that renames the loop variable for added comprehensibility, I'm okay with it. Changing the semantics to a more expansive version like Serge was talking about later on wouldn't break anything. But I don't think there's any reason to change it. --Andy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Eric W. Biederman (ebied...@xmission.com): Serge E. Hallyn se...@hallyn.com writes: Note: I acked your patch before and still don't object to it. In which case it would be child_user_ns1 [10-19] child_user_ns2 [10-19] child_user_ns3 [12-12] Yes. You have to nest uids. with unfortunate mappings - ns1 and ns2 should have had nonoverlapping ranges, but in any case now uid 1000 in ns1 can exert privilege over ns3. Again, uids comparisons will succeed for file access anyway, so ns1 can 0wn ns2 and ns3 other ways. Yes yours is the more realistic scenario. Mine was simplified to be clear. Heck I'm starting to think the bug is a feature - surely given the mappings above I meant for ns1 and ns2 to bleed privilege to each other? The serious problem is that privileges can bleed up. A user in ns3 can wind up owning ns2 or ns1. Which totally defeats the permission model. You have CAP_DAC_OVERRIDE so you don't even need access to files you own, etc, etc. Would that not require intervention from the init_user_ns? In my example above (let's add that ns2 is owned by kuid.uid=1000 in init_user_ns), root in child_user_ns2 cannot map kuid.val=0 or kuid.val=1000 into ns3 because 0 and 1000 are not in the range 10-19. So there is no uid in child_user_ns3 which is able to spoof uid=0 in child_user_ns1. Right. It does require having the uid of the owner of ns1 or ns2 in ns3. So you have to explicitly allow it. What I don't see is any point in allowing something like that. The point isn't specifically to allow that, but rather to not muddle the kuids with the user namespace *. If I clone two tasks in separate user namespace but give them the same uid mappings, how should the uids between those namespaces be related? The way you're doing it now they will equate for DAC checks but not for privilege checks. It feels ad-hoc and flaky. A child user namespace having capabilities against processes in it's parent seems totally bizarre and pretty dangerous from a capabilities standpoint. How would it have them against its parent? That said Serge I think I have lost track of the point of your question. Look at it this way. You've introduced kuids which allow uids to be disassociated from the user namespace * in kernel. You have the uid mapping rules which enforce some sanity (requiring nesting). If the parent ns has re-used an active uid of its own to give to a child namespace, then any DAC-governed objects (files in particular) will be owned by that uid in the child user ns. I'm not sure - does this also apply to /proc/$$/fd/* access? -serge -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Linus Torvalds (torva...@linux-foundation.org): On Fri, Dec 14, 2012 at 10:12 AM, Eric W. Biederman ebied...@xmission.com wrote: That said Serge I think I have lost track of the point of your question. .. and I'm a bit unsure what I should do about this all. Including pulling the pull request that actually can make this all matter. Sorry I didn't mean to complicate this. I did ack the patch and we can cull the cc list for continued discussion. In practical terms, the only thing the patch prevent is having two separate tasks each clone a new user ns with the same uid mapping, and having consistent relationships between the same uids between the namespaces. It's worth it to prevent (or while we consider) the case Andy and Eric bring up. Hmm? Any consensus? Acked-by: Serge Hallyn serge.hal...@canonical.com -serge -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Linus Torvalds torva...@linux-foundation.org writes: On Fri, Dec 14, 2012 at 10:12 AM, Eric W. Biederman ebied...@xmission.com wrote: That said Serge I think I have lost track of the point of your question. .. and I'm a bit unsure what I should do about this all. Including pulling the pull request that actually can make this all matter. Hmm? Any consensus? It looks like we have consensus (baring the color of the shed) of what the code should look like for v3.8. From the most embarrassingly timed, but most useful review by Andy I have 4 fixes queued up in my development tree. Fixing cap_capable to test for the parent namespace. Fixing setns to require nsown_capable(CAP_SYS_ADMIN) -- Fixing commit_creds to not clear task dumpable unnecessarily. Fixing a typo in the description. What I would like to do is to do is what I would if this was not the middle of the merge window with changes like this. Toss those patches out for last round of review. Possibly toss the last two patches if there are any problems because they are not necessary. Put the patches in my for-next branch and have them sit in linux-next for a day or three. Send you an updated pull request. I am recovering from a cold so I am running slower than I would like this week and would really rather not rush getting these patches out. What I don't want to be is so cautious and careful that you decide to pass on my pull request. The code is harmless with user namespaces disabled. The code has been baking for a long time, some of it for much too long and it is as solid as I think it will get out before being merged. Nor is the code complex Andy managed to dig and figure it all out in about a day. Linus does that work for you? Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Serge E. Hallyn se...@hallyn.com writes: Quoting Eric W. Biederman (ebied...@xmission.com): A child user namespace having capabilities against processes in it's parent seems totally bizarre and pretty dangerous from a capabilities standpoint. How would it have them against its parent? init_user_ns userns a --- created by kuid 1 userns b -- created by kuid 2 process c in userns b with kuid 1 Serge read the first permisison check in common_cap. Think what happens in the above example. For the rest I understand your concern. Serge please read and look at the patches I have posted to fix the issues Andy found with the user namespace tree. Especially the fix to commit_creds. After you have looked at the patches to fix the issues I will be happy to discuss things further with you. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Eric W. Biederman (ebied...@xmission.com): Serge E. Hallyn se...@hallyn.com writes: Quoting Eric W. Biederman (ebied...@xmission.com): A child user namespace having capabilities against processes in it's parent seems totally bizarre and pretty dangerous from a capabilities standpoint. How would it have them against its parent? init_user_ns userns a --- created by kuid 1 Now a mapping needs to be set up (by a task with CAP_SYS_ADMIN in init_user_ns) which allows kuids 1 and 2 to be used by userns a. Otherwise (if no mapping is set up) userns a only has the overlowuid. Realistically only kuids over 10 (let's say) would used. i.e. kuids 100,000-199,999 would map to container uids 0-99,999. userns b -- created by kuid 2 Now a mapping needs to be set up (by a task with CAP_SYS_ADMIN in userns a) which allows kuids 1 and 2 to be used by userns b. If userns had been mapped with kuids 100,000-199,999 mapping to uids 0-9, then only kuids in that range could be mapped into userns b. process c in userns b with kuid 1 Serge read the first permisison check in common_cap. Think what happens in the above example. For the rest I understand your concern. Ok. Then we can discuss my concern later (after the new year). Serge please read and look at the patches I have posted to fix the issues Andy found with the user namespace tree. Especially the fix to commit_creds. The setns fixes were IMO the most important - and interesting - ones :) Thanks, Andy! After you have looked at the patches to fix the issues I will be happy to discuss things further with you. Thanks, -serge -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
"Serge E. Hallyn" writes: > Quoting Eric W. Biederman (ebied...@xmission.com): >> >> Andy Lutomirski pointed out that the current behavior of allowing the >> owner of a user namespace to have all caps when that owner is not in a >> parent user namespace is wrong. > > To make sure I understand right, the issue is when a uid is mapped > into multiple namespaces. Yes. i.e. uid 1000 in ns1 may own ns2, but uid 1000 in ns3 does not? I am not certain of your example. The simple case is: init_user_ns: child_user_ns1 (owned by uid == 0 [in all user namespaces]) child_user_ns2 (owned by uid == 0 [ in all user namespaces]) root (uid == 0) in child_user_ns2 has all rights over anything in child_user_ns1. Thank you for looking. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Eric W. Biederman (ebied...@xmission.com):
>
> Andy Lutomirski pointed out that the current behavior of allowing the
> owner of a user namespace to have all caps when that owner is not in a
> parent user namespace is wrong.
To make sure I understand right, the issue is when a uid is mapped
into multiple namespaces, i.e. uid 1000 in ns1 may own ns2, but uid
1000 in ns3 does not?
> This is a bug introduced by the kuid conversion which made it possible
> for the owner of a user namespace to live in a child user namespace. I
> goofed and totally missed this implication.
>
> Serge and can you please take a look and see if my corrected cap_capable
> reads correctly to you.
>
> Andy or anyone else that wants to give me a second eyeball and double
> check me on this I would appreciate it.
>
> Signed-off-by: "Eric W. Biederman"
Acked-by: Serge Hallyn
> ---
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 6dbae46..4639f44 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -70,37 +70,44 @@ int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
> *
> * NOTE WELL: cap_has_capability() cannot be used like the kernel's capable()
> * and has_capability() functions. That is, it has the reverse semantics:
> * cap_has_capability() returns 0 when a task has a capability, but the
> * kernel's capable() and has_capability() returns 1 for this case.
> */
> int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
> int cap, int audit)
> {
> for (;;) {
> - /* The owner of the user namespace has all caps. */
> - if (targ_ns != _user_ns && uid_eq(targ_ns->owner,
> cred->euid))
> - return 0;
> + struct user_namespace *parent_ns;
>
> /* Do we have the necessary capabilities? */
> if (targ_ns == cred->user_ns)
> return cap_raised(cred->cap_effective, cap) ? 0 :
> -EPERM;
>
> /* Have we tried all of the parent namespaces? */
> if (targ_ns == _user_ns)
> return -EPERM;
>
> + parent_ns = targ_ns->parent;
> +
> + /*
> + * The owner of the user namespace in the parent user
> + * namespace has all caps.
> + */
> + if ((parent_ns == cred->user_ns) && uid_eq(targ_ns->owner,
> cred->euid))
> + return 0;
> +
> /*
> - *If you have a capability in a parent user ns, then you have
> + * If you have a capability in a parent user ns, then you have
>* it over all children user namespaces as well.
>*/
> - targ_ns = targ_ns->parent;
> + targ_ns = parent_ns;
> }
>
> /* We never get here */
> }
>
> /**
> * cap_settime - Determine whether the current process may set the system
> clock
> * @ts: The time to set
> * @tz: The timezone to set
> *
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
On Thu, Dec 13, 2012 at 6:33 PM, Eric W. Biederman wrote: > > Andy thank you for your review. > > Andy Lutomirski writes: >> This is confusing enough that I can't immediately tell whether it's >> correct. I think it's close but out of order. > > Yeah. That is the trick. Figuring out how to write that code so it is > correct and obvious. > > I have added a comment at the top and removed the extra variable I was > adding. > > The order except for verifying a user_ns->parent is valid by checking > for targ_ns == _user_ns doesn't make a difference. Because > cred->user_ns can only be one of targ_ns or targ_ns->parent. > >> Should this be transitive? > > Yes. >> I.e. suppose uid 1 owns a child of >> init_user_ns and uid 2 (mapped in the first ns as the identity) owns >> an inner ns. Does uid 2 in the root ns have all caps inside? I'd say >> no, but I don't have a great argument for that. > > I also say no. It is more code and it doesn't fit my nice small > definition. You have to be the owner and you have to be in the parent > of the target user namespace. Being able to remember the rules in > your head is important. > >> But uid 1 presumably >> does have caps because it could enter the parent with setns, then >> change uid, then enter the child. > > Yes. uid 1 does have caps. > >> How about (severely whitespace damaged): > > You know that makes the termination condition a bit clearer. But it > looses the nice place to put a comment when we loop again. This loop > is just subtle enough that I want to preserve my comments. > > I think I must have put -EPERM towards the end for the same reason to > make it clear that is the termination condition. > > In practice I think it is important to have the cap_raised case first, > as that is the common case, and if we can be clear and still test that > case first it means the code will be faster. With my reordering it is > obvious that nothing strange happens in the initial user namespace with > the owner test after the exit when we are the initial user namespace. Ah. You are correct about the ordering. I read it slightly wrong. I'd still suggest using a variable like "here" instead of "targ_ns". The latter is confusing because it changes on the second and later iterations. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Andy thank you for your review.
Andy Lutomirski writes:
> This is confusing enough that I can't immediately tell whether it's
> correct. I think it's close but out of order.
Yeah. That is the trick. Figuring out how to write that code so it is
correct and obvious.
I have added a comment at the top and removed the extra variable I was
adding.
The order except for verifying a user_ns->parent is valid by checking
for targ_ns == _user_ns doesn't make a difference. Because
cred->user_ns can only be one of targ_ns or targ_ns->parent.
> Should this be transitive?
Yes.
> I.e. suppose uid 1 owns a child of
> init_user_ns and uid 2 (mapped in the first ns as the identity) owns
> an inner ns. Does uid 2 in the root ns have all caps inside? I'd say
> no, but I don't have a great argument for that.
I also say no. It is more code and it doesn't fit my nice small
definition. You have to be the owner and you have to be in the parent
of the target user namespace. Being able to remember the rules in
your head is important.
> But uid 1 presumably
> does have caps because it could enter the parent with setns, then
> change uid, then enter the child.
Yes. uid 1 does have caps.
> How about (severely whitespace damaged):
You know that makes the termination condition a bit clearer. But it
looses the nice place to put a comment when we loop again. This loop
is just subtle enough that I want to preserve my comments.
I think I must have put -EPERM towards the end for the same reason to
make it clear that is the termination condition.
In practice I think it is important to have the cap_raised case first,
as that is the common case, and if we can be clear and still test that
case first it means the code will be faster. With my reordering it is
obvious that nothing strange happens in the initial user namespace with
the owner test after the exit when we are the initial user namespace.
> int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
> int cap, int audit)
> {
> struct user_namespace *here = targ_ns;
>
> /* Walk up the namespace hierarchy until we find our own namespace. */
> for (;;) {
> /* The owner of an ancestor namespace has all caps, if
> that owner is in the parent ns. */
> if (cred->user_ns == here->parent &&
> uid_eq(targ_ns->owner, cred->euid))
> return 0;
This would have needed a check that (here != _user_ns). Because
the init_user_ns does not have a parent.
> /* Do we have the necessary capabilities? */
> if (here == cred->user_ns)
> return cap_raised(cred->cap_effective, cap) ?
> 0 : -EPERM;
>
> /* Have we tried all of the parent namespaces? */
> if (here == _user_ns)
> return -EPERM;
> else
> here = targ_ns->parent;
> }
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
On Thu, Dec 13, 2012 at 2:39 PM, Eric W. Biederman
wrote:
>
> Andy Lutomirski pointed out that the current behavior of allowing the
> owner of a user namespace to have all caps when that owner is not in a
> parent user namespace is wrong.
>
> This is a bug introduced by the kuid conversion which made it possible
> for the owner of a user namespace to live in a child user namespace. I
> goofed and totally missed this implication.
>
> Serge and can you please take a look and see if my corrected cap_capable
> reads correctly to you.
>
> Andy or anyone else that wants to give me a second eyeball and double
> check me on this I would appreciate it.
>
> Signed-off-by: "Eric W. Biederman"
>
> ---
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 6dbae46..4639f44 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -70,37 +70,44 @@ int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
> *
> * NOTE WELL: cap_has_capability() cannot be used like the kernel's capable()
> * and has_capability() functions. That is, it has the reverse semantics:
> * cap_has_capability() returns 0 when a task has a capability, but the
> * kernel's capable() and has_capability() returns 1 for this case.
> */
> int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
> int cap, int audit)
> {
> for (;;) {
> - /* The owner of the user namespace has all caps. */
> - if (targ_ns != _user_ns && uid_eq(targ_ns->owner,
> cred->euid))
> - return 0;
> + struct user_namespace *parent_ns;
>
> /* Do we have the necessary capabilities? */
> if (targ_ns == cred->user_ns)
> return cap_raised(cred->cap_effective, cap) ? 0 :
> -EPERM;
>
> /* Have we tried all of the parent namespaces? */
> if (targ_ns == _user_ns)
> return -EPERM;
>
> + parent_ns = targ_ns->parent;
> +
> + /*
> +* The owner of the user namespace in the parent user
> +* namespace has all caps.
> +*/
> + if ((parent_ns == cred->user_ns) && uid_eq(targ_ns->owner,
> cred->euid))
> + return 0;
This is confusing enough that I can't immediately tell whether it's
correct. I think it's close but out of order.
Should this be transitive? I.e. suppose uid 1 owns a child of
init_user_ns and uid 2 (mapped in the first ns as the identity) owns
an inner ns. Does uid 2 in the root ns have all caps inside? I'd say
no, but I don't have a great argument for that. But uid 1 presumably
does have caps because it could enter the parent with setns, then
change uid, then enter the child.
How about (severely whitespace damaged):
int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
int cap, int audit)
{
struct user_namespace *here = targ_ns;
/* Walk up the namespace hierarchy until we find our own namespace. */
for (;;) {
/* The owner of an ancestor namespace has all caps, if
that owner is in the parent ns. */
if (cred->user_ns == here->parent &&
uid_eq(targ_ns->owner, cred->euid))
return 0;
/* Do we have the necessary capabilities? */
if (here == cred->user_ns)
return cap_raised(cred->cap_effective, cap) ?
0 : -EPERM;
/* Have we tried all of the parent namespaces? */
if (here == _user_ns)
return -EPERM;
else
here = targ_ns->parent;
}
}
--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Linus Torvalds writes: > On Thu, Dec 13, 2012 at 2:39 PM, Eric W. Biederman > wrote: >> >> Andy Lutomirski pointed out that the current behavior of allowing the >> owner of a user namespace to have all caps when that owner is not in a >> parent user namespace is wrong. >> >> This is a bug introduced by the kuid conversion which made it possible >> for the owner of a user namespace to live in a child user namespace. I >> goofed and totally missed this implication. > > Hmm. Shouldn't this be cc: stable if it was introduced in the kuid > conversion? Or is it only an issue with your new namespace tree (which > I haven't pulled yet)? It should be CC stable. I think I have fixed the bug I am hoping to get a second pair of eyeballs before I send the patch officially. The test for _user_ns keeps the bugs from affecting kernels with user namespaces disabled. The bug exists in 3.5 and 3.6 but barely matters because you can't enable user namespaces without additional patches. The bug exists in 3.7 but is should be of limited affect because distributions are likely to prefer enabling nfs and fuse over user namespaces. I am going to step away for about an hour or so and then with hopefully fresh eyes myself work to push the good version. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
On Thu, Dec 13, 2012 at 2:39 PM, Eric W. Biederman wrote: > > Andy Lutomirski pointed out that the current behavior of allowing the > owner of a user namespace to have all caps when that owner is not in a > parent user namespace is wrong. > > This is a bug introduced by the kuid conversion which made it possible > for the owner of a user namespace to live in a child user namespace. I > goofed and totally missed this implication. Hmm. Shouldn't this be cc: stable if it was introduced in the kuid conversion? Or is it only an issue with your new namespace tree (which I haven't pulled yet)? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Andy Lutomirski pointed out that the current behavior of allowing the
owner of a user namespace to have all caps when that owner is not in a
parent user namespace is wrong.
This is a bug introduced by the kuid conversion which made it possible
for the owner of a user namespace to live in a child user namespace. I
goofed and totally missed this implication.
Serge and can you please take a look and see if my corrected cap_capable
reads correctly to you.
Andy or anyone else that wants to give me a second eyeball and double
check me on this I would appreciate it.
Signed-off-by: "Eric W. Biederman"
---
diff --git a/security/commoncap.c b/security/commoncap.c
index 6dbae46..4639f44 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -70,37 +70,44 @@ int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
*
* NOTE WELL: cap_has_capability() cannot be used like the kernel's capable()
* and has_capability() functions. That is, it has the reverse semantics:
* cap_has_capability() returns 0 when a task has a capability, but the
* kernel's capable() and has_capability() returns 1 for this case.
*/
int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
int cap, int audit)
{
for (;;) {
- /* The owner of the user namespace has all caps. */
- if (targ_ns != _user_ns && uid_eq(targ_ns->owner,
cred->euid))
- return 0;
+ struct user_namespace *parent_ns;
/* Do we have the necessary capabilities? */
if (targ_ns == cred->user_ns)
return cap_raised(cred->cap_effective, cap) ? 0 :
-EPERM;
/* Have we tried all of the parent namespaces? */
if (targ_ns == _user_ns)
return -EPERM;
+ parent_ns = targ_ns->parent;
+
+ /*
+* The owner of the user namespace in the parent user
+* namespace has all caps.
+*/
+ if ((parent_ns == cred->user_ns) && uid_eq(targ_ns->owner,
cred->euid))
+ return 0;
+
/*
-*If you have a capability in a parent user ns, then you have
+* If you have a capability in a parent user ns, then you have
* it over all children user namespaces as well.
*/
- targ_ns = targ_ns->parent;
+ targ_ns = parent_ns;
}
/* We never get here */
}
/**
* cap_settime - Determine whether the current process may set the system clock
* @ts: The time to set
* @tz: The timezone to set
*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Andy Lutomirski pointed out that the current behavior of allowing the
owner of a user namespace to have all caps when that owner is not in a
parent user namespace is wrong.
This is a bug introduced by the kuid conversion which made it possible
for the owner of a user namespace to live in a child user namespace. I
goofed and totally missed this implication.
Serge and can you please take a look and see if my corrected cap_capable
reads correctly to you.
Andy or anyone else that wants to give me a second eyeball and double
check me on this I would appreciate it.
Signed-off-by: Eric W. Biederman ebied...@xmission.com
---
diff --git a/security/commoncap.c b/security/commoncap.c
index 6dbae46..4639f44 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -70,37 +70,44 @@ int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
*
* NOTE WELL: cap_has_capability() cannot be used like the kernel's capable()
* and has_capability() functions. That is, it has the reverse semantics:
* cap_has_capability() returns 0 when a task has a capability, but the
* kernel's capable() and has_capability() returns 1 for this case.
*/
int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
int cap, int audit)
{
for (;;) {
- /* The owner of the user namespace has all caps. */
- if (targ_ns != init_user_ns uid_eq(targ_ns-owner,
cred-euid))
- return 0;
+ struct user_namespace *parent_ns;
/* Do we have the necessary capabilities? */
if (targ_ns == cred-user_ns)
return cap_raised(cred-cap_effective, cap) ? 0 :
-EPERM;
/* Have we tried all of the parent namespaces? */
if (targ_ns == init_user_ns)
return -EPERM;
+ parent_ns = targ_ns-parent;
+
+ /*
+* The owner of the user namespace in the parent user
+* namespace has all caps.
+*/
+ if ((parent_ns == cred-user_ns) uid_eq(targ_ns-owner,
cred-euid))
+ return 0;
+
/*
-*If you have a capability in a parent user ns, then you have
+* If you have a capability in a parent user ns, then you have
* it over all children user namespaces as well.
*/
- targ_ns = targ_ns-parent;
+ targ_ns = parent_ns;
}
/* We never get here */
}
/**
* cap_settime - Determine whether the current process may set the system clock
* @ts: The time to set
* @tz: The timezone to set
*
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
On Thu, Dec 13, 2012 at 2:39 PM, Eric W. Biederman ebied...@xmission.com wrote: Andy Lutomirski pointed out that the current behavior of allowing the owner of a user namespace to have all caps when that owner is not in a parent user namespace is wrong. This is a bug introduced by the kuid conversion which made it possible for the owner of a user namespace to live in a child user namespace. I goofed and totally missed this implication. Hmm. Shouldn't this be cc: stable if it was introduced in the kuid conversion? Or is it only an issue with your new namespace tree (which I haven't pulled yet)? Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Linus Torvalds torva...@linux-foundation.org writes: On Thu, Dec 13, 2012 at 2:39 PM, Eric W. Biederman ebied...@xmission.com wrote: Andy Lutomirski pointed out that the current behavior of allowing the owner of a user namespace to have all caps when that owner is not in a parent user namespace is wrong. This is a bug introduced by the kuid conversion which made it possible for the owner of a user namespace to live in a child user namespace. I goofed and totally missed this implication. Hmm. Shouldn't this be cc: stable if it was introduced in the kuid conversion? Or is it only an issue with your new namespace tree (which I haven't pulled yet)? It should be CC stable. I think I have fixed the bug I am hoping to get a second pair of eyeballs before I send the patch officially. The test for init_user_ns keeps the bugs from affecting kernels with user namespaces disabled. The bug exists in 3.5 and 3.6 but barely matters because you can't enable user namespaces without additional patches. The bug exists in 3.7 but is should be of limited affect because distributions are likely to prefer enabling nfs and fuse over user namespaces. I am going to step away for about an hour or so and then with hopefully fresh eyes myself work to push the good version. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
On Thu, Dec 13, 2012 at 2:39 PM, Eric W. Biederman
ebied...@xmission.com wrote:
Andy Lutomirski pointed out that the current behavior of allowing the
owner of a user namespace to have all caps when that owner is not in a
parent user namespace is wrong.
This is a bug introduced by the kuid conversion which made it possible
for the owner of a user namespace to live in a child user namespace. I
goofed and totally missed this implication.
Serge and can you please take a look and see if my corrected cap_capable
reads correctly to you.
Andy or anyone else that wants to give me a second eyeball and double
check me on this I would appreciate it.
Signed-off-by: Eric W. Biederman ebied...@xmission.com
---
diff --git a/security/commoncap.c b/security/commoncap.c
index 6dbae46..4639f44 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -70,37 +70,44 @@ int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
*
* NOTE WELL: cap_has_capability() cannot be used like the kernel's capable()
* and has_capability() functions. That is, it has the reverse semantics:
* cap_has_capability() returns 0 when a task has a capability, but the
* kernel's capable() and has_capability() returns 1 for this case.
*/
int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
int cap, int audit)
{
for (;;) {
- /* The owner of the user namespace has all caps. */
- if (targ_ns != init_user_ns uid_eq(targ_ns-owner,
cred-euid))
- return 0;
+ struct user_namespace *parent_ns;
/* Do we have the necessary capabilities? */
if (targ_ns == cred-user_ns)
return cap_raised(cred-cap_effective, cap) ? 0 :
-EPERM;
/* Have we tried all of the parent namespaces? */
if (targ_ns == init_user_ns)
return -EPERM;
+ parent_ns = targ_ns-parent;
+
+ /*
+* The owner of the user namespace in the parent user
+* namespace has all caps.
+*/
+ if ((parent_ns == cred-user_ns) uid_eq(targ_ns-owner,
cred-euid))
+ return 0;
This is confusing enough that I can't immediately tell whether it's
correct. I think it's close but out of order.
Should this be transitive? I.e. suppose uid 1 owns a child of
init_user_ns and uid 2 (mapped in the first ns as the identity) owns
an inner ns. Does uid 2 in the root ns have all caps inside? I'd say
no, but I don't have a great argument for that. But uid 1 presumably
does have caps because it could enter the parent with setns, then
change uid, then enter the child.
How about (severely whitespace damaged):
int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
int cap, int audit)
{
struct user_namespace *here = targ_ns;
/* Walk up the namespace hierarchy until we find our own namespace. */
for (;;) {
/* The owner of an ancestor namespace has all caps, if
that owner is in the parent ns. */
if (cred-user_ns == here-parent
uid_eq(targ_ns-owner, cred-euid))
return 0;
/* Do we have the necessary capabilities? */
if (here == cred-user_ns)
return cap_raised(cred-cap_effective, cap) ?
0 : -EPERM;
/* Have we tried all of the parent namespaces? */
if (here == init_user_ns)
return -EPERM;
else
here = targ_ns-parent;
}
}
--Andy
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Andy thank you for your review.
Andy Lutomirski l...@amacapital.net writes:
This is confusing enough that I can't immediately tell whether it's
correct. I think it's close but out of order.
Yeah. That is the trick. Figuring out how to write that code so it is
correct and obvious.
I have added a comment at the top and removed the extra variable I was
adding.
The order except for verifying a user_ns-parent is valid by checking
for targ_ns == init_user_ns doesn't make a difference. Because
cred-user_ns can only be one of targ_ns or targ_ns-parent.
Should this be transitive?
Yes.
I.e. suppose uid 1 owns a child of
init_user_ns and uid 2 (mapped in the first ns as the identity) owns
an inner ns. Does uid 2 in the root ns have all caps inside? I'd say
no, but I don't have a great argument for that.
I also say no. It is more code and it doesn't fit my nice small
definition. You have to be the owner and you have to be in the parent
of the target user namespace. Being able to remember the rules in
your head is important.
But uid 1 presumably
does have caps because it could enter the parent with setns, then
change uid, then enter the child.
Yes. uid 1 does have caps.
How about (severely whitespace damaged):
You know that makes the termination condition a bit clearer. But it
looses the nice place to put a comment when we loop again. This loop
is just subtle enough that I want to preserve my comments.
I think I must have put -EPERM towards the end for the same reason to
make it clear that is the termination condition.
In practice I think it is important to have the cap_raised case first,
as that is the common case, and if we can be clear and still test that
case first it means the code will be faster. With my reordering it is
obvious that nothing strange happens in the initial user namespace with
the owner test after the exit when we are the initial user namespace.
int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
int cap, int audit)
{
struct user_namespace *here = targ_ns;
/* Walk up the namespace hierarchy until we find our own namespace. */
for (;;) {
/* The owner of an ancestor namespace has all caps, if
that owner is in the parent ns. */
if (cred-user_ns == here-parent
uid_eq(targ_ns-owner, cred-euid))
return 0;
This would have needed a check that (here != init_user_ns). Because
the init_user_ns does not have a parent.
/* Do we have the necessary capabilities? */
if (here == cred-user_ns)
return cap_raised(cred-cap_effective, cap) ?
0 : -EPERM;
/* Have we tried all of the parent namespaces? */
if (here == init_user_ns)
return -EPERM;
else
here = targ_ns-parent;
}
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
On Thu, Dec 13, 2012 at 6:33 PM, Eric W. Biederman ebied...@xmission.com wrote: Andy thank you for your review. Andy Lutomirski l...@amacapital.net writes: This is confusing enough that I can't immediately tell whether it's correct. I think it's close but out of order. Yeah. That is the trick. Figuring out how to write that code so it is correct and obvious. I have added a comment at the top and removed the extra variable I was adding. The order except for verifying a user_ns-parent is valid by checking for targ_ns == init_user_ns doesn't make a difference. Because cred-user_ns can only be one of targ_ns or targ_ns-parent. Should this be transitive? Yes. I.e. suppose uid 1 owns a child of init_user_ns and uid 2 (mapped in the first ns as the identity) owns an inner ns. Does uid 2 in the root ns have all caps inside? I'd say no, but I don't have a great argument for that. I also say no. It is more code and it doesn't fit my nice small definition. You have to be the owner and you have to be in the parent of the target user namespace. Being able to remember the rules in your head is important. But uid 1 presumably does have caps because it could enter the parent with setns, then change uid, then enter the child. Yes. uid 1 does have caps. How about (severely whitespace damaged): You know that makes the termination condition a bit clearer. But it looses the nice place to put a comment when we loop again. This loop is just subtle enough that I want to preserve my comments. I think I must have put -EPERM towards the end for the same reason to make it clear that is the termination condition. In practice I think it is important to have the cap_raised case first, as that is the common case, and if we can be clear and still test that case first it means the code will be faster. With my reordering it is obvious that nothing strange happens in the initial user namespace with the owner test after the exit when we are the initial user namespace. Ah. You are correct about the ordering. I read it slightly wrong. I'd still suggest using a variable like here instead of targ_ns. The latter is confusing because it changes on the second and later iterations. --Andy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Quoting Eric W. Biederman (ebied...@xmission.com):
Andy Lutomirski pointed out that the current behavior of allowing the
owner of a user namespace to have all caps when that owner is not in a
parent user namespace is wrong.
To make sure I understand right, the issue is when a uid is mapped
into multiple namespaces, i.e. uid 1000 in ns1 may own ns2, but uid
1000 in ns3 does not?
This is a bug introduced by the kuid conversion which made it possible
for the owner of a user namespace to live in a child user namespace. I
goofed and totally missed this implication.
Serge and can you please take a look and see if my corrected cap_capable
reads correctly to you.
Andy or anyone else that wants to give me a second eyeball and double
check me on this I would appreciate it.
Signed-off-by: Eric W. Biederman ebied...@xmission.com
Acked-by: Serge Hallyn serge.hal...@canonical.com
---
diff --git a/security/commoncap.c b/security/commoncap.c
index 6dbae46..4639f44 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -70,37 +70,44 @@ int cap_netlink_send(struct sock *sk, struct sk_buff *skb)
*
* NOTE WELL: cap_has_capability() cannot be used like the kernel's capable()
* and has_capability() functions. That is, it has the reverse semantics:
* cap_has_capability() returns 0 when a task has a capability, but the
* kernel's capable() and has_capability() returns 1 for this case.
*/
int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
int cap, int audit)
{
for (;;) {
- /* The owner of the user namespace has all caps. */
- if (targ_ns != init_user_ns uid_eq(targ_ns-owner,
cred-euid))
- return 0;
+ struct user_namespace *parent_ns;
/* Do we have the necessary capabilities? */
if (targ_ns == cred-user_ns)
return cap_raised(cred-cap_effective, cap) ? 0 :
-EPERM;
/* Have we tried all of the parent namespaces? */
if (targ_ns == init_user_ns)
return -EPERM;
+ parent_ns = targ_ns-parent;
+
+ /*
+ * The owner of the user namespace in the parent user
+ * namespace has all caps.
+ */
+ if ((parent_ns == cred-user_ns) uid_eq(targ_ns-owner,
cred-euid))
+ return 0;
+
/*
- *If you have a capability in a parent user ns, then you have
+ * If you have a capability in a parent user ns, then you have
* it over all children user namespaces as well.
*/
- targ_ns = targ_ns-parent;
+ targ_ns = parent_ns;
}
/* We never get here */
}
/**
* cap_settime - Determine whether the current process may set the system
clock
* @ts: The time to set
* @tz: The timezone to set
*
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] Fix cap_capable to only allow owners in the parent user namespace to have caps.
Serge E. Hallyn se...@hallyn.com writes: Quoting Eric W. Biederman (ebied...@xmission.com): Andy Lutomirski pointed out that the current behavior of allowing the owner of a user namespace to have all caps when that owner is not in a parent user namespace is wrong. To make sure I understand right, the issue is when a uid is mapped into multiple namespaces. Yes. i.e. uid 1000 in ns1 may own ns2, but uid 1000 in ns3 does not? I am not certain of your example. The simple case is: init_user_ns: child_user_ns1 (owned by uid == 0 [in all user namespaces]) child_user_ns2 (owned by uid == 0 [ in all user namespaces]) root (uid == 0) in child_user_ns2 has all rights over anything in child_user_ns1. Thank you for looking. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
