diff --git a/builder/dockerfile/copy.go b/builder/dockerfile/copy.go index c7db943f5c..223623ccd6 100644 --- a/builder/dockerfile/copy.go +++ b/builder/dockerfile/copy.go @@ -56,6 +56,7 @@ type copyInstruction struct { cmdName string infos []copyInfo dest string + chownStr string allowLocalDecompression bool } @@ -369,6 +370,7 @@ func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote b type copyFileOptions struct { decompress bool archiver *archive.Archiver + chownPair idtools.IDPair } func performCopyForInfo(dest copyInfo, source copyInfo, options copyFileOptions) error { @@ -388,7 +390,7 @@ func performCopyForInfo(dest copyInfo, source copyInfo, options copyFileOptions) return errors.Wrapf(err, "source path not found") } if src.IsDir() { - return copyDirectory(archiver, srcPath, destPath) + return copyDirectory(archiver, srcPath, destPath, options.chownPair) } if options.decompress && archive.IsArchivePath(srcPath) && !source.noDecompress { return archiver.UntarPath(srcPath, destPath) @@ -405,26 +407,28 @@ func performCopyForInfo(dest copyInfo, source copyInfo, options copyFileOptions) // is a symlink destPath = filepath.Join(destPath, filepath.Base(source.path)) } - return copyFile(archiver, srcPath, destPath) + return copyFile(archiver, srcPath, destPath, options.chownPair) } -func copyDirectory(archiver *archive.Archiver, source, dest string) error { +func copyDirectory(archiver *archive.Archiver, source, dest string, chownPair idtools.IDPair) error { + destExists, err := isExistingDirectory(dest) + if err != nil { + return errors.Wrapf(err, "failed to query destination path") + } if err := archiver.CopyWithTar(source, dest); err != nil { return errors.Wrapf(err, "failed to copy directory") } - return fixPermissions(source, dest, archiver.IDMappings.RootPair()) + return fixPermissions(source, dest, chownPair, !destExists) } -func copyFile(archiver *archive.Archiver, source, dest string) error { - rootIDs := archiver.IDMappings.RootPair() - - if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest), 0755, rootIDs); err != nil { +func copyFile(archiver *archive.Archiver, source, dest string, chownPair idtools.IDPair) error { + if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest), 0755, chownPair); err != nil { return errors.Wrapf(err, "failed to create new directory") } if err := archiver.CopyFileWithTar(source, dest); err != nil { return errors.Wrapf(err, "failed to copy file") } - return fixPermissions(source, dest, rootIDs) + return fixPermissions(source, dest, chownPair, false) } func endsInSlash(path string) bool { diff --git a/builder/dockerfile/copy_unix.go b/builder/dockerfile/copy_unix.go index 326d95bb39..a4a5e05235 100644 --- a/builder/dockerfile/copy_unix.go +++ b/builder/dockerfile/copy_unix.go @@ -9,10 +9,16 @@ import ( "github.com/docker/docker/pkg/idtools" ) -func fixPermissions(source, destination string, rootIDs idtools.IDPair) error { - skipChownRoot, err := isExistingDirectory(destination) - if err != nil { - return err +func fixPermissions(source, destination string, rootIDs idtools.IDPair, overrideSkip bool) error { + var ( + skipChownRoot bool + err error + ) + if !overrideSkip { + skipChownRoot, err = isExistingDirectory(destination) + if err != nil { + return err + } } // We Walk on the source rather than on the destination because we don't diff --git a/builder/dockerfile/copy_windows.go b/builder/dockerfile/copy_windows.go index 78f5b09457..e4b15bcc10 100644 --- a/builder/dockerfile/copy_windows.go +++ b/builder/dockerfile/copy_windows.go @@ -2,7 +2,7 @@ package dockerfile import "github.com/docker/docker/pkg/idtools" -func fixPermissions(source, destination string, rootIDs idtools.IDPair) error { +func fixPermissions(source, destination string, rootIDs idtools.IDPair, overrideSkip bool) error { // chown is not supported on Windows return nil } diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 182dd7ca8d..3b13a8c95a 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -158,6 +158,7 @@ func add(req dispatchRequest) error { if err != nil { return err } + copyInstruction.chownStr = flChown.Value copyInstruction.allowLocalDecompression = true return req.builder.performCopy(req.state, copyInstruction) @@ -189,6 +190,7 @@ func dispatchCopy(req dispatchRequest) error { if err != nil { return err } + copyInstruction.chownStr = flChown.Value return req.builder.performCopy(req.state, copyInstruction) } diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index c0d6081d06..60a73a5fff 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -7,13 +7,18 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "path/filepath" + "strconv" "strings" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" "github.com/docker/docker/api/types/container" "github.com/docker/docker/image" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/stringid" + "github.com/docker/docker/pkg/symlink" + lcUser "github.com/opencontainers/runc/libcontainer/user" "github.com/pkg/errors" ) @@ -107,10 +112,16 @@ func (b *Builder) exportImage(state *dispatchState, imageMount *imageMount, runC func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error { srcHash := getSourceHashFromInfos(inst.infos) + var chownComment string + if inst.chownStr != "" { + chownComment = fmt.Sprintf("--chown=%s", inst.chownStr) + } + commentStr := fmt.Sprintf("%s %s%s in %s ", inst.cmdName, chownComment, srcHash, inst.dest) + // TODO: should this have been using origPaths instead of srcHash in the comment? runConfigWithCommentCmd := copyRunConfig( state.runConfig, - withCmdCommentString(fmt.Sprintf("%s %s in %s ", inst.cmdName, srcHash, inst.dest), b.platform)) + withCmdCommentString(commentStr, b.platform)) hit, err := b.probeCache(state, runConfigWithCommentCmd) if err != nil || hit { return err @@ -125,9 +136,21 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error return err } + chownPair := b.archiver.IDMappings.RootPair() + // if a chown was requested, perform the steps to get the uid, gid + // translated (if necessary because of user namespaces), and replace + // the root pair with the chown pair for copy operations + if inst.chownStr != "" { + chownPair, err = parseChownFlag(inst.chownStr, destInfo.root, b.archiver.IDMappings) + if err != nil { + return errors.Wrapf(err, "unable to convert uid/gid chown string to host mapping") + } + } + opts := copyFileOptions{ decompress: inst.allowLocalDecompression, archiver: b.archiver, + chownPair: chownPair, } for _, info := range inst.infos { if err := performCopyForInfo(destInfo, info, opts); err != nil { @@ -137,6 +160,88 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error return b.exportImage(state, imageMount, runConfigWithCommentCmd) } +func parseChownFlag(chown, ctrRootPath string, idMappings *idtools.IDMappings) (idtools.IDPair, error) { + var userStr, grpStr string + parts := strings.Split(chown, ":") + if len(parts) > 2 { + return idtools.IDPair{}, errors.New("invalid chown string format: " + chown) + } + if len(parts) == 1 { + // if no group specified, use the user spec as group as well + userStr, grpStr = parts[0], parts[0] + } else { + userStr, grpStr = parts[0], parts[1] + } + + passwdPath, err := symlink.FollowSymlinkInScope(filepath.Join(ctrRootPath, "etc", "passwd"), ctrRootPath) + if err != nil { + return idtools.IDPair{}, errors.Wrapf(err, "can't resolve /etc/passwd path in container rootfs") + } + groupPath, err := symlink.FollowSymlinkInScope(filepath.Join(ctrRootPath, "etc", "group"), ctrRootPath) + if err != nil { + return idtools.IDPair{}, errors.Wrapf(err, "can't resolve /etc/group path in container rootfs") + } + uid, err := lookupUser(userStr, passwdPath) + if err != nil { + return idtools.IDPair{}, errors.Wrapf(err, "can't find uid for user "+userStr) + } + gid, err := lookupGroup(grpStr, groupPath) + if err != nil { + return idtools.IDPair{}, errors.Wrapf(err, "can't find gid for group "+grpStr) + } + + // convert as necessary because of user namespaces + chownPair, err := idMappings.ToHost(idtools.IDPair{UID: uid, GID: gid}) + if err != nil { + return idtools.IDPair{}, errors.Wrapf(err, "unable to convert uid/gid to host mapping") + } + return chownPair, nil +} + +func lookupUser(userStr, filepath string) (int, error) { + // if the string is actually a uid integer, parse to int and return + // as we don't need to translate with the help of files + uid, err := strconv.Atoi(userStr) + if err == nil { + return uid, nil + } + users, err := lcUser.ParsePasswdFileFilter(filepath, func(u lcUser.User) bool { + if u.Name == userStr { + return true + } + return false + }) + if err != nil { + return 0, err + } + if len(users) == 0 { + return 0, errors.New("no such user: " + userStr) + } + return users[0].Uid, nil +} + +func lookupGroup(groupStr, filepath string) (int, error) { + // if the string is actually a gid integer, parse to int and return + // as we don't need to translate with the help of files + gid, err := strconv.Atoi(groupStr) + if err == nil { + return gid, nil + } + groups, err := lcUser.ParseGroupFileFilter(filepath, func(g lcUser.Group) bool { + if g.Name == groupStr { + return true + } + return false + }) + if err != nil { + return 0, err + } + if len(groups) == 0 { + return 0, errors.New("no such group: " + groupStr) + } + return groups[0].Gid, nil +} + func createDestInfo(workingDir string, inst copyInstruction, imageMount *imageMount) (copyInfo, error) { // Twiddle the destination when it's a relative path - meaning, make it // relative to the WORKINGDIR diff --git a/builder/dockerfile/internals_test.go b/builder/dockerfile/internals_test.go index 8073cc6713..ed7b4cdaf5 100644 --- a/builder/dockerfile/internals_test.go +++ b/builder/dockerfile/internals_test.go @@ -2,6 +2,8 @@ package dockerfile import ( "fmt" + "os" + "path/filepath" "runtime" "testing" @@ -11,6 +13,7 @@ import ( "github.com/docker/docker/builder" "github.com/docker/docker/builder/remotecontext" "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/idtools" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -129,3 +132,130 @@ func TestCopyRunConfig(t *testing.T) { } } + +func TestChownFlagParsing(t *testing.T) { + testFiles := map[string]string{ + "passwd": `root:x:0:0::/bin:/bin/false +bin:x:1:1::/bin:/bin/false +wwwwww:x:21:33::/bin:/bin/false +unicorn:x:1001:1002::/bin:/bin/false + `, + "group": `root:x:0: +bin:x:1: +wwwwww:x:33: +unicorn:x:1002: +somegrp:x:5555: +othergrp:x:6666: + `, + } + // test mappings for validating use of maps + idMaps := []idtools.IDMap{ + { + ContainerID: 0, + HostID: 100000, + Size: 65536, + }, + } + remapped := idtools.NewIDMappingsFromMaps(idMaps, idMaps) + unmapped := &idtools.IDMappings{} + + contextDir, cleanup := createTestTempDir(t, "", "builder-chown-parse-test") + defer cleanup() + + if err := os.Mkdir(filepath.Join(contextDir, "etc"), 0755); err != nil { + t.Fatalf("error creating test directory: %v", err) + } + + for filename, content := range testFiles { + createTestTempFile(t, filepath.Join(contextDir, "etc"), filename, content, 0644) + } + + // positive tests + for _, testcase := range []struct { + name string + chownStr string + idMapping *idtools.IDMappings + expected idtools.IDPair + }{ + { + name: "UIDNoMap", + chownStr: "1", + idMapping: unmapped, + expected: idtools.IDPair{UID: 1, GID: 1}, + }, + { + name: "UIDGIDNoMap", + chownStr: "0:1", + idMapping: unmapped, + expected: idtools.IDPair{UID: 0, GID: 1}, + }, + { + name: "UIDWithMap", + chownStr: "0", + idMapping: remapped, + expected: idtools.IDPair{UID: 100000, GID: 100000}, + }, + { + name: "UIDGIDWithMap", + chownStr: "1:33", + idMapping: remapped, + expected: idtools.IDPair{UID: 100001, GID: 100033}, + }, + { + name: "UserNoMap", + chownStr: "bin:5555", + idMapping: unmapped, + expected: idtools.IDPair{UID: 1, GID: 5555}, + }, + { + name: "GroupWithMap", + chownStr: "0:unicorn", + idMapping: remapped, + expected: idtools.IDPair{UID: 100000, GID: 101002}, + }, + { + name: "UserOnlyWithMap", + chownStr: "unicorn", + idMapping: remapped, + expected: idtools.IDPair{UID: 101001, GID: 101002}, + }, + } { + t.Run(testcase.name, func(t *testing.T) { + idPair, err := parseChownFlag(testcase.chownStr, contextDir, testcase.idMapping) + require.NoError(t, err, "Failed to parse chown flag: %q", testcase.chownStr) + assert.Equal(t, testcase.expected, idPair, "chown flag mapping failure") + }) + } + + // error tests + for _, testcase := range []struct { + name string + chownStr string + idMapping *idtools.IDMappings + descr string + }{ + { + name: "BadChownFlagFormat", + chownStr: "bob:1:555", + idMapping: unmapped, + descr: "invalid chown string format: bob:1:555", + }, + { + name: "UserNoExist", + chownStr: "bob", + idMapping: unmapped, + descr: "can't find uid for user bob: no such user: bob", + }, + { + name: "GroupNoExist", + chownStr: "root:bob", + idMapping: unmapped, + descr: "can't find gid for group bob: no such group: bob", + }, + } { + t.Run(testcase.name, func(t *testing.T) { + _, err := parseChownFlag(testcase.chownStr, contextDir, testcase.idMapping) + assert.EqualError(t, err, testcase.descr, "Expected error string doesn't match") + }) + } +} diff --git a/container/container.go b/container/container.go index e719c96c4f..86e0111445 100644 --- a/container/container.go +++ b/container/container.go @@ -44,7 +44,6 @@ import ( "github.com/docker/libnetwork/options" "github.com/docker/libnetwork/types" agentexec "github.com/docker/swarmkit/agent/exec" - "github.com/opencontainers/runc/libcontainer/user" "golang.org/x/net/context" ) @@ -332,36 +331,6 @@ func (container *Container) GetRootResourcePath(path string) (string, error) { return symlink.FollowSymlinkInScope(filepath.Join(container.Root, cleanPath), container.Root) } -// ParseUserGrp takes `username` in the format of username, uid, username:groupname, -// uid:gid, username:gid, or uid:groupname and parses the passwd file in the container -// to return the ExecUser referred to by `username`. -func (container *Container) ParseUserGrp(username string) (*user.ExecUser, error) { - passwdPath, err := user.GetPasswdPath() - if err != nil { - return nil, err - } - passwdPath, err = container.GetResourcePath(passwdPath) - if err != nil { - return nil, err - } - - groupPath, err := user.GetGroupPath() - if err != nil { - return nil, err - } - groupPath, err = container.GetResourcePath(groupPath) - if err != nil { - return nil, err - } - - execUser, err := user.GetExecUserPath(username, nil, passwdPath, groupPath) - if err != nil { - return nil, err - } - - return execUser, nil -} - // ExitOnNext signals to the monitor that it should not restart the container // after we send the kill signal. func (container *Container) ExitOnNext() { diff --git a/integration-cli/docker_api_build_test.go b/integration-cli/docker_api_build_test.go index c1ab7661e0..00c58448aa 100644 --- a/integration-cli/docker_api_build_test.go +++ b/integration-cli/docker_api_build_test.go @@ -410,6 +410,35 @@ func (s *DockerSuite) TestBuildAddRemoteNoDecompress(c *check.C) { assert.Contains(c, string(out), "Successfully built") } +func (s *DockerSuite) TestBuildChownOnCopy(c *check.C) { + testRequires(c, DaemonIsLinux) + dockerfile := `FROM busybox + RUN echo 'test1:x:1001:1001::/bin:/bin/false' >> /etc/passwd + RUN echo 'test1:x:1001:' >> /etc/group + RUN echo 'test2:x:1002:' >> /etc/group + COPY --chown=test1:1002 . /new_dir + RUN ls -l / + RUN [ $(ls -l / | grep new_dir | awk '{print $3":"$4}') = 'test1:test2' ] + RUN [ $(ls -nl / | grep new_dir | awk '{print $3":"$4}') = '1001:1002' ] + ` + ctx := fakecontext.New(c, "", + fakecontext.WithDockerfile(dockerfile), + fakecontext.WithFile("test_file1", "some test content"), + ) + defer ctx.Close() + + res, body, err := request.Post( + "/build", + request.RawContent(ctx.AsTarReader(c)), + request.ContentType("application/x-tar")) + c.Assert(err, checker.IsNil) + c.Assert(res.StatusCode, checker.Equals, http.StatusOK) + + out, err := testutil.ReadBody(body) + require.NoError(c, err) + assert.Contains(c, string(out), "Successfully built") +} + func (s *DockerSuite) TestBuildWithSession(c *check.C) { testRequires(c, ExperimentalDaemon) diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index fbda41de90..5a3d3efc65 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -602,78 +602,6 @@ RUN [ $(cat "/test dir/test_file6") = 'test6' ]`, command, command, command, com } } -func (s *DockerSuite) TestBuildAddChownFlag(c *check.C) { - testRequires(c, DaemonIsLinux) // Linux specific test - name := "testaddtonewdest" - ctx, err := fakeContext(`FROM busybox -RUN echo 'test1:x:1001:1001::/bin:/bin/false' >> /etc/passwd -RUN echo 'test1:x:1001:' >> /etc/group -RUN echo 'test2:x:1002:' >> /etc/group -ADD --chown=test1:1002 . /new_dir -RUN ls -l / -RUN [ $(ls -l / | grep new_dir | awk '{print $3":"$4}') = 'test1:test2' ]`, - map[string]string{ - "test_dir/test_file": "test file", - }) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - if _, err := buildImageFromContext(name, ctx, true); err != nil { - c.Fatal(err) - } -} - -func (s *DockerDaemonSuite) TestBuildAddChownFlagUserNamespace(c *check.C) { - testRequires(c, DaemonIsLinux) // Linux specific test - - c.Assert(s.d.StartWithBusybox("--userns-remap", "default"), checker.IsNil) - - name := "testaddtonewdest" - ctx, err := fakeContext(`FROM busybox -RUN echo 'test1:x:1001:1001::/bin:/bin/false' >> /etc/passwd -RUN echo 'test1:x:1001:' >> /etc/group -RUN echo 'test2:x:1002:' >> /etc/group -ADD --chown=test1:1002 . /new_dir -RUN ls -l / -RUN [ $(ls -l / | grep new_dir | awk '{print $3":"$4}') = 'test1:test2' ]`, - map[string]string{ - "test_dir/test_file": "test file", - }) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - if _, err := buildImageFromContext(name, ctx, true); err != nil { - c.Fatal(err) - } -} - -func (s *DockerSuite) TestBuildCopyChownFlag(c *check.C) { - testRequires(c, DaemonIsLinux) // Linux specific test - name := "testaddtonewdest" - ctx, err := fakeContext(`FROM busybox -RUN echo 'test1:x:1001:1001::/bin:/bin/false' >> /etc/passwd -RUN echo 'test1:x:1001:' >> /etc/group -RUN echo 'test2:x:1002:' >> /etc/group -COPY --chown=test1:1002 . /new_dir -RUN ls -l / -RUN [ $(ls -l / | grep new_dir | awk '{print $3":"$4}') = 'test1:test2' ]`, - map[string]string{ - "test_dir/test_file": "test file", - }) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - - if _, err := buildImageFromContext(name, ctx, true); err != nil { - c.Fatal(err) - } -} - func (s *DockerSuite) TestBuildCopyFileWithWhitespaceOnWindows(c *check.C) { testRequires(c, DaemonIsWindows) dockerfile := `FROM ` + testEnv.MinimalBaseImage() + `