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 --- test/system/065-cp.bats | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'test/system') 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 From 31b11b5cd62093d86891d32e0912661814d6b78b Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Mar 2021 15:57:51 +0100 Subject: podman cp: fix copying to a non-existent dir Copy is full of perils. Some of them are the nuances when copying directories. Who would have thought that * cp dir foo * cp dir/ foo * cp dir/. foo are all supposed to yield the same result when foo does not exist. `podman cp` now supports all three notations, which required to massage the front-end code in `cmd/podman` a bit. The tests have been extended and partially rewritten to test container->host and host->container copy operations. Signed-off-by: Valentin Rothberg --- cmd/podman/containers/cp.go | 49 +++++++++++++++++++++++++++++------ test/system/065-cp.bats | 62 +++++++++++++++++++++++---------------------- 2 files changed, 73 insertions(+), 38 deletions(-) (limited to 'test/system') diff --git a/cmd/podman/containers/cp.go b/cmd/podman/containers/cp.go index 7a62d982c..27aacc6e5 100644 --- a/cmd/podman/containers/cp.go +++ b/cmd/podman/containers/cp.go @@ -160,6 +160,25 @@ func copyFromContainer(container string, containerPath string, hostPath string) } } + // If we copy a directory via the "." notation and the host path does + // not exist, we need to make sure that the destination on the host + // gets created; otherwise the contents of the source directory will be + // written to the destination's parent directory. + // + // While we could cut it short on the host and do create the directory + // ourselves, we would run into problems trying to that the other way + // around when copying into a container. Instead, to keep both + // implementations symmetrical, we need to massage the code a bit to + // let Buildah's copier package create the destination. + // + // Hence, whenever "." is the source and the destination does not exist, + // we copy the source's parent and let the copier package create the + // destination via the Rename option. + containerTarget := containerInfo.LinkTarget + if hostInfoErr != nil && containerInfo.IsDir && strings.HasSuffix(containerTarget, ".") { + containerTarget = filepath.Dir(containerTarget) + } + reader, writer := io.Pipe() hostCopy := func() error { defer reader.Close() @@ -193,10 +212,10 @@ func copyFromContainer(container string, containerPath string, hostPath string) ChownFiles: &idPair, IgnoreDevices: true, } - if !containerInfo.IsDir && (!hostInfo.IsDir || hostInfoErr != nil) { + if (!containerInfo.IsDir && !hostInfo.IsDir) || hostInfoErr != nil { // If we're having a file-to-file copy, make sure to // rename accordingly. - putOptions.Rename = map[string]string{filepath.Base(containerInfo.LinkTarget): hostBaseName} + putOptions.Rename = map[string]string{filepath.Base(containerTarget): hostBaseName} } dir := hostInfo.LinkTarget if !hostInfo.IsDir { @@ -210,7 +229,7 @@ func copyFromContainer(container string, containerPath string, hostPath string) containerCopy := func() error { defer writer.Close() - copyFunc, err := registry.ContainerEngine().ContainerCopyToArchive(registry.GetContext(), container, containerInfo.LinkTarget, writer) + copyFunc, err := registry.ContainerEngine().ContainerCopyToArchive(registry.GetContext(), container, containerTarget, writer) if err != nil { return err } @@ -278,6 +297,19 @@ func copyToContainer(container string, containerPath string, hostPath string) er containerBaseName = filepath.Base(containerInfo.LinkTarget) } + // If we copy a directory via the "." notation and the container path + // does not exist, we need to make sure that the destination on the + // container gets created; otherwise the contents of the source + // directory will be written to the destination's parent directory. + // + // Hence, whenever "." is the source and the destination does not + // exist, we copy the source's parent and let the copier package create + // the destination via the Rename option. + hostTarget := hostInfo.LinkTarget + if containerInfoErr != nil && hostInfo.IsDir && strings.HasSuffix(hostTarget, ".") { + hostTarget = filepath.Dir(hostTarget) + } + var stdinFile string if isStdin { if !containerInfo.IsDir { @@ -318,15 +350,16 @@ func copyToContainer(container string, containerPath string, hostPath string) er } getOptions := buildahCopiah.GetOptions{ - // Unless the specified points to ".", we want to copy the base directory. - KeepDirectoryNames: hostInfo.IsDir && filepath.Base(hostPath) != ".", + // Unless the specified path points to ".", we want to + // copy the base directory. + KeepDirectoryNames: hostInfo.IsDir && filepath.Base(hostTarget) != ".", } - if !hostInfo.IsDir && (!containerInfo.IsDir || containerInfoErr != nil) { + if (!hostInfo.IsDir && !containerInfo.IsDir) || containerInfoErr != nil { // If we're having a file-to-file copy, make sure to // rename accordingly. - getOptions.Rename = map[string]string{filepath.Base(hostInfo.LinkTarget): containerBaseName} + getOptions.Rename = map[string]string{filepath.Base(hostTarget): containerBaseName} } - if err := buildahCopiah.Get("/", "", getOptions, []string{hostInfo.LinkTarget}, writer); err != nil { + if err := buildahCopiah.Get("/", "", getOptions, []string{hostTarget}, writer); err != nil { return errors.Wrap(err, "error copying from host") } return nil diff --git a/test/system/065-cp.bats b/test/system/065-cp.bats index 24d8a9fd1..43b21587f 100644 --- a/test/system/065-cp.bats +++ b/test/system/065-cp.bats @@ -192,20 +192,19 @@ load helpers @test "podman cp dir from host to container" { - dirname=dir-test - srcdir=$PODMAN_TMPDIR/$dirname - mkdir -p $srcdir + srcdir=$PODMAN_TMPDIR + mkdir -p $srcdir/dir/sub local -a randomcontent=( random-0-$(random_string 10) random-1-$(random_string 15) ) - echo "${randomcontent[0]}" > $srcdir/hostfile0 - echo "${randomcontent[1]}" > $srcdir/hostfile1 + echo "${randomcontent[0]}" > $srcdir/dir/sub/hostfile0 + echo "${randomcontent[1]}" > $srcdir/dir/sub/hostfile1 # "." and "dir/." will copy the contents, so make sure that a dir ending # with dot is treated correctly. - mkdir -p $srcdir. - cp $srcdir/* $srcdir./ + mkdir -p $srcdir/dir. + cp -r $srcdir/dir/* $srcdir/dir. run_podman run -d --name cpcontainer --workdir=/srv $IMAGE sleep infinity run_podman exec cpcontainer mkdir /srv/subdir @@ -216,12 +215,15 @@ load helpers # format is: | | | tests=" - | / | /dir-test | copy to root - . | / | /dir-test. | copy dotdir to root - / | /tmp | /tmp/dir-test | copy to tmp - /. | /usr/ | /usr/ | copy contents of dir to usr/ - | . | /srv/dir-test | copy to workdir (rel path) - | subdir/. | /srv/subdir/dir-test | copy to workdir subdir (rel path) + dir | / | /dir/sub | copy dir to root + dir. | / | /dir./sub | copy dir. to root + dir/ | /tmp | /tmp/dir/sub | copy dir/ to tmp + dir/. | /usr/ | /usr/sub | copy dir/. usr/ + dir/sub | . | /srv/sub | copy dir/sub to workdir (rel path) + dir/sub/. | subdir/. | /srv/subdir | copy dir/sub/. to workdir subdir (rel path) + dir | /newdir1 | /newdir1/sub | copy dir to newdir1 + dir/ | /newdir2 | /newdir2/sub | copy dir/ to newdir2 + dir/. | /newdir3 | /newdir3/sub | copy dir/. to newdir3 " # RUNNING container @@ -230,12 +232,10 @@ load helpers if [[ $src == "''" ]];then unset src fi - run_podman cp $srcdir$src cpcontainer:$dest - run_podman exec cpcontainer ls $dest_fullname - run_podman exec cpcontainer cat $dest_fullname/hostfile0 - is "$output" "${randomcontent[0]}" "$description (cp -> ctr:$dest)" - run_podman exec cpcontainer cat $dest_fullname/hostfile1 - is "$output" "${randomcontent[1]}" "$description (cp -> ctr:$dest)" + run_podman cp $srcdir/$src cpcontainer:$dest + run_podman exec cpcontainer cat $dest_fullname/hostfile0 $dest_fullname/hostfile1 + is "${lines[0]}" "${randomcontent[0]}" "$description (cp -> ctr:$dest)" + is "${lines[1]}" "${randomcontent[1]}" "$description (cp -> ctr:$dest)" done < <(parse_table "$tests") run_podman kill cpcontainer run_podman rm -f cpcontainer @@ -247,7 +247,7 @@ load helpers unset src fi run_podman create --name cpcontainer --workdir=/srv $cpimage sleep infinity - run_podman cp $srcdir$src cpcontainer:$dest + run_podman cp $srcdir/$src cpcontainer:$dest run_podman start cpcontainer run_podman exec cpcontainer cat $dest_fullname/hostfile0 $dest_fullname/hostfile1 is "${lines[0]}" "${randomcontent[0]}" "$description (cp -> ctr:$dest)" @@ -280,17 +280,19 @@ load helpers run_podman commit -q cpcontainer cpimage="$output" - # format is: | | + # format is: | | | tests=" - /srv | /srv/subdir | copy /srv - /srv/ | /srv/subdir | copy /srv/ - /srv/. | /subdir | copy /srv/. - /srv/subdir/. | | copy /srv/subdir/. - /tmp/subdir. | /subdir. | copy /tmp/subdir. +/srv | | /srv/subdir | copy /srv +/srv | /newdir | /newdir/subdir | copy /srv to /newdir +/srv/ | | /srv/subdir | copy /srv/ +/srv/. | | /subdir | copy /srv/. +/srv/. | /newdir | /newdir/subdir | copy /srv/. to /newdir +/srv/subdir/. | | | copy /srv/subdir/. +/tmp/subdir. | | /subdir. | copy /tmp/subdir. " # RUNNING container - while read src dest_fullname description; do + while read src dest dest_fullname description; do if [[ $src == "''" ]];then unset src fi @@ -300,7 +302,7 @@ load helpers if [[ $dest_fullname == "''" ]];then unset dest_fullname fi - run_podman cp cpcontainer:$src $destdir + run_podman cp cpcontainer:$src $destdir$dest is "$(< $destdir$dest_fullname/containerfile0)" "${randomcontent[0]}" "$description" is "$(< $destdir$dest_fullname/containerfile1)" "${randomcontent[1]}" "$description" rm -rf $destdir/* @@ -310,7 +312,7 @@ load helpers # CREATED container run_podman create --name cpcontainer --workdir=/srv $cpimage - while read src dest_fullname description; do + while read src dest dest_fullname description; do if [[ $src == "''" ]];then unset src fi @@ -320,7 +322,7 @@ load helpers if [[ $dest_fullname == "''" ]];then unset dest_fullname fi - run_podman cp cpcontainer:$src $destdir + run_podman cp cpcontainer:$src $destdir$dest is "$(< $destdir$dest_fullname/containerfile0)" "${randomcontent[0]}" "$description" is "$(< $destdir$dest_fullname/containerfile1)" "${randomcontent[1]}" "$description" rm -rf $destdir/* -- cgit v1.2.3-54-g00ecf From 1f2f7e74590090fe8ba68b25f83f2ca08e2f201d Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 9 Mar 2021 08:54:21 +0100 Subject: podman cp: evaluate symlink correctly when copying from container When copying from a container, make sure to evaluate the symlinks correctly. Add tests copying a symlinked directory from a running and a non-running container to execute both path-resolution paths. Signed-off-by: Valentin Rothberg --- libpod/container_stat_linux.go | 42 +++++++++++++++++++++++++++++++++--------- test/system/065-cp.bats | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 9 deletions(-) (limited to 'test/system') diff --git a/libpod/container_stat_linux.go b/libpod/container_stat_linux.go index 307b75c14..0b4d9e2df 100644 --- a/libpod/container_stat_linux.go +++ b/libpod/container_stat_linux.go @@ -64,6 +64,13 @@ func (c *Container) stat(ctx context.Context, containerMountPoint string, contai containerPath = "/." } + // Wildcards are not allowed. + // TODO: it's now technically possible wildcards. + // We may consider enabling support in the future. + if strings.Contains(containerPath, "*") { + return nil, "", "", copy.ErrENOENT + } + if c.state.State == define.ContainerStateRunning { // If the container is running, we need to join it's mount namespace // and stat there. @@ -88,7 +95,8 @@ func (c *Container) stat(ctx context.Context, containerMountPoint string, contai } if statInfo.IsSymlink { - // Evaluated symlinks are always relative to the container's mount point. + // Symlinks are already evaluated and always relative to the + // container's mount point. absContainerPath = statInfo.ImmediateTarget } else if strings.HasPrefix(resolvedPath, containerMountPoint) { // If the path is on the container's mount point, strip it off. @@ -143,15 +151,31 @@ func secureStat(root string, path string) (*copier.StatForItem, error) { if len(globStats) != 1 { return nil, errors.Errorf("internal error: secureStat: expected 1 item but got %d", len(globStats)) } - - stat, exists := globStats[0].Results[glob] // only one glob passed, so that's okay - if !exists { - return nil, copy.ErrENOENT + if len(globStats) != 1 { + return nil, errors.Errorf("internal error: secureStat: expected 1 result but got %d", len(globStats[0].Results)) } - var statErr error - if stat.Error != "" { - statErr = errors.New(stat.Error) + // NOTE: the key in the map differ from `glob` when hitting symlink. + // Hence, we just take the first (and only) key/value pair. + for _, stat := range globStats[0].Results { + var statErr error + if stat.Error != "" { + statErr = errors.New(stat.Error) + } + // If necessary evaluate the symlink + if stat.IsSymlink { + target, err := copier.Eval(root, path, copier.EvalOptions{}) + if err != nil { + return nil, errors.Wrap(err, "error evaluating symlink in container") + } + // Need to make sure the symlink is relative to the root! + target = strings.TrimPrefix(target, root) + target = filepath.Join("/", target) + stat.ImmediateTarget = target + } + return stat, statErr } - return stat, statErr + + // Nothing found! + return nil, copy.ErrENOENT } diff --git a/test/system/065-cp.bats b/test/system/065-cp.bats index 43b21587f..73e807843 100644 --- a/test/system/065-cp.bats +++ b/test/system/065-cp.bats @@ -333,6 +333,46 @@ load helpers } +@test "podman cp symlinked directory from container" { + destdir=$PODMAN_TMPDIR/cp-weird-symlink + mkdir -p $destdir + + # Create 3 files with random content in the container. + local -a randomcontent=( + random-0-$(random_string 10) + random-1-$(random_string 15) + ) + + run_podman run -d --name cpcontainer $IMAGE sleep infinity + run_podman exec cpcontainer sh -c "echo ${randomcontent[0]} > /tmp/containerfile0" + run_podman exec cpcontainer sh -c "echo ${randomcontent[1]} > /tmp/containerfile1" + run_podman exec cpcontainer sh -c "mkdir /tmp/sub && cd /tmp/sub && ln -s .. weirdlink" + + # Commit the image for testing non-running containers + run_podman commit -q cpcontainer + cpimage="$output" + + # RUNNING container + # NOTE: /dest does not exist yet but is expected to be created during copy + run_podman cp cpcontainer:/tmp/sub/weirdlink $destdir/dest + run cat $destdir/dest/containerfile0 $destdir/dest/containerfile1 + is "${lines[0]}" "${randomcontent[0]}" "eval symlink - running container" + is "${lines[1]}" "${randomcontent[1]}" "eval symlink - running container" + + run_podman kill cpcontainer + run_podman rm -f cpcontainer + run rm -rf $srcdir/dest + + # CREATED container + run_podman create --name cpcontainer $cpimage + run_podman cp cpcontainer:/tmp/sub/weirdlink $destdir/dest + run cat $destdir/dest/containerfile0 $destdir/dest/containerfile1 + is "${lines[0]}" "${randomcontent[0]}" "eval symlink - created container" + is "${lines[1]}" "${randomcontent[1]}" "eval symlink - created container" + run_podman rm -f cpcontainer +} + + @test "podman cp file from host to container volume" { srcdir=$PODMAN_TMPDIR/cp-test-volume mkdir -p $srcdir -- cgit v1.2.3-54-g00ecf