basic support for running in an user namespace #1729

giuseppe · 2018-08-09T10:55:02Z

- What I did
fixed some issues to allow CRI-O running in an user namespace

- How I did it
Allow to configure all directories used by CRI-O

- How to verify it
create a namespace, and run CRI-O with a configuration file where all directories are writeable by the user

giuseppe · 2018-08-09T16:10:34Z

/cc @AkihiroSuda

k8s-ci-robot · 2018-08-09T16:10:34Z

@giuseppe: GitHub didn't allow me to request PR reviews from the following users: AkihiroSuda.

Note that only kubernetes-incubator members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @AkihiroSuda

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

AkihiroSuda · 2018-08-09T16:30:27Z

🎉 I'll try to add cri-o to Usernetes https://github.com/rootless-containers/usernetes

How did you test rootless cri-o?
With Usernetes or with crictl?

AkihiroSuda · 2018-08-09T16:36:11Z

server/rootless.go

+
+func makeOCIConfigurationRootless(g *generate.Generator) error {
+	g.Config.Linux.Resources = nil
+	g.Config.Process.OOMScoreAdj = nil


I'm not familiar with apparmor yet but probably it does not work with rootless?

I am not familiar either but I guess it won't work, similarly with SELinux, I'll force them to ""

SELinux would work in UserNS. SElinux doesn't care about DAC.

giuseppe · 2018-08-09T18:55:32Z

@AkihiroSuda I've only used crictl with some basic containers, I'd expect it to fail with more complex ones :-) I am going to test it with Usernetes

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe · 2018-08-14T17:33:59Z

dropping WIP as I could test it with Usernetes and I am able to deploy a simple POD.

rhatdan · 2018-08-14T18:12:15Z

/test all

mrunalp · 2018-08-14T23:16:29Z

lib/config.go

