Merge pull request #17035 from estesp/fix-build-dir-perms

Correct build-time directory creation with user namespaced daemon

daemon/daemonbuilder/builder.go

@@ -17,10 +17,10 @@ import (
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/httputils"
+	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/parsers"
 	"github.com/docker/docker/pkg/progressreader"
-	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/pkg/urlutil"
 	"github.com/docker/docker/registry"
 	"github.com/docker/docker/runconfig"
@@ -180,7 +180,7 @@ func (d Docker) Copy(c *daemon.Container, destPath string, src builder.FileInfo,
 		destPath = filepath.Join(destPath, filepath.Base(srcPath))
 	}
 
-	if err := system.MkdirAll(filepath.Dir(destPath), 0755); err != nil {
+	if err := idtools.MkdirAllNewAs(filepath.Dir(destPath), 0755, rootUID, rootGID); err != nil {
 		return err
 	}
 	if err := d.Archiver.CopyFileWithTar(srcPath, destPath); err != nil {

pkg/chrootarchive/archive.go

@@ -8,7 +8,7 @@ import (
 	"path/filepath"
 
 	"github.com/docker/docker/pkg/archive"
-	"github.com/docker/docker/pkg/system"
+	"github.com/docker/docker/pkg/idtools"
 )
 
 var chrootArchiver = &archive.Archiver{Untar: Untar}
@@ -41,9 +41,14 @@ func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions
 		options.ExcludePatterns = []string{}
 	}
 
+	rootUID, rootGID, err := idtools.GetRootUIDGID(options.UIDMaps, options.GIDMaps)
+	if err != nil {
+		return err
+	}
+
 	dest = filepath.Clean(dest)
 	if _, err := os.Stat(dest); os.IsNotExist(err) {
-		if err := system.MkdirAll(dest, 0777); err != nil {
+		if err := idtools.MkdirAllNewAs(dest, 0755, rootUID, rootGID); err != nil {
 			return err
 		}
 	}

pkg/idtools/idtools.go

@@ -38,13 +38,20 @@ const (
 // ownership to the requested uid/gid.  If the directory already exists, this
 // function will still change ownership to the requested uid/gid pair.
 func MkdirAllAs(path string, mode os.FileMode, ownerUID, ownerGID int) error {
-	return mkdirAs(path, mode, ownerUID, ownerGID, true)
+	return mkdirAs(path, mode, ownerUID, ownerGID, true, true)
+}
+
+// MkdirAllNewAs creates a directory (include any along the path) and then modifies
+// ownership ONLY of newly created directories to the requested uid/gid. If the
+// directories along the path exist, no change of ownership will be performed
+func MkdirAllNewAs(path string, mode os.FileMode, ownerUID, ownerGID int) error {
+	return mkdirAs(path, mode, ownerUID, ownerGID, true, false)
 }
 
 // MkdirAs creates a directory and then modifies ownership to the requested uid/gid.
 // If the directory already exists, this function still changes ownership
 func MkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int) error {
-	return mkdirAs(path, mode, ownerUID, ownerGID, false)
+	return mkdirAs(path, mode, ownerUID, ownerGID, false, true)
 }
 
 // GetRootUIDGID retrieves the remapped root uid/gid pair from the set of maps.

pkg/idtools/idtools_unix.go

@@ -0,0 +1,60 @@
+// +build !windows
+
+package idtools
+
+import (
+	"os"
+	"path/filepath"
+
+	"github.com/docker/docker/pkg/system"
+)
+
+func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll, chownExisting bool) error {
+	// make an array containing the original path asked for, plus (for mkAll == true)
+	// all path components leading up to the complete path that don't exist before we MkdirAll
+	// so that we can chown all of them properly at the end.  If chownExisting is false, we won't
+	// chown the full directory path if it exists
+	var paths []string
+	if _, err := os.Stat(path); err != nil && os.IsNotExist(err) {
+		paths = []string{path}
+	} else if err == nil && chownExisting {
+		if err := os.Chown(path, ownerUID, ownerGID); err != nil {
+			return err
+		}
+		// short-circuit--we were called with an existing directory and chown was requested
+		return nil
+	} else if err == nil {
+		// nothing to do; directory path fully exists already and chown was NOT requested
+		return nil
+	}
+
+	if mkAll {
+		// walk back to "/" looking for directories which do not exist
+		// and add them to the paths array for chown after creation
+		dirPath := path
+		for {
+			dirPath = filepath.Dir(dirPath)
+			if dirPath == "/" {
+				break
+			}
+			if _, err := os.Stat(dirPath); err != nil && os.IsNotExist(err) {
+				paths = append(paths, dirPath)
+			}
+		}
+		if err := system.MkdirAll(path, mode); err != nil && !os.IsExist(err) {
+			return err
+		}
+	} else {
+		if err := os.Mkdir(path, mode); err != nil && !os.IsExist(err) {
+			return err
+		}
+	}
+	// even if it existed, we will chown the requested path + any subpaths that
+	// didn't exist when we called MkdirAll
+	for _, pathComponent := range paths {
+		if err := os.Chown(pathComponent, ownerUID, ownerGID); err != nil {
+			return err
+		}
+	}
+	return nil
+}

pkg/idtools/idtools_unix_test.go

@@ -0,0 +1,243 @@
+// +build !windows
+
+package idtools
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"syscall"
+	"testing"
+)
+
+type node struct {
+	uid int
+	gid int
+}
+
+func TestMkdirAllAs(t *testing.T) {
+	dirName, err := ioutil.TempDir("", "mkdirall")
+	if err != nil {
+		t.Fatalf("Couldn't create temp dir: %v", err)
+	}
+	defer os.RemoveAll(dirName)
+
+	testTree := map[string]node{
+		"usr":              {0, 0},
+		"usr/bin":          {0, 0},
+		"lib":              {33, 33},
+		"lib/x86_64":       {45, 45},
+		"lib/x86_64/share": {1, 1},
+	}
+
+	if err := buildTree(dirName, testTree); err != nil {
+		t.Fatal(err)
+	}
+
+	// test adding a directory to a pre-existing dir; only the new dir is owned by the uid/gid
+	if err := MkdirAllAs(filepath.Join(dirName, "usr", "share"), 0755, 99, 99); err != nil {
+		t.Fatal(err)
+	}
+	testTree["usr/share"] = node{99, 99}
+	verifyTree, err := readTree(dirName, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := compareTrees(testTree, verifyTree); err != nil {
+		t.Fatal(err)
+	}
+
+	// test 2-deep new directories--both should be owned by the uid/gid pair
+	if err := MkdirAllAs(filepath.Join(dirName, "lib", "some", "other"), 0755, 101, 101); err != nil {
+		t.Fatal(err)
+	}
+	testTree["lib/some"] = node{101, 101}
+	testTree["lib/some/other"] = node{101, 101}
+	verifyTree, err = readTree(dirName, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := compareTrees(testTree, verifyTree); err != nil {
+		t.Fatal(err)
+	}
+
+	// test a directory that already exists; should be chowned, but nothing else
+	if err := MkdirAllAs(filepath.Join(dirName, "usr"), 0755, 102, 102); err != nil {
+		t.Fatal(err)
+	}
+	testTree["usr"] = node{102, 102}
+	verifyTree, err = readTree(dirName, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := compareTrees(testTree, verifyTree); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestMkdirAllNewAs(t *testing.T) {
+
+	dirName, err := ioutil.TempDir("", "mkdirnew")
+	if err != nil {
+		t.Fatalf("Couldn't create temp dir: %v", err)
+	}
+	defer os.RemoveAll(dirName)
+
+	testTree := map[string]node{
+		"usr":              {0, 0},
+		"usr/bin":          {0, 0},
+		"lib":              {33, 33},
+		"lib/x86_64":       {45, 45},
+		"lib/x86_64/share": {1, 1},
+	}
+
+	if err := buildTree(dirName, testTree); err != nil {
+		t.Fatal(err)
+	}
+
+	// test adding a directory to a pre-existing dir; only the new dir is owned by the uid/gid
+	if err := MkdirAllNewAs(filepath.Join(dirName, "usr", "share"), 0755, 99, 99); err != nil {
+		t.Fatal(err)
+	}
+	testTree["usr/share"] = node{99, 99}
+	verifyTree, err := readTree(dirName, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := compareTrees(testTree, verifyTree); err != nil {
+		t.Fatal(err)
+	}
+
+	// test 2-deep new directories--both should be owned by the uid/gid pair
+	if err := MkdirAllNewAs(filepath.Join(dirName, "lib", "some", "other"), 0755, 101, 101); err != nil {
+		t.Fatal(err)
+	}
+	testTree["lib/some"] = node{101, 101}
+	testTree["lib/some/other"] = node{101, 101}
+	verifyTree, err = readTree(dirName, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := compareTrees(testTree, verifyTree); err != nil {
+		t.Fatal(err)
+	}
+
+	// test a directory that already exists; should NOT be chowned
+	if err := MkdirAllNewAs(filepath.Join(dirName, "usr"), 0755, 102, 102); err != nil {
+		t.Fatal(err)
+	}
+	verifyTree, err = readTree(dirName, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := compareTrees(testTree, verifyTree); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestMkdirAs(t *testing.T) {
+
+	dirName, err := ioutil.TempDir("", "mkdir")
+	if err != nil {
+		t.Fatalf("Couldn't create temp dir: %v", err)
+	}
+	defer os.RemoveAll(dirName)
+
+	testTree := map[string]node{
+		"usr": {0, 0},
+	}
+	if err := buildTree(dirName, testTree); err != nil {
+		t.Fatal(err)
+	}
+
+	// test a directory that already exists; should just chown to the requested uid/gid
+	if err := MkdirAs(filepath.Join(dirName, "usr"), 0755, 99, 99); err != nil {
+		t.Fatal(err)
+	}
+	testTree["usr"] = node{99, 99}
+	verifyTree, err := readTree(dirName, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := compareTrees(testTree, verifyTree); err != nil {
+		t.Fatal(err)
+	}
+
+	// create a subdir under a dir which doesn't exist--should fail
+	if err := MkdirAs(filepath.Join(dirName, "usr", "bin", "subdir"), 0755, 102, 102); err == nil {
+		t.Fatalf("Trying to create a directory with Mkdir where the parent doesn't exist should have failed")
+	}
+
+	// create a subdir under an existing dir; should only change the ownership of the new subdir
+	if err := MkdirAs(filepath.Join(dirName, "usr", "bin"), 0755, 102, 102); err != nil {
+		t.Fatal(err)
+	}
+	testTree["usr/bin"] = node{102, 102}
+	verifyTree, err = readTree(dirName, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := compareTrees(testTree, verifyTree); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func buildTree(base string, tree map[string]node) error {
+	for path, node := range tree {
+		fullPath := filepath.Join(base, path)
+		if err := os.MkdirAll(fullPath, 0755); err != nil {
+			return fmt.Errorf("Couldn't create path: %s; error: %v", fullPath, err)
+		}
+		if err := os.Chown(fullPath, node.uid, node.gid); err != nil {
+			return fmt.Errorf("Couldn't chown path: %s; error: %v", fullPath, err)
+		}
+	}
+	return nil
+}
+
+func readTree(base, root string) (map[string]node, error) {
+	tree := make(map[string]node)
+
+	dirInfos, err := ioutil.ReadDir(base)
+	if err != nil {
+		return nil, fmt.Errorf("Couldn't read directory entries for %q: %v", base, err)
+	}
+
+	for _, info := range dirInfos {
+		s := &syscall.Stat_t{}
+		if err := syscall.Stat(filepath.Join(base, info.Name()), s); err != nil {
+			return nil, fmt.Errorf("Can't stat file %q: %v", filepath.Join(base, info.Name()), err)
+		}
+		tree[filepath.Join(root, info.Name())] = node{int(s.Uid), int(s.Gid)}
+		if info.IsDir() {
+			// read the subdirectory
+			subtree, err := readTree(filepath.Join(base, info.Name()), filepath.Join(root, info.Name()))
+			if err != nil {
+				return nil, err
+			}
+			for path, nodeinfo := range subtree {
+				tree[path] = nodeinfo
+			}
+		}
+	}
+	return tree, nil
+}
+
+func compareTrees(left, right map[string]node) error {
+	if len(left) != len(right) {
+		return fmt.Errorf("Trees aren't the same size")
+	}
+	for path, nodeLeft := range left {
+		if nodeRight, ok := right[path]; ok {
+			if nodeRight.uid != nodeLeft.uid || nodeRight.gid != nodeLeft.gid {
+				// mismatch
+				return fmt.Errorf("mismatched ownership for %q: expected: %d:%d, got: %d:%d", path,
+					nodeLeft.uid, nodeLeft.gid, nodeRight.uid, nodeRight.gid)
+			}
+			continue
+		}
+		return fmt.Errorf("right tree didn't contain path %q", path)
+	}
+	return nil
+}

pkg/idtools/idtools_windows.go

@@ -0,0 +1,18 @@
+// +build windows
+
+package idtools
+
+import (
+	"os"
+
+	"github.com/docker/docker/pkg/system"
+)
+
+// Platforms such as Windows do not support the UID/GID concept. So make this
+// just a wrapper around system.MkdirAll.
+func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll, chownExisting bool) error {
+	if err := system.MkdirAll(path, mode); err != nil && !os.IsExist(err) {
+		return err
+	}
+	return nil
+}

pkg/idtools/usergroupadd_linux.go

@@ -2,13 +2,10 @@ package idtools
 
 import (
 	"fmt"
-	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
 	"syscall"
-
-	"github.com/docker/docker/pkg/system"
 )
 
 // add a user and/or group to Linux /etc/passwd, /etc/group using standard
@@ -156,20 +153,3 @@ func findUnused(file string, id int) (int, error) {
 		}
 	}
 }
-
-func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll bool) error {
-	if mkAll {
-		if err := system.MkdirAll(path, mode); err != nil && !os.IsExist(err) {
-			return err
-		}
-	} else {
-		if err := os.Mkdir(path, mode); err != nil && !os.IsExist(err) {
-			return err
-		}
-	}
-	// even if it existed, we will chown to change ownership as requested
-	if err := os.Chown(path, ownerUID, ownerGID); err != nil {
-		return err
-	}
-	return nil
-}

pkg/idtools/usergroupadd_unsupported.go

@@ -2,12 +2,7 @@
 
 package idtools
 
-import (
-	"fmt"
-	"os"
-
-	"github.com/docker/docker/pkg/system"
-)
+import "fmt"
 
 // AddNamespaceRangesUser takes a name and finds an unused uid, gid pair
 // and calls the appropriate helper function to add the group and then
@@ -15,12 +10,3 @@ import (
 func AddNamespaceRangesUser(name string) (int, int, error) {
 	return -1, -1, fmt.Errorf("No support for adding users or groups on this OS")
 }
-
-// Platforms such as Windows do not support the UID/GID concept. So make this
-// just a wrapper around system.MkdirAll.
-func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll bool) error {
-	if err := system.MkdirAll(path, mode); err != nil && !os.IsExist(err) {
-		return err
-	}
-	return nil
-}