diff options
| -rw-r--r-- | cmd/podman/cliconfig/create.go | 1 | ||||
| -rw-r--r-- | cmd/podman/cp.go | 71 | ||||
| -rw-r--r-- | completions/bash/podman | 1 | ||||
| -rw-r--r-- | docs/podman-cp.1.md | 4 | ||||
| -rw-r--r-- | test/e2e/cp_test.go | 23 |
5 files changed, 91 insertions, 9 deletions
diff --git a/cmd/podman/cliconfig/create.go b/cmd/podman/cliconfig/create.go index 49ab3d827..5fb2eed10 100644 --- a/cmd/podman/cliconfig/create.go +++ b/cmd/podman/cliconfig/create.go @@ -24,4 +24,5 @@ type BuildValues struct { type CpValues struct { PodmanCommand Extract bool + Pause bool } diff --git a/cmd/podman/cp.go b/cmd/podman/cp.go index 8240cc193..907bde4b9 100644 --- a/cmd/podman/cp.go +++ b/cmd/podman/cp.go @@ -13,10 +13,12 @@ import ( "github.com/containers/libpod/cmd/podman/cliconfig" "github.com/containers/libpod/cmd/podman/libpodruntime" "github.com/containers/libpod/libpod" + "github.com/containers/libpod/pkg/rootless" "github.com/containers/storage" "github.com/containers/storage/pkg/archive" "github.com/containers/storage/pkg/chrootarchive" "github.com/containers/storage/pkg/idtools" + securejoin "github.com/cyphar/filepath-securejoin" digest "github.com/opencontainers/go-digest" specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" @@ -49,6 +51,7 @@ func init() { cpCommand.Command = _cpCommand flags := cpCommand.Flags() flags.BoolVar(&cpCommand.Extract, "extract", false, "Extract the tar file into the destination directory.") + flags.BoolVar(&cpCommand.Pause, "pause", true, "Pause the container while copying") cpCommand.SetHelpTemplate(HelpTemplate()) cpCommand.SetUsageTemplate(UsageTemplate()) rootCmd.AddCommand(cpCommand.Command) @@ -66,11 +69,10 @@ func cpCmd(c *cliconfig.CpValues) error { } defer runtime.Shutdown(false) - extract := c.Flag("extract").Changed - return copyBetweenHostAndContainer(runtime, args[0], args[1], extract) + return copyBetweenHostAndContainer(runtime, args[0], args[1], c.Extract, c.Pause) } -func copyBetweenHostAndContainer(runtime *libpod.Runtime, src string, dest string, extract bool) error { +func copyBetweenHostAndContainer(runtime *libpod.Runtime, src string, dest string, extract bool, pause bool) error { srcCtr, srcPath := parsePath(runtime, src) destCtr, destPath := parsePath(runtime, dest) @@ -93,6 +95,38 @@ func copyBetweenHostAndContainer(runtime *libpod.Runtime, src string, dest strin return err } defer ctr.Unmount(false) + + // We can't pause rootless containers. + if pause && rootless.IsRootless() { + state, err := ctr.State() + if err != nil { + return err + } + if state == libpod.ContainerStateRunning { + return errors.Errorf("cannot copy into running rootless container with pause set - pass --pause=false to force copying") + } + } + + if pause && !rootless.IsRootless() { + if err := ctr.Pause(); err != nil { + // An invalid state error is fine. + // The container isn't running or is already paused. + // TODO: We can potentially start the container while + // the copy is running, which still allows a race where + // malicious code could mess with the symlink. + if errors.Cause(err) != libpod.ErrCtrStateInvalid { + return err + } + } else if err == nil { + // Only add the defer if we actually paused + defer func() { + if err := ctr.Unpause(); err != nil { + logrus.Errorf("Error unpausing container after copying: %v", err) + } + }() + } + } + user, err := getUser(mountPoint, ctr.User()) if err != nil { return err @@ -112,19 +146,38 @@ func copyBetweenHostAndContainer(runtime *libpod.Runtime, src string, dest strin var glob []string if isFromHostToCtr { if filepath.IsAbs(destPath) { - destPath = filepath.Join(mountPoint, destPath) - + cleanedPath, err := securejoin.SecureJoin(mountPoint, destPath) + if err != nil { + return err + } + destPath = cleanedPath } else { - if err = idtools.MkdirAllAndChownNew(filepath.Join(mountPoint, ctr.WorkingDir()), 0755, hostOwner); err != nil { + ctrWorkDir, err := securejoin.SecureJoin(mountPoint, ctr.WorkingDir()) + if err != nil { + return err + } + if err = idtools.MkdirAllAndChownNew(ctrWorkDir, 0755, hostOwner); err != nil { return errors.Wrapf(err, "error creating directory %q", destPath) } - destPath = filepath.Join(mountPoint, ctr.WorkingDir(), destPath) + cleanedPath, err := securejoin.SecureJoin(mountPoint, filepath.Join(ctr.WorkingDir(), destPath)) + if err != nil { + return err + } + destPath = cleanedPath } } else { if filepath.IsAbs(srcPath) { - srcPath = filepath.Join(mountPoint, srcPath) + cleanedPath, err := securejoin.SecureJoin(mountPoint, srcPath) + if err != nil { + return err + } + srcPath = cleanedPath } else { - srcPath = filepath.Join(mountPoint, ctr.WorkingDir(), srcPath) + cleanedPath, err := securejoin.SecureJoin(mountPoint, filepath.Join(ctr.WorkingDir(), srcPath)) + if err != nil { + return err + } + srcPath = cleanedPath } } glob, err = filepath.Glob(srcPath) diff --git a/completions/bash/podman b/completions/bash/podman index 98038d19c..49c8c0e52 100644 --- a/completions/bash/podman +++ b/completions/bash/podman @@ -2388,6 +2388,7 @@ _podman_cp() { --help -h --extract + --pause " _complete_ "$boolean_options" } diff --git a/docs/podman-cp.1.md b/docs/podman-cp.1.md index 406dd51df..76fe57a9e 100644 --- a/docs/podman-cp.1.md +++ b/docs/podman-cp.1.md @@ -61,6 +61,10 @@ If you use a : in a local machine path, you must be explicit with a relative or Extract the tar file into the destination directory. If the destination directory is not provided, extract the tar file into the root directory. +**--pause** + +Pause the container while copying into it to avoid potential security issues around symlinks. Defaults to *true*. + ## ALTERNATIVES Podman has much stronger capabilities than just `podman cp` to achieve copy files between host and container. diff --git a/test/e2e/cp_test.go b/test/e2e/cp_test.go index f8df5d3d0..3c0288b19 100644 --- a/test/e2e/cp_test.go +++ b/test/e2e/cp_test.go @@ -158,4 +158,27 @@ var _ = Describe("Podman cp", func() { os.Remove("file.tar") os.RemoveAll(testDirPath) }) + + It("podman cp symlink", func() { + srcPath := filepath.Join(podmanTest.RunRoot, "cp_test.txt") + fromHostToContainer := []byte("copy from host to container") + err := ioutil.WriteFile(srcPath, fromHostToContainer, 0644) + Expect(err).To(BeNil()) + + session := podmanTest.Podman([]string{"run", "-d", ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + name := session.OutputToString() + + session = podmanTest.Podman([]string{"exec", name, "ln", "-s", "/tmp", "/test"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"cp", "--pause=false", srcPath, name + ":/test"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + _, err = os.Stat("/tmp/cp_test.txt") + Expect(err).To(Not(BeNil())) + }) }) |