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

Sharing /dev/ with the container breaks /dev/ptmx on the host #80

Closed
willmtemple opened this issue Jul 1, 2015 · 2 comments
Closed

Comments

@willmtemple
Copy link

When a container is launched with a bind mount between /dev (on the host) and /dev (in the container), all subsequent attempts to open /dev/ptmx fail.

setupPtmx() in libcontainer/rootfs_linux.go will unconditionally remove /dev/ptmx and then symlink it to /dev/pts/ptmx , which is a character file that is untouchable by unprivileged users. This prevents unprivileged programs which rely on getpt from running on the host (such as X terminals, screen, and tmux).

I discovered this via docker, but it happens in any libcontainer app if /dev/ is shared with the container before setupPtmx. Is there a reason for libcontainer to touch /dev (at all) if it's mounted directly from the host? I have tested this on Fedora 22 and 21, SELinux is set to permissive, as well as Arch Linux without SELinux.

I can reproduce this consistently.

uname -a: Linux wtemple.localdomain 4.0.6-300.fc22.x86_64 #1 SMP Tue Jun 23 13:58:53 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

config.json:

{
    "version": "0.1.1",
    "platform": {
        "os": "linux",
        "arch": "amd64"
    },
    "process": {
        "terminal": true,
        "user": "root",
        "args": [
            "/bin/sh"
        ],
        "env": [
            "PATH=/bin,/sbin"
        ],
        "cwd": ""
    },
    "root": {
        "path": "rootfs",
        "readonly": true
    },
    "hostname": "shell",
    "mounts": [
        {
            "type": "proc",
            "source": "proc",
            "destination": "/proc",
            "options": ""
        },
        {
            "type": "sysfs",
            "source": "sysfs",
            "destination": "/sys",
            "options": "nosuid,noexec,nodev"
        },
        {
            "type": "bind",
            "source": "/dev",
            "destination": "/dev",
            "options": "bind"
        }
    ],
    "linux": {
        "namespaces": [
            {
                "type": "process"
            },
            {
                "type": "network"
            },
            {
                "type": "mount"
            },
            {
                "type": "ipc"
            },
            {
                "type": "uts"
            }
        ],
        "capabilities": [
            "AUDIT_WRITE",
            "KILL",
            "NET_BIND_SERVICE"
        ],
        "devices": [
            "null"
        ]
    }
}
@kyle-walker
Copy link

This seems to be due to the default SELinux policy for a Fedora system which includes:

$ sudo semanage fcontext -l | grep ptmx
/dev/ptmx character device system_u:object_r:ptmx_t:s0

This omits the Symlink that replaces the character device. The following resolves the condition in my testing:

$ sudo semanage fcontext -a -f l -t ptmx_t /dev/ptmx

I would recommend:

$ diff -up docker.fc.bak docker.fc
--- docker.fc.bak 2016-03-08 11:33:38.894580384 -0500
+++ docker.fc 2016-03-08 11:33:52.348740405 -0500
@@ -1,5 +1,7 @@
/root/.docker gen_context(system_u:object_r:docker_home_t,s0)

+/dev/ptmx -l gen_context(system_u:object_r:ptmx_t:s0)
+
/usr/bin/docker -- gen_context(system_u:object_r:docker_exec_t,s0)

/usr/lib/systemd/system/docker.service -- gen_context(system_u:object_r:docker_unit_file_t,s0)

Ninja edit>
Had to edit the above so that the correct "ptmx_t" context was included in the patch.

  • Kyle

@AkihiroSuda
Copy link
Member

moby/moby#21808 is hitting this issue again.

#96 (comment) introduced setupDev := len(config.Devices) != 0 for this issue.

if err := setupPtmx(config, console); err != nil {

But I'm not sure there is any path that sets setupDev to false, because config.Devices seems to be always set to non-empty(3baae2d, #536) in spec_linux.go: createDevices().

AkihiroSuda added a commit to AkihiroSuda/runc that referenced this issue Apr 8, 2016
setupDev was introduced in opencontainers#96, but broken since opencontainers#536 because spec 0.3.0 introduced default devices.

Fix opencontainers#80 again
Fix moby/moby#21808
AkihiroSuda added a commit to AkihiroSuda/runc that referenced this issue Apr 8, 2016
setupDev was introduced in opencontainers#96, but broken since opencontainers#536 because spec 0.3.0 introduced default devices.

Fix opencontainers#80 again
Fix moby/moby#21808

Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda added a commit to AkihiroSuda/runc that referenced this issue Apr 8, 2016
setupDev was introduced in opencontainers#96, but broken since opencontainers#536 because spec 0.3.0 introduced default devices.

Fix opencontainers#80 again
Fix moby/moby#21808

Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda added a commit to AkihiroSuda/runc that referenced this issue Apr 11, 2016
setupDev was introduced in opencontainers#96, but broken since opencontainers#536 because spec 0.3.0 introduced default devices.

Fix opencontainers#80 again
Fix moby/moby#21808

Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda added a commit to AkihiroSuda/runc that referenced this issue Apr 11, 2016
setupDev was introduced in opencontainers#96, but broken since opencontainers#536 because spec 0.3.0 introduced default devices.

Fix opencontainers#80 again
Fix moby/moby#21808

Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda added a commit to AkihiroSuda/runc that referenced this issue Apr 11, 2016
setupDev was introduced in opencontainers#96, but broken since opencontainers#536 because spec 0.3.0 introduced default devices.

Fix opencontainers#80 again
Fix moby/moby#21808

Signed-off-by: Akihiro Suda <[email protected]>
LK4D4 pushed a commit to LK4D4/runc that referenced this issue Apr 11, 2016
setupDev was introduced in opencontainers#96, but broken since opencontainers#536 because spec 0.3.0 introduced default devices.

Fix opencontainers#80 again
Fix moby/moby#21808

Signed-off-by: Akihiro Suda <[email protected]>
Signed-off-by: Alexander Morozov <[email protected]>
LK4D4 pushed a commit to LK4D4/runc that referenced this issue Apr 11, 2016
setupDev was introduced in opencontainers#96, but broken since opencontainers#536 because spec 0.3.0 introduced default devices.

Fix opencontainers#80 again
Fix moby/moby#21808

Signed-off-by: Akihiro Suda <[email protected]>
Signed-off-by: Alexander Morozov <[email protected]>
stefanberger pushed a commit to stefanberger/runc that referenced this issue Sep 8, 2017
specs: fix the description for the [ug]idMappings
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

No branches or pull requests

3 participants