Merge pull request #9630 from vrothberg/cp-rootless-eperms

podman cp: ignore EPERMs in rootless mode

11 files changed, 239 insertions, 96 deletions

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/docs/source/markdown/podman-cp.1.md b/docs/source/markdown/podman-cp.1.md
index 56511c244..bafbbdf3b 100644
--- a/docs/source/markdown/podman-cp.1.md
+++ b/docs/source/markdown/podman-cp.1.md
@@ -57,6 +57,8 @@ If you use a : in a local machine path, you must be explicit with a relative or
 
 Using `-` as the *src_path* streams the contents of STDIN as a tar archive. The command extracts the content of the tar to the *DEST_PATH* in the container. In this case, *dest_path* must specify a directory. Using `-` as the *dest_path* streams the contents of the resource (can be a directory) as a tar archive to STDOUT.
 
+Note that `podman cp` ignores permission errors when copying from a running rootless container.  The TTY devices inside a rootless container are owned by the host's root user and hence cannot be read inside the container's user namespace.
+
 ## OPTIONS
 
 ## ALTERNATIVES
diff --git a/go.mod b/go.mod
index 4e8c6e8bf..8223d6ad9 100644
--- a/go.mod
+++ b/go.mod
@@ -10,7 +10,7 @@ require (
 	github.com/checkpoint-restore/go-criu v0.0.0-20190109184317-bdb7599cd87b
 	github.com/containernetworking/cni v0.8.1
 	github.com/containernetworking/plugins v0.9.1
-	github.com/containers/buildah v1.19.7
+	github.com/containers/buildah v1.19.8
 	github.com/containers/common v0.35.0
 	github.com/containers/conmon v2.0.20+incompatible
 	github.com/containers/image/v5 v5.10.2
diff --git a/go.sum b/go.sum
index 751d26ce3..d52b85817 100644
--- a/go.sum
+++ b/go.sum
@@ -94,8 +94,8 @@ github.com/containernetworking/cni v0.8.1/go.mod h1:LGwApLUm2FpoOfxTDEeq8T9ipbpZ
 github.com/containernetworking/plugins v0.8.7/go.mod h1:R7lXeZaBzpfqapcAbHRW8/CYwm0dHzbz0XEjofx0uB0=
 github.com/containernetworking/plugins v0.9.1 h1:FD1tADPls2EEi3flPc2OegIY1M9pUa9r2Quag7HMLV8=
 github.com/containernetworking/plugins v0.9.1/go.mod h1:xP/idU2ldlzN6m4p5LmGiwRDjeJr6FLK6vuiUwoH7P8=
-github.com/containers/buildah v1.19.7 h1:/g11GlhTo177xFex+5GHlF22hq01SyWaJuSA26UGFNU=
-github.com/containers/buildah v1.19.7/go.mod h1:VnyHWgNmfR1d89/zJ/F4cbwOzaQS+6sBky46W7dCo3E=
+github.com/containers/buildah v1.19.8 h1:4TzmetfKPQF5hh6GgMwbAfrD50j+PAcsRiWDnx+gCI8=
+github.com/containers/buildah v1.19.8/go.mod h1:VnyHWgNmfR1d89/zJ/F4cbwOzaQS+6sBky46W7dCo3E=
 github.com/containers/common v0.33.4/go.mod h1:PhgL71XuC4jJ/1BIqeP7doke3aMFkCP90YBXwDeUr9g=
 github.com/containers/common v0.35.0 h1:1OLZ2v+Tj/CN9BTQkKZ5VOriOiArJedinMMqfJRUI38=
 github.com/containers/common v0.35.0/go.mod h1:gs1th7XFTOvVUl4LDPdQjOfOeNiVRDbQ7CNrZ0wS6F8=
diff --git a/libpod/container_copy_linux.go b/libpod/container_copy_linux.go
index 66ccd2f1f..5c275c641 100644
--- a/libpod/container_copy_linux.go
+++ b/libpod/container_copy_linux.go
@@ -14,7 +14,7 @@ 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/podman/v3/pkg/rootless"
 	"github.com/containers/storage/pkg/idtools"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/opencontainers/runtime-spec/specs-go"
@@ -62,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
 	}
@@ -81,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,
@@ -121,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())
 
