From 6a58b69e6b97580f916755f0da615b7dbd58d7cc Mon Sep 17 00:00:00 2001 From: Dave Dykstra <2129743+DrDaveD@users.noreply.github.com> Date: Oct 02 2019 20:15:41 +0000 Subject: add PR 4522 in version 3.4.1-1.2 --- diff --git a/4522.patch b/4522.patch new file mode 100644 index 0000000..026726e --- /dev/null +++ b/4522.patch @@ -0,0 +1,253 @@ +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 + } diff --git a/singularity.spec b/singularity.spec index 3ac2d43..ea66a36 100644 --- a/singularity.spec +++ b/singularity.spec @@ -30,13 +30,15 @@ Summary: Application and environment virtualization Name: singularity Version: 3.4.1 -Release: 1.1%{?dist} +Release: 1.2%{?dist} # https://spdx.org/licenses/BSD-3-Clause-LBNL.html License: BSD-3-Clause-LBNL URL: https://www.sylabs.io/singularity/ Source: %{name}-%{version}.tar.gz # https://github.com/sylabs/singularity/pull/4346.patch Patch0: 4346.patch +# https://github.com/sylabs/singularity/pull/4522.patch +Patch1: 4522.patch ExclusiveOS: linux # RPM_BUILD_ROOT wasn't being set ... for some reason %if "%{sles_version}" == "11" @@ -99,6 +101,7 @@ export PATH=$GOPATH/bin:$PATH cd $GOPATH/%{singgopath} patch -p1 <%{PATCH0} +patch -p1 <%{PATCH1} # Not all of these parameters currently have an effect, but they might be # used someday. They are the same parameters as in the configure macro. @@ -165,6 +168,9 @@ chmod 644 $RPM_BUILD_ROOT%{_sysconfdir}/singularity/actions/* %changelog +* Thu Sep 26 2019 Dave Dykstra - 3.4.1-1.2 +- Add PR #4522 to fix sandbox rootless builds + * Mon Sep 23 2019 Dave Dykstra - 3.4.1-1.1 - Update to upstream 3.4.0-1, keeping only config fakeroot cli PR #4346