Browse code

Correct build-time directory creation with user namespaced daemon

This fixes errors in ownership on directory creation during build that
can cause inaccessible files depending on the paths in the Dockerfile
and non-existing directories in the starting image.

Add tests for the mkdir variants in pkg/idtools

Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)

Phil Estes authored on 2015/10/15 03:35:48
Showing 8 changed files
... ...
@@ -17,10 +17,10 @@ import (
17 17
 	"github.com/docker/docker/image"
18 18
 	"github.com/docker/docker/pkg/archive"
19 19
 	"github.com/docker/docker/pkg/httputils"
20
+	"github.com/docker/docker/pkg/idtools"
20 21
 	"github.com/docker/docker/pkg/ioutils"
21 22
 	"github.com/docker/docker/pkg/parsers"
22 23
 	"github.com/docker/docker/pkg/progressreader"
23
-	"github.com/docker/docker/pkg/system"
24 24
 	"github.com/docker/docker/pkg/urlutil"
25 25
 	"github.com/docker/docker/registry"
26 26
 	"github.com/docker/docker/runconfig"
... ...
@@ -180,7 +180,7 @@ func (d Docker) Copy(c *daemon.Container, destPath string, src builder.FileInfo,
180 180
 		destPath = filepath.Join(destPath, filepath.Base(srcPath))
181 181
 	}
182 182
 
183
-	if err := system.MkdirAll(filepath.Dir(destPath), 0755); err != nil {
183
+	if err := idtools.MkdirAllNewAs(filepath.Dir(destPath), 0755, rootUID, rootGID); err != nil {
184 184
 		return err
185 185
 	}
186 186
 	if err := d.Archiver.CopyFileWithTar(srcPath, destPath); err != nil {
... ...
@@ -8,7 +8,7 @@ import (
8 8
 	"path/filepath"
9 9
 
10 10
 	"github.com/docker/docker/pkg/archive"
11
-	"github.com/docker/docker/pkg/system"
11
+	"github.com/docker/docker/pkg/idtools"
12 12
 )
13 13
 
14 14
 var chrootArchiver = &archive.Archiver{Untar: Untar}
... ...
@@ -41,9 +41,14 @@ func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions
41 41
 		options.ExcludePatterns = []string{}
42 42
 	}
43 43
 
44
+	rootUID, rootGID, err := idtools.GetRootUIDGID(options.UIDMaps, options.GIDMaps)
45
+	if err != nil {
46
+		return err
47
+	}
48
+
44 49
 	dest = filepath.Clean(dest)
45 50
 	if _, err := os.Stat(dest); os.IsNotExist(err) {
46
-		if err := system.MkdirAll(dest, 0777); err != nil {
51
+		if err := idtools.MkdirAllNewAs(dest, 0755, rootUID, rootGID); err != nil {
47 52
 			return err
48 53
 		}
49 54
 	}
... ...
@@ -38,13 +38,20 @@ const (
38 38
 // ownership to the requested uid/gid.  If the directory already exists, this
39 39
 // function will still change ownership to the requested uid/gid pair.
40 40
 func MkdirAllAs(path string, mode os.FileMode, ownerUID, ownerGID int) error {
41
-	return mkdirAs(path, mode, ownerUID, ownerGID, true)
41
+	return mkdirAs(path, mode, ownerUID, ownerGID, true, true)
42
+}
43
+
44
+// MkdirAllNewAs creates a directory (include any along the path) and then modifies
45
+// ownership ONLY of newly created directories to the requested uid/gid. If the
46
+// directories along the path exist, no change of ownership will be performed
47
+func MkdirAllNewAs(path string, mode os.FileMode, ownerUID, ownerGID int) error {
48
+	return mkdirAs(path, mode, ownerUID, ownerGID, true, false)
42 49
 }
43 50
 
44 51
 // MkdirAs creates a directory and then modifies ownership to the requested uid/gid.
45 52
 // If the directory already exists, this function still changes ownership
46 53
 func MkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int) error {
47
-	return mkdirAs(path, mode, ownerUID, ownerGID, false)
54
+	return mkdirAs(path, mode, ownerUID, ownerGID, false, true)
48 55
 }
49 56
 
50 57
 // GetRootUIDGID retrieves the remapped root uid/gid pair from the set of maps.
51 58
new file mode 100644
... ...
@@ -0,0 +1,60 @@
0
+// +build !windows
1
+
2
+package idtools
3
+
4
+import (
5
+	"os"
6
+	"path/filepath"
7
+
8
+	"github.com/docker/docker/pkg/system"
9
+)
10
+
11
+func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll, chownExisting bool) error {
12
+	// make an array containing the original path asked for, plus (for mkAll == true)
13
+	// all path components leading up to the complete path that don't exist before we MkdirAll
14
+	// so that we can chown all of them properly at the end.  If chownExisting is false, we won't
15
+	// chown the full directory path if it exists
16
+	var paths []string
17
+	if _, err := os.Stat(path); err != nil && os.IsNotExist(err) {
18
+		paths = []string{path}
19
+	} else if err == nil && chownExisting {
20
+		if err := os.Chown(path, ownerUID, ownerGID); err != nil {
21
+			return err
22
+		}
23
+		// short-circuit--we were called with an existing directory and chown was requested
24
+		return nil
25
+	} else if err == nil {
26
+		// nothing to do; directory path fully exists already and chown was NOT requested
27
+		return nil
28
+	}
29
+
30
+	if mkAll {
31
+		// walk back to "/" looking for directories which do not exist
32
+		// and add them to the paths array for chown after creation
33
+		dirPath := path
34
+		for {
35
+			dirPath = filepath.Dir(dirPath)
36
+			if dirPath == "/" {
37
+				break
38
+			}
39
+			if _, err := os.Stat(dirPath); err != nil && os.IsNotExist(err) {
40
+				paths = append(paths, dirPath)
41
+			}
42
+		}
43
+		if err := system.MkdirAll(path, mode); err != nil && !os.IsExist(err) {
44
+			return err
45
+		}
46
+	} else {
47
+		if err := os.Mkdir(path, mode); err != nil && !os.IsExist(err) {
48
+			return err
49
+		}
50
+	}
51
+	// even if it existed, we will chown the requested path + any subpaths that
52
+	// didn't exist when we called MkdirAll
53
+	for _, pathComponent := range paths {
54
+		if err := os.Chown(pathComponent, ownerUID, ownerGID); err != nil {
55
+			return err
56
+		}
57
+	}
58
+	return nil
59
+}
0 60
new file mode 100644
... ...
@@ -0,0 +1,243 @@
0
+// +build !windows
1
+
2
+package idtools
3
+
4
+import (
5
+	"fmt"
6
+	"io/ioutil"
7
+	"os"
8
+	"path/filepath"
9
+	"syscall"
10
+	"testing"
11
+)
12
+
13
+type node struct {
14
+	uid int
15
+	gid int
16
+}
17
+
18
+func TestMkdirAllAs(t *testing.T) {
19
+	dirName, err := ioutil.TempDir("", "mkdirall")
20
+	if err != nil {
21
+		t.Fatalf("Couldn't create temp dir: %v", err)
22
+	}
23
+	defer os.RemoveAll(dirName)
24
+
25
+	testTree := map[string]node{
26
+		"usr":              {0, 0},
27
+		"usr/bin":          {0, 0},
28
+		"lib":              {33, 33},
29
+		"lib/x86_64":       {45, 45},
30
+		"lib/x86_64/share": {1, 1},
31
+	}
32
+
33
+	if err := buildTree(dirName, testTree); err != nil {
34
+		t.Fatal(err)
35
+	}
36
+
37
+	// test adding a directory to a pre-existing dir; only the new dir is owned by the uid/gid
38
+	if err := MkdirAllAs(filepath.Join(dirName, "usr", "share"), 0755, 99, 99); err != nil {
39
+		t.Fatal(err)
40
+	}
41
+	testTree["usr/share"] = node{99, 99}
42
+	verifyTree, err := readTree(dirName, "")
43
+	if err != nil {
44
+		t.Fatal(err)
45
+	}
46
+	if err := compareTrees(testTree, verifyTree); err != nil {
47
+		t.Fatal(err)
48
+	}
49
+
50
+	// test 2-deep new directories--both should be owned by the uid/gid pair
51
+	if err := MkdirAllAs(filepath.Join(dirName, "lib", "some", "other"), 0755, 101, 101); err != nil {
52
+		t.Fatal(err)
53
+	}
54
+	testTree["lib/some"] = node{101, 101}
55
+	testTree["lib/some/other"] = node{101, 101}
56
+	verifyTree, err = readTree(dirName, "")
57
+	if err != nil {
58
+		t.Fatal(err)
59
+	}
60
+	if err := compareTrees(testTree, verifyTree); err != nil {
61
+		t.Fatal(err)
62
+	}
63
+
64
+	// test a directory that already exists; should be chowned, but nothing else
65
+	if err := MkdirAllAs(filepath.Join(dirName, "usr"), 0755, 102, 102); err != nil {
66
+		t.Fatal(err)
67
+	}
68
+	testTree["usr"] = node{102, 102}
69
+	verifyTree, err = readTree(dirName, "")
70
+	if err != nil {
71
+		t.Fatal(err)
72
+	}
73
+	if err := compareTrees(testTree, verifyTree); err != nil {
74
+		t.Fatal(err)
75
+	}
76
+}
77
+
78
+func TestMkdirAllNewAs(t *testing.T) {
79
+
80
+	dirName, err := ioutil.TempDir("", "mkdirnew")
81
+	if err != nil {
82
+		t.Fatalf("Couldn't create temp dir: %v", err)
83
+	}
84
+	defer os.RemoveAll(dirName)
85
+
86
+	testTree := map[string]node{
87
+		"usr":              {0, 0},
88
+		"usr/bin":          {0, 0},
89
+		"lib":              {33, 33},
90
+		"lib/x86_64":       {45, 45},
91
+		"lib/x86_64/share": {1, 1},
92
+	}
93
+
94
+	if err := buildTree(dirName, testTree); err != nil {
95
+		t.Fatal(err)
96
+	}
97
+
98
+	// test adding a directory to a pre-existing dir; only the new dir is owned by the uid/gid
99
+	if err := MkdirAllNewAs(filepath.Join(dirName, "usr", "share"), 0755, 99, 99); err != nil {
100
+		t.Fatal(err)
101
+	}
102
+	testTree["usr/share"] = node{99, 99}
103
+	verifyTree, err := readTree(dirName, "")
104
+	if err != nil {
105
+		t.Fatal(err)
106
+	}
107
+	if err := compareTrees(testTree, verifyTree); err != nil {
108
+		t.Fatal(err)
109
+	}
110
+
111
+	// test 2-deep new directories--both should be owned by the uid/gid pair
112
+	if err := MkdirAllNewAs(filepath.Join(dirName, "lib", "some", "other"), 0755, 101, 101); err != nil {
113
+		t.Fatal(err)
114
+	}
115
+	testTree["lib/some"] = node{101, 101}
116
+	testTree["lib/some/other"] = node{101, 101}
117
+	verifyTree, err = readTree(dirName, "")
118
+	if err != nil {
119
+		t.Fatal(err)
120
+	}
121
+	if err := compareTrees(testTree, verifyTree); err != nil {
122
+		t.Fatal(err)
123
+	}
124
+
125
+	// test a directory that already exists; should NOT be chowned
126
+	if err := MkdirAllNewAs(filepath.Join(dirName, "usr"), 0755, 102, 102); err != nil {
127
+		t.Fatal(err)
128
+	}
129
+	verifyTree, err = readTree(dirName, "")
130
+	if err != nil {
131
+		t.Fatal(err)
132
+	}
133
+	if err := compareTrees(testTree, verifyTree); err != nil {
134
+		t.Fatal(err)
135
+	}
136
+}
137
+
138
+func TestMkdirAs(t *testing.T) {
139
+
140
+	dirName, err := ioutil.TempDir("", "mkdir")
141
+	if err != nil {
142
+		t.Fatalf("Couldn't create temp dir: %v", err)
143
+	}
144
+	defer os.RemoveAll(dirName)
145
+
146
+	testTree := map[string]node{
147
+		"usr": {0, 0},
148
+	}
149
+	if err := buildTree(dirName, testTree); err != nil {
150
+		t.Fatal(err)
151
+	}
152
+
153
+	// test a directory that already exists; should just chown to the requested uid/gid
154
+	if err := MkdirAs(filepath.Join(dirName, "usr"), 0755, 99, 99); err != nil {
155
+		t.Fatal(err)
156
+	}
157
+	testTree["usr"] = node{99, 99}
158
+	verifyTree, err := readTree(dirName, "")
159
+	if err != nil {
160
+		t.Fatal(err)
161
+	}
162
+	if err := compareTrees(testTree, verifyTree); err != nil {
163
+		t.Fatal(err)
164
+	}
165
+
166
+	// create a subdir under a dir which doesn't exist--should fail
167
+	if err := MkdirAs(filepath.Join(dirName, "usr", "bin", "subdir"), 0755, 102, 102); err == nil {
168
+		t.Fatalf("Trying to create a directory with Mkdir where the parent doesn't exist should have failed")
169
+	}
170
+
171
+	// create a subdir under an existing dir; should only change the ownership of the new subdir
172
+	if err := MkdirAs(filepath.Join(dirName, "usr", "bin"), 0755, 102, 102); err != nil {
173
+		t.Fatal(err)
174
+	}
175
+	testTree["usr/bin"] = node{102, 102}
176
+	verifyTree, err = readTree(dirName, "")
177
+	if err != nil {
178
+		t.Fatal(err)
179
+	}
180
+	if err := compareTrees(testTree, verifyTree); err != nil {
181
+		t.Fatal(err)
182
+	}
183
+}
184
+
185
+func buildTree(base string, tree map[string]node) error {
186
+	for path, node := range tree {
187
+		fullPath := filepath.Join(base, path)
188
+		if err := os.MkdirAll(fullPath, 0755); err != nil {
189
+			return fmt.Errorf("Couldn't create path: %s; error: %v", fullPath, err)
190
+		}
191
+		if err := os.Chown(fullPath, node.uid, node.gid); err != nil {
192
+			return fmt.Errorf("Couldn't chown path: %s; error: %v", fullPath, err)
193
+		}
194
+	}
195
+	return nil
196
+}
197
+
198
+func readTree(base, root string) (map[string]node, error) {
199
+	tree := make(map[string]node)
200
+
201
+	dirInfos, err := ioutil.ReadDir(base)
202
+	if err != nil {
203
+		return nil, fmt.Errorf("Couldn't read directory entries for %q: %v", base, err)
204
+	}
205
+
206
+	for _, info := range dirInfos {
207
+		s := &syscall.Stat_t{}
208
+		if err := syscall.Stat(filepath.Join(base, info.Name()), s); err != nil {
209
+			return nil, fmt.Errorf("Can't stat file %q: %v", filepath.Join(base, info.Name()), err)
210
+		}
211
+		tree[filepath.Join(root, info.Name())] = node{int(s.Uid), int(s.Gid)}
212
+		if info.IsDir() {
213
+			// read the subdirectory
214
+			subtree, err := readTree(filepath.Join(base, info.Name()), filepath.Join(root, info.Name()))
215
+			if err != nil {
216
+				return nil, err
217
+			}
218
+			for path, nodeinfo := range subtree {
219
+				tree[path] = nodeinfo
220
+			}
221
+		}
222
+	}
223
+	return tree, nil
224
+}
225
+
226
+func compareTrees(left, right map[string]node) error {
227
+	if len(left) != len(right) {
228
+		return fmt.Errorf("Trees aren't the same size")
229
+	}
230
+	for path, nodeLeft := range left {
231
+		if nodeRight, ok := right[path]; ok {
232
+			if nodeRight.uid != nodeLeft.uid || nodeRight.gid != nodeLeft.gid {
233
+				// mismatch
234
+				return fmt.Errorf("mismatched ownership for %q: expected: %d:%d, got: %d:%d", path,
235
+					nodeLeft.uid, nodeLeft.gid, nodeRight.uid, nodeRight.gid)
236
+			}
237
+			continue
238
+		}
239
+		return fmt.Errorf("right tree didn't contain path %q", path)
240
+	}
241
+	return nil
242
+}
0 243
new file mode 100644
... ...
@@ -0,0 +1,18 @@
0
+// +build windows
1
+
2
+package idtools
3
+
4
+import (
5
+	"os"
6
+
7
+	"github.com/docker/docker/pkg/system"
8
+)
9
+
10
+// Platforms such as Windows do not support the UID/GID concept. So make this
11
+// just a wrapper around system.MkdirAll.
12
+func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll, chownExisting bool) error {
13
+	if err := system.MkdirAll(path, mode); err != nil && !os.IsExist(err) {
14
+		return err
15
+	}
16
+	return nil
17
+}
... ...
@@ -2,13 +2,10 @@ package idtools
2 2
 
3 3
 import (
4 4
 	"fmt"
5
-	"os"
6 5
 	"os/exec"
7 6
 	"path/filepath"
8 7
 	"strings"
9 8
 	"syscall"
10
-
11
-	"github.com/docker/docker/pkg/system"
12 9
 )
13 10
 
14 11
 // 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) {
156 156
 		}
