Backport 34ae47a22629 the real one

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

contrib/rpm/buildah.spec

@@ -26,7 +26,7 @@
 
 Name:           buildah
 # Bump version in buildah.go too
-Version:        1.19.7
+Version:        1.19.8
 Release:        1.git%{shortcommit}%{?dist}
 Summary:        A command line tool used to creating OCI Images
 License:        ASL 2.0
@@ -100,6 +100,9 @@ make DESTDIR=%{buildroot} PREFIX=%{_prefix} install install.completions
 %{_datadir}/bash-completion/completions/*
 
 %changelog
+* Mon Mar 8, 2021 Valentin Rothberg <[email protected]> 1.19.8-1
+- copier: support ignoring EPERMs
+
 * Thu Mar 4, 2021 Dan Walsh <[email protected]> 1.19.7-1
 - Set upperdir permissions based on source
 

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)

copier/copier_linux_test.go

@@ -0,0 +1,149 @@
+package copier
+
+import (
+	"archive/tar"
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/containers/storage/pkg/reexec"
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/syndtr/gocapability/capability"
+)
+
+func init() {
+	reexec.Register("get", getWrappedMain)
+}
+
+type getWrappedOptions struct {
+	Root, Directory string
+	GetOptions      GetOptions
+	Globs           []string
+	DropCaps        []capability.Cap
+}
+
+func getWrapped(root string, directory string, getOptions GetOptions, globs []string, dropCaps []capability.Cap, bulkWriter io.Writer) error {
+	options := getWrappedOptions{
+		Root:       root,
+		Directory:  directory,
+		GetOptions: getOptions,
+		Globs:      globs,
+		DropCaps:   dropCaps,
+	}
+	encoded, err := json.Marshal(&options)
+	if err != nil {
+		return errors.Wrapf(err, "error marshalling options")
+	}
+	cmd := reexec.Command("get")
+	cmd.Env = append(cmd.Env, "OPTIONS="+string(encoded))
+	cmd.Stdout = bulkWriter
+	stderrBuf := bytes.Buffer{}
+	cmd.Stderr = &stderrBuf
+	err = cmd.Run()
+	if stderrBuf.Len() > 0 {
+		if err != nil {
+			return fmt.Errorf("%v: %s", err, stderrBuf.String())
+		}
+		return fmt.Errorf("%s", stderrBuf.String())
+	}
+	return err
+}
+
+func getWrappedMain() {
+	var options getWrappedOptions
+	if err := json.Unmarshal([]byte(os.Getenv("OPTIONS")), &options); err != nil {
+		fmt.Fprintf(os.Stderr, "%v", err)
+		os.Exit(1)
+	}
+	if len(options.DropCaps) > 0 {
+		caps, err := capability.NewPid(0)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "%v", err)
+			os.Exit(1)
+		}
+		for _, capType := range []capability.CapType{
+			capability.AMBIENT,
+			capability.BOUNDING,
+			capability.INHERITABLE,
+			capability.PERMITTED,
+			capability.EFFECTIVE,
+		} {
+			for _, cap := range options.DropCaps {
+				if caps.Get(capType, cap) {
+					caps.Unset(capType, cap)
+				}
+			}
+			if err := caps.Apply(capType); err != nil {
+				fmt.Fprintf(os.Stderr, "error dropping capability %+v: %v", options.DropCaps, err)
+				os.Exit(1)
+			}
+		}
+	}
+	if err := Get(options.Root, options.Directory, options.GetOptions, options.Globs, os.Stdout); err != nil {
+		fmt.Fprintf(os.Stderr, "%v", err)
+		os.Exit(1)
+	}
+}
+
+func TestGetPermissionErrorNoChroot(t *testing.T) {
+	couldChroot := canChroot
+	canChroot = false
+	testGetPermissionError(t)
+	canChroot = couldChroot
+}
+
+func TestGetPermissionErrorChroot(t *testing.T) {
+	if uid != 0 {
+		t.Skipf("chroot() requires root privileges, skipping")
+	}
+	couldChroot := canChroot
+	canChroot = true
+	testGetPermissionError(t)
+	canChroot = couldChroot
+}
+
+func testGetPermissionError(t *testing.T) {
+	dropCaps := []capability.Cap{capability.CAP_DAC_OVERRIDE, capability.CAP_DAC_READ_SEARCH}
+	tmp, err := ioutil.TempDir("", "")
+	require.NoError(t, err, "error creating temp directory")
+	err = os.Mkdir(filepath.Join(tmp, "unreadable-directory"), 0000)
+	require.NoError(t, err, "error creating an unreadable directory")
+	err = os.Mkdir(filepath.Join(tmp, "readable-directory"), 0755)
+	require.NoError(t, err, "error creating a readable directory")
+	err = os.Mkdir(filepath.Join(tmp, "readable-directory", "unreadable-subdirectory"), 0000)
+	require.NoError(t, err, "error creating an unreadable subdirectory")
+	err = ioutil.WriteFile(filepath.Join(tmp, "unreadable-file"), []byte("hi, i'm a file that you can't read"), 0000)
+	require.NoError(t, err, "error creating an unreadable file")
+	err = ioutil.WriteFile(filepath.Join(tmp, "readable-file"), []byte("hi, i'm also a file, and you can read me"), 0644)
+	require.NoError(t, err, "error creating a readable file")
+	err = ioutil.WriteFile(filepath.Join(tmp, "readable-directory", "unreadable-file"), []byte("hi, i'm also a file that you can't read"), 0000)
+	require.NoError(t, err, "error creating an unreadable file in a readable directory")
+	for _, ignore := range []bool{false, true} {
+		t.Run(fmt.Sprintf("ignore=%v", ignore), func(t *testing.T) {
+			var buf bytes.Buffer
+			err = getWrapped(tmp, tmp, GetOptions{IgnoreUnreadable: ignore}, []string{"."}, dropCaps, &buf)
+			if ignore {
+				assert.NoError(t, err, "expected no errors")
+				tr := tar.NewReader(&buf)
+				items := 0
+				_, err := tr.Next()
+				for err == nil {
+					items++
+					_, err = tr.Next()
+				}
+				assert.True(t, errors.Is(err, io.EOF), "expected EOF to finish read contents")
+				assert.Equalf(t, 2, items, "expected two readable items, got %d", items)
+			} else {
+				assert.Error(t, err, "expected an error")
+				assert.Truef(t, errorIsPermission(err), "expected the error (%v) to be a permission error", err)
+			}
+		})
+	}
+}

copier/copier_test.go

@@ -491,9 +491,8 @@ func testPut(t *testing.T) {
 					t.Skipf("test archive %q can only be tested with root privileges, skipping", testArchives[i].name)
 				}
 
-				tmp, err := ioutil.TempDir("", "copier-test-")
-				require.NoError(t, err, "error creating temporary directory")
-				defer os.RemoveAll(tmp)
+				tmp, err := ioutil.TempDir("", "")
+				require.NoError(t, err, "error creating temp directory")
 
 				archive := makeArchive(testArchives[i].headers, testArchives[i].contents)
 				err = Put(tmp, tmp, PutOptions{UIDMap: uidMap, GIDMap: gidMap, Rename: renames.renames}, archive)
@@ -532,9 +531,8 @@ func testPut(t *testing.T) {
 					{Name: "test", Typeflag: tar.TypeDir, Size: 0, Mode: 0755, ModTime: testDate},
 					{Name: "test", Typeflag: typeFlag, Size: 0, Mode: 0755, Linkname: "target", ModTime: testDate},
 				})
-				tmp, err := ioutil.TempDir("", "copier-test-")
-				require.NoError(t, err, "error creating temporary directory")
-				defer os.RemoveAll(tmp)
+				tmp, err := ioutil.TempDir("", "")
+				require.NoError(t, err, "error creating temp directory")
 				err = Put(tmp, tmp, PutOptions{UIDMap: uidMap, GIDMap: gidMap, NoOverwriteDirNonDir: !overwrite}, bytes.NewReader(archive))
 				if overwrite {
 					if unwrapError(err) != syscall.EPERM {
@@ -558,9 +556,8 @@ func testPut(t *testing.T) {
 					{Name: "link", Typeflag: tar.TypeLink, Size: 0, Mode: 0600, ModTime: testDate, Linkname: "test"},
 					{Name: "unrelated", Typeflag: tar.TypeReg, Size: 0, Mode: 0600, ModTime: testDate},
 				})
-				tmp, err := ioutil.TempDir("", "copier-test-")
-				require.NoError(t, err, "error creating temporary directory")
-				defer os.RemoveAll(tmp)
+				tmp, err := ioutil.TempDir("", "")
+				require.NoError(t, err, "error creating temp directory")
 				err = Put(tmp, tmp, PutOptions{UIDMap: uidMap, GIDMap: gidMap, IgnoreDevices: ignoreDevices}, bytes.NewReader(archive))
 				require.Nilf(t, err, "expected to extract content with typeflag %c without an error: %v", typeFlag, err)
 				fileList, err := enumerateFiles(tmp)