cmd/snap-update-ns: create missing mount points automatically.

cmd/libsnap-confine-private/cgroup-freezer-support.c

@@ -44,7 +44,7 @@ void sc_cgroup_freezer_join(const char *snap_name, pid_t pid)
 		    snap_name);
 	}
 	// Since we may be running from a setuid but not setgid executable, ensure
-	// that the group and owner of the hierarchy directory is root.root. 
+	// that the group and owner of the hierarchy directory is root.root.
 	if (fchownat(hierarchy_fd, "", 0, 0, AT_EMPTY_PATH) < 0) {
 		die("cannot change owner of freezer cgroup hierarchy for snap %s to root.root", snap_name);
 	}

cmd/snap-confine/snap-confine.apparmor.in

@@ -481,6 +481,9 @@
         # Needed to perform mount/unmounts.
         capability sys_admin,
 
+        # Allow freezing and thawing the per-snap cgroup freezers
+        /sys/fs/cgroup/freezer/snap.*/freezer.state rw,
+
         # Support mount profiles via the content interface. This should correspond
         # to permutations of $SNAP -> $SNAP for reading and $SNAP_{DATA,COMMON} ->
         # $SNAP_{DATA,COMMON} for both reading and writing.
@@ -498,6 +501,16 @@
         mount options=(rw bind) /var/snap/*/** -> /var/snap/*/**,
         mount options=(ro bind) /var/snap/*/** -> /var/snap/*/**,
 
+        # Allow creating missing mount directories under $SNAP_DATA.
+        #
+        # The "tree" of permissions is needed for SecureMkdirAll that uses
+        # open(..., O_NOFOLLOW) and mkdirat() using the resulting file
+        # descriptor.
+        / r,
+        /var/ r,
+        /var/snap/{,*/} r,
+        /var/snap/*/**/ rw,
+
         # Allow the content interface to bind fonts from the host filesystem
         mount options=(ro bind) /var/lib/snapd/hostfs/usr/share/fonts/ -> /snap/*/*/**,
         # Allow the desktop interface to bind fonts from the host filesystem

cmd/snap-update-ns/change.go

@@ -21,7 +21,8 @@ package main
 
 import (
 	"fmt"
-	"path"
+	"os"
+	"path/filepath"
 	"sort"
 	"strings"
 	"syscall"
@@ -53,26 +54,40 @@ func (c Change) String() string {
 	return fmt.Sprintf("%s (%s)", c.Action, c.Entry)
 }
 
-var (
-	sysMount   = syscall.Mount
-	sysUnmount = syscall.Unmount
-)
-
-const unmountNoFollow = 8
-
 // Perform executes the desired mount or unmount change using system calls.
 // Filesystems that depend on helper programs or multiple independent calls to
 // the kernel (--make-shared, for example) are unsupported.
 func (c *Change) Perform() error {
-	switch c.Action {
-	case Mount:
-		flags, err := mount.OptsToFlags(c.Entry.Options)
-		if err != nil {
+	if c.Action == Mount {
+		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 err
 		}
-		return sysMount(c.Entry.Name, c.Entry.Dir, c.Entry.Type, uintptr(flags), "")
+		// 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 err
+			}
+		}
+	}
+	return c.lowLevelPerform()
+}
+
+// lowLevelPerform is simple bridge from Change to mount / unmount syscall.
+func (c *Change) lowLevelPerform() error {
+	switch c.Action {
+	case Mount:
+		flags, unparsed := mount.OptsToCommonFlags(c.Entry.Options)
+		return sysMount(c.Entry.Name, c.Entry.Dir, c.Entry.Type, uintptr(flags), strings.Join(unparsed, ","))
 	case Unmount:
-		return sysUnmount(c.Entry.Dir, unmountNoFollow)
+		return sysUnmount(c.Entry.Dir, UMOUNT_NOFOLLOW)
+	case Keep:
+		return nil
 	}
 	return fmt.Errorf("cannot process mount change, unknown action: %q", c.Action)
 }
