remote,build: error if containerignore is symlink

Drop support for remote use-cases when `.containerignore` or
`.dockerignore` is a symlink pointing to arbitrary location on host.

This backport addresses: CVE-2022-4122
https://issues.redhat.com/browse/RHEL-13467

Signed-off-by: Aditya R <arajan@redhat.com>
Signed-off-by: tomsweeneyredhat <tsweeney@redhat.com>

pkg/api/handlers/compat/images_build.go

@@ -23,6 +23,7 @@ import (
 	"github.com/containers/podman/v4/pkg/auth"
 	"github.com/containers/podman/v4/pkg/channel"
 	"github.com/containers/podman/v4/pkg/rootless"
+	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/storage/pkg/archive"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/gorilla/schema"
@@ -502,6 +503,12 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 	reporter := channel.NewWriter(make(chan []byte))
 	defer reporter.Close()
 
+	_, ignoreFile, err := util.ParseDockerignore(containerFiles, contextDirectory)
+	if err != nil {
+		utils.Error(w, http.StatusInternalServerError, fmt.Errorf("processing ignore file: %w", err))
+		return
+	}
+
 	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
 	buildOptions := buildahDefine.BuildOptions{
 		AddCapabilities: addCaps,
@@ -540,6 +547,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		ForceRmIntermediateCtrs:        query.ForceRm,
 		From:                           fromImage,
 		IgnoreUnrecognizedInstructions: query.Ignore,
+		IgnoreFile:                     ignoreFile,
 		Isolation:                      isolation,
 		Jobs:                           &jobs,
 		Labels:                         labels,

pkg/bindings/images/build.go

@@ -21,6 +21,7 @@ import (
 	"github.com/containers/podman/v4/pkg/auth"
 	"github.com/containers/podman/v4/pkg/bindings"
 	"github.com/containers/podman/v4/pkg/domain/entities"
+	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/storage/pkg/fileutils"
 	"github.com/containers/storage/pkg/ioutils"
 	"github.com/docker/go-units"
@@ -317,14 +318,6 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 		stdout = options.Out
 	}
 
-	excludes := options.Excludes
-	if len(excludes) == 0 {
-		excludes, err = parseDockerignore(options.ContextDirectory)
-		if err != nil {
-			return nil, err
-		}
-	}
-
 	contextDir, err := filepath.Abs(options.ContextDirectory)
 	if err != nil {
 		logrus.Errorf("Cannot find absolute path of %v: %v", options.ContextDirectory, err)
@@ -370,6 +363,8 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 		if strings.HasPrefix(containerfile, contextDir+string(filepath.Separator)) {
 			containerfile = strings.TrimPrefix(containerfile, contextDir+string(filepath.Separator))
 			dontexcludes = append(dontexcludes, "!"+containerfile)
+			dontexcludes = append(dontexcludes, "!"+containerfile+".dockerignore")
+			dontexcludes = append(dontexcludes, "!"+containerfile+".containerignore")
 		} else {
 			// If Containerfile does not exists assume it is in context directory, do Not add to tarfile
 			if _, err := os.Lstat(containerfile); err != nil {
@@ -377,6 +372,9 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 					return nil, err
 				}
 				containerfile = c
+				dontexcludes = append(dontexcludes, "!"+containerfile)
+				dontexcludes = append(dontexcludes, "!"+containerfile+".dockerignore")
+				dontexcludes = append(dontexcludes, "!"+containerfile+".containerignore")
 			} else {
 				// If Containerfile does exists but is not in context directory add it to the tarfile
 				tarContent = append(tarContent, containerfile)
@@ -384,6 +382,7 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 		}
 		newContainerFiles = append(newContainerFiles, filepath.ToSlash(containerfile))
 	}
+
 	if len(newContainerFiles) > 0 {
 		cFileJSON, err := json.Marshal(newContainerFiles)
 		if err != nil {
@@ -392,6 +391,14 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 		params.Set("dockerfile", string(cFileJSON))
 	}
 
+	excludes := options.Excludes
+	if len(excludes) == 0 {
+		excludes, _, err = util.ParseDockerignore(newContainerFiles, options.ContextDirectory)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	// build secrets are usually absolute host path or relative to context dir on host
 	// in any case move secret to current context and ship the tar.
 	if secrets := options.CommonBuildOpts.Secrets; len(secrets) > 0 {
@@ -651,23 +658,3 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) {
 	})
 	return rc, nil
 }
-
-func parseDockerignore(root string) ([]string, error) {
-	ignore, err := ioutil.ReadFile(filepath.Join(root, ".containerignore"))
-	if err != nil {
-		var dockerIgnoreErr error
-		ignore, dockerIgnoreErr = ioutil.ReadFile(filepath.Join(root, ".dockerignore"))
-		if dockerIgnoreErr != nil && !os.IsNotExist(dockerIgnoreErr) {
-			return nil, errors.Wrapf(err, "error reading .containerignore: '%s'", root)
-		}
-	}
-	rawexcludes := strings.Split(string(ignore), "\n")
-	excludes := make([]string, 0, len(rawexcludes))
-	for _, e := range rawexcludes {
-		if len(e) == 0 || e[0] == '#' {
-			continue
-		}
-		excludes = append(excludes, e)
-	}
-	return excludes, nil
-}

pkg/util/utils.go

@@ -23,6 +23,7 @@ import (
 	"github.com/containers/podman/v4/pkg/signal"
 	"github.com/containers/storage/pkg/idtools"
 	stypes "github.com/containers/storage/types"
+	securejoin "github.com/cyphar/filepath-securejoin"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
@@ -54,6 +55,76 @@ func parseCreds(creds string) (string, string) {
 	return up[0], up[1]
 }
 
+// Takes build context and validates `.containerignore` or `.dockerignore`
+// if they are symlink outside of buildcontext. Returns list of files to be
+// excluded and resolved path to the ignore files inside build context or error
+func ParseDockerignore(containerfiles []string, root string) ([]string, string, error) {
+	ignoreFile := ""
+	path, err := securejoin.SecureJoin(root, ".containerignore")
+	if err != nil {
+		return nil, ignoreFile, err
+	}
+	// set resolved ignore file so imagebuildah
+	// does not attempts to re-resolve it
+	ignoreFile = path
+	ignore, err := os.ReadFile(path)
+	if err != nil {
+		var dockerIgnoreErr error
+		path, symlinkErr := securejoin.SecureJoin(root, ".dockerignore")
+		if symlinkErr != nil {
+			return nil, ignoreFile, symlinkErr
+		}
+		// set resolved ignore file so imagebuildah
+		// does not attempts to re-resolve it
+		ignoreFile = path
+		ignore, dockerIgnoreErr = os.ReadFile(path)
+		if os.IsNotExist(dockerIgnoreErr) {
+			// In this case either ignorefile was not found
+			// or it is a symlink to unexpected file in such
+			// case manually set ignorefile to `/dev/null` so
+			// internally imagebuildah does not attempts to re-resolve
+			// this invalid symlink and instead reads a blank file.
+			ignoreFile = "/dev/null"
+		}
+		// after https://github.com/containers/buildah/pull/4239 build supports
+		// <Containerfile>.containerignore or <Containerfile>.dockerignore as ignore file
+		// so remote must support parsing that.
+		if dockerIgnoreErr != nil {
+			for _, containerfile := range containerfiles {
+				if _, err := os.Stat(filepath.Join(root, containerfile+".containerignore")); err == nil {
+					path, symlinkErr = securejoin.SecureJoin(root, containerfile+".containerignore")
+					if symlinkErr == nil {
+						ignoreFile = path
+						ignore, dockerIgnoreErr = os.ReadFile(path)
+					}
+				}
+				if _, err := os.Stat(filepath.Join(root, containerfile+".dockerignore")); err == nil {
+					path, symlinkErr = securejoin.SecureJoin(root, containerfile+".dockerignore")
+					if symlinkErr == nil {
+						ignoreFile = path
+						ignore, dockerIgnoreErr = os.ReadFile(path)
+					}
+				}
+				if dockerIgnoreErr == nil {
+					break
+				}
+			}
+		}
+		if dockerIgnoreErr != nil && !os.IsNotExist(dockerIgnoreErr) {
+			return nil, ignoreFile, err
+		}
+	}
+	rawexcludes := strings.Split(string(ignore), "\n")
+	excludes := make([]string, 0, len(rawexcludes))
+	for _, e := range rawexcludes {
+		if len(e) == 0 || e[0] == '#' {
+			continue
+		}
+		excludes = append(excludes, e)
+	}
+	return excludes, ignoreFile, nil
+}
+
 // ParseRegistryCreds takes a credentials string in the form USERNAME:PASSWORD
 // and returns a DockerAuthConfig
 func ParseRegistryCreds(creds string) (*types.DockerAuthConfig, error) {

test/buildah-bud/apply-podman-deltas

@@ -193,8 +193,9 @@ skip_if_remote "--stdin option will not be implemented in podman-remote" \
 ###############################################################################
 # BEGIN tests which are skipped due to actual podman or podman-remote bugs.
 
-skip_if_remote "Podman #12838: different error messages" \
-               "bud with .dockerignore #2"
+skip_if_remote "different error messages between podman & podman-remote" \
+               "bud with .dockerignore #2" \
+               "bud with .dockerignore #4"
 
 # These two tests, new in 2022-01, invoke podman (create, export) in ways
 # that don't work with podman-remote due to the use of --registries-conf

test/e2e/build/containerignore-symlink/.dockerignore

@@ -0,0 +1 @@
+/tmp/private_file

test/e2e/build/containerignore-symlink/Dockerfile

@@ -0,0 +1,2 @@
+FROM alpine
+COPY / /dir

test/e2e/build_test.go

@@ -372,6 +372,33 @@ RUN find /test`, ALPINE)
 		Expect(session.OutputToString()).To(ContainSubstring("/test/dummy"))
 	})
 
+	It("podman remote build must not allow symlink for ignore files", func() {
+		// Create a random file where symlink must be resolved
+		// but build should not be able to access it.
+		f, err := os.Create(filepath.Join("/tmp", "private_file"))
+		Expect(err).ToNot(HaveOccurred())
+		// Mark hello to be ignored in outerfile, but it should not be ignored.
+		_, err = f.WriteString("hello\n")
+		Expect(err).ToNot(HaveOccurred())
+		defer f.Close()
+
+		if IsRemote() {
+			podmanTest.StopRemoteService()
+			podmanTest.StartRemoteService()
+		} else {
+			Skip("Only valid at remote test")
+		}
+
+		session := podmanTest.Podman([]string{"build", "--pull-never", "-t", "test", "build/containerignore-symlink/"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"run", "--rm", "test", "ls", "/dir"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		Expect(session.OutputToString()).To(ContainSubstring("hello"))
+	})
+
 	It("podman remote test container/docker file is not at root of context dir", func() {
 		if IsRemote() {
 			podmanTest.StopRemoteService()