From f51057680c6fea81f449c83f0f2e0aeac422a5bb Mon Sep 17 00:00:00 2001 From: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com> Date: Oct 22 2019 20:00:07 +0000 Subject: 4522.patch no longer needed --- diff --git a/4522.patch b/4522.patch deleted file mode 100644 index 026726e..0000000 --- a/4522.patch +++ /dev/null @@ -1,253 +0,0 @@ -From 712dadda599ef9fe5982c00e776127653a836a3d Mon Sep 17 00:00:00 2001 -From: David Trudgian -Date: Wed, 25 Sep 2019 15:48:09 -0500 -Subject: [PATCH] Fix #4524 - set permissions on rootless sandboxes - -This fix addresses #4524, where the recent change to use the umoci -extracter for OCI layers can lead to sandboxes that cannot be mv'd across -filesystems (leading to a build failure), or deleted/modified by the -user. - -Return to the <=3.3 behaviour here, by modifying permissions on the -temporary rootfs sandbox once it is extracted, and before the mv to -its final location. ---- - e2e/internal/e2e/fileutil.go | 11 ++ - e2e/regressions/build.go | 50 +++++++- - .../pkg/build/sources/oci_unpack_linux.go | 119 +++++++++++++++++- - 3 files changed, 177 insertions(+), 3 deletions(-) - -diff --git a/e2e/internal/e2e/fileutil.go b/e2e/internal/e2e/fileutil.go -index ff4f5131dc..c7b59399c9 100644 ---- a/e2e/internal/e2e/fileutil.go -+++ b/e2e/internal/e2e/fileutil.go -@@ -88,3 +88,14 @@ func FileExists(t *testing.T, path string) bool { - // todo: we should check if it is a file - return PathExists(t, path) - } -+ -+// PathPerms return true if the path (file or directory) has specified permissions, false otherwise. -+func PathPerms(t *testing.T, path string, perms os.FileMode) bool { -+ s, err := os.Stat(path) -+ -+ if err != nil { -+ t.Fatalf("While stating file: %v", err) -+ } -+ -+ return s.Mode().Perm() == perms -+} -diff --git a/e2e/regressions/build.go b/e2e/regressions/build.go -index 01156c4625..4394727bb4 100644 ---- a/e2e/regressions/build.go -+++ b/e2e/regressions/build.go -@@ -7,6 +7,7 @@ package regressions - - import ( - "os" -+ "path" - "path/filepath" - "testing" - -@@ -56,6 +57,51 @@ func (c *regressionsTests) issue4203(t *testing.T) { - ) - } - -+// This test will build a sandbox, as a non-root user from a dockerhub image -+// that contains a single folder and file with `000` permissions. -+// To verify we force files to be accessible / moveable / removable by the user -+// we check for `700` and `400` permissions on the folder and file respectively. -+func (c *regressionsTests) issue4524(t *testing.T) { -+ sandbox := filepath.Join(c.env.TestDir, "issue_4524") -+ -+ c.env.RunSingularity( -+ t, -+ e2e.WithPrivileges(false), -+ e2e.WithCommand("build"), -+ e2e.WithArgs("--sandbox", sandbox, "docker://sylabsio/issue4524"), -+ e2e.PostRun(func(t *testing.T) { -+ -+ // If we failed to build the sandbox completely, leave what we have for -+ // investigation. -+ if t.Failed() { -+ t.Logf("Test %s failed, not removing directory %s", t.Name(), sandbox) -+ return -+ } -+ -+ if !e2e.PathPerms(t, path.Join(sandbox, "directory"), 0700) { -+ t.Error("Expected 0700 permissions on 000 test directory in rootless sandbox") -+ } -+ if !e2e.PathPerms(t, path.Join(sandbox, "file"), 0600) { -+ t.Error("Expected 0600 permissions on 000 test file in rootless sandbox") -+ } -+ -+ // If the permissions aren't as we expect them to be, leave what we have for -+ // investigation. -+ if t.Failed() { -+ t.Logf("Test %s failed, not removing directory %s", t.Name(), sandbox) -+ return -+ } -+ -+ err := os.RemoveAll(sandbox) -+ if err != nil { -+ t.Logf("Cannot remove sandbox directory: %#v", err) -+ } -+ -+ }), -+ e2e.ExpectExit(0), -+ ) -+} -+ - // RunE2ETests is the main func to trigger the test suite - func RunE2ETests(env e2e.TestEnv) func(*testing.T) { - c := ®ressionsTests{ -@@ -63,7 +109,7 @@ func RunE2ETests(env e2e.TestEnv) func(*testing.T) { - } - - return func(t *testing.T) { -- // https://github.com/sylabs/singularity/issues/4203 -- t.Run("Issue4203", c.issue4203) -+ t.Run("Issue 4203", c.issue4203) // https://github.com/sylabs/singularity/issues/4203 -+ t.Run("Issue 4524", c.issue4524) // https://github.com/sylabs/singularity/issues/4524 - } - } -diff --git a/internal/pkg/build/sources/oci_unpack_linux.go b/internal/pkg/build/sources/oci_unpack_linux.go -index decbe585ee..c6ed43f987 100644 ---- a/internal/pkg/build/sources/oci_unpack_linux.go -+++ b/internal/pkg/build/sources/oci_unpack_linux.go -@@ -14,12 +14,15 @@ import ( - "encoding/json" - "fmt" - "os" -+ "path/filepath" -+ "sort" - - "github.com/containers/image/types" - "github.com/openSUSE/umoci" - umocilayer "github.com/openSUSE/umoci/oci/layer" - "github.com/openSUSE/umoci/pkg/idtools" - imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" -+ "github.com/sylabs/singularity/internal/pkg/sylog" - sytypes "github.com/sylabs/singularity/pkg/build/types" - ) - -@@ -68,5 +71,119 @@ func unpackRootfs(b *sytypes.Bundle, tmpfsRef types.ImageReference, sysCtx *type - os.RemoveAll(b.Rootfs()) - - // Unpack root filesystem -- return umocilayer.UnpackRootfs(context.Background(), engineExt, b.Rootfs(), manifest, &mapOptions) -+ err = umocilayer.UnpackRootfs(context.Background(), engineExt, b.Rootfs(), manifest, &mapOptions) -+ if err != nil { -+ return fmt.Errorf("error unpacking rootfs: %s", err) -+ } -+ -+ // If this is a rootless extraction we need to mangle permissions to fix #4524. This -+ // returns to the <=3.3 permissions on the rootfs, with the exception that umoci -+ // correctly applies permission changes across layers when extracting. -+ if mapOptions.Rootless { -+ sylog.Debugf("Modifying rootless permissions on temporary rootfs") -+ return fixPermsRootless(b.Rootfs()) -+ } -+ -+ return nil -+} -+ -+// fixPermsRootless forces permissions on the rootfs so that it can be easily -+// moved and deleted by a non-root user owner. -+func fixPermsRootless(rootfs string) (err error) { -+ errors := 0 -+ err = permWalk(rootfs, func(path string, f os.FileInfo, err error) error { -+ if err != nil { -+ sylog.Errorf("Unable to access sandbox path %s: %s", path, err) -+ errors++ -+ return nil -+ } -+ // Directories must have the owner 'rx' bits to allow traversal and reading on move, and the 'w' bit -+ // so their content can be deleted by the user when the rootfs/sandbox is deleted -+ switch mode := f.Mode(); { -+ case mode.IsDir(): -+ if err := os.Chmod(path, f.Mode().Perm()|0700); err != nil { -+ sylog.Errorf("Error setting rootless permission for %s: %s", path, err) -+ errors++ -+ } -+ case mode.IsRegular(): -+ // Regular files must have the owner 'r' bit so that everything can be read in order to -+ // copy or move the rootfs/sandbox around. Also, the `w` bit as the build does write into -+ // some files (e.g. resolv.conf) in the container rootfs. -+ if err := os.Chmod(path, f.Mode().Perm()|0600); err != nil { -+ sylog.Errorf("Error setting rootless permission for %s: %s", path, err) -+ errors++ -+ } -+ } -+ return nil -+ }) -+ -+ if errors > 0 { -+ err = fmt.Errorf("%d errors were encountered when setting permissions", errors) -+ } -+ -+ return err -+} -+ -+// permWalk is similar to os.Walk - but: -+// 1. The skipDir checks are removed (we never want to skip anything here) -+// 2. Our walk will call walkFn on a directory *before* attempting to look -+// inside that directory. -+func permWalk(root string, walkFn filepath.WalkFunc) error { -+ info, err := os.Lstat(root) -+ if err != nil { -+ return fmt.Errorf("could not access rootfs %s: %s", root, err) -+ } -+ return walk(root, info, walkFn) -+} -+ -+func walk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { -+ if !info.IsDir() { -+ return walkFn(path, info, nil) -+ } -+ -+ // Unlike filepath.walk we call walkFn *before* trying to list the content of -+ // the directory, so that walkFn has a chance to assign perms that allow us into -+ // the directory, if we can't get in there already. -+ if err := walkFn(path, info, nil); err != nil { -+ return err -+ } -+ -+ names, err := readDirNames(path) -+ if err != nil { -+ return err -+ } -+ -+ for _, name := range names { -+ filename := filepath.Join(path, name) -+ fileInfo, err := os.Lstat(filename) -+ if err != nil { -+ if err := walkFn(filename, fileInfo, err); err != nil { -+ return err -+ } -+ } else { -+ err = walk(filename, fileInfo, walkFn) -+ if err != nil { -+ if !fileInfo.IsDir() { -+ return err -+ } -+ } -+ } -+ } -+ return nil -+} -+ -+// readDirNames reads the directory named by dirname and returns -+// a sorted list of directory entries. -+func readDirNames(dirname string) ([]string, error) { -+ f, err := os.Open(dirname) -+ if err != nil { -+ return nil, err -+ } -+ names, err := f.Readdirnames(-1) -+ f.Close() -+ if err != nil { -+ return nil, err -+ } -+ sort.Strings(names) -+ return names, nil - }