diff --git a/libpod/container_copy_linux.go b/libpod/container_copy_linux.go index 66ccd2f1f4..14a415cc1a 100644 --- a/libpod/container_copy_linux.go +++ b/libpod/container_copy_linux.go @@ -14,7 +14,6 @@ import ( "github.com/containers/buildah/pkg/chrootuser" "github.com/containers/buildah/util" "github.com/containers/podman/v3/libpod/define" - "github.com/containers/storage" "github.com/containers/storage/pkg/idtools" "github.com/docker/docker/pkg/archive" "github.com/opencontainers/runtime-spec/specs-go" @@ -62,15 +61,16 @@ func (c *Container) copyFromArchive(ctx context.Context, path string, reader io. } } - decompressed, err := archive.DecompressStream(reader) + // Make sure we chown the files to the container's main user and group ID. + user, err := getContainerUser(c, mountPoint) if err != nil { unmount() return nil, err } + idPair := idtools.IDPair{UID: int(user.UID), GID: int(user.GID)} - idMappings, idPair, err := getIDMappingsAndPair(c, mountPoint) + decompressed, err := archive.DecompressStream(reader) if err != nil { - decompressed.Close() unmount() return nil, err } @@ -81,10 +81,10 @@ func (c *Container) copyFromArchive(ctx context.Context, path string, reader io. defer unmount() defer decompressed.Close() putOptions := buildahCopiah.PutOptions{ - UIDMap: idMappings.UIDMap, - GIDMap: idMappings.GIDMap, - ChownDirs: idPair, - ChownFiles: idPair, + UIDMap: c.config.IDMappings.UIDMap, + GIDMap: c.config.IDMappings.GIDMap, + ChownDirs: &idPair, + ChownFiles: &idPair, } return c.joinMountAndExec(ctx, @@ -121,11 +121,25 @@ func (c *Container) copyToArchive(ctx context.Context, path string, writer io.Wr return nil, err } - idMappings, idPair, err := getIDMappingsAndPair(c, mountPoint) + // We optimistically chown to the host user. In case of a hypothetical + // container-to-container copy, the reading side will chown back to the + // container user. + user, err := getContainerUser(c, mountPoint) + if err != nil { + unmount() + return nil, err + } + hostUID, hostGID, err := util.GetHostIDs( + idtoolsToRuntimeSpec(c.config.IDMappings.UIDMap), + idtoolsToRuntimeSpec(c.config.IDMappings.GIDMap), + user.UID, + user.GID, + ) if err != nil { unmount() return nil, err } + idPair := idtools.IDPair{UID: int(hostUID), GID: int(hostGID)} logrus.Debugf("Container copy *from* %q (resolved: %q) on container %q (ID: %s)", path, resolvedPath, c.Name(), c.ID()) @@ -134,10 +148,10 @@ func (c *Container) copyToArchive(ctx context.Context, path string, writer io.Wr getOptions := buildahCopiah.GetOptions{ // Unless the specified points to ".", we want to copy the base directory. KeepDirectoryNames: statInfo.IsDir && filepath.Base(path) != ".", - UIDMap: idMappings.UIDMap, - GIDMap: idMappings.GIDMap, - ChownDirs: idPair, - ChownFiles: idPair, + UIDMap: c.config.IDMappings.UIDMap, + GIDMap: c.config.IDMappings.GIDMap, + ChownDirs: &idPair, + ChownFiles: &idPair, Excludes: []string{"dev", "proc", "sys"}, } return c.joinMountAndExec(ctx, @@ -148,29 +162,7 @@ func (c *Container) copyToArchive(ctx context.Context, path string, writer io.Wr }, nil } -// getIDMappingsAndPair returns the ID mappings for the container and the host -// ID pair. -func getIDMappingsAndPair(container *Container, containerMount string) (*storage.IDMappingOptions, *idtools.IDPair, error) { - user, err := getContainerUser(container, containerMount) - if err != nil { - return nil, nil, err - } - - idMappingOpts, err := container.IDMappings() - if err != nil { - return nil, nil, err - } - - hostUID, hostGID, err := util.GetHostIDs(idtoolsToRuntimeSpec(idMappingOpts.UIDMap), idtoolsToRuntimeSpec(idMappingOpts.GIDMap), user.UID, user.GID) - if err != nil { - return nil, nil, err - } - - idPair := idtools.IDPair{UID: int(hostUID), GID: int(hostGID)} - return &idMappingOpts, &idPair, nil -} - -// getContainerUser returns the specs.User of the container. +// getContainerUser returns the specs.User and ID mappings of the container. func getContainerUser(container *Container, mountPoint string) (specs.User, error) { userspec := container.Config().User diff --git a/test/system/065-cp.bats b/test/system/065-cp.bats index 88ed983d8d..24d8a9fd1b 100644 --- a/test/system/065-cp.bats +++ b/test/system/065-cp.bats @@ -88,6 +88,7 @@ load helpers run_podman rmi -f $cpimage } + @test "podman cp file from host to container tmpfs mount" { srcdir=$PODMAN_TMPDIR/cp-test-file-host-to-ctr mkdir -p $srcdir @@ -113,6 +114,22 @@ load helpers } +@test "podman cp file from host to container and check ownership" { + srcdir=$PODMAN_TMPDIR/cp-test-file-host-to-ctr + mkdir -p $srcdir + content=cp-user-test-$(random_string 10) + echo "content" > $srcdir/hostfile + userid=$(id -u) + + run_podman run --user=$userid --userns=keep-id -d --name cpcontainer $IMAGE sleep infinity + run_podman cp $srcdir/hostfile cpcontainer:/tmp/hostfile + run_podman exec cpcontainer stat -c "%u" /tmp/hostfile + is "$output" "$userid" "copied file is chowned to the container user" + run_podman kill cpcontainer + run_podman rm -f cpcontainer +} + + @test "podman cp file from container to host" { srcdir=$PODMAN_TMPDIR/cp-test-file-ctr-to-host mkdir -p $srcdir