Browse code

Revert "Add chroot for tar packing operations"

This reverts commit 3e057d527d0c7994ba97c01614d09e95dc7642aa.

Signed-off-by: Tibor Vass <tibor@docker.com>

Tibor Vass authored on 2019/06/12 13:06:14
Showing 7 changed files
... ...
@@ -39,11 +39,11 @@ func extractArchive(i interface{}, src io.Reader, dst string, opts *archive.TarO
39 39
 	return chrootarchive.UntarWithRoot(src, dst, opts, root)
40 40
 }
41 41
 
42
-func archivePath(i interface{}, src string, opts *archive.TarOptions, root string) (io.ReadCloser, error) {
42
+func archivePath(i interface{}, src string, opts *archive.TarOptions) (io.ReadCloser, error) {
43 43
 	if ap, ok := i.(archiver); ok {
44 44
 		return ap.ArchivePath(src, opts)
45 45
 	}
46
-	return chrootarchive.Tar(src, opts, root)
46
+	return archive.TarWithOptions(src, opts)
47 47
 }
48 48
 
49 49
 // ContainerCopy performs a deprecated operation of archiving the resource at
... ...
@@ -239,7 +239,7 @@ func (daemon *Daemon) containerArchivePath(container *container.Container, path
239 239
 	sourceDir, sourceBase := driver.Dir(resolvedPath), driver.Base(resolvedPath)
240 240
 	opts := archive.TarResourceRebaseOpts(sourceBase, driver.Base(absPath))
241 241
 
242
-	data, err := archivePath(driver, sourceDir, opts, container.BaseFS.Path())
242
+	data, err := archivePath(driver, sourceDir, opts)
243 243
 	if err != nil {
244 244
 		return nil, nil, err
245 245
 	}
... ...
@@ -433,7 +433,7 @@ func (daemon *Daemon) containerCopy(container *container.Container, resource str
433 433
 	archive, err := archivePath(driver, basePath, &archive.TarOptions{
434 434
 		Compression:  archive.Uncompressed,
435 435
 		IncludeFiles: filter,
436
-	}, container.BaseFS.Path())
436
+	})
437 437
 	if err != nil {
438 438
 		return nil, err
439 439
 	}
... ...
@@ -70,7 +70,7 @@ func (daemon *Daemon) containerExport(container *container.Container) (arch io.R
70 70
 		Compression: archive.Uncompressed,
71 71
 		UIDMaps:     daemon.idMapping.UIDs(),
72 72
 		GIDMaps:     daemon.idMapping.GIDs(),
73
-	}, basefs.Path())
73
+	})
74 74
 	if err != nil {
75 75
 		rwlayer.Unmount()
76 76
 		return nil, err
... ...
@@ -87,11 +87,3 @@ func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions
87 87
 
88 88
 	return invokeUnpack(r, dest, options, root)
89 89
 }
90
-
91
-// Tar tars the requested path while chrooted to the specified root.
92
-func Tar(srcPath string, options *archive.TarOptions, root string) (io.ReadCloser, error) {
93
-	if options == nil {
94
-		options = &archive.TarOptions{}
95
-	}
96
-	return invokePack(srcPath, options, root)
97
-}
... ...
@@ -12,11 +12,9 @@ import (
12 12
 	"os"
13 13
 	"path/filepath"
14 14
 	"runtime"
15
-	"strings"
16 15
 
17 16
 	"github.com/docker/docker/pkg/archive"
18 17
 	"github.com/docker/docker/pkg/reexec"
19
-	"github.com/pkg/errors"
20 18
 )
21 19
 
22 20
 // untar is the entry-point for docker-untar on re-exec. This is not used on
