summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorValentin Rothberg <rothberg@redhat.com>2021-03-05 11:57:14 +0100
committerValentin Rothberg <rothberg@redhat.com>2021-03-09 09:02:14 +0100
commita61d70cf8ef8666be87758dd3875a01ccdcc4c53 (patch)
tree7eb0ce6fe6ad7591c80ef86802bd3216ffb574e2
parent2abfef3809abc59e8d29bcbcf2b5e0aa7141fb6d (diff)
downloadpodman-a61d70cf8ef8666be87758dd3875a01ccdcc4c53.tar.gz
podman-a61d70cf8ef8666be87758dd3875a01ccdcc4c53.tar.bz2
podman-a61d70cf8ef8666be87758dd3875a01ccdcc4c53.zip
podman cp: fix ownership
Make sure the files are chowned to the host/container user, depending on where things are being copied to. Fixes: #9626 Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
-rw-r--r--libpod/container_copy_linux.go64
-rw-r--r--test/system/065-cp.bats17
2 files changed, 45 insertions, 36 deletions
diff --git a/libpod/container_copy_linux.go b/libpod/container_copy_linux.go
index 9dd7e7e9c..5c275c641 100644
--- a/libpod/container_copy_linux.go
+++ b/libpod/container_copy_linux.go
@@ -15,7 +15,6 @@ import (
"github.com/containers/buildah/util"
"github.com/containers/podman/v3/libpod/define"
"github.com/containers/podman/v3/pkg/rootless"
- "github.com/containers/storage"
"github.com/containers/storage/pkg/idtools"
"github.com/docker/docker/pkg/archive"
"github.com/opencontainers/runtime-spec/specs-go"
@@ -63,15 +62,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
}
@@ -82,10 +82,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,
@@ -122,11 +122,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())
@@ -135,10 +149,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"},
// Ignore EPERMs when copying from rootless containers
// since we cannot read TTY devices. Those are owned
@@ -154,29 +168,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 88ed983d8..24d8a9fd1 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