Skip to content

Commit

Permalink
BACKPORT: Fix copy API (docker cp, etc) uid/gid handling
Browse files Browse the repository at this point in the history
Upstream reference:
moby@8a7ff5f

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1461071

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
  • Loading branch information
runcom committed Jul 10, 2017
1 parent f55a118 commit c66c3eb
Show file tree
Hide file tree
Showing 14 changed files with 209 additions and 98 deletions.
7 changes: 5 additions & 2 deletions api/client/container/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type copyOptions struct {
source string
destination string
followLink bool
copyUIDGID bool
}

type copyDirection int
Expand Down Expand Up @@ -67,6 +68,7 @@ func NewCopyCommand(dockerCli *client.DockerCli) *cobra.Command {
flags := cmd.Flags()

flags.BoolVarP(&opts.followLink, "follow-link", "L", false, "Always follow symbol link in SRC_PATH")
flags.BoolVarP(&opts.copyUIDGID, "archive", "a", false, "Archive mode (copy all uid/gid information)")

return cmd
}
Expand All @@ -93,7 +95,7 @@ func runCopy(dockerCli *client.DockerCli, opts copyOptions) error {
case fromContainer:
return copyFromContainer(ctx, dockerCli, srcContainer, srcPath, dstPath, cpParam)
case toContainer:
return copyToContainer(ctx, dockerCli, srcPath, dstContainer, dstPath, cpParam)
return copyToContainer(ctx, dockerCli, srcPath, dstContainer, dstPath, cpParam, opts.copyUIDGID)
case acrossContainers:
// Copying between containers isn't supported.
return fmt.Errorf("copying between containers is not supported")
Expand Down Expand Up @@ -176,7 +178,7 @@ func copyFromContainer(ctx context.Context, dockerCli *client.DockerCli, srcCont
return archive.CopyTo(preArchive, srcInfo, dstPath)
}

func copyToContainer(ctx context.Context, dockerCli *client.DockerCli, srcPath, dstContainer, dstPath string, cpParam *cpConfig) (err error) {
func copyToContainer(ctx context.Context, dockerCli *client.DockerCli, srcPath, dstContainer, dstPath string, cpParam *cpConfig, copyUIDGID bool) (err error) {
if srcPath != "-" {
// Get an absolute source path.
srcPath, err = resolveLocalPath(srcPath)
Expand Down Expand Up @@ -266,6 +268,7 @@ func copyToContainer(ctx context.Context, dockerCli *client.DockerCli, srcPath,

options := types.CopyToContainerOptions{
AllowOverwriteDirWithFile: false,
CopyUIDGID: copyUIDGID,
}

return dockerCli.Client().CopyToContainer(ctx, dstContainer, resolvedDstPath, content, options)
Expand Down
2 changes: 1 addition & 1 deletion api/server/router/container/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type copyBackend interface {
ContainerArchivePath(name string, path string) (content io.ReadCloser, stat *types.ContainerPathStat, err error)
ContainerCopy(name string, res string) (io.ReadCloser, error)
ContainerExport(name string, out io.Writer) error
ContainerExtractToDir(name, path string, noOverwriteDirNonDir bool, content io.Reader) error
ContainerExtractToDir(name, path string, copyUIDGID, noOverwriteDirNonDir bool, content io.Reader) error
ContainerStatPath(name string, path string) (stat *types.ContainerPathStat, err error)
}

Expand Down
4 changes: 3 additions & 1 deletion api/server/router/container/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,5 +115,7 @@ func (s *containerRouter) putContainersArchive(ctx context.Context, w http.Respo
}

noOverwriteDirNonDir := httputils.BoolValue(r, "noOverwriteDirNonDir")
return s.backend.ContainerExtractToDir(v.Name, v.Path, noOverwriteDirNonDir, r.Body)
copyUIDGID := httputils.BoolValue(r, "copyUIDGID")

return s.backend.ContainerExtractToDir(v.Name, v.Path, copyUIDGID, noOverwriteDirNonDir, r.Body)
}
23 changes: 14 additions & 9 deletions daemon/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ func (daemon *Daemon) ContainerArchivePath(name string, path string) (content io
// be ErrExtractPointNotDirectory. If noOverwriteDirNonDir is true then it will
// be an error if unpacking the given content would cause an existing directory
// to be replaced with a non-directory and vice versa.
func (daemon *Daemon) ContainerExtractToDir(name, path string, noOverwriteDirNonDir bool, content io.Reader) error {
func (daemon *Daemon) ContainerExtractToDir(name, path string, copyUIDGID, noOverwriteDirNonDir bool, content io.Reader) error {
container, err := daemon.GetContainer(name)
if err != nil {
return err
}

return daemon.containerExtractToDir(container, path, noOverwriteDirNonDir, content)
return daemon.containerExtractToDir(container, path, copyUIDGID, noOverwriteDirNonDir, content)
}

// containerStatPath stats the filesystem resource at the specified path in this
Expand Down Expand Up @@ -174,7 +174,7 @@ func (daemon *Daemon) containerArchivePath(container *container.Container, path
// noOverwriteDirNonDir is true then it will be an error if unpacking the
// given content would cause an existing directory to be replaced with a non-
// directory and vice versa.
func (daemon *Daemon) containerExtractToDir(container *container.Container, path string, noOverwriteDirNonDir bool, content io.Reader) (err error) {
func (daemon *Daemon) containerExtractToDir(container *container.Container, path string, copyUIDGID, noOverwriteDirNonDir bool, content io.Reader) (err error) {
container.Lock()
defer container.Unlock()

Expand Down Expand Up @@ -257,13 +257,18 @@ func (daemon *Daemon) containerExtractToDir(container *container.Container, path
return ErrRootFSReadOnly
}

uid, gid := daemon.GetRemappedUIDGID()
options := &archive.TarOptions{
NoOverwriteDirNonDir: noOverwriteDirNonDir,
ChownOpts: &archive.TarChownOptions{
UID: uid, GID: gid, // TODO: should all ownership be set to root (either real or remapped)?
},
options := daemon.defaultTarCopyOptions(noOverwriteDirNonDir)

if copyUIDGID {
var err error
// tarCopyOptions will appropriately pull in the right uid/gid for the
// user/group and will set the options.
options, err = daemon.tarCopyOptions(container, noOverwriteDirNonDir)
if err != nil {
return err
}
}

if err := chrootarchive.Untar(content, resolvedPath, options); err != nil {
return err
}
Expand Down
16 changes: 16 additions & 0 deletions daemon/archive_tarcopyoptions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package daemon

import (
"github.com/docker/docker/pkg/archive"
)

// defaultTarCopyOptions is the setting that is used when unpacking an archive
// for a copy API event.
func (daemon *Daemon) defaultTarCopyOptions(noOverwriteDirNonDir bool) *archive.TarOptions {
uidMaps, gidMaps := daemon.GetUIDGIDMaps()
return &archive.TarOptions{
NoOverwriteDirNonDir: noOverwriteDirNonDir,
UIDMaps: uidMaps,
GIDMaps: gidMaps,
}
}
30 changes: 30 additions & 0 deletions daemon/archive_tarcopyoptions_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// +build !windows

package daemon

import (
"github.com/docker/docker/container"
"github.com/docker/docker/pkg/archive"
"github.com/opencontainers/runc/libcontainer/user"
)

func (daemon *Daemon) tarCopyOptions(container *container.Container, noOverwriteDirNonDir bool) (*archive.TarOptions, error) {
if container.Config.User == "" {
return daemon.defaultTarCopyOptions(noOverwriteDirNonDir), nil
}

// TODO(runcom): maybe use the newer docker/pkg/idtools which supports lookup
// users by getent also.
user, err := user.LookupUser(container.Config.User)
if err != nil {
return nil, err
}

return &archive.TarOptions{
NoOverwriteDirNonDir: noOverwriteDirNonDir,
ChownOpts: &archive.TarChownOptions{
UID: user.Uid,
GID: user.Gid,
},
}, nil
}
12 changes: 12 additions & 0 deletions daemon/archive_tarcopyoptions_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// +build windows

package daemon

import (
"github.com/docker/docker/container"
"github.com/docker/docker/pkg/archive"
)

func (daemon *Daemon) tarCopyOptions(container *container.Container, noOverwriteDirNonDir bool) (*archive.TarOptions, error) {
return daemon.defaultTarCopyOptions(noOverwriteDirNonDir), nil
}
Loading

0 comments on commit c66c3eb

Please sign in to comment.