cmd/snap-update-ns: fix mount rules for font sharing

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

@@ -517,9 +517,9 @@
         # 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
-        mount options=(ro bind) /var/lib/snapd/hostfs/usr/share/fonts -> /usr/share/fonts,
-        mount options=(ro bind) /var/lib/snapd/hostfs/usr/local/share/fonts -> /usr/local/share/fonts,
-        mount options=(ro bind) /var/lib/snapd/hostfs/var/cache/fontconfig -> /var/cache/fontconfig,
+        mount options=(ro bind) /var/lib/snapd/hostfs/usr/share/fonts/ -> /usr/share/fonts/,
+        mount options=(ro bind) /var/lib/snapd/hostfs/usr/local/share/fonts/ -> /usr/local/share/fonts/,
+        mount options=(ro bind) /var/lib/snapd/hostfs/var/cache/fontconfig/ -> /var/cache/fontconfig/,
 
         # Allow unmounts matching possible mounts listed above.
         umount /snap/*/*/**,

cmd/snap-update-ns/change.go

@@ -101,7 +101,7 @@ func (c *Change) lowLevelPerform() error {
 // lists are processed and a "diff" of mount changes is produced. The mount
 // changes, when applied in order, transform the current profile into the
 // desired profile.
-func NeededChanges(currentProfile, desiredProfile *mount.Profile) []Change {
+func NeededChanges(currentProfile, desiredProfile *mount.Profile) []*Change {
 	// Copy both profiles as we will want to mutate them.
 	current := make([]mount.Entry, len(currentProfile.Entries))
 	copy(current, currentProfile.Entries)
@@ -149,21 +149,21 @@ func NeededChanges(currentProfile, desiredProfile *mount.Profile) []Change {
 	}
 
 	// We are now ready to compute the necessary mount changes.
-	var changes []Change
+	var changes []*Change
 
 	// Unmount entries not reused in reverse to handle children before their parent.
 	for i := len(current) - 1; i >= 0; i-- {
 		if reuse[current[i].Dir] {
-			changes = append(changes, Change{Action: Keep, Entry: current[i]})
+			changes = append(changes, &Change{Action: Keep, Entry: current[i]})
 		} else {
-			changes = append(changes, Change{Action: Unmount, Entry: current[i]})
+			changes = append(changes, &Change{Action: Unmount, Entry: current[i]})
 		}
 	}
 
 	// Mount desired entries not reused.
 	for i := range desired {
 		if !reuse[desired[i].Dir] {
-			changes = append(changes, Change{Action: Mount, Entry: desired[i]})
+			changes = append(changes, &Change{Action: Mount, Entry: desired[i]})
 		}
 	}
 

cmd/snap-update-ns/change_test.go

@@ -75,7 +75,7 @@ func (s *changeSuite) TestNeededChangesNoChange(c *C) {
 	current := &mount.Profile{Entries: []mount.Entry{{Dir: "/common/stuf"}}}
 	desired := &mount.Profile{Entries: []mount.Entry{{Dir: "/common/stuf"}}}
 	changes := update.NeededChanges(current, desired)
-	c.Assert(changes, DeepEquals, []update.Change{
+	c.Assert(changes, DeepEquals, []*update.Change{
 		{Entry: mount.Entry{Dir: "/common/stuf"}, Action: update.Keep},
 	})
 }
@@ -85,7 +85,7 @@ func (s *changeSuite) TestNeededChangesTrivialMount(c *C) {
 	current := &mount.Profile{}
 	desired := &mount.Profile{Entries: []mount.Entry{{Dir: "/common/stuf"}}}
 	changes := update.NeededChanges(current, desired)
-	c.Assert(changes, DeepEquals, []update.Change{
+	c.Assert(changes, DeepEquals, []*update.Change{
 		{Entry: desired.Entries[0], Action: update.Mount},
 	})
 }
@@ -95,7 +95,7 @@ func (s *changeSuite) TestNeededChangesTrivialUnmount(c *C) {
 	current := &mount.Profile{Entries: []mount.Entry{{Dir: "/common/stuf"}}}
 	desired := &mount.Profile{}
 	changes := update.NeededChanges(current, desired)
-	c.Assert(changes, DeepEquals, []update.Change{
+	c.Assert(changes, DeepEquals, []*update.Change{
 		{Entry: current.Entries[0], Action: update.Unmount},
 	})
 }
@@ -108,7 +108,7 @@ func (s *changeSuite) TestNeededChangesUnmountOrder(c *C) {
 	}}
 	desired := &mount.Profile{}
 	changes := update.NeededChanges(current, desired)
-	c.Assert(changes, DeepEquals, []update.Change{
+	c.Assert(changes, DeepEquals, []*update.Change{
 		{Entry: mount.Entry{Dir: "/common/stuf/extra"}, Action: update.Unmount},
 		{Entry: mount.Entry{Dir: "/common/stuf"}, Action: update.Unmount},
 	})
@@ -122,7 +122,7 @@ func (s *changeSuite) TestNeededChangesMountOrder(c *C) {
 		{Dir: "/common/stuf"},
 	}}
 	changes := update.NeededChanges(current, desired)
-	c.Assert(changes, DeepEquals, []update.Change{
+	c.Assert(changes, DeepEquals, []*update.Change{
 		{Entry: mount.Entry{Dir: "/common/stuf"}, Action: update.Mount},
 		{Entry: mount.Entry{Dir: "/common/stuf/extra"}, Action: update.Mount},
 	})
@@ -141,7 +141,7 @@ func (s *changeSuite) TestNeededChangesChangedParentSameChild(c *C) {
 		{Dir: "/common/unrelated"},
 	}}
 	changes := update.NeededChanges(current, desired)
-	c.Assert(changes, DeepEquals, []update.Change{
+	c.Assert(changes, DeepEquals, []*update.Change{
 		{Entry: mount.Entry{Dir: "/common/unrelated"}, Action: update.Keep},
 		{Entry: mount.Entry{Dir: "/common/stuf/extra"}, Action: update.Unmount},
 		{Entry: mount.Entry{Dir: "/common/stuf", Name: "/dev/sda1"}, Action: update.Unmount},
@@ -163,7 +163,7 @@ func (s *changeSuite) TestNeededChangesSameParentChangedChild(c *C) {
 		{Dir: "/common/unrelated"},
 	}}
 	changes := update.NeededChanges(current, desired)
-	c.Assert(changes, DeepEquals, []update.Change{
+	c.Assert(changes, DeepEquals, []*update.Change{
 		{Entry: mount.Entry{Dir: "/common/unrelated"}, Action: update.Keep},
 		{Entry: mount.Entry{Dir: "/common/stuf/extra", Name: "/dev/sda1"}, Action: update.Unmount},
 		{Entry: mount.Entry{Dir: "/common/stuf"}, Action: update.Keep},
@@ -189,7 +189,7 @@ func (s *changeSuite) TestNeededChangesSmartEntryComparison(c *C) {
 		{Dir: "/a/b/c"},
 	}}
 	changes := update.NeededChanges(current, desired)
-	c.Assert(changes, DeepEquals, []update.Change{
+	c.Assert(changes, DeepEquals, []*update.Change{
 		{Entry: mount.Entry{Dir: "/a/b/c"}, Action: update.Unmount},
 		{Entry: mount.Entry{Dir: "/a/b", Name: "/dev/sda1"}, Action: update.Unmount},
 		{Entry: mount.Entry{Dir: "/a/b-1/3"}, Action: update.Unmount},

cmd/snap-update-ns/main.go

@@ -140,7 +140,7 @@ func run() error {
 			logger.Noticef("cannot change mount namespace of snap %q according to change %s: %s", snapName, change, err)
 			continue
 		}
-		changesMade = append(changesMade, &change)
+		changesMade = append(changesMade, change)
 	}
 
 	// Compute the new current profile so that it contains only changes that were made

tests/lib/pkgdb.sh

@@ -370,6 +370,7 @@ pkg_dependencies_ubuntu_classic(){
         python3-yaml
         upower
         weston
+        xdg-user-dirs
         xdg-utils
         "
 
@@ -427,6 +428,7 @@ pkg_dependencies_fedora(){
         mock
         redhat-lsb-core
         rpm-build
+        xdg-user-dirs
         "
 }
 
@@ -442,6 +444,7 @@ pkg_dependencies_opensuse(){
         osc
         rng-tools
         xdg-utils
+        xdg-user-dirs
         "
 }
 

tests/lib/snaps/test-snapd-desktop/bin/check-dirs

@@ -0,0 +1,5 @@
+#!/bin/sh
+
+for dir in "$@"; do
+    ls $dir
+done

tests/lib/snaps/test-snapd-desktop/bin/check-files

@@ -0,0 +1,5 @@
+#!/bin/sh
+
+for file in "$@"; do
+    cat $file
+done

tests/lib/snaps/test-snapd-desktop/meta/snap.yaml

@@ -0,0 +1,12 @@
+name: test-snapd-desktop
+version: 1.0
+summary: Basic desktop interface test snap
+description: A basic snap to access dektop interface
+
+apps:
+    check-files:
+        command: bin/check-files
+        plugs: [desktop]
+    check-dirs:
+        command: bin/check-dirs
+        plugs: [desktop]

tests/main/interfaces-desktop/task.yaml

@@ -0,0 +1,61 @@
+summary: Ensure that the desktop interface works.
+
+details: |
+    The desktop interface allows to access the different resources.
+
+    The test-snapd-desktop snap checks files and dirs are accessible through the
+    desktop interface.
+
+systems: [-ubuntu-core-*]
+
+prepare: |
+    echo "Given the desktop snap is installed"
+    snap try $TESTSLIB/snaps/test-snapd-desktop
+
+execute: |
+    CONNECTED_PATTERN=":desktop +test-snapd-desktop"
+    DISCONNECTED_PATTERN="\- +test-snapd-desktop:desktop"
+
+    dirs="/var/cache/fontconfig /usr/share/icons /usr/share/pixmaps"
+    files="/etc/xdg/user-dirs.conf /etc/xdg/user-dirs.defaults"
+
+    echo "The plug is connected by default"
+    snap interfaces | MATCH "$CONNECTED_PATTERN"
+
+    echo "Then the snap is able to desktop files and directories"
+    test-snapd-desktop.check-files $files
+    test-snapd-desktop.check-dirs $dirs
+
+    echo "The mount namespace contains shared font directories"
+    for d in /usr/share/fonts /usr/local/share/fonts /var/cache/fontconfig; do
+        if [ -d "$d" ]; then
+                cat /run/snapd/ns/snap.test-snapd-desktop.fstab | MATCH "$d"
+                test-snapd-desktop.check-dirs "$d"
+        fi
+    done
+
+    if [ "$(snap debug confinement)" = partial ] ; then
+        exit 0
+    fi
+
+    echo "When the plug is disconnected"
+    snap disconnect test-snapd-desktop:desktop
+    snap interfaces | MATCH "$DISCONNECTED_PATTERN"
+
+    echo "Then the snap is not able to access the desktop files"
+    for file in $files; do
+        if test-snapd-desktop.check-files $file 2>${PWD}/call.error; then
+            echo "Expected permission error calling desktop with disconnected plug"
+            exit 1
+        fi
+        MATCH "Permission denied" < call.error
+    done
+
+    echo "Then the snap is not able to access the desktop dirs"
+    for dir in $dirs; do
+        if test-snapd-desktop.check-dirs $dir 2>${PWD}/call.error; then
+            echo "Expected permission error calling desktop with disconnected plug"
+            exit 1
+        fi
+        MATCH "Permission denied" < call.error
+    done