@@ -94,10 +109,10 @@ func NeededChanges(currentProfile, desiredProfile *mount.Profile) []Change {
 	// easily test if a given directory is a subdirectory with
 	// strings.HasPrefix coupled with an extra slash character.
 	for i := range current {
-		current[i].Dir = path.Clean(current[i].Dir)
+		current[i].Dir = filepath.Clean(current[i].Dir)
 	}
 	for i := range desired {
-		desired[i].Dir = path.Clean(desired[i].Dir)
+		desired[i].Dir = filepath.Clean(desired[i].Dir)
 	}
 
 	// Sort both lists by directory name with implicit trailing slash.

cmd/snap-update-ns/change_test.go

@@ -21,6 +21,7 @@ package main_test
 
 import (
 	"errors"
+	"os"
 
 	. "gopkg.in/check.v1"
 
@@ -47,6 +48,12 @@ func (s *changeSuite) SetUpTest(c *C) {
 	s.BaseTest.AddCleanup(update.MockSystemCalls(s.sys))
 }
 
+func (s *changeSuite) TestFakeFileInfo(c *C) {
+	c.Assert(update.FileInfoDir.IsDir(), Equals, true)
+	c.Assert(update.FileInfoFile.IsDir(), Equals, false)
+	c.Assert(update.FileInfoSymlink.IsDir(), Equals, false)
+}
+
 func (s *changeSuite) TestString(c *C) {
 	change := update.Change{
 		Entry:  mount.Entry{Dir: "/a/b", Name: "/dev/sda1"},
@@ -195,30 +202,157 @@ func (s *changeSuite) TestNeededChangesSmartEntryComparison(c *C) {
 
 // Change.Perform calls the mount system call.
 func (s *changeSuite) TestPerformMount(c *C) {
-	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "source", Dir: "target", Type: "type"}}
-	c.Assert(chg.Perform(), IsNil)
-	c.Assert(s.sys.Calls(), DeepEquals, []string{`mount "source" "target" "type" 0 ""`})
+	s.sys.InsertLstatResult(`lstat "/target"`, update.FileInfoDir)
+	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "/source", Dir: "/target", Type: "type"}}
+	err := chg.Perform()
+	c.Assert(err, IsNil)
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		`lstat "/target"`,
+		`mount "/source" "/target" "type" 0 ""`,
+	})
+}
+
+// Change.Perform calls the mount system call (for bind mounts).
+func (s *changeSuite) TestPerformBindMount(c *C) {
+	s.sys.InsertLstatResult(`lstat "/source"`, update.FileInfoDir)
+	s.sys.InsertLstatResult(`lstat "/target"`, update.FileInfoDir)
+	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "/source", Dir: "/target", Type: "type", Options: []string{"bind"}}}
+	err := chg.Perform()
+	c.Check(err, IsNil)
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		`lstat "/target"`,
+		`lstat "/source"`,
+		`mount "/source" "/target" "type" MS_BIND ""`,
+	})
+}
+
+// Change.Perform creates the missing mount target.
+func (s *changeSuite) TestPerformMountAutomaticMkdirTarget(c *C) {
+	s.sys.InsertFault(`lstat "/target"`, os.ErrNotExist)
+	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "/source", Dir: "/target", Type: "type"}}
+	err := chg.Perform()
+	c.Assert(err, IsNil)
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		`lstat "/target"`,
+		`open "/" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`mkdirat 3 "target" 0755`,
+		`openat 3 "target" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`fchown 4 0 0`,
+		`close 4`,
+		`close 3`,
+		`mount "/source" "/target" "type" 0 ""`,
+	})
+}
+
+// Change.Perform creates the missing bind-mount source.
+func (s *changeSuite) TestPerformMountAutomaticMkdirSource(c *C) {
+	s.sys.InsertLstatResult(`lstat "/target"`, update.FileInfoDir)
+	s.sys.InsertFault(`lstat "/source"`, os.ErrNotExist)
+	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "/source", Dir: "/target", Type: "type", Options: []string{"bind"}}}
+	err := chg.Perform()
+	c.Assert(err, IsNil)
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		`lstat "/target"`,
+		`lstat "/source"`,
+		`open "/" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`mkdirat 3 "source" 0755`,
+		`openat 3 "source" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`fchown 4 0 0`,
+		`close 4`,
+		`close 3`,
+		`mount "/source" "/target" "type" MS_BIND ""`,
+	})
+}
+
+// Change.Perform rejects mount target if it is a symlink.
+func (s *changeSuite) TestPerformMountRejectsTargetSymlink(c *C) {
+	s.sys.InsertLstatResult(`lstat "/target"`, update.FileInfoSymlink)
+	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "/source", Dir: "/target", Type: "type"}}
+	err := chg.Perform()
+	c.Assert(err, ErrorMatches, `cannot use "/target" for mounting, not a directory`)
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		`lstat "/target"`,
+	})
+}
+
+// Change.Perform rejects bind-mount target if it is a symlink.
+func (s *changeSuite) TestPerformBindMountRejectsTargetSymlink(c *C) {
+	s.sys.InsertLstatResult(`lstat "/target"`, update.FileInfoSymlink)
+	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "/source", Dir: "/target", Type: "type", Options: []string{"bind"}}}
+	err := chg.Perform()
+	c.Assert(err, ErrorMatches, `cannot use "/target" for mounting, not a directory`)
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		`lstat "/target"`,
+	})
+}
+
+// Change.Perform rejects bind-mount source if it is a symlink.
+func (s *changeSuite) TestPerformBindMountRejectsSourceSymlink(c *C) {
+	s.sys.InsertLstatResult(`lstat "/target"`, update.FileInfoDir)
+	s.sys.InsertLstatResult(`lstat "/source"`, update.FileInfoSymlink)
+	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "/source", Dir: "/target", Type: "type", Options: []string{"bind"}}}
+	err := chg.Perform()
+	c.Assert(err, ErrorMatches, `cannot use "/source" for mounting, not a directory`)
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		`lstat "/target"`,
+		`lstat "/source"`,
+	})
+}
+
+// Change.Perform returns errors from os.Lstat (apart from ErrNotExist)
+func (s *changeSuite) TestPerformMountLstatError(c *C) {
+	s.sys.InsertFault(`lstat "/target"`, errTesting)
+	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "/source", Dir: "/target", Type: "type"}}
+	err := chg.Perform()
+	c.Assert(err, ErrorMatches, `cannot inspect "/target": testing`)
+	c.Assert(s.sys.Calls(), DeepEquals, []string{`lstat "/target"`})
+}
+
+// Change.Perform returns errors from os.MkdirAll
+func (s *changeSuite) TestPerformMountMkdirAllError(c *C) {
+	s.sys.InsertFault(`lstat "/target"`, os.ErrNotExist)
+	s.sys.InsertFault(`mkdirat 3 "target" 0755`, errTesting)
+	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "/source", Dir: "/target", Type: "type"}}
+	err := chg.Perform()
+	c.Assert(err, ErrorMatches, `cannot mkdir path segment "target": testing`)
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		`lstat "/target"`,
+		`open "/" O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY 0`,
+		`mkdirat 3 "target" 0755`,
+		`close 3`,
+	})
 }
 
 // Change.Perform returns errors from mount system call
 func (s *changeSuite) TestPerformMountError(c *C) {
-	s.sys.InsertFault(`mount "source" "target" "type" 0 ""`, errTesting)
-	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "source", Dir: "target", Type: "type"}}
-	c.Assert(chg.Perform(), Equals, errTesting)
-	c.Assert(s.sys.Calls(), DeepEquals, []string{`mount "source" "target" "type" 0 ""`})
+	s.sys.InsertLstatResult(`lstat "/target"`, update.FileInfoDir)
+	s.sys.InsertFault(`mount "/source" "/target" "type" 0 ""`, errTesting)
+	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "/source", Dir: "/target", Type: "type"}}
+	err := chg.Perform()
+	c.Assert(err, Equals, errTesting)
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		`lstat "/target"`,
+		`mount "/source" "/target" "type" 0 ""`,
+	})
 }
 
-// Change.Perform returns errors from bad flags
-func (s *changeSuite) TestPerformMountOptionError(c *C) {
-	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "source", Dir: "target", Type: "type", Options: []string{"bogus"}}}
-	c.Assert(chg.Perform(), ErrorMatches, `unsupported mount option: "bogus"`)
-	c.Assert(s.sys.Calls(), HasLen, 0)
+// Change.Perform passes unrecognized options to mount.
+func (s *changeSuite) TestPerformMountOptions(c *C) {
+	s.sys.InsertLstatResult(`lstat "target"`, update.FileInfoDir)
+	chg := &update.Change{Action: update.Mount, Entry: mount.Entry{Name: "source", Dir: "target", Type: "type", Options: []string{"funky"}}}
+	err := chg.Perform()
+	c.Assert(err, IsNil)
+	c.Assert(s.sys.Calls(), DeepEquals, []string{
+		`lstat "target"`,
+		`mount "source" "target" "type" 0 "funky"`,
+	})
 }
 
 // Change.Perform calls the unmount system call.
 func (s *changeSuite) TestPerformUnmount(c *C) {
 	chg := &update.Change{Action: update.Unmount, Entry: mount.Entry{Name: "source", Dir: "target", Type: "type"}}
-	c.Assert(chg.Perform(), IsNil)
+	err := chg.Perform()
+	c.Assert(err, IsNil)
 	// The flag 8 is UMOUNT_NOFOLLOW
 	c.Assert(s.sys.Calls(), DeepEquals, []string{`unmount "target" UMOUNT_NOFOLLOW`})
 }
@@ -227,13 +361,15 @@ func (s *changeSuite) TestPerformUnmount(c *C) {
 func (s *changeSuite) TestPerformUnountError(c *C) {
 	s.sys.InsertFault(`unmount "target" UMOUNT_NOFOLLOW`, errTesting)
 	chg := &update.Change{Action: update.Unmount, Entry: mount.Entry{Name: "source", Dir: "target", Type: "type"}}
-	c.Assert(chg.Perform(), Equals, errTesting)
+	err := chg.Perform()
+	c.Assert(err, Equals, errTesting)
 	c.Assert(s.sys.Calls(), DeepEquals, []string{`unmount "target" UMOUNT_NOFOLLOW`})
 }
 
 // Change.Perform handles unknown actions.
 func (s *changeSuite) TestPerformUnknownAction(c *C) {
 	chg := &update.Change{Action: update.Action(42)}
-	c.Assert(chg.Perform(), ErrorMatches, `cannot process mount change, unknown action: .*`)
+	err := chg.Perform()
+	c.Assert(err, ErrorMatches, `cannot process mount change, unknown action: .*`)
 	c.Assert(s.sys.Calls(), HasLen, 0)
 }