From 28e2a604b472b3b3daea31d10da007dfac10b028 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 27 Jun 2022 20:22:11 +0200 Subject: vendor: update containers/common Signed-off-by: Giuseppe Scrivano --- go.mod | 2 +- go.sum | 5 ++--- vendor/github.com/containers/common/pkg/parse/parse.go | 7 ++++++- vendor/modules.txt | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 92f32a787..933aed725 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/containernetworking/cni v1.1.1 github.com/containernetworking/plugins v1.1.1 github.com/containers/buildah v1.26.1-0.20220609225314-e66309ebde8c - github.com/containers/common v0.48.1-0.20220624132904-722a80e139ec + github.com/containers/common v0.48.1-0.20220627112538-97d9656daba8 github.com/containers/conmon v2.0.20+incompatible github.com/containers/image/v5 v5.21.2-0.20220617075545-929f14a56f5c github.com/containers/ocicrypt v1.1.5 diff --git a/go.sum b/go.sum index 05e5564ab..bc754b1e4 100644 --- a/go.sum +++ b/go.sum @@ -338,8 +338,8 @@ github.com/containernetworking/plugins v1.1.1/go.mod h1:Sr5TH/eBsGLXK/h71HeLfX19 github.com/containers/buildah v1.26.1-0.20220609225314-e66309ebde8c h1:/fKyiLFFuceBPZGJ0Lig7ElURhfsslAOw1BOcItD+X8= github.com/containers/buildah v1.26.1-0.20220609225314-e66309ebde8c/go.mod h1:b0L+u2Dam7soWGn5sVTK31L++Xrf80AbGvK5z9D2+lw= github.com/containers/common v0.48.1-0.20220608111710-dbecabbe82c9/go.mod h1:WBLwq+i7bicCpH54V70HM6s7jqDAESTlYnd05XXp0ac= -github.com/containers/common v0.48.1-0.20220624132904-722a80e139ec h1:kcfKciMCi6/A72M8TSX2ZYJFa5Yu9hC2Mct8FkuW1Xw= -github.com/containers/common v0.48.1-0.20220624132904-722a80e139ec/go.mod h1:KDNk8Lazjjjqs9643cKH9dnq6IXB4Lf7evya5GiFcg0= +github.com/containers/common v0.48.1-0.20220627112538-97d9656daba8 h1:d9CnUqml4SeEWGfQ781UR4quow9xdf7Q0hYqBYFRH4E= +github.com/containers/common v0.48.1-0.20220627112538-97d9656daba8/go.mod h1:UDe7OTpNdtJA2T80Sp7yB0yTaj79f4kMNQbTsNxsqoY= github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg= github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I= github.com/containers/image/v5 v5.21.2-0.20220511203756-fe4fd4ed8be4/go.mod h1:OsX9sFexyGF0FCNAjfcVFv3IwMqDyLyV/WQY/roLPcE= @@ -1268,7 +1268,6 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= -github.com/stretchr/testify v1.7.4/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.7.5 h1:s5PTfem8p8EbKQOctVV53k6jCJt3UX4IEJzwh+C324Q= github.com/stretchr/testify v1.7.5/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= diff --git a/vendor/github.com/containers/common/pkg/parse/parse.go b/vendor/github.com/containers/common/pkg/parse/parse.go index 6c4958cc2..43b783e0c 100644 --- a/vendor/github.com/containers/common/pkg/parse/parse.go +++ b/vendor/github.com/containers/common/pkg/parse/parse.go @@ -14,7 +14,7 @@ import ( // ValidateVolumeOpts validates a volume's options func ValidateVolumeOpts(options []string) ([]string, error) { - var foundRootPropagation, foundRWRO, foundLabelChange, bindType, foundExec, foundDev, foundSuid, foundChown, foundUpperDir, foundWorkDir int + var foundRootPropagation, foundRWRO, foundLabelChange, bindType, foundExec, foundDev, foundSuid, foundChown, foundUpperDir, foundWorkDir, foundCopy int finalOpts := make([]string, 0, len(options)) for _, opt := range options { // support advanced options like upperdir=/path, workdir=/path @@ -88,6 +88,11 @@ func ValidateVolumeOpts(options []string) ([]string, error) { // are intended to be always safe to use, even not on OS // X). continue + case "copy", "nocopy": + foundCopy++ + if foundCopy > 1 { + return nil, errors.Errorf("invalid options %q, can only specify 1 'copy' or 'nocopy' option", strings.Join(options, ", ")) + } default: return nil, errors.Errorf("invalid option type %q", opt) } diff --git a/vendor/modules.txt b/vendor/modules.txt index e72779e09..7642cbff5 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -111,7 +111,7 @@ github.com/containers/buildah/pkg/rusage github.com/containers/buildah/pkg/sshagent github.com/containers/buildah/pkg/util github.com/containers/buildah/util -# github.com/containers/common v0.48.1-0.20220624132904-722a80e139ec +# github.com/containers/common v0.48.1-0.20220627112538-97d9656daba8 ## explicit github.com/containers/common/libimage github.com/containers/common/libimage/define -- cgit v1.2.3-54-g00ecf From 443a2afdb5bf471bca03a6de649e0978b13996ab Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Sat, 25 Jun 2022 14:19:04 +0200 Subject: volumes: switch order of checks avoid any I/O operation on the volume if the source directory is empty. This is useful on network file systems (since CAP_DAC_OVERRIDE is not honored) where the root user might not have enough privileges to perform an I/O operation on the NFS mount but the user running inside the container has. [NO NEW TESTS NEEDED] it needs a setup with a network file system Signed-off-by: Giuseppe Scrivano --- libpod/container_internal.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index ae61298f3..fe2a19a4e 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1642,19 +1642,6 @@ func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string) if vol.state.NeedsCopyUp { logrus.Debugf("Copying up contents from container %s to volume %s", c.ID(), vol.Name()) - // If the volume is not empty, we should not copy up. - volMount := vol.mountPoint() - contents, err := ioutil.ReadDir(volMount) - if err != nil { - return nil, errors.Wrapf(err, "error listing contents of volume %s mountpoint when copying up from container %s", vol.Name(), c.ID()) - } - if len(contents) > 0 { - // The volume is not empty. It was likely modified - // outside of Podman. For safety, let's not copy up into - // it. Fixes CVE-2020-1726. - return vol, nil - } - srcDir, err := securejoin.SecureJoin(mountpoint, v.Dest) if err != nil { return nil, errors.Wrapf(err, "error calculating destination path to copy up container %s volume %s", c.ID(), vol.Name()) @@ -1688,6 +1675,19 @@ func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string) return vol, nil } + // If the volume is not empty, we should not copy up. + volMount := vol.mountPoint() + contents, err := ioutil.ReadDir(volMount) + if err != nil { + return nil, errors.Wrapf(err, "error listing contents of volume %s mountpoint when copying up from container %s", vol.Name(), c.ID()) + } + if len(contents) > 0 { + // The volume is not empty. It was likely modified + // outside of Podman. For safety, let's not copy up into + // it. Fixes CVE-2020-1726. + return vol, nil + } + // Set NeedsCopyUp to false since we are about to do first copy // Do not copy second time. vol.state.NeedsCopyUp = false -- cgit v1.2.3-54-g00ecf From a4094530bc8b0d5fdbd8ad4ce9a136fe15ee7cee Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Sat, 25 Jun 2022 14:21:52 +0200 Subject: volume: drop TODO comment the two operations are equivalent since securejoin.SecureJoin() has solved the symlinks. Prefer the Lstat version though to make sure symlinks are never resolved and we do not end up using a path on the host. Signed-off-by: Giuseppe Scrivano --- libpod/container_internal.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index fe2a19a4e..8112c9a59 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1648,8 +1648,6 @@ func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string) } // Do a manual stat on the source directory to verify existence. // Skip the rest if it exists. - // TODO: Should this be stat or lstat? I'm using lstat because I - // think copy-up doesn't happen when the source is a link. srcStat, err := os.Lstat(srcDir) if err != nil { if os.IsNotExist(err) { -- cgit v1.2.3-54-g00ecf From aada13f244d52dad9b6a1cfaa725e9d36d75a858 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Sat, 25 Jun 2022 18:59:53 +0200 Subject: volume: new options [no]copy add two new options to the volume create command: copy and nocopy. When nocopy is specified, the files from the container image are not copied up to the volume. Closes: https://github.com/containers/podman/issues/14722 Signed-off-by: Giuseppe Scrivano --- docs/source/markdown/podman-create.1.md | 11 ++++++----- docs/source/markdown/podman-run.1.md | 11 ++++++----- docs/source/markdown/podman-volume-create.1.md | 3 ++- libpod/container_internal.go | 4 +++- libpod/runtime_volume_linux.go | 2 +- libpod/volume_internal.go | 6 ++++++ pkg/util/mountOpts.go | 7 ++++++- test/e2e/run_volume_test.go | 8 ++++++++ 8 files changed, 38 insertions(+), 14 deletions(-) diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md index 40fca0f3a..425ce7bcc 100644 --- a/docs/source/markdown/podman-create.1.md +++ b/docs/source/markdown/podman-create.1.md @@ -1295,13 +1295,14 @@ The _options_ is a comma-separated list and can be: * **rw**|**ro** * **z**|**Z** -* [**r**]**shared**|[**r**]**slave**|[**r**]**private**[**r**]**unbindable** -* [**r**]**bind** -* [**no**]**exec** -* [**no**]**dev** -* [**no**]**suid** * [**O**] * [**U**] +* [**no**]**copy** +* [**no**]**dev** +* [**no**]**exec** +* [**no**]**suid** +* [**r**]**bind** +* [**r**]**shared**|[**r**]**slave**|[**r**]**private**[**r**]**unbindable** The `CONTAINER-DIR` must be an absolute path such as `/src/docs`. The volume will be mounted into the container at this directory. diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index 488bf6777..5b45c3350 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -1362,13 +1362,14 @@ The _options_ is a comma-separated list and can be: [[1]](#Footnote1) * **rw**|**ro** * **z**|**Z** -* [**r**]**shared**|[**r**]**slave**|[**r**]**private**[**r**]**unbindable** -* [**r**]**bind** -* [**no**]**exec** -* [**no**]**dev** -* [**no**]**suid** * [**O**] * [**U**] +* [**no**]**copy** +* [**no**]**dev** +* [**no**]**exec** +* [**no**]**suid** +* [**r**]**bind** +* [**r**]**shared**|[**r**]**slave**|[**r**]**private**[**r**]**unbindable** The `CONTAINER-DIR` must be an absolute path such as `/src/docs`. The volume will be mounted into the container at this directory. diff --git a/docs/source/markdown/podman-volume-create.1.md b/docs/source/markdown/podman-volume-create.1.md index 31e109791..32b10da84 100644 --- a/docs/source/markdown/podman-volume-create.1.md +++ b/docs/source/markdown/podman-volume-create.1.md @@ -31,9 +31,10 @@ Set metadata for a volume (e.g., --label mykey=value). Set driver specific options. For the default driver, **local**, this allows a volume to be configured to mount a filesystem on the host. -For the `local` driver the following options are supported: `type`, `device`, and `o`. +For the `local` driver the following options are supported: `type`, `device`, `o`, and `[no]copy`. The `type` option sets the type of the filesystem to be mounted, and is equivalent to the `-t` flag to **mount(8)**. The `device` option sets the device to be mounted, and is equivalent to the `device` argument to **mount(8)**. +The `copy` option enables copying files from the container image path where the mount is created to the newly created volume on the first run. `copy` is the default. The `o` option sets options for the mount, and is equivalent to the `-o` flag to **mount(8)** with these exceptions: diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 8112c9a59..64696cc27 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -21,6 +21,7 @@ import ( "github.com/containers/common/pkg/cgroups" "github.com/containers/common/pkg/chown" "github.com/containers/common/pkg/config" + cutil "github.com/containers/common/pkg/util" "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/libpod/events" "github.com/containers/podman/v4/pkg/ctime" @@ -1639,7 +1640,8 @@ func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string) if err := vol.update(); err != nil { return nil, err } - if vol.state.NeedsCopyUp { + _, hasNoCopy := vol.config.Options["nocopy"] + if vol.state.NeedsCopyUp && !cutil.StringInSlice("nocopy", v.Options) && !hasNoCopy { logrus.Debugf("Copying up contents from container %s to volume %s", c.ID(), vol.Name()) srcDir, err := securejoin.SecureJoin(mountpoint, v.Dest) diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index f8788e183..a1a3b0bfa 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -73,7 +73,7 @@ func (r *Runtime) newVolume(options ...VolumeCreateOption) (_ *Volume, deferredE return nil, errors.Wrapf(err, "invalid volume option %s for driver 'local'", key) } } - case "o", "type", "uid", "gid", "size", "inodes", "noquota": + case "o", "type", "uid", "gid", "size", "inodes", "noquota", "copy", "nocopy": // Do nothing, valid keys default: return nil, errors.Wrapf(define.ErrInvalidArg, "invalid mount option %s for driver 'local'", key) diff --git a/libpod/volume_internal.go b/libpod/volume_internal.go index e0ebb729d..24522c0f9 100644 --- a/libpod/volume_internal.go +++ b/libpod/volume_internal.go @@ -55,6 +55,12 @@ func (v *Volume) needsMount() bool { if _, ok := v.config.Options["NOQUOTA"]; ok { index++ } + if _, ok := v.config.Options["nocopy"]; ok { + index++ + } + if _, ok := v.config.Options["copy"]; ok { + index++ + } // when uid or gid is set there is also the "o" option // set so we have to ignore this one as well if index > 0 { diff --git a/pkg/util/mountOpts.go b/pkg/util/mountOpts.go index e37394619..d1dd75a82 100644 --- a/pkg/util/mountOpts.go +++ b/pkg/util/mountOpts.go @@ -25,7 +25,7 @@ type defaultMountOptions struct { // The sourcePath variable, if not empty, contains a bind mount source. func ProcessOptions(options []string, isTmpfs bool, sourcePath string) ([]string, error) { var ( - foundWrite, foundSize, foundProp, foundMode, foundExec, foundSuid, foundDev, foundCopyUp, foundBind, foundZ, foundU, foundOverlay, foundIdmap bool + foundWrite, foundSize, foundProp, foundMode, foundExec, foundSuid, foundDev, foundCopyUp, foundBind, foundZ, foundU, foundOverlay, foundIdmap, foundCopy bool ) newOptions := make([]string, 0, len(options)) @@ -55,6 +55,11 @@ func ProcessOptions(options []string, isTmpfs bool, sourcePath string) ([]string } switch splitOpt[0] { + case "copy", "nocopy": + if foundCopy { + return nil, errors.Wrapf(ErrDupeMntOption, "only one of 'nocopy' and 'copy' can be used") + } + foundCopy = true case "O": foundOverlay = true case "volume-opt": diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index edb657695..8cc2a68de 100644 --- a/test/e2e/run_volume_test.go +++ b/test/e2e/run_volume_test.go @@ -452,6 +452,14 @@ var _ = Describe("Podman run with volumes", func() { separateVolumeSession.WaitWithDefaultTimeout() Expect(separateVolumeSession).Should(Exit(0)) Expect(separateVolumeSession.OutputToString()).To(Equal(baselineOutput)) + + copySession := podmanTest.Podman([]string{"run", "--rm", "-v", "testvol3:/etc/apk:copy", ALPINE, "stat", "-c", "%h", "/etc/apk/arch"}) + copySession.WaitWithDefaultTimeout() + Expect(copySession).Should(Exit(0)) + + noCopySession := podmanTest.Podman([]string{"run", "--rm", "-v", "testvol4:/etc/apk:nocopy", ALPINE, "stat", "-c", "%h", "/etc/apk/arch"}) + noCopySession.WaitWithDefaultTimeout() + Expect(noCopySession).Should(Exit(1)) }) It("podman named volume copyup symlink", func() { -- cgit v1.2.3-54-g00ecf