157 157
 	}
158 158
 }
159
-
160
-func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll bool) error {
161
-	if mkAll {
162
-		if err := system.MkdirAll(path, mode); err != nil && !os.IsExist(err) {
163
-			return err
164
-		}
165
-	} else {
166
-		if err := os.Mkdir(path, mode); err != nil && !os.IsExist(err) {
167
-			return err
168
-		}
169
-	}
170
-	// even if it existed, we will chown to change ownership as requested
171
-	if err := os.Chown(path, ownerUID, ownerGID); err != nil {
172
-		return err
173
-	}
174
-	return nil
175
-}
... ...
@@ -2,12 +2,7 @@
2 2
 
3 3
 package idtools
4 4
 
5
-import (
6
-	"fmt"
7
-	"os"
8
-
9
-	"github.com/docker/docker/pkg/system"
10
-)
5
+import "fmt"
11 6
 
12 7
 // AddNamespaceRangesUser takes a name and finds an unused uid, gid pair
13 8
 // and calls the appropriate helper function to add the group and then
... ...
@@ -15,12 +10,3 @@ import (
15 15
 func AddNamespaceRangesUser(name string) (int, int, error) {
16 16
 	return -1, -1, fmt.Errorf("No support for adding users or groups on this OS")
17 17
 }
18
-
19
-// Platforms such as Windows do not support the UID/GID concept. So make this
20
-// just a wrapper around system.MkdirAll.
21
-func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll bool) error {
22
-	if err := system.MkdirAll(path, mode); err != nil && !os.IsExist(err) {
23
-		return err
24
-	}
25
-	return nil
26
-}