Merge pull request #5946 from dashea/dshea-1.27-CVE-2024-11218

[release-1.27] Backport fix for CVE-2024-11218

Author: openshift-merge-bot[bot]

cmd/buildah/run.go

@@ -8,10 +8,12 @@ import (
 
 	"github.com/containers/buildah"
 	internalParse "github.com/containers/buildah/internal/parse"
+	internalUtil "github.com/containers/buildah/internal/util"
 	buildahcli "github.com/containers/buildah/pkg/cli"
+	"github.com/containers/buildah/pkg/overlay"
 	"github.com/containers/buildah/pkg/parse"
 	"github.com/containers/buildah/util"
-	"github.com/containers/storage/pkg/lockfile"
+	"github.com/containers/storage/pkg/mount"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
@@ -101,6 +103,16 @@ func runCmd(c *cobra.Command, args []string, iopts runInputOptions) error {
 		return errors.New("command must be specified")
 	}
 
+	tmpDir, err := os.MkdirTemp(internalUtil.GetTempDir(), "buildahvolume")
+	if err != nil {
+		return fmt.Errorf("creating temporary directory: %w", err)
+	}
+	defer func() {
+		if err := os.Remove(tmpDir); err != nil {
+			logrus.Debugf("removing should-be-empty temporary directory %q: %v", tmpDir, err)
+		}
+	}()
+
 	store, err := getStore(c)
 	if err != nil {
 		return err
@@ -159,13 +171,31 @@ func runCmd(c *cobra.Command, args []string, iopts runInputOptions) error {
 	if err != nil {
 		return fmt.Errorf("error building system context: %w", err)
 	}
-	mounts, mountedImages, lockedTargets, err := internalParse.GetVolumes(systemContext, store, iopts.volumes, iopts.mounts, iopts.contextDir)
+	mounts, mountedImages, intermediateMounts, _, lockedTargets, err := internalParse.GetVolumes(systemContext, store, builder.MountLabel, iopts.volumes, iopts.mounts, iopts.contextDir, tmpDir)
 	if err != nil {
 		return err
 	}
+	defer func() {
+		if err := overlay.CleanupContent(tmpDir); err != nil {
+			logrus.Debugf("unmounting overlay mounts under %q: %v", tmpDir, err)
+		}
+		for _, intermediateMount := range intermediateMounts {
+			if err := mount.Unmount(intermediateMount); err != nil {
+				logrus.Debugf("unmounting mount %q: %v", intermediateMount, err)
+			}
+			if err := os.Remove(intermediateMount); err != nil {
+				logrus.Debugf("removing should-be-empty mount directory %q: %v", intermediateMount, err)
+			}
+		}
+		for _, mountedImage := range mountedImages {
+			if _, err := store.UnmountImage(mountedImage, false); err != nil {
+				logrus.Debugf("unmounting image %q: %v", mountedImage, err)
+			}
+		}
+		// unlock if any locked files from this RUN statement
+		internalParse.UnlockLockArray(lockedTargets)
+	}()
 	options.Mounts = mounts
-	// Run() will automatically clean them up.
-	options.ExternalImageMounts = mountedImages
 	options.CgroupManager = globalFlagResults.CgroupManager
 
 	runerr := builder.Run(args, options)
@@ -181,31 +211,5 @@ func runCmd(c *cobra.Command, args []string, iopts runInputOptions) error {
 		conditionallyAddHistory(builder, c, "%s %s", shell, strings.Join(args, " "))
 		return builder.Save()
 	}
-	// unlock if any locked files from this RUN statement
-	for _, path := range lockedTargets {
-		_, err := os.Stat(path)
-		if err != nil {
-			// Lockfile not found this might be a problem,
-			// since LockedTargets must contain list of all locked files
-			// don't break here since we need to unlock other files but
-			// log so user can take a look
-			logrus.Warnf("Lockfile %q was expected here, stat failed with %v", path, err)
-			continue
-		}
-		lockfile, err := lockfile.GetLockfile(path)
-		if err != nil {
-			// unable to get lockfile
-			// lets log error and continue
-			// unlocking other files
-			logrus.Warn(err)
-			continue
-		}
-		if lockfile.Locked() {
-			lockfile.Unlock()
-		} else {
-			logrus.Warnf("Lockfile %q was expected to be locked, this is unexpected", path)
-			continue
-		}
-	}
 	return runerr
 }

define/types.go

@@ -106,13 +106,13 @@ type BuildOutputOption struct {
 	IsStdout bool
 }
 
-// TempDirForURL checks if the passed-in string looks like a URL or -.  If it is,
-// TempDirForURL creates a temporary directory, arranges for its contents to be
-// the contents of that URL, and returns the temporary directory's path, along
-// with the name of a subdirectory which should be used as the build context
-// (which may be empty or ".").  Removal of the temporary directory is the
-// responsibility of the caller.  If the string doesn't look like a URL,
-// TempDirForURL returns empty strings and a nil error code.
+// TempDirForURL checks if the passed-in string looks like a URL or "-".  If it
+// is, TempDirForURL creates a temporary directory, arranges for its contents
+// to be the contents of that URL, and returns the temporary directory's path,
+// along with the relative name of a subdirectory which should be used as the
+// build context (which may be empty or ".").  Removal of the temporary
+// directory is the responsibility of the caller.  If the string doesn't look
+// like a URL or "-", TempDirForURL returns empty strings and a nil error code.
 func TempDirForURL(dir, prefix, url string) (name string, subdir string, err error) {
 	if !strings.HasPrefix(url, "http://") &&
 		!strings.HasPrefix(url, "https://") &&
@@ -125,19 +125,24 @@ func TempDirForURL(dir, prefix, url string) (name string, subdir string, err err
 	if err != nil {
 		return "", "", fmt.Errorf("error creating temporary directory for %q: %w", url, err)
 	}
+	downloadDir := filepath.Join(name, "download")
+	if err = os.MkdirAll(downloadDir, 0o700); err != nil {
+		return "", "", fmt.Errorf("creating directory %q for %q: %w", downloadDir, url, err)
+	}
 	urlParsed, err := urlpkg.Parse(url)
 	if err != nil {
 		return "", "", fmt.Errorf("error parsing url %q: %w", url, err)
 	}
 	if strings.HasPrefix(url, "git://") || strings.HasSuffix(urlParsed.Path, ".git") {
-		combinedOutput, gitSubDir, err := cloneToDirectory(url, name)
+		combinedOutput, gitSubDir, err := cloneToDirectory(url, downloadDir)
 		if err != nil {
 			if err2 := os.RemoveAll(name); err2 != nil {
 				logrus.Debugf("error removing temporary directory %q: %v", name, err2)
 			}
 			return "", "", fmt.Errorf("cloning %q to %q:\n%s: %w", url, name, string(combinedOutput), err)
 		}
-		return name, gitSubDir, nil
+		logrus.Debugf("Build context is at %q", downloadDir)
+		return name, filepath.Join(filepath.Base(downloadDir), gitSubDir), nil
 	}
 	if strings.HasPrefix(url, "github.com/") {
 		ghurl := url
@@ -146,28 +151,29 @@ func TempDirForURL(dir, prefix, url string) (name string, subdir string, err err
 		subdir = path.Base(ghurl) + "-master"
 	}
 	if strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://") {
-		err = downloadToDirectory(url, name)
+		err = downloadToDirectory(url, downloadDir)
 		if err != nil {
 			if err2 := os.RemoveAll(name); err2 != nil {
 				logrus.Debugf("error removing temporary directory %q: %v", name, err2)
 			}
-			return "", subdir, err
+			return "", "", err
 		}
-		return name, subdir, nil
+		logrus.Debugf("Build context is at %q", filepath.Join(downloadDir, subdir))
+		return name, filepath.Join(filepath.Base(downloadDir), subdir), nil
 	}
 	if url == "-" {
-		err = stdinToDirectory(name)
+		err = stdinToDirectory(downloadDir)
 		if err != nil {
 			if err2 := os.RemoveAll(name); err2 != nil {
 				logrus.Debugf("error removing temporary directory %q: %v", name, err2)
 			}
-			return "", subdir, err
+			return "", "", err
 		}
-		logrus.Debugf("Build context is at %q", name)
-		return name, subdir, nil
+		logrus.Debugf("Build context is at %q", filepath.Join(downloadDir, subdir))
+		return name, filepath.Join(filepath.Base(downloadDir), subdir), nil
 	}
 	logrus.Debugf("don't know how to retrieve %q", url)
-	if err2 := os.Remove(name); err2 != nil {
+	if err2 := os.RemoveAll(name); err2 != nil {
 		logrus.Debugf("error removing temporary directory %q: %v", name, err2)
 	}
 	return "", "", errors.New("unreachable code reached")

docs/buildah-run.1.md

@@ -99,7 +99,7 @@ BUILDAH\_ISOLATION environment variable.  `export BUILDAH_ISOLATION=oci`
 
 Attach a filesystem mount to the container
 
-Current supported mount TYPES are bind, cache, secret and tmpfs. <sup>[[1]](#Footnote1)</sup>
+Current supported mount TYPES are bind, cache, secret and tmpfs. Writes to `bind` and `tmpfs` mounts are discarded after the command finishes, while changes to `cache` mounts persist across uses. <sup>[[1]](#Footnote1)</sup>
 
        e.g.
 
@@ -111,19 +111,19 @@ Current supported mount TYPES are bind, cache, secret and tmpfs. <sup>[[1]](#Foo
 
        Common Options:
 
-              · src, source: mount source spec for bind and volume. Mandatory for bind. If `from` is specified, `src` is the subpath in the `from` field.
+              · src, source: mount source spec for bind and cache. Mandatory for bind. If `from` is specified, `src` is the subpath in the `from` field.
 
-              · dst, destination, target: mount destination spec.
+              · dst, destination, target: location where the command being run should see the content being mounted.
 
-              · ro, read-only: true or false (default).
+              · ro, read-only: (default true for `type=bind`, false for `type=tmpfs`, `type=cache`).
 
        Options specific to bind:
 
               · bind-propagation: shared, slave, private, rshared, rslave, or rprivate(default). See also mount(2).
 
               . bind-nonrecursive: do not setup a recursive bind mount.  By default it is recursive.
 
-              · from: stage or image name for the root of the source. Defaults to the build context.
+              · from: image name for the root of the source. Defaults to **--contextdir**, mandatory if **--contextdir** was not specified.
 
               · z: Set shared SELinux label on mounted destination. Use if SELinux is enabled on host machine.
 
@@ -143,7 +143,7 @@ Current supported mount TYPES are bind, cache, secret and tmpfs. <sup>[[1]](#Foo
 
        Options specific to cache:
 
-              · id: Create a separate cache directory for a particular id.
+              · id: Distinguish this cache from other caches using this ID rather than the target mount path.
 
               · mode: File mode for new cache directory in octal. Default 0755.
 
@@ -155,6 +155,8 @@ Current supported mount TYPES are bind, cache, secret and tmpfs. <sup>[[1]](#Foo
 
               · from: stage name for the root of the source. Defaults to host cache directory.
 
+              · sharing: Whether other users of this cache need to wait for this command to complete (`sharing=locked`) or not (`sharing=shared`, which is the default).
+
               · z: Set shared SELinux label on mounted destination. Use if SELinux is enabled on host machine.
 
               · Z: Set private SELinux label on mounted destination. Use if SELinux is enabled on host machine.

imagebuildah/stage_executor.go

@@ -516,7 +516,11 @@ func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]inte
 							// to `mountPoint` replaced from additional
 							// build-context. Reason: Parser will use this
 							//  `from` to refer from stageMountPoints map later.
-							stageMountPoints[from] = internal.StageMountDetails{IsStage: false, MountPoint: mountPoint}
+							stageMountPoints[from] = internal.StageMountDetails{
+								IsAdditionalBuildContext: true,
+								IsImage:                  true,
+								MountPoint:               mountPoint,
+							}
 							break
 						} else {
 							// Most likely this points to path on filesystem
@@ -548,7 +552,10 @@ func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]inte
 									mountPoint = additionalBuildContext.DownloadedCache
 								}
 							}
-							stageMountPoints[from] = internal.StageMountDetails{IsStage: true, MountPoint: mountPoint}
+							stageMountPoints[from] = internal.StageMountDetails{
+								IsAdditionalBuildContext: true,
+								MountPoint:               mountPoint,
+							}
 							break
 						}
 					}
@@ -559,14 +566,20 @@ func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]inte
 						return nil, err
 					}
 					if otherStage, ok := s.executor.stages[from]; ok && otherStage.index < s.index {
-						stageMountPoints[from] = internal.StageMountDetails{IsStage: true, MountPoint: otherStage.mountPoint}
+						stageMountPoints[from] = internal.StageMountDetails{
+							IsStage:    true,
+							MountPoint: otherStage.mountPoint,
+						}
 						break
 					} else {
 						mountPoint, err := s.getImageRootfs(s.ctx, from)
 						if err != nil {
 							return nil, fmt.Errorf("%s from=%s: no stage or image found with that name", flag, from)
 						}
-						stageMountPoints[from] = internal.StageMountDetails{IsStage: false, MountPoint: mountPoint}
+						stageMountPoints[from] = internal.StageMountDetails{
+							IsImage:    true,
+							MountPoint: mountPoint,
+						}
 						break
 					}
 				default:

internal/open/open.go

@@ -0,0 +1,39 @@
+package open
+
+import (
+	"errors"
+	"fmt"
+	"syscall"
+)
+
+// InChroot opens the file at `path` after chrooting to `root` and then
+// changing its working directory to `wd`.  Both `wd` and `path` are evaluated
+// in the chroot.
+// Returns a file handle, an Errno value if there was an error and the
+// underlying error was a standard library error code, and a non-empty error if
+// one was detected.
+func InChroot(root, wd, path string, mode int, perm uint32) (fd int, errno syscall.Errno, err error) {
+	requests := requests{
+		Root: root,
+		Wd:   wd,
+		Open: []request{
+			{
+				Path:  path,
+				Mode:  mode,
+				Perms: perm,
+			},
+		},
+	}
+	results := inChroot(requests)
+	if len(results.Open) != 1 {
+		return -1, 0, fmt.Errorf("got %d results back instead of 1", len(results.Open))
+	}
+	if results.Open[0].Err != "" {
+		if results.Open[0].Errno != 0 {
+			err = fmt.Errorf("%s: %w", results.Open[0].Err, results.Open[0].Errno)
+		} else {
+			err = errors.New(results.Open[0].Err)
+		}
+	}
+	return int(results.Open[0].Fd), results.Open[0].Errno, err
+}