From e034cedce7df632eaa52636d34d1d8364733b402 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Tue, 29 Nov 2016 01:25:06 +1100 Subject: [PATCH] libcontainer: init: only pass stateDirFd when creating a container If we pass a file descriptor to the host filesystem while joining a container, there is a race condition where a process inside the container can ptrace(2) the joining process and stop it from closing its file descriptor to the stateDirFd. Then the process can access the *host* filesystem from that file descriptor. This was fixed in part by 5d93fed3d27f ("Set init processes as non-dumpable"), but that fix is more of a hail-mary than an actual fix for the underlying issue. To fix this, don't open or pass the stateDirFd to the init process unless we're creating a new container. A proper fix for this would be to remove the need for even passing around directory file descriptors (which are quite dangerous in the context of mount namespaces). There is still an issue with containers that have CAP_SYS_PTRACE and are using the setns(2)-style of joining a container namespace. Currently I'm not really sure how to fix it without rampant layer violation. Fixes: CVE-2016-9962 Fixes: 5d93fed3d27f ("Set init processes as non-dumpable") Signed-off-by: Aleksa Sarai --- libcontainer/container_linux.go | 30 +++++++++++++++++----------- libcontainer/factory_linux.go | 34 ++++++++++++++++++-------------- libcontainer/init_linux.go | 5 ++--- libcontainer/process_linux.go | 2 -- libcontainer/setns_init_linux.go | 9 ++------- 5 files changed, 41 insertions(+), 39 deletions(-) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 611b40ad1..9f0043fb0 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -296,21 +296,29 @@ func (c *linuxContainer) newParentProcess(p *Process, doInit bool) (parentProces if err != nil { return nil, newSystemErrorWithCause(err, "creating new init pipe") } - rootDir, err := os.Open(c.root) - if err != nil { - return nil, err - } - cmd, err := c.commandTemplate(p, childPipe, rootDir) + cmd, err := c.commandTemplate(p, childPipe) if err != nil { return nil, newSystemErrorWithCause(err, "creating new command template") } if !doInit { - return c.newSetnsProcess(p, cmd, parentPipe, childPipe, rootDir) + return c.newSetnsProcess(p, cmd, parentPipe, childPipe) } + + // We only set up rootDir if we're not doing a `runc exec`. The reason for + // this is to avoid cases where a racing, unprivileged process inside the + // container can get access to the statedir file descriptor (which would + // allow for container rootfs escape). + rootDir, err := os.Open(c.root) + if err != nil { + return nil, err + } + cmd.ExtraFiles = append(cmd.ExtraFiles, rootDir) + cmd.Env = append(cmd.Env, + fmt.Sprintf("_LIBCONTAINER_STATEDIR=%d", stdioFdCount+len(cmd.ExtraFiles)-1)) return c.newInitProcess(p, cmd, parentPipe, childPipe, rootDir) } -func (c *linuxContainer) commandTemplate(p *Process, childPipe, rootDir *os.File) (*exec.Cmd, error) { +func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File) (*exec.Cmd, error) { cmd := exec.Command(c.initArgs[0], c.initArgs[1:]...) cmd.Stdin = p.Stdin cmd.Stdout = p.Stdout @@ -319,10 +327,9 @@ func (c *linuxContainer) commandTemplate(p *Process, childPipe, rootDir *os.File if cmd.SysProcAttr == nil { cmd.SysProcAttr = &syscall.SysProcAttr{} } - cmd.ExtraFiles = append(p.ExtraFiles, childPipe, rootDir) + cmd.ExtraFiles = append(p.ExtraFiles, childPipe) cmd.Env = append(cmd.Env, - fmt.Sprintf("_LIBCONTAINER_INITPIPE=%d", stdioFdCount+len(cmd.ExtraFiles)-2), - fmt.Sprintf("_LIBCONTAINER_STATEDIR=%d", stdioFdCount+len(cmd.ExtraFiles)-1)) + fmt.Sprintf("_LIBCONTAINER_INITPIPE=%d", stdioFdCount+len(cmd.ExtraFiles)-1)) // NOTE: when running a container with no PID namespace and the parent process spawning the container is // PID1 the pdeathsig is being delivered to the container's init process by the kernel for some reason // even with the parent still running. @@ -360,7 +367,7 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, c }, nil } -func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe, rootDir *os.File) (*setnsProcess, error) { +func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe *os.File) (*setnsProcess, error) { cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initSetns)) state, err := c.currentState() if err != nil { @@ -382,7 +389,6 @@ func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe, config: c.newInitConfig(p), process: p, bootstrapData: data, - rootDir: rootDir, }, nil } diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index e4b942adf..b90bc042d 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -222,29 +222,33 @@ func (l *LinuxFactory) Type() string { // StartInitialization loads a container by opening the pipe fd from the parent to read the configuration and state // This is a low level implementation detail of the reexec and should not be consumed externally func (l *LinuxFactory) StartInitialization() (err error) { - var pipefd, rootfd int - for _, pair := range []struct { - k string - v *int - }{ - {"_LIBCONTAINER_INITPIPE", &pipefd}, - {"_LIBCONTAINER_STATEDIR", &rootfd}, - } { + var ( + pipefd, rootfd int + envInitPipe = os.Getenv("_LIBCONTAINER_INITPIPE") + envStateDir = os.Getenv("_LIBCONTAINER_STATEDIR") + ) - s := os.Getenv(pair.k) - - i, err := strconv.Atoi(s) - if err != nil { - return fmt.Errorf("unable to convert %s=%s to int", pair.k, s) - } - *pair.v = i + // Get the INITPIPE. + pipefd, err = strconv.Atoi(envInitPipe) + if err != nil { + return fmt.Errorf("unable to convert _LIBCONTAINER_INITPIPE=%s to int: %s", envInitPipe, err) } + var ( pipe = os.NewFile(uintptr(pipefd), "pipe") it = initType(os.Getenv("_LIBCONTAINER_INITTYPE")) ) defer pipe.Close() + // Only init processes have STATEDIR. + rootfd = -1 + if it == initStandard { + rootfd, err = strconv.Atoi(envStateDir) + if err != nil { + return fmt.Errorf("unable to convert _LIBCONTAINER_STATEDIR=%s to int: %s", envStateDir, err) + } + } + // clear the current process's environment to clean any libcontainer // specific env vars. os.Clearenv() diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 9d5f06802..01f32f506 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -77,9 +77,8 @@ func newContainerInit(t initType, pipe *os.File, stateDirFD int) (initer, error) switch t { case initSetns: return &linuxSetnsInit{ - pipe: pipe, - config: config, - stateDirFD: stateDirFD, + pipe: pipe, + config: config, }, nil case initStandard: return &linuxStandardInit{ diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 3ff2000b2..50e90757e 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -51,7 +51,6 @@ type setnsProcess struct { fds []string process *Process bootstrapData io.Reader - rootDir *os.File } func (p *setnsProcess) startTime() (string, error) { @@ -70,7 +69,6 @@ func (p *setnsProcess) start() (err error) { defer p.parentPipe.Close() err = p.cmd.Start() p.childPipe.Close() - p.rootDir.Close() if err != nil { return newSystemErrorWithCause(err, "starting setns process") } diff --git a/libcontainer/setns_init_linux.go b/libcontainer/setns_init_linux.go index 372dfd357..2e80450d6 100644 --- a/libcontainer/setns_init_linux.go +++ b/libcontainer/setns_init_linux.go @@ -5,7 +5,6 @@ package libcontainer import ( "fmt" "os" - "syscall" "github.com/opencontainers/runc/libcontainer/apparmor" "github.com/opencontainers/runc/libcontainer/keys" @@ -17,9 +16,8 @@ import ( // linuxSetnsInit performs the container's initialization for running a new process // inside an existing container. type linuxSetnsInit struct { - pipe *os.File - config *initConfig - stateDirFD int + pipe *os.File + config *initConfig } func (l *linuxSetnsInit) getSessionRingName() string { @@ -60,8 +58,5 @@ func (l *linuxSetnsInit) Init() error { if err := label.SetProcessLabel(l.config.ProcessLabel); err != nil { return err } - // close the statedir fd before exec because the kernel resets dumpable in the wrong order - // https://github.com/torvalds/linux/blob/v4.9/fs/exec.c#L1290-L1318 - syscall.Close(l.stateDirFD) return system.Execv(l.config.Args[0], l.config.Args[0:], os.Environ()) }