From 2abfef3809abc59e8d29bcbcf2b5e0aa7141fb6d Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Mar 2021 10:33:27 +0100 Subject: podman cp: ignore EPERMs in rootless mode Ignore permission errors when copying from a rootless container. TTY devices inside rootless containers are owned by the host's root user which is "nobody" inside the container's user namespace rendering us unable to even read them. Enable the integration test which was temporarily disabled for rootless users. Signed-off-by: Valentin Rothberg --- libpod/container_copy_linux.go | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'libpod/container_copy_linux.go') diff --git a/libpod/container_copy_linux.go b/libpod/container_copy_linux.go index 66ccd2f1f..9dd7e7e9c 100644 --- a/libpod/container_copy_linux.go +++ b/libpod/container_copy_linux.go @@ -14,6 +14,7 @@ import ( "github.com/containers/buildah/pkg/chrootuser" "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" @@ -139,6 +140,11 @@ func (c *Container) copyToArchive(ctx context.Context, path string, writer io.Wr 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 + // by the host's root and hence "nobody" inside the + // container's user namespace. + IgnoreUnreadable: rootless.IsRootless() && c.state.State == define.ContainerStateRunning, } return c.joinMountAndExec(ctx, func() error { -- cgit v1.2.3-54-g00ecf From a61d70cf8ef8666be87758dd3875a01ccdcc4c53 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Mar 2021 11:57:14 +0100 Subject: 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 --- libpod/container_copy_linux.go | 64 ++++++++++++++++++------------------------ test/system/065-cp.bats | 17 +++++++++++ 2 files changed, 45 insertions(+), 36 deletions(-) (limited to 'libpod/container_copy_linux.go') 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 -- cgit v1.2.3-54-g00ecf