Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update Dockerfile, with Giuseppe's newuidmap #23

Merged
merged 1 commit into from
Oct 15, 2018

Conversation

AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Oct 12, 2018

EDIT: waiting for docker/cli#1347 to be merged


Signed-off-by: Akihiro Suda suda.akihiro@lab.ntt.co.jp

With shadow-maint/shadow#132 , we should no longer need --privileged for running RootlessKit inside Docker containers.

@AkihiroSuda
Copy link
Member Author

unshare -rmn mount -t sysfs none /sys inside container seems to require some privilege..?

$ id -u
1001
$ unshare -rmn mount -t sysfs none /sys
(success)
$ docker run --rm --user 1000 --privileged ubuntu:18.04 unshare -rmn mount -t sysfs none /sys
(success)
$ docker run --rm --user 1000 --security-opt seccomp=unconfined --security-opt apparmor=unconfined --cap-add all --net=host ubuntu:18.04 unshare -rmn mount -t sysfs none /sys
mount: /sys: permission denied.

@AkihiroSuda
Copy link
Member Author

@giuseppe @cyphar @jessfraz could you help me about the sysfs mount? ^^

@AkihiroSuda
Copy link
Member Author

Uh, read-only mount on /sys prohibits unshare -rmn mount -t sysfs none /sys...

https://github.com/moby/moby/blob/07ccc6d8c81d96e07f60184dbe03e75c552eba47/daemon/oci_linux.go#L607-L616

$ docker run -it --privileged --rm --entrypoint bash rootless-containers/rootlesskit:test 
user@5ffda338c480:/$ unshare -rmn mount -t sysfs none /sys; echo $?
0
user@5ffda338c480:/$ sudo mount -o remount,ro /sys
user@5ffda338c480:/$ unshare -rmn mount -t sysfs none /sys; echo $?
mount: /sys: permission denied.
32
user@5ffda338c480:/$ mkdir /tmp/sys2
user@5ffda338c480:/$ unshare -rmn mount -t sysfs none /tmp/sys2; echo $?
mount: /tmp/sys2: permission denied.
32
user@5ffda338c480:/$ echo 'why :('
why :(

@giuseppe
Copy link
Contributor

giuseppe commented Oct 12, 2018

user@5ffda338c480:/$ echo 'why :(' why :(

I think the error happens because of: https://github.com/torvalds/linux/blob/master/fs/namespace.c#L3326-L3328

It should be fine to mount another /sys that is also 'ro'. This works on Fedora with Linux 4.18.9-200.fc28.x86_64:

# docker run --rm  --security-opt seccomp=unconfined --cap-add all -ti ubuntu:18.04 \
  bash  -c 'umount /sys/firmware; \
  chroot --userspec 1000:1000 / unshare -rmn mount -o ro -t sysfs none /sys'

@AkihiroSuda
Copy link
Member Author

Thanks, but it seems we still need to unmount /sys/firmware as the root in the container...

@giuseppe
Copy link
Contributor

Thanks, but it seems we still need to unmount /sys/firmware as the root in the container...

yes, there must already be a fully visible /sys mounted

@AkihiroSuda AkihiroSuda changed the title update Dockerfile, with Giuseppe's newuidmap [WIP:waiting for docker/cli#1347] update Dockerfile, with Giuseppe's newuidmap Oct 12, 2018
@cyphar
Copy link
Member

cyphar commented Oct 13, 2018

Yes, this is quite similar to the other mnt_already_visible problems we had with /proc. Basically in order to mount any filesystem in a userns you need to have a non-masked mount of it in your mount namespace, with mount options that are not more restricted than what you're trying to mount.

(This is a somewhat odd restriction for /proc and /sys since they are namespaced implicitly -- and this has been brought to the attention of Eric in the past, but his proposed fix of procfs2 is not something we should expect soon.)

@AkihiroSuda AkihiroSuda changed the title [WIP:waiting for docker/cli#1347] update Dockerfile, with Giuseppe's newuidmap update Dockerfile, with Giuseppe's newuidmap Oct 15, 2018
@AkihiroSuda
Copy link
Member Author

updated PR.
Mounting sysfs is now skipped if it is not possible

Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
@AkihiroSuda AkihiroSuda merged commit 148a049 into rootless-containers:master Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants