snap-update-ns: WIP on generalized mount profiles

This unfinished change lets snap-update-ns create mounts in nearly any
location. Writable holes are created automatically using overlayfs. They
are also undone when the corresponding entry goes away.

This will allow most of the new layout features to operate. It will also
allow to improve the content interface to implement the aggregation
features (through a .d-style directories).

There are lots of thins missing still:
 - needed changes logic doesn't understand synthesized mount entries,
   those need to be annotated properly and preserved using a pair of
   mount options: x-snapd-mount-id=unique, x-snapd-synthesized-for=unique
 - integration tests need to be extended to cover the new features
 - security review needs to identify things we should refuse to
   support (such as bi-directionally shared mount locations)
 - certain things in overlayfs are not allowed but also not checked for
   (see kernel docs for detials). Those need to be implemented as checks
   here.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

cmd/snap-update-ns/change.go

@@ -61,25 +61,37 @@ func (c Change) String() string {
 // Perform may synthesize *additional* changes that were necessary to perform
 // the this change (such as transparently mounted tmpfs and overlayfs.
 func (c *Change) Perform() ([]*Change, error) {
+	var synth []*Change
+
 	if c.Action == Mount {
 		// TODO: use the right mode and ownership.
 		mode := os.FileMode(0755)
 		uid := 0
 		gid := 0
 		// Create target mount directory if needed.
-		if err := ensureMountPoint(c.Entry.Dir, mode, uid, gid); err != nil {
-			return nil, err
+		extraChange, err := ensureMountPointMaybeUsingOverlay(c.Entry.Dir, mode, uid, gid)
+		if extraChange != nil {
+			synth = append(synth, extraChange)
+		}
+		if err != nil {
+			return synth, err
 		}
+
 		// If this is a bind mount then create the source directory as well.
 		// This allows snaps to share a subset of their data easily.
 		flags, _ := mount.OptsToCommonFlags(c.Entry.Options)
 		if flags&syscall.MS_BIND != 0 {
-			if err := ensureMountPoint(c.Entry.Name, mode, uid, gid); err != nil {
-				return nil, err
+			extraChange, err := ensureMountPointMaybeUsingOverlay(c.Entry.Name, mode, uid, gid)
+			if extraChange != nil {
+				synth = append(synth, extraChange)
+			}
+			if err != nil {
+				return synth, err
 			}
 		}
 	}
-	return nil, c.lowLevelPerform()
+
+	return synth, c.lowLevelPerform()
 }
 
 // lowLevelPerform is a version of Perform that doesn't contain any special behavior.

cmd/snap-update-ns/export_test.go

@@ -40,8 +40,10 @@ var (
 	FreezeSnapProcesses = freezeSnapProcesses
 	ThawSnapProcesses   = thawSnapProcesses
 	// utils
-	SecureMkdirAll   = secureMkdirAll
-	EnsureMountPoint = ensureMountPoint
+	SecureMkdirAll                    = secureMkdirAll
+	MountOverlayAt                    = mountOverlayAt
+	EnsureMountPoint                  = ensureMountPoint
+	EnsureMountPointMaybeUsingOverlay = ensureMountPointMaybeUsingOverlay
 )
 
 // fakeFileInfo implements os.FileInfo for one of the tests.

cmd/snap-update-ns/utils.go

@@ -25,6 +25,8 @@ import (
 	"path"
 	"strings"
 	"syscall"
+
+	"github.com/snapcore/snapd/interfaces/mount"
 )
 
 // not available through syscall
@@ -45,10 +47,21 @@ var (
 	sysUnmount = syscall.Unmount
 	sysFchown  = syscall.Fchown
 
-	secureMkdirAll   = SecureMkdirAllImpl
-	ensureMountPoint = EnsureMountPointImpl
+	ensureMountPoint                  = EnsureMountPointImpl
+	ensureMountPointMaybeUsingOverlay = EnsureMountPointMaybeUsingOverlayImpl
+	mountOverlayAt                    = MountOverlayAtImpl
+	secureMkdirAll                    = SecureMkdirAllImpl
 )
 
+// ReadOnlyFsError is an error encapsulating encountered EROFS.
+type ReadOnlyFsError struct {
+	Path string
+}
+
+func (e *ReadOnlyFsError) Error() string {
+	return fmt.Sprintf("cannot operate on read-only filesystem at %s", e.Path)
+}
+
 // SecureMkdirAll is the secure variant of os.MkdirAll.
 //
 // Unlike the regular version this implementation does not follow any symbolic
@@ -83,7 +96,7 @@ func SecureMkdirAllImpl(name string, perm os.FileMode, uid, gid int) error {
 	// segment using the O_NOFOLLOW and O_DIRECTORY flag so that symlink
 	// attacks are impossible to carry out.
 	segments := strings.Split(path.Clean(name), "/")
-	for _, segment := range segments {
+	for i, segment := range segments {
 		if segment == "" {
 			// Skip empty element corresponding to the leading slash.
 			continue
@@ -93,6 +106,11 @@ func SecureMkdirAllImpl(name string, perm os.FileMode, uid, gid int) error {
 			switch err {
 			case syscall.EEXIST:
 				made = false
+			case syscall.EROFS:
+				// Treat EROFS specially: this is a hint that we have to poke a
+				// hole using overlayfs. The path below is the location where
+				// we need to poke the hole.
+				return &ReadOnlyFsError{Path: strings.Join(segments[:i], "/")}
 			default:
 				return fmt.Errorf("cannot mkdir path segment %q, %v", segment, err)
 			}
@@ -124,6 +142,57 @@ func SecureMkdirAllImpl(name string, perm os.FileMode, uid, gid int) error {
 	return nil
 }
 
+func MountOverlayAtImpl(dir string) (*Change, error) {
+	// Overlay uses three directories: lower, upper and work.
+	// Lower is our read-only substrate Upper is an ephemeral
+	// sub-directory of /tmp (see below). Work is an empty
+	// sibling of upper, needed by overlayfs to function.
+	//
+	// The lower directory cannot be in /tmp as we use (private) /tmp for the
+	// overlay machinery itself.
+	//
+	// The upper directory is already on top of an existing host-based /tmp
+	// directory, thanks to how snap-confine is arranigng the per-snap, private
+	// /tmp directory.
+	lowerDir := dir
+	upperDir := path.Join("/tmp/.snap.overlays", dir)
+	workDir := path.Join("/tmp/.snap.workdirs", dir)
+
+	for _, blacklistDir := range []string{"/tmp"} {
+		if strings.HasPrefix(lowerDir, blacklistDir+"/") || lowerDir == blacklistDir {
+			return nil, fmt.Errorf("refusing to create overlay at %q", lowerDir)
+		}
+	}
+
+	// Create upper and work directories.
+	if err := secureMkdirAll(upperDir, 0755, 0, 0); err != nil {
+		return nil, err
+	}
+	if err := secureMkdirAll(workDir, 0755, 0, 0); err != nil {
+		return nil, err
+	}
+
+	// Create and perform and return a change describing the overlay mount.
+	change := &Change{
+		Action: Mount,
+		Entry: mount.Entry{
+			Name: "none",
+			Dir:  dir,
+			Type: "overlay",
+			Options: []string{
+				// Format the options, note the mount escape logic for paths.
+				fmt.Sprintf("lowerdir=%s", mount.Escape(lowerDir)),
+				fmt.Sprintf("upperdir=%s", mount.Escape(upperDir)),
+				fmt.Sprintf("workdir=%s", mount.Escape(workDir)),
+			},
+		},
+	}
+	if err := change.lowLevelPerform(); err != nil {
+		return nil, fmt.Errorf("cannot mount overlay at %q, %v", dir, err)
+	}
+	return change, nil
+}
+
 func EnsureMountPointImpl(path string, mode os.FileMode, uid int, gid int) error {
 	// If the mount point is not present then create a directory in its
 	// place.  This is very naive, doesn't handle read-only file systems
@@ -147,3 +216,17 @@ func EnsureMountPointImpl(path string, mode os.FileMode, uid int, gid int) error
 	}
 	return nil
 }
+
+func EnsureMountPointMaybeUsingOverlayImpl(dir string, mode os.FileMode, uid int, gid int) (*Change, error) {
+	var extraChange *Change
+	err := ensureMountPoint(dir, mode, uid, gid)
+	if err != nil {
+		if err2, ok := err.(*ReadOnlyFsError); ok {
+			extraChange, err = mountOverlayAt(err2.Path)
+		}
+		if err == nil {
+			err = ensureMountPoint(dir, mode, uid, gid)
+		}
+	}
+	return extraChange, err
+}

cmd/snap-update-ns/utils_test.go

@@ -20,11 +20,13 @@
 package main_test
 
 import (
+	"os"
 	"syscall"
 
 	. "gopkg.in/check.v1"
 
 	update "github.com/snapcore/snapd/cmd/snap-update-ns"
+	"github.com/snapcore/snapd/interfaces/mount"
 	"github.com/snapcore/snapd/testutil"
 )
 
@@ -73,6 +75,22 @@ func (s *utilsSuite) TestSecureMkdirAllAbsolute(c *C) {
 	})
 }
 
+// Ensure that we can detect read only filesystems.
+func (s *utilsSuite) TestSecureMkdirAllROFS(c *C) {
+	s.sys.InsertFault(`mkdirat 4 "path" 0755`, syscall.EROFS)
+	err := update.SecureMkdirAllImpl("/rofs/path", 0755, 123, 456)
+	c.Assert(err, ErrorMatches, `cannot operate on read-only filesystem at /rofs`)
+	c.Assert(err.(*update.ReadOnlyFsError).Path, Equals, "/rofs")
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		`open "/" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`mkdirat 3 "rofs" 0755`,
+		`openat 3 "rofs" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`close 3`,
+		`fchown 4 123 456`,
+		`mkdirat 4 "path" 0755`,
+	})
+}
+
 // Ensure that we don't chown existing directories.
 func (s *utilsSuite) TestSecureMkdirAllExistingDirsDontChown(c *C) {
 	s.sys.InsertFault(`mkdirat 3 "abs" 0755`, syscall.EEXIST)
@@ -90,3 +108,145 @@ func (s *utilsSuite) TestSecureMkdirAllExistingDirsDontChown(c *C) {
 		`close 3`,
 	})
 }
+
+// Explore how mounting overlay looks like technically
+func (s *utilsSuite) TestMountOverlayAt(c *C) {
+	change, err := update.MountOverlayAtImpl("/abs/path")
+	c.Assert(err, IsNil)
+	c.Assert(change, DeepEquals, &update.Change{
+		Action: update.Mount,
+		Entry: mount.Entry{
+			Name: "none",
+			Dir:  "/abs/path",
+			Type: "overlay",
+			Options: []string{
+				"lowerdir=/abs/path",
+				"upperdir=/tmp/.snap.overlays/abs/path",
+				"workdir=/tmp/.snap.workdirs/abs/path",
+			},
+		},
+	})
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		// Create "/tmp/.snap.overlays/abs/path".
+		`open "/" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`mkdirat 3 "tmp" 0755`,
+		`openat 3 "tmp" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`close 3`,
+		`fchown 4 0 0`,
+		`mkdirat 4 ".snap.overlays" 0755`,
+		`openat 4 ".snap.overlays" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`close 4`,
+		`fchown 3 0 0`,
+		`mkdirat 3 "abs" 0755`,
+		`openat 3 "abs" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`close 3`,
+		`fchown 4 0 0`,
+		`mkdirat 4 "path" 0755`,
+		`openat 4 "path" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`close 4`,
+		`fchown 3 0 0`, `close 3`,
+		// Create "/tmp/.snap.workdirs/abs/path".
+		`open "/" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`mkdirat 3 "tmp" 0755`,
+		`openat 3 "tmp" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`close 3`, `fchown 4 0 0`,
+		`mkdirat 4 ".snap.workdirs" 0755`,
+		`openat 4 ".snap.workdirs" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`close 4`,
+		`fchown 3 0 0`,
+		`mkdirat 3 "abs" 0755`,
+		`openat 3 "abs" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`close 3`,
+		`fchown 4 0 0`,
+		`mkdirat 4 "path" 0755`,
+		`openat 4 "path" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`close 4`,
+		`fchown 3 0 0`,
+		`close 3`,
+		// Mount overlay at /abs/path with appropriate {lower,upper,work}dirs.
+		`mount "none" "/abs/path" "overlay" 0 "lowerdir=/abs/path,upperdir=/tmp/.snap.overlays/abs/path,workdir=/tmp/.snap.workdirs/abs/path"`,
+	})
+}
+
+// Ensure that we cannot put overlays over any of the restricted locations.
+func (s *utilsSuite) TestMountOverlayAtRestrictedLocation(c *C) {
+	for _, dir := range []string{"/tmp", "/tmp/", "/tmp/stuff"} {
+		change, err := update.MountOverlayAtImpl(dir)
+		c.Assert(err, ErrorMatches, `refusing to create overlay at ".*"`, Commentf("dir: %q", dir))
+		c.Assert(change, IsNil)
+	}
+}
+
+// Ensure that errors are handled.
+
+func (s *utilsSuite) TestMountOverlayAtErrors1(c *C) {
+	s.sys.InsertFault(`mkdirat 4 ".snap.overlays" 0755`, syscall.EPERM)
+	_, err := update.MountOverlayAtImpl("/abs/path")
+	c.Assert(err, ErrorMatches, `cannot mkdir path segment ".snap.overlays", operation not permitted`)
+	c.Assert(s.sys.Calls(), Not(testutil.Contains),
+		`mount "none" "/abs/path" "overlay" 0 "lowerdir=/abs/path,upperdir=/tmp/.snap.overlays/abs/path,workdir=/tmp/.snap.workdirs/abs/path"`)
+}
+
+func (s *utilsSuite) TestMountOverlayAtErrors2(c *C) {
+	s.sys.InsertFault(`mkdirat 4 ".snap.workdirs" 0755`, syscall.EPERM)
+	_, err := update.MountOverlayAtImpl("/abs/path")
+	c.Assert(err, ErrorMatches, `cannot mkdir path segment ".snap.workdirs", operation not permitted`)
+	c.Assert(s.sys.Calls(), Not(testutil.Contains),
+		`mount "none" "/abs/path" "overlay" 0 "lowerdir=/abs/path,upperdir=/tmp/.snap.overlays/abs/path,workdir=/tmp/.snap.workdirs/abs/path"`)
+}
+
+func (s *utilsSuite) TestMountOverlayAtErrors3(c *C) {
+	s.sys.InsertFault(`mount "none" "/abs/path" "overlay" 0 "lowerdir=/abs/path,upperdir=/tmp/.snap.overlays/abs/path,workdir=/tmp/.snap.workdirs/abs/path"`, syscall.EPERM)
+	_, err := update.MountOverlayAtImpl("/abs/path")
+	c.Assert(err, ErrorMatches, `cannot mount overlay at "/abs/path", operation not permitted`)
+}
+
+func (s *utilsSuite) TestEnsureMountPointSuperExistingDir(c *C) {
+	s.sys.InsertLstatResult(`lstat "/abs/path"`, update.FileInfoDir)
+	change, err := update.EnsureMountPointMaybeUsingOverlayImpl("/abs/path", 0755, 0, 0)
+	c.Assert(err, IsNil)
+	c.Assert(change, IsNil)
+}
+
+func (s *utilsSuite) TestEnsureMountPointSuperExistingFile(c *C) {
+	s.sys.InsertLstatResult(`lstat "/abs/path"`, update.FileInfoFile)
+	change, err := update.EnsureMountPointMaybeUsingOverlayImpl("/abs/path", 0755, 0, 0)
+	c.Assert(err, ErrorMatches, `cannot use "/abs/path" for mounting, not a directory`)
+	c.Assert(change, IsNil)
+}
+
+func (s *utilsSuite) TestEnsureMountPointSuperExistingSymlink(c *C) {
+	s.sys.InsertLstatResult(`lstat "/abs/path"`, update.FileInfoSymlink)
+	change, err := update.EnsureMountPointMaybeUsingOverlayImpl("/abs/path", 0755, 0, 0)
+	c.Assert(err, ErrorMatches, `cannot use "/abs/path" for mounting, not a directory`)
+	c.Assert(change, IsNil)
+}
+
+func (s *utilsSuite) TestEnsureMountPointSuperPermissionDenied(c *C) {
+	s.sys.InsertFault(`lstat "/abs/path"`, syscall.EPERM)
+	change, err := update.EnsureMountPointMaybeUsingOverlayImpl("/abs/path", 0755, 0, 0)
+	c.Assert(err, ErrorMatches, `cannot inspect "/abs/path", operation not permitted`)
+	c.Assert(change, IsNil)
+}
+
+func (s *utilsSuite) TestEnsureMountPointSuperROFS(c *C) {
+	s.sys.InsertFault(`lstat "/abs/path"`, os.ErrNotExist)
+	s.sys.InsertFault(`mkdirat 4 "path" 0755`, syscall.EROFS)
+	change, err := update.EnsureMountPointMaybeUsingOverlayImpl("/abs/path", 0755, 0, 0)
+	c.Assert(err, IsNil)
+	c.Assert(change, DeepEquals, &update.Change{
+		Action: update.Mount,
+		Entry: mount.Entry{
+			Name: "none",
+			Dir:  "/abs",
+			Type: "overlay",
+			Options: []string{
+				"lowerdir=/abs",
+				"upperdir=/tmp/.snap.overlays/abs",
+				"workdir=/tmp/.snap.workdirs/abs",
+			},
+		},
+	})
+	c.Assert(s.sys.Calls(), testutil.Contains,
+		`mount "none" "/abs" "overlay" 0 "lowerdir=/abs,upperdir=/tmp/.snap.overlays/abs,workdir=/tmp/.snap.workdirs/abs"`)
+}