@@ -96,6 +96,9 @@ type RootConfig struct {
 	// File-based locking is required when multiple users of lib are
 	// present on the same system
 	FileLocking bool `toml:"file_locking"`
+
+	// FileLockingPath specifies the path to use for the locking.
+	FileLockingPath bool `toml:"file_locking_path"`


nit: can we fix this commit to not introduce this as bool and rewrite as string in next commit?

absolutely, I thought I already did it, apparently I screwed the rebase. Pushed a new version

mrunalp · 2018-08-14T23:16:47Z

I have one nit, otherwise looks fine 👍

AkihiroSuda · 2018-08-15T02:02:49Z

server/rootless.go

+		Destination: "/sys",
+		Type:        "bind",
+		Source:      "/sys",
+		Options:     []string{"nosuid", "noexec", "nodev", "ro", "rbind"},


ro, rbind does not make /sys/fs/cgroup read-only recursively, could you cherry-pick https://gist.github.com/AkihiroSuda/889b29c56a3c9b546a843fb03f11d60a ?

I am seeing an issue when I try to mount cgroup after the /sys/ bind mount. I am trying to fix it in podman first, as the same bind mount is used there and I am using this patch:

diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index d9888e99..1bcb92ad 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -44,6 +44,13 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint Options: []string{"nosuid", "noexec", "nodev", "ro", "rbind"}, } g.AddMount(sysMnt) + cgroupMnt := spec.Mount{ + Destination: "/sys/fs/cgroup", + Type: "cgroup", + Source: "cgroup", + Options: []string{"nosuid", "noexec", "nodev", "relatime", "ro"}, + } + g.AddMount(cgroupMnt) } if rootless.IsRootless() { g.RemoveMount("/dev/pts")

The error I see is:

container_linux.go:336: starting container process caused "process_linux.go:402: container init caused \"rootfs_linux.go:57: mounting \\\"/sys/fs/cgroup\\\" to rootfs \\\"/tmp/test/rootfs\\\" at \\\"/sys/fs/cgroup\\\" caused \\\"stat /tmp/test/rootfs/sys/fs/cgroup/systemd/user.slice/user-1000.slice/user@1000.service/gnome-terminal-server.service/foo: no such file or directory\\\"\""

Also, is it possible to exploit it without privileged helpers?

Sorry you need to compile runc with patched libcontainer/rootfs_linux.go. I'll open PR in a few days.

As the issue won't be exploited without privileged helpers (e.g. pam_cgfs or manual chown invocation), the fix can be postponed.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe · 2018-08-15T14:50:53Z

I've done another change in the rootless code, now /sys is mounted from the host only when not creating a new network namespace

mrunalp · 2018-08-15T21:50:45Z

/test all

giuseppe · 2018-08-16T07:15:04Z

/test e2e_rhel

giuseppe · 2018-08-16T14:32:36Z

tests are happy. @mrunalp

Allow to run it in an user namespace. This is only a first step but I was able to run a pod and some basic containers. Leave it undocumented for now as we don't want to stick with this API. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe · 2018-08-16T15:12:32Z

I forgot to address one comment from @rhatdan, I've included this change now:

@@ -19,7 +19,6 @@ func makeOCIConfigurationRootless(g *generate.Generator) error {
        g.Config.Linux.Resources = nil
        g.Config.Process.OOMScoreAdj = nil
        g.Config.Process.ApparmorProfile = ""
-       g.Config.Process.SelinuxLabel = ""

looks like we need to test again :-(

/test all

mrunalp

LGTM

giuseppe requested review from mrunalp and runcom as code owners August 9, 2018 10:55

k8s-ci-robot added cncf-cla: yes size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 9, 2018

giuseppe force-pushed the rootless branch 3 times, most recently from f90d36d to 91f1dbd Compare August 9, 2018 13:01

AkihiroSuda reviewed Aug 9, 2018

View reviewed changes

AkihiroSuda mentioned this pull request Aug 9, 2018

add cri-o rootless-containers/usernetes#11

Closed

giuseppe force-pushed the rootless branch 3 times, most recently from e3b5bc2 to 9bc3d44 Compare August 13, 2018 11:31

config: move file_locking to the correct place

d6dfba3

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe force-pushed the rootless branch 2 times, most recently from 11ad164 to 91ef229 Compare August 14, 2018 17:32

giuseppe changed the title ~~[WIP] basic support for running in an user namespace~~ basic support for running in an user namespace Aug 14, 2018

giuseppe force-pushed the rootless branch from 91ef229 to 2ce75f2 Compare August 14, 2018 17:55

giuseppe mentioned this pull request Aug 14, 2018

usernetes: add support for CRI-O rootless-containers/usernetes#15

Merged

mrunalp reviewed Aug 14, 2018

View reviewed changes

AkihiroSuda reviewed Aug 15, 2018

View reviewed changes

giuseppe added 3 commits August 15, 2018 16:28

config: allow to override file_locking_path

eea99fa

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

config: add missing container_exits_dir to the config template

26253ad

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

config: allow to override attach socket dir

7390e9c

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe added 2 commits August 15, 2018 16:28

oci: propagate XDG_RUNTIME_DIR to conmon

ef4367d

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

oci: fix segfault if cgroupfs cannot be configured

6e03ced

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe force-pushed the rootless branch from 2ce75f2 to 749faf9 Compare August 15, 2018 14:49

crio: basic support for rootless mode

65c41c1

Allow to run it in an user namespace. This is only a first step but I was able to run a pod and some basic containers. Leave it undocumented for now as we don't want to stick with this API. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe force-pushed the rootless branch from 749faf9 to 65c41c1 Compare August 16, 2018 15:11

mrunalp approved these changes Aug 16, 2018

View reviewed changes

mrunalp merged commit 2accad9 into cri-o:master Aug 17, 2018

AkihiroSuda mentioned this pull request Nov 9, 2018

[UMBRELLA] patch submission status rootless-containers/usernetes#42

Closed

3 tasks

AkihiroSuda mentioned this pull request Jun 4, 2019

KEP: New cgroup driver: "none" (for Rootless) kubernetes/enhancements#1084

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

basic support for running in an user namespace #1729

basic support for running in an user namespace #1729

giuseppe commented Aug 9, 2018

giuseppe commented Aug 9, 2018

k8s-ci-robot commented Aug 9, 2018

AkihiroSuda commented Aug 9, 2018

AkihiroSuda Aug 9, 2018

giuseppe Aug 13, 2018

rhatdan Aug 16, 2018

giuseppe commented Aug 9, 2018

giuseppe commented Aug 14, 2018

rhatdan commented Aug 14, 2018

mrunalp Aug 14, 2018

giuseppe Aug 15, 2018

mrunalp commented Aug 14, 2018

AkihiroSuda Aug 15, 2018

giuseppe Aug 15, 2018

giuseppe Aug 15, 2018

AkihiroSuda Aug 15, 2018

giuseppe commented Aug 15, 2018

mrunalp commented Aug 15, 2018

giuseppe commented Aug 16, 2018

giuseppe commented Aug 16, 2018

giuseppe commented Aug 16, 2018

mrunalp left a comment

basic support for running in an user namespace #1729

basic support for running in an user namespace #1729

Conversation

giuseppe commented Aug 9, 2018

giuseppe commented Aug 9, 2018

k8s-ci-robot commented Aug 9, 2018

AkihiroSuda commented Aug 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giuseppe commented Aug 9, 2018

giuseppe commented Aug 14, 2018

rhatdan commented Aug 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrunalp commented Aug 14, 2018

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

giuseppe commented Aug 15, 2018

mrunalp commented Aug 15, 2018

giuseppe commented Aug 16, 2018

giuseppe commented Aug 16, 2018

giuseppe commented Aug 16, 2018

mrunalp left a comment

Choose a reason for hiding this comment