[PATCH] Pass root to chroot to for chroot Untar

This is useful for preventing CVE-2018-15664 where a malicious container
process can take advantage of a race on symlink resolution/sanitization.

Before this change chrootarchive would chroot to the destination
directory which is attacker controlled. With this patch we always chroot
to the container's root which is not attacker controlled.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Origin: upstream, https://github.com/moby/moby/pull/39292

Gbp-Pq: Name cve-2018-15664-01-pass-root-to-chroot-to-for-chroot-untar.patch

diff --git a/engine/daemon/archive.go b/engine/daemon/archive.go
index 9c7971b56ea3e8425733beed7d28b1228711a277..9f56ca750392dc7b9f875104c48003041aa96693 100644 (file)
--- a/engine/daemon/archive.go
+++ b/engine/daemon/archive.go
@@ -31,11 +31,12 @@ type archiver interface {
 }
 
 // helper functions to extract or archive
-func extractArchive(i interface{}, src io.Reader, dst string, opts *archive.TarOptions) error {
+func extractArchive(i interface{}, src io.Reader, dst string, opts *archive.TarOptions, root string) error {
        if ea, ok := i.(extractor); ok {
                return ea.ExtractArchive(src, dst, opts)
        }
-       return chrootarchive.Untar(src, dst, opts)
+
+       return chrootarchive.UntarWithRoot(src, dst, opts, root)
 }
 
 func archivePath(i interface{}, src string, opts *archive.TarOptions) (io.ReadCloser, error) {
@@ -367,7 +368,7 @@ func (daemon *Daemon) containerExtractToDir(container *container.Container, path
                }
        }
 
-       if err := extractArchive(driver, content, resolvedPath, options); err != nil {
+       if err := extractArchive(driver, content, resolvedPath, options, container.BaseFS.Path()); err != nil {
                return err
        }
 

diff --git a/engine/pkg/chrootarchive/archive.go b/engine/pkg/chrootarchive/archive.go
index 2d9d662830b7b4305d416a41475eb31f78e06549..7ebca3774c3dcbc6a53ceef89d2d3132bc5ae9a3 100644 (file)
--- a/engine/pkg/chrootarchive/archive.go
+++ b/engine/pkg/chrootarchive/archive.go
@@ -27,18 +27,34 @@ func NewArchiver(idMapping *idtools.IdentityMapping) *archive.Archiver {
 // The archive may be compressed with one of the following algorithms:
 //  identity (uncompressed), gzip, bzip2, xz.
 func Untar(tarArchive io.Reader, dest string, options *archive.TarOptions) error {
-       return untarHandler(tarArchive, dest, options, true)
+       return untarHandler(tarArchive, dest, options, true, dest)
+}
+
+// UntarWithRoot is the same as `Untar`, but allows you to pass in a root directory
+// The root directory is the directory that will be chrooted to.
+// `dest` must be a path within `root`, if it is not an error will be returned.
+//
+// `root` should set to a directory which is not controlled by any potentially
+// malicious process.
+//
+// This should be used to prevent a potential attacker from manipulating `dest`
+// such that it would provide access to files outside of `dest` through things
+// like symlinks. Normally `ResolveSymlinksInScope` would handle this, however
+// sanitizing symlinks in this manner is inherrently racey:
+// ref: CVE-2018-15664
+func UntarWithRoot(tarArchive io.Reader, dest string, options *archive.TarOptions, root string) error {
+       return untarHandler(tarArchive, dest, options, true, root)
 }
 
 // UntarUncompressed reads a stream of bytes from `archive`, parses it as a tar archive,
 // and unpacks it into the directory at `dest`.
 // The archive must be an uncompressed stream.
 func UntarUncompressed(tarArchive io.Reader, dest string, options *archive.TarOptions) error {
-       return untarHandler(tarArchive, dest, options, false)
+       return untarHandler(tarArchive, dest, options, false, dest)
 }
 
 // Handler for teasing out the automatic decompression
-func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions, decompress bool) error {
+func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions, decompress bool, root string) error {
        if tarArchive == nil {
                return fmt.Errorf("Empty archive")
        }
@@ -69,5 +85,5 @@ func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions
                r = decompressedArchive
        }
 
-       return invokeUnpack(r, dest, options)
+       return invokeUnpack(r, dest, options, root)
 }

diff --git a/engine/pkg/chrootarchive/archive_unix.go b/engine/pkg/chrootarchive/archive_unix.go
index 5df8afd662055e09d8a58179f5500224b09599f4..96f07c4bb4d688412e405e944b9127783954b43d 100644 (file)
--- a/engine/pkg/chrootarchive/archive_unix.go
+++ b/engine/pkg/chrootarchive/archive_unix.go
@@ -10,6 +10,7 @@ import (
        "io"
        "io/ioutil"
        "os"
+       "path/filepath"
        "runtime"
 
        "github.com/docker/docker/pkg/archive"
@@ -30,11 +31,21 @@ func untar() {
                fatal(err)
        }
 
-       if err := chroot(flag.Arg(0)); err != nil {
+       dst := flag.Arg(0)
+       var root string
+       if len(flag.Args()) > 1 {
+               root = flag.Arg(1)
+       }
+
+       if root == "" {
+               root = dst
+       }
+
+       if err := chroot(root); err != nil {
                fatal(err)
        }
 
-       if err := archive.Unpack(os.Stdin, "/", options); err != nil {
+       if err := archive.Unpack(os.Stdin, dst, options); err != nil {
                fatal(err)
        }
        // fully consume stdin in case it is zero padded
@@ -45,7 +56,7 @@ func untar() {
        os.Exit(0)
 }
 
-func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.TarOptions) error {
+func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.TarOptions, root string) error {
 
        // We can't pass a potentially large exclude list directly via cmd line
        // because we easily overrun the kernel's max argument/environment size
@@ -57,7 +68,21 @@ func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.T
                return fmt.Errorf("Untar pipe failure: %v", err)
        }
 
-       cmd := reexec.Command("docker-untar", dest)
+       if root != "" {
+               relDest, err := filepath.Rel(root, dest)
+               if err != nil {
+                       return err
+               }
+               if relDest == "." {
+                       relDest = "/"
+               }
+               if relDest[0] != '/' {
+                       relDest = "/" + relDest
+               }
+               dest = relDest
+       }
+
+       cmd := reexec.Command("docker-untar", dest, root)
        cmd.Stdin = decompressedArchive
 
        cmd.ExtraFiles = append(cmd.ExtraFiles, r)
@@ -69,6 +94,7 @@ func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.T
                w.Close()
                return fmt.Errorf("Untar error on re-exec cmd: %v", err)
        }
+
        //write the options to the pipe for the untar exec to read
        if err := json.NewEncoder(w).Encode(options); err != nil {
                w.Close()

diff --git a/engine/pkg/chrootarchive/archive_windows.go b/engine/pkg/chrootarchive/archive_windows.go
index f2973132a3916d3c069abe01fd8234105df47a8e..bd5712c5c04cb7c1f5ed28be9eda00c8df0d8dfc 100644 (file)
--- a/engine/pkg/chrootarchive/archive_windows.go
+++ b/engine/pkg/chrootarchive/archive_windows.go
@@ -14,7 +14,7 @@ func chroot(path string) error {
 
 func invokeUnpack(decompressedArchive io.ReadCloser,
        dest string,
-       options *archive.TarOptions) error {
+       options *archive.TarOptions, root string) error {
        // Windows is different to Linux here because Windows does not support
        // chroot. Hence there is no point sandboxing a chrooted process to
        // do the unpack. We call inline instead within the daemon process.