From 72d8a77d522896ec73e07f49a1c1bcb44bbf3bbd Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Tue, 23 Aug 2016 11:33:38 +0300 Subject: [PATCH 1/2] Make host directory mounts use idtools.MkdirAllNewAs This makes sure that: 1. Already existing directories are left untouched 2. Newly created directories are chowned to the correct root UID/GID in case of user namespaces 3. All parent directories still get created with host root UID/GID Fix #21738 Signed-off-by: Antonis Kalipetis --- daemon/volumes_unix.go | 3 ++- volume/volume.go | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/daemon/volumes_unix.go b/daemon/volumes_unix.go index ac073e5f46..5b8d398faa 100644 --- a/daemon/volumes_unix.go +++ b/daemon/volumes_unix.go @@ -28,7 +28,8 @@ func (daemon *Daemon) setupMounts(c *container.Container) ([]container.Mount, er if err := daemon.lazyInitializeVolume(c.ID, m); err != nil { return nil, err } - path, err := m.Setup(c.MountLabel) + rootUID, rootGID := daemon.GetRemappedUIDGID() + path, err := m.Setup(c.MountLabel, rootUID, rootGID) if err != nil { return nil, err } diff --git a/volume/volume.go b/volume/volume.go index 66ef6d2b50..f6e28a4840 100644 --- a/volume/volume.go +++ b/volume/volume.go @@ -6,8 +6,8 @@ import ( "strings" "syscall" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/pkg/system" mounttypes "github.com/docker/engine-api/types/mount" "github.com/opencontainers/runc/libcontainer/label" ) @@ -107,7 +107,7 @@ type MountPoint struct { // Setup sets up a mount point by either mounting the volume if it is // configured, or creating the source directory if supplied. -func (m *MountPoint) Setup(mountLabel string) (string, error) { +func (m *MountPoint) Setup(mountLabel string, rootUID, rootGID int) (string, error) { if m.Volume != nil { if m.ID == "" { m.ID = stringid.GenerateNonCryptoID() @@ -117,8 +117,9 @@ func (m *MountPoint) Setup(mountLabel string) (string, error) { if len(m.Source) == 0 { return "", fmt.Errorf("Unable to setup mount point, neither source nor volume defined") } - // system.MkdirAll() produces an error if m.Source exists and is a file (not a directory), - if err := system.MkdirAll(m.Source, 0755); err != nil { + // idtools.MkdirAllNewAs() produces an error if m.Source exists and is a file (not a directory) + // also, makes sure that if the directory is created, the correct remapped rootUID/rootGID will own it + if err := idtools.MkdirAllNewAs(m.Source, 0755, rootUID, rootGID); err != nil { if perr, ok := err.(*os.PathError); ok { if perr.Err != syscall.ENOTDIR { return "", err From c6bff578b4f169e2fbd67dfbe9f0ac147fc8c255 Mon Sep 17 00:00:00 2001 From: Antonis Kalipetis Date: Mon, 5 Sep 2016 13:02:13 +0300 Subject: [PATCH 2/2] Add test for checking created directories on remapped root Signed-off-by: Antonis Kalipetis --- integration-cli/docker_cli_userns_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/integration-cli/docker_cli_userns_test.go b/integration-cli/docker_cli_userns_test.go index 1be80f64dd..acf74238b2 100644 --- a/integration-cli/docker_cli_userns_test.go +++ b/integration-cli/docker_cli_userns_test.go @@ -7,11 +7,13 @@ import ( "io/ioutil" "os" "os/exec" + "path" "path/filepath" "strconv" "strings" "github.com/docker/docker/pkg/integration/checker" + "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/pkg/system" "github.com/go-check/check" ) @@ -29,6 +31,10 @@ func (s *DockerDaemonSuite) TestDaemonUserNamespaceRootSetting(c *check.C) { defer os.RemoveAll(tmpDir) + // Set a non-existent path + tmpDirNotExists := path.Join(os.TempDir(), "userns"+stringid.GenerateRandomID()) + defer os.RemoveAll(tmpDirNotExists) + // we need to find the uid and gid of the remapped root from the daemon's root dir info uidgid := strings.Split(filepath.Base(s.d.root), ".") c.Assert(uidgid, checker.HasLen, 2, check.Commentf("Should have gotten uid/gid strings from root dirname: %s", filepath.Base(s.d.root))) @@ -40,11 +46,17 @@ func (s *DockerDaemonSuite) TestDaemonUserNamespaceRootSetting(c *check.C) { // writable by the remapped root UID/GID pair c.Assert(os.Chown(tmpDir, uid, gid), checker.IsNil) - out, err := s.d.Cmd("run", "-d", "--name", "userns", "-v", tmpDir+":/goofy", "busybox", "sh", "-c", "touch /goofy/testfile; top") + out, err := s.d.Cmd("run", "-d", "--name", "userns", "-v", tmpDir+":/goofy", "-v", tmpDirNotExists+":/donald", "busybox", "sh", "-c", "touch /goofy/testfile; top") c.Assert(err, checker.IsNil, check.Commentf("Output: %s", out)) user := s.findUser(c, "userns") c.Assert(uidgid[0], checker.Equals, user) + // check that the created directory is owned by remapped uid:gid + statNotExists, err := system.Stat(tmpDirNotExists) + c.Assert(err, checker.IsNil) + c.Assert(statNotExists.UID(), checker.Equals, uint32(uid), check.Commentf("Created directory not owned by remapped root UID")) + c.Assert(statNotExists.GID(), checker.Equals, uint32(gid), check.Commentf("Created directory not owned by remapped root GID")) + pid, err := s.d.Cmd("inspect", "--format={{.State.Pid}}", "userns") c.Assert(err, checker.IsNil, check.Commentf("Could not inspect running container: out: %q", pid)) // check the uid and gid maps for the PID to ensure root is remapped