From 0718d30723e6fb705b61316d1fa18f0f91de1a58 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Wed, 3 Mar 2021 16:45:43 -0500 Subject: [PATCH 1/2] copier: add GetOptions.IgnoreUnreadable Add an IgnoreUnreadable flag to copier.GetOptions to suppress errors from copier.Get() that would pass the os.IsPermission() test, if they're encountered while attempting to read files or descend into directories. Backported-by: Valentin Rothberg Signed-off-by: Nalin Dahyabhai --- copier/copier.go | 42 ++++++++-- copier/copier_linux_test.go | 149 ++++++++++++++++++++++++++++++++++++ copier/copier_test.go | 15 ++-- 3 files changed, 190 insertions(+), 16 deletions(-) create mode 100644 copier/copier_linux_test.go diff --git a/copier/copier.go b/copier/copier.go index b5e107d4ba0..52d8133c712 100644 --- a/copier/copier.go +++ b/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) diff --git a/copier/copier_linux_test.go b/copier/copier_linux_test.go new file mode 100644 index 00000000000..8aa0d53368f --- /dev/null +++ b/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) + } + }) + } +} diff --git a/copier/copier_test.go b/copier/copier_test.go index 59e3c6f7a83..f19736daddc 100644 --- a/copier/copier_test.go +++ b/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) From d1c9523dcfb96a286c78da317fb3844ddd5b6476 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Mar 2021 16:07:21 +0100 Subject: [PATCH 2/2] v1.19.8 * copier: add GetOptions.IgnoreUnreadable Signed-off-by: Valentin Rothberg --- buildah.go | 2 +- contrib/rpm/buildah.spec | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/buildah.go b/buildah.go index 77d313c5866..427950c5cab 100644 --- a/buildah.go +++ b/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 diff --git a/contrib/rpm/buildah.spec b/contrib/rpm/buildah.spec index 8866e92b1d2..f936f65ecdc 100644 --- a/contrib/rpm/buildah.spec +++ b/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 1.19.8-1 +- copier: support ignoring EPERMs + * Thu Mar 4, 2021 Dan Walsh 1.19.7-1 - Set upperdir permissions based on source