From 40be5dba473aa57a388fe26bb79ac38a66653f31 Mon Sep 17 00:00:00 2001 From: Phil Estes Date: Wed, 17 Feb 2016 23:57:42 -0800 Subject: [PATCH] Fix copy chown settings to not default to real root This corrects `docker cp` behavior when user namespaces are enabled. Instead of chown'ing copied-in files to real root (0,0), the code queries for the remapped root uid & gid and sets the chown option properly. Docker-DCO-1.1-Signed-off-by: Phil Estes (github: estesp) --- daemon/archive.go | 8 ++-- .../docker_cli_cp_to_container_unix_test.go | 39 +++++++++++++++++++ integration-cli/docker_utils.go | 17 ++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 integration-cli/docker_cli_cp_to_container_unix_test.go diff --git a/daemon/archive.go b/daemon/archive.go index 5d0e9e0b52..5cf6985210 100644 --- a/daemon/archive.go +++ b/daemon/archive.go @@ -250,13 +250,13 @@ func (daemon *Daemon) containerExtractToDir(container *container.Container, path return ErrRootFSReadOnly } + uid, gid := daemon.GetRemappedUIDGID() options := &archive.TarOptions{ - ChownOpts: &archive.TarChownOptions{ - UID: 0, GID: 0, // TODO: use config.User? Remap to userns root? - }, NoOverwriteDirNonDir: noOverwriteDirNonDir, + ChownOpts: &archive.TarChownOptions{ + UID: uid, GID: gid, // TODO: should all ownership be set to root (either real or remapped)? + }, } - if err := chrootarchive.Untar(content, resolvedPath, options); err != nil { return err } diff --git a/integration-cli/docker_cli_cp_to_container_unix_test.go b/integration-cli/docker_cli_cp_to_container_unix_test.go new file mode 100644 index 0000000000..45d85ba5d1 --- /dev/null +++ b/integration-cli/docker_cli_cp_to_container_unix_test.go @@ -0,0 +1,39 @@ +// +build !windows + +package main + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/docker/docker/pkg/integration/checker" + "github.com/docker/docker/pkg/system" + "github.com/go-check/check" +) + +// Check ownership is root, both in non-userns and userns enabled modes +func (s *DockerSuite) TestCpCheckDestOwnership(c *check.C) { + testRequires(c, DaemonIsLinux, SameHostDaemon) + tmpVolDir := getTestDir(c, "test-cp-tmpvol") + containerID := makeTestContainer(c, + testContainerOptions{volumes: []string{fmt.Sprintf("%s:/tmpvol", tmpVolDir)}}) + + tmpDir := getTestDir(c, "test-cp-to-check-ownership") + defer os.RemoveAll(tmpDir) + + makeTestContentInDir(c, tmpDir) + + srcPath := cpPath(tmpDir, "file1") + dstPath := containerCpPath(containerID, "/tmpvol", "file1") + + err := runDockerCp(c, srcPath, dstPath) + c.Assert(err, checker.IsNil) + + stat, err := system.Stat(filepath.Join(tmpVolDir, "file1")) + c.Assert(err, checker.IsNil) + uid, gid, err := getRootUIDGID() + c.Assert(err, checker.IsNil) + c.Assert(stat.UID(), checker.Equals, uint32(uid), check.Commentf("Copied file not owned by container root UID")) + c.Assert(stat.GID(), checker.Equals, uint32(gid), check.Commentf("Copied file not owned by container root GID")) +} diff --git a/integration-cli/docker_utils.go b/integration-cli/docker_utils.go index e4f05c0b54..6349b11db8 100644 --- a/integration-cli/docker_utils.go +++ b/integration-cli/docker_utils.go @@ -1781,6 +1781,23 @@ func runSleepingContainerInImage(c *check.C, image string, extraArgs ...string) return dockerCmd(c, args...) } +func getRootUIDGID() (int, int, error) { + uidgid := strings.Split(filepath.Base(dockerBasePath), ".") + if len(uidgid) == 1 { + //user namespace remapping is not turned on; return 0 + return 0, 0, nil + } + uid, err := strconv.Atoi(uidgid[0]) + if err != nil { + return 0, 0, err + } + gid, err := strconv.Atoi(uidgid[1]) + if err != nil { + return 0, 0, err + } + return uid, gid, nil +} + // minimalBaseImage returns the name of the minimal base image for the current // daemon platform. func minimalBaseImage() string {