@@ -134,11 +149,16 @@ 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
+			// 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 {
@@ -148,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/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/e2e/cp_test.go b/test/e2e/cp_test.go
index c0fb61544..c0fb3f887 100644
--- a/test/e2e/cp_test.go
+++ b/test/e2e/cp_test.go
@@ -212,7 +212,6 @@ var _ = Describe("Podman cp", func() {
 
 	// Copy the root dir "/" of a container to the host.
 	It("podman cp the root directory from the ctr to an existing directory on the host ", func() {
-		SkipIfRootless("cannot copy tty devices in rootless mode")
 		container := "copyroottohost"
 		session := podmanTest.RunTopContainer(container)
 		session.WaitWithDefaultTimeout()
diff --git a/test/system/065-cp.bats b/test/system/065-cp.bats
index 88ed983d8..73e807843 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
@@ -175,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
@@ -199,12 +215,15 @@ load helpers
 
     # format is: <source arg to cp (appended to srcdir)> | <destination arg to cp> | <full dest path> | <test name>
     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
@@ -213,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
@@ -230,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)"
@@ -263,17 +280,19 @@ load helpers
     run_podman commit -q cpcontainer
     cpimage="$output"
 
-    # format is: <source arg to cp (appended to /srv)> | <full dest path> | <test name>
+    # format is: <source arg to cp (appended to /srv)> | <dest> | <full dest path> | <test name>
     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
@@ -283,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/*
@@ -293,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
@@ -303,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/*
@@ -314,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
diff --git a/vendor/github.com/containers/buildah/buildah.go b/vendor/github.com/containers/buildah/buildah.go
index 77d313c58..427950c5c 100644
--- a/vendor/github.com/containers/buildah/buildah.go
+++ b/vendor/github.com/containers/buildah/buildah.go
@@ -28,7 +28,7 @@ const (
 	Package = "buildah"
 	// Version for the Package.  Bump version in contrib/rpm/buildah.spec
 	// too.
-	Version = "1.19.7"
+	Version = "1.19.8"
 	// The value we use to identify what type of information, currently a
 	// serialized Builder structure, we are using as per-container state.
 	// This should only be changed when we make incompatible changes to
diff --git a/vendor/github.com/containers/buildah/copier/copier.go b/vendor/github.com/containers/buildah/copier/copier.go
index b5e107d4b..52d8133c7 100644
--- a/vendor/github.com/containers/buildah/copier/copier.go
+++ b/vendor/github.com/containers/buildah/copier/copier.go
@@ -284,6 +284,7 @@ type GetOptions struct {
 	KeepDirectoryNames bool              // don't strip the top directory's basename from the paths of items in subdirectories
 	Rename             map[string]string // rename items with the specified names, or under the specified names
 	NoDerefSymlinks    bool              // don't follow symlinks when globs match them
+	IgnoreUnreadable   bool              // ignore errors reading items, instead of returning an error
 }
 
 // Get produces an archive containing items that match the specified glob
@@ -1035,6 +1036,14 @@ func copierHandlerStat(req request, pm *fileutils.PatternMatcher) *response {
 	return &response{Stat: statResponse{Globs: stats}}
 }
 
+func errorIsPermission(err error) bool {
+	err = errors.Cause(err)
+	if err == nil {
+		return false
+	}
+	return os.IsPermission(err) || strings.Contains(err.Error(), "permission denied")
+}
+
 func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMatcher, idMappings *idtools.IDMappings) (*response, func() error, error) {
 	statRequest := req
 	statRequest.Request = requestStat
@@ -1111,6 +1120,12 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
 				options.ExpandArchives = false
 				walkfn := func(path string, info os.FileInfo, err error) error {
 					if err != nil {
+						if options.IgnoreUnreadable && errorIsPermission(err) {
+							if info != nil && info.IsDir() {
+								return filepath.SkipDir
+							}
+							return nil
+						}
 						return errors.Wrapf(err, "copier: get: error reading %q", path)
 					}
 					// compute the path of this item
@@ -1150,7 +1165,13 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
 						symlinkTarget = target
 					}
 					// add the item to the outgoing tar stream
-					return copierHandlerGetOne(info, symlinkTarget, rel, path, options, tw, hardlinkChecker, idMappings)
+					if err := copierHandlerGetOne(info, symlinkTarget, rel, path, options, tw, hardlinkChecker, idMappings); err != nil {
+						if req.GetOptions.IgnoreUnreadable && errorIsPermission(err) {
+							return nil
+						}
+						return err
+					}
+					return nil
 				}
 				// walk the directory tree, checking/adding items individually
 				if err := filepath.Walk(item, walkfn); err != nil {
@@ -1170,6 +1191,9 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
 				// dereferenced, be sure to use the name of the
 				// link.
 				if err := copierHandlerGetOne(info, "", filepath.Base(queue[i]), item, req.GetOptions, tw, hardlinkChecker, idMappings); err != nil {
+					if req.GetOptions.IgnoreUnreadable && errorIsPermission(err) {
+						continue
+					}
 					return errors.Wrapf(err, "copier: get: %q", queue[i])
 				}
 				itemsCopied++
@@ -1250,7 +1274,7 @@ func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath str
 		if options.ExpandArchives && isArchivePath(contentPath) {
 			f, err := os.Open(contentPath)
 			if err != nil {
-				return errors.Wrapf(err, "error opening %s", contentPath)
+				return errors.Wrapf(err, "error opening file for reading archive contents")
 			}
 			defer f.Close()
 			rc, _, err := compression.AutoDecompress(f)
@@ -1321,17 +1345,21 @@ func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath str
 			hdr.Mode = int64(*options.ChmodFiles)
 		}
 	}
+	var f *os.File
+	if hdr.Typeflag == tar.TypeReg {
+		// open the file first so that we don't write a header for it if we can't actually read it
+		f, err = os.Open(contentPath)
+		if err != nil {
+			return errors.Wrapf(err, "error opening file for adding its contents to archive")
+		}
+		defer f.Close()
+	}
 	// output the header
 	if err = tw.WriteHeader(hdr); err != nil {
 		return errors.Wrapf(err, "error writing header for %s (%s)", contentPath, hdr.Name)
 	}
 	if hdr.Typeflag == tar.TypeReg {
 		// output the content
-		f, err := os.Open(contentPath)
-		if err != nil {
-			return errors.Wrapf(err, "error opening %s", contentPath)
-		}
-		defer f.Close()
 		n, err := io.Copy(tw, f)
 		if err != nil {
 			return errors.Wrapf(err, "error copying %s", contentPath)
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 2450446c3..c988d641b 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -72,7 +72,7 @@ github.com/containernetworking/plugins/pkg/utils/hwaddr
 github.com/containernetworking/plugins/pkg/utils/sysctl
 github.com/containernetworking/plugins/plugins/ipam/host-local/backend
 github.com/containernetworking/plugins/plugins/ipam/host-local/backend/allocator
-# github.com/containers/buildah v1.19.7
+# github.com/containers/buildah v1.19.8
 github.com/containers/buildah
 github.com/containers/buildah/bind
 github.com/containers/buildah/chroot