Message-ID: <CALCETrUhd41LFfF9epbVYJSOwqBq308Z8RZG9tzyPfx+Joe15Q@mail.gmail.com>
Date:	Mon, 21 Jul 2014 15:16:20 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Aditya Kali <adityakali@...gle.com>
Cc:	Linux Containers <containers@...ts.linux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	cgroups@...r.kernel.org, Li Zefan <lizefan@...wei.com>,
	Linux API <linux-api@...r.kernel.org>,
	Tejun Heo <tj@...nel.org>, Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH 5/5] cgroup: introduce cgroup namespaces

On Mon, Jul 21, 2014 at 3:11 PM, Aditya Kali <adityakali@...gle.com> wrote:
> On Fri, Jul 18, 2014 at 11:57 AM, Andy Lutomirski <luto@...capital.net> wrote:
>> On Fri, Jul 18, 2014 at 11:51 AM, Aditya Kali <adityakali@...gle.com> wrote:
>>> On Fri, Jul 18, 2014 at 9:51 AM, Andy Lutomirski <luto@...capital.net> wrote:
>>>> On Jul 17, 2014 1:56 PM, "Aditya Kali" <adityakali@...gle.com> wrote:
>>>>>
>>>>> On Thu, Jul 17, 2014 at 12:57 PM, Andy Lutomirski <luto@...capital.net> wrote:
>>>>> > What happens if someone moves a task in a cgroup namespace outside of
>>>>> > the namespace root cgroup?
>>>>> >
>>>>>
>>>>> Attempt to move a task outside of cgroupns root will fail with EPERM.
>>>>> This is true irrespective of the privileges of the process attempting
>>>>> this. Once cgroupns is created, the task will be confined to the
>>>>> cgroup hierarchy under its cgroupns root until it dies.
>>>>
>>>> Can a task in a non-init userns create a cgroupns?  If not, that's
>>>> unusual.  If so, is it problematic if they can prevent themselves from
>>>> being moved?
>>>>
>>>
>>> Currently, only a task with CAP_SYS_ADMIN in the init-userns can
>>> create cgroupns. It is stricter than for other namespaces, yes.
>>
>> I'm slightly hesitant to have unshare(CLONE_NEWUSER |
>> CLONE_NEWCGROUPNS | ...) start having weird side effects that are
>> visible outside the namespace, especially when those side effects
>> don't happen (because the call fails entirely) if
>> unshare(CLONE_NEWUSER) happens first.  I don't see a real problem with
>> it, but it's weird.
>>
>
> I expect this to be only in the initial version of the patch. We can
> make this consistent with other namespaces once we figure out how
> cgroupns can be safely enabled for non-init-userns.
>
>>>
>>>> I hate to say it, but it might be worth requiring explicit permission
>>>> from the cgroup manager for this.  For example, there could be a new
>>>> cgroup attribute may_unshare, and any attempt to unshare the cgroup ns
>>>> will fail with -EPERM unless the caller is in a may_share=1 cgroup.
>>>> may_unshare in a parent cgroup would not give child cgroups the
>>>> ability to unshare.
>>>>
>>>
>>> What you suggest can be done. The current patch-set punts the problem
>>> of permission checking by only allowing unshare from a
>>> capable(CAP_SYS_ADMIN) process. This can be implemented as a follow-up
>>> improvement to cgroupns feature if we want to open it to non-init
>>> userns.
>>>
>>> Being said that, I would argue that even if we don't have this
>>> explicit permission and relax the check to non-init userns, it should
>>> be 'OK' to let ns_capable(current_user_ns(), CAP_SYS_ADMIN) tasks to
>>> unshare cgroupns (basically, if you can "create" a cgroup hierarchy,
>>> you should probably be allowed to unshare() it).
>>
>> But non-init-userns tasks can't create cgroup hierarchies, unless I
>> misunderstand the current code.  And, if they can, I bet I can find
>> three or four serious security issues in an hour or two. :)
>>
>
> Task running in non-init userns can create cgroup hierarchies if you
> chown/chgrp their cgroup root to the task user:

Won't the systemd people hate you forever for this suggestion?  (I do
exactly this myself...)


> This is a powerful feature as it allows non-root tasks to run
> container-management tools and provision their resources properly. But
> this makes implementing your suggestion of having 'cgroup.may_unshare'
> file tricky as the cgroup owner (task) will be able to set it and
> still unshare cgroupns. Instead, may be we could just check if the
> task has appropriate (write?) permissions on the cgroup directory
> before allowing nested cgroupns creation.

I bet that systemd will want to set may_unshare but not give write
access.  Who knows?

> [shudder]
> I am surprised that this even works correctly.
>
> Either way, may be checking cgroup directory permissions will work for
> you? i.e., if you "chown" a cgroup directory to the user, it should be
> OK if the user's task unshares cgroupns under that cgroup and you
> don't care about moving tasks from under that cgroup. Without
> ownership of the cgroup directory, creation of cgroupns will be
> disallowed. What do you think?

I think this is *safe* but may not useful for eventual systemd stuff.
Not really sure.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists