From 712dadda599ef9fe5982c00e776127653a836a3d Mon Sep 17 00:00:00 2001
From: David Trudgian <dave@trudgian.net>
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
}