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

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,
+		},
 	})
 }