... ...
@@ -26,7 +24,7 @@ func untar() {
26 26
 	runtime.LockOSThread()
27 27
 	flag.Parse()
28 28
 
29
-	var options archive.TarOptions
29
+	var options *archive.TarOptions
30 30
 
31 31
 	//read the options from the pipe "ExtraFiles"
32 32
 	if err := json.NewDecoder(os.NewFile(3, "options")).Decode(&options); err != nil {
... ...
@@ -47,7 +45,7 @@ func untar() {
47 47
 		fatal(err)
48 48
 	}
49 49
 
50
-	if err := archive.Unpack(os.Stdin, dst, &options); err != nil {
50
+	if err := archive.Unpack(os.Stdin, dst, options); err != nil {
51 51
 		fatal(err)
52 52
 	}
53 53
 	// fully consume stdin in case it is zero padded
... ...
@@ -59,9 +57,6 @@ func untar() {
59 59
 }
60 60
 
61 61
 func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.TarOptions, root string) error {
62
-	if root == "" {
63
-		return errors.New("must specify a root to chroot to")
64
-	}
65 62
 
66 63
 	// We can't pass a potentially large exclude list directly via cmd line
67 64
 	// because we easily overrun the kernel's max argument/environment size
... ...
@@ -117,92 +112,3 @@ func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.T
117 117
 	}
118 118
 	return nil
119 119
 }
120
-
121
-func tar() {
122
-	runtime.LockOSThread()
123
-	flag.Parse()
124
-
125
-	src := flag.Arg(0)
126
-	var root string
127
-	if len(flag.Args()) > 1 {
128
-		root = flag.Arg(1)
129
-	}
130
-
131
-	if root == "" {
132
-		root = src
133
-	}
134
-
135
-	if err := realChroot(root); err != nil {
136
-		fatal(err)
137
-	}
138
-
139
-	var options archive.TarOptions
140
-	if err := json.NewDecoder(os.Stdin).Decode(&options); err != nil {
141
-		fatal(err)
142
-	}
143
-
144
-	rdr, err := archive.TarWithOptions(src, &options)
145
-	if err != nil {
146
-		fatal(err)
147
-	}
148
-	defer rdr.Close()
149
-
150
-	if _, err := io.Copy(os.Stdout, rdr); err != nil {
151
-		fatal(err)
152
-	}
153
-
154
-	os.Exit(0)
155
-}
156
-
157
-func invokePack(srcPath string, options *archive.TarOptions, root string) (io.ReadCloser, error) {
158
-	if root == "" {
159
-		return nil, errors.New("root path must not be empty")
160
-	}
161
-
162
-	relSrc, err := filepath.Rel(root, srcPath)
163
-	if err != nil {
164
-		return nil, err
165
-	}
166
-	if relSrc == "." {
167
-		relSrc = "/"
168
-	}
169
-	if relSrc[0] != '/' {
170
-		relSrc = "/" + relSrc
171
-	}
172
-
173
-	// make sure we didn't trim a trailing slash with the call to `Rel`
174
-	if strings.HasSuffix(srcPath, "/") && !strings.HasSuffix(relSrc, "/") {
175
-		relSrc += "/"
176
-	}
177
-
178
-	cmd := reexec.Command("docker-tar", relSrc, root)
179
-
180
-	errBuff := bytes.NewBuffer(nil)
181
-	cmd.Stderr = errBuff
182
-
183
-	tarR, tarW := io.Pipe()
184
-	cmd.Stdout = tarW
185
-
186
-	stdin, err := cmd.StdinPipe()
187
-	if err != nil {
188
-		return nil, errors.Wrap(err, "error getting options pipe for tar process")
189
-	}
190
-
191
-	if err := cmd.Start(); err != nil {
192
-		return nil, errors.Wrap(err, "tar error on re-exec cmd")
193
-	}
194
-
195
-	go func() {
196
-		err := cmd.Wait()
197
-		err = errors.Wrapf(err, "error processing tar file: %s", errBuff)
198
-		tarW.CloseWithError(err)
199
-	}()
200
-
201
-	if err := json.NewEncoder(stdin).Encode(options); err != nil {
202
-		stdin.Close()
203
-		return nil, errors.Wrap(err, "tar json encode to pipe failed")
204
-	}
205
-	stdin.Close()
206
-
207
-	return tarR, nil
208
-}
... ...
@@ -3,14 +3,11 @@
3 3
 package chrootarchive
4 4
 
5 5
 import (
6
-	gotar "archive/tar"
7 6
 	"bytes"
8 7
 	"io"
9 8
 	"io/ioutil"
10 9
 	"os"
11
-	"path"
12 10
 	"path/filepath"
13
-	"strings"
14 11
 	"testing"
15 12
 
16 13
 	"github.com/docker/docker/pkg/archive"
... ...
@@ -78,94 +75,3 @@ func TestUntarWithMaliciousSymlinks(t *testing.T) {
78 78
 	assert.NilError(t, err)
79 79
 	assert.Equal(t, string(hostData), "pwn3d")
80 80
 }
81
-
82
-// Test for CVE-2018-15664
83
-// Assures that in the case where an "attacker" controlled path is a symlink to
84
-// some path outside of a container's rootfs that we do not unwittingly leak
85
-// host data into the archive.
86
-func TestTarWithMaliciousSymlinks(t *testing.T) {
87
-	dir, err := ioutil.TempDir("", t.Name())
88
-	assert.NilError(t, err)
89
-	// defer os.RemoveAll(dir)
90
-	t.Log(dir)
91
-
92
-	root := filepath.Join(dir, "root")
93
-
94
-	err = os.MkdirAll(root, 0755)
95
-	assert.NilError(t, err)
96
-
97
-	hostFileData := []byte("I am a host file")
98
-
99
-	// Add a file into a directory above root
100
-	// Ensure that we can't access this file while tarring.
101
-	err = ioutil.WriteFile(filepath.Join(dir, "host-file"), hostFileData, 0644)
102
-	assert.NilError(t, err)
103
-
104
-	safe := filepath.Join(root, "safe")
105
-	err = unix.Symlink(dir, safe)
106
-	assert.NilError(t, err)
107
-
108
-	data := filepath.Join(dir, "data")
109
-	err = os.MkdirAll(data, 0755)
110
-	assert.NilError(t, err)
111
-
112
-	type testCase struct {
113
-		p        string
114
-		includes []string
115
-	}
116
-
117
-	cases := []testCase{
118
-		{p: safe, includes: []string{"host-file"}},
119
-		{p: safe + "/", includes: []string{"host-file"}},
120
-		{p: safe, includes: nil},
121
-		{p: safe + "/", includes: nil},
122
-		{p: root, includes: []string{"safe/host-file"}},
123
-		{p: root, includes: []string{"/safe/host-file"}},
124
-		{p: root, includes: nil},
125
-	}
126
-
127
-	maxBytes := len(hostFileData)
128
-
129
-	for _, tc := range cases {
130
-		t.Run(path.Join(tc.p+"_"+strings.Join(tc.includes, "_")), func(t *testing.T) {
131
-			// Here if we use archive.TarWithOptions directly or change the "root" parameter
132
-			// to be the same as "safe", data from the host will be leaked into the archive
133
-			var opts *archive.TarOptions
134
-			if tc.includes != nil {
135
-				opts = &archive.TarOptions{
136
-					IncludeFiles: tc.includes,
137
-				}
138
-			}
139
-			rdr, err := Tar(tc.p, opts, root)
140
-			assert.NilError(t, err)
141
-			defer rdr.Close()
142
-
143
-			tr := gotar.NewReader(rdr)
144
-			assert.Assert(t, !isDataInTar(t, tr, hostFileData, int64(maxBytes)), "host data leaked to archive")
145
-		})
146
-	}
147
-}
148
-
149
-func isDataInTar(t *testing.T, tr *gotar.Reader, compare []byte, maxBytes int64) bool {
150
-	for {
151
-		h, err := tr.Next()
152
-		if err == io.EOF {
153
-			break
154
-		}
155
-		assert.NilError(t, err)
156
-
157
-		if h.Size == 0 {
158
-			continue
159
-		}
160
-		assert.Assert(t, h.Size <= maxBytes, "%s: file size exceeds max expected size %d: %d", h.Name, maxBytes, h.Size)
161
-
162
-		data := make([]byte, int(h.Size))
163
-		_, err = io.ReadFull(tr, data)
164
-		assert.NilError(t, err)
165
-		if bytes.Contains(data, compare) {
166
-			return true
167
-		}
168
-	}
169
-
170
-	return false
171
-}
... ...
@@ -20,10 +20,3 @@ func invokeUnpack(decompressedArchive io.ReadCloser,
20 20
 	// do the unpack. We call inline instead within the daemon process.
21 21
 	return archive.Unpack(decompressedArchive, longpath.AddPrefix(dest), options)
22 22
 }
23
-
24
-func invokePack(srcPath string, options *archive.TarOptions, root string) (io.ReadCloser, error) {
25
-	// Windows is different to Linux here because Windows does not support
26
-	// chroot. Hence there is no point sandboxing a chrooted process to
27
-	// do the pack. We call inline instead within the daemon process.
28
-	return archive.TarWithOptions(srcPath, options)
29
-}
... ...
@@ -14,7 +14,6 @@ import (
14 14
 func init() {
15 15
 	reexec.Register("docker-applyLayer", applyLayer)
16 16
 	reexec.Register("docker-untar", untar)
17
-	reexec.Register("docker-tar", tar)
18 17
 }
19 18
 
20 19
 func fatal(err error) {