diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 186faab0c..9e1b74d77 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -350,6 +350,23 @@ func (c *linuxContainer) deleteExecFifo() { os.Remove(fifoName) } +// includeExecFifo opens the container's execfifo as a pathfd, so that the +// container cannot access the statedir (and the FIFO itself remains +// un-opened). It then adds the FifoFd to the given exec.Cmd as an inherited +// fd, with _LIBCONTAINER_FIFOFD set to its fd number. +func (c *linuxContainer) includeExecFifo(cmd *exec.Cmd) error { + fifoName := filepath.Join(c.root, execFifoFilename) + fifoFd, err := unix.Open(fifoName, unix.O_PATH|unix.O_CLOEXEC, 0) + if err != nil { + return err + } + + cmd.ExtraFiles = append(cmd.ExtraFiles, os.NewFile(uintptr(fifoFd), fifoName)) + cmd.Env = append(cmd.Env, + fmt.Sprintf("_LIBCONTAINER_FIFOFD=%d", stdioFdCount+len(cmd.ExtraFiles)-1)) + return nil +} + func (c *linuxContainer) newParentProcess(p *Process, doInit bool) (parentProcess, error) { parentPipe, childPipe, err := utils.NewSockPair("init") if err != nil { @@ -363,18 +380,15 @@ func (c *linuxContainer) newParentProcess(p *Process, doInit bool) (parentProces 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 + // We only set up fifoFd if we're not doing a `runc exec`. The historic + // reason for this is that previously we would pass a dirfd that allowed + // for container rootfs escape (and not doing it in `runc exec` avoided + // that problem), but we no longer do that. However, there's no need to do + // this for `runc exec` so we just keep it this way to be safe. + if err := c.includeExecFifo(cmd); err != nil { + return nil, newSystemErrorWithCause(err, "including execfifo in cmd.Exec setup") } - 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) + return c.newInitProcess(p, cmd, parentPipe, childPipe) } func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File) (*exec.Cmd, error) { @@ -406,7 +420,7 @@ func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File) (*exec. return cmd, nil } -func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe, rootDir *os.File) (*initProcess, error) { +func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe *os.File) (*initProcess, error) { cmd.Env = append(cmd.Env, "_LIBCONTAINER_INITTYPE="+string(initStandard)) nsMaps := make(map[configs.NamespaceType]string) for _, ns := range c.config.Namespaces { @@ -429,7 +443,6 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, c process: p, bootstrapData: data, sharePidns: sharePidns, - rootDir: rootDir, }, nil } diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index 42b6f5a05..947bdea1c 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -233,10 +233,10 @@ func (l *LinuxFactory) Type() string { // 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 + pipefd, fifofd int consoleSocket *os.File envInitPipe = os.Getenv("_LIBCONTAINER_INITPIPE") - envStateDir = os.Getenv("_LIBCONTAINER_STATEDIR") + envFifoFd = os.Getenv("_LIBCONTAINER_FIFOFD") envConsole = os.Getenv("_LIBCONTAINER_CONSOLE") ) @@ -252,11 +252,11 @@ func (l *LinuxFactory) StartInitialization() (err error) { ) defer pipe.Close() - // Only init processes have STATEDIR. - rootfd = -1 + // Only init processes have FIFOFD. + fifofd = -1 if it == initStandard { - if rootfd, err = strconv.Atoi(envStateDir); err != nil { - return fmt.Errorf("unable to convert _LIBCONTAINER_STATEDIR=%s to int: %s", envStateDir, err) + if fifofd, err = strconv.Atoi(envFifoFd); err != nil { + return fmt.Errorf("unable to convert _LIBCONTAINER_FIFOFD=%s to int: %s", envFifoFd, err) } } @@ -291,7 +291,7 @@ func (l *LinuxFactory) StartInitialization() (err error) { } }() - i, err := newContainerInit(it, pipe, consoleSocket, rootfd) + i, err := newContainerInit(it, pipe, consoleSocket, fifofd) if err != nil { return err } diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 911861aed..2020bb7a5 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -68,7 +68,7 @@ type initer interface { Init() error } -func newContainerInit(t initType, pipe *os.File, consoleSocket *os.File, stateDirFD int) (initer, error) { +func newContainerInit(t initType, pipe *os.File, consoleSocket *os.File, fifoFd int) (initer, error) { var config *initConfig if err := json.NewDecoder(pipe).Decode(&config); err != nil { return nil, err @@ -89,7 +89,7 @@ func newContainerInit(t initType, pipe *os.File, consoleSocket *os.File, stateDi consoleSocket: consoleSocket, parentPid: unix.Getppid(), config: config, - stateDirFD: stateDirFD, + fifoFd: fifoFd, }, nil } return nil, fmt.Errorf("unknown init type %q", t) diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index c5495e4cb..50f9af574 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -203,7 +203,6 @@ type initProcess struct { process *Process bootstrapData io.Reader sharePidns bool - rootDir *os.File } func (p *initProcess) pid() int { @@ -258,7 +257,6 @@ func (p *initProcess) start() error { err := p.cmd.Start() p.process.ops = p p.childPipe.Close() - p.rootDir.Close() if err != nil { p.process.ops = nil return newSystemErrorWithCause(err, "starting init process command") diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index 580b3fe45..fbcf3a6ac 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -22,7 +22,7 @@ type linuxStandardInit struct { pipe *os.File consoleSocket *os.File parentPid int - stateDirFD int + fifoFd int config *initConfig } @@ -164,9 +164,11 @@ func (l *linuxStandardInit) Init() error { } // close the pipe to signal that we have completed our init. l.pipe.Close() - // wait for the fifo to be opened on the other side before - // exec'ing the users process. - fd, err := unix.Openat(l.stateDirFD, execFifoFilename, os.O_WRONLY|unix.O_CLOEXEC, 0) + // Wait for the FIFO to be opened on the other side before exec-ing the + // user process. We open it through /proc/self/fd/$fd, because the fd that + // was given to us was an O_PATH fd to the fifo itself. Linux allows us to + // re-open an O_PATH fd through /proc. + fd, err := unix.Open(fmt.Sprintf("/proc/self/fd/%d", l.fifoFd), unix.O_WRONLY|unix.O_CLOEXEC, 0) if err != nil { return newSystemErrorWithCause(err, "openat exec fifo") } @@ -180,7 +182,7 @@ func (l *linuxStandardInit) Init() error { } // 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 - unix.Close(l.stateDirFD) + unix.Close(l.fifoFd) if err := syscall.Exec(name, l.config.Args[0:], os.Environ()); err != nil { return newSystemErrorWithCause(err, "exec user process") }