interfaces/cups: add cups-socket-directory attr, use to specify mount…

… rules in backend (#10427)

interfaces/cups: add cups-socket-directory attr, use to specify mount rules in backend

Make the cups interface slot have an optional attribute, cups-socket-directory, which
is used to specify the directory where the cups socket that the slotting snap
is going to share with client snaps.

This directory then is mounted into client snap's mount namespaces via the
mount backend at /var/cups/, such that client applications wishing to
print need to adjust the location of where they expect the cups socket to
be at all, but then will always end up sending print requests to the cups snap,
which will mediate requests to print based on whether or not the client snap
has a connected cups plug or not.

The primary advantage of this is that it means we don't need to make the cups
interface implictly slotted by the system snap, and it can always be provided
by the cups snap, and we can then make any snap with a cups interface plug
auto-connect to the slot from the cups snap unambiguously, providing all snap
clients the ability to print without any extra connections necessary,
regardless of their distro or what the client snap is.

cmd/snap-exec/main.go

@@ -194,6 +194,18 @@ func execApp(snapApp, revision, command string, args []string) error {
 		env.ExtendWithExpanded(eenv)
 	}
 
+	// this is a workaround for the lack of an environment backend in interfaces
+	// where we want certain interfaces when connected to add environment
+	// variables to plugging snap apps, but this is a lot simpler as a
+	// work-around
+	// we currently only handle the CUPS_SERVER environment variable, setting it
+	// to /var/cups/ if that directory exists - it should not exist anywhere
+	// except in a strictly confined snap where we setup the bind mount from the
+	// source cups slot snap to the plugging snap
+	if exists, _, _ := osutil.DirExists(dirs.GlobalRootDir + "/var/cups"); exists {
+		env["CUPS_SERVER"] = "/var/cups/cups.sock"
+	}
+
 	// strings.Split() is ok here because we validate all app fields and the
 	// whitelist is pretty strict (see snap/validate.go:appContentWhitelist)
 	// (see also overlord/snapstate/check_snap.go's normPath)

cmd/snap-exec/main_test.go

@@ -181,6 +181,41 @@ func (s *snapExecSuite) TestSnapExecAppIntegration(c *C) {
 	// See also https://bugs.launchpad.net/snapd/+bug/1860369
 	c.Check(execEnv, Not(testutil.Contains), "TEST_PATH=/vanilla")
 	c.Check(execEnv, testutil.Contains, "TEST_PATH=/custom")
+
+	// ensure that CUPS_SERVER is absent since we didn't mock the /var/cups dir
+	c.Check(execEnv, Not(testutil.Contains), "CUPS_SERVER=/var/cups")
+}
+
+func (s *snapExecSuite) TestSnapExecAppIntegrationCupsServerWorkaround(c *C) {
+	dir := c.MkDir()
+	dirs.SetRootDir(dir)
+	snaptest.MockSnap(c, string(mockYaml), &snap.SideInfo{
+		Revision: snap.R("42"),
+	})
+
+	// mock the /var/cups dir so that we observe CUPS_SERVER set
+	err := os.MkdirAll(filepath.Join(dir, "/var/cups"), 0755)
+	c.Assert(err, IsNil)
+
+	execArgv0 := ""
+	execArgs := []string{}
+	execEnv := []string{}
+	restore := snapExec.MockSyscallExec(func(argv0 string, argv []string, env []string) error {
+		execArgv0 = argv0
+		execArgs = argv
+		execEnv = env
+		return nil
+	})
+	defer restore()
+
+	// launch and verify its run the right way
+	err = snapExec.ExecApp("snapname.app", "42", "stop", []string{"arg1", "arg2"})
+	c.Assert(err, IsNil)
+	c.Check(execArgv0, Equals, fmt.Sprintf("%s/snapname/42/stop-app", dirs.SnapMountDir))
+	c.Check(execArgs, DeepEquals, []string{execArgv0, "arg1", "arg2"})
+
+	// ensure that CUPS_SERVER is now set since we did mock the /var/cups dir
+	c.Check(execEnv, testutil.Contains, "CUPS_SERVER=/var/cups/cups.sock")
 }
 
 func (s *snapExecSuite) TestSnapExecAppCommandChainIntegration(c *C) {

interfaces/builtin/cups.go

@@ -19,6 +19,17 @@
 
 package builtin
 
+import (
+	"fmt"
+	"strings"
+
+	"github.com/snapcore/snapd/interfaces"
+	"github.com/snapcore/snapd/interfaces/apparmor"
+	"github.com/snapcore/snapd/interfaces/mount"
+	"github.com/snapcore/snapd/osutil"
+	"github.com/snapcore/snapd/snap"
+)
+
 // On systems where the slot is provided by an app snap, the cups interface is
 // the companion interface to the cups-control interface. The design of these
 // interfaces is based on the idea that the slot implementation (eg cupsd) is
@@ -46,17 +57,162 @@ const cupsBaseDeclarationSlots = `
 const cupsConnectedPlugAppArmor = `
 # Allow communicating with the cups server
 
-#include <abstractions/cups-client>
+# Do not allow reading the user or global client.conf for this snap, as this may
+# allow a user to point an application at an unconfined cupsd which could be 
+# used to load printer drivers etc. We only want client snaps with the cups 
+# interface plug connected to be able to talk to a version of cupsd which is
+# strictly confined and performs mediation. This means only allowing to talk to
+# /var/cups/cups.sock and not /run/cups/cups.sock since snapd has no way to know
+# if the latter cupsd is confined and performs mediation, but the upstream 
+# maintained cups snap providing a cups slot will always perform mediation.
+# As such, do not use the <abstractions/cups-client> include file here.
+
+# Allow reading the personal settings for cups like default printer, etc.
+owner @{HOME}/.cups/lpoptions r,
+
 /{,var/}run/cups/printcap r,
+
+# Allow talking to the snap version of cupsd socket that we expose via bind 
+# mounts from a snap providing the cups slot to this snap.
+/var/cups/cups.sock rw,
 `
 
+type cupsInterface struct {
+	commonInterface
+}
+
+func (iface *cupsInterface) AppArmorConnectedSlot(spec *apparmor.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error {
+	return nil
+}
+
+func validateCupsSocketDirSlotAttr(a interfaces.Attrer, snapInfo *snap.Info) (string, error) {
+	// Allow an empty specification for the slot, in which case we don't perform
+	// any mounts, etc. This is mainly to prevent errors in systems which still
+	// have the old cups snap installed that haven't been updated to use the new
+	// snap with the new slot declaration
+	if _, ok := a.Lookup("cups-socket-directory"); !ok {
+		return "", nil
+	}
+
+	var cupsdSocketSourceDir string
+	if err := a.Attr("cups-socket-directory", &cupsdSocketSourceDir); err != nil {
+		return "", err
+	}
+
+	// make sure that the cups socket dir is not an AppArmor Regular expression
+	if err := apparmor.ValidateNoAppArmorRegexp(cupsdSocketSourceDir); err != nil {
+		return "", fmt.Errorf("cups-socket-directory is not usable: %v", err)
+	}
+
+	if !cleanSubPath(cupsdSocketSourceDir) {
+		return "", fmt.Errorf("cups-socket-directory is not clean: %q", cupsdSocketSourceDir)
+	}
+
+	// validate that the setting for cups-socket-directory is in $SNAP_DATA or
+	// $SNAP_COMMON, we don't allow any other directories for the slot socket
+	// dir
+	// TODO: should we also allow /run/$SNAP_INSTANCE_NAME/ too ?
+	if !strings.HasPrefix(cupsdSocketSourceDir, "$SNAP_COMMON") && !strings.HasPrefix(cupsdSocketSourceDir, "$SNAP_DATA") {
+		return "", fmt.Errorf("cups-socket-directory must be a directory of $SNAP_COMMON or $SNAP_DATA")
+	}
+	// otherwise it must have a prefix of either SNAP_COMMON or SNAP_DATA,
+	// validate that it has no other variables in it
+	err := snap.ValidatePathVariables(cupsdSocketSourceDir)
+	if err != nil {
+		return "", err
+	}
+
+	// The path starts with $ and ValidatePathVariables() ensures
+	// path contains only $SNAP, $SNAP_DATA, $SNAP_COMMON, and no
+	// other $VARs are present. It is ok to use
+	// ExpandSnapVariables() since it only expands $SNAP, $SNAP_DATA
+	// and $SNAP_COMMON
+	return snapInfo.ExpandSnapVariables(cupsdSocketSourceDir), nil
+}
+
+func (iface *cupsInterface) BeforePrepareSlot(slot *snap.SlotInfo) error {
+	// verify that the snap has a cups-socket-directory interface attribute, which is
+	// needed to identify where to find the cups socket is located in the snap
+	// providing the cups socket
+	_, err := validateCupsSocketDirSlotAttr(slot, slot.Snap)
+	return err
+}
+
+func (iface *cupsInterface) AppArmorConnectedPlug(spec *apparmor.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error {
+	cupsdSocketSourceDir, err := validateCupsSocketDirSlotAttr(slot, slot.Snap())
+	if err != nil {
+		return err
+	}
+
+	// add the base snippet
+	spec.AddSnippet(cupsConnectedPlugAppArmor)
+
+	if cupsdSocketSourceDir == "" {
+		// no other rules, this is the legacy slot without the additional
+		// attribute
+		return nil
+	}
+
+	// add rules to access the socket dir from the slot location directly
+	// this is necessary otherwise clients get denials like this:
+	// apparmor="DENIED" operation="connect"
+	// profile="snap.test-snapd-cups-consumer.bin"
+	// name="/var/snap/test-snapd-cups-provider/common/cups.sock"
+	// pid=3195747 comm="nc" requested_mask="wr" denied_mask="wr" fsuid=0 ouid=0
+	// this denial is the same that would happen for the content interface, so
+	// we employ the same workaround from the content interface here too
+	spec.AddSnippet(fmt.Sprintf(`
+# In addition to the bind mount, add any AppArmor rules so that
+# snaps may directly access the slot implementation's files. Due
+# to a limitation in the kernel's LSM hooks for AF_UNIX, these
+# are needed for using named sockets within the exported
+# directory.
+"%s/**" mrwklix,`, cupsdSocketSourceDir))
+
+	// setup the snap-update-ns rules for bind mounting for the plugging snap
+	emit := spec.AddUpdateNSf
+
+	emit("  # Mount cupsd socket from cups snap to client snap\n")
+	// note the trailing "/" is needed - we ensured that cupsdSocketSourceDir is
+	// clean when we validated it, so it will not have a trailing "/" so we are
+	// safe to add this here
+	emit("  mount options=(rw bind) \"%s/\" -> /var/cups/,\n", cupsdSocketSourceDir)
+	emit("  umount /var/cups/,\n")
+
+	apparmor.GenWritableProfile(emit, cupsdSocketSourceDir, 1)
+	apparmor.GenWritableProfile(emit, "/var/cups", 1)
+
+	return nil
+}
+
+func (iface *cupsInterface) MountConnectedPlug(spec *mount.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error {
+	cupsdSocketSourceDir, err := validateCupsSocketDirSlotAttr(slot, slot.Snap())
+	if err != nil {
+		return err
+	}
+
+	if cupsdSocketSourceDir == "" {
+		// no other rules, this is the legacy slot without the additional
+		// attribute
+		return nil
+	}
+
+	// add a bind mount of the cups-socket-directory to /var/cups of the plugging snap
+	return spec.AddMountEntry(osutil.MountEntry{
+		Name:    cupsdSocketSourceDir,
+		Dir:     "/var/cups/",
+		Options: []string{"bind", "rw"},
+	})
+}
+
 func init() {
-	registerIface(&commonInterface{
-		name:                  "cups",
-		summary:               cupsSummary,
-		implicitOnCore:        false,
-		implicitOnClassic:     false,
-		baseDeclarationSlots:  cupsBaseDeclarationSlots,
-		connectedPlugAppArmor: cupsConnectedPlugAppArmor,
+	registerIface(&cupsInterface{
+		commonInterface: commonInterface{
+			name:                 "cups",
+			summary:              cupsSummary,
+			implicitOnCore:       false,
+			implicitOnClassic:    false,
+			baseDeclarationSlots: cupsBaseDeclarationSlots,
+		},
 	})
 }