builder: make ADD and COPY obey the USER directive

daemon/archive.go

@@ -5,7 +5,9 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
+	"syscall"
 
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/container"
@@ -14,6 +16,7 @@ import (
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/engine-api/types"
+	"github.com/opencontainers/runc/libcontainer/user"
 )
 
 // ErrExtractPointNotDirectory is used to convey that the operation to extract
@@ -331,6 +334,53 @@ func (daemon *Daemon) containerCopy(container *container.Container, resource str
 	return reader, nil
 }
 
+// GetContainerUser evaluates the uid:gid of the given USER-style string using
+// the container's /etc/{passwd,group} files.
+func (daemon *Daemon) GetContainerUser(c *container.Container, userSpec string) (int, int, error) {
+	// Use same defaults as libcontainer. An empty userSpec will result in the
+	// following settings being returned.
+	defaultExecUser := user.ExecUser{
+		Uid:  syscall.Getuid(),
+		Gid:  syscall.Getgid(),
+		Home: "/",
+	}
+
+	// If we're on windows, don't bother with anything since we can't chown()
+	// anyway.
+	if runtime.GOOS == "windows" {
+		return defaultExecUser.Uid, defaultExecUser.Gid, nil
+	}
+
+	// Get the right path.
+	passwdPath, err := user.GetPasswdPath()
+	if err != nil {
+		return -1, -1, err
+	}
+	// Scope it to the container's filesystem.
+	passwdPath, err = c.GetResourcePath(passwdPath)
+	if err != nil {
+		return -1, -1, err
+	}
+
+	// Get the right path.
+	groupPath, err := user.GetGroupPath()
+	if err != nil {
+		return -1, -1, err
+	}
+	// Scope it to the container's filesystem.
+	groupPath, err = c.GetResourcePath(groupPath)
+	if err != nil {
+		return -1, -1, err
+	}
+
+	execUser, err := user.GetExecUserPath(userSpec, &defaultExecUser, passwdPath, groupPath)
+	if err != nil {
+		return -1, -1, err
+	}
+
+	return execUser.Uid, execUser.Gid, nil
+}
+
 // CopyOnBuild copies/extracts a source FileInfo to a destination path inside a container
 // specified by a container object.
 // TODO: make sure callers don't unnecessarily convert destPath with filepath.FromSlash (Copy does it already).
