Adds user namespace support to libcontainer #304

mrunalp · 2014-12-17T20:21:55Z

The build will fail as this depends on golang 1.4
I am sending the PR, hoping to get people to test it and give feedback.
There are still log statements for debugging that I will clean up (but keeping them now to make it easier to catch issues).

@crosbymichael @vmarmol @rjnagal PTAL and try it out :)

dqminh · 2014-12-18T09:48:05Z

yay, i tried with a custom Dockerfile that has go 1.4 and it seems to work.

# inside sh nsinit shell
drwxr-xr-x    2 root     root        4.0K Dec 18 09:42 .
drwxrwxr-x    5 65534    65534       4.0K Dec 18 09:42 ..
-rw-r--r--    1 root     root           6 Dec 18 09:42 test

# 26792 is the pid of sh process spawned by make sh
❯ cat /proc/26792/gid_map                                                                                
         0       1000          1
         1          1        999
      1001       1001       9000
❯ cat /proc/26792/uid_map                                                                                
         0       1000          1
         1          1        999
      1001       1001       9000

mrunalp · 2014-12-18T18:08:18Z

@dqminh Thanks for testing it out! :)

avagin · 2014-12-24T09:28:46Z

mount/init.go

@@ -58,14 +58,17 @@ func InitializeMountNamespace(rootfs, console string, sysReadonly bool, mountCon
 		return fmt.Errorf("create device nodes %s", err)


mknod isn't allowed in a non-root userns

The setup process is run in the root userns. The init is started (cloning into userns) and then waits for setup to run and set things up. Setup joins all the namespaces of init except userns. Thanks for taking a look. Feedback and suggestions appreciated.

@mrunalp Sorry, I don't get the idea with a setup process. Which actions can't be done from userns? Could you elaborate? Maybe you can add a comment before the SetupContainer() function.
LoadContainerEnvironment(), apparmor.ApplyProfile(), label.SetProcessLabel() affect only a current process. Now we call them for the setup process and don't call for the init process. Where are these parameters applied to the init process?
Thanks.

You are right about SetProcessLabel and ApplyProfile. I have to move them to init. I was aware but missed moving them back. I will post an updated patch with more comments. Thanks.

Sent from my iPhone

On Jan 13, 2015, at 12:29 AM, Andrew Vagin notifications@github.com wrote:

In mount/init.go:

@@ -58,14 +58,17 @@ func InitializeMountNamespace(rootfs, console string, sysReadonly bool, mountCon
return fmt.Errorf("create device nodes %s", err)
@mrunalp Sorry, I don't get the idea with a setup process. Which actions can't be done from userns? Could you elaborate? Maybe you can add a comment before the SetupContainer() function.
LoadContainerEnvironment(), apparmor.ApplyProfile(), label.SetProcessLabel() affect only a current process. Now we call them for the setup process and don't call for the init process. Where are these parameters applied to the init process?
Thanks.

—
Reply to this email directly or view it on GitHub.

mrunalp · 2015-01-02T19:27:51Z

@avagin Yes, we need to add support for userns to nsenter. I was thinking of doing that in follow on patches after this one gets in.

crosbymichael · 2015-01-13T00:55:30Z

This is looking good, just installed Go 1.4 for testing

crosbymichael · 2015-01-13T01:30:14Z

Maybe we can get this merged this week.

ping @LK4D4 @vmarmol

mrunalp · 2015-01-13T01:31:53Z

Cool, I will rebase it and address comments.

mrunalp · 2015-01-16T17:01:41Z

Ping anyone? :)

LK4D4 · 2015-01-16T17:04:27Z

@mrunalp Hah, sorry, will review now.

LK4D4 · 2015-01-16T17:42:10Z

console/console.go

@@ -21,7 +21,7 @@ func Setup(rootfs, consolePath, mountLabel string) error {
 		return err
 	}

-	if err := os.Chown(consolePath, 0, 0); err != nil {
+	if err := os.Chown(consolePath, hostRootUid, hostRootUid); err != nil {


Is this always true, that hostRootUid == hostRootGid?

We could make that assumption since it doesn't make sense to have separate uid/gid mappings. (Even though we have exposed that).

I think we can pass down hostRootGid as well.

mrunalp · 2015-01-19T17:35:24Z

@LK4D4 Any more comments? :)

crosbymichael · 2015-01-19T18:27:08Z

namespaces/init.go

+		return err
+	}
+	if consolePath != "" {
+		if err := console.OpenAndDup("/dev/console"); err != nil {


Should this be consolePath instead of hard coded /dev/console?

The mount setup is done before we reach here to setup /dev/console and the original consolePath isn't available anymore.

crosbymichael · 2015-01-19T18:41:12Z

namespaces/execin.go

+// to perform these operations.
+func SetupContainer(container *libcontainer.Config, args []string) error {
+	consolePath := ""
+	dataPath := args[0]


Shouldn't this just be parameters of the function?

Yes, fixing.

Signed-off-by: Mrunal Patel <mrunalp@gmail.com> (github: mrunalp) Adds sample configuration to test user namespaces. Signed-off-by: Mrunal Patel <mrunalp@gmail.com> (github: mrunalp) Rebases to master. Signed-off-by: Mrunal Patel <mrunalp@gmail.com> (github: mrunalp) Fixes integration tests. Signed-off-by: Mrunal Patel <mrunalp@gmail.com> (github: mrunalp) Move selinux labeling, apparmor profile and restrict kernel files back to init. Signed-off-by: Mrunal Patel <mrunalp@gmail.com> (github: mrunalp) Separate the code paths for userns and default cases. Signed-off-by: Mrunal Patel <mrunalp@gmail.com> (github: mrunalp) tty not required for setup Signed-off-by: Mrunal Patel <mrunalp@gmail.com> (github: mrunalp) Cleanup and address review comments. Signed-off-by: Mrunal Patel <mrunalp@gmail.com> (github: mrunalp) Remove debug logs and other cleanup. Signed-off-by: Mrunal Patel <mrunalp@gmail.com> (github: mrunalp) Use function paramaters for SetupContainer. Signed-off-by: Mrunal Patel <mrunalp@gmail.com> (github: mrunalp)

crosbymichael · 2015-01-19T19:53:00Z

LGTM

LK4D4 · 2015-01-19T19:55:17Z

LGTM

Adds user namespace support to libcontainer

brauner · 2015-01-20T21:49:27Z

Really looking forward to this. Out of impatience when can we expect to make use of uid and gid mappings with docker run?

mrunalp · 2015-01-20T21:54:41Z

@brauner Don't know when that will land, but work has started on basic docker integration. Keep an eye on the PRs :)

brauner · 2015-01-20T21:56:39Z

@mrunalp excellent! :)

slafs · 2015-01-23T10:26:25Z

This is great! Can you please link a relevant PR from Docker to this one?

LK4D4 · 2015-01-23T14:36:59Z

@slafs We will link when we create it :)

TomasTomecek · 2015-09-24T08:16:44Z

@slafs moby/moby#12648

LK4D4 mentioned this pull request Dec 22, 2014

Add supplemental UNIX groups to a container at 'docker run' time moby/moby#9360

Closed

avagin reviewed Dec 24, 2014
View reviewed changes

mrunalp force-pushed the feature/user_namespaces branch 2 times, most recently from 95746d1 to ce7d276 Compare January 5, 2015 21:50

mrunalp force-pushed the feature/user_namespaces branch from be4b381 to 6647d5b Compare January 13, 2015 21:04

LK4D4 reviewed Jan 16, 2015
View reviewed changes

crosbymichael reviewed Jan 19, 2015
View reviewed changes

mrunalp force-pushed the feature/user_namespaces branch from 465f07f to 4dd5396 Compare January 19, 2015 18:31

crosbymichael reviewed Jan 19, 2015
View reviewed changes

mrunalp force-pushed the feature/user_namespaces branch from 7ad27ca to b0eece8 Compare January 19, 2015 19:52

LK4D4 added a commit that referenced this pull request Jan 19, 2015

Merge pull request #304 from mrunalp/feature/user_namespaces

b89cd0c

Adds user namespace support to libcontainer

LK4D4 merged commit b89cd0c into docker-archive:master Jan 19, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds user namespace support to libcontainer #304

Adds user namespace support to libcontainer #304

mrunalp commented Dec 17, 2014

dqminh commented Dec 18, 2014

mrunalp commented Dec 18, 2014

avagin Dec 24, 2014

mrunalp Dec 25, 2014

avagin Jan 13, 2015

mrunalp Jan 13, 2015

mrunalp commented Jan 2, 2015

crosbymichael commented Jan 13, 2015

crosbymichael commented Jan 13, 2015

mrunalp commented Jan 13, 2015

mrunalp commented Jan 16, 2015

LK4D4 commented Jan 16, 2015

LK4D4 Jan 16, 2015

mrunalp Jan 16, 2015

mrunalp Jan 16, 2015

mrunalp Jan 16, 2015

mrunalp commented Jan 19, 2015

crosbymichael Jan 19, 2015

mrunalp Jan 19, 2015

crosbymichael Jan 19, 2015

crosbymichael Jan 19, 2015

mrunalp Jan 19, 2015

mrunalp Jan 19, 2015

crosbymichael commented Jan 19, 2015

LK4D4 commented Jan 19, 2015

brauner commented Jan 20, 2015

mrunalp commented Jan 20, 2015

brauner commented Jan 20, 2015

slafs commented Jan 23, 2015

LK4D4 commented Jan 23, 2015

TomasTomecek commented Sep 24, 2015

		@@ -58,14 +58,17 @@ func InitializeMountNamespace(rootfs, console string, sysReadonly bool, mountCon
		return fmt.Errorf("create device nodes %s", err)

Adds user namespace support to libcontainer #304

Adds user namespace support to libcontainer #304

Conversation

mrunalp commented Dec 17, 2014

dqminh commented Dec 18, 2014

mrunalp commented Dec 18, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrunalp commented Jan 2, 2015

crosbymichael commented Jan 13, 2015

crosbymichael commented Jan 13, 2015

mrunalp commented Jan 13, 2015

mrunalp commented Jan 16, 2015

LK4D4 commented Jan 16, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrunalp commented Jan 19, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crosbymichael commented Jan 19, 2015

LK4D4 commented Jan 19, 2015

brauner commented Jan 20, 2015

mrunalp commented Jan 20, 2015

brauner commented Jan 20, 2015

slafs commented Jan 23, 2015

LK4D4 commented Jan 23, 2015

TomasTomecek commented Sep 24, 2015