@@ -339,7 +389,6 @@ func (daemon *Daemon) CopyOnBuild(cID string, destPath string, src builder.FileI
 	srcPath := src.Path()
 	destExists := true
 	destDir := false
-	rootUID, rootGID := daemon.GetRemappedUIDGID()
 
 	// Work in daemon-local OS specific file paths
 	destPath = filepath.FromSlash(destPath)
@@ -377,7 +426,25 @@ func (daemon *Daemon) CopyOnBuild(cID string, destPath string, src builder.FileI
 		destExists = false
 	}
 
+	// Get the mappings required to convert container <-> host.
 	uidMaps, gidMaps := daemon.GetUIDGIDMaps()
+
+	// Get the UID and GID we should copy as, resolved to the host.
+	// This results in the USER directive correctly affecting the owner of all
+	// files ADD'd and COPY'd during a build (with the correct userns mapping).
+	copyUID, copyGID, err := daemon.GetContainerUser(c, c.Config.User)
+	if err != nil {
+		return err
+	}
+	copyUID, err = idtools.ToHost(copyUID, uidMaps)
+	if err != nil {
+		return err
+	}
+	copyGID, err = idtools.ToHost(copyGID, gidMaps)
+	if err != nil {
+		return err
+	}
+
 	archiver := &archive.Archiver{
 		Untar:   chrootarchive.Untar,
 		UIDMaps: uidMaps,
@@ -389,7 +456,7 @@ func (daemon *Daemon) CopyOnBuild(cID string, destPath string, src builder.FileI
 		if err := archiver.CopyWithTar(srcPath, destPath); err != nil {
 			return err
 		}
-		return fixPermissions(srcPath, destPath, rootUID, rootGID, destExists)
+		return fixPermissions(srcPath, destPath, copyUID, copyGID, destExists)
 	}
 	if decompress && archive.IsArchivePath(srcPath) {
 		// Only try to untar if it is a file and that we've been told to decompress (when ADD-ing a remote file)
@@ -418,12 +485,12 @@ func (daemon *Daemon) CopyOnBuild(cID string, destPath string, src builder.FileI
 		destPath = filepath.Join(destPath, src.Name())
 	}
 
-	if err := idtools.MkdirAllNewAs(filepath.Dir(destPath), 0755, rootUID, rootGID); err != nil {
+	if err := idtools.MkdirAllNewAs(filepath.Dir(destPath), 0755, copyUID, copyGID); err != nil {
 		return err
 	}
 	if err := archiver.CopyFileWithTar(srcPath, destPath); err != nil {
 		return err
 	}
 
-	return fixPermissions(srcPath, destPath, rootUID, rootGID, destExists)
+	return fixPermissions(srcPath, destPath, copyUID, copyGID, destExists)
 }

docs/reference/builder.md

@@ -581,7 +581,11 @@ the source will be copied inside the destination container.
     ADD test relativeDir/          # adds "test" to `WORKDIR`/relativeDir/
     ADD test /absoluteDir/         # adds "test" to /absoluteDir/
 
-All new files and directories are created with a UID and GID of 0.
+All new files and directories are created with the owner being the same user as
+specified by the `USER` directive. If the user has not been set the default is
+the same as `RUN`, the user with a UID and GID of 0. The only exception to this
+being the unpacking of archives, which retains the owners of the files in the
+archive.
 
 In the case where `<src>` is a remote file URL, the destination will
 have permissions of 600. If the remote file being retrieved has an HTTP
@@ -693,7 +697,9 @@ the source will be copied inside the destination container.
     COPY test relativeDir/   # adds "test" to `WORKDIR`/relativeDir/
     COPY test /absoluteDir/  # adds "test" to /absoluteDir/
 
-All new files and directories are created with a UID and GID of 0.
+All new files and directories are created with the owner being the same user as
+specified by the `USER` directive. If the user has not been set the default is
+the same as `RUN`, the user with a UID and GID of 0.
 
 > **Note**:
 > If you build using STDIN (`docker build - < somefile`), there is no

integration-cli/docker_cli_build_unix_test.go

@@ -80,7 +80,7 @@ func (s *DockerSuite) TestBuildResourceConstraintsAreUsed(c *check.C) {
 	c.Assert(c2.Ulimits, checker.IsNil, check.Commentf("resource leaked from build for Ulimits"))
 }
 
-func (s *DockerSuite) TestBuildAddChangeOwnership(c *check.C) {
+func (s *DockerSuite) TestBuildAddThenChmod(c *check.C) {
 	testRequires(c, DaemonIsLinux)
 	name := "testbuildaddown"
 
@@ -115,7 +115,89 @@ func (s *DockerSuite) TestBuildAddChangeOwnership(c *check.C) {
 	defer ctx.Close()
 
 	if _, err := buildImageFromContext(name, ctx, true); err != nil {
-		c.Fatalf("build failed to complete for TestBuildAddChangeOwnership: %v", err)
+		c.Fatalf("build failed to complete for TestBuildAddThenChmod: %v", err)
+	}
+}
+
+// Test for #6119.
+func (s *DockerSuite) TestBuildAddObeysUserConfig(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+	name := "testbuildaddobeyuser"
+
+	ctx := func() *FakeContext {
+		dockerfile := `
+			FROM busybox
+			USER 1337:1337
+			ADD foo /bar/
+			RUN [ $(stat -c %u:%g "/bar") = '1337:1337' ]
+			RUN [ $(stat -c %u:%g "/bar/foo") = '1337:1337' ]
+			`
+		tmpDir, err := ioutil.TempDir("", "fake-context")
+		c.Assert(err, check.IsNil)
+		testFile, err := os.Create(filepath.Join(tmpDir, "foo"))
+		if err != nil {
+			c.Fatalf("failed to create foo file: %v", err)
+		}
+		defer testFile.Close()
+
+		chownCmd := exec.Command("chown", "daemon:daemon", "foo")
+		chownCmd.Dir = tmpDir
+		out, _, err := runCommandWithOutput(chownCmd)
+		if err != nil {
+			c.Fatal(err, out)
+		}
+
+		if err := ioutil.WriteFile(filepath.Join(tmpDir, "Dockerfile"), []byte(dockerfile), 0644); err != nil {
+			c.Fatalf("failed to open destination dockerfile: %v", err)
+		}
+		return fakeContextFromDir(tmpDir)
+	}()
+
+	defer ctx.Close()
+
+	if _, err := buildImageFromContext(name, ctx, true); err != nil {
+		c.Fatalf("build failed to complete for TestBuildAddObeysUserConfig: %v", err)
+	}
+}
+
+// Test for #6119.
+func (s *DockerSuite) TestBuildCopyObeysUserConfig(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+	name := "testbuildcopyobeyuser"
+
+	ctx := func() *FakeContext {
+		dockerfile := `
+			FROM busybox
+			USER 1337:1337
+			COPY foo /bar/
+			RUN [ $(stat -c %u:%g "/bar") = '1337:1337' ]
+			RUN [ $(stat -c %u:%g "/bar/foo") = '1337:1337' ]
+			`
+		tmpDir, err := ioutil.TempDir("", "fake-context")
+		c.Assert(err, check.IsNil)
+		testFile, err := os.Create(filepath.Join(tmpDir, "foo"))
+		if err != nil {
+			c.Fatalf("failed to create foo file: %v", err)
+		}
+		defer testFile.Close()
+
+		chownCmd := exec.Command("chown", "daemon:daemon", "foo")
+		chownCmd.Dir = tmpDir
+		out, _, err := runCommandWithOutput(chownCmd)
+		if err != nil {
+			c.Fatal(err, out)
+		}
+
+		if err := ioutil.WriteFile(filepath.Join(tmpDir, "Dockerfile"), []byte(dockerfile), 0644); err != nil {
+			c.Fatalf("failed to open destination dockerfile: %v", err)
+		}
+		return fakeContextFromDir(tmpDir)
+	}()
+
+	defer ctx.Close()
+
+	if _, err := buildImageFromContext(name, ctx, true); err != nil {
+		c.Fatalf("build failed to complete for TestBuildCopyObeysUserConfig: %v", err)
 	}
 }